Skip to content

Commit

Permalink
Remove legacy SHA-2 CBC ciphers.
Browse files Browse the repository at this point in the history 
All CBC ciphers in TLS are broken and insecure. TLS 1.2 introduced
AEAD-based ciphers which avoid their many problems. It also introduced
new CBC ciphers based on HMAC-SHA256 and HMAC-SHA384 that share the same
flaws as the original HMAC-SHA1 ones. These serve no purpose. Old
clients don't support them, they have the highest overhead of all TLS
ciphers, and new clients can use AEADs anyway.

Remove them from libssl. This is the smaller, more easily reverted
portion of the removal. If it survives a week or so, we can unwind a lot
more code elsewhere in libcrypto. This removal will allow us to clear
some indirect calls from crypto/cipher_extra/tls_cbc.c, aligning with
the recommendations here:

https://github.com/HACS-workshop/spectre-mitigations/blob/master/crypto_guidelines.md#2-avoid-indirect-branches-in-constant-time-code

Update-Note: The following cipher suites are removed:
- TLS_RSA_WITH_AES_128_CBC_SHA256
- TLS_RSA_WITH_AES_256_CBC_SHA256
- TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256
- TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384
- TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256
- TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384

Change-Id: I7ade0fc1fa2464626560d156659893899aab6f77
Reviewed-on: https://boringssl-review.googlesource.com/27944
Reviewed-by: Adam Langley <agl@google.com>
  • Loading branch information
