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