From: Richard Levitte Date: Mon, 27 Oct 2025 12:57:56 +0000 (+0100) Subject: BIGNUM: Adjust the requirements on 'top' and the 'd' array for OSSL_FN compat X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=dd286f40abeba86e01be69bc9c3d8cb6dd7dd44b;p=thirdparty%2Fopenssl.git BIGNUM: Adjust the requirements on 'top' and the 'd' array for OSSL_FN compat BIGNUM is quite sloppy with its contents of the 'd' array above 'top'. This has been further exasperated by the 'bn_pollute' macro, which makes that slop quite explicit. That's fine within a purely BIGNUM context. Enter OSSL_FN, which requires that the whole 'd' array is numerically consistent, not just the BN_ULONGs up to 'top'. This will, of course, cause trouble as soon as an OSSL_FN that's integrated in a BIGNUM gets passed to OSSL_FN functions. To ensure consistency, the following updates are made: - [only for BIGNUMs in which 'data' is non-NULL] when decreasing 'top', all BN_ULONGs between the preceding 'top' and the new 'top' must be made zero. - Drop bn_pollute() entirely, as it's now more harmful than useful. - Modify bn_check_top() to better check the consistency of BIGNUM with integrated OSSL_FN, by checking that the part of the 'd' array between 'top' and 'dmax' is all zeroes. - Add the function 'bn_set_top()', which is recommended to use instead of assigning 'top' directly, as it will zeroise the intermediary limbs in the 'd' array when 'top' decreases. On using 'bn_set_top()', it's highly recommended to use it everywhere, unless you can be absolutely sure that the BIGNUM that's modified will never be checked with 'bn_check_top()' or passed to any OSSL_FN function. Related-to: doc/designs/fixed-size-large-numbers.md Reviewed-by: Dmitry Belyavskiy Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/29015) --- diff --git a/crypto/bn/bn_add.c b/crypto/bn/bn_add.c index 88d77c534f0..1784091b015 100644 --- a/crypto/bn/bn_add.c +++ b/crypto/bn/bn_add.c @@ -96,7 +96,7 @@ int BN_uadd(BIGNUM *r, const BIGNUM *a, const BIGNUM *b) if (bn_wexpand(r, max + 1) == NULL) return 0; - r->top = max; + bn_set_top(r, max); ap = a->d; bp = b->d; @@ -114,7 +114,7 @@ int BN_uadd(BIGNUM *r, const BIGNUM *a, const BIGNUM *b) carry &= (t2 == 0); } *rp = carry; - r->top += (int)carry; + bn_set_top(r, r->top + (int)carry); r->neg = 0; bn_check_top(r); @@ -162,9 +162,8 @@ int BN_usub(BIGNUM *r, const BIGNUM *a, const BIGNUM *b) while (max && *--rp == 0) max--; - r->top = max; + bn_set_top(r, max); r->neg = 0; - bn_pollute(r); return 1; } diff --git a/crypto/bn/bn_blind.c b/crypto/bn/bn_blind.c index 7722ed1e481..681b086b18f 100644 --- a/crypto/bn/bn_blind.c +++ b/crypto/bn/bn_blind.c @@ -185,7 +185,7 @@ int BN_BLINDING_invert_ex(BIGNUM *n, const BIGNUM *r, BN_BLINDING *b, } mask = (BN_ULONG)0 - ((rtop - ntop) >> (8 * sizeof(ntop) - 1)); /* always true, if (rtop >= ntop) n->top = r->top; */ - n->top = (int)((rtop & ~mask) | (ntop & mask)); + bn_set_top(n, (int)((rtop & ~mask) | (ntop & mask))); n->flags |= (BN_FLG_FIXED_TOP & ~mask); } ret = bn_mul_mont_fixed_top(n, n, r, b->m_ctx, ctx); diff --git a/crypto/bn/bn_conv.c b/crypto/bn/bn_conv.c index f295e5db9a9..ff49ebca810 100644 --- a/crypto/bn/bn_conv.c +++ b/crypto/bn/bn_conv.c @@ -178,7 +178,7 @@ int BN_hex2bn(BIGNUM **bn, const char *a) } j -= BN_BYTES * 2; } - ret->top = h; + bn_set_top(ret, h); bn_correct_top(ret); *bn = ret; diff --git a/crypto/bn/bn_div.c b/crypto/bn/bn_div.c index fc61d70a9da..6abe1b07fcc 100644 --- a/crypto/bn/bn_div.c +++ b/crypto/bn/bn_div.c @@ -61,7 +61,7 @@ int BN_div(BIGNUM *dv, BIGNUM *rem, const BIGNUM *m, const BIGNUM *d, BN_zero(dv); if (bn_wexpand(dv, 1) == NULL) goto end; - dv->top = 1; + bn_set_top(dv, 1); if (!BN_lshift(D, D, nm - nd)) goto end; @@ -310,7 +310,9 @@ int bn_div_fixed_top(BIGNUM *dv, BIGNUM *rm, const BIGNUM *num, if (bn_wexpand(snum, div_n + 1) == NULL) goto err; memset(&(snum->d[num_n]), 0, (div_n - num_n + 1) * sizeof(BN_ULONG)); - snum->top = num_n = div_n + 1; + num_n = div_n + 1; + bn_set_top(snum, num_n); + snum->flags |= BN_FLG_FIXED_TOP; } loop = num_n - div_n; @@ -330,13 +332,15 @@ int bn_div_fixed_top(BIGNUM *dv, BIGNUM *rm, const BIGNUM *num, goto err; num_neg = num->neg; res->neg = (num_neg ^ divisor->neg); - res->top = loop; + bn_set_top(res, loop); res->flags |= BN_FLG_FIXED_TOP; resp = &(res->d[loop]); /* space for temp */ if (!bn_wexpand(tmp, (div_n + 1))) goto err; + tmp->top = div_n + 1; + tmp->flags |= BN_FLG_FIXED_TOP; for (i = 0; i < loop; i++, wnumtop--) { BN_ULONG q, l0; @@ -446,7 +450,7 @@ int bn_div_fixed_top(BIGNUM *dv, BIGNUM *rm, const BIGNUM *num, } /* snum holds remainder, it's as wide as divisor */ snum->neg = num_neg; - snum->top = div_n; + bn_set_top(snum, div_n); snum->flags |= BN_FLG_FIXED_TOP; if (rm != NULL && bn_rshift_fixed_top(rm, snum, norm_shift) == 0) diff --git a/crypto/bn/bn_exp.c b/crypto/bn/bn_exp.c index f5231921b74..9015b1d0d53 100644 --- a/crypto/bn/bn_exp.c +++ b/crypto/bn/bn_exp.c @@ -402,7 +402,7 @@ int BN_mod_exp_mont(BIGNUM *rr, const BIGNUM *a, const BIGNUM *p, r->d[0] = (0 - m->d[0]) & BN_MASK2; for (i = 1; i < j; i++) r->d[i] = (~m->d[i]) & BN_MASK2; - r->top = j; + bn_set_top(r, j); r->flags |= BN_FLG_FIXED_TOP; } else #endif @@ -468,7 +468,8 @@ int BN_mod_exp_mont(BIGNUM *rr, const BIGNUM *a, const BIGNUM *p, val[0]->d[0] = 1; /* borrow val[0] */ for (i = 1; i < j; i++) val[0]->d[i] = 0; - val[0]->top = j; + bn_set_top(val[0], j); + val[0]->flags |= BN_FLG_FIXED_TOP; if (!BN_mod_mul_montgomery(rr, r, val[0], mont, ctx)) goto err; } else @@ -581,7 +582,7 @@ static int MOD_EXP_CTIME_COPY_FROM_PREBUF(BIGNUM *b, int top, } } - b->top = top; + bn_set_top(b, top); b->flags |= BN_FLG_FIXED_TOP; return 1; } @@ -681,7 +682,7 @@ int bn_mod_exp_mont_fixed_top(BIGNUM *rr, const BIGNUM *a, const BIGNUM *p, goto err; RSAZ_1024_mod_exp_avx2(rr->d, a->d, p->d, m->d, mont->RR.d, mont->n0[0]); - rr->top = 16; + bn_set_top(rr, 16); rr->neg = 0; bn_correct_top(rr); ret = 1; @@ -690,7 +691,7 @@ int bn_mod_exp_mont_fixed_top(BIGNUM *rr, const BIGNUM *a, const BIGNUM *p, if (NULL == bn_wexpand(rr, 8)) goto err; RSAZ_512_mod_exp(rr->d, a->d, p->d, m->d, mont->n0[0], mont->RR.d); - rr->top = 8; + bn_set_top(rr, 8); rr->neg = 0; bn_correct_top(rr); ret = 1; @@ -1488,12 +1489,12 @@ int BN_mod_exp_mont_consttime_x2(BIGNUM *rr1, const BIGNUM *a1, const BIGNUM *p1 mont2->RR.d, mont2->n0[0], mod_bits); - rr1->top = topn; + bn_set_top(rr1, topn); rr1->neg = 0; bn_correct_top(rr1); bn_check_top(rr1); - rr2->top = topn; + bn_set_top(rr2, topn); rr2->neg = 0; bn_correct_top(rr2); bn_check_top(rr2); diff --git a/crypto/bn/bn_gf2m.c b/crypto/bn/bn_gf2m.c index 7ca6587fb41..814b0da7bb1 100644 --- a/crypto/bn/bn_gf2m.c +++ b/crypto/bn/bn_gf2m.c @@ -267,7 +267,7 @@ int BN_GF2m_add(BIGNUM *r, const BIGNUM *a, const BIGNUM *b) r->d[i] = at->d[i]; } - r->top = at->top; + bn_set_top(r, at->top); bn_correct_top(r); return 1; @@ -305,7 +305,7 @@ int BN_GF2m_mod_arr(BIGNUM *r, const BIGNUM *a, const int p[]) for (j = 0; j < a->top; j++) { r->d[j] = a->d[j]; } - r->top = a->top; + bn_set_top(r, a->top); } z = r->d; @@ -419,7 +419,7 @@ int BN_GF2m_mod_mul_arr(BIGNUM *r, const BIGNUM *a, const BIGNUM *b, zlen = a->top + b->top + 4; if (!bn_wexpand(s, zlen)) goto err; - s->top = zlen; + bn_set_top(s, zlen); for (i = 0; i < zlen; i++) s->d[i] = 0; @@ -498,7 +498,7 @@ int BN_GF2m_mod_sqr_arr(BIGNUM *r, const BIGNUM *a, const int p[], s->d[2 * i] = SQR0(a->d[i]); } - s->top = 2 * a->top; + bn_set_top(s, 2 * a->top); bn_correct_top(s); if (!BN_GF2m_mod_arr(r, s, p)) goto err; @@ -618,20 +618,20 @@ static int BN_GF2m_mod_inv_vartime(BIGNUM *r, const BIGNUM *a, udp = u->d; for (i = u->top; i < top; i++) udp[i] = 0; - u->top = top; + bn_set_top(u, top); if (!bn_wexpand(b, top)) goto err; bdp = b->d; bdp[0] = 1; for (i = 1; i < top; i++) bdp[i] = 0; - b->top = top; + bn_set_top(b, top); if (!bn_wexpand(c, top)) goto err; cdp = c->d; for (i = 0; i < top; i++) cdp[i] = 0; - c->top = top; + bn_set_top(c, top); vdp = v->d; /* It pays off to "cache" *->d pointers, * because it allows optimizer to be more * aggressive. But we don't have to "cache" diff --git a/crypto/bn/bn_intern.c b/crypto/bn/bn_intern.c index e369a167b95..dcd9e8b3dbf 100644 --- a/crypto/bn/bn_intern.c +++ b/crypto/bn/bn_intern.c @@ -176,6 +176,7 @@ void bn_set_static_words(BIGNUM *a, const BN_ULONG *words, int size) */ a->data = NULL; a->d = (BN_ULONG *)words; + /* No need to call bn_set_top() in this case */ a->dmax = a->top = size; a->neg = 0; a->flags |= BN_FLG_STATIC_DATA; @@ -189,8 +190,11 @@ int bn_set_words(BIGNUM *a, const BN_ULONG *words, int num_words) return 0; } + /* TODO(FIXNUM): In the future, we'll use an OSSL_FN function on a->data */ memcpy(a->d, words, sizeof(BN_ULONG) * num_words); - a->top = num_words; + + /* TODO(FIXNUM): The following two lines are TO BE REMOVED */ + bn_set_top(a, num_words); bn_correct_top(a); return 1; } diff --git a/crypto/bn/bn_lib.c b/crypto/bn/bn_lib.c index c06bf11c84c..0bcda684e3a 100644 --- a/crypto/bn/bn_lib.c +++ b/crypto/bn/bn_lib.c @@ -372,7 +372,7 @@ BIGNUM *BN_copy(BIGNUM *a, const BIGNUM *b) ossl_fn_copy_internal_limbs(a->data, b->d, bn_words); } a->neg = b->neg; - a->top = b->top; + bn_set_top(a, b->top); a->flags |= b->flags & BN_FLG_FIXED_TOP; bn_check_top(a); return a; @@ -428,7 +428,7 @@ void BN_clear(BIGNUM *a) else if (a->d != NULL) OPENSSL_cleanse(a->d, sizeof(*a->d) * a->dmax); a->neg = 0; - a->top = 0; + bn_set_top(a, 0); a->flags &= ~BN_FLG_FIXED_TOP; } @@ -449,7 +449,7 @@ int BN_set_word(BIGNUM *a, BN_ULONG w) return 0; a->neg = 0; a->d[0] = w; - a->top = (w ? 1 : 0); + bn_set_top(a, (w ? 1 : 0)); a->flags &= ~BN_FLG_FIXED_TOP; bn_check_top(a); return 1; @@ -531,7 +531,7 @@ static BIGNUM *bin2bn(const unsigned char *s, int len, BIGNUM *ret, } /* If it was all zeros, we're done */ if (len == 0) { - ret->top = 0; + bn_set_top(ret, 0); return ret; } n = ((len - 1) / BN_BYTES) + 1; /* Number of resulting bignum chunks */ @@ -539,7 +539,7 @@ static BIGNUM *bin2bn(const unsigned char *s, int len, BIGNUM *ret, BN_free(bn); return NULL; } - ret->top = n; + bn_set_top(ret, n); ret->neg = neg; for (i = 0; n-- > 0; i++) { BN_ULONG l = 0; /* Accumulator */ @@ -820,7 +820,7 @@ int BN_cmp(const BIGNUM *a, const BIGNUM *b) int BN_set_bit(BIGNUM *a, int n) { - int i, j, k; + int i, j; if (n < 0) return 0; @@ -830,9 +830,12 @@ int BN_set_bit(BIGNUM *a, int n) if (a->top <= i) { if (bn_wexpand(a, i + 1) == NULL) return 0; - for (k = a->top; k < i + 1; k++) - a->d[k] = 0; - a->top = i + 1; + /* + * If 'a' is actually expanded, we know that the expanded + * part of the 'd' array is zeroed during allocation, so + * no need to zero it again here. + */ + bn_set_top(a, i + 1); a->flags &= ~BN_FLG_FIXED_TOP; } @@ -885,9 +888,9 @@ int ossl_bn_mask_bits_fixed_top(BIGNUM *a, int n) if (w >= a->top) return 0; if (b == 0) - a->top = w; + bn_set_top(a, w); else { - a->top = w + 1; + bn_set_top(a, w + 1); a->d[w] &= ~(BN_MASK2 << b); } a->flags |= BN_FLG_FIXED_TOP; @@ -1054,7 +1057,7 @@ int BN_security_bits(int L, int N) void BN_zero_ex(BIGNUM *a) { a->neg = 0; - a->top = 0; + bn_set_top(a, 0); a->flags &= ~BN_FLG_FIXED_TOP; } @@ -1065,7 +1068,12 @@ int BN_abs_is_word(const BIGNUM *a, const BN_ULONG w) int BN_is_zero(const BIGNUM *a) { - return a->top == 0; + if ((a->flags & BN_FLG_FIXED_TOP) == 0) + return a->top == 0; + for (size_t i = a->top; i-- > 0;) + if (a->d[i] != (BN_ULONG)0) + return 0; + return 1; } int BN_is_one(const BIGNUM *a) @@ -1196,6 +1204,11 @@ void bn_correct_top_consttime(BIGNUM *a) } mask = constant_time_eq_int(atop, 0); + /* + * We just went through the whole 'd' array to identify where + * any leading set of zeros are located, so there's no need to + * call bn_set_top() here. + */ a->top = atop; a->neg = constant_time_select_int(mask, 0, a->neg); a->flags &= ~BN_FLG_FIXED_TOP; @@ -1212,10 +1225,14 @@ void bn_correct_top(BIGNUM *a) if (*ftl != 0) break; } + /* + * We just verified that the BN_ULONGs between a->top and + * tmp_top are all zero, so there's no need to call + * bn_set_top() here. + */ a->top = tmp_top; } if (a->top == 0) a->neg = 0; a->flags &= ~BN_FLG_FIXED_TOP; - bn_pollute(a); } diff --git a/crypto/bn/bn_local.h b/crypto/bn/bn_local.h index 7980dee76d0..daa28216ca5 100644 --- a/crypto/bn/bn_local.h +++ b/crypto/bn/bn_local.h @@ -10,6 +10,9 @@ #ifndef OSSL_CRYPTO_BN_LOCAL_H #define OSSL_CRYPTO_BN_LOCAL_H +#include +#include + #include #include "internal/cryptlib.h" #include "internal/numbers.h" @@ -18,21 +21,13 @@ #include "../fn/fn_local.h" /* - * These preprocessor symbols control various aspects of the bignum headers - * and library code. They're not defined by any "normal" configuration, as - * they are intended for development and testing purposes. NB: defining - * them can be useful for debugging application code as well as openssl - * itself. BN_DEBUG - turn on various debugging alterations to the bignum - * code BN_RAND_DEBUG - uses random poisoning of unused words to trip up - * mismanagement of bignum internals. Enable BN_RAND_DEBUG is known to - * break some of the OpenSSL tests. + * BN_RAND_DEBUG was historically used to poison unused words in bignum data, + * for integrity debugging purposes. This isn't done any more, but enabling + * BN_RAND_DEBUG also defined BN_DEBUG, which we preserve for the moment. */ #if defined(BN_RAND_DEBUG) && !defined(BN_DEBUG) #define BN_DEBUG #endif -#if defined(BN_RAND_DEBUG) -#include -#endif /* * This should limit the stack usage due to alloca to about 4K. @@ -130,30 +125,37 @@ /*- * Bignum consistency macros + * * There is one "API" macro, bn_fix_top(), for stripping leading zeroes from * bignum data after direct manipulations on the data. There is also an * "internal" macro, bn_check_top(), for verifying that there are no leading - * zeroes. Unfortunately, some auditing is required due to the fact that - * bn_fix_top() has become an overabused duct-tape because bignum data is - * occasionally passed around in an inconsistent state. So the following - * changes have been made to sort this out; + * zeroes, and in case the BIGNUM has an integrated OSSL_FN, check the + * consistency of the integration, including that the unused part of the + * data is all zeros. + * + * Unfortunately, some auditing is required due to the fact that bn_fix_top() + * has become an overabused duck-tape because bignum data is occasionally + * passed around in an inconsistent state. So the following changes have been + * made to sort this out; + * * - bn_fix_top()s implementation has been moved to bn_correct_top() - * - if BN_DEBUG isn't defined, bn_fix_top() maps to bn_correct_top(), and - * bn_check_top() is as before. - * - if BN_DEBUG *is* defined; - * - bn_check_top() tries to pollute unused words even if the bignum 'top' is - * consistent. (ed: only if BN_RAND_DEBUG is defined) + * - if BN_DEBUG isn't defined: + * - bn_check_top() does nothing. + * - bn_fix_top() maps to bn_correct_top() + * - if BN_DEBUG is defined: + * - bn_check_top() performs its consistency checks * - bn_fix_top() maps to bn_check_top() rather than "fixing" anything. + * * The idea is to have debug builds flag up inconsistent bignums when they - * occur. If that occurs in a bn_fix_top(), we examine the code in question; if - * the use of bn_fix_top() was appropriate (ie. it follows directly after code - * that manipulates the bignum) it is converted to bn_correct_top(), and if it - * was not appropriate, we convert it permanently to bn_check_top() and track - * down the cause of the bug. Eventually, no internal code should be using the - * bn_fix_top() macro. External applications and libraries should try this with - * their own code too, both in terms of building against the openssl headers - * with BN_DEBUG defined *and* linking with a version of OpenSSL built with it - * defined. This not only improves external code, it provides more test + * occur. If that occurs in a bn_fix_top(), we examine the code in question; + * if the use of bn_fix_top() was appropriate (ie. it follows directly after + * code that manipulates the bignum) it is converted to bn_correct_top(), + * and if it was not appropriate, we convert it permanently to bn_check_top() + * and track down the cause of the bug. Eventually, no internal code should be + * using the bn_fix_top() macro. External applications and libraries should try + * this with their own code too, both in terms of building against the openssl + * headers with BN_DEBUG defined *and* linking with a version of OpenSSL built + * with it defined. This not only improves external code, it provides more test * coverage for openssl's own code. */ @@ -172,38 +174,37 @@ * all operations manipulating the bit in question in non-BN_DEBUG build. */ #define BN_FLG_FIXED_TOP 0x10000 -#ifdef BN_RAND_DEBUG -#define bn_pollute(a) \ - do { \ - const BIGNUM *_bnum1 = (a); \ - if (_bnum1->top < _bnum1->dmax) { \ - unsigned char _tmp_char; \ - /* We cast away const without the compiler knowing, any \ - * *genuinely* constant variables that aren't mutable \ - * wouldn't be constructed with top!=dmax. */ \ - BN_ULONG *_not_const; \ - memcpy(&_not_const, &_bnum1->d, sizeof(_not_const)); \ - (void)RAND_bytes(&_tmp_char, 1); /* Debug only - safe to ignore error return */ \ - memset(_not_const + _bnum1->top, _tmp_char, \ - sizeof(*_not_const) * (_bnum1->dmax - _bnum1->top)); \ - } \ - } while (0) -#else -#define bn_pollute(a) -#endif -#define bn_check_top(a) \ - do { \ - const BIGNUM *_bnum2 = (a); \ - if (_bnum2 != NULL) { \ - int _top = _bnum2->top; \ - /* BIGNUM <-> OSSL_FN compat checks */ \ - assert((_bnum2->data == NULL /* && _bnum2->d == NULL */) \ - || (_bnum2->d == _bnum2->data->d \ - && _bnum2->dmax == _bnum2->data->dsize)); \ - /* BIGNUM specific checks */ \ - assert((_top == 0 && !_bnum2->neg) || (_top && ((_bnum2->flags & BN_FLG_FIXED_TOP) || _bnum2->d[_top - 1] != 0))); \ - bn_pollute(_bnum2); \ - } \ + +static ossl_inline bool bn_check_zero(BN_ULONG *words, int num_words) +{ + for (int i = 0; i < num_words; i++) + if (words[i] != 0) + return false; + return true; +} + +#define bn_check_top(a) \ + do { \ + const BIGNUM *_bnum2 = (a); \ + if (_bnum2 != NULL) { \ + int _top = _bnum2->top; \ + int _dmax = _bnum2->dmax; \ + BN_ULONG *_d = _bnum2->d; \ + /* BIGNUM <-> OSSL_FN compat checks */ \ + if (_bnum2->data != NULL) { \ + /* Assertion for the future */ \ + /* assert(_bnum2->d == NULL); */ \ + assert(_d == _bnum2->data->d); \ + assert(_dmax == _bnum2->data->dsize); \ + assert(bn_check_zero(&_d[_top], _dmax - _top)); \ + } \ + /* BIGNUM specific checks */ \ + if (_top == 0) { \ + assert(!_bnum2->neg); \ + } else if ((_bnum2->flags & BN_FLG_FIXED_TOP) == 0) { \ + assert(_bnum2->d[_top - 1] != 0); \ + } \ + } \ } while (0) #define bn_fix_top(a) bn_check_top(a) @@ -220,7 +221,6 @@ #else /* !BN_DEBUG */ #define BN_FLG_FIXED_TOP 0 -#define bn_pollute(a) #define bn_check_top(a) #define bn_fix_top(a) bn_correct_top(a) #define bn_check_size(bn, bits) @@ -699,4 +699,29 @@ 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); +/** + * Set top on a given BIGNUM. If it has an associated OSSL_FN (the 'data' + * field is non-NULL), and the new 'top' is less than the existing 'top', + * zeroise the space between them. + * + * @param[in] b The BIGNUM instance to zeroise + * @param[in] newtop The new 'top' + * @returns the new 'top' + * @pre b must not be NULL and newtop must be zero or positive + */ +static ossl_inline int bn_set_top(BIGNUM *b, int newtop) +{ + assert(b != NULL && newtop >= 0); + + if (b->data != NULL && newtop < b->top) { + BN_ULONG *start = &(b->d[newtop]); + size_t bytes = sizeof(BN_ULONG) * (b->top - newtop); + + memset(start, 0, bytes); + } + + b->top = newtop; + return b->top; +} + #endif diff --git a/crypto/bn/bn_mod.c b/crypto/bn/bn_mod.c index ae087295459..18c5d6f0684 100644 --- a/crypto/bn/bn_mod.c +++ b/crypto/bn/bn_mod.c @@ -90,7 +90,7 @@ int bn_mod_add_fixed_top(BIGNUM *r, const BIGNUM *a, const BIGNUM *b, rp[i] = (carry & tp[i]) | (~carry & rp[i]); ((volatile BN_ULONG *)tp)[i] = 0; } - r->top = (int)mtop; + bn_set_top(r, (int)mtop); r->flags |= BN_FLG_FIXED_TOP; r->neg = 0; @@ -176,7 +176,7 @@ int bn_mod_sub_fixed_top(BIGNUM *r, const BIGNUM *a, const BIGNUM *b, carry += (rp[i] < ta); } - r->top = (int)mtop; + bn_set_top(r, (int)mtop); r->flags |= BN_FLG_FIXED_TOP; r->neg = 0; diff --git a/crypto/bn/bn_mont.c b/crypto/bn/bn_mont.c index 384e64c3f82..8173554c5f3 100644 --- a/crypto/bn/bn_mont.c +++ b/crypto/bn/bn_mont.c @@ -47,7 +47,7 @@ int bn_mul_mont_fixed_top(BIGNUM *r, const BIGNUM *a, const BIGNUM *b, return 0; if (bn_mul_mont(r->d, a->d, b->d, mont->N.d, mont->n0, num)) { r->neg = a->neg ^ b->neg; - r->top = num; + bn_set_top(r, num); r->flags |= BN_FLG_FIXED_TOP; return 1; } @@ -95,7 +95,7 @@ static int bn_from_montgomery_word(BIGNUM *ret, BIGNUM *r, BN_MONT_CTX *mont) n = &(mont->N); nl = n->top; if (nl == 0) { - ret->top = 0; + bn_set_top(ret, 0); return 1; } @@ -113,7 +113,7 @@ static int bn_from_montgomery_word(BIGNUM *ret, BIGNUM *r, BN_MONT_CTX *mont) rp[i] &= v; } - r->top = max; + bn_set_top(r, max); r->flags |= BN_FLG_FIXED_TOP; n0 = mont->n0[0]; @@ -132,7 +132,7 @@ static int bn_from_montgomery_word(BIGNUM *ret, BIGNUM *r, BN_MONT_CTX *mont) if (bn_wexpand(ret, nl) == NULL) return 0; - ret->top = nl; + bn_set_top(ret, nl); ret->flags |= BN_FLG_FIXED_TOP; ret->neg = r->neg; @@ -326,7 +326,7 @@ int BN_MONT_CTX_set(BN_MONT_CTX *mont, const BIGNUM *mod, BN_CTX *ctx) Ri->neg = 0; Ri->d[0] = BN_MASK2; Ri->d[1] = BN_MASK2; - Ri->top = 2; + bn_set_top(Ri, 2); } if (!BN_div(Ri, NULL, Ri, &tmod, ctx)) goto err; diff --git a/crypto/bn/bn_mpi.c b/crypto/bn/bn_mpi.c index d2a86a3eada..17ffc6a476f 100644 --- a/crypto/bn/bn_mpi.c +++ b/crypto/bn/bn_mpi.c @@ -65,7 +65,7 @@ BIGNUM *BN_mpi2bn(const unsigned char *d, int n, BIGNUM *ain) if (len == 0) { a->neg = 0; - a->top = 0; + bn_set_top(a, 0); return a; } d += 4; diff --git a/crypto/bn/bn_mul.c b/crypto/bn/bn_mul.c index ff4bf2ca256..f6f1d4718e0 100644 --- a/crypto/bn/bn_mul.c +++ b/crypto/bn/bn_mul.c @@ -546,6 +546,7 @@ int bn_mul_fixed_top(BIGNUM *r, const BIGNUM *a, const BIGNUM *b, BN_CTX *ctx) if (al == 4) { if (bn_wexpand(rr, 8) == NULL) goto err; + rr->flags |= BN_FLG_FIXED_TOP; rr->top = 8; bn_mul_comba4(rr->d, a->d, b->d); goto end; @@ -554,6 +555,7 @@ int bn_mul_fixed_top(BIGNUM *r, const BIGNUM *a, const BIGNUM *b, BN_CTX *ctx) if (al == 8) { if (bn_wexpand(rr, 16) == NULL) goto err; + rr->flags |= BN_FLG_FIXED_TOP; rr->top = 16; bn_mul_comba8(rr->d, a->d, b->d); goto end; @@ -582,26 +584,34 @@ int bn_mul_fixed_top(BIGNUM *r, const BIGNUM *a, const BIGNUM *b, BN_CTX *ctx) if (al > j || bl > j) { if (bn_wexpand(t, k * 4) == NULL) goto err; + t->top = k * 4; + t->flags |= BN_FLG_FIXED_TOP; if (bn_wexpand(rr, k * 4) == NULL) goto err; + rr->top = k * 4; + rr->flags |= BN_FLG_FIXED_TOP; bn_mul_part_recursive(rr->d, a->d, b->d, j, al - j, bl - j, t->d); } else { /* al <= j || bl <= j */ if (bn_wexpand(t, k * 2) == NULL) goto err; + t->top = k * 2; + t->flags |= BN_FLG_FIXED_TOP; if (bn_wexpand(rr, k * 2) == NULL) goto err; + rr->top = k * 2; + rr->flags |= BN_FLG_FIXED_TOP; bn_mul_recursive(rr->d, a->d, b->d, j, al - j, bl - j, t->d); } - rr->top = top; + bn_set_top(rr, top); goto end; } } #endif /* BN_RECURSION */ if (bn_wexpand(rr, top) == NULL) goto err; - rr->top = top; + bn_set_top(rr, top); bn_mul_normal(rr->d, a->d, al, b->d, bl); #if defined(BN_MUL_COMBA) || defined(BN_RECURSION) diff --git a/crypto/bn/bn_nist.c b/crypto/bn/bn_nist.c index d6a27aa66e4..93ffb4241b4 100644 --- a/crypto/bn/bn_nist.c +++ b/crypto/bn/bn_nist.c @@ -457,7 +457,7 @@ int BN_nist_mod_192(BIGNUM *r, const BIGNUM *a, const BIGNUM *field, ? r_d : c_d; nist_cp_bn(r_d, res, BN_NIST_192_TOP); - r->top = BN_NIST_192_TOP; + bn_set_top(r, BN_NIST_192_TOP); bn_correct_top(r); return 1; @@ -628,7 +628,7 @@ int BN_nist_mod_224(BIGNUM *r, const BIGNUM *a, const BIGNUM *field, ? r_d : c_d; nist_cp_bn(r_d, res, BN_NIST_224_TOP); - r->top = BN_NIST_224_TOP; + bn_set_top(r, BN_NIST_224_TOP); bn_correct_top(r); return 1; @@ -857,7 +857,7 @@ int BN_nist_mod_256(BIGNUM *r, const BIGNUM *a, const BIGNUM *field, ? r_d : c_d; nist_cp_bn(r_d, res, BN_NIST_256_TOP); - r->top = BN_NIST_256_TOP; + bn_set_top(r, BN_NIST_256_TOP); bn_correct_top(r); return 1; @@ -1124,7 +1124,7 @@ int BN_nist_mod_384(BIGNUM *r, const BIGNUM *a, const BIGNUM *field, ? r_d : c_d; nist_cp_bn(r_d, res, BN_NIST_384_TOP); - r->top = BN_NIST_384_TOP; + bn_set_top(r, BN_NIST_384_TOP); bn_correct_top(r); return 1; @@ -1195,7 +1195,7 @@ int BN_nist_mod_521(BIGNUM *r, const BIGNUM *a, const BIGNUM *field, ? r_d : t_d; nist_cp_bn(r_d, res, BN_NIST_521_TOP); - r->top = BN_NIST_521_TOP; + bn_set_top(r, BN_NIST_521_TOP); bn_correct_top(r); return 1; diff --git a/crypto/bn/bn_shift.c b/crypto/bn/bn_shift.c index 1ba635096e2..2e6f7c7401f 100644 --- a/crypto/bn/bn_shift.c +++ b/crypto/bn/bn_shift.c @@ -23,7 +23,7 @@ int BN_lshift1(BIGNUM *r, const BIGNUM *a) r->neg = a->neg; if (bn_wexpand(r, a->top + 1) == NULL) return 0; - r->top = a->top; + bn_set_top(r, a->top); } else { if (bn_wexpand(r, a->top + 1) == NULL) return 0; @@ -37,7 +37,7 @@ int BN_lshift1(BIGNUM *r, const BIGNUM *a) c = t >> (BN_BITS2 - 1); } *rp = c; - r->top += (int)c; + bn_set_top(r, r->top + (int)c); bn_check_top(r); return 1; } @@ -62,11 +62,10 @@ int BN_rshift1(BIGNUM *r, const BIGNUM *a) r->neg = a->neg; } rp = r->d; - r->top = i; t = ap[--i]; rp[i] = t >> 1; c = t << (BN_BITS2 - 1); - r->top -= (t == 1); + bn_set_top(r, i + (t > 1)); while (i > 0) { t = ap[--i]; rp[i] = ((t >> 1) & BN_MASK2) | c; @@ -141,7 +140,7 @@ int bn_lshift_fixed_top(BIGNUM *r, const BIGNUM *a, int n) memset(r->d, 0, sizeof(*t) * nw); r->neg = a->neg; - r->top = a->top + nw + 1; + bn_set_top(r, a->top + nw + 1); r->flags |= BN_FLG_FIXED_TOP; return 1; @@ -209,7 +208,7 @@ int bn_rshift_fixed_top(BIGNUM *r, const BIGNUM *a, int n) t[i] = l >> rb; r->neg = a->neg; - r->top = top; + bn_set_top(r, top); r->flags |= BN_FLG_FIXED_TOP; return 1; diff --git a/crypto/bn/bn_sqr.c b/crypto/bn/bn_sqr.c index 34b33a30a3b..078d007011f 100644 --- a/crypto/bn/bn_sqr.c +++ b/crypto/bn/bn_sqr.c @@ -34,7 +34,7 @@ int bn_sqr_fixed_top(BIGNUM *r, const BIGNUM *a, BN_CTX *ctx) al = a->top; if (al <= 0) { - r->top = 0; + bn_set_top(r, 0); r->neg = 0; return 1; } @@ -91,8 +91,12 @@ int bn_sqr_fixed_top(BIGNUM *r, const BIGNUM *a, BN_CTX *ctx) #endif } + /* Ensure that tmp won't cause any trouble */ + tmp->top = tmp->dmax; + tmp->flags |= BN_FLG_FIXED_TOP; + rr->neg = 0; - rr->top = max; + bn_set_top(rr, max); rr->flags |= BN_FLG_FIXED_TOP; if (r != rr && BN_copy(r, rr) == NULL) goto err; @@ -100,7 +104,6 @@ int bn_sqr_fixed_top(BIGNUM *r, const BIGNUM *a, BN_CTX *ctx) ret = 1; err: bn_check_top(rr); - bn_check_top(tmp); BN_CTX_end(ctx); return ret; }