[openssl-commits] [openssl] OpenSSL_1_0_2-stable update

Andy Polyakov appro at openssl.org
Wed Aug 1 14:34:08 UTC 2018


The branch OpenSSL_1_0_2-stable has been updated
       via  29d8bda90ce824263317eae5354388f79844dd51 (commit)
       via  983e1ad235caa45d710eaa5f0d2de504d782a348 (commit)
       via  e3ab8cc460d1a43fe6310c8d9a92589db1d4f8a3 (commit)
       via  6a815969776e3329fdffcc12c77e047e3a15be78 (commit)
       via  83325a68ad5fdfc359ab9d82a0e0da8e5fe7ede1 (commit)
       via  c9046a05ec0fc3377e1077b401652d76ee5ce908 (commit)
       via  327b2c011342280c7fd5e312a4fff2a01083d2d6 (commit)
       via  c1c0e4f1a358072767860764cd43335fc7316176 (commit)
       via  7cca1f96bf82b22ab49f179bae7df1562d0a104b (commit)
      from  d69f31fcc38878769c8c917f8724c5aef10fd847 (commit)


- Log -----------------------------------------------------------------
commit 29d8bda90ce824263317eae5354388f79844dd51
Author: Andy Polyakov <appro at openssl.org>
Date:   Mon Jul 30 12:39:08 2018 +0200

    CHANGES: mention blinding reverting in ECDSA.
    
    Reviewed-by: Rich Salz <rsalz at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/6810)

commit 983e1ad235caa45d710eaa5f0d2de504d782a348
Author: Andy Polyakov <appro at openssl.org>
Date:   Mon Jul 30 12:37:17 2018 +0200

    ecdsa/ecs_ossl.c: switch to fixed-length Montgomery multiplication.
    
    (back-ported from commit 37132c9702328940a99b1307f742ab094ef754a7)
    
    Reviewed-by: Rich Salz <rsalz at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/6810)

commit e3ab8cc460d1a43fe6310c8d9a92589db1d4f8a3
Author: Billy Brumley <bbrumley at gmail.com>
Date:   Wed Jan 20 13:18:21 2016 +0200

    Fix BN_gcd errors for some curves
    
    Those even order that do not play nicely with Montgomery arithmetic
    
    (back-ported from commit 3a6a4a93518fbb3d96632bfdcb538d340f29c56b)
    
    Reviewed-by: Andy Polyakov <appro at openssl.org>
    Reviewed-by: Rich Salz <rsalz at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/6810)

commit 6a815969776e3329fdffcc12c77e047e3a15be78
Author: Andy Polyakov <appro at openssl.org>
Date:   Wed Jul 25 10:29:51 2018 +0200

    bn/bn_mod.c: harmonize BN_mod_add_quick with original implementation.
    
    New implementation failed to correctly reset r->neg flag. Spotted by
    OSSFuzz.
    
    Reviewed-by: Rich Salz <rsalz at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/6810)
    
    (cherry picked from commit 70a579ae2f37437a1e02331eeaa84e1b68ba021e)

commit 83325a68ad5fdfc359ab9d82a0e0da8e5fe7ede1
Author: Andy Polyakov <appro at openssl.org>
Date:   Fri Jul 6 15:55:34 2018 +0200

    ecdsa/ecs_ossl.c: revert blinding in ECDSA signature.
    
    Originally suggested solution for "Return Of the Hidden Number Problem"
    is arguably too expensive. While it has marginal impact on slower
    curves, none to ~6%, optimized implementations suffer real penalties.
    Most notably sign with P-256 went more than 2 times[!] slower. Instead,
    just implement constant-time BN_mod_add_quick.
    
    Reviewed-by: Rich Salz <rsalz at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/6810)
    
    (cherry picked from commit 3fc7a9b96cbed0c3da6f53c08e34d8d0c982745f)
    
    Resolved onflicts:
    	crypto/ec/ecdsa_ossl.c
    	crypto/include/internal/bn_int.h

commit c9046a05ec0fc3377e1077b401652d76ee5ce908
Author: Andy Polyakov <appro at openssl.org>
Date:   Fri Jul 6 15:13:15 2018 +0200

    bn/bn_{mont|exp}.c: switch to zero-padded intermediate vectors.
    
    Note that exported functions maintain original behaviour, so that
    external callers won't observe difference. While internally we can
    now perform Montogomery multiplication on fixed-length vectors, fixed
    at modulus size. The new functions, bn_to_mont_fixed_top and
    bn_mul_mont_fixed_top, are declared in bn_int.h, because one can use
    them even outside bn, e.g. in RSA, DSA, ECDSA...
    
    Reviewed-by: Rich Salz <rsalz at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/6810)
    
    (cherry picked from commit 71883868ea5b33416ae8283bcc38dd2d97e5006b)
    
    Resolved conflicts:
    	crypto/bn/bn_exp.c
    	crypto/bn/bn_lcl.h
    	crypto/bn/bn_mont.c
    	crypto/include/internal/bn_int.h

commit 327b2c011342280c7fd5e312a4fff2a01083d2d6
Author: Andy Polyakov <appro at openssl.org>
Date:   Fri Jul 6 15:02:29 2018 +0200

    bn/bn_lib.c: add BN_FLG_FIXED_TOP flag.
    
    The new flag marks vectors that were not treated with bn_correct_top,
    in other words such vectors are permitted to be zero padded. For now
    it's BN_DEBUG-only flag, as initial use case for zero-padded vectors
    would be controlled Montgomery multiplication/exponentiation, not
    general purpose. For general purpose use another type might be more
    appropriate. Advantage of this suggestion is that it's possible to
    back-port it...
    
    bn/bn_div.c: fix memory sanitizer problem.
    bn/bn_sqr.c: harmonize with BN_mul.
    
    Reviewed-by: Rich Salz <rsalz at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/6810)
    
    (cherry picked from commit 305b68f1a2b6d4d0aa07a6ab47ac372f067a40bb)
    
    Resolved conflicts:
    	crypto/bn/bn_lcl.h
    	crypto/bn/bn_lib.c

