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);
