Remove Z = 1 special-case in generic point_get_affine.

As the point may be the output of some private key operation, whether Z
accidentally hit one is secret.

Bug: 239
Change-Id: I7db34cd3b5dd5ca4b96980e8993a9b4eda49eb88
Reviewed-on: https://boringssl-review.googlesource.com/27664
Reviewed-by: Adam Langley <alangley@gmail.com>
This commit is contained in:
David Benjamin 2018-04-23 18:33:20 -04:00 committed by Adam Langley
parent f5858ca008
commit 5c0e0cec83

View File

@ -184,68 +184,58 @@ static int ec_GFp_mont_point_get_affine_coordinates(const EC_GROUP *group,
BN_CTX_start(ctx); BN_CTX_start(ctx);
if (BN_cmp(&point->Z, &group->one) == 0) { // transform (X, Y, Z) into (x, y) := (X/Z^2, Y/Z^3)
// |point| is already affine.
if (x != NULL && !BN_from_montgomery(x, &point->X, group->mont, ctx)) { BIGNUM *Z_1 = BN_CTX_get(ctx);
BIGNUM *Z_2 = BN_CTX_get(ctx);
BIGNUM *Z_3 = BN_CTX_get(ctx);
if (Z_1 == NULL ||
Z_2 == NULL ||
Z_3 == NULL) {
goto err;
}
// The straightforward way to calculate the inverse of a Montgomery-encoded
// value where the result is Montgomery-encoded is:
//
// |BN_from_montgomery| + invert + |BN_to_montgomery|.
//
// This is equivalent, but more efficient, because |BN_from_montgomery|
// is more efficient (at least in theory) than |BN_to_montgomery|, since it
// doesn't have to do the multiplication before the reduction.
//
// Use Fermat's Little Theorem instead of |BN_mod_inverse_odd| since this
// inversion may be done as the final step of private key operations.
// Unfortunately, this is suboptimal for ECDSA verification.
if (!BN_from_montgomery(Z_1, &point->Z, group->mont, ctx) ||
!BN_from_montgomery(Z_1, Z_1, group->mont, ctx) ||
!bn_mod_inverse_prime(Z_1, Z_1, &group->field, ctx, group->mont)) {
goto err;
}
if (!BN_mod_mul_montgomery(Z_2, Z_1, Z_1, group->mont, ctx)) {
goto err;
}
// Instead of using |BN_from_montgomery| to convert the |x| coordinate
// and then calling |BN_from_montgomery| again to convert the |y|
// coordinate below, convert the common factor |Z_2| once now, saving one
// reduction.
if (!BN_from_montgomery(Z_2, Z_2, group->mont, ctx)) {
goto err;
}
if (x != NULL) {
if (!BN_mod_mul_montgomery(x, &point->X, Z_2, group->mont, ctx)) {
goto err; goto err;
} }
if (y != NULL && !BN_from_montgomery(y, &point->Y, group->mont, ctx)) { }
if (y != NULL) {
if (!BN_mod_mul_montgomery(Z_3, Z_2, Z_1, group->mont, ctx) ||
!BN_mod_mul_montgomery(y, &point->Y, Z_3, group->mont, ctx)) {
goto err; goto err;
} }
} else {
// transform (X, Y, Z) into (x, y) := (X/Z^2, Y/Z^3)
BIGNUM *Z_1 = BN_CTX_get(ctx);
BIGNUM *Z_2 = BN_CTX_get(ctx);
BIGNUM *Z_3 = BN_CTX_get(ctx);
if (Z_1 == NULL ||
Z_2 == NULL ||
Z_3 == NULL) {
goto err;
}
// The straightforward way to calculate the inverse of a Montgomery-encoded
// value where the result is Montgomery-encoded is:
//
// |BN_from_montgomery| + invert + |BN_to_montgomery|.
//
// This is equivalent, but more efficient, because |BN_from_montgomery|
// is more efficient (at least in theory) than |BN_to_montgomery|, since it
// doesn't have to do the multiplication before the reduction.
//
// Use Fermat's Little Theorem instead of |BN_mod_inverse_odd| since this
// inversion may be done as the final step of private key operations.
// Unfortunately, this is suboptimal for ECDSA verification.
if (!BN_from_montgomery(Z_1, &point->Z, group->mont, ctx) ||
!BN_from_montgomery(Z_1, Z_1, group->mont, ctx) ||
!bn_mod_inverse_prime(Z_1, Z_1, &group->field, ctx, group->mont)) {
goto err;
}
if (!BN_mod_mul_montgomery(Z_2, Z_1, Z_1, group->mont, ctx)) {
goto err;
}
// Instead of using |BN_from_montgomery| to convert the |x| coordinate
// and then calling |BN_from_montgomery| again to convert the |y|
// coordinate below, convert the common factor |Z_2| once now, saving one
// reduction.
if (!BN_from_montgomery(Z_2, Z_2, group->mont, ctx)) {
goto err;
}
if (x != NULL) {
if (!BN_mod_mul_montgomery(x, &point->X, Z_2, group->mont, ctx)) {
goto err;
}
}
if (y != NULL) {
if (!BN_mod_mul_montgomery(Z_3, Z_2, Z_1, group->mont, ctx) ||
!BN_mod_mul_montgomery(y, &point->Y, Z_3, group->mont, ctx)) {
goto err;
}
}
} }
ret = 1; ret = 1;