From 5c0e0cec83a2c30eee4213b4f90112a46c1843df Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Mon, 23 Apr 2018 18:33:20 -0400 Subject: [PATCH] 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 --- crypto/fipsmodule/ec/ec_montgomery.c | 106 ++++++++++++--------------- 1 file changed, 48 insertions(+), 58 deletions(-) diff --git a/crypto/fipsmodule/ec/ec_montgomery.c b/crypto/fipsmodule/ec/ec_montgomery.c index 9ede5a10..d80fa23c 100644 --- a/crypto/fipsmodule/ec/ec_montgomery.c +++ b/crypto/fipsmodule/ec/ec_montgomery.c @@ -184,68 +184,58 @@ static int ec_GFp_mont_point_get_affine_coordinates(const EC_GROUP *group, BN_CTX_start(ctx); - if (BN_cmp(&point->Z, &group->one) == 0) { - // |point| is already affine. - if (x != NULL && !BN_from_montgomery(x, &point->X, group->mont, ctx)) { + // 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 && !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; } - } 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;