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>

3 files changed, 13 insertions, 23 deletions

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 @@ bool ssl_cipher_requires_server_key_exchange(const SSL_CIPHER *cipher);
 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 @@ bool ssl_tls13_cipher_meets_policy(uint16_t cipher_id,
 }
 
 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 int ssl_ext_supported_versions_add_serverhello(SSL_HANDSHAKE *hs,
 }
 
 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 @@ static enum ssl_hs_wait_t do_select_parameters(SSL_HANDSHAKE *hs) {
                  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 @@ static enum ssl_hs_wait_t do_send_hello_retry_request(SSL_HANDSHAKE *hs) {
 
   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 @@ static enum ssl_hs_wait_t do_send_hello_retry_request(SSL_HANDSHAKE *hs) {
       !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) {