From ede973a89a5bf8784855d2bea9c8ae4a5e3e32ee Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Tue, 19 Aug 2014 14:12:39 -0400 Subject: [PATCH] Tidy up cipher ordering. To align with what Chrome sends on NSS, remove all 3DES cipher suites except RSA_WITH_3DES_EDE_CBC_SHA. This avoids having to order a PFS 3DES cipher against a non-PFS 3DES cipher. Remove the strength sort which wanted place AES_256_CBC ahead of AES_128_GCM and is not especially useful (everything under 128 is either 3DES or DES). Instead, explicitly order all the bulk ciphers. Continue to prefer PFS over non-PFS and ECDHE over DHE. This gives the following order in Chromium. We can probably prune it a bit (DHE_DSS, DH_*) in a follow-up. TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256 (0xcc14) Forward Secrecy 256 TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256 (0xcc13) Forward Secrecy 256 TLS_DHE_RSA_WITH_CHACHA20_POLY1305_SHA256 (0xcc15) Forward Secrecy 256 TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (0xc02f) Forward Secrecy 128 TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256 (0xc02b) Forward Secrecy 128 TLS_DHE_DSS_WITH_AES_128_GCM_SHA256 (0xa2) Forward Secrecy* 128 TLS_DHE_RSA_WITH_AES_128_GCM_SHA256 (0x9e) Forward Secrecy 128 TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA (0xc014) Forward Secrecy 256 TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA (0xc00a) Forward Secrecy 256 TLS_DHE_RSA_WITH_AES_256_CBC_SHA (0x39) Forward Secrecy 256 TLS_DHE_DSS_WITH_AES_256_CBC_SHA (0x38) Forward Secrecy* 256 TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA (0xc013) Forward Secrecy 128 TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA (0xc009) Forward Secrecy 128 TLS_DHE_RSA_WITH_AES_128_CBC_SHA (0x33) Forward Secrecy 128 TLS_DHE_DSS_WITH_AES_128_CBC_SHA (0x32) Forward Secrecy* 128 TLS_ECDHE_RSA_WITH_RC4_128_SHA (0xc011) Forward Secrecy 128 TLS_ECDHE_ECDSA_WITH_RC4_128_SHA (0xc007) Forward Secrecy 128 TLS_DH_DSS_WITH_AES_128_GCM_SHA256 (0xa4) 128 TLS_DH_RSA_WITH_AES_128_GCM_SHA256 (0xa0) 128 TLS_RSA_WITH_AES_128_GCM_SHA256 (0x9c) 128 TLS_DH_RSA_WITH_AES_256_CBC_SHA (0x37) 256 TLS_DH_DSS_WITH_AES_256_CBC_SHA (0x36) 256 TLS_RSA_WITH_AES_256_CBC_SHA (0x35) 256 TLS_DH_RSA_WITH_AES_128_CBC_SHA (0x31) 128 TLS_DH_DSS_WITH_AES_128_CBC_SHA (0x30) 128 TLS_RSA_WITH_AES_128_CBC_SHA (0x2f) 128 TLS_RSA_WITH_RC4_128_SHA (0x5) 128 TLS_RSA_WITH_RC4_128_MD5 (0x4) 128 TLS_RSA_WITH_3DES_EDE_CBC_SHA (0xa) 112 BUG=405091 Change-Id: Ib8dd28469414a4eb496788a57a215e7e21f8c37f Reviewed-on: https://boringssl-review.googlesource.com/1559 Reviewed-by: Adam Langley --- ssl/s3_lib.c | 176 -------------------------------------- ssl/ssl_ciph.c | 45 +++++----- ssl/test/runner/runner.go | 2 - 3 files changed, 21 insertions(+), 202 deletions(-) diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c index 9a29bf8ad..1cf01508c 100644 --- a/ssl/s3_lib.c +++ b/ssl/s3_lib.c @@ -250,22 +250,6 @@ const SSL_CIPHER ssl3_ciphers[]={ 56, }, -/* Cipher 0D */ - { - 1, - SSL3_TXT_DH_DSS_DES_192_CBC3_SHA, - SSL3_CK_DH_DSS_DES_192_CBC3_SHA, - SSL_kDHd, - SSL_aDH, - SSL_3DES, - SSL_SHA1, - SSL_SSLV3, - SSL_NOT_EXP|SSL_HIGH|SSL_FIPS, - SSL_HANDSHAKE_MAC_DEFAULT|TLS1_PRF, - 112, - 168, - }, - /* Cipher 0F */ { 1, @@ -282,22 +266,6 @@ const SSL_CIPHER ssl3_ciphers[]={ 56, }, -/* Cipher 10 */ - { - 1, - SSL3_TXT_DH_RSA_DES_192_CBC3_SHA, - SSL3_CK_DH_RSA_DES_192_CBC3_SHA, - SSL_kDHr, - SSL_aDH, - SSL_3DES, - SSL_SHA1, - SSL_SSLV3, - SSL_NOT_EXP|SSL_HIGH|SSL_FIPS, - SSL_HANDSHAKE_MAC_DEFAULT|TLS1_PRF, - 112, - 168, - }, - /* The Ephemeral DH ciphers */ /* Cipher 12 */ @@ -316,22 +284,6 @@ const SSL_CIPHER ssl3_ciphers[]={ 56, }, -/* Cipher 13 */ - { - 1, - SSL3_TXT_EDH_DSS_DES_192_CBC3_SHA, - SSL3_CK_EDH_DSS_DES_192_CBC3_SHA, - SSL_kEDH, - SSL_aDSS, - SSL_3DES, - SSL_SHA1, - SSL_SSLV3, - SSL_NOT_EXP|SSL_HIGH|SSL_FIPS, - SSL_HANDSHAKE_MAC_DEFAULT|TLS1_PRF, - 112, - 168, - }, - /* Cipher 15 */ { 1, @@ -348,22 +300,6 @@ const SSL_CIPHER ssl3_ciphers[]={ 56, }, -/* Cipher 16 */ - { - 1, - SSL3_TXT_EDH_RSA_DES_192_CBC3_SHA, - SSL3_CK_EDH_RSA_DES_192_CBC3_SHA, - SSL_kEDH, - SSL_aRSA, - SSL_3DES, - SSL_SHA1, - SSL_SSLV3, - SSL_NOT_EXP|SSL_HIGH|SSL_FIPS, - SSL_HANDSHAKE_MAC_DEFAULT|TLS1_PRF, - 112, - 168, - }, - /* Cipher 18 */ { 1, @@ -396,22 +332,6 @@ const SSL_CIPHER ssl3_ciphers[]={ 56, }, -/* Cipher 1B */ - { - 1, - SSL3_TXT_ADH_DES_192_CBC_SHA, - SSL3_CK_ADH_DES_192_CBC_SHA, - SSL_kEDH, - SSL_aNULL, - SSL_3DES, - SSL_SHA1, - SSL_SSLV3, - SSL_NOT_EXP|SSL_HIGH|SSL_FIPS, - SSL_HANDSHAKE_MAC_DEFAULT|TLS1_PRF, - 112, - 168, - }, - /* New AES ciphersuites */ /* Cipher 2F */ { @@ -829,22 +749,6 @@ const SSL_CIPHER ssl3_ciphers[]={ 128, }, - /* Cipher 8B */ - { - 1, - TLS1_TXT_PSK_WITH_3DES_EDE_CBC_SHA, - TLS1_CK_PSK_WITH_3DES_EDE_CBC_SHA, - SSL_kPSK, - SSL_aPSK, - SSL_3DES, - SSL_SHA1, - SSL_TLSV1, - SSL_NOT_EXP|SSL_HIGH|SSL_FIPS, - SSL_HANDSHAKE_MAC_DEFAULT|TLS1_PRF, - 112, - 168, - }, - /* Cipher 8C */ { 1, @@ -1094,22 +998,6 @@ const SSL_CIPHER ssl3_ciphers[]={ 128, }, - /* Cipher C003 */ - { - 1, - TLS1_TXT_ECDH_ECDSA_WITH_DES_192_CBC3_SHA, - TLS1_CK_ECDH_ECDSA_WITH_DES_192_CBC3_SHA, - SSL_kECDHe, - SSL_aECDH, - SSL_3DES, - SSL_SHA1, - SSL_TLSV1, - SSL_NOT_EXP|SSL_HIGH|SSL_FIPS, - SSL_HANDSHAKE_MAC_DEFAULT|TLS1_PRF, - 112, - 168, - }, - /* Cipher C004 */ { 1, @@ -1158,22 +1046,6 @@ const SSL_CIPHER ssl3_ciphers[]={ 128, }, - /* Cipher C008 */ - { - 1, - TLS1_TXT_ECDHE_ECDSA_WITH_DES_192_CBC3_SHA, - TLS1_CK_ECDHE_ECDSA_WITH_DES_192_CBC3_SHA, - SSL_kEECDH, - SSL_aECDSA, - SSL_3DES, - SSL_SHA1, - SSL_TLSV1, - SSL_NOT_EXP|SSL_HIGH|SSL_FIPS, - SSL_HANDSHAKE_MAC_DEFAULT|TLS1_PRF, - 112, - 168, - }, - /* Cipher C009 */ { 1, @@ -1222,22 +1094,6 @@ const SSL_CIPHER ssl3_ciphers[]={ 128, }, - /* Cipher C00D */ - { - 1, - TLS1_TXT_ECDH_RSA_WITH_DES_192_CBC3_SHA, - TLS1_CK_ECDH_RSA_WITH_DES_192_CBC3_SHA, - SSL_kECDHr, - SSL_aECDH, - SSL_3DES, - SSL_SHA1, - SSL_TLSV1, - SSL_NOT_EXP|SSL_HIGH|SSL_FIPS, - SSL_HANDSHAKE_MAC_DEFAULT|TLS1_PRF, - 112, - 168, - }, - /* Cipher C00E */ { 1, @@ -1286,22 +1142,6 @@ const SSL_CIPHER ssl3_ciphers[]={ 128, }, - /* Cipher C012 */ - { - 1, - TLS1_TXT_ECDHE_RSA_WITH_DES_192_CBC3_SHA, - TLS1_CK_ECDHE_RSA_WITH_DES_192_CBC3_SHA, - SSL_kEECDH, - SSL_aRSA, - SSL_3DES, - SSL_SHA1, - SSL_TLSV1, - SSL_NOT_EXP|SSL_HIGH|SSL_FIPS, - SSL_HANDSHAKE_MAC_DEFAULT|TLS1_PRF, - 112, - 168, - }, - /* Cipher C013 */ { 1, @@ -1350,22 +1190,6 @@ const SSL_CIPHER ssl3_ciphers[]={ 128, }, - /* Cipher C017 */ - { - 1, - TLS1_TXT_ECDH_anon_WITH_DES_192_CBC3_SHA, - TLS1_CK_ECDH_anon_WITH_DES_192_CBC3_SHA, - SSL_kEECDH, - SSL_aNULL, - SSL_3DES, - SSL_SHA1, - SSL_TLSV1, - SSL_NOT_EXP|SSL_HIGH|SSL_FIPS, - SSL_HANDSHAKE_MAC_DEFAULT|TLS1_PRF, - 112, - 168, - }, - /* Cipher C018 */ { 1, diff --git a/ssl/ssl_ciph.c b/ssl/ssl_ciph.c index 237155363..10aa13eef 100644 --- a/ssl/ssl_ciph.c +++ b/ssl/ssl_ciph.c @@ -1244,47 +1244,44 @@ STACK_OF(SSL_CIPHER) *ssl_create_cipher_list(const SSL_METHOD *ssl_method, co_list, &head, &tail); - /* Now arrange all ciphers by preference: */ + /* Now arrange all ciphers by preference: + * TODO(davidben): Compute this order once and copy it. */ /* Everything else being equal, prefer ephemeral ECDH over other key exchange mechanisms */ ssl_cipher_apply_rule(0, SSL_kEECDH, 0, 0, 0, 0, 0, CIPHER_ADD, -1, 0, &head, &tail); ssl_cipher_apply_rule(0, SSL_kEECDH, 0, 0, 0, 0, 0, CIPHER_DEL, -1, 0, &head, &tail); - /* AES is our preferred symmetric cipher */ - ssl_cipher_apply_rule(0, 0, 0, SSL_AES, 0, 0, 0, CIPHER_ADD, -1, 0, &head, &tail); + /* Order the bulk ciphers. + * 1. CHACHA20_POLY1305. + * 2. AES_256_GCM and AES_128_GCM. + * 3. Legacy non-AEAD ciphers. AES_256_CBC, AES-128_CBC, RC4_128_SHA, + * RC4_128_MD5, 3DES_EDE_CBC_SHA. + * TODO(davidben): Prefer AES_GCM over CHACHA20 if there is hardware + * support. */ + ssl_cipher_apply_rule(0, 0, 0, SSL_CHACHA20POLY1305, 0, 0, 0, CIPHER_ADD, -1, 0, &head, &tail); + ssl_cipher_apply_rule(0, 0, 0, SSL_AES256GCM, 0, 0, 0, CIPHER_ADD, -1, 0, &head, &tail); + ssl_cipher_apply_rule(0, 0, 0, SSL_AES128GCM, 0, 0, 0, CIPHER_ADD, -1, 0, &head, &tail); + ssl_cipher_apply_rule(0, 0, 0, SSL_AES256, 0, 0, 0, CIPHER_ADD, -1, 0, &head, &tail); + ssl_cipher_apply_rule(0, 0, 0, SSL_AES128, 0, 0, 0, CIPHER_ADD, -1, 0, &head, &tail); + ssl_cipher_apply_rule(0, 0, 0, SSL_RC4, ~SSL_MD5, 0, 0, CIPHER_ADD, -1, 0, &head, &tail); + ssl_cipher_apply_rule(0, 0, 0, SSL_RC4, SSL_MD5, 0, 0, CIPHER_ADD, -1, 0, &head, &tail); + ssl_cipher_apply_rule(0, 0, 0, SSL_3DES, 0, 0, 0, CIPHER_ADD, -1, 0, &head, &tail); /* Temporarily enable everything else for sorting */ ssl_cipher_apply_rule(0, 0, 0, 0, 0, 0, 0, CIPHER_ADD, -1, 0, &head, &tail); - /* Low priority for MD5 */ - ssl_cipher_apply_rule(0, 0, 0, 0, SSL_MD5, 0, 0, CIPHER_ORD, -1, 0, &head, &tail); + /* Move ciphers without forward secrecy to the end. */ + ssl_cipher_apply_rule(0, ~(SSL_kEDH|SSL_kEECDH), 0, 0, 0, 0, 0, CIPHER_ORD, -1, 0, &head, &tail); /* Move anonymous ciphers to the end. Usually, these will remain disabled. * (For applications that allow them, they aren't too bad, but we prefer - * authenticated ciphers.) */ + * authenticated ciphers.) + * TODO(davidben): Remove them altogether? */ ssl_cipher_apply_rule(0, 0, SSL_aNULL, 0, 0, 0, 0, CIPHER_ORD, -1, 0, &head, &tail); - /* Move ciphers without forward secrecy to the end */ - ssl_cipher_apply_rule(0, 0, SSL_aECDH, 0, 0, 0, 0, CIPHER_ORD, -1, 0, &head, &tail); - /* ssl_cipher_apply_rule(0, 0, SSL_aDH, 0, 0, 0, 0, CIPHER_ORD, -1, 0, &head, &tail); */ - ssl_cipher_apply_rule(0, SSL_kRSA, 0, 0, 0, 0, 0, CIPHER_ORD, -1, 0, &head, &tail); - ssl_cipher_apply_rule(0, SSL_kPSK, 0,0, 0, 0, 0, CIPHER_ORD, -1, 0, &head, &tail); - - /* RC4 is sort-of broken -- move the the end */ - ssl_cipher_apply_rule(0, 0, 0, SSL_RC4, 0, 0, 0, CIPHER_ORD, -1, 0, &head, &tail); - - /* Now sort by symmetric encryption strength. The above ordering remains - * in force within each class */ - if (!ssl_cipher_strength_sort(&head, &tail)) - { - OPENSSL_free(co_list); - return NULL; - } - /* Now disable everything (maintaining the ordering!) */ ssl_cipher_apply_rule(0, 0, 0, 0, 0, 0, 0, CIPHER_DEL, -1, 0, &head, &tail); - /* * We also need cipher aliases for selecting based on the rule_str. * There might be two types of entries in the rule_str: 1) names diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go index b39bcedf9..4a5b762af 100644 --- a/ssl/test/runner/runner.go +++ b/ssl/test/runner/runner.go @@ -699,7 +699,6 @@ var testCipherSuites = []struct { {"AES128-SHA", TLS_RSA_WITH_AES_128_CBC_SHA}, {"AES256-GCM", TLS_RSA_WITH_AES_256_GCM_SHA384}, {"AES256-SHA", TLS_RSA_WITH_AES_256_CBC_SHA}, - {"DHE-RSA-3DES-SHA", TLS_DHE_RSA_WITH_3DES_EDE_CBC_SHA}, {"DHE-RSA-AES128-GCM", TLS_DHE_RSA_WITH_AES_128_GCM_SHA256}, {"DHE-RSA-AES128-SHA", TLS_DHE_RSA_WITH_AES_128_CBC_SHA}, {"DHE-RSA-AES256-GCM", TLS_DHE_RSA_WITH_AES_256_GCM_SHA384}, @@ -708,7 +707,6 @@ var testCipherSuites = []struct { {"ECDHE-ECDSA-AES128-SHA", TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA}, {"ECDHE-ECDSA-AES256-SHA", TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA}, {"ECDHE-ECDSA-RC4-SHA", TLS_ECDHE_ECDSA_WITH_RC4_128_SHA}, - {"ECDHE-RSA-3DES-SHA", TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_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-AES256-GCM", TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384},