[openssl] OpenSSL_1_1_0-stable update

nic.tuv at gmail.com nic.tuv at gmail.com
Fri Sep 6 23:31:28 UTC 2019


The branch OpenSSL_1_1_0-stable has been updated
       via  207a56437916a715bcf6e299c868c75a17ad8fc0 (commit)
       via  e6f559f9de74a0f861d7de2786c64b62fe4ea3ed (commit)
       via  212a75004207234fa677f73e71a1c42b541b009e (commit)
       via  84930232519ae6506101c65d4389619173b6cded (commit)
       via  f7546d64699dc6090d3a9359945cbe785404506e (commit)
      from  f9d1cbe64eebc9e40c7e1f0da4f8f3d98b903b91 (commit)


- Log -----------------------------------------------------------------
commit 207a56437916a715bcf6e299c868c75a17ad8fc0
Author: Nicola Tuveri <nic.tuv at gmail.com>
Date:   Fri Sep 6 14:05:26 2019 +0300

    [ec/ecp_nistp*.c] restyle: use {} around `else` too
    
    Reviewed-by: Matt Caswell <matt at openssl.org>
    Reviewed-by: Bernd Edlinger <bernd.edlinger at hotmail.de>
    (Merged from https://github.com/openssl/openssl/pull/9511)
    
    (cherry picked from commit 4fe2ee3a449a8ca2886584e221f34ff0ef5de119)

commit e6f559f9de74a0f861d7de2786c64b62fe4ea3ed
Author: Nicola Tuveri <nic.tuv at gmail.com>
Date:   Fri Sep 6 01:31:45 2019 +0300

    [ec/ecp_nistp*.c] remove flip_endian()
    
    Replace flip_endian() by using the little endian specific
    BN_bn2lebinpad() and BN_lebin2bn().
    
    Reviewed-by: Matt Caswell <matt at openssl.org>
    Reviewed-by: Bernd Edlinger <bernd.edlinger at hotmail.de>
    (Merged from https://github.com/openssl/openssl/pull/9511)
    
    (cherry picked from commit e0b660c27d8d97b4ad9e2098cc957de26872c0ef)

commit 212a75004207234fa677f73e71a1c42b541b009e
Author: Nicola Tuveri <nic.tuv at gmail.com>
Date:   Fri Sep 6 00:18:36 2019 +0300

    Uniform BN_bn2binpad() and BN_bn2lebinpad() implementations
    
    Reviewed-by: Matt Caswell <matt at openssl.org>
    Reviewed-by: Bernd Edlinger <bernd.edlinger at hotmail.de>
    (Merged from https://github.com/openssl/openssl/pull/9511)
    
    (cherry picked from commit 1b338abe3abb8c73f004c34d4b8a9272b89dfd5d)

commit 84930232519ae6506101c65d4389619173b6cded
Author: Nicola Tuveri <nic.tuv at gmail.com>
Date:   Fri Aug 2 02:08:34 2019 +0300

    Make BN_num_bits() consttime upon BN_FLG_CONSTTIME
    
    This issue was partially addressed by commit
    972c87dfc7e765bd28a4964519c362f0d3a58ca4, which hardened its callee
    BN_num_bits_word() to avoid leaking the most-significant word of its
    argument via branching and memory access pattern.
    The commit message also reported:
    > There are a few places where BN_num_bits is called on an input where
    > the bit length is also secret. This does *not* fully resolve those
    > cases as we still only look at the top word.
    
    BN_num_bits() is called directly or indirectly (e.g., through
    BN_num_bytes() or BN_bn2binpad() ) in various parts of the `crypto/ec`
    code, notably in all the currently supported implementations of scalar
    multiplication (in the generic path through ec_scalar_mul_ladder() as
    well as in dedicated methods like ecp_nistp{224,256,521}.c and
    ecp_nistz256.c).
    
    Under the right conditions, a motivated SCA attacker could retrieve the
    secret bitlength of a secret nonce through this vulnerability,
    potentially leading, ultimately, to recover a long-term secret key.
    
    With this commit, exclusively for BIGNUMs that are flagged with
    BN_FLG_CONSTTIME, instead of accessing only bn->top, all the limbs of
    the BIGNUM are accessed up to bn->dmax and bitwise masking is used to
    avoid branching.
    
    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 should be already done at the top level
    alongside setting the BN_FLG_CONSTTIME.
    
    Thanks to David Schrammel and Samuel Weiser for reporting this issue
    through responsible disclosure.
    
    Reviewed-by: Matt Caswell <matt at openssl.org>
    Reviewed-by: Bernd Edlinger <bernd.edlinger at hotmail.de>
    (Merged from https://github.com/openssl/openssl/pull/9511)
    
    (cherry picked from commit 8b44198b916015f77bef1befa26edb48ad8a0238)

commit f7546d64699dc6090d3a9359945cbe785404506e
Author: Nicola Tuveri <nic.tuv at gmail.com>
Date:   Fri Aug 2 01:33:05 2019 +0300

    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 at openssl.org>
    Reviewed-by: Bernd Edlinger <bernd.edlinger at hotmail.de>
    (Merged from https://github.com/openssl/openssl/pull/9511)
    
    (cherry picked from commit 805315d3a20f7274195eed75b06c391dacf3b197)

-----------------------------------------------------------------------

Summary of changes:
 crypto/bn/bn_lib.c       | 81 ++++++++++++++++++++++++++++++++++++------------
 crypto/ec/ecp_nistp224.c | 62 ++++++++++++++++--------------------
 crypto/ec/ecp_nistp256.c | 63 ++++++++++++++++---------------------
 crypto/ec/ecp_nistp521.c | 63 ++++++++++++++++---------------------
 4 files changed, 141 insertions(+), 128 deletions(-)

diff --git a/crypto/bn/bn_lib.c b/crypto/bn/bn_lib.c
index de46d0f2d1..144653e2b4 100644
--- a/crypto/bn/bn_lib.c
+++ b/crypto/bn/bn_lib.c
@@ -132,13 +132,57 @@ int BN_num_bits_word(BN_ULONG l)
     return bits;
 }
 
+/*
+ * This function still leaks `a->dmax`: it's caller's responsibility to
+ * expand the input `a` in advance to a public length.
+ */
+static ossl_inline
+int bn_num_bits_consttime(const BIGNUM *a)
+{
+    int j, ret;
+    unsigned int mask, past_i;
+    int i = a->top - 1;
+    bn_check_top(a);
+
+    for (j = 0, past_i = 0, ret = 0; j < a->dmax; j++) {
+        mask = constant_time_eq_int(i, j); /* 0xff..ff if i==j, 0x0 otherwise */
+
+        ret += BN_BITS2 & (~mask & ~past_i);
+        ret += BN_num_bits_word(a->d[j]) & mask;
+
+        past_i |= mask; /* past_i will become 0xff..ff after i==j */
+    }
+
+    /*
+     * if BN_is_zero(a) => i is -1 and ret contains garbage, so we mask the
+     * final result.
+     */
+    mask = ~(constant_time_eq_int(i, ((int)-1)));
+
+    return ret & mask;
+}
+
 int BN_num_bits(const BIGNUM *a)
 {
     int i = a->top - 1;
     bn_check_top(a);
 
+    if (a->flags & BN_FLG_CONSTTIME) {
+        /*
+         * We assume that BIGNUMs flagged as CONSTTIME have also been expanded
+         * so that a->dmax is not leaking secret information.
+         *
+         * In other words, it's the caller's responsibility to ensure `a` has
+         * been preallocated in advance to a public length if we hit this
+         * branch.
+         *
+         */
+        return bn_num_bits_consttime(a);
+    }
+
     if (BN_is_zero(a))
         return 0;
+
     return ((i * BN_BITS2) + BN_num_bits_word(a->d[i]));
 }
 
@@ -499,8 +543,11 @@ BIGNUM *BN_bin2bn(const unsigned char *s, int len, BIGNUM *ret)
     return (ret);
 }
 
+typedef enum {big, little} endianess_t;
+
 /* ignore negative */
-static int bn2binpad(const BIGNUM *a, unsigned char *to, int tolen)
+static
+int bn2binpad(const BIGNUM *a, unsigned char *to, int tolen, endianess_t endianess)
 {
     int n;
     size_t i, lasti, j, atop, mask;
@@ -532,10 +579,17 @@ static int bn2binpad(const BIGNUM *a, unsigned char *to, int tolen)
 
     lasti = atop - 1;
     atop = a->top * BN_BYTES;
-    for (i = 0, j = 0, to += tolen; j < (size_t)tolen; j++) {
+    if (endianess == big)
+        to += tolen; /* start from the end of the buffer */
+    for (i = 0, j = 0; j < (size_t)tolen; j++) {
+        unsigned char val;
         l = a->d[i / BN_BYTES];
         mask = 0 - ((j - atop) >> (8 * sizeof(i) - 1));
-        *--to = (unsigned char)(l >> (8 * (i % BN_BYTES)) & mask);
+        val = (unsigned char)(l >> (8 * (i % BN_BYTES)) & mask);
+        if (endianess == big)
+            *--to = val;
+        else
+            *to++ = val;
         i += (i - lasti) >> (8 * sizeof(i) - 1); /* stay on last limb */
     }
 
@@ -546,12 +600,12 @@ int BN_bn2binpad(const BIGNUM *a, unsigned char *to, int tolen)
 {
     if (tolen < 0)
         return -1;
-    return bn2binpad(a, to, tolen);
+    return bn2binpad(a, to, tolen, big);
 }
 
 int BN_bn2bin(const BIGNUM *a, unsigned char *to)
 {
-    return bn2binpad(a, to, -1);
+    return bn2binpad(a, to, -1, big);
 }
 
 BIGNUM *BN_lebin2bn(const unsigned char *s, int len, BIGNUM *ret)
@@ -603,22 +657,9 @@ BIGNUM *BN_lebin2bn(const unsigned char *s, int len, BIGNUM *ret)
 
 int BN_bn2lebinpad(const BIGNUM *a, unsigned char *to, int tolen)
 {
-    int i;
-    BN_ULONG l;
-    bn_check_top(a);
-    i = BN_num_bytes(a);
-    if (tolen < i)
+    if (tolen < 0)
         return -1;
-    /* Add trailing zeroes if necessary */
-    if (tolen > i)
-        memset(to + i, 0, tolen - i);
-    to += i;
-    while (i--) {
-        l = a->d[i / BN_BYTES];
-        to--;
-        *to = (unsigned char)(l >> (8 * (i % BN_BYTES))) & 0xff;
-    }
-    return tolen;
+    return bn2binpad(a, to, tolen, little);
 }
 
 int BN_ucmp(const BIGNUM *a, const BIGNUM *b)
diff --git a/crypto/ec/ecp_nistp224.c b/crypto/ec/ecp_nistp224.c
index 651dcb10a0..2763398a33 100644
--- a/crypto/ec/ecp_nistp224.c
+++ b/crypto/ec/ecp_nistp224.c
@@ -320,34 +320,21 @@ static void felem_to_bin28(u8 out[28], const felem in)
     }
 }
 
-/* To preserve endianness when using BN_bn2bin and BN_bin2bn */
-static void flip_endian(u8 *out, const u8 *in, unsigned len)
-{
-    unsigned i;
-    for (i = 0; i < len; ++i)
-        out[i] = in[len - 1 - i];
-}
-
 /* From OpenSSL BIGNUM to internal representation */
 static int BN_to_felem(felem out, const BIGNUM *bn)
 {
-    felem_bytearray b_in;
     felem_bytearray b_out;
-    unsigned num_bytes;
+    int 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)) {
+    if (BN_is_negative(bn)) {
         ECerr(EC_F_BN_TO_FELEM, EC_R_BIGNUM_OUT_OF_RANGE);
         return 0;
     }
-    if (BN_is_negative(bn)) {
+    num_bytes = BN_bn2lebinpad(bn, b_out, sizeof(b_out));
+    if (num_bytes < 0) {
         ECerr(EC_F_BN_TO_FELEM, EC_R_BIGNUM_OUT_OF_RANGE);
         return 0;
     }
-    num_bytes = BN_bn2bin(bn, b_in);
-    flip_endian(b_out, b_in, num_bytes);
     bin28_to_felem(out, b_out);
     return 1;
 }
@@ -355,10 +342,9 @@ static int BN_to_felem(felem out, const BIGNUM *bn)
 /* From internal representation to OpenSSL BIGNUM */
 static BIGNUM *felem_to_BN(BIGNUM *out, const felem in)
 {
-    felem_bytearray b_in, b_out;
-    felem_to_bin28(b_in, in);
-    flip_endian(b_out, b_in, sizeof(b_out));
-    return BN_bin2bn(b_out, sizeof(b_out), out);
+    felem_bytearray b_out;
+    felem_to_bin28(b_out, in);
+    return BN_lebin2bn(b_out, sizeof(b_out), out);
 }
 
 /******************************************************************************/
@@ -1402,8 +1388,7 @@ int ec_GFp_nistp224_points_mul(const EC_GROUP *group, EC_POINT *r,
     felem_bytearray *secrets = NULL;
     felem (*pre_comp)[17][3] = NULL;
     felem *tmp_felems = NULL;
-    felem_bytearray tmp;
-    unsigned num_bytes;
+    int num_bytes;
     int have_pre_comp = 0;
     size_t num_points = num;
     felem x_in, y_in, z_in, x_out, y_out, z_out;
@@ -1480,14 +1465,12 @@ int ec_GFp_nistp224_points_mul(const EC_GROUP *group, EC_POINT *r,
          * i.e., they contribute nothing to the linear combination
          */
         for (i = 0; i < num_points; ++i) {
-            if (i == num)
+            if (i == num) {
                 /* the generator */
-            {
                 p = EC_GROUP_get0_generator(group);
                 p_scalar = scalar;
-            } else
+            } else {
                 /* the i^th point */
-            {
                 p = points[i];
                 p_scalar = scalars[i];
             }
@@ -1503,10 +1486,16 @@ 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);
-                } else
-                    num_bytes = BN_bn2bin(p_scalar, tmp);
-                flip_endian(secrets[i], tmp, num_bytes);
+                    num_bytes = BN_bn2lebinpad(tmp_scalar,
+                                               secrets[i], sizeof(secrets[i]));
+                } else {
+                    num_bytes = BN_bn2lebinpad(p_scalar,
+                                               secrets[i], sizeof(secrets[i]));
+                }
+                if (num_bytes < 0) {
+                    ECerr(EC_F_EC_GFP_NISTP224_POINTS_MUL, ERR_R_BN_LIB);
+                    goto err;
+                }
                 /* precompute multiples */
                 if ((!BN_to_felem(x_out, p->X)) ||
                     (!BN_to_felem(y_out, p->Y)) ||
@@ -1549,20 +1538,21 @@ 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);
-        } else
-            num_bytes = BN_bn2bin(scalar, tmp);
-        flip_endian(g_secret, tmp, num_bytes);
+            num_bytes = BN_bn2lebinpad(tmp_scalar, g_secret, sizeof(g_secret));
+        } else {
+            num_bytes = BN_bn2lebinpad(scalar, g_secret, sizeof(g_secret));
+        }
         /* do the multiplication with generator precomputation */
         batch_mul(x_out, y_out, z_out,
                   (const felem_bytearray(*))secrets, num_points,
                   g_secret,
                   mixed, (const felem(*)[17][3])pre_comp, g_pre_comp);
-    } else
+    } else {
         /* do the multiplication without generator precomputation */
         batch_mul(x_out, y_out, z_out,
                   (const felem_bytearray(*))secrets, num_points,
                   NULL, mixed, (const felem(*)[17][3])pre_comp, NULL);
+    }
     /* reduce the output to its unique minimal representation */
     felem_contract(x_in, x_out);
     felem_contract(y_in, y_out);
