Mark pop/clear error stack in der2key_decode_p8 #15067
Closed
Conversation
This file contains hidden or 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
This commit sets the error mark before calling d2i_X509_SIG and clear it if that function call is successful. The motivation for this is that if d2i_X509_SIG returns NULL then the else clause will be entered and d2i_PKCS8_PRIV_KEY_INFO will be called. If d2i_X509_SIG raised any errors those error will be on the error stack when d2i_PKCS8_PRIV_KEY_INFO gets called, and even if it returns successfully those errors will still be on the error stack. We ran into this issue when upgrading Node.js to 3.0.0-alpha15. More details can be found in the ref links below. Refs: nodejs/node#38373 Refs: https://github.com/danbev/learning-libcrypto/blob/master/notes/wrong-tag-issue2.md
t8m
requested changes
Apr 28, 2021
Fix comment style and declaration warning.
t8m
reviewed
Apr 28, 2021
Move endif macro.
Add empty line after declaration.
Fix address sanitizer failure.
levitte
approved these changes
Apr 28, 2021
Contributor
Author
|
I've tried to reproduce the failing address sanitizer test locally but I've not been able to reproduce these failures. |
t8m
reviewed
Apr 28, 2021
t8m
reviewed
Apr 28, 2021
Remove PROVIDER and checks that are already checked in test_privatekey_to_pkcs8.
Move testctx check to after declarations.
t8m
approved these changes
Apr 29, 2021
Member
|
@levitte still OK? |
Member
|
ping @levitte for re-review |
kaduk
approved these changes
May 8, 2021
levitte
approved these changes
May 9, 2021
Collaborator
|
This pull request is ready to merge |
openssl-machine
pushed a commit
that referenced
this pull request
May 9, 2021
This commit sets the error mark before calling d2i_X509_SIG and clear it if that function call is successful. The motivation for this is that if d2i_X509_SIG returns NULL then the else clause will be entered and d2i_PKCS8_PRIV_KEY_INFO will be called. If d2i_X509_SIG raised any errors those error will be on the error stack when d2i_PKCS8_PRIV_KEY_INFO gets called, and even if it returns successfully those errors will still be on the error stack. We ran into this issue when upgrading Node.js to 3.0.0-alpha15. More details can be found in the ref links below. Refs: nodejs/node#38373 Refs: https://github.com/danbev/learning-libcrypto/blob/master/notes/wrong-tag-issue2.md Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Ben Kaduk <kaduk@mit.edu> (Merged from #15067)
Contributor
|
Pushed to master, so closing. |
devnexen
pushed a commit
to devnexen/openssl
that referenced
this pull request
Jul 7, 2021
This commit sets the error mark before calling d2i_X509_SIG and clear it if that function call is successful. The motivation for this is that if d2i_X509_SIG returns NULL then the else clause will be entered and d2i_PKCS8_PRIV_KEY_INFO will be called. If d2i_X509_SIG raised any errors those error will be on the error stack when d2i_PKCS8_PRIV_KEY_INFO gets called, and even if it returns successfully those errors will still be on the error stack. We ran into this issue when upgrading Node.js to 3.0.0-alpha15. More details can be found in the ref links below. Refs: nodejs/node#38373 Refs: https://github.com/danbev/learning-libcrypto/blob/master/notes/wrong-tag-issue2.md Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Ben Kaduk <kaduk@mit.edu> (Merged from openssl#15067)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
This commit sets the error mark before calling
d2i_X509_SIGand clear it if that function call is successful.
The motivation for this is that if
d2i_X509_SIGreturns NULL then theelse clause will be entered and
d2i_PKCS8_PRIV_KEY_INFOwill becalled. If
d2i_X509_SIGraised any errors those error will be on theerror stack when
d2i_PKCS8_PRIV_KEY_INFOgets called, and even if itreturns successfully those errors will still be on the error stack.
We ran into this issue when upgrading Node.js to 3.0.0-alpha15.
More details can be found in the ref links below.
Refs: nodejs/node#38373
Refs: https://github.com/danbev/learning-libcrypto/blob/master/notes/wrong-tag-issue2.md
Checklist