[openssl-commits] [openssl] master update
Dr. Stephen Henson
steve at openssl.org
Fri Oct 6 18:19:14 UTC 2017
The branch master has been updated
via 6447e8184cf6deca233d38ab3e9c9aa6ba60e9a5 (commit)
via f48d826e33cac6f88cf41da0af9f54a287bdbadd (commit)
via ff6d20a67bca5a585124bb47c2672dec3594ff95 (commit)
from f0b843c1f4eade5f9d54f826b16cec5ebd15a502 (commit)
- Log -----------------------------------------------------------------
commit 6447e8184cf6deca233d38ab3e9c9aa6ba60e9a5
Author: Dr. Stephen Henson <steve at openssl.org>
Date: Tue Sep 26 16:17:44 2017 +0100
Merge tls1_check_curve into tls1_check_group_id
The function tls_check_curve is only called on clients and contains
almost identical functionaity to tls1_check_group_id when called from
a client. Merge the two.
Reviewed-by: Rich Salz <rsalz at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4475)
commit f48d826e33cac6f88cf41da0af9f54a287bdbadd
Author: Dr. Stephen Henson <steve at openssl.org>
Date: Tue Sep 26 15:41:34 2017 +0100
Change curves to groups where relevant
Reviewed-by: Rich Salz <rsalz at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4475)
commit ff6d20a67bca5a585124bb47c2672dec3594ff95
Author: Dr. Stephen Henson <steve at openssl.org>
Date: Tue Sep 26 15:28:16 2017 +0100
Use separate functions for supported and peer groups lists
Reviewed-by: Rich Salz <rsalz at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4475)
-----------------------------------------------------------------------
Summary of changes:
ssl/ssl_locl.h | 13 +++--
ssl/statem/extensions.c | 16 +++---
ssl/statem/extensions_clnt.c | 34 ++++++-------
ssl/statem/extensions_srvr.c | 20 ++++----
ssl/statem/statem_clnt.c | 14 +++---
ssl/t1_lib.c | 115 ++++++++++++++++++-------------------------
6 files changed, 99 insertions(+), 113 deletions(-)
diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h
index 960182e..c73035d 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);
@@ -2340,7 +2347,7 @@ SSL_COMP *ssl3_comp_find(STACK_OF(SSL_COMP) *sk, int n);
# ifndef OPENSSL_NO_EC
__owur const TLS_GROUP_INFO *tls1_group_id_lookup(uint16_t curve_id);
-__owur int tls1_check_curve(SSL *s, const unsigned char *p, size_t len);
+__owur int tls1_check_group_id(SSL *s, uint16_t group_id);
__owur uint16_t tls1_shared_group(SSL *s, int nmatch);
__owur int tls1_set_groups(uint16_t **pext, size_t *pextlen,
int *curves, size_t ncurves);
@@ -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 4e8dc70..f6a200f 100644
--- a/ssl/statem/extensions.c
+++ b/ssl/statem/extensions.c
@@ -1167,25 +1167,25 @@ static int final_key_share(SSL *s, unsigned int context, int sent, int *al)
&& (!s->hit
|| (s->ext.psk_kex_mode & TLSEXT_KEX_MODE_FLAG_KE_DHE)
!= 0)) {
- const uint16_t *pcurves, *clntcurves;
- size_t num_curves, clnt_num_curves, i;
+ const uint16_t *pgroups, *clntgroups;
+ size_t num_groups, clnt_num_groups, i;
unsigned int group_id = 0;
/* 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, &clntgroups, &clnt_num_groups);
+ tls1_get_supported_groups(s, &pgroups, &num_groups);
/* Find the first group we allow that is also in client's list */
- for (i = 0; i < num_curves; i++) {
- group_id = pcurves[i];
+ for (i = 0; i < num_groups; i++) {
+ group_id = pgroups[i];
- if (check_in_list(s, group_id, clntcurves, clnt_num_curves, 1))
+ if (check_in_list(s, group_id, clntgroups, clnt_num_groups, 1))
break;
}
- if (i < num_curves) {
+ if (i < num_groups) {
/* A shared group exists so send a HelloRetryRequest */
s->s3->group_id = group_id;
s->hello_retry_request = 1;
diff --git a/ssl/statem/extensions_clnt.c b/ssl/statem/extensions_clnt.c
index 047f2d0..c1f98b4 100644
--- a/ssl/statem/extensions_clnt.c
+++ b/ssl/statem/extensions_clnt.c
@@ -139,8 +139,8 @@ EXT_RETURN tls_construct_ctos_supported_groups(SSL *s, WPACKET *pkt,
unsigned int context, X509 *x,
size_t chainidx, int *al)
{
- const uint16_t *pcurves = NULL;
- size_t num_curves = 0, i;
+ const uint16_t *pgroups = NULL;
+ size_t num_groups = 0, i;
if (!use_ecc(s))
return EXT_RETURN_NOT_SENT;
@@ -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, &pgroups, &num_groups);
if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_supported_groups)
/* Sub-packet for supported_groups extension */
@@ -160,8 +160,8 @@ EXT_RETURN tls_construct_ctos_supported_groups(SSL *s, WPACKET *pkt,
return EXT_RETURN_FAIL;
}
/* Copy curve ID if supported */
- for (i = 0; i < num_curves; i++) {
- uint16_t ctmp = pcurves[i];
+ for (i = 0; i < num_groups; i++) {
+ uint16_t ctmp = pgroups[i];
if (tls_curve_allowed(s, ctmp, SSL_SECOP_CURVE_SUPPORTED)) {
if (!WPACKET_put_bytes_u16(pkt, ctmp)) {
@@ -590,8 +590,8 @@ EXT_RETURN tls_construct_ctos_key_share(SSL *s, WPACKET *pkt,
size_t chainidx, int *al)
{
#ifndef OPENSSL_NO_TLS1_3
- size_t i, num_curves = 0;
- const uint16_t *pcurves = NULL;
+ size_t i, num_groups = 0;
+ const uint16_t *pgroups = NULL;
uint16_t curve_id = 0;
/* key_share 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, &pgroups, &num_groups);
/*
* TODO(TLS1.3): Make the number of key_shares sent configurable. For
@@ -613,12 +613,12 @@ EXT_RETURN tls_construct_ctos_key_share(SSL *s, WPACKET *pkt,
if (s->s3->group_id != 0) {
curve_id = s->s3->group_id;
} else {
- for (i = 0; i < num_curves; i++) {
+ for (i = 0; i < num_groups; i++) {
- if (!tls_curve_allowed(s, pcurves[i], SSL_SECOP_CURVE_SUPPORTED))
+ if (!tls_curve_allowed(s, pgroups[i], SSL_SECOP_CURVE_SUPPORTED))
continue;
- curve_id = pcurves[i];
+ curve_id = pgroups[i];
break;
}
}
@@ -1514,8 +1514,8 @@ int tls_parse_stoc_key_share(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
}
if ((context & SSL_EXT_TLS1_3_HELLO_RETRY_REQUEST) != 0) {
- const uint16_t *pcurves = NULL;
- size_t i, num_curves;
+ const uint16_t *pgroups = NULL;
+ size_t i, num_groups;
if (PACKET_remaining(pkt) != 0) {
*al = SSL_AD_DECODE_ERROR;
@@ -1534,12 +1534,12 @@ 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);
- for (i = 0; i < num_curves; i++) {
- if (group_id == pcurves[i])
+ tls1_get_supported_groups(s, &pgroups, &num_groups);
+ for (i = 0; i < num_groups; i++) {
+ if (group_id == pgroups[i])
break;
}
- if (i >= num_curves
+ if (i >= num_groups
|| !tls_curve_allowed(s, group_id, SSL_SECOP_CURVE_SUPPORTED)) {
*al = SSL_AD_ILLEGAL_PARAMETER;
SSLerr(SSL_F_TLS_PARSE_STOC_KEY_SHARE, SSL_R_BAD_KEY_SHARE);
diff --git a/ssl/statem/extensions_srvr.c b/ssl/statem/extensions_srvr.c
index 3be9754..8bf3a76 100644
--- a/ssl/statem/extensions_srvr.c
+++ b/ssl/statem/extensions_srvr.c
@@ -499,8 +499,8 @@ int tls_parse_ctos_key_share(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
#ifndef OPENSSL_NO_TLS1_3
unsigned int group_id;
PACKET key_share_list, encoded_pt;
- const uint16_t *clntcurves, *srvrcurves;
- size_t clnt_num_curves, srvr_num_curves;
+ const uint16_t *clntgroups, *srvrgroups;
+ size_t clnt_num_groups, srvr_num_groups;
int found = 0;
if (s->hit && (s->ext.psk_kex_mode & TLSEXT_KEX_MODE_FLAG_KE_DHE) == 0)
@@ -519,11 +519,11 @@ int tls_parse_ctos_key_share(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
return 0;
}
- /* Get our list of supported curves */
- tls1_get_grouplist(s, 0, &srvrcurves, &srvr_num_curves);
- /* Get the clients list of supported curves. */
- tls1_get_grouplist(s, 1, &clntcurves, &clnt_num_curves);
- if (clnt_num_curves == 0) {
+ /* Get our list of supported groups */
+ tls1_get_supported_groups(s, &srvrgroups, &srvr_num_groups);
+ /* Get the clients list of supported groups. */
+ tls1_get_peer_groups(s, &clntgroups, &clnt_num_groups);
+ if (clnt_num_groups == 0) {
/*
* This can only happen if the supported_groups extension was not sent,
* because we verify that the length is non-zero when we process that
@@ -553,14 +553,14 @@ int tls_parse_ctos_key_share(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
continue;
/* Check if this share is in supported_groups sent from client */
- if (!check_in_list(s, group_id, clntcurves, clnt_num_curves, 0)) {
+ if (!check_in_list(s, group_id, clntgroups, clnt_num_groups, 0)) {
*al = SSL_AD_ILLEGAL_PARAMETER;
SSLerr(SSL_F_TLS_PARSE_CTOS_KEY_SHARE, SSL_R_BAD_KEY_SHARE);
return 0;
}
/* Check if this share is for a group we can use */
- if (!check_in_list(s, group_id, srvrcurves, srvr_num_curves, 1)) {
+ if (!check_in_list(s, group_id, srvrgroups, srvr_num_groups, 1)) {
/* Share not suitable */
continue;
}
@@ -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/statem/statem_clnt.c b/ssl/statem/statem_clnt.c
index 8ca4737..2ad33f2 100644
--- a/ssl/statem/statem_clnt.c
+++ b/ssl/statem/statem_clnt.c
@@ -2037,29 +2037,29 @@ static int tls_process_ske_ecdhe(SSL *s, PACKET *pkt, EVP_PKEY **pkey, int *al)
{
#ifndef OPENSSL_NO_EC
PACKET encoded_pt;
- const unsigned char *ecparams;
+ unsigned int curve_type, curve_id;
/*
* Extract elliptic curve parameters and the server's ephemeral ECDH
- * public key. For now we only support named (not generic) curves and
+ * public key. We only support named (not generic) curves and
* ECParameters in this case is just three bytes.
*/
- if (!PACKET_get_bytes(pkt, &ecparams, 3)) {
+ if (!PACKET_get_1(pkt, &curve_type) || !PACKET_get_net_2(pkt, &curve_id)) {
*al = SSL_AD_DECODE_ERROR;
SSLerr(SSL_F_TLS_PROCESS_SKE_ECDHE, SSL_R_LENGTH_TOO_SHORT);
return 0;
}
/*
- * Check curve is one of our preferences, if not server has sent an
- * invalid curve. ECParameters is 3 bytes.
+ * Check curve is named curve type and one of our preferences, if not
+ * server has sent an invalid curve.
*/
- if (!tls1_check_curve(s, ecparams, 3)) {
+ if (curve_type != NAMED_CURVE_TYPE || !tls1_check_group_id(s, curve_id)) {
*al = SSL_AD_ILLEGAL_PARAMETER;
SSLerr(SSL_F_TLS_PROCESS_SKE_ECDHE, SSL_R_WRONG_CURVE);
return 0;
}
- if ((s->s3->peer_tmp = ssl_generate_param_group(ecparams[2])) == NULL) {
+ if ((s->s3->peer_tmp = ssl_generate_param_group(curve_id)) == NULL) {
*al = SSL_AD_INTERNAL_ERROR;
SSLerr(SSL_F_TLS_PROCESS_SKE_ECDHE,
SSL_R_UNABLE_TO_FIND_ECDH_PARAMETERS);
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index 6ca9994..bb097ed 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -186,12 +186,12 @@ static const uint16_t suiteb_curves[] = {
TLSEXT_curve_P_384
};
-const TLS_GROUP_INFO *tls1_group_id_lookup(uint16_t curve_id)
+const TLS_GROUP_INFO *tls1_group_id_lookup(uint16_t group_id)
{
/* ECC curves from RFC 4492 and RFC 7027 */
- if (curve_id < 1 || curve_id > OSSL_NELEM(nid_list))
+ if (group_id < 1 || group_id > OSSL_NELEM(nid_list))
return NULL;
- return &nid_list[curve_id - 1];
+ return &nid_list[group_id - 1];
}
static uint16_t tls1_nid2group_id(int nid)
@@ -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;
}
@@ -278,34 +268,6 @@ static int tls1_in_list(uint16_t id, const uint16_t *list, size_t listlen)
return 0;
}
-/* Check a curve is one of our preferences */
-int tls1_check_curve(SSL *s, const unsigned char *p, size_t len)
-{
- const uint16_t *curves;
- size_t num_curves;
- uint16_t curve_id;
-
- if (len != 3 || p[0] != NAMED_CURVE_TYPE)
- return 0;
- curve_id = (p[1] << 8) | p[2];
- /* Check curve matches Suite B preferences */
- if (tls1_suiteb(s)) {
- unsigned long cid = s->s3->tmp.new_cipher->id;
- if (cid == TLS1_CK_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256) {
- if (curve_id != TLSEXT_curve_P_256)
- return 0;
- } else if (cid == TLS1_CK_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384) {
- if (curve_id != TLSEXT_curve_P_384)
- return 0;
- } else /* Should never happen */
- return 0;
- }
- tls1_get_grouplist(s, 0, &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);
-}
-
/*-
* For nmatch >= 0, return the id of the |nmatch|th shared group or 0
* if there is no match.
@@ -341,15 +303,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];
@@ -502,7 +465,7 @@ static int tls1_check_pkey_comp(SSL *s, EVP_PKEY *pkey)
}
/* Check a group id matches preferences */
-static int tls1_check_group_id(SSL *s, uint16_t group_id)
+int tls1_check_group_id(SSL *s, uint16_t group_id)
{
const uint16_t *groups;
size_t groups_len;
@@ -510,20 +473,36 @@ static int tls1_check_group_id(SSL *s, uint16_t group_id)
if (group_id == 0)
return 0;
- if (!tls_curve_allowed(s, group_id, SSL_SECOP_CURVE_CHECK))
- return 0;
+ /* Check for Suite B compliance */
+ if (tls1_suiteb(s) && s->s3->tmp.new_cipher != NULL) {
+ unsigned long cid = s->s3->tmp.new_cipher->id;
+
+ if (cid == TLS1_CK_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256) {
+ if (group_id != TLSEXT_curve_P_256)
+ return 0;
+ } else if (cid == TLS1_CK_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384) {
+ if (group_id != TLSEXT_curve_P_384)
+ return 0;
+ } else {
+ /* Should never happen */
+ 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;
+ if (!tls_curve_allowed(s, group_id, SSL_SECOP_CURVE_CHECK))
+ return 0;
+
/* For clients, nothing more to check */
if (!s->server)
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
More information about the openssl-commits
mailing list