Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork33.7k
gh-100372: Use BIO_eof to detect EOF for SSL_FILETYPE_ASN1#100373
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
In PEM, we need to parse until error and then suppressPEM_R_NO_START_LINE, because PEM allows arbitrary leading and trailingdata. DER, however, does not. Parsing until error and suppressingASN1_R_HEADER_TOO_LONG doesn't quite work because that error alsocovers some cases that should be rejected.Instead, check BIO_eof early and stop the loop that way.
davidben commentedMar 16, 2023
@tiran This PR look reasonable? Anything missing on my end? |
Yhg1s commentedMar 24, 2023
I don't think this is a security issue, or at least not serious enough to backport to security-only releases. Do you disagree,@davidben, or is there a security angle I'm missing? I'm not sure if this should be backported to 3.11/3.10 either. It's a bug, but it doesn't feel important enough to backport and risk breaking users who rely on the old broken behaviour. |
miss-islington commentedMar 24, 2023
Status check is done, and it's a success ✅. |
davidben commentedMar 24, 2023
Nah, can't think of any security angle. Just generally improving behavior and reducing dependency on OpenSSL error codes. (Conditioning on OpenSSL error codes can be a bit messy. Sometimes you have to, like the PEM case here, but other times the error codes don't correspond enough to clear, stable conditions to condition on. :-( ) |
…honGH-100373)In PEM, we need to parse until error and then suppress `PEM_R_NO_START_LINE`, because PEM allows arbitrary leading and trailing data. DER, however, does not. Parsing until error and suppressing `ASN1_R_HEADER_TOO_LONG` doesn't quite work because that error also covers some cases that should be rejected.Instead, check `BIO_eof` early and stop the loop that way.Automerge-Triggered-By: GH:Yhg1s
…honGH-100373)In PEM, we need to parse until error and then suppress `PEM_R_NO_START_LINE`, because PEM allows arbitrary leading and trailing data. DER, however, does not. Parsing until error and suppressing `ASN1_R_HEADER_TOO_LONG` doesn't quite work because that error also covers some cases that should be rejected.Instead, check `BIO_eof` early and stop the loop that way.Automerge-Triggered-By: GH:Yhg1s
Uh oh!
There was an error while loading.Please reload this page.
In PEM, we need to parse until error and then suppress
PEM_R_NO_START_LINE, because PEM allows arbitrary leading and trailing data. DER, however, does not. Parsing until error and suppressingASN1_R_HEADER_TOO_LONGdoesn't quite work because that error also covers some cases that should be rejected.Instead, check
BIO_eofearly and stop the loop that way.Automerge-Triggered-By: GH:Yhg1s