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>
diff --git a/ssl/handshake.cc b/ssl/handshake.cc index 141287e..dece144 100644 --- a/ssl/handshake.cc +++ b/ssl/handshake.cc
@@ -148,7 +148,6 @@ 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);
diff --git a/ssl/internal.h b/ssl/internal.h index 9b397f2..17fe602 100644 --- a/ssl/internal.h +++ b/ssl/internal.h
@@ -1307,8 +1307,7 @@ // 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_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
diff --git a/ssl/t1_lib.cc b/ssl/t1_lib.cc index e32ef47..32311ff 100644 --- a/ssl/t1_lib.cc +++ b/ssl/t1_lib.cc
@@ -299,24 +299,18 @@ 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 @@ // 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 @@ 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 @@ } // 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 @@ 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 @@ 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) {
diff --git a/ssl/tls13_client.cc b/ssl/tls13_client.cc index 38df531..2ac6195 100644 --- a/ssl/tls13_client.cc +++ b/ssl/tls13_client.cc
@@ -113,18 +113,7 @@ } // 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;