Fix miscellaneous engine problems#16846
Closed
mattcaswell wants to merge 10 commits intoopenssl:masterfrom
Closed
Conversation
Ciphers in the daysnc engine were failing to copy their context properly in the event of EVP_CIPHER_CTX_copy() because they did not define the flag EVP_CIPH_CUSTOM_FLAG Fixes openssl#16844
pkey_set_type should not consume the ENGINE references that may be passed to it. Fixes openssl#16757
provider_util.c failed to free ENGINE references when clearing a cipher or a digest. Additionally ciphers and digests were not copied correctly, which would lead to double-frees if it were not for the previously mentioned leaks. Fixes openssl#16845
Add some tests which would have caught the issues fixed in the previous 3 commits related to engine handling.
mattcaswell
added
branch: master
Member
Author
|
CIs for this are now green. Please take a look. |
Member
Author
|
All nits fixed and fixups pushed. Please take another look. |
t8m
approved these changes
Oct 18, 2021
t8m
added
approval: done
openssl-machine
added
approval: ready to merge
Collaborator
|
This pull request is ready to merge |
openssl-machine
pushed a commit
that referenced
this pull request
Oct 19, 2021
Ciphers in the daysnc engine were failing to copy their context properly in the event of EVP_CIPHER_CTX_copy() because they did not define the flag EVP_CIPH_CUSTOM_FLAG Fixes #16844 Reviewed-by: Tomas Mraz <[email protected]> (Merged from #16846)
openssl-machine
pushed a commit
that referenced
this pull request
Oct 19, 2021
pkey_set_type should not consume the ENGINE references that may be passed to it. Fixes #16757 Reviewed-by: Tomas Mraz <[email protected]> (Merged from #16846)
openssl-machine
pushed a commit
that referenced
this pull request
Oct 19, 2021
provider_util.c failed to free ENGINE references when clearing a cipher or a digest. Additionally ciphers and digests were not copied correctly, which would lead to double-frees if it were not for the previously mentioned leaks. Fixes #16845 Reviewed-by: Tomas Mraz <[email protected]> (Merged from #16846)
openssl-machine
pushed a commit
that referenced
this pull request
Oct 19, 2021
Ciphers in the daysnc engine were failing to copy their context properly in the event of EVP_CIPHER_CTX_copy() because they did not define the flag EVP_CIPH_CUSTOM_FLAG Fixes #16844 Reviewed-by: Tomas Mraz <[email protected]> (Merged from #16846) (cherry picked from commit a0cbc2d)
openssl-machine
pushed a commit
that referenced
this pull request
Oct 19, 2021
Add some tests which would have caught the issues fixed in the previous 3 commits related to engine handling. Reviewed-by: Tomas Mraz <[email protected]> (Merged from #16846)
openssl-machine
pushed a commit
that referenced
this pull request
Oct 19, 2021
pkey_set_type should not consume the ENGINE references that may be passed to it. Fixes #16757 Reviewed-by: Tomas Mraz <[email protected]> (Merged from #16846) (cherry picked from commit f7d6868)
openssl-machine
pushed a commit
that referenced
this pull request
Oct 19, 2021
provider_util.c failed to free ENGINE references when clearing a cipher or a digest. Additionally ciphers and digests were not copied correctly, which would lead to double-frees if it were not for the previously mentioned leaks. Fixes #16845 Reviewed-by: Tomas Mraz <[email protected]> (Merged from #16846) (cherry picked from commit 86c15ba)
openssl-machine
pushed a commit
that referenced
this pull request
Oct 19, 2021
Add some tests which would have caught the issues fixed in the previous 3 commits related to engine handling. Reviewed-by: Tomas Mraz <[email protected]> (Merged from #16846) (cherry picked from commit 0299094)
Member
Author
|
Pushed to master and 3.0. Thanks. |
This was referenced Oct 19, 2021
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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 PR fixes 3 different problems:
Fixes #16757
Fixes #16845
Fixes #16844
This PR is targeting master and 3.0. A backport will need to be done for 1.1.1 (not all issues apply, and some tweaks will be required).
Note: investigating these issues highlighted that we have a behaviour change between 1.1.1 and 3.0 wrt CMAC keys. In 1.1.1 if you attempt to create a CMAC key with EVP_PKEY_new_CMAC_key(), but supply key data that is too short, then we notice this right away. However in 3.0 we don't notice it until you actually try and use the key. This means we'll need to tweak my test during backporting as well.