[openssl-commits] [openssl] OpenSSL_1_0_2-stable update

Andy Polyakov appro at openssl.org
Fri Aug 10 19:08:38 UTC 2018


The branch OpenSSL_1_0_2-stable has been updated
       via  ec3f996b3066ecaaec87ba5ad29c606aeac0740d (commit)
       via  df6b67becc1f41c2eeee7e20ff10b5ec42ced58b (commit)
       via  6412738be390dd9bf680cef89f22e4c810ab065f (commit)
      from  f72a7ce8bc0a5c0866c6a848a7f54854d67aeba2 (commit)


- Log -----------------------------------------------------------------
commit ec3f996b3066ecaaec87ba5ad29c606aeac0740d
Author: Andy Polyakov <appro at openssl.org>
Date:   Sun Feb 4 15:24:54 2018 +0100

    rsa/*: switch to BN_bn2binpad.
    
    Reviewed-by: Rich Salz <rsalz at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/6889)
    
    (cherry picked from commit 582ad5d4d9b7703eb089016935133e3a18ea8205)
    
    Resolved conflicts:
    	crypto/rsa/rsa_ossl.c
    	crypto/rsa/rsa_pk1.c

commit df6b67becc1f41c2eeee7e20ff10b5ec42ced58b
Author: Andy Polyakov <appro at openssl.org>
Date:   Mon Jul 16 18:17:44 2018 +0200

    bn/bn_lib.c address Coverity nit in bn2binpad.
    
    It was false positive, but one can as well view it as readability issue.
    Switch even to unsigned indices because % BN_BYTES takes 4-6 instructions
    with signed dividend vs. 1 (one) with unsigned.
    
    Reviewed-by: Rich Salz <rsalz at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/6889)
    
    (cherry picked from commit 83e034379fa3f6f0d308ec75fbcb137e26154aec)

commit 6412738be390dd9bf680cef89f22e4c810ab065f
Author: Andy Polyakov <appro at openssl.org>
Date:   Sun Feb 4 15:20:29 2018 +0100

    bn/bn_lib.c: add computationally constant-time bn_bn2binpad.
    
    "Computationally constant-time" means that it might still leak
    information about input's length, but only in cases when input
    is missing complete BN_ULONG limbs. But even then leak is possible
    only if attacker can observe memory access pattern with limb
    granularity.
    
    Reviewed-by: Rich Salz <rsalz at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/6889)
    
    (cherry picked from commit 89d8aade5f4011ddeea7827f08ec544c914f275a)
    
    Resolved conflicts:
    	crypto/bn/bn_lib.c

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

Summary of changes:
 crypto/bn/bn_lib.c    | 35 +++++++++++++++++++++++++++++
 crypto/bn_int.h       |  2 ++
 crypto/rsa/rsa_eay.c  | 39 +++++++++++---------------------
 crypto/rsa/rsa_oaep.c | 39 +++++++++++++++++++-------------
 crypto/rsa/rsa_pk1.c  | 62 +++++++++++++++++++++++++++++++++++----------------
 crypto/rsa/rsa_ssl.c  |  8 +++++++
 6 files changed, 125 insertions(+), 60 deletions(-)

diff --git a/crypto/bn/bn_lib.c b/crypto/bn/bn_lib.c
index c6005bf..03bd8cd 100644
--- a/crypto/bn/bn_lib.c
+++ b/crypto/bn/bn_lib.c
@@ -614,6 +614,41 @@ BIGNUM *BN_bin2bn(const unsigned char *s, int len, BIGNUM *ret)
 }
 
 /* ignore negative */
