Fix dh_rfc5114 option in genpkey. #14883
Conversation
slontis
added
approval: review pending
slontis
force-pushed
the
dh_rfc5114_fix
branch
2 times, most recently
from
285657e
to
31df095
Compare
Fixes openssl#14145 Fixes openssl#13956 Fixes openssl#13952 Fixes openssl#13871 Fixes openssl#14054 Fixes openssl#14444 Updated documentation for app to indicate what options are available for DH and DHX keys. DH and DHX now have different keymanager gen_set_params() methods. Added CHANGES entry to indicate the breaking change.
|
Quite a few changes in the last 3 commits - had to rebase to update the commit message.. |
|
This errors in the setparams if bad parameters are passed in. If we dont do this then potentially it would do something different from what was expected. |
|
The CI failures are relevatnt. |
|
(Except for the external-tests, that is a different thing) |
| @@ -113,9 +113,7 @@ const DH_NAMED_GROUP *ossl_ffc_numbers_to_dh_named_group(const BIGNUM *p, | |||
| if (BN_cmp(p, dh_named_groups[i].p) == 0 | |||
| && BN_cmp(g, dh_named_groups[i].g) == 0 | |||
| /* Verify q is correct if it exists */ | |||
| && ((q != NULL && BN_cmp(q, dh_named_groups[i].q) == 0) | |||
| /* Do not match RFC 5114 groups without q */ | |||
| || (q == NULL && dh_named_groups[i].uid > 3))) | |||
There was a problem hiding this comment.
NOTE - this line was a hack to fix a decoder test issue - the root cause was fixed so this is no longer needed. This allows RFC5114 to be able to be loaded correctly as a named group. (The original problem was that the DH asn1 callback was not setting the nid - but it was for DHX asn1)
|
NITS addressed |
t8m
added
approval: done
openssl-machine
added
approval: ready to merge
|
This pull request is ready to merge |
Fix dh_rfc5114 option in genpkey. Fixes #14145 Fixes #13956 Fixes #13952 Fixes #13871 Fixes #14054 Fixes #14444 Updated documentation for app to indicate what options are available for DH and DHX keys. DH and DHX now have different keymanager gen_set_params() methods. Added CHANGES entry to indicate the breaking change. Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from #14883)
|
Merged to master. I've added a new title line of the commit message as the original one did not really cover all the change. Thank you for the contribution, Shane! |
Updated documentation for app to indicate what options are available for
DH and DHX keys. Added CHANGES entry to indicate the breaking change.
Checklist