Remove TODO in rsa_ameth.c #14524
Closed
Remove TODO in rsa_ameth.c #14524
Conversation
Fixes #14390 The only caller of this function tests EVP_KEYMGMT_is_a() beforehand which will fail if the RSA key types do not match. So the test is not necessary. The assert has been removed when it does the test.
| @@ -1714,7 +1714,7 @@ void *evp_pkey_export_to_provider(EVP_PKEY *pk, OSSL_LIB_CTX *libctx, | |||
| } | |||
|
|
|||
| /* Make sure that the keymgmt key type matches the legacy NID */ | |||
| if (!ossl_assert(EVP_KEYMGMT_is_a(tmp_keymgmt, OBJ_nid2sn(pk->type)))) | |||
| if (!EVP_KEYMGMT_is_a(tmp_keymgmt, OBJ_nid2sn(pk->type))) | |||
levitte
Mar 12, 2021
Member
Any reason why you removed the assertion?
Any reason why you removed the assertion?
levitte
Mar 12, 2021
Member
Aha, was it possibly for the added negative test in test/keymgmt_internal/test.c? Ok, I've no concerns then...
Aha, was it possibly for the added negative test in test/keymgmt_internal/test.c? Ok, I've no concerns then...
|
24 hours has passed since 'approval: done' was set, but this PR has failing CI tests. Once the tests pass it will get moved to 'approval: ready to merge' automatically, alternatively please review and set the label manually. |
|
This pull request is ready to merge |
|
Merged to master. |
openssl-machine
pushed a commit
that referenced
this pull request
Mar 14, 2021
Fixes #14390 The only caller of this function tests EVP_KEYMGMT_is_a() beforehand which will fail if the RSA key types do not match. So the test is not necessary. The assert has been removed when it does the test. Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Paul Dale <pauli@openssl.org> (Merged from #14524)
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.
Fixes #14390
The only caller of this function tests EVP_KEYMGMT_is_a() beforehand
which will fail if the RSA key types do not match. So the test is not
necessary. The assert has been removed when it does the test.
Checklist