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
Implement blinding for scalar multiplication #11361
Closed
dfaranha
wants to merge
8
commits into
openssl:OpenSSL_1_0_2-stable
from
dfaranha:OpenSSL_1_0_2-stable
Closed
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
103064e
Implement blinding for scalar multiplication to avoid leaking bits of…
dfaranha a776fa0
Extend blinding implementation to prime curves in jacobian coordinates.
dfaranha 8f64f15
Fix case when ec_mul_consttime() is called for binary curves.
dfaranha 070f1a3
Update patch to fix error handling and blind r and s independently.
dfaranha 4f77706
Replace group->order with group->field.
dfaranha 831a58b
Updated comments to reflect PR discussion.
dfaranha d4c7fad
Correct precision for binary field elements, free top bit of random f…
dfaranha ba93463
Generate a proper field element instead of a random bit string.
dfaranha File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains 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
This file contains 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
This file contains 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
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -421,9 +421,80 @@ static int ec_mul_consttime(const EC_GROUP *group, EC_POINT *r, | |
|| (bn_wexpand(&r->Z, group_top) == NULL)) | ||
goto err; | ||
|
||
/* top bit is a 1, in a fixed pos */ | ||
if (!EC_POINT_copy(r, s)) | ||
goto err; | ||
if (group->meth->field_type == NID_X9_62_prime_field) { | ||
/* | ||
* Apply blinding independently to r and s: use point r as temporary | ||
* variable. | ||
* | ||
* Blinding requires using a projective representation, and later we | ||
* implement the ladder step using EC_POINT_add() and EC_POINT_dbl(): | ||
* this requires EC_METHODs that support projective coordinates in their | ||
* point addition and point doubling implementations. | ||
* | ||
* All the built-in EC_METHODs for prime curves at the moment satisfy | ||
* this condition, while the EC_METHOD for binary curves does not. | ||
* | ||
* This is why we check for a prime field to apply blinding. | ||
*/ | ||
|
||
/* first randomize r->Z to blind s. */ | ||
do { | ||
if (!BN_rand(&r->Z, BN_num_bits(&group->field), 0, 0)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Edit: sorry wrong place in the code for that comment :\ Here you want
|
||
ECerr(EC_F_EC_MUL_CONSTTIME, ERR_R_BN_LIB); | ||
goto err; | ||
} | ||
} while (BN_is_zero(&r->Z)); | ||
|
||
/* convert r->Z to the correct field representation. */ | ||
if (group->meth->field_encode != NULL | ||
&& !group->meth->field_encode(group, &r->Z, &r->Z, ctx)) { | ||
goto err; | ||
} | ||
|
||
/* scale s->X and s->Y by r->Z^2 and r->Z^3, respectively. */ | ||
if (!group->meth->field_sqr(group, &r->X, &r->Z, ctx) | ||
|| !group->meth->field_mul(group, &r->Y, &r->X, &r->Z, ctx)) { | ||
goto err; | ||
} | ||
if (!group->meth->field_mul(group, &s->X, &s->X, &r->X, ctx) | ||
|| !group->meth->field_mul(group, &s->Y, &s->Y, &r->Y, ctx) | ||
|| !group->meth->field_mul(group, &s->Z, &s->Z, &r->Z, ctx)) { | ||
goto err; | ||
} | ||
/* mark the flag in s to full projective coordinates. */ | ||
s->Z_is_one = 0; | ||
|
||
/* blinding: now rerandomize r->Z to make r a blinded copy of s. */ | ||
do { | ||
if (!BN_rand(&r->Z, BN_num_bits(&group->field), 0, 0)) { | ||
ECerr(EC_F_EC_MUL_CONSTTIME, ERR_R_BN_LIB); | ||
goto err; | ||
} | ||
} while (BN_is_zero(&r->Z)); | ||
|
||
/* convert r->Z to the correct field representation. */ | ||
if (group->meth->field_encode != NULL | ||
&& !group->meth->field_encode(group, &r->Z, &r->Z, ctx)) { | ||
goto err; | ||
} | ||
|
||
/* scale r->X and r->Y by r->Z^2 and r->Z^3, respectively. */ | ||
if (!group->meth->field_sqr(group, &r->X, &r->Z, ctx) | ||
|| !group->meth->field_mul(group, &r->Y, &r->X, &r->Z, ctx)) { | ||
goto err; | ||
} | ||
if (!group->meth->field_mul(group, &r->X, &s->X, &r->X, ctx) | ||
|| !group->meth->field_mul(group, &r->Y, &s->Y, &r->Y, ctx) | ||
|| !group->meth->field_mul(group, &r->Z, &s->Z, &r->Z, ctx)) { | ||
goto err; | ||
} | ||
/* mark the flag in s to full projective coordinates. */ | ||
r->Z_is_one = 0; | ||
} else { | ||
/* top bit is a 1, in a fixed pos; binary curves are not blinded here */ | ||
if (!EC_POINT_copy(r, s)) | ||
goto err; | ||
} | ||
|
||
EC_POINT_BN_set_flags(r, BN_FLG_CONSTTIME); | ||
|
||
|
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.
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 don't think this is correct. E.g. for B-233 the field polynomial will have 234 bits while field elements have 233 bits. I think it's
BN_rand(z1, BN_num_bits(&group->field) - 1, -1, 0))
where I think the
top=-1
allows the top bit to be random and not fixed.Can you verify in the debugger?
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.
Uhm, good point, let me check!
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.
FWIW this is what we're doing in 111 and master
openssl/crypto/ec/ec2_smpl.c
Lines 731 to 738 in 3cb55fe
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.
BN_num_bits(group->field) - 1
makes total sense. We wanted to fix the top bit to guarantee that z1 and z2 had exactly the same length, but this is indeed not necessary anymore. I can commit and push both fixes shortly.