diff --git a/crypto/ec/ecp_nistp256.c b/crypto/ec/ecp_nistp256.c
index dfd98c62fe..8f3063563c 100644
--- a/crypto/ec/ecp_nistp256.c
+++ b/crypto/ec/ecp_nistp256.c
@@ -146,34 +146,21 @@ static void smallfelem_to_bin32(u8 out[32], const smallfelem in)
     *((u64 *)&out[24]) = in[3];
 }
 
-/* To preserve endianness when using BN_bn2bin and BN_bin2bn */
-static void flip_endian(u8 *out, const u8 *in, unsigned len)
-{
-    unsigned i;
-    for (i = 0; i < len; ++i)
-        out[i] = in[len - 1 - i];
-}
-
 /* BN_to_felem converts an OpenSSL BIGNUM into an felem */
 static int BN_to_felem(felem out, const BIGNUM *bn)
 {
-    felem_bytearray b_in;
     felem_bytearray b_out;
-    unsigned num_bytes;
+    int 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)) {
+    if (BN_is_negative(bn)) {
         ECerr(EC_F_BN_TO_FELEM, EC_R_BIGNUM_OUT_OF_RANGE);
         return 0;
     }
-    if (BN_is_negative(bn)) {
+    num_bytes = BN_bn2lebinpad(bn, b_out, sizeof(b_out));
+    if (num_bytes < 0) {
         ECerr(EC_F_BN_TO_FELEM, EC_R_BIGNUM_OUT_OF_RANGE);
         return 0;
     }
-    num_bytes = BN_bn2bin(bn, b_in);
-    flip_endian(b_out, b_in, num_bytes);
     bin32_to_felem(out, b_out);
     return 1;
 }
@@ -181,10 +168,9 @@ static int BN_to_felem(felem out, const BIGNUM *bn)
 /* felem_to_BN converts an felem into an OpenSSL BIGNUM */
 static BIGNUM *smallfelem_to_BN(BIGNUM *out, const smallfelem in)
 {
-    felem_bytearray b_in, b_out;
-    smallfelem_to_bin32(b_in, in);
-    flip_endian(b_out, b_in, sizeof(b_out));
-    return BN_bin2bn(b_out, sizeof(b_out), out);
+    felem_bytearray b_out;
+    smallfelem_to_bin32(b_out, in);
+    return BN_lebin2bn(b_out, sizeof(b_out), out);
 }
 
 /*-
@@ -2019,8 +2005,8 @@ int ec_GFp_nistp256_points_mul(const EC_GROUP *group, EC_POINT *r,
     felem_bytearray *secrets = NULL;
     smallfelem (*pre_comp)[17][3] = NULL;
     smallfelem *tmp_smallfelems = NULL;
-    felem_bytearray tmp;
-    unsigned i, num_bytes;
+    unsigned i;
+    int num_bytes;
     int have_pre_comp = 0;
     size_t num_points = num;
     smallfelem x_in, y_in, z_in;
@@ -2099,17 +2085,15 @@ int ec_GFp_nistp256_points_mul(const EC_GROUP *group, EC_POINT *r,
         memset(secrets, 0, sizeof(*secrets) * num_points);
         memset(pre_comp, 0, sizeof(*pre_comp) * num_points);
         for (i = 0; i < num_points; ++i) {
-            if (i == num)
+            if (i == num) {
                 /*
                  * we didn't have a valid precomputation, so we pick the
                  * generator
                  */
-            {
                 p = EC_GROUP_get0_generator(group);
                 p_scalar = scalar;
-            } else
+            } else {
                 /* the i^th point */
-            {
                 p = points[i];
                 p_scalar = scalars[i];
             }
@@ -2125,10 +2109,16 @@ 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);
-                } else
-                    num_bytes = BN_bn2bin(p_scalar, tmp);
-                flip_endian(secrets[i], tmp, num_bytes);
+                    num_bytes = BN_bn2lebinpad(tmp_scalar,
+                                               secrets[i], sizeof(secrets[i]));
+                } else {
+                    num_bytes = BN_bn2lebinpad(p_scalar,
+                                               secrets[i], sizeof(secrets[i]));
+                }
+                if (num_bytes < 0) {
+                    ECerr(EC_F_EC_GFP_NISTP256_POINTS_MUL, ERR_R_BN_LIB);
+                    goto err;
+                }
                 /* precompute multiples */
                 if ((!BN_to_felem(x_out, p->X)) ||
                     (!BN_to_felem(y_out, p->Y)) ||
@@ -2173,20 +2163,21 @@ 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);
-        } else
-            num_bytes = BN_bn2bin(scalar, tmp);
-        flip_endian(g_secret, tmp, num_bytes);
+            num_bytes = BN_bn2lebinpad(tmp_scalar, g_secret, sizeof(g_secret));
+        } else {
+            num_bytes = BN_bn2lebinpad(scalar, g_secret, sizeof(g_secret));
+        }
         /* do the multiplication with generator precomputation */
         batch_mul(x_out, y_out, z_out,
                   (const felem_bytearray(*))secrets, num_points,
                   g_secret,
                   mixed, (const smallfelem(*)[17][3])pre_comp, g_pre_comp);
