Remove superfluous NULL checks. Add Andy's BN_FLG comment.

Reviewed-by: Andy Polyakov <appro@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6009)

diff --git a/crypto/bn/bn_lib.c b/crypto/bn/bn_lib.c
index a446880ec727228a3a44986b9cad73e3d4600b61..91553d439103aeb1dab27642abff7a548c03118f 100644 (file)
--- a/crypto/bn/bn_lib.c
+++ b/crypto/bn/bn_lib.c
@@ -743,12 +743,27 @@ void BN_consttime_swap(BN_ULONG condition, BIGNUM *a, BIGNUM *b, int nwords)
     a->neg ^= t;
     b->neg ^= t;
 
-    /*
-     * cannot just arbitrarily swap flags.
-     * The way a->d is allocated etc.
-     * BN_FLG_MALLOCED, BN_FLG_STATIC_DATA, ...
+    /*-
+     * Idea behind BN_FLG_STATIC_DATA is actually to
+     * indicate that data may not be written to.
+     * Intention is actually to treat it as it's
+     * read-only data, and some (if not most) of it does
+     * reside in read-only segment. In other words
+     * observation of BN_FLG_STATIC_DATA in
+     * BN_consttime_swap should be treated as fatal
+     * condition. It would either cause SEGV or
+     * effectively cause data corruption.
+     * BN_FLG_MALLOCED refers to BN structure itself,
+     * and hence must be preserved. Remaining flags are
+     * BN_FLG_CONSTIME and BN_FLG_SECURE. Latter must be
+     * preserved, because it determines how x->d was
+     * allocated and hence how to free it. This leaves
+     * BN_FLG_CONSTTIME that one can do something about.
+     * To summarize it's sufficient to mask and swap
+     * BN_FLG_CONSTTIME alone. BN_FLG_STATIC_DATA should
+     * be treated as fatal.
      */
-    t = (a->flags ^ b->flags) & condition & BN_FLG_CONSTTIME;
+    t = ((a->flags ^ b->flags) & BN_FLG_CONSTTIME) & condition;
     a->flags ^= t;
     b->flags ^= t;
 

diff --git a/crypto/ec/ec_mult.c b/crypto/ec/ec_mult.c
index 1ed74492288ac4691cbd01d4700b9f05b46708f6..0779e4f7bbe28360ee305ca5a23ad4401561ebf9 100644 (file)
--- a/crypto/ec/ec_mult.c
+++ b/crypto/ec/ec_mult.c
@@ -142,9 +142,6 @@ static int ec_mul_consttime(const EC_GROUP *group, EC_POINT *r,
     if (ctx == NULL && (ctx = new_ctx = BN_CTX_secure_new()) == NULL)
         goto err;
 
-    if ((group->order == NULL) || (group->field == NULL))
-        goto err;
-
     order_bits = BN_num_bits(group->order);
 
     s = EC_POINT_new(group);
@@ -152,8 +149,6 @@ static int ec_mul_consttime(const EC_GROUP *group, EC_POINT *r,
         goto err;
 
     if (point == NULL) {
-        if (group->generator == NULL)
-            goto err;
         if (!EC_POINT_copy(s, group->generator))
             goto err;
     } else {