diff options
author | Adam Langley <agl@chromium.org> | 2014-12-16 14:03:47 +0100 |
---|---|---|
committer | Emilia Kasper <emilia@openssl.org> | 2014-12-17 14:01:19 +0100 |
commit | 4aecfd4d9f366c849c9627ab666d1b1addc024e6 (patch) | |
tree | 63ac22feece3cb58678472c0c922dd3cc0bd8336 /ssl/s3_srvr.c | |
parent | 57dc72e018ddecf222b6c5e598793b75069d42a2 (diff) |
Premaster secret handling fixes
From BoringSSL
- Send an alert when the client key exchange isn't correctly formatted.
- Reject overly short RSA ciphertexts to avoid a (benign) out-of-bounds memory access.
Reviewed-by: Kurt Roeckx <kurt@openssl.org>
Diffstat (limited to 'ssl/s3_srvr.c')
-rw-r--r-- | ssl/s3_srvr.c | 36 |
1 files changed, 28 insertions, 8 deletions
diff --git a/ssl/s3_srvr.c b/ssl/s3_srvr.c index 7a61864bb6..02c8c10aa1 100644 --- a/ssl/s3_srvr.c +++ b/ssl/s3_srvr.c @@ -2276,6 +2276,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; /* FIX THIS UP EAY EAY EAY EAY */ if (s->s3->tmp.use_rsa_tmp) @@ -2314,8 +2315,9 @@ int ssl3_get_client_key_exchange(SSL *s) { if (!(s->options & SSL_OP_TLS_D5_BUG)) { + al = SSL_AD_DECODE_ERROR; SSLerr(SSL_F_SSL3_GET_CLIENT_KEY_EXCHANGE,SSL_R_TLS_RSA_ENCRYPTED_VALUE_LENGTH_IS_WRONG); - goto err; + goto f_err; } else p-=2; @@ -2324,6 +2326,20 @@ int ssl3_get_client_key_exchange(SSL *s) n=i; } + /* + * Reject overly short RSA ciphertext because we want to be sure + * that the buffer size makes it safe to iterate over the entire + * size of a premaster secret (SSL_MAX_MASTER_KEY_LENGTH). The + * actual expected size is larger due to RSA padding, but the + * bound is sufficient to be safe. + */ + if (n < SSL_MAX_MASTER_KEY_LENGTH) + { + al = SSL_AD_DECRYPT_ERROR; + SSLerr(SSL_F_SSL3_GET_CLIENT_KEY_EXCHANGE, SSL_R_TLS_RSA_ENCRYPTED_VALUE_LENGTH_IS_WRONG); + goto f_err; + } + /* We must not leak whether a decryption failure occurs because * of Bleichenbacher's attack on PKCS #1 v1.5 RSA padding (see * RFC 2246, section 7.4.7.1). The code follows that advice of @@ -2371,19 +2387,23 @@ int ssl3_get_client_key_exchange(SSL *s) * to remain non-zero (0xff). */ decrypt_good &= version_good; - /* Now copy rand_premaster_secret over p using - * decrypt_good_mask. */ - for (i = 0; i < (int) sizeof(rand_premaster_secret); i++) + /* + * Now copy rand_premaster_secret over from p using + * decrypt_good_mask. If decryption failed, then p does not + * contain valid plaintext, however, a check above guarantees + * it is still sufficiently large to read from. + */ + for (j = 0; j < sizeof(rand_premaster_secret); j++) { - p[i] = constant_time_select_8(decrypt_good, p[i], - rand_premaster_secret[i]); + p[j] = constant_time_select_8(decrypt_good, p[j], + rand_premaster_secret[j]); } s->session->master_key_length= s->method->ssl3_enc->generate_master_secret(s, s->session->master_key, - p,i); - OPENSSL_cleanse(p,i); + p,sizeof(rand_premaster_secret)); + OPENSSL_cleanse(p,sizeof(rand_premaster_secret)); } else #endif |