commit c1c0e4f1a358072767860764cd43335fc7316176
Author: Andy Polyakov <appro at openssl.org>
Date:   Fri Jul 6 14:54:34 2018 +0200

    bn/bn_mont.c: improve readability of post-condition code.
    
    Reviewed-by: Rich Salz <rsalz at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/6810)
    
    (cherry picked from commit 6c90182a5f87af1a1e462536e7123ad2afb84c43)

commit 7cca1f96bf82b22ab49f179bae7df1562d0a104b
Author: Andy Polyakov <appro at openssl.org>
Date:   Fri Jul 6 13:16:40 2018 +0200

    bn/bn_lib.c: remove bn_check_top from bn_expand2.
    
    Trouble is that addition is postponing expansion till carry is
    calculated, and if addition carries, top word can be zero, which
    triggers assertion in bn_check_top.
    
    Reviewed-by: Rich Salz <rsalz at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/6810)
    
    (cherry picked from commit e42395e637c3507b80b25c7ed63236898822d2f1)
    
    Resolved conflicts:
    	crypto/bn/bn_lib.c

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

Summary of changes:
 CHANGES                 |  4 ++
 crypto/Makefile         |  2 +-
 crypto/bn/bn.h          | 17 ++++++++-
 crypto/bn/bn_div.c      |  1 +
 crypto/bn/bn_exp.c      | 47 +++++++++++++-----------
 crypto/bn/bn_lcl.h      |  1 +
 crypto/bn/bn_lib.c      | 14 +++----
 crypto/bn/bn_mod.c      | 67 ++++++++++++++++++++++++++++++---
 crypto/bn/bn_mont.c     | 58 ++++++++++++++++++++---------
 crypto/bn/bn_sqr.c      | 10 +----
 crypto/bn_int.h         | 13 +++++++
 crypto/ec/ec_lib.c      |  8 +++-
 crypto/ecdsa/ecs_ossl.c | 98 +++++++++++++++----------------------------------
 13 files changed, 207 insertions(+), 133 deletions(-)
 create mode 100644 crypto/bn_int.h

diff --git a/CHANGES b/CHANGES
index 1bf0f0b..b8e2f86 100644
--- a/CHANGES
+++ b/CHANGES
@@ -9,6 +9,10 @@
 
  Changes between 1.0.2o and 1.0.2p [xx XXX xxxx]
 
+  *) Revert blinding in ECDSA sign and instead make problematic addition
+     length-invariant. Switch even to fixed-length Montgomery multiplication.
+     [Andy Polyakov]
+
   *) Change generating and checking of primes so that the error rate of not
      being prime depends on the intended use based on the size of the input.
      For larger primes this will result in more rounds of Miller-Rabin.
diff --git a/crypto/Makefile b/crypto/Makefile
index 7869996..ad1b9f0 100644
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -45,7 +45,7 @@ SRC= $(LIBSRC)
 EXHEADER= crypto.h opensslv.h opensslconf.h ebcdic.h symhacks.h \
 	ossl_typ.h
 HEADER=	cryptlib.h buildinf.h md32_common.h o_time.h o_str.h o_dir.h \
-	constant_time_locl.h $(EXHEADER)
+	constant_time_locl.h bn_int.h $(EXHEADER)
 
 ALL=    $(GENERAL) $(SRC) $(HEADER)
 
diff --git a/crypto/bn/bn.h b/crypto/bn/bn.h
index c056bba..c84a1b5 100644
--- a/crypto/bn/bn.h
+++ b/crypto/bn/bn.h
@@ -824,6 +824,16 @@ BIGNUM *bn_dup_expand(const BIGNUM *a, int words); /* unused */
 /* We only need assert() when debugging */
 #  include <assert.h>
 
+/*
+ * The new BN_FLG_FIXED_TOP flag marks vectors that were not treated with
+ * bn_correct_top, in other words such vectors are permitted to have zeros
+ * in most significant limbs. Such vectors are used internally to achieve
+ * execution time invariance for critical operations with private keys.
+ * It's BN_DEBUG-only flag, because user application is not supposed to
+ * observe it anyway. Moreover, optimizing compiler would actually remove
+ * all operations manipulating the bit in question in non-BN_DEBUG build.
+ */
+#  define BN_FLG_FIXED_TOP 0x10000
 #  ifdef BN_DEBUG_RAND
 /* To avoid "make update" cvs wars due to BN_DEBUG, use some tricks */
 #   ifndef RAND_pseudo_bytes
@@ -856,8 +866,10 @@ int RAND_pseudo_bytes(unsigned char *buf, int num);
         do { \
                 const BIGNUM *_bnum2 = (a); \
                 if (_bnum2 != NULL) { \
-                        assert((_bnum2->top == 0) || \
-                                (_bnum2->d[_bnum2->top - 1] != 0)); \
+                        int _top = _bnum2->top; \
+                        assert((_top == 0) || \
+                               (_bnum2->flags & BN_FLG_FIXED_TOP) || \
+                               (_bnum2->d[_top - 1] != 0)); \
                         bn_pollute(_bnum2); \
                 } \
         } while(0)
