[openssl-commits] [openssl] master update

[Matt Caswell]

The branch master has been updated
       via  9e5b50b54d1032634979c224f2dd11c84f2900b7 (commit)
      from  32c6985349ba134761f75a3f61814234d096a1df (commit)


- Log -----------------------------------------------------------------
commit 9e5b50b54d1032634979c224f2dd11c84f2900b7
Author: Billy Brumley <bbrumley at gmail.com>
Date:   Thu Apr 26 18:08:36 2018 +0300

    fix: BN_swap mishandles flags
    
    Reviewed-by: Rich Salz <rsalz at openssl.org>
    Reviewed-by: Matt Caswell <matt at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/6099)

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

Summary of changes:
 crypto/bn/bn_lib.c | 11 +++++---
 test/bntest.c      | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 80 insertions(+), 4 deletions(-)

diff --git a/crypto/bn/bn_lib.c b/crypto/bn/bn_lib.c
index 91553d4..5bb996e 100644
--- a/crypto/bn/bn_lib.c
+++ b/crypto/bn/bn_lib.c
@@ -300,6 +300,11 @@ BIGNUM *BN_copy(BIGNUM *a, const BIGNUM *b)
     return a;
 }
 
+#define FLAGS_DATA(flags) ((flags) & (BN_FLG_STATIC_DATA \
+                                    | BN_FLG_CONSTTIME   \
+                                    | BN_FLG_SECURE))
+#define FLAGS_STRUCT(flags) ((flags) & (BN_FLG_MALLOCED))
+
 void BN_swap(BIGNUM *a, BIGNUM *b)
 {
     int flags_old_a, flags_old_b;
@@ -327,10 +332,8 @@ void BN_swap(BIGNUM *a, BIGNUM *b)
     b->dmax = tmp_dmax;
     b->neg = tmp_neg;
 
-    a->flags =
-        (flags_old_a & BN_FLG_MALLOCED) | (flags_old_b & BN_FLG_STATIC_DATA);
-    b->flags =
-        (flags_old_b & BN_FLG_MALLOCED) | (flags_old_a & BN_FLG_STATIC_DATA);
+    a->flags = FLAGS_STRUCT(flags_old_a) | FLAGS_DATA(flags_old_b);
+    b->flags = FLAGS_STRUCT(flags_old_b) | FLAGS_DATA(flags_old_a);
     bn_check_top(a);
     bn_check_top(b);
 }
diff --git a/test/bntest.c b/test/bntest.c
index d5b5e04..629707a 100644
--- a/test/bntest.c
+++ b/test/bntest.c
@@ -151,6 +151,78 @@ static int rand_neg(void)
 }
 
 
+static int test_swap(void)
+{
+    BIGNUM *a = NULL, *b = NULL, *c = NULL, *d = NULL;
+    int top, cond, st = 0;
+
+    if (!TEST_ptr(a = BN_new())
+            || !TEST_ptr(b = BN_new())
+            || !TEST_ptr(c = BN_new())
+            || !TEST_ptr(d = BN_new()))
+        goto err;
+
+    BN_bntest_rand(a, 1024, 1, 0);
+    BN_bntest_rand(b, 1024, 1, 0);
+    BN_copy(c, a);
+    BN_copy(d, b);
+    top = BN_num_bits(a)/BN_BITS2;
+
+    /* regular swap */
+    BN_swap(a, b);
+    if (!equalBN("swap", a, d)
+            || !equalBN("swap", b, c))
+        goto err;
+
+    /* conditional swap: true */
+    cond = 1;
+    BN_consttime_swap(cond, a, b, top);
+    if (!equalBN("cswap true", a, c)
+            || !equalBN("cswap true", b, d))
+        goto err;
+
+    /* conditional swap: false */
+    cond = 0;
+    BN_consttime_swap(cond, a, b, top);
+    if (!equalBN("cswap false", a, c)
+            || !equalBN("cswap false", b, d))
+        goto err;
+
+    /* same tests but checking flag swap */
+    BN_set_flags(a, BN_FLG_CONSTTIME);
+
+    BN_swap(a, b);
+    if (!equalBN("swap, flags", a, d)
+            || !equalBN("swap, flags", b, c)
+            || !TEST_true(BN_get_flags(b, BN_FLG_CONSTTIME))
+            || !TEST_false(BN_get_flags(a, BN_FLG_CONSTTIME)))
+        goto err;
+
+    cond = 1;
+    BN_consttime_swap(cond, a, b, top);
+    if (!equalBN("cswap true, flags", a, c)
+            || !equalBN("cswap true, flags", b, d)
+            || !TEST_true(BN_get_flags(a, BN_FLG_CONSTTIME))
+            || !TEST_false(BN_get_flags(b, BN_FLG_CONSTTIME)))
+        goto err;
+
+    cond = 0;
+    BN_consttime_swap(cond, a, b, top);
+    if (!equalBN("cswap false, flags", a, c)
+            || !equalBN("cswap false, flags", b, d)
+            || !TEST_true(BN_get_flags(a, BN_FLG_CONSTTIME))
+            || !TEST_false(BN_get_flags(b, BN_FLG_CONSTTIME)))
+        goto err;
+
+    st = 1;
+ err:
+    BN_free(a);
+    BN_free(b);
+    BN_free(c);
+    BN_free(d);
+    return st;
+}
+
 static int test_sub(void)
 {
     BIGNUM *a = NULL, *b = NULL, *c = NULL;
@@ -2118,6 +2190,7 @@ int setup_tests(void)
         ADD_TEST(test_badmod);
         ADD_TEST(test_expmodzero);
         ADD_TEST(test_smallprime);
+        ADD_TEST(test_swap);
 #ifndef OPENSSL_NO_EC2M
         ADD_TEST(test_gf2m_add);
         ADD_TEST(test_gf2m_mod);