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 <matt@openssl.org>
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from https://github.com/openssl/openssl/pull/9793)

diff --git a/crypto/ec/ecp_nistp224.c b/crypto/ec/ecp_nistp224.c
index 121f587b58b644d4e770dcb3927268d014fde5df..877edc9cf0d855b890f2ca274f230528ebd7f439 100644 (file)
--- a/crypto/ec/ecp_nistp224.c
+++ b/crypto/ec/ecp_nistp224.c
@@ -37,6 +37,7 @@
 # include <string.h>
 # include <openssl/err.h>
 # 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 378f0bae0857d86796808a0d25867f7dc5543dcf..a9b2c062a9a4d6152992cef5b200dd48feb23135 100644 (file)
--- a/crypto/ec/ecp_nistp256.c
+++ b/crypto/ec/ecp_nistp256.c
@@ -38,6 +38,7 @@
 # include <string.h>
 # include <openssl/err.h>
 # 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 1a42068c01f9c9e32aefcbc3a69ec1f2c6e0faf4..9765e9866d60d78e459e0fa4d84aa28cb6f34f5a 100644 (file)
--- a/crypto/ec/ecp_nistp521.c
+++ b/crypto/ec/ecp_nistp521.c
@@ -38,6 +38,7 @@
 # include <string.h>
 # include <openssl/err.h>
 # 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,