+static int bn2binpad(const BIGNUM *a, unsigned char *to, int tolen)
+{
+    int n;
+    size_t i, inc, lasti, j;
+    BN_ULONG l;
+
+    n = BN_num_bytes(a);
+    if (tolen == -1)
+        tolen = n;
+    else if (tolen < n)
+        return -1;
+
+    if (n == 0) {
+        OPENSSL_cleanse(to, tolen);
+        return tolen;
+    }
+
+    lasti = n - 1;
+    for (i = 0, inc = 1, j = tolen; j > 0;) {
+        l = a->d[i / BN_BYTES];
+        to[--j] = (unsigned char)(l >> (8 * (i % BN_BYTES)) & (0 - inc));
+        inc = (i - lasti) >> (8 * sizeof(i) - 1);
+        i += inc; /* stay on top limb */
+    }
+
+    return tolen;
+}
+
+int bn_bn2binpad(const BIGNUM *a, unsigned char *to, int tolen)
+{
+    if (tolen < 0)
+        return -1;
+    return bn2binpad(a, to, tolen);
+}
+
 int BN_bn2bin(const BIGNUM *a, unsigned char *to)
 {
     int n, i;
diff --git a/crypto/bn_int.h b/crypto/bn_int.h
index 9683e5f..9c42d6f 100644
--- a/crypto/bn_int.h
+++ b/crypto/bn_int.h
@@ -11,3 +11,5 @@ int bn_to_mont_fixed_top(BIGNUM *r, const BIGNUM *a, BN_MONT_CTX *mont,
                          BN_CTX *ctx);
 int bn_mod_add_fixed_top(BIGNUM *r, const BIGNUM *a, const BIGNUM *b,
                          const BIGNUM *m);
+
+int bn_bn2binpad(const BIGNUM *a, unsigned char *to, int tolen);
diff --git a/crypto/rsa/rsa_eay.c b/crypto/rsa/rsa_eay.c
index b147fff..b9c6855 100644
--- a/crypto/rsa/rsa_eay.c
+++ b/crypto/rsa/rsa_eay.c
@@ -114,6 +114,7 @@
 #include <openssl/bn.h>
 #include <openssl/rsa.h>
 #include <openssl/rand.h>
+#include "bn_int.h"
 
 #ifndef RSA_NULL
 
@@ -156,7 +157,7 @@ static int RSA_eay_public_encrypt(int flen, const unsigned char *from,
                                   unsigned char *to, RSA *rsa, int padding)
 {
     BIGNUM *f, *ret;
-    int i, j, k, num = 0, r = -1;
+    int i, num = 0, r = -1;
     unsigned char *buf = NULL;
     BN_CTX *ctx = NULL;
 
@@ -232,15 +233,10 @@ static int RSA_eay_public_encrypt(int flen, const unsigned char *from,
         goto err;
 
     /*
-     * put in leading 0 bytes if the number is less than the length of the
-     * modulus
+     * BN_bn2binpad puts in leading 0 bytes if the number is less than
+     * the length of the modulus.
      */
-    j = BN_num_bytes(ret);
-    i = BN_bn2bin(ret, &(to[num - j]));
-    for (k = 0; k < (num - i); k++)
-        to[k] = 0;
-
-    r = num;
+    r = bn_bn2binpad(ret, to, num);
  err:
     if (ctx != NULL) {
         BN_CTX_end(ctx);
@@ -349,7 +345,7 @@ static int RSA_eay_private_encrypt(int flen, const unsigned char *from,
                                    unsigned char *to, RSA *rsa, int padding)
 {
     BIGNUM *f, *ret, *res;
-    int i, j, k, num = 0, r = -1;
+    int i, num = 0, r = -1;
     unsigned char *buf = NULL;
     BN_CTX *ctx = NULL;
     int local_blinding = 0;
@@ -459,15 +455,10 @@ static int RSA_eay_private_encrypt(int flen, const unsigned char *from,
         res = ret;
 
     /*
-     * put in leading 0 bytes if the number is less than the length of the
-     * modulus
+     * BN_bn2binpad puts in leading 0 bytes if the number is less than
+     * the length of the modulus.
      */
-    j = BN_num_bytes(res);
-    i = BN_bn2bin(res, &(to[num - j]));
-    for (k = 0; k < (num - i); k++)
-        to[k] = 0;
-
-    r = num;
+    r = bn_bn2binpad(res, to, num);
  err:
     if (ctx != NULL) {
         BN_CTX_end(ctx);
@@ -485,7 +476,6 @@ static int RSA_eay_private_decrypt(int flen, const unsigned char *from,
 {
     BIGNUM *f, *ret;
     int j, num = 0, r = -1;
-    unsigned char *p;
     unsigned char *buf = NULL;
     BN_CTX *ctx = NULL;
     int local_blinding = 0;
@@ -576,8 +566,7 @@ static int RSA_eay_private_decrypt(int flen, const unsigned char *from,
         if (!rsa_blinding_invert(blinding, ret, unblind, ctx))
             goto err;
 
-    p = buf;
-    j = BN_bn2bin(ret, p);      /* j is only used with no-padding mode */
+    j = bn_bn2binpad(ret, buf, num);
 
     switch (padding) {
     case RSA_PKCS1_PADDING:
@@ -592,7 +581,7 @@ static int RSA_eay_private_decrypt(int flen, const unsigned char *from,
         r = RSA_padding_check_SSLv23(to, num, buf, j, num);
         break;
     case RSA_NO_PADDING:
-        r = RSA_padding_check_none(to, num, buf, j, num);
+        memcpy(to, buf, (r = j));
         break;
     default:
         RSAerr(RSA_F_RSA_EAY_PRIVATE_DECRYPT, RSA_R_UNKNOWN_PADDING_TYPE);
@@ -619,7 +608,6 @@ static int RSA_eay_public_decrypt(int flen, const unsigned char *from,
 {
     BIGNUM *f, *ret;
     int i, num = 0, r = -1;
-    unsigned char *p;
     unsigned char *buf = NULL;
     BN_CTX *ctx = NULL;
 
@@ -684,8 +672,7 @@ static int RSA_eay_public_decrypt(int flen, const unsigned char *from,
         if (!BN_sub(ret, rsa->n, ret))
             goto err;
 
-    p = buf;
-    i = BN_bn2bin(ret, p);
+    i = bn_bn2binpad(ret, buf, num);
 
     switch (padding) {
     case RSA_PKCS1_PADDING:
@@ -695,7 +682,7 @@ static int RSA_eay_public_decrypt(int flen, const unsigned char *from,
         r = RSA_padding_check_X931(to, num, buf, i, num);
         break;
     case RSA_NO_PADDING:
-        r = RSA_padding_check_none(to, num, buf, i, num);
+        memcpy(to, buf, (r = i));
         break;
     default:
         RSAerr(RSA_F_RSA_EAY_PUBLIC_DECRYPT, RSA_R_UNKNOWN_PADDING_TYPE);
diff --git a/crypto/rsa/rsa_oaep.c b/crypto/rsa/rsa_oaep.c
index 9def7a0..3fb8f6b 100644
--- a/crypto/rsa/rsa_oaep.c
+++ b/crypto/rsa/rsa_oaep.c
@@ -153,32 +153,41 @@ int RSA_padding_check_PKCS1_OAEP_mgf1(unsigned char *to, int tlen,
 
     dblen = num - mdlen - 1;
     db = OPENSSL_malloc(dblen);
-    em = OPENSSL_malloc(num);
-    if (db == NULL || em == NULL) {
+    if (db == NULL) {
         RSAerr(RSA_F_RSA_PADDING_CHECK_PKCS1_OAEP_MGF1, ERR_R_MALLOC_FAILURE);
         goto cleanup;
     }
 
-    /*
-     * Always do this zero-padding copy (even when num == flen) to avoid
-     * leaking that information. The copy still leaks some side-channel
-     * information, but it's impossible to have a fixed  memory access
-     * pattern since we can't read out of the bounds of |from|.
-     *
-     * TODO(emilia): Consider porting BN_bn2bin_padded from BoringSSL.
-     */
-    memset(em, 0, num);
-    memcpy(em + num - flen, from, flen);
+    if (flen != num) {
+        em = OPENSSL_malloc(num);
+        if (em == NULL) {
+            RSAerr(RSA_F_RSA_PADDING_CHECK_PKCS1_OAEP_MGF1,
+                   ERR_R_MALLOC_FAILURE);
+            goto cleanup;
+        }
+
+        /*
+         * Caller is encouraged to pass zero-padded message created with
+         * BN_bn2binpad, but if it doesn't, we do this zero-padding copy
+         * to avoid leaking that information. The copy still leaks some
+         * side-channel information, but it's impossible to have a fixed
+         * memory access pattern since we can't read out of the bounds of
+         * |from|.
+         */
+        memset(em, 0, num);
+        memcpy(em + num - flen, from, flen);
+        from = em;
+    }
 
     /*
      * The first byte must be zero, however we must not leak if this is
      * true. See James H. Manger, "A Chosen Ciphertext  Attack on RSA
      * Optimal Asymmetric Encryption Padding (OAEP) [...]", CRYPTO 2001).
      */
-    good = constant_time_is_zero(em[0]);
+    good = constant_time_is_zero(from[0]);
 
-    maskedseed = em + 1;
-    maskeddb = em + 1 + mdlen;
+    maskedseed = from + 1;
+    maskeddb = from + 1 + mdlen;
 
     if (PKCS1_MGF1(seed, mdlen, maskeddb, dblen, mgf1md))
         goto cleanup;
diff --git a/crypto/rsa/rsa_pk1.c b/crypto/rsa/rsa_pk1.c
index 50397c3..5d7882a 100644
--- a/crypto/rsa/rsa_pk1.c
+++ b/crypto/rsa/rsa_pk1.c
@@ -98,6 +98,27 @@ int RSA_padding_check_PKCS1_type_1(unsigned char *to, int tlen,
     const unsigned char *p;
 
     p = from;
+
+    /*
+     * The format is
+     * 00 || 01 || PS || 00 || D
+     * PS - padding string, at least 8 bytes of FF
+     * D  - data.
+     */
+
+    if (num < 11)
+        return -1;
+
+    /* Accept inputs with and without the leading 0-byte. */
+    if (num == flen) {
+        if ((*p++) != 0x00) {
+            RSAerr(RSA_F_RSA_PADDING_CHECK_PKCS1_TYPE_1,
+                   RSA_R_INVALID_PADDING);
+            return -1;
+        }
+        flen--;
+    }
+
     if ((num != (flen + 1)) || (*(p++) != 01)) {
         RSAerr(RSA_F_RSA_PADDING_CHECK_PKCS1_TYPE_1,
                RSA_R_BLOCK_TYPE_IS_NOT_01);
@@ -203,28 +224,31 @@ int RSA_padding_check_PKCS1_type_2(unsigned char *to, int tlen,
     if (num < 11)
         goto err;
 
-    em = OPENSSL_malloc(num);
-    if (em == NULL) {
-        RSAerr(RSA_F_RSA_PADDING_CHECK_PKCS1_TYPE_2, ERR_R_MALLOC_FAILURE);
-        return -1;
+    if (flen != num) {
+        em = OPENSSL_malloc(num);
+        if (em == NULL) {
+            RSAerr(RSA_F_RSA_PADDING_CHECK_PKCS1_TYPE_2, ERR_R_MALLOC_FAILURE);
+            return -1;
+        }
+        /*
+         * Caller is encouraged to pass zero-padded message created with
+         * BN_bn2binpad, but if it doesn't, we do this zero-padding copy
+         * to avoid leaking that information. The copy still leaks some
+         * side-channel information, but it's impossible to have a fixed
+         * memory access pattern since we can't read out of the bounds of
+         * |from|.
+         */
+        memset(em, 0, num);
+        memcpy(em + num - flen, from, flen);
+        from = em;
     }
-    memset(em, 0, num);
-    /*
-     * Always do this zero-padding copy (even when num == flen) to avoid
-     * leaking that information. The copy still leaks some side-channel
-     * information, but it's impossible to have a fixed  memory access
-     * pattern since we can't read out of the bounds of |from|.
-     *
-     * TODO(emilia): Consider porting BN_bn2bin_padded from BoringSSL.
-     */
-    memcpy(em + num - flen, from, flen);
 
-    good = constant_time_is_zero(em[0]);
-    good &= constant_time_eq(em[1], 2);
+    good = constant_time_is_zero(from[0]);
+    good &= constant_time_eq(from[1], 2);
 
     found_zero_byte = 0;
     for (i = 2; i < num; i++) {
-        unsigned int equals0 = constant_time_is_zero(em[i]);
+        unsigned int equals0 = constant_time_is_zero(from[i]);
         zero_index =
             constant_time_select_int(~found_zero_byte & equals0, i,
                                      zero_index);
@@ -232,7 +256,7 @@ int RSA_padding_check_PKCS1_type_2(unsigned char *to, int tlen,
     }
 
     /*
-     * PS must be at least 8 bytes long, and it starts two bytes into |em|.
+     * PS must be at least 8 bytes long, and it starts two bytes into |from|.
      * If we never found a 0-byte, then |zero_index| is 0 and the check
      * also fails.
      */
@@ -261,7 +285,7 @@ int RSA_padding_check_PKCS1_type_2(unsigned char *to, int tlen,
         goto err;
     }
 
-    memcpy(to, em + msg_index, mlen);
+    memcpy(to, from + msg_index, mlen);
 
  err:
     if (em != NULL) {
diff --git a/crypto/rsa/rsa_ssl.c b/crypto/rsa/rsa_ssl.c
index 746e01f..831f75a 100644
--- a/crypto/rsa/rsa_ssl.c
+++ b/crypto/rsa/rsa_ssl.c
@@ -112,6 +112,14 @@ int RSA_padding_check_SSLv23(unsigned char *to, int tlen,
         RSAerr(RSA_F_RSA_PADDING_CHECK_SSLV23, RSA_R_DATA_TOO_SMALL);
         return (-1);
     }
+    /* Accept even zero-padded input */
+    if (flen == num) {
+        if (*(p++) != 0) {
+            RSAerr(RSA_F_RSA_PADDING_CHECK_SSLV23, RSA_R_BLOCK_TYPE_IS_NOT_02);
+            return -1;
+        }
+        flen--;
+    }
     if ((num != (flen + 1)) || (*(p++) != 02)) {
         RSAerr(RSA_F_RSA_PADDING_CHECK_SSLV23, RSA_R_BLOCK_TYPE_IS_NOT_02);
         return (-1);


More information about the openssl-commits mailing list