[openssl] OpenSSL_1_1_0-stable update

nic.tuv at gmail.com nic.tuv at gmail.com
Thu Feb 21 20:05:35 UTC 2019


The branch OpenSSL_1_1_0-stable has been updated
       via  f499873c2ff5a6da5f1a23c099730f97c822e90c (commit)
       via  c4e901dbdb217a78fcca75478dd8cf3720f6219c (commit)
      from  b7fc0784c4cfe81db8728f814925c6f98dd948d1 (commit)


- Log -----------------------------------------------------------------
commit f499873c2ff5a6da5f1a23c099730f97c822e90c
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()
    
    (cherry picked from commit c8147d37ccaaf28c430d3fb45a14af36597e48b8)
    
    Reviewed-by: Matt Caswell <matt at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/8294)

commit c4e901dbdb217a78fcca75478dd8cf3720f6219c
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.
    
    (manually cherry picked from commit fe16ae5f95fa86ddb049a8d1e2caee0b80b32282)
    
    Reviewed-by: Matt Caswell <matt at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/8294)

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

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

diff --git a/crypto/bn/bn_ctx.c b/crypto/bn/bn_ctx.c
index 68c0468..51db38b 100644
--- a/crypto/bn/bn_ctx.c
+++ b/crypto/bn/bn_ctx.c
@@ -1,5 +1,5 @@
 /*
- * Copyright 2000-2016 The OpenSSL Project Authors. All Rights Reserved.
+ * Copyright 2000-2019 The OpenSSL Project Authors. All Rights Reserved.
  *
  * Licensed under the OpenSSL license (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 686eab8..606cc11 100644
--- a/test/bntest.c
+++ b/test/bntest.c
@@ -1,5 +1,5 @@
 /*
- * Copyright 1995-2016 The OpenSSL Project Authors. All Rights Reserved.
+ * Copyright 1995-2019 The OpenSSL Project Authors. All Rights Reserved.
  *
  * Licensed under the OpenSSL license (the "License").  You may not use
  * this file except in compliance with the License.  You can obtain a copy
@@ -85,6 +85,7 @@ int test_sqrt(BIO *bp, BN_CTX *ctx);
 int test_small_prime(BIO *bp, BN_CTX *ctx);
 int test_bn2dec(BIO *bp);
 int rand_neg(void);
+static int test_ctx_consttime_flag(void);
 static int results = 0;
 
 static unsigned char lst[] =
@@ -312,11 +313,18 @@ int main(int argc, char *argv[])
         goto err;
     (void)BIO_flush(out);
 #endif
+
+    /* silently flush any pre-existing error on the stack */
+    ERR_clear_error();
+
+    message(out, "BN_CTX_get BN_FLG_CONSTTIME");
+    if (!test_ctx_consttime_flag())
+        goto err;
+    (void)BIO_flush(out);
+
     BN_CTX_free(ctx);
     BIO_free(out);
 
-    ERR_print_errors_fp(stderr);
-
 #ifndef OPENSSL_NO_CRYPTO_MDEBUG
     if (CRYPTO_mem_leaks_fp(stderr) <= 0)
         EXIT(1);
@@ -2092,3 +2100,100 @@ int rand_neg(void)
 
     return (sign[(neg++) % 8]);
 }
+
+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 (NULL == (b[i] = BN_CTX_get(c))) {
+            fprintf(stderr, "ERROR: BN_CTX_get() failed.\n");
+            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 (NULL == (b[i] = BN_CTX_get(c))) {
+            fprintf(stderr, "ERROR: BN_CTX_get() failed.\n");
+            goto err;
+        }
+        if (BN_get_flags(b[i], BN_FLG_CONSTTIME) != 0) {
+            fprintf(stderr, "ERROR: BN_FLG_CONSTTIME should not be set.\n");
+            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 (NULL == (nctx = BN_CTX_new())) {
+        fprintf(stderr, "ERROR: BN_CTX_new() failed.\n");
+        goto err;
+    }
+    if (NULL == (sctx = BN_CTX_secure_new())) {
+        fprintf(stderr, "ERROR: BN_CTX_secure_new() failed.\n");
+        goto err;
+    }
+
+    for (i = 0; i < 2; i++) {
+        BN_CTX *c = i == 0 ? nctx : sctx;
+        if (!test_ctx_set_ct_flag(c)
+                || !test_ctx_check_ct_flag(c))
+            goto err;
+    }
+
+    st = 1;
+ err:
+    BN_CTX_free(nctx);
+    BN_CTX_free(sctx);
+    return st;
+}


More information about the openssl-commits mailing list