Use Span and Array for the curve list.

There seems to be a GCC bug that requires kDefaultGroups having an
explicit cast, but this is still much nicer than void(const uint16_t **,
size_t *) functions.

Bug: 132
Change-Id: Id586d402ca0b8a01370353ff17295e71ee219ff3
Reviewed-on: https://boringssl-review.googlesource.com/20668
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>

ssl/handshake.cc

@@ -148,7 +148,6 @@ SSL_HANDSHAKE::~SSL_HANDSHAKE() {
   OPENSSL_free(key_share_bytes);
   OPENSSL_free(ecdh_public_key);
   OPENSSL_free(peer_sigalgs);
-  OPENSSL_free(peer_supported_group_list);
   OPENSSL_free(server_params);
   ssl->ctx->x509_method->hs_flush_cached_ca_names(this);
   OPENSSL_free(certificate_types);

ssl/internal.h

@@ -1307,8 +1307,7 @@ struct SSL_HANDSHAKE {
   // peer_supported_group_list contains the supported group IDs advertised by
   // the peer. This is only set on the server's end. The server does not
   // advertise this extension to the client.
-  uint16_t *peer_supported_group_list = nullptr;
-  size_t peer_supported_group_list_len = 0;
+  Array<uint16_t> peer_supported_group_list;
 
   // peer_key is the peer's ECDH key for a TLS 1.2 client.
   Array<uint8_t> peer_key;
@@ -2368,14 +2367,12 @@ int tls1_change_cipher_state(SSL_HANDSHAKE *hs, evp_aead_direction_t direction);
 int tls1_generate_master_secret(SSL_HANDSHAKE *hs, uint8_t *out,
                                 const uint8_t *premaster, size_t premaster_len);
 
-// tls1_get_grouplist sets |*out_group_ids| and |*out_group_ids_len| to the
-// locally-configured group preference list.
-void tls1_get_grouplist(SSL *ssl, const uint16_t **out_group_ids,
-                        size_t *out_group_ids_len);
+// tls1_get_grouplist returns the locally-configured group preference list.
+Span<const uint16_t> tls1_get_grouplist(const SSL *ssl);
 
 // tls1_check_group_id returns one if |group_id| is consistent with
 // locally-configured group preferences.
-int tls1_check_group_id(SSL *ssl, uint16_t group_id);
+int tls1_check_group_id(const SSL *ssl, uint16_t group_id);
 
 // tls1_get_shared_group sets |*out_group_id| to the first preferred shared
 // group between client and server preferences and returns one. If none may be

ssl/t1_lib.cc

@@ -299,24 +299,18 @@ static const uint16_t kDefaultGroups[] = {
     SSL_CURVE_SECP384R1,
 };
 
-void tls1_get_grouplist(SSL *ssl, const uint16_t **out_group_ids,
-                        size_t *out_group_ids_len) {
-  *out_group_ids = ssl->supported_group_list;
-  *out_group_ids_len = ssl->supported_group_list_len;
-  if (!*out_group_ids) {
-    *out_group_ids = kDefaultGroups;
-    *out_group_ids_len = OPENSSL_ARRAY_SIZE(kDefaultGroups);
+Span<const uint16_t> tls1_get_grouplist(const SSL *ssl) {
+  if (ssl->supported_group_list != nullptr) {
+    return MakeConstSpan(ssl->supported_group_list,
+                         ssl->supported_group_list_len);
   }
+  return Span<const uint16_t>(kDefaultGroups);
 }
 
 int tls1_get_shared_group(SSL_HANDSHAKE *hs, uint16_t *out_group_id) {
   SSL *const ssl = hs->ssl;
   assert(ssl->server);
 
-  const uint16_t *groups, *pref, *supp;
-  size_t groups_len, pref_len, supp_len;
-  tls1_get_grouplist(ssl, &groups, &groups_len);
-
   // Clients are not required to send a supported_groups extension. In this
   // case, the server is free to pick any group it likes. See RFC 4492,
   // section 4, paragraph 3.
@@ -326,22 +320,20 @@ int tls1_get_shared_group(SSL_HANDSHAKE *hs, uint16_t *out_group_id) {
   // support our favoured group. Thus we do not special-case an emtpy
   // |peer_supported_group_list|.
 
+  Span<const uint16_t> groups = tls1_get_grouplist(ssl);
+  Span<const uint16_t> pref, supp;
   if (ssl->options & SSL_OP_CIPHER_SERVER_PREFERENCE) {
     pref = groups;
-    pref_len = groups_len;
     supp = hs->peer_supported_group_list;
-    supp_len = hs->peer_supported_group_list_len;
   } else {
     pref = hs->peer_supported_group_list;
-    pref_len = hs->peer_supported_group_list_len;
     supp = groups;
-    supp_len = groups_len;
   }
 
-  for (size_t i = 0; i < pref_len; i++) {
-    for (size_t j = 0; j < supp_len; j++) {
-      if (pref[i] == supp[j]) {
-        *out_group_id = pref[i];
+  for (uint16_t pref_group : pref) {
+    for (uint16_t supp_group : supp) {
+      if (pref_group == supp_group) {
+        *out_group_id = pref_group;
         return 1;
       }
     }
@@ -414,12 +406,9 @@ err:
   return 0;
 }
 
-int tls1_check_group_id(SSL *ssl, uint16_t group_id) {
-  const uint16_t *groups;
-  size_t groups_len;
-  tls1_get_grouplist(ssl, &groups, &groups_len);
-  for (size_t i = 0; i < groups_len; i++) {
-    if (groups[i] == group_id) {
+int tls1_check_group_id(const SSL *ssl, uint16_t group_id) {
+  for (uint16_t supported : tls1_get_grouplist(ssl)) {
+    if (supported == group_id) {
       return 1;
     }
   }
@@ -2134,10 +2123,8 @@ static int ext_key_share_add_clienthello(SSL_HANDSHAKE *hs, CBB *out) {
     }
 
     // Predict the most preferred group.
-    const uint16_t *groups;
-    size_t groups_len;
-    tls1_get_grouplist(ssl, &groups, &groups_len);
-    if (groups_len == 0) {
+    Span<const uint16_t> groups = tls1_get_grouplist(ssl);
+    if (groups.size() == 0) {
       OPENSSL_PUT_ERROR(SSL, SSL_R_NO_GROUPS_SPECIFIED);
       return 0;
     }
@@ -2368,12 +2355,8 @@ static int ext_supported_groups_add_clienthello(SSL_HANDSHAKE *hs, CBB *out) {
     return 0;
   }
 
-  const uint16_t *groups;
-  size_t groups_len;
-  tls1_get_grouplist(ssl, &groups, &groups_len);
-
-  for (size_t i = 0; i < groups_len; i++) {
-    if (!CBB_add_u16(&groups_bytes, groups[i])) {
+  for (uint16_t group : tls1_get_grouplist(ssl)) {
+    if (!CBB_add_u16(&groups_bytes, group)) {
       return 0;
     }
   }
@@ -2404,31 +2387,20 @@ static int ext_supported_groups_parse_clienthello(SSL_HANDSHAKE *hs,
     return 0;
   }
 
-  hs->peer_supported_group_list =
-      (uint16_t *)OPENSSL_malloc(CBS_len(&supported_group_list));
-  if (hs->peer_supported_group_list == NULL) {
-    *out_alert = SSL_AD_INTERNAL_ERROR;
+  Array<uint16_t> groups;
+  if (!groups.Init(CBS_len(&supported_group_list) / 2)) {
     return 0;
   }
-
-  const size_t num_groups = CBS_len(&supported_group_list) / 2;
-  for (size_t i = 0; i < num_groups; i++) {
-    if (!CBS_get_u16(&supported_group_list,
-                     &hs->peer_supported_group_list[i])) {
-      goto err;
+  for (size_t i = 0; i < groups.size(); i++) {
+    if (!CBS_get_u16(&supported_group_list, &groups[i])) {
+      *out_alert = SSL_AD_INTERNAL_ERROR;
+      return 0;
     }
   }
 
   assert(CBS_len(&supported_group_list) == 0);
-  hs->peer_supported_group_list_len = num_groups;
-
+  hs->peer_supported_group_list = std::move(groups);
   return 1;
-
-err:
-  OPENSSL_free(hs->peer_supported_group_list);
-  hs->peer_supported_group_list = NULL;
-  *out_alert = SSL_AD_INTERNAL_ERROR;
-  return 0;
 }
 
 static int ext_supported_groups_add_serverhello(SSL_HANDSHAKE *hs, CBB *out) {

ssl/tls13_client.cc

@@ -113,18 +113,7 @@ static enum ssl_hs_wait_t do_read_hello_retry_request(SSL_HANDSHAKE *hs) {
     }
 
     // The group must be supported.
-    const uint16_t *groups;
-    size_t groups_len;
-    tls1_get_grouplist(ssl, &groups, &groups_len);
-    int found = 0;
-    for (size_t i = 0; i < groups_len; i++) {
-      if (groups[i] == group_id) {
-        found = 1;
-        break;
-      }
-    }
-
-    if (!found) {
+    if (!tls1_check_group_id(ssl, group_id)) {
       ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER);
       OPENSSL_PUT_ERROR(SSL, SSL_R_WRONG_CURVE);
       return ssl_hs_error;