mirror/openssl
2
0
Fork 0
mirror of https://github.com/openssl/openssl.git synced 2025-02-17 14:32:04 +08:00
Code Issues Packages Projects Releases Wiki Activity

Make RSA key exchange code actually constant-time.

Browse Source 
Using RSA_PKCS1_PADDING with RSA_private_decrypt is inherently unsafe.
The API requires writing output on success and touching the error queue
on error. Thus, although the padding check itself is constant-time as of
294d1e36c2, and the logic after the
decryption in the SSL code is constant-time as of
adb46dbc6d, the API boundary in the middle
still leaks whether the padding check succeeded, giving us our
much-loved Bleichenbacher padding oracle.

Instead, PKCS#1 padding must be handled by the caller which uses
RSA_NO_PADDING, in timing-sensitive code integrated with the
Bleichenbacher mitigation. Removing PKCS#1 padding in constant time is
actually much simpler when the expected length is a constant (and if
it's not a constant, avoiding a padding oracle seems unlikely), so just
do it inline.

Signed-off-by: Kurt Roeckx <kurt@roeckx.be>
Reviewed-by: Rich Salz <rsalz@openssl.org>

GH: #1222
This commit is contained in:
David Benjamin 2016-06-16 14:15:19 -04:00 committed by Kurt Roeckx
parent 01238aec40
commit 5b8fa431ae
1 changed files with 36 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

51
ssl/statem/statem_srvr.c
View File

@ -2087,7 +2087,7 @@ MSG_PROCESS_RETURN tls_process_client_key_exchange(SSL *s, PACKET *pkt)
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 */
rsa = EVP_PKEY_get0_RSA(s->cert->pkeys[SSL_PKEY_RSA_ENC].privatekey);
@ -2144,17 +2144,37 @@ MSG_PROCESS_RETURN tls_process_client_key_exchange(SSL *s, PACKET *pkt)
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(PACKET_remaining(&enc_premaster),
PACKET_data(&enc_premaster),
rsa_decrypt, rsa, RSA_PKCS1_PADDING);
ERR_clear_error();
rsa_decrypt, 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_TLS_PROCESS_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(rsa_decrypt[0], 0) &
constant_time_eq_int_8(rsa_decrypt[1], 2);
for (j = 2; j < padding_len - 1; j++) {
decrypt_good &= ~constant_time_is_zero_8(rsa_decrypt[j]);
}
decrypt_good &= constant_time_is_zero_8(rsa_decrypt[padding_len - 1]);
/*
* If the version in the decrypted pre-master secret is correct then
@ -2165,10 +2185,10 @@ MSG_PROCESS_RETURN tls_process_client_key_exchange(SSL *s, PACKET *pkt)
* constant time and are treated like any other decryption error.
*/
version_good =
constant_time_eq_8(rsa_decrypt[0],
constant_time_eq_8(rsa_decrypt[padding_len],
(unsigned)(s->client_version >> 8));
version_good &=
constant_time_eq_8(rsa_decrypt[1],
constant_time_eq_8(rsa_decrypt[padding_len + 1],
(unsigned)(s->client_version & 0xff));
/*
@ -2182,10 +2202,10 @@ MSG_PROCESS_RETURN tls_process_client_key_exchange(SSL *s, PACKET *pkt)
*/
if (s->options & SSL_OP_TLS_ROLLBACK_BUG) {
unsigned char workaround_good;
workaround_good =
constant_time_eq_8(rsa_decrypt[0], (unsigned)(s->version >> 8));
workaround_good = constant_time_eq_8(rsa_decrypt[padding_len],
(unsigned)(s->version >> 8));
workaround_good &=
constant_time_eq_8(rsa_decrypt[1],
constant_time_eq_8(rsa_decrypt[padding_len + 1],
(unsigned)(s->version & 0xff));
version_good |= workaround_good;
}
@ -2203,12 +2223,13 @@ MSG_PROCESS_RETURN tls_process_client_key_exchange(SSL *s, PACKET *pkt)
* it is still sufficiently large to read from.
*/
for (j = 0; j < sizeof(rand_premaster_secret); j++) {
rsa_decrypt[j] =
constant_time_select_8(decrypt_good, rsa_decrypt[j],
rsa_decrypt[padding_len + j] =
constant_time_select_8(decrypt_good,
rsa_decrypt[padding_len + j],
rand_premaster_secret[j]);
}
if (!ssl_generate_master_secret(s, rsa_decrypt,
if (!ssl_generate_master_secret(s, rsa_decrypt + padding_len,
sizeof(rand_premaster_secret), 0)) {
al = SSL_AD_INTERNAL_ERROR;
SSLerr(SSL_F_TLS_PROCESS_CLIENT_KEY_EXCHANGE, ERR_R_INTERNAL_ERROR);