@davidben @agl
davidben authored and agl committed May 2, 2018
1 parent 71666cb commit 6e678ee
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 146 deletions.
6 changes: 1 addition & 5 deletions include/openssl/ssl.h
View file
Expand Up @@ -1319,11 +1319,7 @@ OPENSSL_EXPORT int SSL_CIPHER_get_bits(const SSL_CIPHER *cipher,
// whose bulk cipher use the corresponding encryption scheme. Note that
// |AES|, |AES128|, and |AES256| match both CBC and GCM ciphers.
//
// |SHA1|, |SHA256|, and |SHA384| match legacy cipher suites using the
// corresponding hash function in their MAC. AEADs are matched by none of
// these.
//
// |SHA| is an alias for |SHA1|.
// |SHA1|, and its alias |SHA|, match legacy cipher suites using HMAC-SHA1.
//
// Although implemented, authentication-only ciphers match no rules and must be
// explicitly selected by name.
Expand Down
4 changes: 1 addition & 3 deletions ssl/internal.h
View file
Expand Up @@ -448,10 +448,8 @@ namespace bssl {

// Bits for |algorithm_mac| (symmetric authentication).
#define SSL_SHA1 0x00000001u
#define SSL_SHA256 0x00000002u
#define SSL_SHA384 0x00000004u
// SSL_AEAD is set for all AEADs.
#define SSL_AEAD 0x00000008u
#define SSL_AEAD 0x00000002u

// Bits for |algorithm_prf| (handshake digest).
#define SSL_HANDSHAKE_MAC_DEFAULT 0x1
Expand Down
110 changes: 0 additions & 110 deletions ssl/ssl_cipher.cc
View file
Expand Up @@ -210,33 +210,6 @@ static const SSL_CIPHER kCiphers[] = {
SSL_HANDSHAKE_MAC_DEFAULT,
},


// TLS v1.2 ciphersuites

// Cipher 3C
{
TLS1_TXT_RSA_WITH_AES_128_SHA256,
"TLS_RSA_WITH_AES_128_CBC_SHA256",
TLS1_CK_RSA_WITH_AES_128_SHA256,
SSL_kRSA,
SSL_aRSA,
SSL_AES128,
SSL_SHA256,
SSL_HANDSHAKE_MAC_SHA256,
},

// Cipher 3D
{
TLS1_TXT_RSA_WITH_AES_256_SHA256,
"TLS_RSA_WITH_AES_256_CBC_SHA256",
TLS1_CK_RSA_WITH_AES_256_SHA256,
SSL_kRSA,
SSL_aRSA,
SSL_AES256,
SSL_SHA256,
SSL_HANDSHAKE_MAC_SHA256,
},

// PSK cipher suites.

// Cipher 8C
Expand Down Expand Up @@ -375,58 +348,6 @@ static const SSL_CIPHER kCiphers[] = {
SSL_HANDSHAKE_MAC_DEFAULT,
},


// HMAC based TLS v1.2 ciphersuites from RFC5289

// Cipher C023
{
TLS1_TXT_ECDHE_ECDSA_WITH_AES_128_SHA256,
"TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256",
TLS1_CK_ECDHE_ECDSA_WITH_AES_128_SHA256,
SSL_kECDHE,
SSL_aECDSA,
SSL_AES128,
SSL_SHA256,
SSL_HANDSHAKE_MAC_SHA256,
},

// Cipher C024
{
TLS1_TXT_ECDHE_ECDSA_WITH_AES_256_SHA384,
"TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384",
TLS1_CK_ECDHE_ECDSA_WITH_AES_256_SHA384,
SSL_kECDHE,
SSL_aECDSA,
SSL_AES256,
SSL_SHA384,
SSL_HANDSHAKE_MAC_SHA384,
},

// Cipher C027
{
TLS1_TXT_ECDHE_RSA_WITH_AES_128_SHA256,
"TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256",
TLS1_CK_ECDHE_RSA_WITH_AES_128_SHA256,
SSL_kECDHE,
SSL_aRSA,
SSL_AES128,
SSL_SHA256,
SSL_HANDSHAKE_MAC_SHA256,
},

// Cipher C028
{
TLS1_TXT_ECDHE_RSA_WITH_AES_256_SHA384,
"TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384",
TLS1_CK_ECDHE_RSA_WITH_AES_256_SHA384,
SSL_kECDHE,
SSL_aRSA,
SSL_AES256,
SSL_SHA384,
SSL_HANDSHAKE_MAC_SHA384,
},


// GCM based TLS v1.2 ciphersuites from RFC5289

// Cipher C02B
Expand Down Expand Up @@ -616,8 +537,6 @@ static const CIPHER_ALIAS kCipherAliases[] = {
// MAC aliases
{"SHA1", ~0u, ~0u, ~0u, SSL_SHA1, 0},
{"SHA", ~0u, ~0u, ~0u, SSL_SHA1, 0},
{"SHA256", ~0u, ~0u, ~0u, SSL_SHA256, 0},
{"SHA384", ~0u, ~0u, ~0u, SSL_SHA384, 0},

// Legacy protocol minimum version aliases. "TLSv1" is intentionally the
// same as "SSLv3".
Expand Down Expand Up @@ -718,23 +637,6 @@ bool ssl_cipher_get_evp_aead(const EVP_AEAD **out_aead,
}

*out_mac_secret_len = SHA_DIGEST_LENGTH;
} else if (cipher->algorithm_mac == SSL_SHA256) {
if (cipher->algorithm_enc == SSL_AES128) {
*out_aead = EVP_aead_aes_128_cbc_sha256_tls();
} else if (cipher->algorithm_enc == SSL_AES256) {
*out_aead = EVP_aead_aes_256_cbc_sha256_tls();
} else {
return false;
}

*out_mac_secret_len = SHA256_DIGEST_LENGTH;
} else if (cipher->algorithm_mac == SSL_SHA384) {
if (cipher->algorithm_enc != SSL_AES256) {
return false;
}

*out_aead = EVP_aead_aes_256_cbc_sha384_tls();
*out_mac_secret_len = SHA384_DIGEST_LENGTH;
} else {
return false;
}
Expand Down Expand Up @@ -1461,10 +1363,6 @@ int SSL_CIPHER_get_digest_nid(const SSL_CIPHER *cipher) {
return NID_undef;
case SSL_SHA1:
return NID_sha1;
case SSL_SHA256:
return NID_sha256;
case SSL_SHA384:
return NID_sha384;
}
assert(0);
return NID_undef;
Expand Down Expand Up @@ -1731,14 +1629,6 @@ const char *SSL_CIPHER_description(const SSL_CIPHER *cipher, char *buf,
mac = "SHA1";
break;

case SSL_SHA256:
mac = "SHA256";
break;

case SSL_SHA384:
mac = "SHA384";
break;

case SSL_AEAD:
mac = "AEAD";
break;
Expand Down
43 changes: 21 additions & 22 deletions ssl/ssl_test.cc
View file
Expand Up @@ -227,7 +227,7 @@ static const CipherTest kCipherTests[] = {
{
// To simplify things, banish all but {ECDHE_RSA,RSA} x
// {CHACHA20,AES_256_CBC,AES_128_CBC} x SHA1.
"!AESGCM:!3DES:!SHA256:!SHA384:"
"!AESGCM:!3DES:"
// Order some ciphers backwards by strength.
"ALL:-CHACHA20:-AES256:-AES128:-ALL:"
// Select ECDHE ones and sort them by strength. Ties should resolve
Expand Down Expand Up @@ -286,15 +286,15 @@ static const CipherTest kCipherTests[] = {
},
// SSLv3 matches everything that existed before TLS 1.2.
{
"AES128-SHA:AES128-SHA256:!SSLv3",
"AES128-SHA:ECDHE-RSA-AES128-GCM-SHA256:!SSLv3",
{
{TLS1_CK_RSA_WITH_AES_128_SHA256, 0},
{TLS1_CK_ECDHE_RSA_WITH_AES_128_GCM_SHA256, 0},
},
false,
},
// TLSv1.2 matches everything added in TLS 1.2.
{
"AES128-SHA:AES128-SHA256:!TLSv1.2",
"AES128-SHA:ECDHE-RSA-AES128-GCM-SHA256:!TLSv1.2",
{
{TLS1_CK_RSA_WITH_AES_128_SHA, 0},
},
Expand All @@ -303,21 +303,21 @@ static const CipherTest kCipherTests[] = {
// The two directives have no intersection. But each component is valid, so
// even in strict mode it is accepted.
{
"AES128-SHA:AES128-SHA256:!TLSv1.2+SSLv3",
"AES128-SHA:ECDHE-RSA-AES128-GCM-SHA256:!TLSv1.2+SSLv3",
{
{TLS1_CK_RSA_WITH_AES_128_SHA, 0},
{TLS1_CK_RSA_WITH_AES_128_SHA256, 0},
{TLS1_CK_ECDHE_RSA_WITH_AES_128_GCM_SHA256, 0},
},
false,
},
// Spaces, semi-colons and commas are separators.
{
"AES128-SHA: AES128-SHA256 AES256-SHA ,AES256-SHA256 ; AES128-GCM-SHA256",
"AES128-SHA: ECDHE-RSA-AES128-GCM-SHA256 AES256-SHA ,ECDHE-ECDSA-AES128-GCM-SHA256 ; AES128-GCM-SHA256",
{
{TLS1_CK_RSA_WITH_AES_128_SHA, 0},
{TLS1_CK_RSA_WITH_AES_128_SHA256, 0},
{TLS1_CK_ECDHE_RSA_WITH_AES_128_GCM_SHA256, 0},
{TLS1_CK_RSA_WITH_AES_256_SHA, 0},
{TLS1_CK_RSA_WITH_AES_256_SHA256, 0},
{TLS1_CK_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, 0},
{TLS1_CK_RSA_WITH_AES_128_GCM_SHA256, 0},
},
// …but not in strict mode.
Expand Down Expand Up @@ -864,22 +864,22 @@ TEST(SSLTest, CipherProperties) {
NID_md5_sha1,
},
{
TLS1_CK_ECDHE_RSA_WITH_AES_128_SHA256,
"TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256",
TLS1_CK_ECDHE_RSA_WITH_AES_128_CBC_SHA,
"TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA",
NID_aes_128_cbc,
NID_sha256,
NID_sha1,
NID_kx_ecdhe,
NID_auth_rsa,
NID_sha256,
NID_md5_sha1,
},
{
TLS1_CK_ECDHE_RSA_WITH_AES_256_SHA384,
"TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384",
TLS1_CK_ECDHE_RSA_WITH_AES_256_CBC_SHA,
"TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA",
NID_aes_256_cbc,
NID_sha384,
NID_sha1,
NID_kx_ecdhe,
NID_auth_rsa,
NID_sha384,
NID_md5_sha1,
},
{
TLS1_CK_ECDHE_RSA_WITH_AES_128_GCM_SHA256,
Expand Down Expand Up @@ -1988,14 +1988,13 @@ TEST(SSLTest, ClientHello) {
0x00, 0x00, 0x23, 0x00, 0x00, 0x00, 0x0b, 0x00, 0x02, 0x01, 0x00, 0x00,
0x0a, 0x00, 0x08, 0x00, 0x06, 0x00, 0x1d, 0x00, 0x17, 0x00, 0x18}},
{TLS1_2_VERSION,
{0x16, 0x03, 0x01, 0x00, 0x8e, 0x01, 0x00, 0x00, 0x8a, 0x03, 0x03, 0x00,
{0x16, 0x03, 0x01, 0x00, 0x82, 0x01, 0x00, 0x00, 0x7e, 0x03, 0x03, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x2a, 0xcc, 0xa9,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x1e, 0xcc, 0xa9,
0xcc, 0xa8, 0xc0, 0x2b, 0xc0, 0x2f, 0xc0, 0x2c, 0xc0, 0x30, 0xc0, 0x09,
0xc0, 0x23, 0xc0, 0x13, 0xc0, 0x27, 0xc0, 0x0a, 0xc0, 0x24, 0xc0, 0x14,
0xc0, 0x28, 0x00, 0x9c, 0x00, 0x9d, 0x00, 0x2f, 0x00, 0x3c, 0x00, 0x35,
0x00, 0x3d, 0x00, 0x0a, 0x01, 0x00, 0x00, 0x37, 0xff, 0x01, 0x00, 0x01,
0xc0, 0x13, 0xc0, 0x0a, 0xc0, 0x14, 0x00, 0x9c, 0x00, 0x9d, 0x00, 0x2f,
0x00, 0x35, 0x00, 0x0a, 0x01, 0x00, 0x00, 0x37, 0xff, 0x01, 0x00, 0x01,
0x00, 0x00, 0x17, 0x00, 0x00, 0x00, 0x23, 0x00, 0x00, 0x00, 0x0d, 0x00,
0x14, 0x00, 0x12, 0x04, 0x03, 0x08, 0x04, 0x04, 0x01, 0x05, 0x03, 0x08,
0x05, 0x05, 0x01, 0x08, 0x06, 0x06, 0x01, 0x02, 0x01, 0x00, 0x0b, 0x00,
Expand Down
6 changes: 0 additions & 6 deletions ssl/test/runner/runner.go
View file
Expand Up @@ -1414,23 +1414,17 @@ var testCipherSuites = []testCipherSuite{
{"3DES-SHA", TLS_RSA_WITH_3DES_EDE_CBC_SHA},
{"AES128-GCM", TLS_RSA_WITH_AES_128_GCM_SHA256},
{"AES128-SHA", TLS_RSA_WITH_AES_128_CBC_SHA},
{"AES128-SHA256", TLS_RSA_WITH_AES_128_CBC_SHA256},
{"AES256-GCM", TLS_RSA_WITH_AES_256_GCM_SHA384},
{"AES256-SHA", TLS_RSA_WITH_AES_256_CBC_SHA},
{"AES256-SHA256", TLS_RSA_WITH_AES_256_CBC_SHA256},
{"ECDHE-ECDSA-AES128-GCM", TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256},
{"ECDHE-ECDSA-AES128-SHA", TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA},
{"ECDHE-ECDSA-AES128-SHA256", TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256},
{"ECDHE-ECDSA-AES256-GCM", TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384},
{"ECDHE-ECDSA-AES256-SHA", TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA},
{"ECDHE-ECDSA-AES256-SHA384", TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384},
{"ECDHE-ECDSA-CHACHA20-POLY1305", TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256},
{"ECDHE-RSA-AES128-GCM", TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256},
{"ECDHE-RSA-AES128-SHA", TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA},
{"ECDHE-RSA-AES128-SHA256", TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256},
{"ECDHE-RSA-AES256-GCM", TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384},
{"ECDHE-RSA-AES256-SHA", TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA},
{"ECDHE-RSA-AES256-SHA384", TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384},
{"ECDHE-RSA-CHACHA20-POLY1305", TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256},
{"PSK-AES128-CBC-SHA", TLS_PSK_WITH_AES_128_CBC_SHA},
{"PSK-AES256-CBC-SHA", TLS_PSK_WITH_AES_256_CBC_SHA},
Expand Down

0 comments on commit 6e678ee

Please sign in to comment.