The branch master has been updated via b50951d36c329b07e1c92d41cc02c9085fb567d7 (commit) via 5ce5f787e3b03c33297b1480ec138938a4ff5e8e (commit) via 34e5292c578321b80d8e474db4be6d90519d8f33 (commit) via 4a1b42801997f3083211a24592d1a691a0747250 (commit) via 612f9d2227897a3dc76c9dc74a8c30aa1dc7b5a1 (commit) via 43b95d736561e64dd7c1c97555f39a98c56d1ae3 (commit) via 0e464d9ddd3b7995bce27d780f8295f47ac5a647 (commit) via 8841154a90914c6f8cb4649beed36f8d1b82a19c (commit) via 1483b8582ce2c77717dea5c369efe56c93cef998 (commit) from 4881d849da23528e19b7312f963d28916d9804b1 (commit)
- Log ----------------------------------------------------------------- commit b50951d36c329b07e1c92d41cc02c9085fb567d7 Author: Dr. Stephen Henson <st...@openssl.org> Date: Sun Sep 24 21:59:39 2017 +0100 Add and use function tls1_in_list to avoid code duplication. [extended tests] Reviewed-by: Rich Salz <rs...@openssl.org> (Merged from https://github.com/openssl/openssl/pull/=4412) commit 5ce5f787e3b03c33297b1480ec138938a4ff5e8e Author: Dr. Stephen Henson <st...@openssl.org> Date: Sun Sep 24 21:58:58 2017 +0100 Use tls1_group_id_lookup in tls1_curve_allowed Reviewed-by: Rich Salz <rs...@openssl.org> (Merged from https://github.com/openssl/openssl/pull/=4412) commit 34e5292c578321b80d8e474db4be6d90519d8f33 Author: Dr. Stephen Henson <st...@openssl.org> Date: Sun Sep 24 03:26:26 2017 +0100 Rename tls1_get_curvelist. Rename tls1_get_curvelist to tls1_get_grouplist, change to void as it can never fail and remove unnecessary return value checks. Clean up the code. Reviewed-by: Rich Salz <rs...@openssl.org> (Merged from https://github.com/openssl/openssl/pull/=4412) commit 4a1b42801997f3083211a24592d1a691a0747250 Author: Dr. Stephen Henson <st...@openssl.org> Date: Sun Sep 24 01:45:27 2017 +0100 Rewrite compression and group checks. Replace existing compression and groups check with two functions. tls1_check_pkey_comp() checks a keys compression algorithms is consistent with extensions. tls1_check_group_id() checks is a group is consistent with extensions and preferences. Rename tls1_ec_nid2curve_id() to tls1_nid2group_id() and make it static. Reviewed-by: Rich Salz <rs...@openssl.org> (Merged from https://github.com/openssl/openssl/pull/=4412) commit 612f9d2227897a3dc76c9dc74a8c30aa1dc7b5a1 Author: Dr. Stephen Henson <st...@openssl.org> Date: Sat Sep 23 02:40:30 2017 +0100 New function ssl_generate_param_group Setup EVP_PKEY structure from a group ID in ssl_generate_param_group, replace duplicate code with this function. Reviewed-by: Rich Salz <rs...@openssl.org> (Merged from https://github.com/openssl/openssl/pull/=4412) commit 43b95d736561e64dd7c1c97555f39a98c56d1ae3 Author: Dr. Stephen Henson <st...@openssl.org> Date: Sat Sep 23 00:15:34 2017 +0100 Replace tls1_ec_curve_id2nid. Replace tls1_ec_curve_id2nid() with tls_group_id_lookup() which returns the TLS_GROUP_INFO for the group. Reviewed-by: Rich Salz <rs...@openssl.org> (Merged from https://github.com/openssl/openssl/pull/=4412) commit 0e464d9ddd3b7995bce27d780f8295f47ac5a647 Author: Dr. Stephen Henson <st...@openssl.org> Date: Fri Sep 22 23:47:54 2017 +0100 Rename tls_curve_info to TLS_GROUP_INFO, move to ssl_locl.h Reviewed-by: Rich Salz <rs...@openssl.org> (Merged from https://github.com/openssl/openssl/pull/=4412) commit 8841154a90914c6f8cb4649beed36f8d1b82a19c Author: Dr. Stephen Henson <st...@openssl.org> Date: Fri Sep 22 23:43:03 2017 +0100 Return group id in tls1_shared_group Reviewed-by: Rich Salz <rs...@openssl.org> (Merged from https://github.com/openssl/openssl/pull/=4412) commit 1483b8582ce2c77717dea5c369efe56c93cef998 Author: Dr. Stephen Henson <st...@openssl.org> Date: Sun Sep 24 01:46:36 2017 +0100 Return correct Suite B curve, fix comment. Reviewed-by: Rich Salz <rs...@openssl.org> (Merged from https://github.com/openssl/openssl/pull/=4412) ----------------------------------------------------------------------- Summary of changes: ssl/s3_lib.c | 78 +++++++-- ssl/ssl_locl.h | 30 ++-- ssl/statem/extensions.c | 14 +- ssl/statem/extensions_clnt.c | 18 +- ssl/statem/extensions_srvr.c | 51 +----- ssl/statem/statem_clnt.c | 33 +--- ssl/statem/statem_srvr.c | 6 +- ssl/t1_lib.c | 403 ++++++++++++++++++++----------------------- 8 files changed, 280 insertions(+), 353 deletions(-) diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c index 4127b28..a8f5637 100644 --- a/ssl/s3_lib.c +++ b/ssl/s3_lib.c @@ -3612,11 +3612,12 @@ long ssl3_ctrl(SSL *s, int cmd, long larg, void *parg) if (parg) { size_t i; int *cptr = parg; + for (i = 0; i < clistlen; i++) { - /* TODO(TLS1.3): Handle DH groups here */ - int nid = tls1_ec_curve_id2nid(clist[i], NULL); - if (nid != 0) - cptr[i] = nid; + const TLS_GROUP_INFO *cinf = tls1_group_id_lookup(clist[i]); + + if (cinf != NULL) + cptr[i] = cinf->nid; else cptr[i] = TLSEXT_nid_unknown | clist[i]; } @@ -3633,8 +3634,16 @@ long ssl3_ctrl(SSL *s, int cmd, long larg, void *parg) &s->ext.supportedgroups_len, parg); case SSL_CTRL_GET_SHARED_GROUP: - return tls1_shared_group(s, larg); + { + uint16_t id = tls1_shared_group(s, larg); + if (larg != -1) { + const TLS_GROUP_INFO *ginf = tls1_group_id_lookup(id); + + return ginf == NULL ? 0 : ginf->nid; + } + return id; + } #endif case SSL_CTRL_SET_SIGALGS: return tls1_set_sigalgs(s->cert, parg, larg, 0); @@ -4581,27 +4590,27 @@ EVP_PKEY *ssl_generate_pkey(EVP_PKEY *pm) return pkey; } #ifndef OPENSSL_NO_EC -/* Generate a private key a curve ID */ -EVP_PKEY *ssl_generate_pkey_curve(int id) +/* Generate a private key from a group ID */ +EVP_PKEY *ssl_generate_pkey_group(uint16_t id) { EVP_PKEY_CTX *pctx = NULL; EVP_PKEY *pkey = NULL; - unsigned int curve_flags; - int nid = tls1_ec_curve_id2nid(id, &curve_flags); + const TLS_GROUP_INFO *ginf = tls1_group_id_lookup(id); + uint16_t gtype; - if (nid == 0) + if (ginf == NULL) goto err; - if ((curve_flags & TLS_CURVE_TYPE) == TLS_CURVE_CUSTOM) { - pctx = EVP_PKEY_CTX_new_id(nid, NULL); - nid = 0; - } else { + gtype = ginf->flags & TLS_CURVE_TYPE; + if (gtype == TLS_CURVE_CUSTOM) + pctx = EVP_PKEY_CTX_new_id(ginf->nid, NULL); + else pctx = EVP_PKEY_CTX_new_id(EVP_PKEY_EC, NULL); - } if (pctx == NULL) goto err; if (EVP_PKEY_keygen_init(pctx) <= 0) goto err; - if (nid != 0 && EVP_PKEY_CTX_set_ec_paramgen_curve_nid(pctx, nid) <= 0) + if (gtype != TLS_CURVE_CUSTOM + && EVP_PKEY_CTX_set_ec_paramgen_curve_nid(pctx, ginf->nid) <= 0) goto err; if (EVP_PKEY_keygen(pctx, &pkey) <= 0) { EVP_PKEY_free(pkey); @@ -4612,6 +4621,43 @@ EVP_PKEY *ssl_generate_pkey_curve(int id) EVP_PKEY_CTX_free(pctx); return pkey; } + +/* + * Generate parameters from a group ID + */ +EVP_PKEY *ssl_generate_param_group(uint16_t id) +{ + EVP_PKEY_CTX *pctx = NULL; + EVP_PKEY *pkey = NULL; + const TLS_GROUP_INFO *ginf = tls1_group_id_lookup(id); + + if (ginf == NULL) + goto err; + + if ((ginf->flags & TLS_CURVE_TYPE) == TLS_CURVE_CUSTOM) { + pkey = EVP_PKEY_new(); + if (pkey != NULL && EVP_PKEY_set_type(pkey, ginf->nid)) + return pkey; + EVP_PKEY_free(pkey); + return NULL; + } + + pctx = EVP_PKEY_CTX_new_id(EVP_PKEY_EC, NULL); + if (pctx == NULL) + goto err; + if (EVP_PKEY_paramgen_init(pctx) <= 0) + goto err; + if (EVP_PKEY_CTX_set_ec_paramgen_curve_nid(pctx, ginf->nid) <= 0) + goto err; + if (EVP_PKEY_paramgen(pctx, &pkey) <= 0) { + EVP_PKEY_free(pkey); + pkey = NULL; + } + + err: + EVP_PKEY_CTX_free(pctx); + return pkey; +} #endif /* Derive secrets for ECDH/DH */ diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h index 5eda636..58156f3 100644 --- a/ssl/ssl_locl.h +++ b/ssl/ssl_locl.h @@ -1349,6 +1349,18 @@ typedef struct sigalg_lookup_st { int curve; } SIGALG_LOOKUP; +typedef struct tls_group_info_st { + int nid; /* Curve NID */ + int secbits; /* Bits of security (from SP800-57) */ + uint16_t flags; /* Flags: currently just group type */ +} TLS_GROUP_INFO; + +/* flags values */ +# define TLS_CURVE_TYPE 0x3 /* Mask for group type */ +# define TLS_CURVE_PRIME 0x0 +# define TLS_CURVE_CHAR2 0x1 +# define TLS_CURVE_CUSTOM 0x2 + typedef struct cert_pkey_st CERT_PKEY; /* @@ -2326,17 +2338,10 @@ __owur int ssl_check_srvr_ecc_cert_and_alg(X509 *x, SSL *s); SSL_COMP *ssl3_comp_find(STACK_OF(SSL_COMP) *sk, int n); # ifndef OPENSSL_NO_EC -/* Flags values from tls1_ec_curve_id2nid() */ -/* Mask for curve type */ -# define TLS_CURVE_TYPE 0x3 -# define TLS_CURVE_PRIME 0x0 -# define TLS_CURVE_CHAR2 0x1 -# define TLS_CURVE_CUSTOM 0x2 -__owur int tls1_ec_curve_id2nid(uint16_t curve_id, unsigned int *pflags); -__owur uint16_t tls1_ec_nid2curve_id(int nid); +__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_shared_group(SSL *s, int nmatch); +__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); __owur int tls1_set_groups_list(uint16_t **pext, size_t *pextlen, @@ -2344,12 +2349,13 @@ __owur int tls1_set_groups_list(uint16_t **pext, size_t *pextlen, void tls1_get_formatlist(SSL *s, const unsigned char **pformats, size_t *num_formats); __owur int tls1_check_ec_tmp_key(SSL *s, unsigned long id); -__owur EVP_PKEY *ssl_generate_pkey_curve(int id); +__owur EVP_PKEY *ssl_generate_pkey_group(uint16_t id); +__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); -__owur int tls1_get_curvelist(SSL *s, int sess, const uint16_t **pcurves, - size_t *num_curves); +void tls1_get_grouplist(SSL *s, int sess, const uint16_t **pcurves, + size_t *num_curves); __owur int tls1_set_server_sigalgs(SSL *s); diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c index 8b88b21..2991310 100644 --- a/ssl/statem/extensions.c +++ b/ssl/statem/extensions.c @@ -1144,18 +1144,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. */ - if (!tls1_get_curvelist(s, 1, &clntcurves, &clnt_num_curves)) { - *al = SSL_AD_INTERNAL_ERROR; - SSLerr(SSL_F_FINAL_KEY_SHARE, ERR_R_INTERNAL_ERROR); - return 0; - } - - /* Get our list of available groups */ - if (!tls1_get_curvelist(s, 0, &pcurves, &num_curves)) { - *al = SSL_AD_INTERNAL_ERROR; - SSLerr(SSL_F_FINAL_KEY_SHARE, ERR_R_INTERNAL_ERROR); - return 0; - } + tls1_get_grouplist(s, 1, &clntcurves, &clnt_num_curves); + tls1_get_grouplist(s, 0, &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 6975b94..047f2d0 100644 --- a/ssl/statem/extensions_clnt.c +++ b/ssl/statem/extensions_clnt.c @@ -149,11 +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 */ - if (!tls1_get_curvelist(s, 0, &pcurves, &num_curves)) { - SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_SUPPORTED_GROUPS, - ERR_R_INTERNAL_ERROR); - return EXT_RETURN_FAIL; - } + tls1_get_grouplist(s, 0, &pcurves, &num_curves); if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_supported_groups) /* Sub-packet for supported_groups extension */ @@ -549,7 +545,7 @@ static int add_key_share(SSL *s, WPACKET *pkt, unsigned int curve_id) */ key_share_key = s->s3->tmp.pkey; } else { - key_share_key = ssl_generate_pkey_curve(curve_id); + key_share_key = ssl_generate_pkey_group(curve_id); if (key_share_key == NULL) { SSLerr(SSL_F_ADD_KEY_SHARE, ERR_R_EVP_LIB); return 0; @@ -608,10 +604,7 @@ EXT_RETURN tls_construct_ctos_key_share(SSL *s, WPACKET *pkt, return EXT_RETURN_FAIL; } - if (!tls1_get_curvelist(s, 0, &pcurves, &num_curves)) { - SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_KEY_SHARE, ERR_R_INTERNAL_ERROR); - return EXT_RETURN_FAIL; - } + tls1_get_grouplist(s, 0, &pcurves, &num_curves); /* * TODO(TLS1.3): Make the number of key_shares sent configurable. For @@ -1541,10 +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 */ - if (!tls1_get_curvelist(s, 0, &pcurves, &num_curves)) { - SSLerr(SSL_F_TLS_PARSE_STOC_KEY_SHARE, ERR_R_INTERNAL_ERROR); - return 0; - } + tls1_get_grouplist(s, 0, &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 583b8dd..3be9754 100644 --- a/ssl/statem/extensions_srvr.c +++ b/ssl/statem/extensions_srvr.c @@ -501,8 +501,7 @@ int tls_parse_ctos_key_share(SSL *s, PACKET *pkt, unsigned int context, X509 *x, PACKET key_share_list, encoded_pt; const uint16_t *clntcurves, *srvrcurves; size_t clnt_num_curves, srvr_num_curves; - int group_nid, found = 0; - unsigned int curve_flags; + int found = 0; if (s->hit && (s->ext.psk_kex_mode & TLSEXT_KEX_MODE_FLAG_KE_DHE) == 0) return 1; @@ -521,18 +520,9 @@ int tls_parse_ctos_key_share(SSL *s, PACKET *pkt, unsigned int context, X509 *x, } /* Get our list of supported curves */ - if (!tls1_get_curvelist(s, 0, &srvrcurves, &srvr_num_curves)) { - *al = SSL_AD_INTERNAL_ERROR; - SSLerr(SSL_F_TLS_PARSE_CTOS_KEY_SHARE, ERR_R_INTERNAL_ERROR); - return 0; - } - + tls1_get_grouplist(s, 0, &srvrcurves, &srvr_num_curves); /* Get the clients list of supported curves. */ - if (!tls1_get_curvelist(s, 1, &clntcurves, &clnt_num_curves)) { - *al = SSL_AD_INTERNAL_ERROR; - SSLerr(SSL_F_TLS_PARSE_CTOS_KEY_SHARE, ERR_R_INTERNAL_ERROR); - return 0; - } + tls1_get_grouplist(s, 1, &clntcurves, &clnt_num_curves); if (clnt_num_curves == 0) { /* * This can only happen if the supported_groups extension was not sent, @@ -575,43 +565,13 @@ int tls_parse_ctos_key_share(SSL *s, PACKET *pkt, unsigned int context, X509 *x, continue; } - group_nid = tls1_ec_curve_id2nid(group_id, &curve_flags); - - if (group_nid == 0) { + if ((s->s3->peer_tmp = ssl_generate_param_group(group_id)) == NULL) { *al = SSL_AD_INTERNAL_ERROR; SSLerr(SSL_F_TLS_PARSE_CTOS_KEY_SHARE, SSL_R_UNABLE_TO_FIND_ECDH_PARAMETERS); return 0; } - if ((curve_flags & TLS_CURVE_TYPE) == TLS_CURVE_CUSTOM) { - /* Can happen for some curves, e.g. X25519 */ - EVP_PKEY *key = EVP_PKEY_new(); - - if (key == NULL || !EVP_PKEY_set_type(key, group_nid)) { - *al = SSL_AD_INTERNAL_ERROR; - SSLerr(SSL_F_TLS_PARSE_CTOS_KEY_SHARE, ERR_R_EVP_LIB); - EVP_PKEY_free(key); - return 0; - } - s->s3->peer_tmp = key; - } else { - /* Set up EVP_PKEY with named curve as parameters */ - EVP_PKEY_CTX *pctx = EVP_PKEY_CTX_new_id(EVP_PKEY_EC, NULL); - - if (pctx == NULL - || EVP_PKEY_paramgen_init(pctx) <= 0 - || EVP_PKEY_CTX_set_ec_paramgen_curve_nid(pctx, - group_nid) <= 0 - || EVP_PKEY_paramgen(pctx, &s->s3->peer_tmp) <= 0) { - *al = SSL_AD_INTERNAL_ERROR; - SSLerr(SSL_F_TLS_PARSE_CTOS_KEY_SHARE, ERR_R_EVP_LIB); - EVP_PKEY_CTX_free(pctx); - return 0; - } - EVP_PKEY_CTX_free(pctx); - pctx = NULL; - } s->s3->group_id = group_id; if (!EVP_PKEY_set1_tls_encodedpoint(s->s3->peer_tmp, @@ -925,7 +885,8 @@ EXT_RETURN tls_construct_stoc_supported_groups(SSL *s, WPACKET *pkt, return EXT_RETURN_NOT_SENT; /* Get our list of supported groups */ - if (!tls1_get_curvelist(s, 0, &groups, &numgroups) || numgroups == 0) { + tls1_get_grouplist(s, 0, &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 5f17864..833450b 100644 --- a/ssl/statem/statem_clnt.c +++ b/ssl/statem/statem_clnt.c @@ -2041,9 +2041,6 @@ 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; - int curve_nid; - unsigned int curve_flags; - EVP_PKEY_CTX *pctx = NULL; /* * Extract elliptic curve parameters and the server's ephemeral ECDH @@ -2065,41 +2062,13 @@ static int tls_process_ske_ecdhe(SSL *s, PACKET *pkt, EVP_PKEY **pkey, int *al) return 0; } - curve_nid = tls1_ec_curve_id2nid(*(ecparams + 2), &curve_flags); - - if (curve_nid == 0) { + if ((s->s3->peer_tmp = ssl_generate_param_group(ecparams[2])) == NULL) { *al = SSL_AD_INTERNAL_ERROR; SSLerr(SSL_F_TLS_PROCESS_SKE_ECDHE, SSL_R_UNABLE_TO_FIND_ECDH_PARAMETERS); return 0; } - if ((curve_flags & TLS_CURVE_TYPE) == TLS_CURVE_CUSTOM) { - EVP_PKEY *key = EVP_PKEY_new(); - - if (key == NULL || !EVP_PKEY_set_type(key, curve_nid)) { - *al = SSL_AD_INTERNAL_ERROR; - SSLerr(SSL_F_TLS_PROCESS_SKE_ECDHE, ERR_R_EVP_LIB); - EVP_PKEY_free(key); - return 0; - } - s->s3->peer_tmp = key; - } else { - /* Set up EVP_PKEY with named curve as parameters */ - pctx = EVP_PKEY_CTX_new_id(EVP_PKEY_EC, NULL); - if (pctx == NULL - || EVP_PKEY_paramgen_init(pctx) <= 0 - || EVP_PKEY_CTX_set_ec_paramgen_curve_nid(pctx, curve_nid) <= 0 - || EVP_PKEY_paramgen(pctx, &s->s3->peer_tmp) <= 0) { - *al = SSL_AD_INTERNAL_ERROR; - SSLerr(SSL_F_TLS_PROCESS_SKE_ECDHE, ERR_R_EVP_LIB); - EVP_PKEY_CTX_free(pctx); - return 0; - } - EVP_PKEY_CTX_free(pctx); - pctx = NULL; - } - if (!PACKET_get_length_prefixed_1(pkt, &encoded_pt)) { *al = SSL_AD_DECODE_ERROR; SSLerr(SSL_F_TLS_PROCESS_SKE_ECDHE, SSL_R_LENGTH_MISMATCH); diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c index 81c8ee4..3e118a8 100644 --- a/ssl/statem/statem_srvr.c +++ b/ssl/statem/statem_srvr.c @@ -2331,7 +2331,6 @@ int tls_construct_server_key_exchange(SSL *s, WPACKET *pkt) #endif #ifndef OPENSSL_NO_EC if (type & (SSL_kECDHE | SSL_kECDHEPSK)) { - int nid; if (s->s3->tmp.pkey != NULL) { SSLerr(SSL_F_TLS_CONSTRUCT_SERVER_KEY_EXCHANGE, @@ -2340,14 +2339,13 @@ int tls_construct_server_key_exchange(SSL *s, WPACKET *pkt) } /* Get NID of appropriate shared curve */ - nid = tls1_shared_group(s, -2); - curve_id = tls1_ec_nid2curve_id(nid); + curve_id = tls1_shared_group(s, -2); if (curve_id == 0) { SSLerr(SSL_F_TLS_CONSTRUCT_SERVER_KEY_EXCHANGE, SSL_R_UNSUPPORTED_ELLIPTIC_CURVE); goto err; } - s->s3->tmp.pkey = ssl_generate_pkey_curve(curve_id); + s->s3->tmp.pkey = ssl_generate_pkey_group(curve_id); /* Generate a new key for this curve */ if (s->s3->tmp.pkey == NULL) { SSLerr(SSL_F_TLS_CONSTRUCT_SERVER_KEY_EXCHANGE, ERR_R_EVP_LIB); diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index 67b06f2..6ca9994 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c @@ -130,18 +130,12 @@ int tls1_clear(SSL *s) #ifndef OPENSSL_NO_EC -typedef struct { - int nid; /* Curve NID */ - int secbits; /* Bits of security (from SP800-57) */ - unsigned int flags; /* Flags: currently just field type */ -} tls_curve_info; - /* * Table of curve information. * Do not delete entries or reorder this array! It is used as a lookup * table: the index of each entry is one less than the TLS curve id. */ -static const tls_curve_info nid_list[] = { +static const TLS_GROUP_INFO nid_list[] = { {NID_sect163k1, 80, TLS_CURVE_CHAR2}, /* sect163k1 (1) */ {NID_sect163r1, 80, TLS_CURVE_CHAR2}, /* sect163r1 (2) */ {NID_sect163r2, 80, TLS_CURVE_CHAR2}, /* sect163r2 (3) */ @@ -192,19 +186,15 @@ static const uint16_t suiteb_curves[] = { TLSEXT_curve_P_384 }; -int tls1_ec_curve_id2nid(uint16_t curve_id, unsigned int *pflags) +const TLS_GROUP_INFO *tls1_group_id_lookup(uint16_t curve_id) { - const tls_curve_info *cinfo; /* ECC curves from RFC 4492 and RFC 7027 */ if (curve_id < 1 || curve_id > OSSL_NELEM(nid_list)) - return 0; - cinfo = nid_list + curve_id - 1; - if (pflags) - *pflags = cinfo->flags; - return cinfo->nid; + return NULL; + return &nid_list[curve_id - 1]; } -uint16_t tls1_ec_nid2curve_id(int nid) +static uint16_t tls1_nid2group_id(int nid) { size_t i; for (i = 0; i < OSSL_NELEM(nid_list); i++) { @@ -218,60 +208,57 @@ uint16_t tls1_ec_nid2curve_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 2 * num_curves. + * 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. */ -int tls1_get_curvelist(SSL *s, int sess, const uint16_t **pcurves, - size_t *num_curves) +void tls1_get_grouplist(SSL *s, int sess, const uint16_t **pcurves, + size_t *pcurveslen) { - size_t pcurveslen = 0; if (sess) { *pcurves = s->session->ext.supportedgroups; - pcurveslen = s->session->ext.supportedgroups_len; - } else { - /* 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); - break; + *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); + break; - case SSL_CERT_FLAG_SUITEB_128_LOS_ONLY: - *pcurves = suiteb_curves; - pcurveslen = 1; - break; + case SSL_CERT_FLAG_SUITEB_128_LOS_ONLY: + *pcurves = suiteb_curves; + *pcurveslen = 1; + break; - case SSL_CERT_FLAG_SUITEB_192_LOS: - *pcurves = suiteb_curves + 2; - pcurveslen = 1; - break; - default: - *pcurves = s->ext.supportedgroups; - pcurveslen = s->ext.supportedgroups_len; - } - if (!*pcurves) { + case SSL_CERT_FLAG_SUITEB_192_LOS: + *pcurves = suiteb_curves + 1; + *pcurveslen = 1; + break; + + default: + if (s->ext.supportedgroups == NULL) { *pcurves = eccurves_default; - pcurveslen = OSSL_NELEM(eccurves_default); + *pcurveslen = OSSL_NELEM(eccurves_default); + } else { + *pcurves = s->ext.supportedgroups; + *pcurveslen = s->ext.supportedgroups_len; } + break; } - - *num_curves = pcurveslen; - return 1; } /* See if curve is allowed by security callback */ int tls_curve_allowed(SSL *s, uint16_t curve, int op) { - const tls_curve_info *cinfo; + const TLS_GROUP_INFO *cinfo = tls1_group_id_lookup(curve); unsigned char ctmp[2]; - if (curve > 0xff) - return 1; - if (curve < 1 || curve > OSSL_NELEM(nid_list)) + + if (cinfo == NULL) return 0; - cinfo = &nid_list[curve - 1]; # ifdef OPENSSL_NO_EC2M if (cinfo->flags & TLS_CURVE_CHAR2) return 0; @@ -281,18 +268,28 @@ int tls_curve_allowed(SSL *s, uint16_t curve, int op) return ssl_security(s, op, cinfo->secbits, cinfo->nid, (void *)ctmp); } +/* Return 1 if "id" is in "list" */ +static int tls1_in_list(uint16_t id, const uint16_t *list, size_t listlen) +{ + size_t i; + for (i = 0; i < listlen; i++) + if (list[i] == id) + return 1; + 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; - size_t num_curves, i; - unsigned int suiteb_flags = tls1_suiteb(s); + if (len != 3 || p[0] != NAMED_CURVE_TYPE) return 0; curve_id = (p[1] << 8) | p[2]; /* Check curve matches Suite B preferences */ - if (suiteb_flags) { + 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) @@ -303,31 +300,28 @@ int tls1_check_curve(SSL *s, const unsigned char *p, size_t len) } else /* Should never happen */ return 0; } - if (!tls1_get_curvelist(s, 0, &curves, &num_curves)) + tls1_get_grouplist(s, 0, &curves, &num_curves); + if (!tls1_in_list(curve_id, curves, num_curves)) return 0; - for (i = 0; i < num_curves; i++) { - if (curve_id == curves[i]) - return tls_curve_allowed(s, curve_id, SSL_SECOP_CURVE_CHECK); - } - return 0; + return tls_curve_allowed(s, curve_id, SSL_SECOP_CURVE_CHECK); } /*- - * For nmatch >= 0, return the NID of the |nmatch|th shared group or NID_undef + * For nmatch >= 0, return the id of the |nmatch|th shared group or 0 * if there is no match. * For nmatch == -1, return number of matches - * For nmatch == -2, return the NID of the group to use for - * an EC tmp key, or NID_undef if there is no match. + * For nmatch == -2, return the id of the group to use for + * a tmp key, or 0 if there is no match. */ -int tls1_shared_group(SSL *s, int nmatch) +uint16_t tls1_shared_group(SSL *s, int nmatch) { const uint16_t *pref, *supp; - size_t num_pref, num_supp, i, j; + size_t num_pref, num_supp, i; int k; /* Can't do anything on client side */ if (s->server == 0) - return -1; + return 0; if (nmatch == -2) { if (tls1_suiteb(s)) { /* @@ -337,46 +331,40 @@ int tls1_shared_group(SSL *s, int nmatch) unsigned long cid = s->s3->tmp.new_cipher->id; if (cid == TLS1_CK_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256) - return NID_X9_62_prime256v1; /* P-256 */ + return TLSEXT_curve_P_256; if (cid == TLS1_CK_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384) - return NID_secp384r1; /* P-384 */ + return TLSEXT_curve_P_384; /* Should never happen */ - return NID_undef; + return 0; } /* If not Suite B just return first preference shared curve */ nmatch = 0; } /* - * Avoid truncation. tls1_get_curvelist takes an int + * Avoid truncation. tls1_get_grouplist takes an int * but s->options is a long... */ - if (!tls1_get_curvelist(s, + tls1_get_grouplist(s, (s->options & SSL_OP_CIPHER_SERVER_PREFERENCE) != 0, - &supp, &num_supp)) - /* In practice, NID_undef == 0 but let's be precise. */ - return nmatch == -1 ? 0 : NID_undef; - if (!tls1_get_curvelist(s, + &supp, &num_supp); + tls1_get_grouplist(s, (s->options & SSL_OP_CIPHER_SERVER_PREFERENCE) == 0, - &pref, &num_pref)) - return nmatch == -1 ? 0 : NID_undef; + &pref, &num_pref); for (k = 0, i = 0; i < num_pref; i++) { uint16_t id = pref[i]; - for (j = 0; j < num_supp; j++) { - if (id == supp[j]) { - if (!tls_curve_allowed(s, id, SSL_SECOP_CURVE_SHARED)) + if (!tls1_in_list(id, supp, num_supp) + || !tls_curve_allowed(s, id, SSL_SECOP_CURVE_SHARED)) continue; - if (nmatch == k) - return tls1_ec_curve_id2nid(id, NULL); - k++; - } - } + if (nmatch == k) + return id; + k++; } if (nmatch == -1) return k; /* Out of range (nmatch > k). */ - return NID_undef; + return 0; } int tls1_set_groups(uint16_t **pext, size_t *pextlen, @@ -396,7 +384,7 @@ int tls1_set_groups(uint16_t **pext, size_t *pextlen, unsigned long idmask; uint16_t id; /* TODO(TLS1.3): Convert for DH groups */ - id = tls1_ec_nid2curve_id(groups[i]); + id = tls1_nid2group_id(groups[i]); idmask = 1L << id; if (!id || (dup_list & idmask)) { OPENSSL_free(glist); @@ -457,88 +445,95 @@ int tls1_set_groups_list(uint16_t **pext, size_t *pextlen, const char *str) return 1; return tls1_set_groups(pext, pextlen, ncb.nid_arr, ncb.nidcnt); } - -/* For an EC key set TLS id and required compression based on parameters */ -static int tls1_set_ec_id(uint16_t *pcurve_id, unsigned char *comp_id, - EC_KEY *ec) +/* Return group id of a key */ +static uint16_t tls1_get_group_id(EVP_PKEY *pkey) { - int curve_nid; + EC_KEY *ec = EVP_PKEY_get0_EC_KEY(pkey); const EC_GROUP *grp; - if (!ec) + + if (ec == NULL) return 0; - /* Determine if it is a prime field */ grp = EC_KEY_get0_group(ec); - if (!grp) - return 0; - /* Determine curve ID */ - curve_nid = EC_GROUP_get_curve_name(grp); - *pcurve_id = tls1_ec_nid2curve_id(curve_nid); - /* If no id return error: we don't support arbitrary explicit curves */ - if (*pcurve_id == 0) - return 0; - if (comp_id) { - if (EC_KEY_get0_public_key(ec) == NULL) - return 0; - if (EC_KEY_get_conv_form(ec) == POINT_CONVERSION_UNCOMPRESSED) { - *comp_id = TLSEXT_ECPOINTFORMAT_uncompressed; - } else { - if ((nid_list[*pcurve_id - 1].flags & TLS_CURVE_TYPE) == TLS_CURVE_PRIME) - *comp_id = TLSEXT_ECPOINTFORMAT_ansiX962_compressed_prime; - else - *comp_id = TLSEXT_ECPOINTFORMAT_ansiX962_compressed_char2; - } - } - return 1; + return tls1_nid2group_id(EC_GROUP_get_curve_name(grp)); } -/* Check an EC key is compatible with extensions */ -static int tls1_check_ec_key(SSL *s, uint16_t curve_id, unsigned char *comp_id) +/* Check a key is compatible with compression extension */ +static int tls1_check_pkey_comp(SSL *s, EVP_PKEY *pkey) { - const unsigned char *pformats; - const uint16_t *pcurves; - size_t num_formats, num_curves, i; - int j; + const EC_KEY *ec; + const EC_GROUP *grp; + unsigned char comp_id; + size_t i; + + /* If not an EC key nothing to check */ + if (EVP_PKEY_id(pkey) != EVP_PKEY_EC) + return 1; + ec = EVP_PKEY_get0_EC_KEY(pkey); + grp = EC_KEY_get0_group(ec); + + /* Get required compression id */ + if (EC_KEY_get_conv_form(ec) == POINT_CONVERSION_UNCOMPRESSED) { + comp_id = TLSEXT_ECPOINTFORMAT_uncompressed; + } else if (SSL_IS_TLS13(s)) { + /* Compression not allowed in TLS 1.3 */ + return 0; + } else { + int field_type = EC_METHOD_get_field_type(EC_GROUP_method_of(grp)); + + if (field_type == NID_X9_62_prime_field) + comp_id = TLSEXT_ECPOINTFORMAT_ansiX962_compressed_prime; + else if (field_type == NID_X9_62_prime_field) + comp_id = TLSEXT_ECPOINTFORMAT_ansiX962_compressed_char2; + else + return 0; + } /* * If point formats extension present check it, otherwise everything is * supported (see RFC4492). */ - if (comp_id && s->session->ext.ecpointformats) { - pformats = s->session->ext.ecpointformats; - num_formats = s->session->ext.ecpointformats_len; - for (i = 0; i < num_formats; i++, pformats++) { - if (*comp_id == *pformats) - break; - } - if (i == num_formats) - return 0; - } - if (curve_id == 0) + if (s->session->ext.ecpointformats == NULL) return 1; - /* Check curve is consistent with client and server preferences */ - for (j = 0; j <= 1; j++) { - if (!tls1_get_curvelist(s, j, &pcurves, &num_curves)) - return 0; - if (j == 1 && num_curves == 0) { - /* - * If we've not received any curves then skip this check. - * RFC 4492 does not require the supported elliptic curves extension - * so if it is not sent we can just choose any curve. - * It is invalid to send an empty list in the elliptic curves - * extension, so num_curves == 0 always means no extension. - */ - break; - } - for (i = 0; i < num_curves; i++) { - if (pcurves[i] == curve_id) - break; - } - if (i == num_curves) - return 0; - /* For clients can only check sent curve list */ - if (!s->server) - break; + + for (i = 0; i < s->session->ext.ecpointformats_len; i++) { + if (s->session->ext.ecpointformats[i] == comp_id) + return 1; } - return 1; + return 0; +} + +/* Check a group id matches preferences */ +static int tls1_check_group_id(SSL *s, uint16_t group_id) + { + const uint16_t *groups; + size_t groups_len; + + if (group_id == 0) + return 0; + + if (!tls_curve_allowed(s, group_id, SSL_SECOP_CURVE_CHECK)) + return 0; + + /* Check group is one of our preferences */ + tls1_get_grouplist(s, 0, &groups, &groups_len); + if (!tls1_in_list(group_id, groups, groups_len)) + 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); + + /* + * RFC 4492 does not require the supported elliptic curves extension + * so if it is not sent we can just choose any curve. + * It is invalid to send an empty list in the supported groups + * extension, so groups_len == 0 always means no extension. + */ + if (groups_len == 0) + return 1; + return tls1_in_list(group_id, groups, groups_len); } void tls1_get_formatlist(SSL *s, const unsigned char **pformats, @@ -566,25 +561,19 @@ void tls1_get_formatlist(SSL *s, const unsigned char **pformats, */ static int tls1_check_cert_param(SSL *s, X509 *x, int check_ee_md) { - unsigned char comp_id; - uint16_t curve_id; + uint16_t group_id; EVP_PKEY *pkey; - int rv; pkey = X509_get0_pubkey(x); - if (!pkey) + if (pkey == NULL) return 0; /* If not EC nothing to do */ if (EVP_PKEY_id(pkey) != EVP_PKEY_EC) return 1; - rv = tls1_set_ec_id(&curve_id, &comp_id, EVP_PKEY_get0_EC_KEY(pkey)); - if (!rv) + /* Check compression */ + if (!tls1_check_pkey_comp(s, pkey)) return 0; - /* - * Can't check curve_id for client certs as we don't have a supported - * curves extension. - */ - rv = tls1_check_ec_key(s, s->server ? curve_id : 0, &comp_id); - if (!rv) + group_id = tls1_get_group_id(pkey); + if (!tls1_check_group_id(s, group_id)) return 0; /* * Special case for suite B. We *MUST* sign using SHA256+P-256 or @@ -596,19 +585,19 @@ static int tls1_check_cert_param(SSL *s, X509 *x, int check_ee_md) CERT *c = s->cert; /* Check to see we have necessary signing algorithm */ - if (curve_id == TLSEXT_curve_P_256) + if (group_id == TLSEXT_curve_P_256) check_md = NID_ecdsa_with_SHA256; - else if (curve_id == TLSEXT_curve_P_384) + else if (group_id == TLSEXT_curve_P_384) check_md = NID_ecdsa_with_SHA384; else return 0; /* Should never happen */ - for (i = 0; i < c->shared_sigalgslen; i++) + for (i = 0; i < c->shared_sigalgslen; i++) { if (check_md == c->shared_sigalgs[i]->sigandhash) - break; - if (i == c->shared_sigalgslen) - return 0; + return 1;; + } + return 0; } - return rv; + return 1; } /* @@ -623,28 +612,18 @@ static int tls1_check_cert_param(SSL *s, X509 *x, int check_ee_md) */ int tls1_check_ec_tmp_key(SSL *s, unsigned long cid) { + /* If not Suite B just need a shared group */ + if (!tls1_suiteb(s)) + return tls1_shared_group(s, 0) != 0; /* * If Suite B, AES128 MUST use P-256 and AES256 MUST use P-384, no other * curves permitted. */ - if (tls1_suiteb(s)) { - uint16_t curve_id; + if (cid == TLS1_CK_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256) + return tls1_check_group_id(s, TLSEXT_curve_P_256); + if (cid == TLS1_CK_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384) + return tls1_check_group_id(s, TLSEXT_curve_P_384); - /* Curve to check determined by ciphersuite */ - if (cid == TLS1_CK_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256) - curve_id = TLSEXT_curve_P_256; - else if (cid == TLS1_CK_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384) - curve_id = TLSEXT_curve_P_384; - else - return 0; - /* Check this curve is acceptable */ - if (!tls1_check_ec_key(s, curve_id, NULL)) - return 0; - return 1; - } - /* Need a shared curve */ - if (tls1_shared_group(s, 0)) - return 1; return 0; } @@ -957,28 +936,27 @@ int tls12_check_peer_sigalg(SSL *s, uint16_t sig, EVP_PKEY *pkey) } #ifndef OPENSSL_NO_EC if (pkeyid == EVP_PKEY_EC) { - EC_KEY *ec = EVP_PKEY_get0_EC_KEY(pkey); - int curve = EC_GROUP_get_curve_name(EC_KEY_get0_group(ec)); - if (SSL_IS_TLS13(s)) { - if (EC_KEY_get_conv_form(ec) != POINT_CONVERSION_UNCOMPRESSED) { - SSLerr(SSL_F_TLS12_CHECK_PEER_SIGALG, - SSL_R_ILLEGAL_POINT_COMPRESSION); - return 0; - } - /* For TLS 1.3 check curve matches signature algorithm */ + /* Check point compression is permitted */ + if (!tls1_check_pkey_comp(s, pkey)) { + SSLerr(SSL_F_TLS12_CHECK_PEER_SIGALG, + SSL_R_ILLEGAL_POINT_COMPRESSION); + return 0; + } + + /* For TLS 1.3 or Suite B check curve matches signature algorithm */ + if (SSL_IS_TLS13(s) || tls1_suiteb(s)) { + EC_KEY *ec = EVP_PKEY_get0_EC_KEY(pkey); + int curve = EC_GROUP_get_curve_name(EC_KEY_get0_group(ec)); + if (lu->curve != NID_undef && curve != lu->curve) { SSLerr(SSL_F_TLS12_CHECK_PEER_SIGALG, SSL_R_WRONG_CURVE); return 0; } - } else { - unsigned char comp_id; - uint16_t curve_id; - - /* Check compression and curve matches extensions */ - if (!tls1_set_ec_id(&curve_id, &comp_id, ec)) - return 0; - if (!s->server && !tls1_check_ec_key(s, curve_id, &comp_id)) { + } + if (!SSL_IS_TLS13(s)) { + /* Check curve matches extensions */ + if (!tls1_check_group_id(s, tls1_get_group_id(pkey))) { SSLerr(SSL_F_TLS12_CHECK_PEER_SIGALG, SSL_R_WRONG_CURVE); return 0; } @@ -990,17 +968,6 @@ int tls12_check_peer_sigalg(SSL *s, uint16_t sig, EVP_PKEY *pkey) SSL_R_WRONG_SIGNATURE_TYPE); return 0; } - /* - * Suite B also requires P-256+SHA256 and P-384+SHA384: - * this matches the TLS 1.3 requirements so we can just - * check the curve is the expected TLS 1.3 value. - * If this fails an inappropriate digest is being used. - */ - if (curve != lu->curve) { - SSLerr(SSL_F_TLS12_CHECK_PEER_SIGALG, - SSL_R_ILLEGAL_SUITEB_DIGEST); - return 0; - } } } } else if (tls1_suiteb(s)) { _____ openssl-commits mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-commits