From ff6d20a67bca5a585124bb47c2672dec3594ff95 Mon Sep 17 00:00:00 2001 From: "Dr. Stephen Henson" Date: Tue, 26 Sep 2017 15:28:16 +0100 Subject: [PATCH] Use separate functions for supported and peer groups lists Reviewed-by: Rich Salz (Merged from https://github.com/openssl/openssl/pull/4475) --- ssl/ssl_locl.h | 11 +++++-- ssl/statem/extensions.c | 4 +-- ssl/statem/extensions_clnt.c | 6 ++-- ssl/statem/extensions_srvr.c | 6 ++-- ssl/t1_lib.c | 61 +++++++++++++++--------------------- 5 files changed, 43 insertions(+), 45 deletions(-) diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h index 960182ed12..95f679bb7c 100644 --- a/ssl/ssl_locl.h +++ b/ssl/ssl_locl.h @@ -2085,6 +2085,13 @@ static ossl_inline int ssl_has_cert(const SSL *s, int idx) && s->cert->pkeys[idx].privatekey != NULL; } +static ossl_inline void tls1_get_peer_groups(SSL *s, const uint16_t **pgroups, + size_t *pgroupslen) +{ + *pgroups = s->session->ext.supportedgroups; + *pgroupslen = s->session->ext.supportedgroups_len; +} + # ifndef OPENSSL_UNIT_TEST __owur int ssl_read_internal(SSL *s, void *buf, size_t num, size_t *readbytes); @@ -2354,8 +2361,8 @@ __owur EVP_PKEY *ssl_generate_param_group(uint16_t id); # endif /* OPENSSL_NO_EC */ __owur int tls_curve_allowed(SSL *s, uint16_t curve, int op); -void tls1_get_grouplist(SSL *s, int sess, const uint16_t **pcurves, - size_t *num_curves); +void tls1_get_supported_groups(SSL *s, const uint16_t **pgroups, + size_t *pgroupslen); __owur int tls1_set_server_sigalgs(SSL *s); diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c index 4e8dc70756..69214c522c 100644 --- a/ssl/statem/extensions.c +++ b/ssl/statem/extensions.c @@ -1174,8 +1174,8 @@ static int final_key_share(SSL *s, unsigned int context, int sent, int *al) /* Check if a shared group exists */ /* Get the clients list of supported groups. */ - tls1_get_grouplist(s, 1, &clntcurves, &clnt_num_curves); - tls1_get_grouplist(s, 0, &pcurves, &num_curves); + tls1_get_peer_groups(s, &clntcurves, &clnt_num_curves); + tls1_get_supported_groups(s, &pcurves, &num_curves); /* Find the first group we allow that is also in client's list */ for (i = 0; i < num_curves; i++) { diff --git a/ssl/statem/extensions_clnt.c b/ssl/statem/extensions_clnt.c index 047f2d0cea..e392ac484b 100644 --- a/ssl/statem/extensions_clnt.c +++ b/ssl/statem/extensions_clnt.c @@ -149,7 +149,7 @@ EXT_RETURN tls_construct_ctos_supported_groups(SSL *s, WPACKET *pkt, * Add TLS extension supported_groups to the ClientHello message */ /* TODO(TLS1.3): Add support for DHE groups */ - tls1_get_grouplist(s, 0, &pcurves, &num_curves); + tls1_get_supported_groups(s, &pcurves, &num_curves); if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_supported_groups) /* Sub-packet for supported_groups extension */ @@ -604,7 +604,7 @@ EXT_RETURN tls_construct_ctos_key_share(SSL *s, WPACKET *pkt, return EXT_RETURN_FAIL; } - tls1_get_grouplist(s, 0, &pcurves, &num_curves); + tls1_get_supported_groups(s, &pcurves, &num_curves); /* * TODO(TLS1.3): Make the number of key_shares sent configurable. For @@ -1534,7 +1534,7 @@ int tls_parse_stoc_key_share(SSL *s, PACKET *pkt, unsigned int context, X509 *x, } /* Validate the selected group is one we support */ - tls1_get_grouplist(s, 0, &pcurves, &num_curves); + tls1_get_supported_groups(s, &pcurves, &num_curves); for (i = 0; i < num_curves; i++) { if (group_id == pcurves[i]) break; diff --git a/ssl/statem/extensions_srvr.c b/ssl/statem/extensions_srvr.c index 3be9754b2a..8183ea574c 100644 --- a/ssl/statem/extensions_srvr.c +++ b/ssl/statem/extensions_srvr.c @@ -520,9 +520,9 @@ int tls_parse_ctos_key_share(SSL *s, PACKET *pkt, unsigned int context, X509 *x, } /* Get our list of supported curves */ - tls1_get_grouplist(s, 0, &srvrcurves, &srvr_num_curves); + tls1_get_supported_groups(s, &srvrcurves, &srvr_num_curves); /* Get the clients list of supported curves. */ - tls1_get_grouplist(s, 1, &clntcurves, &clnt_num_curves); + tls1_get_peer_groups(s, &clntcurves, &clnt_num_curves); if (clnt_num_curves == 0) { /* * This can only happen if the supported_groups extension was not sent, @@ -885,7 +885,7 @@ EXT_RETURN tls_construct_stoc_supported_groups(SSL *s, WPACKET *pkt, return EXT_RETURN_NOT_SENT; /* Get our list of supported groups */ - tls1_get_grouplist(s, 0, &groups, &numgroups); + tls1_get_supported_groups(s, &groups, &numgroups); if (numgroups == 0) { SSLerr(SSL_F_TLS_CONSTRUCT_STOC_SUPPORTED_GROUPS, ERR_R_INTERNAL_ERROR); return EXT_RETURN_FAIL; diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index 6ca9994988..e002013d4f 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c @@ -205,47 +205,37 @@ static uint16_t tls1_nid2group_id(int nid) } /* - * Get curves list, if "sess" is set return client curves otherwise - * preferred list. - * Sets |num_curves| to the number of curves in the list, i.e., - * the length of |pcurves| is num_curves. - * Returns 1 on success and 0 if the client curves list has invalid format. - * The latter indicates an internal error: we should not be accepting such - * lists in the first place. + * Set *pgroups to the supported groups list and *pgroupslen to + * the number of groups supported. */ -void tls1_get_grouplist(SSL *s, int sess, const uint16_t **pcurves, - size_t *pcurveslen) +void tls1_get_supported_groups(SSL *s, const uint16_t **pgroups, + size_t *pgroupslen) { - if (sess) { - *pcurves = s->session->ext.supportedgroups; - *pcurveslen = s->session->ext.supportedgroups_len; - return; - } /* For Suite B mode only include P-256, P-384 */ switch (tls1_suiteb(s)) { case SSL_CERT_FLAG_SUITEB_128_LOS: - *pcurves = suiteb_curves; - *pcurveslen = OSSL_NELEM(suiteb_curves); + *pgroups = suiteb_curves; + *pgroupslen = OSSL_NELEM(suiteb_curves); break; case SSL_CERT_FLAG_SUITEB_128_LOS_ONLY: - *pcurves = suiteb_curves; - *pcurveslen = 1; + *pgroups = suiteb_curves; + *pgroupslen = 1; break; case SSL_CERT_FLAG_SUITEB_192_LOS: - *pcurves = suiteb_curves + 1; - *pcurveslen = 1; + *pgroups = suiteb_curves + 1; + *pgroupslen = 1; break; default: if (s->ext.supportedgroups == NULL) { - *pcurves = eccurves_default; - *pcurveslen = OSSL_NELEM(eccurves_default); + *pgroups = eccurves_default; + *pgroupslen = OSSL_NELEM(eccurves_default); } else { - *pcurves = s->ext.supportedgroups; - *pcurveslen = s->ext.supportedgroups_len; + *pgroups = s->ext.supportedgroups; + *pgroupslen = s->ext.supportedgroups_len; } break; } @@ -300,7 +290,7 @@ int tls1_check_curve(SSL *s, const unsigned char *p, size_t len) } else /* Should never happen */ return 0; } - tls1_get_grouplist(s, 0, &curves, &num_curves); + tls1_get_supported_groups(s, &curves, &num_curves); if (!tls1_in_list(curve_id, curves, num_curves)) return 0; return tls_curve_allowed(s, curve_id, SSL_SECOP_CURVE_CHECK); @@ -341,15 +331,16 @@ uint16_t tls1_shared_group(SSL *s, int nmatch) nmatch = 0; } /* - * Avoid truncation. tls1_get_grouplist takes an int - * but s->options is a long... + * If server preference set, our groups are the preference order + * otherwise peer decides. */ - tls1_get_grouplist(s, - (s->options & SSL_OP_CIPHER_SERVER_PREFERENCE) != 0, - &supp, &num_supp); - tls1_get_grouplist(s, - (s->options & SSL_OP_CIPHER_SERVER_PREFERENCE) == 0, - &pref, &num_pref); + if (s->options & SSL_OP_CIPHER_SERVER_PREFERENCE) { + tls1_get_supported_groups(s, &pref, &num_pref); + tls1_get_peer_groups(s, &supp, &num_supp); + } else { + tls1_get_peer_groups(s, &pref, &num_pref); + tls1_get_supported_groups(s, &supp, &num_supp); + } for (k = 0, i = 0; i < num_pref; i++) { uint16_t id = pref[i]; @@ -514,7 +505,7 @@ static int tls1_check_group_id(SSL *s, uint16_t group_id) return 0; /* Check group is one of our preferences */ - tls1_get_grouplist(s, 0, &groups, &groups_len); + tls1_get_supported_groups(s, &groups, &groups_len); if (!tls1_in_list(group_id, groups, groups_len)) return 0; @@ -523,7 +514,7 @@ static int tls1_check_group_id(SSL *s, uint16_t group_id) return 1; /* Check group is one of peers preferences */ - tls1_get_grouplist(s, 1, &groups, &groups_len); + tls1_get_peer_groups(s, &groups, &groups_len); /* * RFC 4492 does not require the supported elliptic curves extension -- 2.39.2