From 32fbdf2025330664bd2a977d3ef62802e0cf817d Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Tue, 7 Apr 2015 01:14:06 -0400 Subject: [PATCH] Remove anonymous cipher suites. These are the remaining untested cipher suites. Rather than add support in runner.go, just remove them altogether. Grepping for this is a little tricky, but nothing enables aNULL (all occurrences disable it), and all occurrences of ["ALL:] seem to be either unused or explicitly disable anonymous ciphers. Change-Id: I4fd4b8dc6a273d6c04a26e93839641ddf738343f Reviewed-on: https://boringssl-review.googlesource.com/4258 Reviewed-by: Adam Langley --- include/openssl/ssl.h | 10 ++---- ssl/d1_srvr.c | 7 ---- ssl/s3_clnt.c | 9 ----- ssl/s3_lib.c | 81 ------------------------------------------- ssl/s3_srvr.c | 7 ---- ssl/ssl_ciph.c | 39 ++++----------------- ssl/ssl_lib.c | 2 -- ssl/ssl_locl.h | 5 ++- ssl/ssl_test.cc | 5 ++- 9 files changed, 14 insertions(+), 151 deletions(-) diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index 831b5118..6ad8e83f 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h @@ -175,8 +175,6 @@ extern "C" { #define SSL_TXT_HIGH "HIGH" #define SSL_TXT_FIPS "FIPS" -#define SSL_TXT_aNULL "aNULL" - #define SSL_TXT_kRSA "kRSA" #define SSL_TXT_kDHE "kDHE" #define SSL_TXT_kEDH "kEDH" /* same as "kDHE" */ @@ -189,14 +187,12 @@ extern "C" { #define SSL_TXT_aPSK "aPSK" #define SSL_TXT_DH "DH" -#define SSL_TXT_DHE "DHE" /* same as "kDHE:-ADH" */ +#define SSL_TXT_DHE "DHE" /* same as "kDHE" */ #define SSL_TXT_EDH "EDH" /* same as "DHE" */ -#define SSL_TXT_ADH "ADH" #define SSL_TXT_RSA "RSA" #define SSL_TXT_ECDH "ECDH" -#define SSL_TXT_ECDHE "ECDHE" /* same as "kECDHE:-AECDH" */ +#define SSL_TXT_ECDHE "ECDHE" /* same as "kECDHE" */ #define SSL_TXT_EECDH "EECDH" /* same as "ECDHE" */ -#define SSL_TXT_AECDH "AECDH" #define SSL_TXT_ECDSA "ECDSA" #define SSL_TXT_PSK "PSK" @@ -238,7 +234,7 @@ extern "C" { /* The following cipher list is used by default. It also is substituted when an * application-defined cipher list string starts with 'DEFAULT'. */ -#define SSL_DEFAULT_CIPHER_LIST "ALL:!aNULL:!eNULL:!SSLv2" +#define SSL_DEFAULT_CIPHER_LIST "ALL" /* As of OpenSSL 1.0.0, ssl_create_cipher_list() in ssl/ssl_ciph.c always * starts with a reasonable order, and all we have to do for DEFAULT is diff --git a/ssl/d1_srvr.c b/ssl/d1_srvr.c index 01f5cbf6..0f217aaf 100644 --- a/ssl/d1_srvr.c +++ b/ssl/d1_srvr.c @@ -320,13 +320,6 @@ int dtls1_accept(SSL *s) { * don't request cert during re-negotiation: */ ((s->session->peer != NULL) && (s->verify_mode & SSL_VERIFY_CLIENT_ONCE)) || - /* never request cert in anonymous ciphersuites - * (see section "Certificate request" in SSL 3 drafts - * and in RFC 2246): */ - ((s->s3->tmp.new_cipher->algorithm_auth & SSL_aNULL) && - /* ... except when the application insists on verification - * (against the specs, but s3_clnt.c accepts this for SSL 3) */ - !(s->verify_mode & SSL_VERIFY_FAIL_IF_NO_PEER_CERT)) || /* With normal PSK Certificates and * Certificate Requests are omitted */ (s->s3->tmp.new_cipher->algorithm_mkey & SSL_kPSK)) { diff --git a/ssl/s3_clnt.c b/ssl/s3_clnt.c index a6e76c97..7826bf49 100644 --- a/ssl/s3_clnt.c +++ b/ssl/s3_clnt.c @@ -1411,15 +1411,6 @@ int ssl3_get_certificate_request(SSL *s) { goto err; } - /* TLS does not like anon-DH with client cert */ - if (s->version > SSL3_VERSION && - (s->s3->tmp.new_cipher->algorithm_auth & SSL_aNULL)) { - ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_UNEXPECTED_MESSAGE); - OPENSSL_PUT_ERROR(SSL, ssl3_get_certificate_request, - SSL_R_TLS_CLIENT_CERT_REQ_WITH_ANON_CIPHER); - goto err; - } - CBS_init(&cbs, s->init_msg, n); ca_sk = sk_X509_NAME_new(ca_dn_cmp); diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c index fe0e7606..f67267fd 100644 --- a/ssl/s3_lib.c +++ b/ssl/s3_lib.c @@ -185,16 +185,6 @@ const SSL_CIPHER ssl3_ciphers[] = { }, - /* The Ephemeral DH ciphers */ - - /* Cipher 18 */ - { - 1, SSL3_TXT_ADH_RC4_128_MD5, SSL3_CK_ADH_RC4_128_MD5, SSL_kDHE, SSL_aNULL, - SSL_RC4, SSL_MD5, SSL_SSLV3, SSL_MEDIUM, - SSL_HANDSHAKE_MAC_DEFAULT | TLS1_PRF, 128, 128, - }, - - /* New AES ciphersuites */ /* Cipher 2F */ @@ -211,13 +201,6 @@ const SSL_CIPHER ssl3_ciphers[] = { SSL_HANDSHAKE_MAC_DEFAULT | TLS1_PRF, 128, 128, }, - /* Cipher 34 */ - { - 1, TLS1_TXT_ADH_WITH_AES_128_SHA, TLS1_CK_ADH_WITH_AES_128_SHA, SSL_kDHE, - SSL_aNULL, SSL_AES128, SSL_SHA1, SSL_TLSV1, SSL_HIGH | SSL_FIPS, - SSL_HANDSHAKE_MAC_DEFAULT | TLS1_PRF, 128, 128, - }, - /* Cipher 35 */ { 1, TLS1_TXT_RSA_WITH_AES_256_SHA, TLS1_CK_RSA_WITH_AES_256_SHA, SSL_kRSA, @@ -232,13 +215,6 @@ const SSL_CIPHER ssl3_ciphers[] = { SSL_HANDSHAKE_MAC_DEFAULT | TLS1_PRF, 256, 256, }, - /* Cipher 3A */ - { - 1, TLS1_TXT_ADH_WITH_AES_256_SHA, TLS1_CK_ADH_WITH_AES_256_SHA, SSL_kDHE, - SSL_aNULL, SSL_AES256, SSL_SHA1, SSL_TLSV1, SSL_HIGH | SSL_FIPS, - SSL_HANDSHAKE_MAC_DEFAULT | TLS1_PRF, 256, 256, - }, - /* TLS v1.2 ciphersuites */ @@ -272,20 +248,6 @@ const SSL_CIPHER ssl3_ciphers[] = { SSL_HANDSHAKE_MAC_SHA256 | TLS1_PRF_SHA256, 256, 256, }, - /* Cipher 6C */ - { - 1, TLS1_TXT_ADH_WITH_AES_128_SHA256, TLS1_CK_ADH_WITH_AES_128_SHA256, - SSL_kDHE, SSL_aNULL, SSL_AES128, SSL_SHA256, SSL_TLSV1_2, - SSL_HIGH | SSL_FIPS, SSL_HANDSHAKE_MAC_SHA256 | TLS1_PRF_SHA256, 128, 128, - }, - - /* Cipher 6D */ - { - 1, TLS1_TXT_ADH_WITH_AES_256_SHA256, TLS1_CK_ADH_WITH_AES_256_SHA256, - SSL_kDHE, SSL_aNULL, SSL_AES256, SSL_SHA256, SSL_TLSV1_2, - SSL_HIGH | SSL_FIPS, SSL_HANDSHAKE_MAC_SHA256 | TLS1_PRF_SHA256, 256, 256, - }, - /* Cipher 8A */ { 1, TLS1_TXT_PSK_WITH_RC4_128_SHA, TLS1_CK_PSK_WITH_RC4_128_SHA, SSL_kPSK, @@ -350,26 +312,6 @@ const SSL_CIPHER ssl3_ciphers[] = { 256, 256, }, - /* Cipher A6 */ - { - 1, TLS1_TXT_ADH_WITH_AES_128_GCM_SHA256, - TLS1_CK_ADH_WITH_AES_128_GCM_SHA256, SSL_kDHE, SSL_aNULL, SSL_AES128GCM, - SSL_AEAD, SSL_TLSV1_2, SSL_HIGH | SSL_FIPS, - SSL_HANDSHAKE_MAC_SHA256 | TLS1_PRF_SHA256 | SSL_CIPHER_ALGORITHM2_AEAD | - SSL_CIPHER_ALGORITHM2_VARIABLE_NONCE_INCLUDED_IN_RECORD, - 128, 128, - }, - - /* Cipher A7 */ - { - 1, TLS1_TXT_ADH_WITH_AES_256_GCM_SHA384, - TLS1_CK_ADH_WITH_AES_256_GCM_SHA384, SSL_kDHE, SSL_aNULL, SSL_AES256GCM, - SSL_AEAD, SSL_TLSV1_2, SSL_HIGH | SSL_FIPS, - SSL_HANDSHAKE_MAC_SHA384 | TLS1_PRF_SHA384 | SSL_CIPHER_ALGORITHM2_AEAD | - SSL_CIPHER_ALGORITHM2_VARIABLE_NONCE_INCLUDED_IN_RECORD, - 256, 256, - }, - /* Cipher C007 */ { 1, TLS1_TXT_ECDHE_ECDSA_WITH_RC4_128_SHA, @@ -417,29 +359,6 @@ const SSL_CIPHER ssl3_ciphers[] = { SSL_HANDSHAKE_MAC_DEFAULT | TLS1_PRF, 256, 256, }, - /* Cipher C016 */ - { - 1, TLS1_TXT_ECDH_anon_WITH_RC4_128_SHA, TLS1_CK_ECDH_anon_WITH_RC4_128_SHA, - SSL_kECDHE, SSL_aNULL, SSL_RC4, SSL_SHA1, SSL_TLSV1, SSL_MEDIUM, - SSL_HANDSHAKE_MAC_DEFAULT | TLS1_PRF, 128, 128, - }, - - /* Cipher C018 */ - { - 1, TLS1_TXT_ECDH_anon_WITH_AES_128_CBC_SHA, - TLS1_CK_ECDH_anon_WITH_AES_128_CBC_SHA, SSL_kECDHE, SSL_aNULL, SSL_AES128, - SSL_SHA1, SSL_TLSV1, SSL_HIGH | SSL_FIPS, - SSL_HANDSHAKE_MAC_DEFAULT | TLS1_PRF, 128, 128, - }, - - /* Cipher C019 */ - { - 1, TLS1_TXT_ECDH_anon_WITH_AES_256_CBC_SHA, - TLS1_CK_ECDH_anon_WITH_AES_256_CBC_SHA, SSL_kECDHE, SSL_aNULL, SSL_AES256, - SSL_SHA1, SSL_TLSV1, SSL_HIGH | SSL_FIPS, - SSL_HANDSHAKE_MAC_DEFAULT | TLS1_PRF, 256, 256, - }, - /* HMAC based TLS v1.2 ciphersuites from RFC5289 */ diff --git a/ssl/s3_srvr.c b/ssl/s3_srvr.c index 25482a2b..eb458f24 100644 --- a/ssl/s3_srvr.c +++ b/ssl/s3_srvr.c @@ -414,13 +414,6 @@ int ssl3_accept(SSL *s) { * don't request cert during re-negotiation: */ ((s->session->peer != NULL) && (s->verify_mode & SSL_VERIFY_CLIENT_ONCE)) || - /* never request cert in anonymous ciphersuites - * (see section "Certificate request" in SSL 3 drafts - * and in RFC 2246): */ - ((s->s3->tmp.new_cipher->algorithm_auth & SSL_aNULL) && - /* ... except when the application insists on verification - * (against the specs, but s3_clnt.c accepts this for SSL 3) */ - !(s->verify_mode & SSL_VERIFY_FAIL_IF_NO_PEER_CERT)) || /* With normal PSK Certificates and * Certificate Requests are omitted */ (s->s3->tmp.new_cipher->algorithm_mkey & SSL_kPSK)) { diff --git a/ssl/ssl_ciph.c b/ssl/ssl_ciph.c index 5ab43e79..1253f8f3 100644 --- a/ssl/ssl_ciph.c +++ b/ssl/ssl_ciph.c @@ -180,10 +180,7 @@ static const SSL_CIPHER cipher_aliases[] = { {0, SSL_TXT_ALL, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, - /* "COMPLEMENTOFDEFAULT" (does *not* include ciphersuites not found in - ALL!) */ - {0, SSL_TXT_CMPDEF, 0, SSL_kDHE | SSL_kECDHE, SSL_aNULL, 0, 0, 0, 0, 0, 0, - 0}, + /* The "COMPLEMENTOFDEFAULT" rule is omitted. It matches nothing. */ /* key exchange aliases * (some of those using only a single bit here combine @@ -203,19 +200,16 @@ static const SSL_CIPHER cipher_aliases[] = /* server authentication aliases */ {0, SSL_TXT_aRSA, 0, 0, SSL_aRSA, 0, 0, 0, 0, 0, 0, 0}, - {0, SSL_TXT_aNULL, 0, 0, SSL_aNULL, 0, 0, 0, 0, 0, 0, 0}, {0, SSL_TXT_aECDSA, 0, 0, SSL_aECDSA, 0, 0, 0, 0, 0, 0, 0}, {0, SSL_TXT_ECDSA, 0, 0, SSL_aECDSA, 0, 0, 0, 0, 0, 0, 0}, {0, SSL_TXT_aPSK, 0, 0, SSL_aPSK, 0, 0, 0, 0, 0, 0, 0}, /* aliases combining key exchange and server authentication */ - {0, SSL_TXT_DHE, 0, SSL_kDHE, ~SSL_aNULL, 0, 0, 0, 0, 0, 0, 0}, - {0, SSL_TXT_EDH, 0, SSL_kDHE, ~SSL_aNULL, 0, 0, 0, 0, 0, 0, 0}, - {0, SSL_TXT_ECDHE, 0, SSL_kECDHE, ~SSL_aNULL, 0, 0, 0, 0, 0, 0, 0}, - {0, SSL_TXT_EECDH, 0, SSL_kECDHE, ~SSL_aNULL, 0, 0, 0, 0, 0, 0, 0}, + {0, SSL_TXT_DHE, 0, SSL_kDHE, 0, 0, 0, 0, 0, 0, 0, 0}, + {0, SSL_TXT_EDH, 0, SSL_kDHE, 0, 0, 0, 0, 0, 0, 0, 0}, + {0, SSL_TXT_ECDHE, 0, SSL_kECDHE, 0, 0, 0, 0, 0, 0, 0, 0}, + {0, SSL_TXT_EECDH, 0, SSL_kECDHE, 0, 0, 0, 0, 0, 0, 0, 0}, {0, SSL_TXT_RSA, 0, SSL_kRSA, SSL_aRSA, 0, 0, 0, 0, 0, 0, 0}, - {0, SSL_TXT_ADH, 0, SSL_kDHE, SSL_aNULL, 0, 0, 0, 0, 0, 0, 0}, - {0, SSL_TXT_AECDH, 0, SSL_kECDHE, SSL_aNULL, 0, 0, 0, 0, 0, 0, 0}, {0, SSL_TXT_PSK, 0, SSL_kPSK, SSL_aPSK, 0, 0, 0, 0, 0, 0, 0}, /* symmetric encryption aliases */ @@ -1006,13 +1000,6 @@ ssl_create_cipher_list(const SSL_PROTOCOL_METHOD *ssl_method, ssl_cipher_apply_rule(0, ~(SSL_kDHE | SSL_kECDHE), 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.) - * TODO(davidben): Remove them altogether? */ - ssl_cipher_apply_rule(0, 0, SSL_aNULL, 0, 0, 0, 0, CIPHER_ORD, -1, 0, &head, - &tail); - /* Now disable everything (maintaining the ordering!) */ ssl_cipher_apply_rule(0, 0, 0, 0, 0, 0, 0, CIPHER_DEL, -1, 0, &head, &tail); @@ -1186,10 +1173,6 @@ const char *SSL_CIPHER_description(const SSL_CIPHER *cipher, char *buf, au = "RSA"; break; - case SSL_aNULL: - au = "None"; - break; - case SSL_aECDSA: au = "ECDSA"; break; @@ -1332,8 +1315,6 @@ const char *SSL_CIPHER_get_kx_name(const SSL_CIPHER *cipher) { switch (cipher->algorithm_auth) { case SSL_aRSA: return "DHE_RSA"; - case SSL_aNULL: - return "DH_anon"; default: assert(0); return "UNKNOWN"; @@ -1347,8 +1328,6 @@ const char *SSL_CIPHER_get_kx_name(const SSL_CIPHER *cipher) { return "ECDHE_RSA"; case SSL_aPSK: return "ECDHE_PSK"; - case SSL_aNULL: - return "ECDH_anon"; default: assert(0); return "UNKNOWN"; @@ -1479,12 +1458,8 @@ int ssl_cipher_get_cert_index(const SSL_CIPHER *c) { * public key in the key exchange, sent in a server Certificate message. * Otherwise it returns 0. */ int ssl_cipher_has_server_public_key(const SSL_CIPHER *cipher) { - /* Anonymous ciphers do not include a server certificate. */ - if (cipher->algorithm_auth & SSL_aNULL) { - return 0; - } - - /* Neither do PSK ciphers, except for RSA_PSK. */ + /* PSK-authenticated ciphers do not use a public key, except for + * RSA_PSK. */ if ((cipher->algorithm_auth & SSL_aPSK) && !(cipher->algorithm_mkey & SSL_kRSA)) { return 0; diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c index a9d1528d..1578dba6 100644 --- a/ssl/ssl_lib.c +++ b/ssl/ssl_lib.c @@ -2030,8 +2030,6 @@ void ssl_get_compatible_server_ciphers(SSL *s, unsigned long *out_mask_k, mask_a |= SSL_aRSA; } - mask_a |= SSL_aNULL; - /* An ECC certificate may be usable for ECDSA cipher suites depending on the * key usage extension and on the client's curve preferences. */ if (have_ecc_cert) { diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h index 6278debb..16fe2c64 100644 --- a/ssl/ssl_locl.h +++ b/ssl/ssl_locl.h @@ -293,9 +293,8 @@ /* Bits for algorithm_auth (server authentication) */ #define SSL_aRSA 0x00000001L /* RSA auth */ -#define SSL_aNULL 0x00000002L /* no auth (i.e. use ADH or AECDH) */ -#define SSL_aECDSA 0x00000004L /* ECDSA auth*/ -#define SSL_aPSK 0x00000008L /* PSK auth */ +#define SSL_aECDSA 0x00000002L /* ECDSA auth*/ +#define SSL_aPSK 0x00000004L /* PSK auth */ /* Bits for algorithm_enc (symmetric encryption) */ #define SSL_3DES 0x00000001L diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc index 22018bb3..aba758e2 100644 --- a/ssl/ssl_test.cc +++ b/ssl/ssl_test.cc @@ -185,6 +185,8 @@ static const char *kBadRules[] = { // Empty cipher lists error at SSL_CTX_set_cipher_list. "", "BOGUS", + // COMPLEMENTOFDEFAULT is empty. + "COMPLEMENTOFDEFAULT", // Invalid command. "?BAR", // Special operators are not allowed if groups are used. @@ -428,12 +430,9 @@ static const CIPHER_RFC_NAME_TEST kCipherRFCNameTests[] = { { SSL3_CK_RSA_DES_192_CBC3_SHA, "TLS_RSA_WITH_3DES_EDE_CBC_SHA" }, { SSL3_CK_RSA_RC4_128_MD5, "TLS_RSA_WITH_RC4_MD5" }, { TLS1_CK_RSA_WITH_AES_128_SHA, "TLS_RSA_WITH_AES_128_CBC_SHA" }, - { TLS1_CK_ADH_WITH_AES_128_SHA, "TLS_DH_anon_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, "TLS_DHE_RSA_WITH_AES_256_CBC_SHA256" }, - { TLS1_CK_ECDH_anon_WITH_AES_128_CBC_SHA, - "TLS_ECDH_anon_WITH_AES_128_CBC_SHA" }, { TLS1_CK_ECDHE_RSA_WITH_AES_128_SHA256, "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256" }, { TLS1_CK_ECDHE_RSA_WITH_AES_256_SHA384,