Remove the get_peer_groups parameter to tls1_get_grouplist.
It's weird and makes things more confusing. Only use it for local
preferences as there is a default. Peer preferences can be read
directly. Also simplify the logic for requiring a non-empty peer group
list for ECDHE. The normal logic will give us this for free.
Change-Id: I1916155fe246be988f20cbf0b1728380ec90ff3d
Reviewed-on: https://boringssl-review.googlesource.com/11527
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index ecb8675..a134c3f 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -312,18 +312,8 @@
#endif
};
-void tls1_get_grouplist(SSL *ssl, int get_peer_groups,
- const uint16_t **out_group_ids,
+void tls1_get_grouplist(SSL *ssl, const uint16_t **out_group_ids,
size_t *out_group_ids_len) {
- if (get_peer_groups) {
- /* Only clients send a supported group list, so this function is only
- * called on the server. */
- assert(ssl->server);
- *out_group_ids = ssl->s3->tmp.peer_supported_group_list;
- *out_group_ids_len = ssl->s3->tmp.peer_supported_group_list_len;
- return;
- }
-
*out_group_ids = ssl->supported_group_list;
*out_group_ids_len = ssl->supported_group_list_len;
if (!*out_group_ids) {
@@ -333,42 +323,35 @@
}
int tls1_get_shared_group(SSL *ssl, uint16_t *out_group_id) {
- const uint16_t *groups, *peer_groups, *pref, *supp;
- size_t groups_len, peer_groups_len, pref_len, supp_len, i, j;
+ assert(ssl->server);
- /* Can't do anything on client side */
- if (ssl->server == 0) {
- return 0;
- }
+ const uint16_t *groups, *pref, *supp;
+ size_t groups_len, pref_len, supp_len;
+ tls1_get_grouplist(ssl, &groups, &groups_len);
- tls1_get_grouplist(ssl, 0 /* local groups */, &groups, &groups_len);
- tls1_get_grouplist(ssl, 1 /* peer groups */, &peer_groups, &peer_groups_len);
-
- if (peer_groups_len == 0) {
- /* 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.
- *
- * However, in the interests of compatibility, we will skip ECDH if the
- * client didn't send an extension because we can't be sure that they'll
- * support our favoured group. */
- return 0;
- }
+ /* 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.
+ *
+ * However, in the interests of compatibility, we will skip ECDH if the
+ * client didn't send an extension because we can't be sure that they'll
+ * support our favoured group. Thus we do not special-case an emtpy
+ * |peer_supported_group_list|. */
if (ssl->options & SSL_OP_CIPHER_SERVER_PREFERENCE) {
pref = groups;
pref_len = groups_len;
- supp = peer_groups;
- supp_len = peer_groups_len;
+ supp = ssl->s3->tmp.peer_supported_group_list;
+ supp_len = ssl->s3->tmp.peer_supported_group_list_len;
} else {
- pref = peer_groups;
- pref_len = peer_groups_len;
+ pref = ssl->s3->tmp.peer_supported_group_list;
+ pref_len = ssl->s3->tmp.peer_supported_group_list_len;
supp = groups;
supp_len = groups_len;
}
- for (i = 0; i < pref_len; i++) {
- for (j = 0; j < supp_len; j++) {
+ 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];
return 1;
@@ -448,7 +431,7 @@
int tls1_check_group_id(SSL *ssl, uint16_t group_id) {
const uint16_t *groups;
size_t groups_len;
- tls1_get_grouplist(ssl, 0 /* local groups */, &groups, &groups_len);
+ tls1_get_grouplist(ssl, &groups, &groups_len);
for (size_t i = 0; i < groups_len; i++) {
if (groups[i] == group_id) {
return 1;
@@ -2067,7 +2050,7 @@
/* Predict the most preferred group. */
const uint16_t *groups;
size_t groups_len;
- tls1_get_grouplist(ssl, 0 /* local groups */, &groups, &groups_len);
+ tls1_get_grouplist(ssl, &groups, &groups_len);
if (groups_len == 0) {
OPENSSL_PUT_ERROR(SSL, SSL_R_NO_GROUPS_SPECIFIED);
return 0;
@@ -2303,7 +2286,7 @@
const uint16_t *groups;
size_t groups_len;
- tls1_get_grouplist(ssl, 0, &groups, &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])) {
