From c63a5ea848cf0ccd3c991198ddff08b36c312340 Mon Sep 17 00:00:00 2001 From: Bernd Edlinger Date: Fri, 14 Jul 2017 17:05:37 +0200 Subject: [PATCH] Backport of 5b8fa43 and remove resolved TODO: see PR#3924. Make RSA key exchange code actually constant-time. Reviewed-by: Andy Polyakov (Merged from https://github.com/openssl/openssl/pull/3935) --- 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 efa1fd3e99..017766ce71 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 ba17f1b562..0fb4845d44 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 -- 2.25.1