-    } else
+    } else {
         /* do the multiplication without generator precomputation */
         batch_mul(x_out, y_out, z_out,
                   (const felem_bytearray(*))secrets, num_points,
                   NULL, mixed, (const smallfelem(*)[17][3])pre_comp, NULL);
+    }
     /* reduce the output to its unique minimal representation */
     felem_contract(x_in, x_out);
     felem_contract(y_in, y_out);
diff --git a/crypto/ec/ecp_nistp521.c b/crypto/ec/ecp_nistp521.c
index 6059c2efc8..7e6b5a9ba2 100644
--- a/crypto/ec/ecp_nistp521.c
+++ b/crypto/ec/ecp_nistp521.c
@@ -169,34 +169,21 @@ static void felem_to_bin66(u8 out[66], const felem in)
     (*((limb *) & out[58])) = in[8];
 }
 
-/* To preserve endianness when using BN_bn2bin and BN_bin2bn */
-static void flip_endian(u8 *out, const u8 *in, unsigned len)
-{
-    unsigned i;
-    for (i = 0; i < len; ++i)
-        out[i] = in[len - 1 - i];
-}
-
 /* BN_to_felem converts an OpenSSL BIGNUM into an felem */
 static int BN_to_felem(felem out, const BIGNUM *bn)
 {
-    felem_bytearray b_in;
     felem_bytearray b_out;
-    unsigned num_bytes;
+    int 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)) {
+    if (BN_is_negative(bn)) {
         ECerr(EC_F_BN_TO_FELEM, EC_R_BIGNUM_OUT_OF_RANGE);
         return 0;
     }
-    if (BN_is_negative(bn)) {
+    num_bytes = BN_bn2lebinpad(bn, b_out, sizeof(b_out));
+    if (num_bytes < 0) {
         ECerr(EC_F_BN_TO_FELEM, EC_R_BIGNUM_OUT_OF_RANGE);
         return 0;
     }
-    num_bytes = BN_bn2bin(bn, b_in);
-    flip_endian(b_out, b_in, num_bytes);
     bin66_to_felem(out, b_out);
     return 1;
 }
@@ -204,10 +191,9 @@ static int BN_to_felem(felem out, const BIGNUM *bn)
 /* felem_to_BN converts an felem into an OpenSSL BIGNUM */
 static BIGNUM *felem_to_BN(BIGNUM *out, const felem in)
 {
-    felem_bytearray b_in, b_out;
-    felem_to_bin66(b_in, in);
-    flip_endian(b_out, b_in, sizeof(b_out));
-    return BN_bin2bn(b_out, sizeof(b_out), out);
+    felem_bytearray b_out;
+    felem_to_bin66(b_out, in);
+    return BN_lebin2bn(b_out, sizeof(b_out), out);
 }
 
 /*-
@@ -1846,8 +1832,8 @@ int ec_GFp_nistp521_points_mul(const EC_GROUP *group, EC_POINT *r,
     felem_bytearray *secrets = NULL;
     felem (*pre_comp)[17][3] = NULL;
     felem *tmp_felems = NULL;
-    felem_bytearray tmp;
-    unsigned i, num_bytes;
+    unsigned i;
+    int num_bytes;
     int have_pre_comp = 0;
     size_t num_points = num;
     felem x_in, y_in, z_in, x_out, y_out, z_out;
@@ -1924,17 +1910,15 @@ int ec_GFp_nistp521_points_mul(const EC_GROUP *group, EC_POINT *r,
          * i.e., they contribute nothing to the linear combination
          */
         for (i = 0; i < num_points; ++i) {
-            if (i == num)
+            if (i == num) {
                 /*
                  * we didn't have a valid precomputation, so we pick the
                  * generator
                  */
-            {
                 p = EC_GROUP_get0_generator(group);
                 p_scalar = scalar;
-            } else
+            } else {
                 /* the i^th point */
-            {
                 p = points[i];
                 p_scalar = scalars[i];
             }
@@ -1950,10 +1934,16 @@ 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);
-                } else
-                    num_bytes = BN_bn2bin(p_scalar, tmp);
-                flip_endian(secrets[i], tmp, num_bytes);
+                    num_bytes = BN_bn2lebinpad(tmp_scalar,
+                                               secrets[i], sizeof(secrets[i]));
+                } else {
+                    num_bytes = BN_bn2lebinpad(p_scalar,
+                                               secrets[i], sizeof(secrets[i]));
+                }
+                if (num_bytes < 0) {
+                    ECerr(EC_F_EC_GFP_NISTP521_POINTS_MUL, ERR_R_BN_LIB);
+                    goto err;
+                }
                 /* precompute multiples */
                 if ((!BN_to_felem(x_out, p->X)) ||
                     (!BN_to_felem(y_out, p->Y)) ||
@@ -1996,21 +1986,22 @@ 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);
-        } else
-            num_bytes = BN_bn2bin(scalar, tmp);
-        flip_endian(g_secret, tmp, num_bytes);
+            num_bytes = BN_bn2lebinpad(tmp_scalar, g_secret, sizeof(g_secret));
+        } else {
+            num_bytes = BN_bn2lebinpad(scalar, g_secret, sizeof(g_secret));
+        }
         /* do the multiplication with generator precomputation */
         batch_mul(x_out, y_out, z_out,
                   (const felem_bytearray(*))secrets, num_points,
                   g_secret,
                   mixed, (const felem(*)[17][3])pre_comp,
                   (const felem(*)[3])g_pre_comp);
-    } else
+    } else {
         /* do the multiplication without generator precomputation */
         batch_mul(x_out, y_out, z_out,
                   (const felem_bytearray(*))secrets, num_points,
                   NULL, mixed, (const felem(*)[17][3])pre_comp, NULL);
+    }
     /* reduce the output to its unique minimal representation */
     felem_contract(x_in, x_out);
     felem_contract(y_in, y_out);


More information about the openssl-commits mailing list