@@ -875,6 +887,7 @@ int RAND_pseudo_bytes(unsigned char *buf, int num);
 
 # 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)
diff --git a/crypto/bn/bn_div.c b/crypto/bn/bn_div.c
index bc37671..460d8b7 100644
--- a/crypto/bn/bn_div.c
+++ b/crypto/bn/bn_div.c
@@ -290,6 +290,7 @@ int BN_div(BIGNUM *dv, BIGNUM *rm, const BIGNUM *num, const BIGNUM *divisor,
     wnum.neg = 0;
     wnum.d = &(snum->d[loop]);
     wnum.top = div_n;
+    wnum.flags = BN_FLG_STATIC_DATA;
     /*
      * only needed when BN_ucmp messes up the values between top and max
      */
diff --git a/crypto/bn/bn_exp.c b/crypto/bn/bn_exp.c
index 2eb393d..36b7ba6 100644
--- a/crypto/bn/bn_exp.c
+++ b/crypto/bn/bn_exp.c
@@ -473,17 +473,17 @@ int BN_mod_exp_mont(BIGNUM *rr, const BIGNUM *a, const BIGNUM *p,
         ret = 1;
         goto err;
     }
-    if (!BN_to_montgomery(val[0], aa, mont, ctx))
+    if (!bn_to_mont_fixed_top(val[0], aa, mont, ctx))
         goto err;               /* 1 */
 
     window = BN_window_bits_for_exponent_size(bits);
     if (window > 1) {
-        if (!BN_mod_mul_montgomery(d, val[0], val[0], mont, ctx))
+        if (!bn_mul_mont_fixed_top(d, val[0], val[0], mont, ctx))
             goto err;           /* 2 */
         j = 1 << (window - 1);
         for (i = 1; i < j; i++) {
             if (((val[i] = BN_CTX_get(ctx)) == NULL) ||
-                !BN_mod_mul_montgomery(val[i], val[i - 1], d, mont, ctx))
+                !bn_mul_mont_fixed_top(val[i], val[i - 1], d, mont, ctx))
                 goto err;
         }
     }
@@ -505,19 +505,15 @@ int BN_mod_exp_mont(BIGNUM *rr, const BIGNUM *a, const BIGNUM *p,
         for (i = 1; i < j; i++)
             r->d[i] = (~m->d[i]) & BN_MASK2;
         r->top = j;
-        /*
-         * Upper words will be zero if the corresponding words of 'm' were
-         * 0xfff[...], so decrement r->top accordingly.
-         */
-        bn_correct_top(r);
+        r->flags |= BN_FLG_FIXED_TOP;
     } else
 #endif
