[openssl-commits] [openssl] OpenSSL_1_1_1-stable update

[nic.tuv at gmail.com]

The branch OpenSSL_1_1_1-stable has been updated
       via  6f172154f5d389a9e52583c6564b2693cb631e7f (commit)
      from  bdb8897691942931184c38a9ab7b69a0977829b7 (commit)


- Log -----------------------------------------------------------------
commit 6f172154f5d389a9e52583c6564b2693cb631e7f
Author: Billy Brumley <bbrumley at gmail.com>
Date:   Fri Nov 9 09:25:43 2018 +0200

    [crypto/bn] swap BN_FLG_FIXED_TOP too
    
    Reviewed-by: Matt Caswell <matt at openssl.org>
    Reviewed-by: Richard Levitte <levitte at openssl.org>
    Reviewed-by: Nicola Tuveri <nic.tuv at gmail.com>
    (Merged from https://github.com/openssl/openssl/pull/7599)
    
    (cherry picked from commit dd41956d80686638d74fd203bd67060f90966280)

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

Summary of changes:
 crypto/bn/bn_lib.c | 42 +++++++++++++++++++++++-------------------
 1 file changed, 23 insertions(+), 19 deletions(-)

diff --git a/crypto/bn/bn_lib.c b/crypto/bn/bn_lib.c
index 266a3dd..80f910c 100644
--- a/crypto/bn/bn_lib.c
+++ b/crypto/bn/bn_lib.c
@@ -767,26 +767,30 @@ void BN_consttime_swap(BN_ULONG condition, BIGNUM *a, BIGNUM *b, int nwords)
     b->neg ^= t;
 
     /*-
-     * 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.
+     * BN_FLG_STATIC_DATA: indicates 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.
+     *
+     * BN_FLG_SECURE: must be preserved, because it determines how x->d was
+     * allocated and hence how to free it.
+     *
+     * BN_FLG_CONSTTIME: sufficient to mask and swap
+     *
+     * BN_FLG_FIXED_TOP: indicates that we haven't called bn_correct_top() on
+     * the data, so the d array may be padded with additional 0 values (i.e.
+     * top could be greater than the minimal value that it could be). We should
+     * be swapping it
      */
-    t = ((a->flags ^ b->flags) & BN_FLG_CONSTTIME) & condition;
+
+#define BN_CONSTTIME_SWAP_FLAGS (BN_FLG_CONSTTIME | BN_FLG_FIXED_TOP)
+
+    t = ((a->flags ^ b->flags) & BN_CONSTTIME_SWAP_FLAGS) & condition;
     a->flags ^= t;
     b->flags ^= t;