From 9c8c418853236bee9d582e27896942c3979078a4 Mon Sep 17 00:00:00 2001 From: Matt Braithwaite Date: Wed, 24 Aug 2016 14:36:54 -0700 Subject: [PATCH] Remove RC4 ciphersuites from TLS. For now, they can be restored by compiling with -DBORINGSSL_RC4_TLS. Of note, this means that `MEDIUM' is now empty. Change-Id: Ic77308e7bd4849bdb2b4882c6b34af85089fe3cc Reviewed-on: https://boringssl-review.googlesource.com/10580 Reviewed-by: David Benjamin Reviewed-by: Matt Braithwaite Commit-Queue: David Benjamin Commit-Queue: Matt Braithwaite CQ-Verified: CQ bot account: commit-bot@chromium.org --- ssl/ssl_cipher.c | 10 +++ ssl/ssl_test.cc | 149 ++++++++++++++++++++++++++++++-------- ssl/test/runner/runner.go | 23 ++++-- 3 files changed, 145 insertions(+), 37 deletions(-) diff --git a/ssl/ssl_cipher.c b/ssl/ssl_cipher.c index 52369a4c..e58d8893 100644 --- a/ssl/ssl_cipher.c +++ b/ssl/ssl_cipher.c @@ -168,6 +168,7 @@ static const SSL_CIPHER kCiphers[] = { SSL_HANDSHAKE_MAC_DEFAULT, }, +#ifdef BORINGSSL_ENABLE_RC4_TLS /* Cipher 04 */ { SSL3_TXT_RSA_RC4_128_MD5, @@ -189,6 +190,7 @@ static const SSL_CIPHER kCiphers[] = { SSL_SHA1, SSL_HANDSHAKE_MAC_DEFAULT, }, +#endif /* Cipher 0A */ { @@ -297,6 +299,7 @@ static const SSL_CIPHER kCiphers[] = { /* PSK cipher suites. */ +#ifdef BORINGSSL_ENABLE_RC4_TLS /* Cipher 8A */ { TLS1_TXT_PSK_WITH_RC4_128_SHA, @@ -307,6 +310,7 @@ static const SSL_CIPHER kCiphers[] = { SSL_SHA1, SSL_HANDSHAKE_MAC_DEFAULT, }, +#endif /* Cipher 8C */ { @@ -422,6 +426,7 @@ static const SSL_CIPHER kCiphers[] = { SSL_HANDSHAKE_MAC_SHA384, }, +#ifdef BORINGSSL_ENABLE_RC4_TLS /* Cipher C007 */ { TLS1_TXT_ECDHE_ECDSA_WITH_RC4_128_SHA, @@ -432,6 +437,7 @@ static const SSL_CIPHER kCiphers[] = { SSL_SHA1, SSL_HANDSHAKE_MAC_DEFAULT, }, +#endif /* Cipher C009 */ { @@ -455,6 +461,7 @@ static const SSL_CIPHER kCiphers[] = { SSL_HANDSHAKE_MAC_DEFAULT, }, +#ifdef BORINGSSL_ENABLE_RC4_TLS /* Cipher C011 */ { TLS1_TXT_ECDHE_RSA_WITH_RC4_128_SHA, @@ -465,6 +472,7 @@ static const SSL_CIPHER kCiphers[] = { SSL_SHA1, SSL_HANDSHAKE_MAC_DEFAULT, }, +#endif /* Cipher C013 */ { @@ -845,6 +853,7 @@ int ssl_cipher_get_evp_aead(const EVP_AEAD **out_aead, *out_fixed_iv_len = 12; break; +#ifdef BORINGSSL_ENABLE_RC4_TLS case SSL_RC4: switch (cipher->algorithm_mac) { case SSL_MD5: @@ -867,6 +876,7 @@ int ssl_cipher_get_evp_aead(const EVP_AEAD **out_aead, return 0; } break; +#endif case SSL_AES128: switch (cipher->algorithm_mac) { diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc index 5884d930..5c68f26a 100644 --- a/ssl/ssl_test.cc +++ b/ssl/ssl_test.cc @@ -169,9 +169,13 @@ static const CipherTest kCipherTests[] = { {TLS1_CK_ECDHE_RSA_WITH_AES_256_CBC_SHA, 0}, {TLS1_CK_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256, 0}, {TLS1_CK_ECDHE_RSA_CHACHA20_POLY1305_OLD, 0}, +#ifdef BORINGSSL_ENABLE_RC4_TLS {TLS1_CK_ECDHE_RSA_WITH_RC4_128_SHA, 0}, +#endif {TLS1_CK_ECDHE_RSA_WITH_AES_128_CBC_SHA, 0}, +#ifdef BORINGSSL_ENABLE_RC4_TLS {SSL3_CK_RSA_RC4_128_SHA, 0}, +#endif {TLS1_CK_RSA_WITH_AES_128_SHA, 0}, {TLS1_CK_RSA_WITH_AES_256_SHA, 0}, }, @@ -255,7 +259,9 @@ static const char *kMustNotIncludeNull[] = { "DEFAULT", "ALL:!eNULL", "ALL:!NULL", +#ifdef BORINGSSL_ENABLE_RC4_TLS "MEDIUM", +#endif "HIGH", "FIPS", "SHA", @@ -269,7 +275,9 @@ static const char *kMustNotIncludeNull[] = { static const char *kMustNotIncludeCECPQ1[] = { "ALL", "DEFAULT", +#ifdef BORINGSSL_ENABLE_RC4_TLS "MEDIUM", +#endif "HIGH", "FIPS", "SHA", @@ -735,7 +743,9 @@ typedef struct { static const CIPHER_RFC_NAME_TEST kCipherRFCNameTests[] = { { SSL3_CK_RSA_DES_192_CBC3_SHA, "TLS_RSA_WITH_3DES_EDE_CBC_SHA" }, +#ifdef BORINGSSL_ENABLE_RC4_TLS { SSL3_CK_RSA_RC4_128_MD5, "TLS_RSA_WITH_RC4_MD5" }, +#endif { TLS1_CK_RSA_WITH_AES_128_SHA, "TLS_RSA_WITH_AES_128_CBC_SHA" }, { TLS1_CK_DHE_RSA_WITH_AES_256_SHA, "TLS_DHE_RSA_WITH_AES_256_CBC_SHA" }, { TLS1_CK_DHE_RSA_WITH_AES_256_SHA256, @@ -750,7 +760,9 @@ static const CIPHER_RFC_NAME_TEST kCipherRFCNameTests[] = { "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256" }, { TLS1_CK_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384, "TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384" }, +#ifdef BORINGSSL_ENABLE_RC4_TLS { TLS1_CK_PSK_WITH_RC4_128_SHA, "TLS_PSK_WITH_RC4_SHA" }, +#endif { TLS1_CK_ECDHE_PSK_WITH_AES_128_CBC_SHA, "TLS_ECDHE_PSK_WITH_AES_128_CBC_SHA" }, { TLS1_CK_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256, @@ -1642,7 +1654,7 @@ static bool ClientHelloMatches(uint16_t version, const uint8_t *expected, SSL_CTX_set_max_version(ctx.get(), version); // Our default cipher list varies by CPU capabilities, so manually place the // ChaCha20 ciphers in front. - if (!SSL_CTX_set_cipher_list(ctx.get(), "CHACHA20:ALL")) { + if (!SSL_CTX_set_cipher_list(ctx.get(), "!RC4:CHACHA20:ALL")) { return false; } ScopedSSL ssl(SSL_new(ctx.get())); @@ -1685,13 +1697,28 @@ static bool ClientHelloMatches(uint16_t version, const uint8_t *expected, // Tests that our ClientHellos do not change unexpectedly. static bool TestClientHello() { static const uint8_t kSSL3ClientHello[] = { - 0x16, 0x03, 0x00, 0x00, 0x47, 0x01, 0x00, 0x00, 0x43, 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, - 0x00, 0x1c, 0xc0, 0x09, 0xc0, 0x13, 0x00, 0x33, 0xc0, 0x0a, 0xc0, - 0x14, 0x00, 0x39, 0xc0, 0x07, 0xc0, 0x11, 0x00, 0x2f, 0x00, 0x35, - 0x00, 0x0a, 0x00, 0x05, 0x00, 0x04, 0x00, 0xff, 0x01, 0x00, + 0x16, + 0x03, 0x00, + 0x00, 0x3f, + 0x01, + 0x00, 0x00, 0x3b, + 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, + 0x00, 0x14, + 0xc0, 0x09, + 0xc0, 0x13, + 0x00, 0x33, + 0xc0, 0x0a, + 0xc0, 0x14, + 0x00, 0x39, + 0x00, 0x2f, + 0x00, 0x35, + 0x00, 0x0a, + 0x00, 0xff, 0x01, 0x00, }; if (!ClientHelloMatches(SSL3_VERSION, kSSL3ClientHello, sizeof(kSSL3ClientHello))) { @@ -1699,12 +1726,27 @@ static bool TestClientHello() { } static const uint8_t kTLS1ClientHello[] = { - 0x16, 0x03, 0x01, 0x00, 0x66, 0x01, 0x00, 0x00, 0x62, 0x03, 0x01, 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, 0x1a, 0xc0, 0x09, - 0xc0, 0x13, 0x00, 0x33, 0xc0, 0x0a, 0xc0, 0x14, 0x00, 0x39, 0xc0, 0x07, - 0xc0, 0x11, 0x00, 0x2f, 0x00, 0x35, 0x00, 0x0a, 0x00, 0x05, 0x00, 0x04, + 0x16, + 0x03, 0x01, + 0x00, 0x5e, + 0x01, + 0x00, 0x00, 0x5a, + 0x03, 0x01, + 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, 0x12, + 0xc0, 0x09, + 0xc0, 0x13, + 0x00, 0x33, + 0xc0, 0x0a, + 0xc0, 0x14, + 0x00, 0x39, + 0x00, 0x2f, + 0x00, 0x35, + 0x00, 0x0a, 0x01, 0x00, 0x00, 0x1f, 0xff, 0x01, 0x00, 0x01, 0x00, 0x00, 0x17, 0x00, 0x00, 0x00, 0x23, 0x00, 0x00, 0x00, 0x0b, 0x00, 0x02, 0x01, 0x00, 0x00, 0x0a, 0x00, 0x08, 0x00, 0x06, 0x00, 0x1d, 0x00, 0x17, 0x00, 0x18, @@ -1715,12 +1757,27 @@ static bool TestClientHello() { } static const uint8_t kTLS11ClientHello[] = { - 0x16, 0x03, 0x01, 0x00, 0x66, 0x01, 0x00, 0x00, 0x62, 0x03, 0x02, 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, 0x1a, 0xc0, 0x09, - 0xc0, 0x13, 0x00, 0x33, 0xc0, 0x0a, 0xc0, 0x14, 0x00, 0x39, 0xc0, 0x07, - 0xc0, 0x11, 0x00, 0x2f, 0x00, 0x35, 0x00, 0x0a, 0x00, 0x05, 0x00, 0x04, + 0x16, + 0x03, 0x01, + 0x00, 0x5e, + 0x01, + 0x00, 0x00, 0x5a, + 0x03, 0x02, + 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, 0x12, + 0xc0, 0x09, + 0xc0, 0x13, + 0x00, 0x33, + 0xc0, 0x0a, + 0xc0, 0x14, + 0x00, 0x39, + 0x00, 0x2f, + 0x00, 0x35, + 0x00, 0x0a, 0x01, 0x00, 0x00, 0x1f, 0xff, 0x01, 0x00, 0x01, 0x00, 0x00, 0x17, 0x00, 0x00, 0x00, 0x23, 0x00, 0x00, 0x00, 0x0b, 0x00, 0x02, 0x01, 0x00, 0x00, 0x0a, 0x00, 0x08, 0x00, 0x06, 0x00, 0x1d, 0x00, 0x17, 0x00, 0x18, @@ -1731,16 +1788,48 @@ static bool TestClientHello() { } static const uint8_t kTLS12ClientHello[] = { - 0x16, 0x03, 0x01, 0x00, 0xa4, 0x01, 0x00, 0x00, 0xa0, 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, 0x42, 0xcc, 0xa9, - 0xcc, 0xa8, 0xcc, 0x14, 0xcc, 0x13, 0xc0, 0x2b, 0xc0, 0x2f, 0x00, 0x9e, - 0xc0, 0x2c, 0xc0, 0x30, 0x00, 0x9f, 0xc0, 0x09, 0xc0, 0x23, 0xc0, 0x13, - 0xc0, 0x27, 0x00, 0x33, 0x00, 0x67, 0xc0, 0x0a, 0xc0, 0x24, 0xc0, 0x14, - 0xc0, 0x28, 0x00, 0x39, 0x00, 0x6b, 0xc0, 0x07, 0xc0, 0x11, 0x00, 0x9c, - 0x00, 0x9d, 0x00, 0x2f, 0x00, 0x3c, 0x00, 0x35, 0x00, 0x3d, 0x00, 0x0a, - 0x00, 0x05, 0x00, 0x04, 0x01, 0x00, 0x00, 0x35, 0xff, 0x01, 0x00, 0x01, + 0x16, + 0x03, 0x01, + 0x00, 0x9c, + 0x01, + 0x00, 0x00, 0x98, + 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, 0x3a, + 0xcc, 0xa9, + 0xcc, 0xa8, + 0xcc, 0x14, + 0xcc, 0x13, + 0xc0, 0x2b, + 0xc0, 0x2f, + 0x00, 0x9e, + 0xc0, 0x2c, + 0xc0, 0x30, + 0x00, 0x9f, + 0xc0, 0x09, + 0xc0, 0x23, + 0xc0, 0x13, + 0xc0, 0x27, + 0x00, 0x33, + 0x00, 0x67, + 0xc0, 0x0a, + 0xc0, 0x24, + 0xc0, 0x14, + 0xc0, 0x28, + 0x00, 0x39, + 0x00, 0x6b, + 0x00, 0x9c, + 0x00, 0x9d, + 0x00, 0x2f, + 0x00, 0x3c, + 0x00, 0x35, + 0x00, 0x3d, + 0x00, 0x0a, + 0x01, 0x00, 0x00, 0x35, 0xff, 0x01, 0x00, 0x01, 0x00, 0x00, 0x17, 0x00, 0x00, 0x00, 0x23, 0x00, 0x00, 0x00, 0x0d, 0x00, 0x12, 0x00, 0x10, 0x06, 0x01, 0x06, 0x03, 0x05, 0x01, 0x05, 0x03, 0x04, 0x01, 0x04, 0x03, 0x02, 0x01, 0x02, 0x03, 0x00, 0x0b, 0x00, 0x02, 0x01, diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go index 54e2661f..e2fe8856 100644 --- a/ssl/test/runner/runner.go +++ b/ssl/test/runner/runner.go @@ -62,6 +62,7 @@ var ( looseErrors = flag.Bool("loose-errors", false, "If true, allow shims to report an untranslated error code.") shimConfigFile = flag.String("shim-config", "", "A config file to use to configure the tests for this shim.") includeDisabled = flag.Bool("include-disabled", false, "If true, also runs disabled tests.") + includeRC4 = flag.Bool("include-rc4", false, "If true, test RC4 ciphersuites.") ) // ShimConfigurations is used with the “json” package and represents a shim @@ -1035,7 +1036,6 @@ var testCipherSuites = []struct { {"ECDHE-ECDSA-AES256-SHA384", TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384}, {"ECDHE-ECDSA-CHACHA20-POLY1305", TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256}, {"ECDHE-ECDSA-CHACHA20-POLY1305-OLD", TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256_OLD}, - {"ECDHE-ECDSA-RC4-SHA", TLS_ECDHE_ECDSA_WITH_RC4_128_SHA}, {"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}, @@ -1044,7 +1044,6 @@ var testCipherSuites = []struct { {"ECDHE-RSA-AES256-SHA384", TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384}, {"ECDHE-RSA-CHACHA20-POLY1305", TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256}, {"ECDHE-RSA-CHACHA20-POLY1305-OLD", TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256_OLD}, - {"ECDHE-RSA-RC4-SHA", TLS_ECDHE_RSA_WITH_RC4_128_SHA}, {"CECPQ1-RSA-CHACHA20-POLY1305-SHA256", TLS_CECPQ1_RSA_WITH_CHACHA20_POLY1305_SHA256}, {"CECPQ1-ECDSA-CHACHA20-POLY1305-SHA256", TLS_CECPQ1_ECDSA_WITH_CHACHA20_POLY1305_SHA256}, {"CECPQ1-RSA-AES256-GCM-SHA384", TLS_CECPQ1_RSA_WITH_AES_256_GCM_SHA384}, @@ -1056,9 +1055,6 @@ var testCipherSuites = []struct { {"ECDHE-PSK-CHACHA20-POLY1305", TLS_ECDHE_PSK_WITH_CHACHA20_POLY1305_SHA256}, {"ECDHE-PSK-AES128-GCM-SHA256", TLS_ECDHE_PSK_WITH_AES_128_GCM_SHA256}, {"ECDHE-PSK-AES256-GCM-SHA384", TLS_ECDHE_PSK_WITH_AES_256_GCM_SHA384}, - {"PSK-RC4-SHA", TLS_PSK_WITH_RC4_128_SHA}, - {"RC4-MD5", TLS_RSA_WITH_RC4_128_MD5}, - {"RC4-SHA", TLS_RSA_WITH_RC4_128_SHA}, {"NULL-SHA", TLS_RSA_WITH_NULL_SHA}, } @@ -2258,6 +2254,19 @@ func addBasicTests() { func addCipherSuiteTests() { const bogusCipher = 0xfe00 + if *includeRC4 { + testCipherSuites = append(testCipherSuites, []struct { + name string + id uint16 + }{ + {"ECDHE-ECDSA-RC4-SHA", TLS_ECDHE_ECDSA_WITH_RC4_128_SHA}, + {"ECDHE-RSA-RC4-SHA", TLS_ECDHE_RSA_WITH_RC4_128_SHA}, + {"PSK-RC4-SHA", TLS_PSK_WITH_RC4_128_SHA}, + {"RC4-MD5", TLS_RSA_WITH_RC4_128_MD5}, + {"RC4-SHA", TLS_RSA_WITH_RC4_128_SHA}, + }...) + } + for _, suite := range testCipherSuites { const psk = "12345" const pskIdentity = "luggage combo" @@ -2424,12 +2433,12 @@ func addCipherSuiteTests() { name: "UnsupportedCipherSuite", config: Config{ MaxVersion: VersionTLS12, - CipherSuites: []uint16{TLS_RSA_WITH_RC4_128_SHA}, + CipherSuites: []uint16{TLS_RSA_WITH_AES_128_CBC_SHA}, Bugs: ProtocolBugs{ IgnorePeerCipherPreferences: true, }, }, - flags: []string{"-cipher", "DEFAULT:!RC4"}, + flags: []string{"-cipher", "DEFAULT:!AES"}, shouldFail: true, expectedError: ":WRONG_CIPHER_RETURNED:", })