-    if (!BN_to_montgomery(r, BN_value_one(), mont, ctx))
+    if (!bn_to_mont_fixed_top(r, BN_value_one(), mont, ctx))
         goto err;
     for (;;) {
         if (BN_is_bit_set(p, wstart) == 0) {
             if (!start) {
-                if (!BN_mod_mul_montgomery(r, r, r, mont, ctx))
+                if (!bn_mul_mont_fixed_top(r, r, r, mont, ctx))
                     goto err;
             }
             if (wstart == 0)
@@ -548,12 +544,12 @@ int BN_mod_exp_mont(BIGNUM *rr, const BIGNUM *a, const BIGNUM *p,
         /* add the 'bytes above' */
         if (!start)
             for (i = 0; i < j; i++) {
-                if (!BN_mod_mul_montgomery(r, r, r, mont, ctx))
+                if (!bn_mul_mont_fixed_top(r, r, r, mont, ctx))
                     goto err;
             }
 
         /* wvalue will be an odd number < 2^window */
-        if (!BN_mod_mul_montgomery(r, r, val[wvalue >> 1], mont, ctx))
+        if (!bn_mul_mont_fixed_top(r, r, val[wvalue >> 1], mont, ctx))
             goto err;
 
         /* move the 'window' down further */
@@ -563,6 +559,11 @@ int BN_mod_exp_mont(BIGNUM *rr, const BIGNUM *a, const BIGNUM *p,
         if (wstart < 0)
             break;
     }
+    /*
+     * Done with zero-padded intermediate BIGNUMs. Final BN_from_montgomery
+     * removes padding [if any] and makes return value suitable for public
+     * API consumer.
+     */
 #if defined(SPARC_T4_MONT)
     if (OPENSSL_sparcv9cap_P[0] & (SPARCV9_VIS3 | SPARCV9_PREFER_FPU)) {
         j = mont->N.top;        /* borrow j */
@@ -681,7 +682,7 @@ static int MOD_EXP_CTIME_COPY_FROM_PREBUF(BIGNUM *b, int top,
     }
 
     b->top = top;
-    bn_correct_top(b);
+    b->flags |= BN_FLG_FIXED_TOP;
     return 1;
 }
 
@@ -852,16 +853,16 @@ int BN_mod_exp_mont_consttime(BIGNUM *rr, const BIGNUM *a, const BIGNUM *p,
         tmp.top = top;
     } else
 #endif
-    if (!BN_to_montgomery(&tmp, BN_value_one(), mont, ctx))
+    if (!bn_to_mont_fixed_top(&tmp, BN_value_one(), mont, ctx))
         goto err;
 
     /* prepare a^1 in Montgomery domain */
     if (a->neg || BN_ucmp(a, m) >= 0) {
         if (!BN_mod(&am, a, m, ctx))
             goto err;
-        if (!BN_to_montgomery(&am, &am, mont, ctx))
+        if (!bn_to_mont_fixed_top(&am, &am, mont, ctx))
             goto err;
-    } else if (!BN_to_montgomery(&am, a, mont, ctx))
+    } else if (!bn_to_mont_fixed_top(&am, a, mont, ctx))
         goto err;
 
 #if defined(SPARC_T4_MONT)
@@ -1128,14 +1129,14 @@ int BN_mod_exp_mont_consttime(BIGNUM *rr, const BIGNUM *a, const BIGNUM *p,
          * performance advantage of sqr over mul).
          */
         if (window > 1) {
-            if (!BN_mod_mul_montgomery(&tmp, &am, &am, mont, ctx))
+            if (!bn_mul_mont_fixed_top(&tmp, &am, &am, mont, ctx))
                 goto err;
             if (!MOD_EXP_CTIME_COPY_TO_PREBUF(&tmp, top, powerbuf, 2,
                                               window))
                 goto err;
             for (i = 3; i < numPowers; i++) {
                 /* Calculate a^i = a^(i-1) * a */
-                if (!BN_mod_mul_montgomery(&tmp, &am, &tmp, mont, ctx))
+                if (!bn_mul_mont_fixed_top(&tmp, &am, &tmp, mont, ctx))
                     goto err;
                 if (!MOD_EXP_CTIME_COPY_TO_PREBUF(&tmp, top, powerbuf, i,
                                                   window))
@@ -1159,7 +1160,7 @@ int BN_mod_exp_mont_consttime(BIGNUM *rr, const BIGNUM *a, const BIGNUM *p,
 
             /* Scan the window, squaring the result as we go */
             for (i = 0; i < window; i++, bits--) {
-                if (!BN_mod_mul_montgomery(&tmp, &tmp, &tmp, mont, ctx))
+                if (!bn_mul_mont_fixed_top(&tmp, &tmp, &tmp, mont, ctx))
                     goto err;
                 wvalue = (wvalue << 1) + BN_is_bit_set(p, bits);
             }
@@ -1172,12 +1173,16 @@ int BN_mod_exp_mont_consttime(BIGNUM *rr, const BIGNUM *a, const BIGNUM *p,
                 goto err;
 
             /* Multiply the result into the intermediate result */
-            if (!BN_mod_mul_montgomery(&tmp, &tmp, &am, mont, ctx))
+            if (!bn_mul_mont_fixed_top(&tmp, &tmp, &am, mont, ctx))
                 goto err;
         }
     }
 
-    /* Convert the final result from montgomery to standard format */
+    /*
+     * Done with zero-padded intermediate BIGNUMs. Final BN_from_montgomery
+     * removes padding [if any] and makes return value suitable for public
+     * API consumer.
+     */
 #if defined(SPARC_T4_MONT)
     if (OPENSSL_sparcv9cap_P[0] & (SPARCV9_VIS3 | SPARCV9_PREFER_FPU)) {
         am.d[0] = 1;            /* borrow am */
diff --git a/crypto/bn/bn_lcl.h b/crypto/bn/bn_lcl.h
index 00f4f09..1aa7fe8 100644
--- a/crypto/bn/bn_lcl.h
+++ b/crypto/bn/bn_lcl.h
@@ -113,6 +113,7 @@
 # define HEADER_BN_LCL_H
 
 # include <openssl/bn.h>
+# include "bn_int.h"
 
 #ifdef  __cplusplus
 extern "C" {
diff --git a/crypto/bn/bn_lib.c b/crypto/bn/bn_lib.c
index f49c61c..c6005bf 100644
--- a/crypto/bn/bn_lib.c
+++ b/crypto/bn/bn_lib.c
@@ -263,8 +263,6 @@ static BN_ULONG *bn_expand_internal(const BIGNUM *b, int words)
     const BN_ULONG *B;
     int i;
 
-    bn_check_top(b);
-
     if (words > (INT_MAX / (4 * BN_BITS2))) {
         BNerr(BN_F_BN_EXPAND_INTERNAL, BN_R_BIGNUM_TOO_LONG);
         return NULL;
@@ -398,8 +396,6 @@ BIGNUM *bn_dup_expand(const BIGNUM *b, int words)
 
 BIGNUM *bn_expand2(BIGNUM *b, int words)
 {
-    bn_check_top(b);
-
     if (words > b->dmax) {
         BN_ULONG *a = bn_expand_internal(b, words);
         if (!a)
@@ -433,7 +429,6 @@ BIGNUM *bn_expand2(BIGNUM *b, int words)
         assert(A == &(b->d[b->dmax]));
     }
 #endif
-    bn_check_top(b);
     return b;
 }
 
@@ -497,14 +492,16 @@ BIGNUM *BN_copy(BIGNUM *a, const BIGNUM *b)
     memcpy(a->d, b->d, sizeof(b->d[0]) * b->top);
 #endif
 
-    a->top = b->top;
     a->neg = b->neg;
+    a->top = b->top;
+    a->flags |= b->flags & BN_FLG_FIXED_TOP;
     bn_check_top(a);
     return (a);
 }
 
 #define FLAGS_DATA(flags) ((flags) & (BN_FLG_STATIC_DATA \
-                                    | BN_FLG_CONSTTIME))
+                                    | BN_FLG_CONSTTIME   \
+                                    | BN_FLG_FIXED_TOP))
 #define FLAGS_STRUCT(flags) ((flags) & (BN_FLG_MALLOCED))
 
 void BN_swap(BIGNUM *a, BIGNUM *b)
@@ -547,6 +544,7 @@ void BN_clear(BIGNUM *a)
         OPENSSL_cleanse(a->d, a->dmax * sizeof(a->d[0]));
     a->top = 0;
     a->neg = 0;
+    a->flags &= ~BN_FLG_FIXED_TOP;
 }
 
 BN_ULONG BN_get_word(const BIGNUM *a)
@@ -567,6 +565,7 @@ int BN_set_word(BIGNUM *a, BN_ULONG w)
     a->neg = 0;
     a->d[0] = w;
     a->top = (w ? 1 : 0);
+    a->flags &= ~BN_FLG_FIXED_TOP;
     bn_check_top(a);
     return (1);
 }
