Remove unused group_id parameter in TLS 1.3 cipher suite selection
This is a remnant of when we tried to correlate AEAD selection with
post-quantum curves. Also remove a redundant call to
tls1_get_shared_group. We already saved the result in hs->new_session,
so there's no need to compute it again.
Change-Id: I2425ad40bf664f4d248e1dcf610f574a6cad68bc
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/66689
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
diff --git a/ssl/internal.h b/ssl/internal.h
index 8be743b..9f61536 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -714,12 +714,11 @@
size_t ssl_cipher_get_record_split_len(const SSL_CIPHER *cipher);
// ssl_choose_tls13_cipher returns an |SSL_CIPHER| corresponding with the best
-// available from |cipher_suites| compatible with |version|, |group_id|, and
-// |policy|. It returns NULL if there isn't a compatible cipher. |has_aes_hw|
-// indicates if the choice should be made as if support for AES in hardware
-// is available.
+// available from |cipher_suites| compatible with |version| and |policy|. It
+// returns NULL if there isn't a compatible cipher. |has_aes_hw| indicates if
+// the choice should be made as if support for AES in hardware is available.
const SSL_CIPHER *ssl_choose_tls13_cipher(CBS cipher_suites, bool has_aes_hw,
- uint16_t version, uint16_t group_id,
+ uint16_t version,
enum ssl_compliance_policy_t policy);
// ssl_tls13_cipher_meets_policy returns true if |cipher_id| is acceptable given
diff --git a/ssl/s3_both.cc b/ssl/s3_both.cc
index 6d33c6d..172de90 100644
--- a/ssl/s3_both.cc
+++ b/ssl/s3_both.cc
@@ -721,7 +721,7 @@
}
const SSL_CIPHER *ssl_choose_tls13_cipher(CBS cipher_suites, bool has_aes_hw,
- uint16_t version, uint16_t group_id,
+ uint16_t version,
enum ssl_compliance_policy_t policy) {
if (CBS_len(&cipher_suites) % 2 != 0) {
return nullptr;
diff --git a/ssl/tls13_server.cc b/ssl/tls13_server.cc
index 707cf84..588d09d 100644
--- a/ssl/tls13_server.cc
+++ b/ssl/tls13_server.cc
@@ -109,18 +109,18 @@
}
static const SSL_CIPHER *choose_tls13_cipher(
- const SSL *ssl, const SSL_CLIENT_HELLO *client_hello, uint16_t group_id) {
+ const SSL *ssl, const SSL_CLIENT_HELLO *client_hello) {
CBS cipher_suites;
CBS_init(&cipher_suites, client_hello->cipher_suites,
client_hello->cipher_suites_len);
const uint16_t version = ssl_protocol_version(ssl);
- return ssl_choose_tls13_cipher(
- cipher_suites,
- ssl->config->aes_hw_override ? ssl->config->aes_hw_override_value
- : EVP_has_aes_hardware(),
- version, group_id, ssl->config->tls13_cipher_policy);
+ return ssl_choose_tls13_cipher(cipher_suites,
+ ssl->config->aes_hw_override
+ ? ssl->config->aes_hw_override_value
+ : EVP_has_aes_hardware(),
+ version, ssl->config->tls13_cipher_policy);
}
static bool add_new_session_tickets(SSL_HANDSHAKE *hs, bool *out_sent_tickets) {
@@ -225,15 +225,8 @@
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, group_id);
+ hs->new_cipher = choose_tls13_cipher(ssl, &client_hello);
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);
@@ -557,7 +550,6 @@
ScopedCBB cbb;
CBB body, session_id, extensions;
- uint16_t group_id;
if (!ssl->method->init_message(ssl, cbb.get(), &body, SSL3_MT_SERVER_HELLO) ||
!CBB_add_u16(&body, TLS1_2_VERSION) ||
!CBB_add_bytes(&body, kHelloRetryRequest, SSL3_RANDOM_SIZE) ||
@@ -565,14 +557,13 @@
!CBB_add_bytes(&session_id, hs->session_id, hs->session_id_len) ||
!CBB_add_u16(&body, SSL_CIPHER_get_protocol_id(hs->new_cipher)) ||
!CBB_add_u8(&body, 0 /* no compression */) ||
- !tls1_get_shared_group(hs, &group_id) ||
!CBB_add_u16_length_prefixed(&body, &extensions) ||
!CBB_add_u16(&extensions, TLSEXT_TYPE_supported_versions) ||
!CBB_add_u16(&extensions, 2 /* length */) ||
!CBB_add_u16(&extensions, ssl->version) ||
!CBB_add_u16(&extensions, TLSEXT_TYPE_key_share) ||
!CBB_add_u16(&extensions, 2 /* length */) ||
- !CBB_add_u16(&extensions, group_id)) {
+ !CBB_add_u16(&extensions, hs->new_session->group_id)) {
return ssl_hs_error;
}
if (hs->ech_is_inner) {
