Do not allow AES_128_GCM_SHA256 with CECPQ2. Just forbid it altogether, so we don't need to worry about a mess of equipreferences. Change-Id: I4921ff326c6047e50c075d4311dd42219bf8318e Reviewed-on: https://boringssl-review.googlesource.com/c/33585 Commit-Queue: Adam Langley <agl@google.com> Reviewed-by: Adam Langley <agl@google.com>
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go index cbce065..477eae8 100644 --- a/ssl/test/runner/runner.go +++ b/ssl/test/runner/runner.go
@@ -8862,7 +8862,7 @@ // Not all ciphers involve a signature. Advertise a list which gives all // versions a signing cipher. signingCiphers := []uint16{ - TLS_AES_128_GCM_SHA256, + TLS_AES_256_GCM_SHA384, TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA, @@ -10732,7 +10732,7 @@ CipherSuites: []uint16{ TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA, - TLS_AES_128_GCM_SHA256, + TLS_AES_256_GCM_SHA384, }, CurvePreferences: []CurveID{curve.id}, }, @@ -10751,7 +10751,7 @@ CipherSuites: []uint16{ TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA, - TLS_AES_128_GCM_SHA256, + TLS_AES_256_GCM_SHA384, }, CurvePreferences: []CurveID{curve.id}, }, @@ -10771,7 +10771,7 @@ CipherSuites: []uint16{ TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA, - TLS_AES_128_GCM_SHA256, + TLS_AES_256_GCM_SHA384, }, CurvePreferences: []CurveID{curve.id}, Bugs: ProtocolBugs{ @@ -10791,7 +10791,7 @@ CipherSuites: []uint16{ TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA, - TLS_AES_128_GCM_SHA256, + TLS_AES_256_GCM_SHA384, }, CurvePreferences: []CurveID{curve.id}, Bugs: ProtocolBugs{ @@ -13976,6 +13976,60 @@ "-expect-cipher-no-aes", strconv.Itoa(int(TLS_CHACHA20_POLY1305_SHA256)), }, }) + + // Test that CECPQ2 cannot be used with TLS_AES_128_GCM_SHA256. + testCases = append(testCases, testCase{ + testType: serverTest, + name: "TLS13-CipherPreference-CECPQ2-AES128Only", + config: Config{ + MaxVersion: VersionTLS13, + CipherSuites: []uint16{ + TLS_AES_128_GCM_SHA256, + }, + }, + flags: []string{ + "-curves", strconv.Itoa(int(CurveCECPQ2)), + }, + shouldFail: true, + expectedError: ":NO_SHARED_CIPHER:", + expectedLocalError: "remote error: handshake failure", + }) + + // Test that CECPQ2 continues to honor AES vs ChaCha20 logic. + testCases = append(testCases, testCase{ + testType: serverTest, + name: "TLS13-CipherPreference-CECPQ2-AES128-ChaCha20-AES256", + config: Config{ + MaxVersion: VersionTLS13, + CipherSuites: []uint16{ + TLS_AES_128_GCM_SHA256, + TLS_CHACHA20_POLY1305_SHA256, + TLS_AES_256_GCM_SHA384, + }, + }, + flags: []string{ + "-curves", strconv.Itoa(int(CurveCECPQ2)), + "-expect-cipher-aes", strconv.Itoa(int(TLS_CHACHA20_POLY1305_SHA256)), + "-expect-cipher-no-aes", strconv.Itoa(int(TLS_CHACHA20_POLY1305_SHA256)), + }, + }) + testCases = append(testCases, testCase{ + testType: serverTest, + name: "TLS13-CipherPreference-CECPQ2-AES128-AES256-ChaCha20", + config: Config{ + MaxVersion: VersionTLS13, + CipherSuites: []uint16{ + TLS_AES_128_GCM_SHA256, + TLS_AES_256_GCM_SHA384, + TLS_CHACHA20_POLY1305_SHA256, + }, + }, + flags: []string{ + "-curves", strconv.Itoa(int(CurveCECPQ2)), + "-expect-cipher-aes", strconv.Itoa(int(TLS_AES_256_GCM_SHA384)), + "-expect-cipher-no-aes", strconv.Itoa(int(TLS_CHACHA20_POLY1305_SHA256)), + }, + }) } func addPeekTests() {
diff --git a/ssl/tls13_server.cc b/ssl/tls13_server.cc index b4c4ca5..7073b57 100644 --- a/ssl/tls13_server.cc +++ b/ssl/tls13_server.cc
@@ -96,33 +96,39 @@ } static const SSL_CIPHER *choose_tls13_cipher( - const SSL *ssl, const SSL_CLIENT_HELLO *client_hello) { + const SSL *ssl, const SSL_CLIENT_HELLO *client_hello, uint16_t group_id) { if (client_hello->cipher_suites_len % 2 != 0) { - return NULL; + return nullptr; } CBS cipher_suites; CBS_init(&cipher_suites, client_hello->cipher_suites, client_hello->cipher_suites_len); - const int aes_is_fine = EVP_has_aes_hardware(); + const bool aes_is_fine = EVP_has_aes_hardware(); + const bool require_256_bit = group_id == SSL_CURVE_CECPQ2; const uint16_t version = ssl_protocol_version(ssl); - const SSL_CIPHER *best = NULL; + const SSL_CIPHER *best = nullptr; while (CBS_len(&cipher_suites) > 0) { uint16_t cipher_suite; if (!CBS_get_u16(&cipher_suites, &cipher_suite)) { - return NULL; + return nullptr; } // Limit to TLS 1.3 ciphers we know about. const SSL_CIPHER *candidate = SSL_get_cipher_by_value(cipher_suite); - if (candidate == NULL || + if (candidate == nullptr || SSL_CIPHER_get_min_version(candidate) > version || SSL_CIPHER_get_max_version(candidate) < version) { continue; } + // Post-quantum key exchanges should be paired with 256-bit ciphers. + if (require_256_bit && candidate->algorithm_enc == SSL_AES128GCM) { + continue; + } + // TLS 1.3 removes legacy ciphers, so honor the client order, but prefer // ChaCha20 if we do not have AES hardware. if (aes_is_fine) { @@ -133,7 +139,7 @@ return candidate; } - if (best == NULL) { + if (best == nullptr) { best = candidate; } } @@ -240,8 +246,15 @@ client_hello.session_id_len); hs->session_id_len = client_hello.session_id_len; + uint16_t group_id; + if (!tls1_get_shared_group(hs, &group_id)) { + OPENSSL_PUT_ERROR(SSL, SSL_R_NO_SHARED_GROUP); + ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_HANDSHAKE_FAILURE); + return ssl_hs_error; + } + // Negotiate the cipher suite. - hs->new_cipher = choose_tls13_cipher(ssl, &client_hello); + hs->new_cipher = choose_tls13_cipher(ssl, &client_hello, group_id); if (hs->new_cipher == NULL) { OPENSSL_PUT_ERROR(SSL, SSL_R_NO_SHARED_CIPHER); ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_HANDSHAKE_FAILURE);