@@ -713,6 +712,7 @@ int BN_set_bit(BIGNUM *a, int n)
         for (k = a->top; k < i + 1; k++)
             a->d[k] = 0;
         a->top = i + 1;
+        a->flags &= ~BN_FLG_FIXED_TOP;
     }
 
     a->d[i] |= (((BN_ULONG)1) << j);
diff --git a/crypto/bn/bn_mod.c b/crypto/bn/bn_mod.c
index ffbce89..23ddd48 100644
--- a/crypto/bn/bn_mod.c
+++ b/crypto/bn/bn_mod.c
@@ -149,18 +149,73 @@ int BN_mod_add(BIGNUM *r, const BIGNUM *a, const BIGNUM *b, const BIGNUM *m,
 
 /*
  * BN_mod_add variant that may be used if both a and b are non-negative and
- * less than m
+ * less than m. The original algorithm was
+ *
+ *    if (!BN_uadd(r, a, b))
+ *       return 0;
+ *    if (BN_ucmp(r, m) >= 0)
+ *       return BN_usub(r, r, m);
+ *
+ * which is replaced with addition, subtracting modulus, and conditional
+ * move depending on whether or not subtraction borrowed.
  */
-int BN_mod_add_quick(BIGNUM *r, const BIGNUM *a, const BIGNUM *b,
-                     const BIGNUM *m)
+int bn_mod_add_fixed_top(BIGNUM *r, const BIGNUM *a, const BIGNUM *b,
+                         const BIGNUM *m)
 {
-    if (!BN_uadd(r, a, b))
+    size_t i, ai, bi, mtop = m->top;
+    BN_ULONG storage[1024 / BN_BITS2];
+    BN_ULONG carry, temp, mask, *rp, *tp = storage;
+    const BN_ULONG *ap, *bp;
+
+    if (bn_wexpand(r, m->top) == NULL)
         return 0;
-    if (BN_ucmp(r, m) >= 0)
-        return BN_usub(r, r, m);
+
+    if (mtop > sizeof(storage) / sizeof(storage[0])
+        && (tp = OPENSSL_malloc(mtop * sizeof(BN_ULONG))) == NULL)
+	return 0;
+
+    ap = a->d != NULL ? a->d : tp;
+    bp = b->d != NULL ? b->d : tp;
+
+    for (i = 0, ai = 0, bi = 0, carry = 0; i < mtop;) {
+        mask = (BN_ULONG)0 - ((i - a->top) >> (8 * sizeof(i) - 1));
+        temp = ((ap[ai] & mask) + carry) & BN_MASK2;
+        carry = (temp < carry);
+
+        mask = (BN_ULONG)0 - ((i - b->top) >> (8 * sizeof(i) - 1));
+        tp[i] = ((bp[bi] & mask) + temp) & BN_MASK2;
+        carry += (tp[i] < temp);
+
+        i++;
+        ai += (i - a->dmax) >> (8 * sizeof(i) - 1);
+        bi += (i - b->dmax) >> (8 * sizeof(i) - 1);
+    }
+    rp = r->d;
+    carry -= bn_sub_words(rp, tp, m->d, mtop);
+    for (i = 0; i < mtop; i++) {
+        rp[i] = (carry & tp[i]) | (~carry & rp[i]);
+        ((volatile BN_ULONG *)tp)[i] = 0;
+    }
+    r->top = mtop;
+    r->neg = 0;
+
+    if (tp != storage)
+        OPENSSL_free(tp);
+
     return 1;
 }
 
+int BN_mod_add_quick(BIGNUM *r, const BIGNUM *a, const BIGNUM *b,
+                     const BIGNUM *m)
+{
+    int ret = bn_mod_add_fixed_top(r, a, b, m);
+
+    if (ret)
+        bn_correct_top(r);
+
+    return ret;
+}
+
 int BN_mod_sub(BIGNUM *r, const BIGNUM *a, const BIGNUM *b, const BIGNUM *m,
                BN_CTX *ctx)
 {
diff --git a/crypto/bn/bn_mont.c b/crypto/bn/bn_mont.c
index c170365..d41434a 100644
--- a/crypto/bn/bn_mont.c
+++ b/crypto/bn/bn_mont.c
@@ -123,12 +123,23 @@
 #define MONT_WORD               /* use the faster word-based algorithm */
 
 #ifdef MONT_WORD
-static int BN_from_montgomery_word(BIGNUM *ret, BIGNUM *r, BN_MONT_CTX *mont);
+static int bn_from_montgomery_word(BIGNUM *ret, BIGNUM *r, BN_MONT_CTX *mont);
 #endif
 
 int BN_mod_mul_montgomery(BIGNUM *r, const BIGNUM *a, const BIGNUM *b,
                           BN_MONT_CTX *mont, BN_CTX *ctx)
 {
+    int ret = bn_mul_mont_fixed_top(r, a, b, mont, ctx);
+
+    bn_correct_top(r);
+    bn_check_top(r);
+
+    return ret;
+}
+
+int bn_mul_mont_fixed_top(BIGNUM *r, const BIGNUM *a, const BIGNUM *b,
+                          BN_MONT_CTX *mont, BN_CTX *ctx)
+{
     BIGNUM *tmp;
     int ret = 0;
 #if defined(OPENSSL_BN_ASM_MONT) && defined(MONT_WORD)
@@ -140,8 +151,8 @@ int BN_mod_mul_montgomery(BIGNUM *r, const BIGNUM *a, const BIGNUM *b,
         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_correct_top(r);
-            return (1);
+            r->flags |= BN_FLG_FIXED_TOP;
+            return 1;
         }
     }
 #endif
@@ -161,13 +172,12 @@ int BN_mod_mul_montgomery(BIGNUM *r, const BIGNUM *a, const BIGNUM *b,
     }
     /* reduce from aRR to aR */
 #ifdef MONT_WORD
-    if (!BN_from_montgomery_word(r, tmp, mont))
+    if (!bn_from_montgomery_word(r, tmp, mont))
         goto err;
 #else
     if (!BN_from_montgomery(r, tmp, mont, ctx))
         goto err;
 #endif
-    bn_check_top(r);
     ret = 1;
  err:
     BN_CTX_end(ctx);
@@ -175,7 +185,7 @@ int BN_mod_mul_montgomery(BIGNUM *r, const BIGNUM *a, const BIGNUM *b,
 }
 
 #ifdef MONT_WORD
-static int BN_from_montgomery_word(BIGNUM *ret, BIGNUM *r, BN_MONT_CTX *mont)
+static int bn_from_montgomery_word(BIGNUM *ret, BIGNUM *r, BN_MONT_CTX *mont)
 {
     BIGNUM *n;
     BN_ULONG *ap, *np, *rp, n0, v, carry;
@@ -205,6 +215,7 @@ static int BN_from_montgomery_word(BIGNUM *ret, BIGNUM *r, BN_MONT_CTX *mont)
 # endif
 
     r->top = max;
+    r->flags |= BN_FLG_FIXED_TOP;
     n0 = mont->n0[0];
 
     /*
@@ -223,6 +234,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;
+    ret->flags |= BN_FLG_FIXED_TOP;
     ret->neg = r->neg;
 
     rp = ret->d;
@@ -233,20 +245,16 @@ static int BN_from_montgomery_word(BIGNUM *ret, BIGNUM *r, BN_MONT_CTX *mont)
      */
     ap = &(r->d[nl]);
 
+    carry -= bn_sub_words(rp, ap, np, nl);
     /*
-     * |v| is one if |ap| - |np| underflowed or zero if it did not. Note |v|
-     * cannot be -1. That would imply the subtraction did not fit in |nl| words,
-     * and we know at most one subtraction is needed.
+     * |carry| is -1 if |ap| - |np| underflowed or zero if it did not. Note
+     * |carry| cannot be 1. That would imply the subtraction did not fit in
+     * |nl| words, and we know at most one subtraction is needed.
      */
-    v = bn_sub_words(rp, ap, np, nl) - carry;
-    v = 0 - v;
     for (i = 0; i < nl; i++) {
-        rp[i] = (v & ap[i]) | (~v & rp[i]);
+        rp[i] = (carry & ap[i]) | (~carry & rp[i]);
         ap[i] = 0;
     }
-    bn_correct_top(r);
-    bn_correct_top(ret);
-    bn_check_top(ret);
 
     return (1);
 }
@@ -260,8 +268,11 @@ int BN_from_montgomery(BIGNUM *ret, const BIGNUM *a, BN_MONT_CTX *mont,
     BIGNUM *t;
 
     BN_CTX_start(ctx);
-    if ((t = BN_CTX_get(ctx)) && BN_copy(t, a))
-        retn = BN_from_montgomery_word(ret, t, mont);
+    if ((t = BN_CTX_get(ctx)) && BN_copy(t, a)) {
+        retn = bn_from_montgomery_word(ret, t, mont);
+        bn_correct_top(ret);
+        bn_check_top(ret);
+    }
     BN_CTX_end(ctx);
 #else                           /* !MONT_WORD */
     BIGNUM *t1, *t2;
@@ -299,6 +310,12 @@ int BN_from_montgomery(BIGNUM *ret, const BIGNUM *a, BN_MONT_CTX *mont,
     return (retn);
 }
 
+int bn_to_mont_fixed_top(BIGNUM *r, const BIGNUM *a, BN_MONT_CTX *mont,
+                         BN_CTX *ctx)
+{
+    return bn_mul_mont_fixed_top(r, a, &(mont->RR), mont, ctx);
+}
+
 BN_MONT_CTX *BN_MONT_CTX_new(void)
 {
     BN_MONT_CTX *ret;
@@ -335,7 +352,7 @@ void BN_MONT_CTX_free(BN_MONT_CTX *mont)
 
 int BN_MONT_CTX_set(BN_MONT_CTX *mont, const BIGNUM *mod, BN_CTX *ctx)
 {
-    int ret = 0;
+    int i, ret = 0;
     BIGNUM *Ri, *R;
 
     if (BN_is_zero(mod))
@@ -466,6 +483,11 @@ int BN_MONT_CTX_set(BN_MONT_CTX *mont, const BIGNUM *mod, BN_CTX *ctx)
     if (!BN_mod(&(mont->RR), &(mont->RR), &(mont->N), ctx))
         goto err;
 
+    for (i = mont->RR.top, ret = mont->N.top; i < ret; i++)
+        mont->RR.d[i] = 0;
+    mont->RR.top = ret;
+    mont->RR.flags |= BN_FLG_FIXED_TOP;
+
     ret = 1;
  err:
     BN_CTX_end(ctx);
diff --git a/crypto/bn/bn_sqr.c b/crypto/bn/bn_sqr.c
index 256d26e..5e69297 100644
--- a/crypto/bn/bn_sqr.c
+++ b/crypto/bn/bn_sqr.c
@@ -135,14 +135,8 @@ int BN_sqr(BIGNUM *r, const BIGNUM *a, BN_CTX *ctx)
     }
 
     rr->neg = 0;
-    /*
-     * If the most-significant half of the top word of 'a' is zero, then the
-     * square of 'a' will max-1 words.
-     */
-    if (a->d[al - 1] == (a->d[al - 1] & BN_MASK2l))
-        rr->top = max - 1;
-    else
-        rr->top = max;
+    rr->top = max;
+    bn_correct_top(rr);
     if (r != rr && BN_copy(r, rr) == NULL)
         goto err;
 
diff --git a/crypto/bn_int.h b/crypto/bn_int.h
new file mode 100644
index 0000000..9683e5f
--- /dev/null
+++ b/crypto/bn_int.h
@@ -0,0 +1,13 @@
+/*
+ * Some BIGNUM functions assume most significant limb to be non-zero, which
+ * is customarily arranged by bn_correct_top. Output from below functions
+ * is not processed with bn_correct_top, and for this reason it may not be
+ * returned out of public API. It may only be passed internally into other
+ * functions known to support non-minimal or zero-padded BIGNUMs.
+ */
+int bn_mul_mont_fixed_top(BIGNUM *r, const BIGNUM *a, const BIGNUM *b,
+                          BN_MONT_CTX *mont, BN_CTX *ctx);
+int bn_to_mont_fixed_top(BIGNUM *r, const BIGNUM *a, BN_MONT_CTX *mont,
+                         BN_CTX *ctx);
+int bn_mod_add_fixed_top(BIGNUM *r, const BIGNUM *a, const BIGNUM *b,
+                         const BIGNUM *m);
diff --git a/crypto/ec/ec_lib.c b/crypto/ec/ec_lib.c
index 3241aa5..0890109 100644
--- a/crypto/ec/ec_lib.c
+++ b/crypto/ec/ec_lib.c
@@ -319,12 +319,16 @@ int EC_GROUP_set_generator(EC_GROUP *group, const EC_POINT *generator,
         BN_zero(&group->cofactor);
 
     /*
-     * We ignore the return value because some groups have an order with
+     * Some groups have an order with
      * factors of two, which makes the Montgomery setup fail.
      * |group->mont_data| will be NULL in this case.
      */
-    ec_precompute_mont_data(group);
+    if (BN_is_odd(&group->order)) {
+        return ec_precompute_mont_data(group);
+    }
 
+    BN_MONT_CTX_free(group->mont_data);
+    group->mont_data = NULL;
     return 1;
 }
 
diff --git a/crypto/ecdsa/ecs_ossl.c b/crypto/ecdsa/ecs_ossl.c
index 6115df7..6940091 100644
--- a/crypto/ecdsa/ecs_ossl.c
+++ b/crypto/ecdsa/ecs_ossl.c
@@ -60,6 +60,7 @@
 #include <openssl/err.h>
 #include <openssl/obj_mac.h>
 #include <openssl/bn.h>
+#include "bn_int.h"
 
 static ECDSA_SIG *ecdsa_do_sign(const unsigned char *dgst, int dlen,
                                 const BIGNUM *, const BIGNUM *,
@@ -251,14 +252,14 @@ static ECDSA_SIG *ecdsa_do_sign(const unsigned char *dgst, int dgst_len,
                                 EC_KEY *eckey)
 {
     int ok = 0, i;
-    BIGNUM *kinv = NULL, *s, *m = NULL, *tmp = NULL, *order = NULL;
-    BIGNUM *blind = NULL, *blindm = NULL;
+    BIGNUM *kinv = NULL, *s, *m = NULL, *order = NULL;
     const BIGNUM *ckinv;
     BN_CTX *ctx = NULL;
     const EC_GROUP *group;
     ECDSA_SIG *ret;
     ECDSA_DATA *ecdsa;
     const BIGNUM *priv_key;
+    BN_MONT_CTX *mont_data;
 
     ecdsa = ecdsa_check(eckey);
     group = EC_KEY_get0_group(eckey);
@@ -270,25 +271,14 @@ static ECDSA_SIG *ecdsa_do_sign(const unsigned char *dgst, int dgst_len,
     }
 
     ret = ECDSA_SIG_new();
-    if (ret == NULL) {
+    if (!ret) {
         ECDSAerr(ECDSA_F_ECDSA_DO_SIGN, ERR_R_MALLOC_FAILURE);
         return NULL;
     }
     s = ret->s;
 
-    ctx = BN_CTX_new();
-    if (ctx == NULL) {
-        ECDSAerr(ECDSA_F_ECDSA_DO_SIGN, ERR_R_MALLOC_FAILURE);
-        goto err;
-    }
-
-    BN_CTX_start(ctx);
-    order = BN_CTX_get(ctx);
-    tmp = BN_CTX_get(ctx);
-    m = BN_CTX_get(ctx);
-    blind = BN_CTX_get(ctx);
-    blindm = BN_CTX_get(ctx);
-    if (blindm == NULL) {
+    if ((ctx = BN_CTX_new()) == NULL || (order = BN_new()) == NULL ||
+        (m = BN_new()) == NULL) {
         ECDSAerr(ECDSA_F_ECDSA_DO_SIGN, ERR_R_MALLOC_FAILURE);
         goto err;
     }
@@ -297,6 +287,8 @@ static ECDSA_SIG *ecdsa_do_sign(const unsigned char *dgst, int dgst_len,
         ECDSAerr(ECDSA_F_ECDSA_DO_SIGN, ERR_R_EC_LIB);
         goto err;
     }
+    mont_data = EC_GROUP_get_mont_data(group);
+
     i = BN_num_bits(order);
     /*
      * Need to truncate digest if it is too long: first truncate whole bytes.
@@ -328,69 +320,37 @@ static ECDSA_SIG *ecdsa_do_sign(const unsigned char *dgst, int dgst_len,
         }
 
         /*
-         * The normal signature calculation is:
-         *
-         *   s := k^-1 * (m + r * priv_key) mod order
-         *
-         * We will blind this to protect against side channel attacks
-         *
-         *   s := blind^-1 * k^-1 * (blind * m + blind * r * priv_key) mod order
+         * With only one multiplicant being in Montgomery domain
+         * multiplication yields real result without post-conversion.
+         * Also note that all operations but last are performed with
+         * zero-padded vectors. Last operation, BN_mod_mul_montgomery
+         * below, returns user-visible value with removed zero padding.
          */
-
-        /* Generate a blinding value */
-        do {
-            if (!BN_rand(blind, BN_num_bits(order) - 1, -1, 0))
-                goto err;
-        } while (BN_is_zero(blind));
-        BN_set_flags(blind, BN_FLG_CONSTTIME);
-        BN_set_flags(blindm, BN_FLG_CONSTTIME);
-        BN_set_flags(tmp, BN_FLG_CONSTTIME);
-
-        /* tmp := blind * priv_key * r mod order */
-        if (!BN_mod_mul(tmp, blind, priv_key, order, ctx)) {
-            ECDSAerr(ECDSA_F_ECDSA_DO_SIGN, ERR_R_BN_LIB);
-            goto err;
-        }
-        if (!BN_mod_mul(tmp, tmp, ret->r, order, ctx)) {
-            ECDSAerr(ECDSA_F_ECDSA_DO_SIGN, ERR_R_BN_LIB);
-            goto err;
-        }
-
-        /* blindm := blind * m mod order */
-        if (!BN_mod_mul(blindm, blind, m, order, ctx)) {
-            ECDSAerr(ECDSA_F_ECDSA_DO_SIGN, ERR_R_BN_LIB);
+        if (!bn_to_mont_fixed_top(s, ret->r, mont_data, ctx)
+            || !bn_mul_mont_fixed_top(s, s, priv_key, mont_data, ctx)) {
             goto err;
         }
-
-        /* s : = (blind * priv_key * r) + (blind * m) mod order */
-        if (!BN_mod_add_quick(s, tmp, blindm, order)) {
-            ECDSAerr(ECDSA_F_ECDSA_DO_SIGN, ERR_R_BN_LIB);
-            goto err;
-        }
-
-        /* s := s * k^-1 mod order */
-        if (!BN_mod_mul(s, s, ckinv, order, ctx)) {
+        if (!bn_mod_add_fixed_top(s, s, m, order)) {
             ECDSAerr(ECDSA_F_ECDSA_DO_SIGN, ERR_R_BN_LIB);
             goto err;
         }
-
-        /* s:= s * blind^-1 mod order */
-        if (BN_mod_inverse(blind, blind, order, ctx) == NULL) {
-            ECDSAerr(ECDSA_F_ECDSA_DO_SIGN, ERR_R_BN_LIB);
-            goto err;
-        }
-        if (!BN_mod_mul(s, s, blind, order, ctx)) {
+        /*
+         * |s| can still be larger than modulus, because |m| can be. In
+         * such case we count on Montgomery reduction to tie it up.
+         */
+        if (!bn_to_mont_fixed_top(s, s, mont_data, ctx)
+            || !BN_mod_mul_montgomery(s, s, ckinv, mont_data, ctx)) {
             ECDSAerr(ECDSA_F_ECDSA_DO_SIGN, ERR_R_BN_LIB);
             goto err;
         }
-
         if (BN_is_zero(s)) {
             /*
              * if kinv and r have been supplied by the caller don't to
              * generate new kinv and r values
              */
             if (in_kinv != NULL && in_r != NULL) {
-                ECDSAerr(ECDSA_F_ECDSA_DO_SIGN, ECDSA_R_NEED_NEW_SETUP_VALUES);
+                ECDSAerr(ECDSA_F_ECDSA_DO_SIGN,
+                         ECDSA_R_NEED_NEW_SETUP_VALUES);
                 goto err;
             }
         } else
@@ -405,11 +365,13 @@ static ECDSA_SIG *ecdsa_do_sign(const unsigned char *dgst, int dgst_len,
         ECDSA_SIG_free(ret);
         ret = NULL;
     }
-    if (ctx != NULL) {
-        BN_CTX_end(ctx);
+    if (ctx)
         BN_CTX_free(ctx);
-    }
-    if (kinv != NULL)
+    if (m)
+        BN_clear_free(m);
+    if (order)
+        BN_free(order);
+    if (kinv)
         BN_clear_free(kinv);
     return ret;
 }


More information about the openssl-commits mailing list