-
-
Notifications
You must be signed in to change notification settings - Fork 9.9k
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make ECDSA_size() use consistent asn1 encoder. #10577
Conversation
if (group == NULL) | ||
return 0; | ||
|
||
i = EC_GROUP_order_bits(group); | ||
if (i == 0) | ||
bn = EC_GROUP_get0_order(group); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was concerned there might be a off by 1 problem here if the order does not have the top bit set..
There are some curves that have 1FFFFF at the top but those ones should be ok..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
r & s are modulus of the order.. so should be less than this upper value
(There's a typo "ECDA_size()" in the commit message) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM as long as the typo @kaduk spotted is fixed.
Sorry for the delay in the review, I should have finally managed to set up proper notifications when I receive an explicit review request!
855de13
to
a3d100b
Compare
Updated the commit to remove the typo.. |
ping |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reapproved
ECDSA signature lengths are calculated using i2d_ECDSA_SIG(). i2d_ECDSA_SIG() was changed in a previous PR to use a custom ASN1 encoder (using WPACKET) so that the normal ASN1 encoder does not need to be pulled into the provider boundary. For consistency ECDSA_size() has been changed to also use i2d_ECDSA_SIG() - this can now be used directly inside the FIPS provider.
a3d100b
to
1cfc318
Compare
rebased with no new changes to the code. |
ECDSA signature lengths are calculated using i2d_ECDSA_SIG(). i2d_ECDSA_SIG() was changed in a previous PR to use a custom ASN1 encoder (using WPACKET) so that the normal ASN1 encoder does not need to be pulled into the provider boundary. For consistency ECDSA_size() has been changed to also use i2d_ECDSA_SIG() - this can now be used directly inside the FIPS provider. Reviewed-by: Nicola Tuveri <nic.tuv@gmail.com> (Merged from #10577)
Thanks romen. Merged to master. |
ECDSA signature lengths are calculated using i2d_ECDSA_SIG().
i2d_ECDSA_SIG() was changed in a previous PR to use a custom ASN1 encoder (using WPACKET)
so that the normal ASN1 encoder does not need to be pulled into the provider boundary.
For consistency ECDSA_size() has been changed to also use i2d_ECDSA_SIG() - this can now
be used directly inside the FIPS provider.
Checklist