From 61c633fbc27f10b9e38c02f97e1eeab0e273d906 Mon Sep 17 00:00:00 2001 From: Tomas Mraz Date: Thu, 11 Apr 2024 13:10:09 +0200 Subject: [PATCH 1/8] Make BN_generate_dsa_nonce() constant time and non-biased Co-authored-by: Paul Dale --- crypto/bn/bn_lib.c | 34 ++++++++++++-- crypto/bn/bn_local.h | 1 + crypto/bn/bn_rand.c | 78 +++++++++++++++++++------------- include/internal/constant_time.h | 12 +++++ 4 files changed, 89 insertions(+), 36 deletions(-) diff --git a/crypto/bn/bn_lib.c b/crypto/bn/bn_lib.c index 9070647b35ea2..85698885edd09 100644 --- a/crypto/bn/bn_lib.c +++ b/crypto/bn/bn_lib.c @@ -708,14 +708,29 @@ int BN_ucmp(const BIGNUM *a, const BIGNUM *b) int i; BN_ULONG t1, t2, *ap, *bp; + ap = a->d; + bp = b->d; + + if (BN_get_flags(a, BN_FLG_CONSTTIME) + && a->top == b->top) { + int res = 0; + + for (i = 0; i < b->top; i++) { + res = constant_time_select_int(constant_time_lt_bn(ap[i], bp[i]), + -1, res); + res = constant_time_select_int(constant_time_lt_bn(bp[i], ap[i]), + 1, res); + } + return res; + } + bn_check_top(a); bn_check_top(b); i = a->top - b->top; if (i != 0) return i; - ap = a->d; - bp = b->d; + for (i = a->top - 1; i >= 0; i--) { t1 = ap[i]; t2 = bp[i]; @@ -827,11 +842,10 @@ int BN_is_bit_set(const BIGNUM *a, int n) return (int)(((a->d[i]) >> j) & ((BN_ULONG)1)); } -int BN_mask_bits(BIGNUM *a, int n) +int ossl_bn_mask_bits_fixed_top(BIGNUM *a, int n) { int b, w; - bn_check_top(a); if (n < 0) return 0; @@ -845,10 +859,20 @@ int BN_mask_bits(BIGNUM *a, int n) a->top = w + 1; a->d[w] &= ~(BN_MASK2 << b); } - bn_correct_top(a); return 1; } +int BN_mask_bits(BIGNUM *a, int n) +{ + int ret; + + bn_check_top(a); + ret = ossl_bn_mask_bits_fixed_top(a, n); + if (ret) + bn_correct_top(a); + return ret; +} + void BN_set_negative(BIGNUM *a, int b) { if (b && !BN_is_zero(a)) diff --git a/crypto/bn/bn_local.h b/crypto/bn/bn_local.h index b5be37ba973e3..8dbf773c73f0a 100644 --- a/crypto/bn/bn_local.h +++ b/crypto/bn/bn_local.h @@ -679,5 +679,6 @@ static ossl_inline BIGNUM *bn_expand(BIGNUM *a, int bits) int ossl_bn_check_prime(const BIGNUM *w, int checks, BN_CTX *ctx, int do_trial_division, BN_GENCB *cb); +int ossl_bn_mask_bits_fixed_top(BIGNUM *a, int n); #endif diff --git a/crypto/bn/bn_rand.c b/crypto/bn/bn_rand.c index a94dfcecdf207..f0bac810b4f84 100644 --- a/crypto/bn/bn_rand.c +++ b/crypto/bn/bn_rand.c @@ -258,20 +258,22 @@ int BN_generate_dsa_nonce(BIGNUM *out, const BIGNUM *range, unsigned char random_bytes[64]; unsigned char digest[SHA512_DIGEST_LENGTH]; unsigned done, todo; - /* We generate |range|+8 bytes of random output. */ - const unsigned num_k_bytes = BN_num_bytes(range) + 8; + /* We generate |range|+1 bytes of random output. */ + const unsigned num_k_bytes = BN_num_bytes(range) + 1; unsigned char private_bytes[96]; unsigned char *k_bytes = NULL; + const int max_n = 64; /* Pr(failure to generate) < 2^max_n */ + int n; int ret = 0; EVP_MD *md = NULL; OSSL_LIB_CTX *libctx = ossl_bn_get_libctx(ctx); if (mdctx == NULL) - goto err; + goto end; k_bytes = OPENSSL_malloc(num_k_bytes); if (k_bytes == NULL) - goto err; + goto end; /* We copy |priv| into a local buffer to avoid exposing its length. */ if (BN_bn2binpad(priv, private_bytes, sizeof(private_bytes)) < 0) { @@ -281,41 +283,55 @@ int BN_generate_dsa_nonce(BIGNUM *out, const BIGNUM *range, * length of the private key. */ ERR_raise(ERR_LIB_BN, BN_R_PRIVATE_KEY_TOO_LARGE); - goto err; + goto end; } md = EVP_MD_fetch(libctx, "SHA512", NULL); if (md == NULL) { ERR_raise(ERR_LIB_BN, BN_R_NO_SUITABLE_DIGEST); - goto err; - } - for (done = 0; done < num_k_bytes;) { - if (RAND_priv_bytes_ex(libctx, random_bytes, sizeof(random_bytes), 0) <= 0) - goto err; - - if (!EVP_DigestInit_ex(mdctx, md, NULL) - || !EVP_DigestUpdate(mdctx, &done, sizeof(done)) - || !EVP_DigestUpdate(mdctx, private_bytes, - sizeof(private_bytes)) - || !EVP_DigestUpdate(mdctx, message, message_len) - || !EVP_DigestUpdate(mdctx, random_bytes, sizeof(random_bytes)) - || !EVP_DigestFinal_ex(mdctx, digest, NULL)) - goto err; - - todo = num_k_bytes - done; - if (todo > SHA512_DIGEST_LENGTH) - todo = SHA512_DIGEST_LENGTH; - memcpy(k_bytes + done, digest, todo); - done += todo; + goto end; } + for (n = 0; n < max_n; n++) { + for (done = 0; done < num_k_bytes;) { + if (RAND_priv_bytes_ex(libctx, random_bytes, sizeof(random_bytes), + 0) <= 0) + goto end; + + if (!EVP_DigestInit_ex(mdctx, md, NULL) + || !EVP_DigestUpdate(mdctx, &done, sizeof(done)) + || !EVP_DigestUpdate(mdctx, private_bytes, + sizeof(private_bytes)) + || !EVP_DigestUpdate(mdctx, message, message_len) + || !EVP_DigestUpdate(mdctx, random_bytes, + sizeof(random_bytes)) + || !EVP_DigestFinal_ex(mdctx, digest, NULL)) + goto end; + + todo = num_k_bytes - done; + if (todo > SHA512_DIGEST_LENGTH) + todo = SHA512_DIGEST_LENGTH; + memcpy(k_bytes + done, digest, todo); + done += todo; + } - if (!BN_bin2bn(k_bytes, num_k_bytes, out)) - goto err; - if (BN_mod(out, out, range, ctx) != 1) - goto err; - ret = 1; + /* Ensure top byte is set to avoid non-constant time in bin2bn */ + k_bytes[0] = 0x80; + if (!BN_bin2bn(k_bytes, num_k_bytes, out)) + goto end; - err: + /* Clear out the top bits and rejection filter into range */ + BN_set_flags(out, BN_FLG_CONSTTIME | BN_FLG_FIXED_TOP); + ossl_bn_mask_bits_fixed_top(out, BN_num_bits(range)); + + if (BN_ucmp(out, range) < 0) { + ret = 1; + goto end; + } + } + /* Failed to generate anything */ + ERR_raise(ERR_LIB_BN, ERR_R_INTERNAL_ERROR); + + end: EVP_MD_CTX_free(mdctx); EVP_MD_free(md); OPENSSL_clear_free(k_bytes, num_k_bytes); diff --git a/include/internal/constant_time.h b/include/internal/constant_time.h index 0ed6f823c11ed..e8244cd57b7b8 100644 --- a/include/internal/constant_time.h +++ b/include/internal/constant_time.h @@ -140,6 +140,18 @@ static ossl_inline uint64_t constant_time_lt_64(uint64_t a, uint64_t b) return constant_time_msb_64(a ^ ((a ^ b) | ((a - b) ^ b))); } +#ifdef BN_ULONG +static ossl_inline BN_ULONG constant_time_msb_bn(BN_ULONG a) +{ + return 0 - (a >> (sizeof(a) * 8 - 1)); +} + +static ossl_inline BN_ULONG constant_time_lt_bn(BN_ULONG a, BN_ULONG b) +{ + return constant_time_msb_bn(a ^ ((a ^ b) | ((a - b) ^ b))); +} +#endif + static ossl_inline unsigned int constant_time_ge(unsigned int a, unsigned int b) { From 9d44fd2d723505d082029fa09d79ec8f6c796f8f Mon Sep 17 00:00:00 2001 From: Tomas Mraz Date: Thu, 25 Apr 2024 15:35:36 +0200 Subject: [PATCH 2/8] Make ossl_gen_deterministic_nonce_rfc6979() constant time --- crypto/bn/bn_lib.c | 17 +++++++++++ crypto/bn/bn_local.h | 1 - crypto/bn/bn_rand.c | 2 +- crypto/bn/bn_shift.c | 6 ++-- crypto/deterministic_nonce.c | 50 ++++++++++++++++++++++++++++---- include/crypto/bn.h | 2 ++ include/internal/constant_time.h | 11 +++++++ 7 files changed, 78 insertions(+), 11 deletions(-) diff --git a/crypto/bn/bn_lib.c b/crypto/bn/bn_lib.c index 85698885edd09..cab87d9959f63 100644 --- a/crypto/bn/bn_lib.c +++ b/crypto/bn/bn_lib.c @@ -859,6 +859,7 @@ int ossl_bn_mask_bits_fixed_top(BIGNUM *a, int n) a->top = w + 1; a->d[w] &= ~(BN_MASK2 << b); } + a->flags |= BN_FLG_FIXED_TOP; return 1; } @@ -1046,6 +1047,22 @@ int BN_is_word(const BIGNUM *a, const BN_ULONG w) return BN_abs_is_word(a, w) && (!w || !a->neg); } +int ossl_bn_is_word_fixed_top(const BIGNUM *a, const BN_ULONG w) +{ + int res, i; + const BN_ULONG *ap = a->d; + + if (a->neg || a->top == 0) + return 0; + + res = constant_time_select_int(constant_time_eq_bn(ap[0], w), 1, 0); + + for (i = 1; i < a->top; i++) + res = constant_time_select_int(constant_time_is_zero_bn(ap[i]), + res, 0); + return res; +} + int BN_is_odd(const BIGNUM *a) { return (a->top > 0) && (a->d[0] & 1); diff --git a/crypto/bn/bn_local.h b/crypto/bn/bn_local.h index 8dbf773c73f0a..b5be37ba973e3 100644 --- a/crypto/bn/bn_local.h +++ b/crypto/bn/bn_local.h @@ -679,6 +679,5 @@ static ossl_inline BIGNUM *bn_expand(BIGNUM *a, int bits) int ossl_bn_check_prime(const BIGNUM *w, int checks, BN_CTX *ctx, int do_trial_division, BN_GENCB *cb); -int ossl_bn_mask_bits_fixed_top(BIGNUM *a, int n); #endif diff --git a/crypto/bn/bn_rand.c b/crypto/bn/bn_rand.c index f0bac810b4f84..6be0c5e941cac 100644 --- a/crypto/bn/bn_rand.c +++ b/crypto/bn/bn_rand.c @@ -320,7 +320,7 @@ int BN_generate_dsa_nonce(BIGNUM *out, const BIGNUM *range, goto end; /* Clear out the top bits and rejection filter into range */ - BN_set_flags(out, BN_FLG_CONSTTIME | BN_FLG_FIXED_TOP); + BN_set_flags(out, BN_FLG_CONSTTIME); ossl_bn_mask_bits_fixed_top(out, BN_num_bits(range)); if (BN_ucmp(out, range) < 0) { diff --git a/crypto/bn/bn_shift.c b/crypto/bn/bn_shift.c index 8fcb04324e6d5..a6976c71306e1 100644 --- a/crypto/bn/bn_shift.c +++ b/crypto/bn/bn_shift.c @@ -156,6 +156,9 @@ int BN_rshift(BIGNUM *r, const BIGNUM *a, int n) return 0; } + bn_check_top(r); + bn_check_top(a); + ret = bn_rshift_fixed_top(r, a, n); bn_correct_top(r); @@ -177,9 +180,6 @@ int bn_rshift_fixed_top(BIGNUM *r, const BIGNUM *a, int n) BN_ULONG *t, *f; BN_ULONG l, m, mask; - bn_check_top(r); - bn_check_top(a); - assert(n >= 0); nw = n / BN_BITS2; diff --git a/crypto/deterministic_nonce.c b/crypto/deterministic_nonce.c index 60af7f6ab6655..a37edea2a1ae6 100644 --- a/crypto/deterministic_nonce.c +++ b/crypto/deterministic_nonce.c @@ -7,11 +7,13 @@ * https://www.openssl.org/source/license.html */ +#include #include #include #include #include #include "internal/deterministic_nonce.h" +#include "crypto/bn.h" /* * Convert a Bit String to an Integer (See RFC 6979 Section 2.3.2) @@ -38,6 +40,36 @@ static int bits2int(BIGNUM *out, int qlen_bits, return 1; } +/* + * Convert as above a Bit String in const time to an Integer w fixed top + * + * Params: + * out The returned Integer as a BIGNUM + * qlen_bits The maximum size of the returned integer in bits. The returned + * Integer is shifted right if inlen is larger than qlen_bits.. + * in, inlen The input Bit String (in bytes). It has sizeof(BN_ULONG) bytes + * prefix with all bits set that needs to be cleared out after + * the conversion. + * Returns: 1 if successful, or 0 otherwise. + */ +static int bits2int_consttime(BIGNUM *out, int qlen_bits, + const unsigned char *in, size_t inlen) +{ + int blen_bits = (inlen - sizeof(BN_ULONG)) * 8; + int shift; + + if (BN_bin2bn(in, (int)inlen, out) == NULL) + return 0; + + BN_set_flags(out, BN_FLG_CONSTTIME); + ossl_bn_mask_bits_fixed_top(out, blen_bits); + + shift = blen_bits - qlen_bits; + if (shift > 0) + return bn_rshift_fixed_top(out, out, shift); + return 1; +} + /* * Convert an Integer to an Octet String (See RFC 6979 2.3.3). * The value is zero padded if required. @@ -155,8 +187,9 @@ int ossl_gen_deterministic_nonce_rfc6979(BIGNUM *out, const BIGNUM *q, { EVP_KDF_CTX *kdfctx = NULL; int ret = 0, rlen = 0, qlen_bits = 0; - unsigned char *entropyx = NULL, *nonceh = NULL, *T = NULL; + unsigned char *entropyx = NULL, *nonceh = NULL, *rbits = NULL, *T = NULL; size_t allocsz = 0; + const size_t prefsz = sizeof(BN_ULONG); if (out == NULL) return 0; @@ -167,15 +200,18 @@ int ossl_gen_deterministic_nonce_rfc6979(BIGNUM *out, const BIGNUM *q, /* Note rlen used here is in bytes since the input values are byte arrays */ rlen = (qlen_bits + 7) / 8; - allocsz = 3 * rlen; + allocsz = prefsz + 3 * rlen; /* Use a single alloc for the buffers T, nonceh and entropyx */ T = (unsigned char *)OPENSSL_zalloc(allocsz); if (T == NULL) return 0; - nonceh = T + rlen; + rbits = T + prefsz; + nonceh = rbits + rlen; entropyx = nonceh + rlen; + memset(T, 0xff, prefsz); + if (!int2octets(entropyx, priv, rlen) || !bits2octets(nonceh, q, qlen_bits, rlen, hm, hmlen)) goto end; @@ -185,10 +221,12 @@ int ossl_gen_deterministic_nonce_rfc6979(BIGNUM *out, const BIGNUM *q, goto end; do { - if (!EVP_KDF_derive(kdfctx, T, rlen, NULL) - || !bits2int(out, qlen_bits, T, rlen)) + if (!EVP_KDF_derive(kdfctx, rbits, rlen, NULL) + || !bits2int_consttime(out, qlen_bits, T, rlen + prefsz)) goto end; - } while (BN_is_zero(out) || BN_is_one(out) || BN_cmp(out, q) >= 0); + } while (ossl_bn_is_word_fixed_top(out, 0) + || ossl_bn_is_word_fixed_top(out, 1) + || BN_ucmp(out, q) >= 0); ret = 1; end: diff --git a/include/crypto/bn.h b/include/crypto/bn.h index f5d8683ebc01b..50d89fa67af15 100644 --- a/include/crypto/bn.h +++ b/include/crypto/bn.h @@ -87,6 +87,8 @@ int bn_lshift_fixed_top(BIGNUM *r, const BIGNUM *a, int n); int bn_rshift_fixed_top(BIGNUM *r, const BIGNUM *a, int n); int bn_div_fixed_top(BIGNUM *dv, BIGNUM *rem, const BIGNUM *m, const BIGNUM *d, BN_CTX *ctx); +int ossl_bn_mask_bits_fixed_top(BIGNUM *a, int n); +int ossl_bn_is_word_fixed_top(const BIGNUM *a, BN_ULONG w); #define BN_PRIMETEST_COMPOSITE 0 #define BN_PRIMETEST_COMPOSITE_WITH_FACTOR 1 diff --git a/include/internal/constant_time.h b/include/internal/constant_time.h index e8244cd57b7b8..f2572ded51998 100644 --- a/include/internal/constant_time.h +++ b/include/internal/constant_time.h @@ -150,6 +150,17 @@ static ossl_inline BN_ULONG constant_time_lt_bn(BN_ULONG a, BN_ULONG b) { return constant_time_msb_bn(a ^ ((a ^ b) | ((a - b) ^ b))); } + +static ossl_inline BN_ULONG constant_time_is_zero_bn(BN_ULONG a) +{ + return constant_time_msb_bn(~a & (a - 1)); +} + +static ossl_inline BN_ULONG constant_time_eq_bn(BN_ULONG a, + BN_ULONG b) +{ + return constant_time_is_zero_bn(a ^ b); +} #endif static ossl_inline unsigned int constant_time_ge(unsigned int a, From b8ba14ed68660180ee845b71c81e0709840d1fa7 Mon Sep 17 00:00:00 2001 From: Tomas Mraz Date: Thu, 25 Apr 2024 19:26:08 +0200 Subject: [PATCH 3/8] Add ossl_bn_priv_rand_range_fixed_top() and use it for EC/DSA --- crypto/bn/bn_rand.c | 45 ++++++++++++++++++++++++++++++++++++++++-- crypto/dsa/dsa_ossl.c | 4 ++-- crypto/ec/ecdsa_ossl.c | 4 ++-- include/crypto/bn.h | 2 ++ 4 files changed, 49 insertions(+), 6 deletions(-) diff --git a/crypto/bn/bn_rand.c b/crypto/bn/bn_rand.c index 6be0c5e941cac..1131987ac7d36 100644 --- a/crypto/bn/bn_rand.c +++ b/crypto/bn/bn_rand.c @@ -184,8 +184,8 @@ static int bnrand_range(BNRAND_FLAG flag, BIGNUM *r, const BIGNUM *range, } else { do { /* range = 11..._2 or range = 101..._2 */ - if (!bnrand(flag, r, n, BN_RAND_TOP_ANY, BN_RAND_BOTTOM_ANY, 0, - ctx)) + if (!bnrand(flag, r, n, BN_RAND_TOP_ANY, BN_RAND_BOTTOM_ANY, + strength, ctx)) return 0; if (!--count) { @@ -238,6 +238,47 @@ int BN_pseudo_rand_range(BIGNUM *r, const BIGNUM *range) # endif #endif +int ossl_bn_priv_rand_range_fixed_top(BIGNUM *r, const BIGNUM *range, + unsigned int strength, BN_CTX *ctx) +{ + int n; + int count = 100; + + if (r == NULL) { + ERR_raise(ERR_LIB_BN, ERR_R_PASSED_NULL_PARAMETER); + return 0; + } + + if (range->neg || BN_is_zero(range)) { + ERR_raise(ERR_LIB_BN, BN_R_INVALID_RANGE); + return 0; + } + + n = BN_num_bits(range); /* n > 0 */ + + /* BN_is_bit_set(range, n - 1) always holds */ + + if (n == 1) { + BN_zero(r); + } else { + BN_set_flags(r, BN_FLG_CONSTTIME); + do { + if (!bnrand(PRIVATE, r, n + 1, BN_RAND_TOP_ONE, BN_RAND_BOTTOM_ANY, + strength, ctx)) + return 0; + + if (!--count) { + ERR_raise(ERR_LIB_BN, BN_R_TOO_MANY_ITERATIONS); + return 0; + } + ossl_bn_mask_bits_fixed_top(r, n); + } + while (BN_ucmp(r, range) >= 0); + } + + return 1; +} + /* * BN_generate_dsa_nonce generates a random number 0 <= out < range. Unlike * BN_rand_range, it also includes the contents of |priv| and |message| in diff --git a/crypto/dsa/dsa_ossl.c b/crypto/dsa/dsa_ossl.c index 234362b6d7499..6a00a0fa89d57 100644 --- a/crypto/dsa/dsa_ossl.c +++ b/crypto/dsa/dsa_ossl.c @@ -286,9 +286,9 @@ static int dsa_sign_setup(DSA *dsa, BN_CTX *ctx_in, dlen, ctx)) goto err; } - } else if (!BN_priv_rand_range_ex(k, dsa->params.q, 0, ctx)) + } else if (!ossl_bn_priv_rand_range_fixed_top(k, dsa->params.q, 0, ctx)) goto err; - } while (BN_is_zero(k)); + } while (ossl_bn_is_word_fixed_top(k, 0)); BN_set_flags(k, BN_FLG_CONSTTIME); BN_set_flags(l, BN_FLG_CONSTTIME); diff --git a/crypto/ec/ecdsa_ossl.c b/crypto/ec/ecdsa_ossl.c index e60877aa057e5..69d966ae14bb0 100644 --- a/crypto/ec/ecdsa_ossl.c +++ b/crypto/ec/ecdsa_ossl.c @@ -202,13 +202,13 @@ static int ecdsa_sign_setup(EC_KEY *eckey, BN_CTX *ctx_in, ctx); } } else { - res = BN_priv_rand_range_ex(k, order, 0, ctx); + res = ossl_bn_priv_rand_range_fixed_top(k, order, 0, ctx); } if (!res) { ERR_raise(ERR_LIB_EC, EC_R_RANDOM_NUMBER_GENERATION_FAILED); goto err; } - } while (BN_is_zero(k)); + } while (ossl_bn_is_word_fixed_top(k, 0)); /* compute r the x-coordinate of generator * k */ if (!EC_POINT_mul(group, tmp_point, k, NULL, NULL, ctx)) { diff --git a/include/crypto/bn.h b/include/crypto/bn.h index 50d89fa67af15..308cf575024a8 100644 --- a/include/crypto/bn.h +++ b/include/crypto/bn.h @@ -89,6 +89,8 @@ int bn_div_fixed_top(BIGNUM *dv, BIGNUM *rem, const BIGNUM *m, const BIGNUM *d, BN_CTX *ctx); int ossl_bn_mask_bits_fixed_top(BIGNUM *a, int n); int ossl_bn_is_word_fixed_top(const BIGNUM *a, BN_ULONG w); +int ossl_bn_priv_rand_range_fixed_top(BIGNUM *r, const BIGNUM *range, + unsigned int strength, BN_CTX *ctx); #define BN_PRIMETEST_COMPOSITE 0 #define BN_PRIMETEST_COMPOSITE_WITH_FACTOR 1 From 77fe3a7078b8f2fcb411a74a135ef112454ce231 Mon Sep 17 00:00:00 2001 From: Tomas Mraz Date: Thu, 25 Apr 2024 20:18:51 +0200 Subject: [PATCH 4/8] Adjust FIPS EC/DSA self test data for different nonce generation --- providers/fips/self_test_data.inc | 49 +++++++++++++++---------------- 1 file changed, 24 insertions(+), 25 deletions(-) diff --git a/providers/fips/self_test_data.inc b/providers/fips/self_test_data.inc index a637efe616264..2eab3cf5c993d 100644 --- a/providers/fips/self_test_data.inc +++ b/providers/fips/self_test_data.inc @@ -1451,14 +1451,14 @@ static const unsigned char ecd_prime_pub[] = { 0x82 }; static const unsigned char ecdsa_prime_expected_sig[] = { - 0x30, 0x3d, 0x02, 0x1d, 0x00, 0xd2, 0x4a, 0xc9, - 0x4f, 0xaf, 0xdb, 0x62, 0xfc, 0x41, 0x4a, 0x81, - 0x2a, 0x9f, 0xcf, 0xa3, 0xda, 0xfe, 0xa3, 0x49, - 0xbd, 0xea, 0xbf, 0x2a, 0x51, 0xb4, 0x0b, 0xc3, - 0xbc, 0x02, 0x1c, 0x7f, 0x30, 0xb7, 0xad, 0xab, - 0x09, 0x6e, 0x3c, 0xad, 0x7f, 0xf9, 0x5e, 0xaa, - 0xe2, 0x38, 0xe5, 0x29, 0x16, 0xc4, 0xc8, 0x77, - 0xa1, 0xf8, 0x60, 0x77, 0x39, 0x7a, 0xec + 0x30, 0x3d, 0x02, 0x1d, 0x00, 0xea, 0x1c, 0x41, + 0x4e, 0x51, 0xf1, 0xcb, 0x84, 0x91, 0x67, 0xa7, + 0x1e, 0xea, 0x50, 0x77, 0x7f, 0x7e, 0xd5, 0x64, + 0xfa, 0xae, 0x82, 0xa0, 0xf9, 0x2a, 0xbc, 0xd2, + 0x53, 0x02, 0x1c, 0x1c, 0xac, 0xc4, 0xba, 0x16, + 0xfd, 0x53, 0x5a, 0x89, 0xf3, 0x66, 0x9a, 0x89, + 0x35, 0x78, 0x45, 0xd7, 0x71, 0x19, 0x2e, 0xf6, + 0xd0, 0xb3, 0xcc, 0xcd, 0x6f, 0x5b, 0xfa }; static const ST_KAT_PARAM ecdsa_prime_key[] = { ST_KAT_PARAM_UTF8STRING(OSSL_PKEY_PARAM_GROUP_NAME, ecd_prime_curve_name), @@ -1486,15 +1486,14 @@ static const unsigned char ecd_bin_pub[] = { 0x99, 0xb6, 0x8f, 0x80, 0x46 }; static const unsigned char ecdsa_bin_expected_sig[] = { - 0x30, 0x3f, 0x02, 0x1d, 0x08, 0x11, 0x7c, 0xcd, - 0xf4, 0xa1, 0x31, 0x9a, 0xc1, 0xfd, 0x50, 0x0e, - 0x5d, 0xa9, 0xb6, 0x0e, 0x95, 0x49, 0xe1, 0xbd, - 0x44, 0xe3, 0x5b, 0xa9, 0x35, 0x94, 0xa5, 0x2f, - 0xae, 0x02, 0x1e, 0x00, 0xe3, 0xba, 0xb8, 0x8f, - 0x4b, 0x05, 0x76, 0x88, 0x1e, 0x49, 0xd6, 0x62, - 0x76, 0xd3, 0x22, 0x4d, 0xa3, 0x7b, 0x04, 0xcc, - 0xfa, 0x7b, 0x41, 0x9b, 0x8c, 0xaf, 0x1b, 0x6d, - 0xbd + 0x30, 0x3e, 0x02, 0x1d, 0x36, 0x11, 0xde, 0xe7, + 0x7c, 0x99, 0x31, 0x9a, 0x90, 0xcf, 0x91, 0x23, + 0x75, 0x9d, 0xe8, 0xf4, 0x20, 0x70, 0x0c, 0x5a, + 0xa6, 0x00, 0x7a, 0xb2, 0x94, 0x41, 0x06, 0x7a, + 0x0f, 0x02, 0x1d, 0x21, 0x95, 0x18, 0x12, 0x84, + 0xf4, 0x18, 0x83, 0x38, 0x02, 0x2a, 0xee, 0x82, + 0xdd, 0x81, 0xe2, 0xbd, 0xb6, 0x51, 0xab, 0xa6, + 0x53, 0x68, 0xd0, 0xf9, 0x26, 0x19, 0xbf, 0x7a }; static const ST_KAT_PARAM ecdsa_bin_key[] = { ST_KAT_PARAM_UTF8STRING(OSSL_PKEY_PARAM_GROUP_NAME, ecd_bin_curve_name), @@ -1622,14 +1621,14 @@ static const unsigned char dsa_priv[] = { 0x40, 0x7e, 0x5c, 0xb7 }; static const unsigned char dsa_expected_sig[] = { - 0x30, 0x3c, 0x02, 0x1c, 0x70, 0xa4, 0x77, 0xb6, - 0x02, 0xb5, 0xd3, 0x07, 0x21, 0x22, 0x2d, 0xe3, - 0x4f, 0x7d, 0xfd, 0xfd, 0x6b, 0x4f, 0x03, 0x27, - 0x4c, 0xd3, 0xb2, 0x8c, 0x7c, 0xc5, 0xc4, 0xdf, - 0x02, 0x1c, 0x11, 0x52, 0x65, 0x16, 0x9f, 0xbd, - 0x4c, 0xe5, 0xab, 0xb2, 0x01, 0xd0, 0x7a, 0x30, - 0x5c, 0xc5, 0xba, 0x22, 0xc6, 0x62, 0x7e, 0xa6, - 0x7d, 0x98, 0x96, 0xc9, 0x77, 0x00 + 0x30, 0x3d, 0x02, 0x1c, 0x1d, 0xad, 0xe0, 0x2b, + 0x4f, 0x84, 0x11, 0x9f, 0x17, 0x56, 0x24, 0x99, + 0x34, 0x8e, 0x25, 0xf1, 0xe1, 0x8a, 0x14, 0x99, + 0x16, 0x2b, 0xa6, 0xcc, 0xc5, 0x9b, 0x8d, 0x4c, + 0x02, 0x1d, 0x00, 0xa2, 0x62, 0x4d, 0xdd, 0x96, + 0xbe, 0xaa, 0xbf, 0xf6, 0x11, 0xed, 0xd6, 0xd5, + 0x04, 0x1c, 0xa0, 0x6e, 0x3d, 0x4b, 0x15, 0xab, + 0x4d, 0x94, 0xe0, 0x4f, 0xde, 0x50, 0x4c }; static const ST_KAT_PARAM dsa_key[] = { From 4f7eab014728ebfeed89d8d85a450e884792de5b Mon Sep 17 00:00:00 2001 From: Tomas Mraz Date: Mon, 29 Apr 2024 17:56:01 +0200 Subject: [PATCH 5/8] fixup! Make BN_generate_dsa_nonce() constant time and non-biased --- crypto/bn/bn_rand.c | 36 +++++++++++++++++++++++++++--------- crypto/dsa/dsa_ossl.c | 5 +++-- crypto/ec/ecdsa_ossl.c | 4 ++-- include/crypto/bn.h | 4 ++++ 4 files changed, 36 insertions(+), 13 deletions(-) diff --git a/crypto/bn/bn_rand.c b/crypto/bn/bn_rand.c index 1131987ac7d36..42008b52ddb3d 100644 --- a/crypto/bn/bn_rand.c +++ b/crypto/bn/bn_rand.c @@ -280,16 +280,17 @@ int ossl_bn_priv_rand_range_fixed_top(BIGNUM *r, const BIGNUM *range, } /* - * BN_generate_dsa_nonce generates a random number 0 <= out < range. Unlike - * BN_rand_range, it also includes the contents of |priv| and |message| in - * the generation so that an RNG failure isn't fatal as long as |priv| + * ossl_bn_gen_dsa_nonce_fixed_top generates a random number 0 <= out < range. + * Unlike BN_rand_range, it also includes the contents of |priv| and |message| + * in the generation so that an RNG failure isn't fatal as long as |priv| * remains secret. This is intended for use in DSA and ECDSA where an RNG * weakness leads directly to private key exposure unless this function is * used. */ -int BN_generate_dsa_nonce(BIGNUM *out, const BIGNUM *range, - const BIGNUM *priv, const unsigned char *message, - size_t message_len, BN_CTX *ctx) +int ossl_bn_gen_dsa_nonce_fixed_top(BIGNUM *out, const BIGNUM *range, + const BIGNUM *priv, + const unsigned char *message, + size_t message_len, BN_CTX *ctx) { EVP_MD_CTX *mdctx = EVP_MD_CTX_new(); /* @@ -315,6 +316,8 @@ int BN_generate_dsa_nonce(BIGNUM *out, const BIGNUM *range, k_bytes = OPENSSL_malloc(num_k_bytes); if (k_bytes == NULL) goto end; + /* Ensure top byte is set to avoid non-constant time in bin2bn */ + k_bytes[0] = 0xff; /* We copy |priv| into a local buffer to avoid exposing its length. */ if (BN_bn2binpad(priv, private_bytes, sizeof(private_bytes)) < 0) { @@ -333,7 +336,7 @@ int BN_generate_dsa_nonce(BIGNUM *out, const BIGNUM *range, goto end; } for (n = 0; n < max_n; n++) { - for (done = 0; done < num_k_bytes;) { + for (done = 1; done < num_k_bytes;) { if (RAND_priv_bytes_ex(libctx, random_bytes, sizeof(random_bytes), 0) <= 0) goto end; @@ -355,8 +358,6 @@ int BN_generate_dsa_nonce(BIGNUM *out, const BIGNUM *range, done += todo; } - /* Ensure top byte is set to avoid non-constant time in bin2bn */ - k_bytes[0] = 0x80; if (!BN_bin2bn(k_bytes, num_k_bytes, out)) goto end; @@ -381,3 +382,20 @@ int BN_generate_dsa_nonce(BIGNUM *out, const BIGNUM *range, OPENSSL_cleanse(private_bytes, sizeof(private_bytes)); return ret; } + +int BN_generate_dsa_nonce(BIGNUM *out, const BIGNUM *range, + const BIGNUM *priv, const unsigned char *message, + size_t message_len, BN_CTX *ctx) +{ + int ret; + + ret = ossl_bn_gen_dsa_nonce_fixed_top(out, range, priv, message, + message_len, ctx); + /* + * This call makes the BN_generate_dsa_nonce non-const-time, thus we + * do not use it internally. But fixed_top BNs currently cannot be returned + * from public API calls. + */ + bn_correct_top(out); + return ret; +} diff --git a/crypto/dsa/dsa_ossl.c b/crypto/dsa/dsa_ossl.c index 6a00a0fa89d57..409830092dc63 100644 --- a/crypto/dsa/dsa_ossl.c +++ b/crypto/dsa/dsa_ossl.c @@ -282,8 +282,9 @@ static int dsa_sign_setup(DSA *dsa, BN_CTX *ctx_in, * We calculate k from SHA512(private_key + H(message) + random). * This protects the private key from a weak PRNG. */ - if (!BN_generate_dsa_nonce(k, dsa->params.q, dsa->priv_key, dgst, - dlen, ctx)) + if (!ossl_bn_gen_dsa_nonce_fixed_top(k, dsa->params.q, + dsa->priv_key, dgst, + dlen, ctx)) goto err; } } else if (!ossl_bn_priv_rand_range_fixed_top(k, dsa->params.q, 0, ctx)) diff --git a/crypto/ec/ecdsa_ossl.c b/crypto/ec/ecdsa_ossl.c index 69d966ae14bb0..1e611f7ffce0b 100644 --- a/crypto/ec/ecdsa_ossl.c +++ b/crypto/ec/ecdsa_ossl.c @@ -198,8 +198,8 @@ static int ecdsa_sign_setup(EC_KEY *eckey, BN_CTX *ctx_in, libctx, propq); #endif } else { - res = BN_generate_dsa_nonce(k, order, priv_key, dgst, dlen, - ctx); + res = ossl_bn_gen_dsa_nonce_fixed_top(k, order, priv_key, + dgst, dlen, ctx); } } else { res = ossl_bn_priv_rand_range_fixed_top(k, order, 0, ctx); diff --git a/include/crypto/bn.h b/include/crypto/bn.h index 308cf575024a8..9a988a467de27 100644 --- a/include/crypto/bn.h +++ b/include/crypto/bn.h @@ -91,6 +91,10 @@ int ossl_bn_mask_bits_fixed_top(BIGNUM *a, int n); int ossl_bn_is_word_fixed_top(const BIGNUM *a, BN_ULONG w); int ossl_bn_priv_rand_range_fixed_top(BIGNUM *r, const BIGNUM *range, unsigned int strength, BN_CTX *ctx); +int ossl_bn_gen_dsa_nonce_fixed_top(BIGNUM *out, const BIGNUM *range, + const BIGNUM *priv, + const unsigned char *message, + size_t message_len, BN_CTX *ctx); #define BN_PRIMETEST_COMPOSITE 0 #define BN_PRIMETEST_COMPOSITE_WITH_FACTOR 1 From 40c2f6c5670787173563ebf14e6a63de76ef6c6c Mon Sep 17 00:00:00 2001 From: Tomas Mraz Date: Tue, 30 Apr 2024 11:33:22 +0200 Subject: [PATCH 6/8] fixup! Make BN_generate_dsa_nonce() constant time and non-biased --- crypto/bn/bn_rand.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crypto/bn/bn_rand.c b/crypto/bn/bn_rand.c index 42008b52ddb3d..a93bd68c736b2 100644 --- a/crypto/bn/bn_rand.c +++ b/crypto/bn/bn_rand.c @@ -336,13 +336,15 @@ int ossl_bn_gen_dsa_nonce_fixed_top(BIGNUM *out, const BIGNUM *range, goto end; } for (n = 0; n < max_n; n++) { + unsigned char i = 0; + for (done = 1; done < num_k_bytes;) { if (RAND_priv_bytes_ex(libctx, random_bytes, sizeof(random_bytes), 0) <= 0) goto end; if (!EVP_DigestInit_ex(mdctx, md, NULL) - || !EVP_DigestUpdate(mdctx, &done, sizeof(done)) + || !EVP_DigestUpdate(mdctx, &i, sizeof(i)) || !EVP_DigestUpdate(mdctx, private_bytes, sizeof(private_bytes)) || !EVP_DigestUpdate(mdctx, message, message_len) @@ -356,6 +358,7 @@ int ossl_bn_gen_dsa_nonce_fixed_top(BIGNUM *out, const BIGNUM *range, todo = SHA512_DIGEST_LENGTH; memcpy(k_bytes + done, digest, todo); done += todo; + ++i; } if (!BN_bin2bn(k_bytes, num_k_bytes, out)) From fa04415864928fd3b76c8e835242ede880897d2f Mon Sep 17 00:00:00 2001 From: Tomas Mraz Date: Tue, 30 Apr 2024 11:33:59 +0200 Subject: [PATCH 7/8] fixup! Adjust FIPS EC/DSA self test data for different nonce generation --- providers/fips/self_test_data.inc | 49 ++++++++++++++++--------------- 1 file changed, 25 insertions(+), 24 deletions(-) diff --git a/providers/fips/self_test_data.inc b/providers/fips/self_test_data.inc index 2eab3cf5c993d..9a612468045ee 100644 --- a/providers/fips/self_test_data.inc +++ b/providers/fips/self_test_data.inc @@ -1451,14 +1451,14 @@ static const unsigned char ecd_prime_pub[] = { 0x82 }; static const unsigned char ecdsa_prime_expected_sig[] = { - 0x30, 0x3d, 0x02, 0x1d, 0x00, 0xea, 0x1c, 0x41, - 0x4e, 0x51, 0xf1, 0xcb, 0x84, 0x91, 0x67, 0xa7, - 0x1e, 0xea, 0x50, 0x77, 0x7f, 0x7e, 0xd5, 0x64, - 0xfa, 0xae, 0x82, 0xa0, 0xf9, 0x2a, 0xbc, 0xd2, - 0x53, 0x02, 0x1c, 0x1c, 0xac, 0xc4, 0xba, 0x16, - 0xfd, 0x53, 0x5a, 0x89, 0xf3, 0x66, 0x9a, 0x89, - 0x35, 0x78, 0x45, 0xd7, 0x71, 0x19, 0x2e, 0xf6, - 0xd0, 0xb3, 0xcc, 0xcd, 0x6f, 0x5b, 0xfa + 0x30, 0x3d, 0x02, 0x1c, 0x48, 0x4f, 0x3c, 0x97, + 0x5b, 0xfa, 0x40, 0x6c, 0xdb, 0xd6, 0x70, 0xb5, + 0xbd, 0x2d, 0xd0, 0xc6, 0x22, 0x93, 0x5a, 0x88, + 0x56, 0xd0, 0xaf, 0x0a, 0x94, 0x92, 0x20, 0x01, + 0x02, 0x1d, 0x00, 0xa4, 0x80, 0xe0, 0x47, 0x88, + 0x8a, 0xef, 0x2a, 0x47, 0x9d, 0x81, 0x9a, 0xbf, + 0x45, 0xc3, 0x6f, 0x9e, 0x2e, 0xc1, 0x44, 0x9f, + 0xfd, 0x79, 0xdb, 0x90, 0x3e, 0xb9, 0xb2 }; static const ST_KAT_PARAM ecdsa_prime_key[] = { ST_KAT_PARAM_UTF8STRING(OSSL_PKEY_PARAM_GROUP_NAME, ecd_prime_curve_name), @@ -1486,14 +1486,15 @@ static const unsigned char ecd_bin_pub[] = { 0x99, 0xb6, 0x8f, 0x80, 0x46 }; static const unsigned char ecdsa_bin_expected_sig[] = { - 0x30, 0x3e, 0x02, 0x1d, 0x36, 0x11, 0xde, 0xe7, - 0x7c, 0x99, 0x31, 0x9a, 0x90, 0xcf, 0x91, 0x23, - 0x75, 0x9d, 0xe8, 0xf4, 0x20, 0x70, 0x0c, 0x5a, - 0xa6, 0x00, 0x7a, 0xb2, 0x94, 0x41, 0x06, 0x7a, - 0x0f, 0x02, 0x1d, 0x21, 0x95, 0x18, 0x12, 0x84, - 0xf4, 0x18, 0x83, 0x38, 0x02, 0x2a, 0xee, 0x82, - 0xdd, 0x81, 0xe2, 0xbd, 0xb6, 0x51, 0xab, 0xa6, - 0x53, 0x68, 0xd0, 0xf9, 0x26, 0x19, 0xbf, 0x7a + 0x30, 0x3f, 0x02, 0x1d, 0x58, 0xe9, 0xd0, 0x84, + 0x5c, 0xad, 0x29, 0x03, 0xf6, 0xa6, 0xbc, 0xe0, + 0x24, 0x6d, 0x9e, 0x79, 0x5d, 0x1e, 0xe8, 0x5a, + 0xc3, 0x31, 0x0a, 0xa9, 0xfb, 0xe3, 0x99, 0x54, + 0x11, 0x02, 0x1e, 0x00, 0xa3, 0x44, 0x28, 0xa3, + 0x70, 0x97, 0x98, 0x17, 0xd7, 0xa6, 0xad, 0x91, + 0xaf, 0x41, 0x69, 0xb6, 0x06, 0x99, 0x39, 0xc7, + 0x63, 0xa4, 0x6a, 0x81, 0xe4, 0x9a, 0x9d, 0x15, + 0x8b }; static const ST_KAT_PARAM ecdsa_bin_key[] = { ST_KAT_PARAM_UTF8STRING(OSSL_PKEY_PARAM_GROUP_NAME, ecd_bin_curve_name), @@ -1621,14 +1622,14 @@ static const unsigned char dsa_priv[] = { 0x40, 0x7e, 0x5c, 0xb7 }; static const unsigned char dsa_expected_sig[] = { - 0x30, 0x3d, 0x02, 0x1c, 0x1d, 0xad, 0xe0, 0x2b, - 0x4f, 0x84, 0x11, 0x9f, 0x17, 0x56, 0x24, 0x99, - 0x34, 0x8e, 0x25, 0xf1, 0xe1, 0x8a, 0x14, 0x99, - 0x16, 0x2b, 0xa6, 0xcc, 0xc5, 0x9b, 0x8d, 0x4c, - 0x02, 0x1d, 0x00, 0xa2, 0x62, 0x4d, 0xdd, 0x96, - 0xbe, 0xaa, 0xbf, 0xf6, 0x11, 0xed, 0xd6, 0xd5, - 0x04, 0x1c, 0xa0, 0x6e, 0x3d, 0x4b, 0x15, 0xab, - 0x4d, 0x94, 0xe0, 0x4f, 0xde, 0x50, 0x4c + 0x30, 0x3c, 0x02, 0x1c, 0x69, 0xc6, 0xd6, 0x9e, + 0x2b, 0x91, 0xea, 0x72, 0xb3, 0x8b, 0x7c, 0x57, + 0x48, 0x75, 0xb7, 0x65, 0xc0, 0xb4, 0xf7, 0xbb, + 0x08, 0xa4, 0x95, 0x77, 0xfc, 0xa7, 0xed, 0x31, + 0x02, 0x1c, 0x4c, 0x2c, 0xff, 0xc6, 0x55, 0xeb, + 0x8f, 0xa7, 0x4f, 0x27, 0xd8, 0xec, 0xfd, 0x62, + 0x73, 0xf2, 0xd1, 0x55, 0xa5, 0xf0, 0x41, 0x68, + 0x34, 0x8d, 0x9e, 0x88, 0x08, 0x06 }; static const ST_KAT_PARAM dsa_key[] = { From 08fbff08d99fedde77b9c675a1437c6ba89da877 Mon Sep 17 00:00:00 2001 From: Tomas Mraz Date: Tue, 30 Apr 2024 11:46:26 +0200 Subject: [PATCH 8/8] Correct top for EC/DSA nonces if BN_DEBUG is on Otherwise following operations would bail out in bn_check_top(). --- crypto/bn/bn_rand.c | 8 ++++++++ crypto/deterministic_nonce.c | 4 ++++ 2 files changed, 12 insertions(+) diff --git a/crypto/bn/bn_rand.c b/crypto/bn/bn_rand.c index a93bd68c736b2..650d05747040a 100644 --- a/crypto/bn/bn_rand.c +++ b/crypto/bn/bn_rand.c @@ -274,6 +274,10 @@ int ossl_bn_priv_rand_range_fixed_top(BIGNUM *r, const BIGNUM *range, ossl_bn_mask_bits_fixed_top(r, n); } while (BN_ucmp(r, range) >= 0); +#ifdef BN_DEBUG + /* With BN_DEBUG on a fixed top number cannot be returned */ + bn_correct_top(r); +#endif } return 1; @@ -370,6 +374,10 @@ int ossl_bn_gen_dsa_nonce_fixed_top(BIGNUM *out, const BIGNUM *range, if (BN_ucmp(out, range) < 0) { ret = 1; +#ifdef BN_DEBUG + /* With BN_DEBUG on a fixed top number cannot be returned */ + bn_correct_top(out); +#endif goto end; } } diff --git a/crypto/deterministic_nonce.c b/crypto/deterministic_nonce.c index a37edea2a1ae6..67a5b98d2b238 100644 --- a/crypto/deterministic_nonce.c +++ b/crypto/deterministic_nonce.c @@ -227,6 +227,10 @@ int ossl_gen_deterministic_nonce_rfc6979(BIGNUM *out, const BIGNUM *q, } while (ossl_bn_is_word_fixed_top(out, 0) || ossl_bn_is_word_fixed_top(out, 1) || BN_ucmp(out, q) >= 0); +#ifdef BN_DEBUG + /* With BN_DEBUG on a fixed top number cannot be returned */ + bn_correct_top(out); +#endif ret = 1; end: