[openssl] master update

nic.tuv at gmail.com nic.tuv at gmail.com
Wed Feb 20 18:22:37 UTC 2019


The branch master has been updated
       via  c8147d37ccaaf28c430d3fb45a14af36597e48b8 (commit)
       via  fe16ae5f95fa86ddb049a8d1e2caee0b80b32282 (commit)
      from  0b76ce99aaa5678b44cb99df464e977975747928 (commit)


- Log -----------------------------------------------------------------
commit c8147d37ccaaf28c430d3fb45a14af36597e48b8
Author: Nicola Tuveri <nic.tuv at gmail.com>
Date:   Fri Feb 8 12:42:25 2019 +0200

    Clear BN_FLG_CONSTTIME on BN_CTX_get()
    
    Reviewed-by: Matt Caswell <matt at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/8253)

commit fe16ae5f95fa86ddb049a8d1e2caee0b80b32282
Author: Nicola Tuveri <nic.tuv at gmail.com>
Date:   Tue Feb 12 00:37:25 2019 +0200

    Test for constant-time flag leakage in BN_CTX
    
    This commit adds a simple unit test to make sure that the constant-time
    flag does not "leak" among BN_CTX frames:
    
    - test_ctx_consttime_flag() initializes (and later frees before
      returning) a BN_CTX object, then it calls in sequence
      test_ctx_set_ct_flag() and test_ctx_check_ct_flag() using the same
      BN_CTX object. The process is run twice, once with a "normal"
      BN_CTX_new() object, then with a BN_CTX_secure_new() one.
    - test_ctx_set_ct_flag() starts a frame in the given BN_CTX and sets the
      BN_FLG_CONSTTIME flag on some of the BIGNUMs obtained from the frame
      before ending it.
    - test_ctx_check_ct_flag() then starts a new frame and gets a number of
      BIGNUMs from it. In absence of leaks, none of the BIGNUMs in the new
      frame should have BN_FLG_CONSTTIME set.
    
    In actual BN_CTX usage inside libcrypto the leak could happen at any
    depth level in the BN_CTX stack, with varying results depending on the
    patterns of sibling trees of nested function calls sharing the same
    BN_CTX object, and the effect of unintended BN_FLG_CONSTTIME on the
    called BN_* functions.
    
    This simple unit test abstracts away this complexity and verifies that
    the leak does not happen between two sibling functions sharing the same
    BN_CTX object at the same level of nesting.
    
    Reviewed-by: Matt Caswell <matt at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/8253)

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

Summary of changes:
 crypto/bn/bn_ctx.c |   4 +-
 test/bntest.c      | 161 ++++++++++++++++++++++++++++++++++++++++-------------
 2 files changed, 125 insertions(+), 40 deletions(-)

diff --git a/crypto/bn/bn_ctx.c b/crypto/bn/bn_ctx.c
index 5106878..d6e7605 100644
--- a/crypto/bn/bn_ctx.c
+++ b/crypto/bn/bn_ctx.c
@@ -1,5 +1,5 @@
 /*
- * Copyright 2000-2018 The OpenSSL Project Authors. All Rights Reserved.
+ * Copyright 2000-2019 The OpenSSL Project Authors. All Rights Reserved.
  *
  * Licensed under the Apache License 2.0 (the "License").  You may not use
  * this file except in compliance with the License.  You can obtain a copy
@@ -227,6 +227,8 @@ BIGNUM *BN_CTX_get(BN_CTX *ctx)
     }
     /* OK, make sure the returned bignum is "zero" */
     BN_zero(ret);
+    /* clear BN_FLG_CONSTTIME if leaked from previous frames */
+    ret->flags &= (~BN_FLG_CONSTTIME);
     ctx->used++;
     CTXDBG_RET(ctx, ret);
     return ret;
diff --git a/test/bntest.c b/test/bntest.c
index e4b71e2..8c8acb6 100644
--- a/test/bntest.c
+++ b/test/bntest.c
@@ -1,5 +1,5 @@
 /*
- * Copyright 1995-2018 The OpenSSL Project Authors. All Rights Reserved.
+ * Copyright 1995-2019 The OpenSSL Project Authors. All Rights Reserved.
  *
  * Licensed under the Apache License 2.0 (the "License").  You may not use
  * this file except in compliance with the License.  You can obtain a copy
@@ -123,7 +123,7 @@ static int getint(STANZA *s, int *out, const char *attribute)
 
     *out = (int)word;
     st = 1;
-err:
+ err:
     BN_free(ret);
     return st;
 }
@@ -138,7 +138,6 @@ static int equalBN(const char *op, const BIGNUM *expected, const BIGNUM *actual)
     return 0;
 }
 
-
 /*
  * Return a "random" flag for if a BN should be negated.
  */
@@ -150,7 +149,6 @@ static int rand_neg(void)
     return sign[(neg++) % 8];
 }
 
-
 static int test_swap(void)
 {
     BIGNUM *a = NULL, *b = NULL, *c = NULL, *d = NULL;
@@ -166,7 +164,7 @@ static int test_swap(void)
     BN_bntest_rand(b, 1024, 1, 0);
     BN_copy(c, a);
     BN_copy(d, b);
-    top = BN_num_bits(a)/BN_BITS2;
+    top = BN_num_bits(a) / BN_BITS2;
 
     /* regular swap */
     BN_swap(a, b);
@@ -252,14 +250,13 @@ static int test_sub(void)
             goto err;
     }
     st = 1;
-err:
+ err:
     BN_free(a);
     BN_free(b);
     BN_free(c);
     return st;
 }
 
-
 static int test_div_recip(void)
 {
     BIGNUM *a = NULL, *b = NULL, *c = NULL, *d = NULL, *e = NULL;
@@ -293,7 +290,7 @@ static int test_div_recip(void)
             goto err;
     }
     st = 1;
-err:
+ err:
     BN_free(a);
     BN_free(b);
     BN_free(c);
@@ -303,7 +300,6 @@ err:
     return st;
 }
 
-
 static int test_mod(void)
 {
     BIGNUM *a = NULL, *b = NULL, *c = NULL, *d = NULL, *e = NULL;
@@ -328,7 +324,7 @@ static int test_mod(void)
             goto err;
     }
     st = 1;
-err:
+ err:
     BN_free(a);
     BN_free(b);
     BN_free(c);
@@ -573,7 +569,7 @@ static int test_modexp_mont5(void)
 
     st = 1;
 
-err:
+ err:
     BN_MONT_CTX_free(mont);
     BN_free(a);
     BN_free(p);
@@ -1141,7 +1137,7 @@ static int file_sum(STANZA *s)
     }
     st = 1;
 
-err:
+ err:
     BN_free(a);
     BN_free(b);
     BN_free(sum);
@@ -1190,7 +1186,7 @@ static int file_lshift1(STANZA *s)
         goto err;
 
     st = 1;
-err:
+ err:
     BN_free(a);
     BN_free(lshift1);
     BN_free(zero);
@@ -1219,7 +1215,7 @@ static int file_lshift(STANZA *s)
         goto err;
 
     st = 1;
-err:
+ err:
     BN_free(a);
     BN_free(lshift);
     BN_free(ret);
@@ -1249,7 +1245,7 @@ static int file_rshift(STANZA *s)
     }
     st = 1;
 
-err:
+ err:
     BN_free(a);
     BN_free(rshift);
     BN_free(ret);
@@ -1306,7 +1302,7 @@ static int file_square(STANZA *s)
 #endif
 
     st = 1;
-err:
+ err:
     BN_free(a);
     BN_free(square);
     BN_free(zero);
@@ -1343,7 +1339,7 @@ static int file_product(STANZA *s)
         goto err;
 
     st = 1;
-err:
+ err:
     BN_free(a);
     BN_free(b);
     BN_free(product);
@@ -1426,7 +1422,7 @@ static int file_quotient(STANZA *s)
     }
 
     st = 1;
-err:
+ err:
     BN_free(a);
     BN_free(b);
     BN_free(quotient);
@@ -1480,7 +1476,7 @@ static int file_modmul(STANZA *s)
     }
 
     st = 1;
-err:
+ err:
     BN_free(a);
     BN_free(b);
     BN_free(m);
@@ -1532,7 +1528,7 @@ static int file_modexp(STANZA *s)
         goto err;
 
     st = 1;
-err:
+ err:
     BN_free(a);
     BN_free(b);
     BN_free(c);
@@ -1560,7 +1556,7 @@ static int file_exp(STANZA *s)
         goto err;
 
     st = 1;
-err:
+ err:
     BN_free(a);
     BN_free(e);
     BN_free(exp);
@@ -1591,7 +1587,7 @@ static int file_modsqrt(STANZA *s)
         goto err;
 
     st = 1;
-err:
+ err:
     BN_free(a);
     BN_free(p);
     BN_free(mod_sqrt);
@@ -1621,8 +1617,8 @@ static int test_bn2padded(void)
 
     /* Test a random numbers at various byte lengths. */
     for (size_t bytes = 128 - 7; bytes <= 128; bytes++) {
-#define TOP_BIT_ON 0
-#define BOTTOM_BIT_NOTOUCH 0
+# define TOP_BIT_ON 0
+# define BOTTOM_BIT_NOTOUCH 0
         if (!TEST_true(BN_rand(n, bytes * 8, TOP_BIT_ON, BOTTOM_BIT_NOTOUCH)))
             goto err;
         if (!TEST_int_eq(BN_num_bytes(n),A) bytes
@@ -1653,7 +1649,7 @@ static int test_bn2padded(void)
     }
 
     st = 1;
-err:
+ err:
     BN_free(n);
     return st;
 #else
@@ -1725,7 +1721,7 @@ static int test_dec2bn(void)
         goto err;
 
     st = 1;
-err:
+ err:
     BN_free(bn);
     return st;
 }
@@ -1791,7 +1787,7 @@ static int test_hex2bn(void)
         goto err;
     st = 1;
 
-err:
+ err:
     BN_free(bn);
     return st;
 }
@@ -1845,7 +1841,7 @@ static int test_asc2bn(void)
         goto err;
 
     st = 1;
-err:
+ err:
     BN_free(bn);
     return st;
 }
@@ -1889,7 +1885,7 @@ static int test_mpi(int i)
     BN_free(bn2);
 
     st = 1;
-err:
+ err:
     BN_free(bn);
     return st;
 }
@@ -1915,7 +1911,7 @@ static int test_rand(void)
         goto err;
 
     st = 1;
-err:
+ err:
     BN_free(bn);
     return st;
 }
@@ -1979,7 +1975,7 @@ static int test_negzero(void)
         goto err;
     st = 1;
 
-err:
+ err:
     BN_free(a);
     BN_free(b);
     BN_free(c);
@@ -2020,7 +2016,7 @@ static int test_badmod(void)
     ERR_clear_error();
 
     if (!TEST_false(BN_mod_exp_mont_consttime(a, BN_value_one(), BN_value_one(),
-                                             zero, ctx, NULL)))
+                                              zero, ctx, NULL)))
         goto err;
     ERR_clear_error();
 
@@ -2042,12 +2038,12 @@ static int test_badmod(void)
     ERR_clear_error();
 
     if (!TEST_false(BN_mod_exp_mont_consttime(a, BN_value_one(), BN_value_one(),
-                                  b, ctx, NULL)))
+                                              b, ctx, NULL)))
         goto err;
     ERR_clear_error();
 
     st = 1;
-err:
+ err:
     BN_free(a);
     BN_free(b);
     BN_free(zero);
@@ -2081,7 +2077,7 @@ static int test_expmodzero(void)
         goto err;
 
     st = 1;
-err:
+ err:
     BN_free(zero);
     BN_free(a);
     BN_free(r);
@@ -2127,7 +2123,7 @@ static int test_expmodone(void)
     }
 
     ret = 1;
-err:
+ err:
     BN_free(r);
     BN_free(a);
     BN_free(p);
@@ -2148,7 +2144,7 @@ static int test_smallprime(void)
         goto err;
 
     st = 1;
-err:
+ err:
     BN_free(r);
     return st;
 }
@@ -2172,7 +2168,7 @@ static int test_is_prime(int i)
     }
 
     ret = 1;
-err:
+ err:
     BN_free(r);
     return ret;
 }
@@ -2195,11 +2191,97 @@ static int test_not_prime(int i)
     }
 
     ret = 1;
-err:
+ err:
     BN_free(r);
     return ret;
 }
 
+static int test_ctx_set_ct_flag(BN_CTX *c)
+{
+    int st = 0;
+    size_t i;
+    BIGNUM *b[15];
+
+    BN_CTX_start(c);
+    for (i = 0; i < OSSL_NELEM(b); i++) {
+        if (!TEST_ptr(b[i] = BN_CTX_get(c)))
+            goto err;
+        if (i % 2 == 1)
+            BN_set_flags(b[i], BN_FLG_CONSTTIME);
+    }
+
+    st = 1;
+ err:
+    BN_CTX_end(c);
+    return st;
+}
+
+static int test_ctx_check_ct_flag(BN_CTX *c)
+{
+    int st = 0;
+    size_t i;
+    BIGNUM *b[30];
+
+    BN_CTX_start(c);
+    for (i = 0; i < OSSL_NELEM(b); i++) {
+        if (!TEST_ptr(b[i] = BN_CTX_get(c)))
+            goto err;
+        if (!TEST_false(BN_get_flags(b[i], BN_FLG_CONSTTIME)))
+            goto err;
+    }
+
+    st = 1;
+ err:
+    BN_CTX_end(c);
+    return st;
+}
+
+static int test_ctx_consttime_flag(void)
+{
+    /*-
+     * The constant-time flag should not "leak" among BN_CTX frames:
+     *
+     * - test_ctx_set_ct_flag() starts a frame in the given BN_CTX and
+     *   sets the BN_FLG_CONSTTIME flag on some of the BIGNUMs obtained
+     *   from the frame before ending it.
+     * - test_ctx_check_ct_flag() then starts a new frame and gets a
+     *   number of BIGNUMs from it. In absence of leaks, none of the
+     *   BIGNUMs in the new frame should have BN_FLG_CONSTTIME set.
+     *
+     * In actual BN_CTX usage inside libcrypto the leak could happen at
+     * any depth level in the BN_CTX stack, with varying results
+     * depending on the patterns of sibling trees of nested function
+     * calls sharing the same BN_CTX object, and the effect of
+     * unintended BN_FLG_CONSTTIME on the called BN_* functions.
+     *
+     * This simple unit test abstracts away this complexity and verifies
+     * that the leak does not happen between two sibling functions
+     * sharing the same BN_CTX object at the same level of nesting.
+     *
+     */
+    BN_CTX *nctx = NULL;
+    BN_CTX *sctx = NULL;
+    size_t i = 0;
+    int st = 0;
+
+    if (!TEST_ptr(nctx = BN_CTX_new())
+            || !TEST_ptr(sctx = BN_CTX_secure_new()))
+        goto err;
+
+    for (i = 0; i < 2; i++) {
+        BN_CTX *c = i == 0 ? nctx : sctx;
+        if (!TEST_true(test_ctx_set_ct_flag(c))
+                || !TEST_true(test_ctx_check_ct_flag(c)))
+            goto err;
+    }
+
+    st = 1;
+ err:
+    BN_CTX_free(nctx);
+    BN_CTX_free(sctx);
+    return st;
+}
+
 static int file_test_run(STANZA *s)
 {
     static const FILETEST filetests[] = {
@@ -2298,6 +2380,7 @@ int setup_tests(void)
         ADD_TEST(test_expmodone);
         ADD_TEST(test_smallprime);
         ADD_TEST(test_swap);
+        ADD_TEST(test_ctx_consttime_flag);
 #ifndef OPENSSL_NO_EC2M
         ADD_TEST(test_gf2m_add);
         ADD_TEST(test_gf2m_mod);


More information about the openssl-commits mailing list