Fix BIO_f_cipher flushing and other BIO related fixes (1.1.1) #19920
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
If an error occurs during a flush on a BIO_f_cipher() then in some cases we could get into an infinite loop. We add a check to make sure we are making progress during flush and exit if not. This issue was reported by Octavio Galland who also demonstrated an infinite loop in CMS encryption as a result of this bug. The security team has assessed this issue as not a CVE. This occurs on *encryption* only which is typically processing trusted data. We are not aware of a way to trigger this with untrusted data.
If the BIO unexpectedly fails to flush then SMIME_crlf_copy() was not correctly reporting the error. We modify it to properly propagate the error condition.
Some things that may go wrong in asn1_bio_write() are serious errors that should be reported as -1, rather than 0 (which just means "we wrote no data").
If the cipher being used in ossl_cms_EncryptedContent_init_bio() has no associated OID then we should report an error rather than continuing on regardless. Continuing on still ends up failing - but later on and with a more cryptic error message.
mattcaswell
added
approval: review pending
mattcaswell
changed the title
|
Lots of CI failures which don't seem related to this PR!!? |
|
I have a couple CI-fixes in my 111 feature branch, but I figured it would not be acceptable for 1.1.1 |
|
wrong buton. sorry! |
t8m
added
triaged: bug
|
I assume backporting the test from 3.0 PR would be too big change as the test_cms recipe has quite evolved since 1.1.1 |
t8m
approved these changes
Dec 16, 2022
hlandau
approved these changes
Dec 16, 2022
hlandau
added
approval: done
openssl-machine
added
approval: ready to merge
|
This pull request is ready to merge |
|
Merged to 1.1.1 branch. |
openssl-machine
pushed a commit
that referenced
this pull request
Dec 22, 2022
If an error occurs during a flush on a BIO_f_cipher() then in some cases we could get into an infinite loop. We add a check to make sure we are making progress during flush and exit if not. This issue was reported by Octavio Galland who also demonstrated an infinite loop in CMS encryption as a result of this bug. The security team has assessed this issue as not a CVE. This occurs on *encryption* only which is typically processing trusted data. We are not aware of a way to trigger this with untrusted data. Reviewed-by: Hugo Landau <hlandau@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from #19920)
openssl-machine
pushed a commit
that referenced
this pull request
Dec 22, 2022
If the BIO unexpectedly fails to flush then SMIME_crlf_copy() was not correctly reporting the error. We modify it to properly propagate the error condition. Reviewed-by: Hugo Landau <hlandau@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from #19920)
openssl-machine
pushed a commit
that referenced
this pull request
Dec 22, 2022
Some things that may go wrong in asn1_bio_write() are serious errors that should be reported as -1, rather than 0 (which just means "we wrote no data"). Reviewed-by: Hugo Landau <hlandau@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from #19920)
openssl-machine
pushed a commit
that referenced
this pull request
Dec 22, 2022
If the cipher being used in ossl_cms_EncryptedContent_init_bio() has no associated OID then we should report an error rather than continuing on regardless. Continuing on still ends up failing - but later on and with a more cryptic error message. Reviewed-by: Hugo Landau <hlandau@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from #19920)
|
Hmm, I think it seems pointless to back-port commit 1354191, |
bernd-edlinger
pushed a commit
to bernd-edlinger/openssl
that referenced
this pull request
Dec 25, 2022
If an error occurs during a flush on a BIO_f_cipher() then in some cases we could get into an infinite loop. We add a check to make sure we are making progress during flush and exit if not. This issue was reported by Octavio Galland who also demonstrated an infinite loop in CMS encryption as a result of this bug. The security team has assessed this issue as not a CVE. This occurs on *encryption* only which is typically processing trusted data. We are not aware of a way to trigger this with untrusted data. Reviewed-by: Hugo Landau <hlandau@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from openssl#19920) (cherry picked from commit e913b91)
bernd-edlinger
pushed a commit
to bernd-edlinger/openssl
that referenced
this pull request
Dec 25, 2022
If an error occurs during a flush on a BIO_f_cipher() then in some cases we could get into an infinite loop. We add a check to make sure we are making progress during flush and exit if not. This issue was reported by Octavio Galland who also demonstrated an infinite loop in CMS encryption as a result of this bug. The security team has assessed this issue as not a CVE. This occurs on *encryption* only which is typically processing trusted data. We are not aware of a way to trigger this with untrusted data. Reviewed-by: Hugo Landau <hlandau@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from openssl#19920) (cherry picked from commit e913b91)
bernd-edlinger
pushed a commit
to bernd-edlinger/openssl
that referenced
this pull request
Dec 25, 2022
If an error occurs during a flush on a BIO_f_cipher() then in some cases we could get into an infinite loop. We add a check to make sure we are making progress during flush and exit if not. This issue was reported by Octavio Galland who also demonstrated an infinite loop in CMS encryption as a result of this bug. The security team has assessed this issue as not a CVE. This occurs on *encryption* only which is typically processing trusted data. We are not aware of a way to trigger this with untrusted data. Reviewed-by: Hugo Landau <hlandau@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from openssl#19920) (cherry picked from commit e913b91)
bernd-edlinger
pushed a commit
to bernd-edlinger/openssl
that referenced
this pull request
Dec 25, 2022
If an error occurs during a flush on a BIO_f_cipher() then in some cases we could get into an infinite loop. We add a check to make sure we are making progress during flush and exit if not. This issue was reported by Octavio Galland who also demonstrated an infinite loop in CMS encryption as a result of this bug. The security team has assessed this issue as not a CVE. This occurs on *encryption* only which is typically processing trusted data. We are not aware of a way to trigger this with untrusted data. Reviewed-by: Hugo Landau <hlandau@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from openssl#19920) (cherry picked from commit e913b91)
bernd-edlinger
pushed a commit
to bernd-edlinger/openssl
that referenced
this pull request
Dec 25, 2022
If an error occurs during a flush on a BIO_f_cipher() then in some cases we could get into an infinite loop. We add a check to make sure we are making progress during flush and exit if not. This issue was reported by Octavio Galland who also demonstrated an infinite loop in CMS encryption as a result of this bug. The security team has assessed this issue as not a CVE. This occurs on *encryption* only which is typically processing trusted data. We are not aware of a way to trigger this with untrusted data. Reviewed-by: Hugo Landau <hlandau@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from openssl#19920) (cherry picked from commit e913b91)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If an error occurs during a flush on a BIO_f_cipher() then in some cases
we could get into an infinite loop. We add a check to make sure we are
making progress during flush and exit if not.
This issue was reported by Octavio Galland who also demonstrated an
infinite loop in CMS encryption as a result of this bug.
The security team has assessed this issue as not a CVE. This occurs on
encryption only which is typically processing trusted data. We are not
aware of a way to trigger this with untrusted data.
Backport of #19918 for 1.1.1.