From c36ceb0b1550f0c2a8d56f81ea33c0d876020a07 Mon Sep 17 00:00:00 2001
From: Adam Langley <agl@chromium.org>
Date: Wed, 24 Apr 2013 14:45:44 -0400
Subject: [PATCH] This change alters the processing of invalid, RSA pre-master
secrets so that bad encryptions are treated like random session keys in
constant time.
(cherry picked from commit adb46dbc6dd7347750df2468c93e8c34bcb93a4b)
Reviewed-by: Rich Salz <rsalz@openssl.org>
---
ssl/s3_srvr.c | 126 ++++++++++++++++++++++++++++++++++----------------
1 file changed, 85 insertions(+), 41 deletions(-)
diff --git a/ssl/s3_srvr.c b/ssl/s3_srvr.c
index 36d28ec8c..5b0ab811d 100644
--- a/ssl/s3_srvr.c
+++ b/ssl/s3_srvr.c
@@ -2010,6 +2010,10 @@ int ssl3_get_client_key_exchange(SSL *s)
#ifndef OPENSSL_NO_RSA
if (alg_k & SSL_kRSA)
{
+ unsigned char rand_premaster_secret[SSL_MAX_MASTER_KEY_LENGTH];
+ int decrypt_len, decrypt_good_mask;
+ unsigned char version_good;
+
/* FIX THIS UP EAY EAY EAY EAY */
if (s->s3->tmp.use_rsa_tmp)
{
@@ -2057,54 +2061,94 @@ int ssl3_get_client_key_exchange(SSL *s)
n=i;
}
- i=RSA_private_decrypt((int)n,p,p,rsa,RSA_PKCS1_PADDING);
+ /* 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
+ * the TLS RFC and generates a random premaster secret for the
+ * case that the decrypt fails. See
+ * https://tools.ietf.org/html/rfc5246#section-7.4.7.1 */
- al = -1;
-
- if (i != SSL_MAX_MASTER_KEY_LENGTH)
+ /* should be RAND_bytes, but we cannot work around a failure. */
+ if (RAND_pseudo_bytes(rand_premaster_secret,
+ sizeof(rand_premaster_secret)) <= 0)
+ goto err;
+ decrypt_len = RSA_private_decrypt((int)n,p,p,rsa,RSA_PKCS1_PADDING);
+ ERR_clear_error();
+
+ /* decrypt_len should be SSL_MAX_MASTER_KEY_LENGTH.
+ * decrypt_good_mask will be zero if so and non-zero otherwise. */
+ decrypt_good_mask = decrypt_len ^ SSL_MAX_MASTER_KEY_LENGTH;
+
+ /* If the version in the decrypted pre-master secret is correct
+ * then version_good will be zero. The Klima-Pokorny-Rosa
+ * extension of Bleichenbacher's attack
+ * (http://eprint.iacr.org/2003/052/) exploits the version
+ * number check as a "bad version oracle". Thus version checks
+ * are done in constant time and are treated like any other
+ * decryption error. */
+ version_good = p[0] ^ (s->client_version>>8);
+ version_good |= p[1] ^ (s->client_version&0xff);
+
+ /* The premaster secret must contain the same version number as
+ * the ClientHello to detect version rollback attacks
+ * (strangely, the protocol does not offer such protection for
+ * DH ciphersuites). However, buggy clients exist that send the
+ * negotiated protocol version instead if the server does not
+ * support the requested protocol version. If
+ * SSL_OP_TLS_ROLLBACK_BUG is set, tolerate such clients. */
+ if (s->options & SSL_OP_TLS_ROLLBACK_BUG)
{
- al=SSL_AD_DECODE_ERROR;
- /* SSLerr(SSL_F_SSL3_GET_CLIENT_KEY_EXCHANGE,SSL_R_BAD_RSA_DECRYPT); */
+ unsigned char workaround_mask = version_good;
+ unsigned char workaround;
+
+ /* workaround_mask will be 0xff if version_good is
+ * non-zero (i.e. the version match failed). Otherwise
+ * it'll be 0x00. */
+ workaround_mask |= workaround_mask >> 4;
+ workaround_mask |= workaround_mask >> 2;
+ workaround_mask |= workaround_mask >> 1;
+ workaround_mask = ~((workaround_mask & 1) - 1);
+
+ workaround = p[0] ^ (s->version>>8);
+ workaround |= p[1] ^ (s->version&0xff);
+
+ /* If workaround_mask is 0xff (i.e. there was a version
+ * mismatch) then we copy the value of workaround over
+ * version_good. */
+ version_good = (workaround & workaround_mask) |
+ (version_good & ~workaround_mask);
}
- if ((al == -1) && !((p[0] == (s->client_version>>8)) && (p[1] == (s->client_version & 0xff))))
- {
- /* The premaster secret must contain the same version number as the
- * ClientHello to detect version rollback attacks (strangely, the
- * protocol does not offer such protection for DH ciphersuites).
- * However, buggy clients exist that send the negotiated protocol
- * version instead if the server does not support the requested
- * protocol version.
- * If SSL_OP_TLS_ROLLBACK_BUG is set, tolerate such clients. */
- if (!((s->options & SSL_OP_TLS_ROLLBACK_BUG) &&
- (p[0] == (s->version>>8)) && (p[1] == (s->version & 0xff))))
- {
- al=SSL_AD_DECODE_ERROR;
- /* SSLerr(SSL_F_SSL3_GET_CLIENT_KEY_EXCHANGE,SSL_R_BAD_PROTOCOL_VERSION_NUMBER); */
+ /* If any bits in version_good are set then they'll poision
+ * decrypt_good_mask and cause rand_premaster_secret to be
+ * used. */
+ decrypt_good_mask |= version_good;
- /* The Klima-Pokorny-Rosa extension of Bleichenbacher's attack
- * (http://eprint.iacr.org/2003/052/) exploits the version
- * number check as a "bad version oracle" -- an alert would
- * reveal that the plaintext corresponding to some ciphertext
- * made up by the adversary is properly formatted except
- * that the version number is wrong. To avoid such attacks,
- * we should treat this just like any other decryption error. */
- }
+ /* decrypt_good_mask will be zero iff decrypt_len ==
+ * SSL_MAX_MASTER_KEY_LENGTH and the version check passed. We
+ * fold the bottom 32 bits of it with an OR so that the LSB
+ * will be zero iff everything is good. This assumes that we'll
+ * never decrypt a value > 2**31 bytes, which seems safe. */
+ decrypt_good_mask |= decrypt_good_mask >> 16;
+ decrypt_good_mask |= decrypt_good_mask >> 8;
+ decrypt_good_mask |= decrypt_good_mask >> 4;
+ decrypt_good_mask |= decrypt_good_mask >> 2;
+ decrypt_good_mask |= decrypt_good_mask >> 1;
+ /* Now select only the LSB and subtract one. If decrypt_len ==
+ * SSL_MAX_MASTER_KEY_LENGTH and the version check passed then
+ * decrypt_good_mask will be all ones. Otherwise it'll be all
+ * zeros. */
+ decrypt_good_mask &= 1;
+ decrypt_good_mask--;
+
+ /* Now copy rand_premaster_secret over p using
+ * decrypt_good_mask. */
+ for (i = 0; i < (int) sizeof(rand_premaster_secret); i++)
+ {
+ p[i] = (p[i] & decrypt_good_mask) |
+ (rand_premaster_secret[i] & ~decrypt_good_mask);
}
- if (al != -1)
- {
- /* Some decryption failure -- use random value instead as countermeasure
- * against Bleichenbacher's attack on PKCS #1 v1.5 RSA padding
- * (see RFC 2246, section 7.4.7.1). */
- ERR_clear_error();
- i = SSL_MAX_MASTER_KEY_LENGTH;
- p[0] = s->client_version >> 8;
- p[1] = s->client_version & 0xff;
- if (RAND_pseudo_bytes(p+2, i-2) <= 0) /* should be RAND_bytes, but we cannot work around a failure */
- goto err;
- }
-
s->session->master_key_length=
s->method->ssl3_enc->generate_master_secret(s,
s->session->master_key,