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;