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

[bernd.edlinger at hotmail.de]

The branch OpenSSL_1_0_2-stable has been updated
       via  c63a5ea848cf0ccd3c991198ddff08b36c312340 (commit)
      from  b83265697cae0e211c55a58078c0650bc78b0908 (commit)


- Log -----------------------------------------------------------------
commit c63a5ea848cf0ccd3c991198ddff08b36c312340
Author: Bernd Edlinger <bernd.edlinger at hotmail.de>
Date:   Fri Jul 14 17:05:37 2017 +0200

    Backport of 5b8fa43 and remove resolved TODO: see PR#3924.
    Make RSA key exchange code actually constant-time.
    
    Reviewed-by: Andy Polyakov <appro at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/3935)

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

Summary of changes:
 crypto/rsa/rsa_pk1.c |  2 --
 ssl/s3_srvr.c        | 36 +++++++++++++++++++++++++++++-------
 2 files changed, 29 insertions(+), 9 deletions(-)

diff --git a/crypto/rsa/rsa_pk1.c b/crypto/rsa/rsa_pk1.c
index efa1fd3..017766c 100644
--- a/crypto/rsa/rsa_pk1.c
+++ b/crypto/rsa/rsa_pk1.c
@@ -255,8 +255,6 @@ int RSA_padding_check_PKCS1_type_2(unsigned char *to, int tlen,
      * We can't continue in constant-time because we need to copy the result
      * and we cannot fake its length. This unavoidably leaks timing
      * information at the API boundary.
-     * TODO(emilia): this could be addressed at the call site,
-     * see BoringSSL commit 0aa0767340baf925bda4804882aab0cb974b2d26.
      */
     if (!good) {
         mlen = -1;
diff --git a/ssl/s3_srvr.c b/ssl/s3_srvr.c
index ba17f1b..0fb4845 100644
--- a/ssl/s3_srvr.c
+++ b/ssl/s3_srvr.c
@@ -2202,7 +2202,7 @@ int ssl3_get_client_key_exchange(SSL *s)
         unsigned char rand_premaster_secret[SSL_MAX_MASTER_KEY_LENGTH];
         int decrypt_len;
         unsigned char decrypt_good, version_good;
-        size_t j;
+        size_t j, padding_len;
 
         /* FIX THIS UP EAY EAY EAY EAY */
         if (s->s3->tmp.use_rsa_tmp) {
@@ -2270,16 +2270,38 @@ int ssl3_get_client_key_exchange(SSL *s)
         if (RAND_bytes(rand_premaster_secret,
                        sizeof(rand_premaster_secret)) <= 0)
             goto err;
+
+        /*
+         * Decrypt with no padding. PKCS#1 padding will be removed as part of
+         * the timing-sensitive code below.
+         */
         decrypt_len =
-            RSA_private_decrypt((int)n, p, p, rsa, RSA_PKCS1_PADDING);
-        ERR_clear_error();
+            RSA_private_decrypt((int)n, p, p, rsa, RSA_NO_PADDING);
+        if (decrypt_len < 0)
+            goto err;
+
+        /* Check the padding. See RFC 3447, section 7.2.2. */
 
         /*
-         * decrypt_len should be SSL_MAX_MASTER_KEY_LENGTH. decrypt_good will
-         * be 0xff if so and zero otherwise.
+         * The smallest padded premaster is 11 bytes of overhead. Small keys
+         * are publicly invalid, so this may return immediately. This ensures
+         * PS is at least 8 bytes.
          */
-        decrypt_good =
-            constant_time_eq_int_8(decrypt_len, SSL_MAX_MASTER_KEY_LENGTH);
+        if (decrypt_len < 11 + SSL_MAX_MASTER_KEY_LENGTH) {
+            al = SSL_AD_DECRYPT_ERROR;
+            SSLerr(SSL_F_SSL3_GET_CLIENT_KEY_EXCHANGE,
+                   SSL_R_DECRYPTION_FAILED);
+            goto f_err;
+        }
+
+        padding_len = decrypt_len - SSL_MAX_MASTER_KEY_LENGTH;
+        decrypt_good = constant_time_eq_int_8(p[0], 0) &
+                       constant_time_eq_int_8(p[1], 2);
+        for (j = 2; j < padding_len - 1; j++) {
+            decrypt_good &= ~constant_time_is_zero_8(p[j]);
+        }
+        decrypt_good &= constant_time_is_zero_8(p[padding_len - 1]);
+        p += padding_len;
 
         /*
          * If the version in the decrypted pre-master secret is correct then