diff --git a/src/crypto/fipsmodule/ec/ec.c b/src/crypto/fipsmodule/ec/ec.c index 2195d065f..04f2a983d 100644 --- a/src/crypto/fipsmodule/ec/ec.c +++ b/src/crypto/fipsmodule/ec/ec.c @@ -1074,8 +1074,10 @@ int ec_point_mul_scalar_base(const EC_GROUP *group, EC_JACOBIAN *r, group->meth->mul_base(group, r, scalar); // Check the result is on the curve to defend against fault attacks or bugs. - // This has negligible cost compared to the multiplication. - if (!ec_GFp_simple_is_on_curve(group, r)) { + // This has negligible cost compared to the multiplication. This can only + // happen on bug or CPU fault, so it okay to leak this. The alternative would + // be to proceed with bad data. + if (!constant_time_declassify_int(ec_GFp_simple_is_on_curve(group, r))) { OPENSSL_PUT_ERROR(EC, ERR_R_INTERNAL_ERROR); return 0; } diff --git a/src/crypto/fipsmodule/ec/ec_montgomery.c b/src/crypto/fipsmodule/ec/ec_montgomery.c index eeaee64ca..78e050769 100644 --- a/src/crypto/fipsmodule/ec/ec_montgomery.c +++ b/src/crypto/fipsmodule/ec/ec_montgomery.c @@ -177,7 +177,8 @@ void ec_GFp_mont_felem_exp(const EC_GROUP *group, EC_FELEM *out, static int ec_GFp_mont_point_get_affine_coordinates(const EC_GROUP *group, const EC_JACOBIAN *point, EC_FELEM *x, EC_FELEM *y) { - if (ec_GFp_simple_is_at_infinity(group, point)) { + if (constant_time_declassify_int( + ec_GFp_simple_is_at_infinity(group, point))) { OPENSSL_PUT_ERROR(EC, EC_R_POINT_AT_INFINITY); return 0; } @@ -317,7 +318,7 @@ void ec_GFp_mont_add(const EC_GROUP *group, EC_JACOBIAN *out, // This case will never occur in the constant-time |ec_GFp_mont_mul|. BN_ULONG is_nontrivial_double = ~xneq & ~yneq & z1nz & z2nz; - if (is_nontrivial_double) { + if (constant_time_declassify_w(is_nontrivial_double)) { ec_GFp_mont_dbl(group, out, a); return; } diff --git a/src/crypto/fipsmodule/ec/internal.h b/src/crypto/fipsmodule/ec/internal.h index 853202638..bb4181515 100644 --- a/src/crypto/fipsmodule/ec/internal.h +++ b/src/crypto/fipsmodule/ec/internal.h @@ -479,7 +479,8 @@ struct ec_method_st { // point_get_affine_coordinates sets |*x| and |*y| to the affine coordinates // of |p|. Either |x| or |y| may be NULL to omit it. It returns one on success - // and zero if |p| is the point at infinity. + // and zero if |p| is the point at infinity. It leaks whether |p| was the + // point at infinity, but otherwise treats |p| as secret. int (*point_get_affine_coordinates)(const EC_GROUP *, const EC_JACOBIAN *p, EC_FELEM *x, EC_FELEM *y); diff --git a/src/crypto/fipsmodule/ec/p224-64.c b/src/crypto/fipsmodule/ec/p224-64.c index ef83b2912..b646e8216 100644 --- a/src/crypto/fipsmodule/ec/p224-64.c +++ b/src/crypto/fipsmodule/ec/p224-64.c @@ -734,8 +734,8 @@ static void p224_point_add(p224_felem x3, p224_felem y3, p224_felem z3, // tmp[i] < 2^116 + 2^64 + 8 < 2^117 p224_felem_reduce(ftmp, tmp); - // the formulae are incorrect if the points are equal - // so we check for this and do doubling if this happens + // The formulae are incorrect if the points are equal, so we check for this + // and do doubling if this happens. x_equal = p224_felem_is_zero(ftmp); y_equal = p224_felem_is_zero(ftmp3); z1_is_zero = p224_felem_is_zero(z1); @@ -743,7 +743,7 @@ static void p224_point_add(p224_felem x3, p224_felem y3, p224_felem z3, // In affine coordinates, (X_1, Y_1) == (X_2, Y_2) p224_limb is_nontrivial_double = x_equal & y_equal & (1 - z1_is_zero) & (1 - z2_is_zero); - if (is_nontrivial_double) { + if (constant_time_declassify_w(is_nontrivial_double)) { p224_point_double(x3, y3, z3, x1, y1, z1); return; } @@ -862,7 +862,8 @@ static crypto_word_t p224_get_bit(const EC_SCALAR *in, size_t i) { static int ec_GFp_nistp224_point_get_affine_coordinates( const EC_GROUP *group, const EC_JACOBIAN *point, EC_FELEM *x, EC_FELEM *y) { - if (ec_GFp_simple_is_at_infinity(group, point)) { + if (constant_time_declassify_int( + ec_GFp_simple_is_at_infinity(group, point))) { OPENSSL_PUT_ERROR(EC, EC_R_POINT_AT_INFINITY); return 0; } diff --git a/src/crypto/fipsmodule/ec/p256-nistz.c b/src/crypto/fipsmodule/ec/p256-nistz.c index dfde2f48f..85343fd95 100644 --- a/src/crypto/fipsmodule/ec/p256-nistz.c +++ b/src/crypto/fipsmodule/ec/p256-nistz.c @@ -422,7 +422,8 @@ static void ecp_nistz256_points_mul_public(const EC_GROUP *group, static int ecp_nistz256_get_affine(const EC_GROUP *group, const EC_JACOBIAN *point, EC_FELEM *x, EC_FELEM *y) { - if (ec_GFp_simple_is_at_infinity(group, point)) { + if (constant_time_declassify_int( + ec_GFp_simple_is_at_infinity(group, point))) { OPENSSL_PUT_ERROR(EC, EC_R_POINT_AT_INFINITY); return 0; } diff --git a/src/crypto/fipsmodule/ec/p256.c b/src/crypto/fipsmodule/ec/p256.c index af211be33..76e865f33 100644 --- a/src/crypto/fipsmodule/ec/p256.c +++ b/src/crypto/fipsmodule/ec/p256.c @@ -324,7 +324,7 @@ static void fiat_p256_point_add(fiat_p256_felem x3, fiat_p256_felem y3, fiat_p256_limb_t is_nontrivial_double = constant_time_is_zero_w(xneq | yneq) & ~constant_time_is_zero_w(z1nz) & ~constant_time_is_zero_w(z2nz); - if (is_nontrivial_double) { + if (constant_time_declassify_w(is_nontrivial_double)) { fiat_p256_point_double(x3, y3, z3, x1, y1, z1); return; } @@ -416,7 +416,8 @@ static crypto_word_t fiat_p256_get_bit(const EC_SCALAR *in, int i) { static int ec_GFp_nistp256_point_get_affine_coordinates( const EC_GROUP *group, const EC_JACOBIAN *point, EC_FELEM *x_out, EC_FELEM *y_out) { - if (ec_GFp_simple_is_at_infinity(group, point)) { + if (constant_time_declassify_int( + ec_GFp_simple_is_at_infinity(group, point))) { OPENSSL_PUT_ERROR(EC, EC_R_POINT_AT_INFINITY); return 0; } diff --git a/src/crypto/fipsmodule/ecdsa/ecdsa.c b/src/crypto/fipsmodule/ecdsa/ecdsa.c index be1954066..1be147f93 100644 --- a/src/crypto/fipsmodule/ecdsa/ecdsa.c +++ b/src/crypto/fipsmodule/ecdsa/ecdsa.c @@ -223,7 +223,7 @@ static ECDSA_SIG *ecdsa_sign_impl(const EC_GROUP *group, int *out_retry, return NULL; } - if (ec_scalar_is_zero(group, &r)) { + if (constant_time_declassify_int(ec_scalar_is_zero(group, &r))) { *out_retry = 1; return NULL; } @@ -250,11 +250,13 @@ static ECDSA_SIG *ecdsa_sign_impl(const EC_GROUP *group, int *out_retry, ec_scalar_inv0_montgomery(group, &tmp, k); // tmp = k^-1 R^2 ec_scalar_from_montgomery(group, &tmp, &tmp); // tmp = k^-1 R ec_scalar_mul_montgomery(group, &s, &s, &tmp); - if (ec_scalar_is_zero(group, &s)) { + if (constant_time_declassify_int(ec_scalar_is_zero(group, &s))) { *out_retry = 1; return NULL; } + CONSTTIME_DECLASSIFY(r.words, sizeof(r.words)); + CONSTTIME_DECLASSIFY(s.words, sizeof(r.words)); ECDSA_SIG *ret = ECDSA_SIG_new(); if (ret == NULL || // !bn_set_words(ret->r, r.words, order->width) || @@ -347,6 +349,10 @@ ECDSA_SIG *ECDSA_do_sign(const uint8_t *digest, size_t digest_len, goto out; } + // TODO(davidben): Move this inside |ec_random_nonzero_scalar| or lower, so + // that all scalars we generate are, by default, secret. + CONSTTIME_SECRET(k.words, sizeof(k.words)); + int retry; ret = ecdsa_sign_impl(group, &retry, priv_key, &k, digest, digest_len); if (ret != NULL || !retry) {