[openssl-commits] [openssl] OpenSSL_1_1_0-stable update

Andy Polyakov appro at openssl.org
Thu Feb 1 20:46:11 UTC 2018


The branch OpenSSL_1_1_0-stable has been updated
       via  723183b5640e9b047acac189c7c7fb78575522d4 (commit)
       via  0b9373ce3c2d39d9a88634913b8191ad4f7a453c (commit)
      from  d8adfdcd2e5de23f3e1d1a1d10c2fda6f4a65c4a (commit)


- Log -----------------------------------------------------------------
commit 723183b5640e9b047acac189c7c7fb78575522d4
Author: David Benjamin <davidben at google.com>
Date:   Tue Jan 23 13:57:10 2018 -0500

    Don't leak the exponent bit width in BN_mod_exp_mont_consttime.
    
    The exponent here is one of d, dmp1, or dmq1 for RSA. This value and its
    bit length are both secret. The only public upper bound is the bit width
    of the corresponding modulus (RSA n, p, and q, respectively).
    
    Although BN_num_bits is constant-time (sort of; see bn_correct_top notes
    in preceding patch), this does not fix the root problem, which is that
    the windows are based on the minimal bit width, not the upper bound. We
    could use BN_num_bits(m), but BN_mod_exp_mont_consttime is public API
    and may be called with larger exponents. Instead, use all top*BN_BITS2
    bits in the BIGNUM. This is still sensitive to the long-standing
    bn_correct_top leak, but we need to fix that regardless.
    
    This may cause us to do a handful of extra multiplications for RSA keys
    which are just above a whole number of words, but that is not a standard
    RSA key size.
    
    Reviewed-by: Paul Dale <paul.dale at oracle.com>
    Reviewed-by: Rich Salz <rsalz at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/5154)
    
    (cherry picked from commit 39eeb64f59ff838f976ad305de7d15747d47a41c)

commit 0b9373ce3c2d39d9a88634913b8191ad4f7a453c
Author: David Benjamin <davidben at google.com>
Date:   Tue Jan 23 13:46:53 2018 -0500

    Make BN_num_bits_word constant-time.
    
    (This patch was written by Andy Polyakov. I only wrote the commit
    message. Mistakes in the analysis are my fault.)
    
    BN_num_bits, by way of BN_num_bits_word, currently leaks the
    most-significant word of its argument via branching and memory access
    pattern.
    
    BN_num_bits is called on RSA prime factors in various places. These have
    public bit lengths, but all bits beyond the high bit are secret. This
    fully resolves those cases.
    
    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. Today, that is guaranteed to be
    non-zero, but only because of the long-standing bn_correct_top timing
    leak. Once that is fixed, a constant-time BN_num_bits on such inputs
    must count bits on each word.
    
    Instead, those cases should not call BN_num_bits at all. In particular,
    BN_mod_exp_mont_consttime uses the exponent bit width to pick windows,
    but it should be using the maximum bit width. The next patch will fix
    this.
    
    Thanks to Dinghao Wu, Danfeng Zhang, Shuai Wang, Pei Wang, and Xiao Liu
    for reporting this issue.
    
    Reviewed-by: Paul Dale <paul.dale at oracle.com>
    Reviewed-by: Rich Salz <rsalz at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/5154)
    
    (cherry picked from commit 972c87dfc7e765bd28a4964519c362f0d3a58ca4)

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

Summary of changes:
 crypto/bn/bn_exp.c |   6 ++-
 crypto/bn/bn_lib.c | 107 ++++++++++++++++++++---------------------------------
 2 files changed, 45 insertions(+), 68 deletions(-)

