From d53a9a6a5d15081275f597aca54497e354750119 Mon Sep 17 00:00:00 2001 From: Tomas Mraz Date: Thu, 11 Apr 2024 13:10:09 +0200 Subject: [PATCH 1/6] Make BN_generate_dsa_nonce() constant time and non-biased Co-authored-by: Paul Dale Reviewed-by: Paul Dale Reviewed-by: Neil Horman (cherry picked from commit d7d1bdcb6aa3d5000bf7f5ebc5518be5c91fd5a5) (Merged from https://github.com/openssl/openssl/pull/24317) (cherry picked from commit 0df711a25da6e99a7ce0dbaf992acb644252385f) Signed-off-by: lcc --- 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 cf1bfe8ab0..bbfa882ad4 100644 --- a/crypto/bn/bn_lib.c +++ b/crypto/bn/bn_lib.c @@ -618,14 +618,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]; @@ -737,11 +752,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; @@ -755,10 +769,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 50e9d26e21..d355d1fce3 100644 --- a/crypto/bn/bn_local.h +++ b/crypto/bn/bn_local.h @@ -676,5 +676,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 2ca426ff76..fb3d7057df 100644 --- a/crypto/bn/bn_rand.c +++ b/crypto/bn/bn_rand.c @@ -260,20 +260,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) { @@ -283,41 +285,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 0ed6f823c1..e8244cd57b 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) { -- Gitee From ed8bc3bb655ab00efbaaa7b96d552da96de87b3f Mon Sep 17 00:00:00 2001 From: Tomas Mraz Date: Thu, 25 Apr 2024 15:35:36 +0200 Subject: [PATCH 2/6] Add ossl_bn_is_word_fixed_top() Also correct some BN_FLG_FIXED_TOP flag handling. Reviewed-by: Paul Dale Reviewed-by: Neil Horman (cherry picked from commit 2d285fa873028f6cff9484a0cdf690fe05d7fb16) (Merged from https://github.com/openssl/openssl/pull/24317) (cherry picked from commit 5dbb2a8ca2c1ba42dfb9445b5ea76adccbdb9744) Signed-off-by: lcc --- crypto/bn/bn_lib.c | 17 +++++++++++++++++ crypto/bn/bn_local.h | 1 - crypto/bn/bn_rand.c | 2 +- crypto/bn/bn_shift.c | 6 +++--- include/crypto/bn.h | 2 ++ include/internal/constant_time.h | 11 +++++++++++ 6 files changed, 34 insertions(+), 5 deletions(-) diff --git a/crypto/bn/bn_lib.c b/crypto/bn/bn_lib.c index bbfa882ad4..1d60015e0e 100644 --- a/crypto/bn/bn_lib.c +++ b/crypto/bn/bn_lib.c @@ -769,6 +769,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; } @@ -959,6 +960,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, 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 d355d1fce3..50e9d26e21 100644 --- a/crypto/bn/bn_local.h +++ b/crypto/bn/bn_local.h @@ -676,6 +676,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 fb3d7057df..b0b3d3ffe2 100644 --- a/crypto/bn/bn_rand.c +++ b/crypto/bn/bn_rand.c @@ -322,7 +322,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 8fcb04324e..a6976c7130 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/include/crypto/bn.h b/include/crypto/bn.h index fd1c09d997..d875ca9e9c 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 e8244cd57b..f2572ded51 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, -- Gitee From 029fd98d9b88878d1412d97d4673913c1aa392e8 Mon Sep 17 00:00:00 2001 From: Tomas Mraz Date: Thu, 25 Apr 2024 19:26:08 +0200 Subject: [PATCH 3/6] Add ossl_bn_priv_rand_range_fixed_top() and use it for EC/DSA Reviewed-by: Paul Dale Reviewed-by: Neil Horman (cherry picked from commit 13b3ca5c998e6db4f7251a56c43541cb1a422bd0) (Merged from https://github.com/openssl/openssl/pull/24317) (cherry picked from commit a70ca93cdbc0ed36bf783b9eadc4cea35986b139) Signed-off-by: lcc --- 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 b0b3d3ffe2..a362e33131 100644 --- a/crypto/bn/bn_rand.c +++ b/crypto/bn/bn_rand.c @@ -186,8 +186,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) { @@ -240,6 +240,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 62f7c70149..1629fad0b3 100644 --- a/crypto/dsa/dsa_ossl.c +++ b/crypto/dsa/dsa_ossl.c @@ -265,9 +265,9 @@ static int dsa_sign_setup(DSA *dsa, BN_CTX *ctx_in, if (!BN_generate_dsa_nonce(k, dsa->params.q, dsa->priv_key, dgst, 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 0bdf45e6e7..1d3ed66623 100644 --- a/crypto/ec/ecdsa_ossl.c +++ b/crypto/ec/ecdsa_ossl.c @@ -151,12 +151,12 @@ static int ecdsa_sign_setup(EC_KEY *eckey, BN_CTX *ctx_in, goto err; } } else { - if (!BN_priv_rand_range_ex(k, order, 0, ctx)) { + if (!ossl_bn_priv_rand_range_fixed_top(k, order, 0, ctx)) { 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 d875ca9e9c..99966e8651 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 -- Gitee From 50df12cf2891d3bccf6ccbed5322a80b86d10ac7 Mon Sep 17 00:00:00 2001 From: Tomas Mraz Date: Mon, 29 Apr 2024 17:56:01 +0200 Subject: [PATCH 4/6] Rename BN_generate_dsa_nonce() to ossl_bn_gen_dsa_nonce_fixed_top() And create a new BN_generate_dsa_nonce() that corrects the BIGNUM top. We do this to avoid leaking fixed top numbers via the public API. Also add a slight optimization in ossl_bn_gen_dsa_nonce_fixed_top() and make it LE/BE agnostic. Reviewed-by: Paul Dale Reviewed-by: Neil Horman (cherry picked from commit 9c85f6cd2d6debe5ef6ef475ff4bf17e0985f7a2) (Merged from https://github.com/openssl/openssl/pull/24317) (cherry picked from commit fdc3efc371be43d5092bb19823e084f54541cbe3) Signed-off-by: lcc --- crypto/bn/bn_rand.c | 41 +++++++++++++++++++++++++++++++---------- crypto/dsa/dsa_ossl.c | 5 +++-- crypto/ec/ecdsa_ossl.c | 4 ++-- include/crypto/bn.h | 4 ++++ 4 files changed, 40 insertions(+), 14 deletions(-) diff --git a/crypto/bn/bn_rand.c b/crypto/bn/bn_rand.c index a362e33131..420909e094 100644 --- a/crypto/bn/bn_rand.c +++ b/crypto/bn/bn_rand.c @@ -282,16 +282,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(); /* @@ -317,6 +318,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) { @@ -335,13 +338,15 @@ 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;) { + 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) @@ -355,10 +360,9 @@ int BN_generate_dsa_nonce(BIGNUM *out, const BIGNUM *range, todo = SHA512_DIGEST_LENGTH; memcpy(k_bytes + done, digest, todo); done += todo; + ++i; } - /* 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; @@ -383,3 +387,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 1629fad0b3..c5ff28485d 100644 --- a/crypto/dsa/dsa_ossl.c +++ b/crypto/dsa/dsa_ossl.c @@ -262,8 +262,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)) goto err; diff --git a/crypto/ec/ecdsa_ossl.c b/crypto/ec/ecdsa_ossl.c index 1d3ed66623..5d51ff9079 100644 --- a/crypto/ec/ecdsa_ossl.c +++ b/crypto/ec/ecdsa_ossl.c @@ -145,8 +145,8 @@ static int ecdsa_sign_setup(EC_KEY *eckey, BN_CTX *ctx_in, /* get random k */ do { if (dgst != NULL) { - if (!BN_generate_dsa_nonce(k, order, priv_key, - dgst, dlen, ctx)) { + if (!ossl_bn_gen_dsa_nonce_fixed_top(k, order, priv_key, + dgst, dlen, ctx)) { ERR_raise(ERR_LIB_EC, EC_R_RANDOM_NUMBER_GENERATION_FAILED); goto err; } diff --git a/include/crypto/bn.h b/include/crypto/bn.h index 99966e8651..c63f2ef77a 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 -- Gitee From 0956785efb2df1745e1a8955b524129dacba51a9 Mon Sep 17 00:00:00 2001 From: Tomas Mraz Date: Tue, 30 Apr 2024 11:46:26 +0200 Subject: [PATCH 5/6] Correct top for EC/DSA nonces if BN_DEBUG is on Otherwise following operations would bail out in bn_check_top(). Reviewed-by: Paul Dale Reviewed-by: Neil Horman (cherry picked from commit a380ae85be287045b1eaa64d23942101a426c080) (Merged from https://github.com/openssl/openssl/pull/24317) (cherry picked from commit 549208d1f1175aca5cc1ea989c4e9e4a41bc558c) Signed-off-by: lcc --- crypto/bn/bn_rand.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/crypto/bn/bn_rand.c b/crypto/bn/bn_rand.c index 420909e094..7fcd03a3cb 100644 --- a/crypto/bn/bn_rand.c +++ b/crypto/bn/bn_rand.c @@ -276,6 +276,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; @@ -372,6 +376,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; } } -- Gitee From b3adfa07b42ea1d490df0df8d60e705a687caba2 Mon Sep 17 00:00:00 2001 From: Tomas Mraz Date: Wed, 15 Jan 2025 18:27:02 +0100 Subject: [PATCH 6/6] Fix timing side-channel in ECDSA signature computation There is a timing signal of around 300 nanoseconds when the top word of the inverted ECDSA nonce value is zero. This can happen with significant probability only for some of the supported elliptic curves. In particular the NIST P-521 curve is affected. To be able to measure this leak, the attacker process must either be located in the same physical computer or must have a very fast network connection with low latency. Attacks on ECDSA nonce are also known as Minerva attack. Fixes CVE-2024-13176 Reviewed-by: Tim Hudson Reviewed-by: Neil Horman Reviewed-by: Paul Dale (Merged from https://github.com/openssl/openssl/pull/26429) (cherry picked from commit 63c40a66c5dc287485705d06122d3a6e74a6a203) Signed-off-by: lcc --- crypto/bn/bn_exp.c | 21 +++++++++++++++------ crypto/ec/ec_lib.c | 7 ++++--- include/crypto/bn.h | 3 +++ 3 files changed, 22 insertions(+), 9 deletions(-) diff --git a/crypto/bn/bn_exp.c b/crypto/bn/bn_exp.c index 4e169ae1f9..a161e58000 100644 --- a/crypto/bn/bn_exp.c +++ b/crypto/bn/bn_exp.c @@ -598,7 +598,7 @@ static int MOD_EXP_CTIME_COPY_FROM_PREBUF(BIGNUM *b, int top, * out by Colin Percival, * http://www.daemonology.net/hyperthreading-considered-harmful/) */ -int BN_mod_exp_mont_consttime(BIGNUM *rr, const BIGNUM *a, const BIGNUM *p, +int bn_mod_exp_mont_fixed_top(BIGNUM *rr, const BIGNUM *a, const BIGNUM *p, const BIGNUM *m, BN_CTX *ctx, BN_MONT_CTX *in_mont) { @@ -615,10 +615,6 @@ int BN_mod_exp_mont_consttime(BIGNUM *rr, const BIGNUM *a, const BIGNUM *p, unsigned int t4 = 0; #endif - bn_check_top(a); - bn_check_top(p); - bn_check_top(m); - if (!BN_is_odd(m)) { ERR_raise(ERR_LIB_BN, BN_R_CALLED_WITH_EVEN_MODULUS); return 0; @@ -1138,7 +1134,7 @@ int BN_mod_exp_mont_consttime(BIGNUM *rr, const BIGNUM *a, const BIGNUM *p, goto err; } else #endif - if (!BN_from_montgomery(rr, &tmp, mont, ctx)) + if (!bn_from_mont_fixed_top(rr, &tmp, mont, ctx)) goto err; ret = 1; err: @@ -1152,6 +1148,19 @@ int BN_mod_exp_mont_consttime(BIGNUM *rr, const BIGNUM *a, const BIGNUM *p, return ret; } +int BN_mod_exp_mont_consttime(BIGNUM *rr, const BIGNUM *a, const BIGNUM *p, + const BIGNUM *m, BN_CTX *ctx, + BN_MONT_CTX *in_mont) +{ + bn_check_top(a); + bn_check_top(p); + bn_check_top(m); + if (!bn_mod_exp_mont_fixed_top(rr, a, p, m, ctx, in_mont)) + return 0; + bn_correct_top(rr); + return 1; +} + int BN_mod_exp_mont_word(BIGNUM *rr, BN_ULONG a, const BIGNUM *p, const BIGNUM *m, BN_CTX *ctx, BN_MONT_CTX *in_mont) { diff --git a/crypto/ec/ec_lib.c b/crypto/ec/ec_lib.c index b1696d93bd..1f0bf1ec79 100644 --- a/crypto/ec/ec_lib.c +++ b/crypto/ec/ec_lib.c @@ -20,6 +20,7 @@ #include #include #include "crypto/ec.h" +#include "crypto/bn.h" #include "internal/nelem.h" #include "ec_local.h" @@ -1262,10 +1263,10 @@ static int ec_field_inverse_mod_ord(const EC_GROUP *group, BIGNUM *r, if (!BN_sub(e, group->order, e)) goto err; /*- - * Exponent e is public. - * No need for scatter-gather or BN_FLG_CONSTTIME. + * Although the exponent is public we want the result to be + * fixed top. */ - if (!BN_mod_exp_mont(r, x, e, group->order, ctx, group->mont_data)) + if (!bn_mod_exp_mont_fixed_top(r, x, e, group->order, ctx, group->mont_data)) goto err; ret = 1; diff --git a/include/crypto/bn.h b/include/crypto/bn.h index c63f2ef77a..f5a54a5139 100644 --- a/include/crypto/bn.h +++ b/include/crypto/bn.h @@ -73,6 +73,9 @@ int bn_set_words(BIGNUM *a, const BN_ULONG *words, int num_words); */ int bn_mul_mont_fixed_top(BIGNUM *r, const BIGNUM *a, const BIGNUM *b, BN_MONT_CTX *mont, BN_CTX *ctx); +int bn_mod_exp_mont_fixed_top(BIGNUM *rr, const BIGNUM *a, const BIGNUM *p, + const BIGNUM *m, BN_CTX *ctx, + BN_MONT_CTX *in_mont); int bn_to_mont_fixed_top(BIGNUM *r, const BIGNUM *a, BN_MONT_CTX *mont, BN_CTX *ctx); int bn_from_mont_fixed_top(BIGNUM *r, const BIGNUM *a, BN_MONT_CTX *mont, -- Gitee