[crypto/ec] for ECC parameters with NULL or zero cofactor, compute it (1.0.2) #9799
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
The cofactor argument to EC_GROUP_set_generator is optional, and SCA mitigations for ECC currently use it. So the library currently falls back to very old SCA-vulnerable code if the cofactor is not present. This PR allows EC_GROUP_set_generator to compute the cofactor for all curves of cryptographic interest. Steering scalar multiplication to more SCA-robust code. This issue affects persisted private keys in explicit parameter form, where the (optional) cofactor field is zero or absent. It also affects curves not built-in to the library, but constructed programatically with explicit parameters, then calling EC_GROUP_set_generator with a nonsensical value (NULL, zero). The very old scalar multiplication code is known to be vulnerable to local uarch attacks, outside of the OpenSSL threat model. New results suggest the code path is also vulnerable to traditional wall clock timing attacks. CVE-2019-1547
…pute it Add CHANGES entry
romen
approved these changes
Sep 7, 2019
Member
romen
left a comment
There was a problem hiding this comment.
LGTM
I also confirmed in gdb that the cofactor is correctly calculated and single point EC_POINT_mul() goes through ec_mul_consttime() as expected.
Member
This has been fixed |
Member
|
I put the |
Contributor
Author
|
Thanks @romen for cleaning up my PRs. Enjoy your vacation :) I carved out some time this morning to finish testing this. The "hack test" with the I wanted to still test manually with a few custom PEMs, since 1.0.2 doesn't have robust So ... ready to go from my behalf. |
This was referenced Sep 9, 2019
mattcaswell
approved these changes
Sep 9, 2019
Member
|
Pushed. Thanks. |
levitte
pushed a commit
that referenced
this pull request
Sep 9, 2019
The cofactor argument to EC_GROUP_set_generator is optional, and SCA mitigations for ECC currently use it. So the library currently falls back to very old SCA-vulnerable code if the cofactor is not present. This PR allows EC_GROUP_set_generator to compute the cofactor for all curves of cryptographic interest. Steering scalar multiplication to more SCA-robust code. This issue affects persisted private keys in explicit parameter form, where the (optional) cofactor field is zero or absent. It also affects curves not built-in to the library, but constructed programatically with explicit parameters, then calling EC_GROUP_set_generator with a nonsensical value (NULL, zero). The very old scalar multiplication code is known to be vulnerable to local uarch attacks, outside of the OpenSSL threat model. New results suggest the code path is also vulnerable to traditional wall clock timing attacks. CVE-2019-1547 Reviewed-by: Nicola Tuveri <nic.tuv@gmail.com> Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from #9799)
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.
Backport of #9727 to 1.0.2.
I still want to do some more testing, but it will have to wait until Sunday night. I thought I'd at least get the PR up for review.