diff --git a/crypto/bn/bn_exp.c b/crypto/bn/bn_exp.c
index 7ebaeb0..8073368 100644
--- a/crypto/bn/bn_exp.c
+++ b/crypto/bn/bn_exp.c
@@ -633,7 +633,11 @@ int BN_mod_exp_mont_consttime(BIGNUM *rr, const BIGNUM *a, const BIGNUM *p,
 
     top = m->top;
 
-    bits = BN_num_bits(p);
+    /*
+     * Use all bits stored in |p|, rather than |BN_num_bits|, so we do not leak
+     * whether the top bits are zero.
+     */
+    bits = p->top * BN_BITS2;
     if (bits == 0) {
         /* x**0 mod 1 is still zero. */
         if (BN_is_one(m)) {
diff --git a/crypto/bn/bn_lib.c b/crypto/bn/bn_lib.c
index b8d4468..6081e88 100644
--- a/crypto/bn/bn_lib.c
+++ b/crypto/bn/bn_lib.c
@@ -88,74 +88,47 @@ const BIGNUM *BN_value_one(void)
 
 int BN_num_bits_word(BN_ULONG l)
 {
-    static const unsigned char bits[256] = {
-        0, 1, 2, 2, 3, 3, 3, 3, 4, 4, 4, 4, 4, 4, 4, 4,
-        5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5,
-        6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6,
-        6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6,
-        7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7,
-        7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7,
-        7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7,
-        7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7,
-        8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8,
-        8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8,
-        8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8,
-        8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8,
-        8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8,
-        8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8,
-        8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8,
-        8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8, 8,
-    };
-
-#if defined(SIXTY_FOUR_BIT_LONG)
-    if (l & 0xffffffff00000000L) {
-        if (l & 0xffff000000000000L) {
-            if (l & 0xff00000000000000L) {
-                return (bits[(int)(l >> 56)] + 56);
-            } else
-                return (bits[(int)(l >> 48)] + 48);
-        } else {
-            if (l & 0x0000ff0000000000L) {
-                return (bits[(int)(l >> 40)] + 40);
-            } else
-                return (bits[(int)(l >> 32)] + 32);
-        }
-    } else
-#else
-# ifdef SIXTY_FOUR_BIT
-    if (l & 0xffffffff00000000LL) {
-        if (l & 0xffff000000000000LL) {
-            if (l & 0xff00000000000000LL) {
-                return (bits[(int)(l >> 56)] + 56);
-            } else
-                return (bits[(int)(l >> 48)] + 48);
-        } else {
-            if (l & 0x0000ff0000000000LL) {
-                return (bits[(int)(l >> 40)] + 40);
-            } else
-                return (bits[(int)(l >> 32)] + 32);
-        }
-    } else
-# endif
-#endif
-    {
-#if defined(THIRTY_TWO_BIT) || defined(SIXTY_FOUR_BIT) || defined(SIXTY_FOUR_BIT_LONG)
-        if (l & 0xffff0000L) {
-            if (l & 0xff000000L)
-                return (bits[(int)(l >> 24L)] + 24);
-            else
-                return (bits[(int)(l >> 16L)] + 16);
-        } else
+    BN_ULONG x, mask;
+    int bits = (l != 0);
+
+#if BN_BITS2 > 32
+    x = l >> 32;
+    mask = (0 - x) & BN_MASK2;
+    mask = (0 - (mask >> (BN_BITS2 - 1)));
+    bits += 32 & mask;
+    l ^= (x ^ l) & mask;
 #endif
-        {
-#if defined(THIRTY_TWO_BIT) || defined(SIXTY_FOUR_BIT) || defined(SIXTY_FOUR_BIT_LONG)
-            if (l & 0xff00L)
-                return (bits[(int)(l >> 8)] + 8);
-            else
-#endif
-                return (bits[(int)(l)]);
-        }
-    }
+
+    x = l >> 16;
+    mask = (0 - x) & BN_MASK2;
+    mask = (0 - (mask >> (BN_BITS2 - 1)));
+    bits += 16 & mask;
+    l ^= (x ^ l) & mask;
+
+    x = l >> 8;
+    mask = (0 - x) & BN_MASK2;
+    mask = (0 - (mask >> (BN_BITS2 - 1)));
+    bits += 8 & mask;
+    l ^= (x ^ l) & mask;
+
+    x = l >> 4;
+    mask = (0 - x) & BN_MASK2;
+    mask = (0 - (mask >> (BN_BITS2 - 1)));
+    bits += 4 & mask;
+    l ^= (x ^ l) & mask;
+
+    x = l >> 2;
+    mask = (0 - x) & BN_MASK2;
+    mask = (0 - (mask >> (BN_BITS2 - 1)));
+    bits += 2 & mask;
+    l ^= (x ^ l) & mask;
+
+    x = l >> 1;
+    mask = (0 - x) & BN_MASK2;
+    mask = (0 - (mask >> (BN_BITS2 - 1)));
+    bits += 1 & mask;
+
+    return bits;
 }
 
 int BN_num_bits(const BIGNUM *a)


More information about the openssl-commits mailing list