From: Nicola Tuveri Date: Thu, 1 Aug 2019 22:33:05 +0000 (+0300) Subject: Fix a SCA leak using BN_bn2bin() X-Git-Tag: OpenSSL_1_0_2t~15 X-Git-Url: https://git.librecmc.org/?a=commitdiff_plain;h=2e9d293447b95c2a69eb5ff07fe974361d779444;p=oweals%2Fopenssl.git Fix a SCA leak using BN_bn2bin() BN_bn2bin() is not constant-time and leaks the number of bits in the processed BIGNUM. The specialized methods in ecp_nistp224.c, ecp_nistp256.c and ecp_nistp521.c internally used BN_bn2bin() to convert scalars into the internal fixed length representation. This can leak during ECDSA/ECDH key generation or handling the nonce while generating an ECDSA signature, when using these implementations. The amount and risk of leaked information useful for a SCA attack varies for each of the three curves, as it depends mainly on the ratio between the bitlength of the curve subgroup order (governing the size of the secret nonce/key) and the limb size for the internal BIGNUM representation (which depends on the compilation target architecture). To fix this, we replace BN_bn2bin() with bn_bn2binpad(), bounding the output length to the width of the internal representation buffer: this length is public. Internally the final implementation of both bn_bn2binpad() and BN_bn2bin() already has masking in place to avoid leaking bn->top through memory access patterns. Memory access pattern still leaks bn->dmax, the size of the lazily allocated buffer for representing the BIGNUM, which is inevitable with the current BIGNUM architecture: reading past bn->dmax would be an out-of-bound read. As such, it's the caller responsibility to ensure that bn->dmax does not leak secret information, by explicitly expanding the internal BIGNUM buffer to a public value sufficient to avoid any lazy reallocation while manipulating it: this is already done at the top level alongside setting the BN_FLG_CONSTTIME. Finally, the internal implementation of bn_bn2binpad() indirectly calls BN_num_bits() via BN_num_bytes(): the current implementation of BN_num_bits() can leak information to a SCA attacker, and is addressed in the next commit. Thanks to David Schrammel and Samuel Weiser for reporting this issue through responsible disclosure. Reviewed-by: Matt Caswell Reviewed-by: Bernd Edlinger (Merged from https://github.com/openssl/openssl/pull/9793) --- diff --git a/crypto/ec/ecp_nistp224.c b/crypto/ec/ecp_nistp224.c index 121f587b58..877edc9cf0 100644 --- a/crypto/ec/ecp_nistp224.c +++ b/crypto/ec/ecp_nistp224.c @@ -37,6 +37,7 @@ # include # include # include "ec_lcl.h" +# include "bn_int.h" /* bn_bn2binpad */ # if defined(__GNUC__) && (__GNUC__ > 3 || (__GNUC__ == 3 && __GNUC_MINOR__ >= 1)) /* even with gcc, the typedef won't work for 32-bit platforms */ @@ -349,8 +350,6 @@ static int BN_to_felem(felem out, const BIGNUM *bn) felem_bytearray b_out; unsigned num_bytes; - /* BN_bn2bin eats leading zeroes */ - memset(b_out, 0, sizeof(b_out)); num_bytes = BN_num_bytes(bn); if (num_bytes > sizeof(b_out)) { ECerr(EC_F_BN_TO_FELEM, EC_R_BIGNUM_OUT_OF_RANGE); @@ -360,7 +359,7 @@ static int BN_to_felem(felem out, const BIGNUM *bn) ECerr(EC_F_BN_TO_FELEM, EC_R_BIGNUM_OUT_OF_RANGE); return 0; } - num_bytes = BN_bn2bin(bn, b_in); + num_bytes = bn_bn2binpad(bn, b_in, sizeof(b_in)); flip_endian(b_out, b_in, num_bytes); bin28_to_felem(out, b_out); return 1; @@ -1532,9 +1531,9 @@ int ec_GFp_nistp224_points_mul(const EC_GROUP *group, EC_POINT *r, ECerr(EC_F_EC_GFP_NISTP224_POINTS_MUL, ERR_R_BN_LIB); goto err; } - num_bytes = BN_bn2bin(tmp_scalar, tmp); + num_bytes = bn_bn2binpad(tmp_scalar, tmp, sizeof(tmp)); } else - num_bytes = BN_bn2bin(p_scalar, tmp); + num_bytes = bn_bn2binpad(p_scalar, tmp, sizeof(tmp)); flip_endian(secrets[i], tmp, num_bytes); /* precompute multiples */ if ((!BN_to_felem(x_out, &p->X)) || @@ -1578,9 +1577,9 @@ int ec_GFp_nistp224_points_mul(const EC_GROUP *group, EC_POINT *r, ECerr(EC_F_EC_GFP_NISTP224_POINTS_MUL, ERR_R_BN_LIB); goto err; } - num_bytes = BN_bn2bin(tmp_scalar, tmp); + num_bytes = bn_bn2binpad(tmp_scalar, tmp, sizeof(tmp)); } else - num_bytes = BN_bn2bin(scalar, tmp); + num_bytes = bn_bn2binpad(scalar, tmp, sizeof(tmp)); flip_endian(g_secret, tmp, num_bytes); /* do the multiplication with generator precomputation */ batch_mul(x_out, y_out, z_out, diff --git a/crypto/ec/ecp_nistp256.c b/crypto/ec/ecp_nistp256.c index 378f0bae08..a9b2c062a9 100644 --- a/crypto/ec/ecp_nistp256.c +++ b/crypto/ec/ecp_nistp256.c @@ -38,6 +38,7 @@ # include # include # include "ec_lcl.h" +# include "bn_int.h" /* bn_bn2binpad */ # if defined(__GNUC__) && (__GNUC__ > 3 || (__GNUC__ == 3 && __GNUC_MINOR__ >= 1)) /* even with gcc, the typedef won't work for 32-bit platforms */ @@ -159,8 +160,6 @@ static int BN_to_felem(felem out, const BIGNUM *bn) felem_bytearray b_out; unsigned num_bytes; - /* BN_bn2bin eats leading zeroes */ - memset(b_out, 0, sizeof(b_out)); num_bytes = BN_num_bytes(bn); if (num_bytes > sizeof(b_out)) { ECerr(EC_F_BN_TO_FELEM, EC_R_BIGNUM_OUT_OF_RANGE); @@ -170,7 +169,7 @@ static int BN_to_felem(felem out, const BIGNUM *bn) ECerr(EC_F_BN_TO_FELEM, EC_R_BIGNUM_OUT_OF_RANGE); return 0; } - num_bytes = BN_bn2bin(bn, b_in); + num_bytes = bn_bn2binpad(bn, b_in, sizeof(b_in)); flip_endian(b_out, b_in, num_bytes); bin32_to_felem(out, b_out); return 1; @@ -2123,9 +2122,9 @@ int ec_GFp_nistp256_points_mul(const EC_GROUP *group, EC_POINT *r, ECerr(EC_F_EC_GFP_NISTP256_POINTS_MUL, ERR_R_BN_LIB); goto err; } - num_bytes = BN_bn2bin(tmp_scalar, tmp); + num_bytes = bn_bn2binpad(tmp_scalar, tmp, sizeof(tmp)); } else - num_bytes = BN_bn2bin(p_scalar, tmp); + num_bytes = bn_bn2binpad(p_scalar, tmp, sizeof(tmp)); flip_endian(secrets[i], tmp, num_bytes); /* precompute multiples */ if ((!BN_to_felem(x_out, &p->X)) || @@ -2171,9 +2170,9 @@ int ec_GFp_nistp256_points_mul(const EC_GROUP *group, EC_POINT *r, ECerr(EC_F_EC_GFP_NISTP256_POINTS_MUL, ERR_R_BN_LIB); goto err; } - num_bytes = BN_bn2bin(tmp_scalar, tmp); + num_bytes = bn_bn2binpad(tmp_scalar, tmp, sizeof(tmp)); } else - num_bytes = BN_bn2bin(scalar, tmp); + num_bytes = bn_bn2binpad(scalar, tmp, sizeof(tmp)); flip_endian(g_secret, tmp, num_bytes); /* do the multiplication with generator precomputation */ batch_mul(x_out, y_out, z_out, diff --git a/crypto/ec/ecp_nistp521.c b/crypto/ec/ecp_nistp521.c index 1a42068c01..9765e9866d 100644 --- a/crypto/ec/ecp_nistp521.c +++ b/crypto/ec/ecp_nistp521.c @@ -38,6 +38,7 @@ # include # include # include "ec_lcl.h" +# include "bn_int.h" /* bn_bn2binpad */ # if defined(__GNUC__) && (__GNUC__ > 3 || (__GNUC__ == 3 && __GNUC_MINOR__ >= 1)) /* even with gcc, the typedef won't work for 32-bit platforms */ @@ -183,8 +184,6 @@ static int BN_to_felem(felem out, const BIGNUM *bn) felem_bytearray b_out; unsigned num_bytes; - /* BN_bn2bin eats leading zeroes */ - memset(b_out, 0, sizeof(b_out)); num_bytes = BN_num_bytes(bn); if (num_bytes > sizeof(b_out)) { ECerr(EC_F_BN_TO_FELEM, EC_R_BIGNUM_OUT_OF_RANGE); @@ -194,7 +193,7 @@ static int BN_to_felem(felem out, const BIGNUM *bn) ECerr(EC_F_BN_TO_FELEM, EC_R_BIGNUM_OUT_OF_RANGE); return 0; } - num_bytes = BN_bn2bin(bn, b_in); + num_bytes = bn_bn2binpad(bn, b_in, sizeof(b_in)); flip_endian(b_out, b_in, num_bytes); bin66_to_felem(out, b_out); return 1; @@ -1935,9 +1934,9 @@ int ec_GFp_nistp521_points_mul(const EC_GROUP *group, EC_POINT *r, ECerr(EC_F_EC_GFP_NISTP521_POINTS_MUL, ERR_R_BN_LIB); goto err; } - num_bytes = BN_bn2bin(tmp_scalar, tmp); + num_bytes = bn_bn2binpad(tmp_scalar, tmp, sizeof(tmp)); } else - num_bytes = BN_bn2bin(p_scalar, tmp); + num_bytes = bn_bn2binpad(p_scalar, tmp, sizeof(tmp)); flip_endian(secrets[i], tmp, num_bytes); /* precompute multiples */ if ((!BN_to_felem(x_out, &p->X)) || @@ -1981,9 +1980,9 @@ int ec_GFp_nistp521_points_mul(const EC_GROUP *group, EC_POINT *r, ECerr(EC_F_EC_GFP_NISTP521_POINTS_MUL, ERR_R_BN_LIB); goto err; } - num_bytes = BN_bn2bin(tmp_scalar, tmp); + num_bytes = bn_bn2binpad(tmp_scalar, tmp, sizeof(tmp)); } else - num_bytes = BN_bn2bin(scalar, tmp); + num_bytes = bn_bn2binpad(scalar, tmp, sizeof(tmp)); flip_endian(g_secret, tmp, num_bytes); /* do the multiplication with generator precomputation */ batch_mul(x_out, y_out, z_out,