From 9e84a42db497e06a38f804b5acd09b6aa4f87db3 Mon Sep 17 00:00:00 2001 From: "Dr. Stephen Henson" Date: Fri, 22 Sep 2017 16:06:52 +0100 Subject: [PATCH] Store groups as uint16_t Instead of storing supported groups in on-the-wire format store them as parsed uint16_t values. This simplifies handling of groups as the values can be directly used instead of being converted. Reviewed-by: Rich Salz (Merged from https://github.com/openssl/openssl/pull/4406) --- ssl/s3_lib.c | 10 +- ssl/ssl_lib.c | 3 +- ssl/ssl_locl.h | 23 ++-- ssl/statem/extensions.c | 7 +- ssl/statem/extensions_clnt.c | 30 +++--- ssl/statem/extensions_srvr.c | 16 +-- ssl/statem/statem_lib.c | 10 +- ssl/statem/statem_locl.h | 5 +- ssl/t1_lib.c | 201 ++++++++++++++++++----------------- 9 files changed, 152 insertions(+), 153 deletions(-) diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c index 8895388576..4127b28ea4 100644 --- a/ssl/s3_lib.c +++ b/ssl/s3_lib.c @@ -3602,25 +3602,23 @@ long ssl3_ctrl(SSL *s, int cmd, long larg, void *parg) #ifndef OPENSSL_NO_EC case SSL_CTRL_GET_GROUPS: { - unsigned char *clist; + uint16_t *clist; size_t clistlen; if (!s->session) return 0; clist = s->session->ext.supportedgroups; - clistlen = s->session->ext.supportedgroups_len / 2; + clistlen = s->session->ext.supportedgroups_len; if (parg) { size_t i; int *cptr = parg; - unsigned int cid, nid; for (i = 0; i < clistlen; i++) { - n2s(clist, cid); /* TODO(TLS1.3): Handle DH groups here */ - nid = tls1_ec_curve_id2nid(cid, NULL); + int nid = tls1_ec_curve_id2nid(clist[i], NULL); if (nid != 0) cptr[i] = nid; else - cptr[i] = TLSEXT_nid_unknown | cid; + cptr[i] = TLSEXT_nid_unknown | clist[i]; } } return (int)clistlen; diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c index 711846dc01..48ce7c179c 100644 --- a/ssl/ssl_lib.c +++ b/ssl/ssl_lib.c @@ -719,7 +719,8 @@ SSL *SSL_new(SSL_CTX *ctx) if (ctx->ext.supportedgroups) { s->ext.supportedgroups = OPENSSL_memdup(ctx->ext.supportedgroups, - ctx->ext.supportedgroups_len); + ctx->ext.supportedgroups_len + * sizeof(ctx->ext.supportedgroups)); if (!s->ext.supportedgroups) goto err; s->ext.supportedgroups_len = ctx->ext.supportedgroups_len; diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h index 95e60414e2..5eda6362a1 100644 --- a/ssl/ssl_locl.h +++ b/ssl/ssl_locl.h @@ -543,7 +543,7 @@ struct ssl_session_st { size_t ecpointformats_len; unsigned char *ecpointformats; /* peer's list */ size_t supportedgroups_len; - unsigned char *supportedgroups; /* peer's list */ + uint16_t *supportedgroups; /* peer's list */ # endif /* OPENSSL_NO_EC */ /* RFC4507 info */ unsigned char *tick; /* Session ticket */ @@ -902,7 +902,7 @@ struct ssl_ctx_st { size_t ecpointformats_len; unsigned char *ecpointformats; size_t supportedgroups_len; - unsigned char *supportedgroups; + uint16_t *supportedgroups; # endif /* OPENSSL_NO_EC */ /* @@ -1205,7 +1205,7 @@ struct ssl_st { unsigned char *ecpointformats; size_t supportedgroups_len; /* our list */ - unsigned char *supportedgroups; + uint16_t *supportedgroups; # endif /* OPENSSL_NO_EC */ /* TLS Session Ticket extension override */ TLS_SESSION_TICKET_EXT *session_ticket; @@ -1523,7 +1523,7 @@ typedef struct ssl3_state_st { /* For clients: peer temporary key */ # if !defined(OPENSSL_NO_EC) || !defined(OPENSSL_NO_DH) /* The group_id for the DH/ECDH key */ - unsigned int group_id; + uint16_t group_id; EVP_PKEY *peer_tmp; # endif @@ -2333,15 +2333,13 @@ SSL_COMP *ssl3_comp_find(STACK_OF(SSL_COMP) *sk, int n); # define TLS_CURVE_CHAR2 0x1 # define TLS_CURVE_CUSTOM 0x2 -#define bytestogroup(bytes) ((unsigned int)(bytes[0] << 8 | bytes[1])) - -__owur int tls1_ec_curve_id2nid(int curve_id, unsigned int *pflags); -__owur int tls1_ec_nid2curve_id(int nid); +__owur int tls1_ec_curve_id2nid(uint16_t curve_id, unsigned int *pflags); +__owur uint16_t tls1_ec_nid2curve_id(int nid); __owur int tls1_check_curve(SSL *s, const unsigned char *p, size_t len); __owur int tls1_shared_group(SSL *s, int nmatch); -__owur int tls1_set_groups(unsigned char **pext, size_t *pextlen, +__owur int tls1_set_groups(uint16_t **pext, size_t *pextlen, int *curves, size_t ncurves); -__owur int tls1_set_groups_list(unsigned char **pext, size_t *pextlen, +__owur int tls1_set_groups_list(uint16_t **pext, size_t *pextlen, const char *str); void tls1_get_formatlist(SSL *s, const unsigned char **pformats, size_t *num_formats); @@ -2349,8 +2347,8 @@ __owur int tls1_check_ec_tmp_key(SSL *s, unsigned long id); __owur EVP_PKEY *ssl_generate_pkey_curve(int id); # endif /* OPENSSL_NO_EC */ -__owur int tls_curve_allowed(SSL *s, const unsigned char *curve, int op); -__owur int tls1_get_curvelist(SSL *s, int sess, const unsigned char **pcurves, +__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); __owur int tls1_set_server_sigalgs(SSL *s); @@ -2410,6 +2408,7 @@ void ssl_clear_hash_ctx(EVP_MD_CTX **hash); __owur long ssl_get_algorithm2(SSL *s); __owur int tls12_copy_sigalgs(SSL *s, WPACKET *pkt, const uint16_t *psig, size_t psiglen); +__owur int tls1_save_u16(PACKET *pkt, uint16_t **pdest, size_t *pdestlen); __owur int tls1_save_sigalgs(SSL *s, PACKET *pkt); __owur int tls1_process_sigalgs(SSL *s); __owur int tls1_set_peer_legacy_sigalg(SSL *s, const EVP_PKEY *pkey); diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c index 61203ed6a3..8b88b21584 100644 --- a/ssl/statem/extensions.c +++ b/ssl/statem/extensions.c @@ -1137,7 +1137,7 @@ 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 unsigned char *pcurves, *pcurvestmp, *clntcurves; + const uint16_t *pcurves, *clntcurves; size_t num_curves, clnt_num_curves, i; unsigned int group_id = 0; @@ -1158,9 +1158,8 @@ static int final_key_share(SSL *s, unsigned int context, int sent, int *al) } /* Find the first group we allow that is also in client's list */ - for (i = 0, pcurvestmp = pcurves; i < num_curves; - i++, pcurvestmp += 2) { - group_id = bytestogroup(pcurvestmp); + for (i = 0; i < num_curves; i++) { + group_id = pcurves[i]; if (check_in_list(s, group_id, clntcurves, clnt_num_curves, 1)) break; diff --git a/ssl/statem/extensions_clnt.c b/ssl/statem/extensions_clnt.c index bffe7aca08..6975b940f7 100644 --- a/ssl/statem/extensions_clnt.c +++ b/ssl/statem/extensions_clnt.c @@ -139,7 +139,7 @@ EXT_RETURN tls_construct_ctos_supported_groups(SSL *s, WPACKET *pkt, unsigned int context, X509 *x, size_t chainidx, int *al) { - const unsigned char *pcurves = NULL, *pcurvestmp; + const uint16_t *pcurves = NULL; size_t num_curves = 0, i; if (!use_ecc(s)) @@ -154,7 +154,6 @@ EXT_RETURN tls_construct_ctos_supported_groups(SSL *s, WPACKET *pkt, ERR_R_INTERNAL_ERROR); return EXT_RETURN_FAIL; } - pcurvestmp = pcurves; if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_supported_groups) /* Sub-packet for supported_groups extension */ @@ -165,10 +164,11 @@ 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++, pcurvestmp += 2) { - if (tls_curve_allowed(s, pcurvestmp, SSL_SECOP_CURVE_SUPPORTED)) { - if (!WPACKET_put_bytes_u8(pkt, pcurvestmp[0]) - || !WPACKET_put_bytes_u8(pkt, pcurvestmp[1])) { + for (i = 0; i < num_curves; i++) { + uint16_t ctmp = pcurves[i]; + + if (tls_curve_allowed(s, ctmp, SSL_SECOP_CURVE_SUPPORTED)) { + if (!WPACKET_put_bytes_u16(pkt, ctmp)) { SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_SUPPORTED_GROUPS, ERR_R_INTERNAL_ERROR); return EXT_RETURN_FAIL; @@ -595,8 +595,8 @@ EXT_RETURN tls_construct_ctos_key_share(SSL *s, WPACKET *pkt, { #ifndef OPENSSL_NO_TLS1_3 size_t i, num_curves = 0; - const unsigned char *pcurves = NULL; - unsigned int curve_id = 0; + const uint16_t *pcurves = NULL; + uint16_t curve_id = 0; /* key_share extension */ if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_key_share) @@ -620,12 +620,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++, pcurves += 2) { + for (i = 0; i < num_curves; i++) { - if (!tls_curve_allowed(s, pcurves, SSL_SECOP_CURVE_SUPPORTED)) + if (!tls_curve_allowed(s, pcurves[i], SSL_SECOP_CURVE_SUPPORTED)) continue; - curve_id = bytestogroup(pcurves); + curve_id = pcurves[i]; break; } } @@ -1521,7 +1521,7 @@ 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) { - unsigned const char *pcurves = NULL; + const uint16_t *pcurves = NULL; size_t i, num_curves; if (PACKET_remaining(pkt) != 0) { @@ -1545,12 +1545,12 @@ int tls_parse_stoc_key_share(SSL *s, PACKET *pkt, unsigned int context, X509 *x, SSLerr(SSL_F_TLS_PARSE_STOC_KEY_SHARE, ERR_R_INTERNAL_ERROR); return 0; } - for (i = 0; i < num_curves; i++, pcurves += 2) { - if (group_id == bytestogroup(pcurves)) + for (i = 0; i < num_curves; i++) { + if (group_id == pcurves[i]) break; } if (i >= num_curves - || !tls_curve_allowed(s, pcurves, SSL_SECOP_CURVE_SUPPORTED)) { + || !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); return 0; diff --git a/ssl/statem/extensions_srvr.c b/ssl/statem/extensions_srvr.c index 0dbec91303..583b8ddda4 100644 --- a/ssl/statem/extensions_srvr.c +++ b/ssl/statem/extensions_srvr.c @@ -499,7 +499,7 @@ 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 unsigned char *clntcurves, *srvrcurves; + const uint16_t *clntcurves, *srvrcurves; size_t clnt_num_curves, srvr_num_curves; int group_nid, found = 0; unsigned int curve_flags; @@ -647,7 +647,7 @@ int tls_parse_ctos_supported_groups(SSL *s, PACKET *pkt, unsigned int context, OPENSSL_free(s->session->ext.supportedgroups); s->session->ext.supportedgroups = NULL; s->session->ext.supportedgroups_len = 0; - if (!PACKET_memdup(&supported_groups_list, + if (!tls1_save_u16(&supported_groups_list, &s->session->ext.supportedgroups, &s->session->ext.supportedgroups_len)) { *al = SSL_AD_INTERNAL_ERROR; @@ -917,7 +917,7 @@ EXT_RETURN tls_construct_stoc_supported_groups(SSL *s, WPACKET *pkt, unsigned int context, X509 *x, size_t chainidx, int *al) { - const unsigned char *groups; + const uint16_t *groups; size_t numgroups, i, first = 1; /* s->s3->group_id is non zero if we accepted a key_share */ @@ -931,14 +931,16 @@ EXT_RETURN tls_construct_stoc_supported_groups(SSL *s, WPACKET *pkt, } /* Copy group ID if supported */ - for (i = 0; i < numgroups; i++, groups += 2) { - if (tls_curve_allowed(s, groups, SSL_SECOP_CURVE_SUPPORTED)) { + for (i = 0; i < numgroups; i++) { + uint16_t group = groups[i]; + + if (tls_curve_allowed(s, group, SSL_SECOP_CURVE_SUPPORTED)) { if (first) { /* * Check if the client is already using our preferred group. If * so we don't need to add this extension */ - if (s->s3->group_id == GET_GROUP_ID(groups, 0)) + if (s->s3->group_id == group) return EXT_RETURN_NOT_SENT; /* Add extension header */ @@ -953,7 +955,7 @@ EXT_RETURN tls_construct_stoc_supported_groups(SSL *s, WPACKET *pkt, first = 0; } - if (!WPACKET_put_bytes_u16(pkt, GET_GROUP_ID(groups, 0))) { + if (!WPACKET_put_bytes_u16(pkt, group)) { SSLerr(SSL_F_TLS_CONSTRUCT_STOC_SUPPORTED_GROUPS, ERR_R_INTERNAL_ERROR); return EXT_RETURN_FAIL; diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c index d2962437c6..84ad2f6f25 100644 --- a/ssl/statem/statem_lib.c +++ b/ssl/statem/statem_lib.c @@ -1964,7 +1964,7 @@ int ssl_set_client_hello_version(SSL *s) * 1) or 0 otherwise. */ #ifndef OPENSSL_NO_EC -int check_in_list(SSL *s, unsigned int group_id, const unsigned char *groups, +int check_in_list(SSL *s, uint16_t group_id, const uint16_t *groups, size_t num_groups, int checkallow) { size_t i; @@ -1972,10 +1972,12 @@ int check_in_list(SSL *s, unsigned int group_id, const unsigned char *groups, if (groups == NULL || num_groups == 0) return 0; - for (i = 0; i < num_groups; i++, groups += 2) { - if (group_id == GET_GROUP_ID(groups, 0) + for (i = 0; i < num_groups; i++) { + uint16_t group = groups[i]; + + if (group_id == group && (!checkallow - || tls_curve_allowed(s, groups, SSL_SECOP_CURVE_CHECK))) { + || tls_curve_allowed(s, group, SSL_SECOP_CURVE_CHECK))) { return 1; } } diff --git a/ssl/statem/statem_locl.h b/ssl/statem/statem_locl.h index ae33fe5e5c..9b76dc021a 100644 --- a/ssl/statem/statem_locl.h +++ b/ssl/statem/statem_locl.h @@ -55,10 +55,7 @@ int statem_flush(SSL *s); typedef int (*confunc_f) (SSL *s, WPACKET *pkt); -#define GET_GROUP_ID(group, idx) \ - (unsigned int)(((group)[(idx) * 2] << 8) | (group)[((idx) * 2) + 1]) - -int check_in_list(SSL *s, unsigned int group_id, const unsigned char *groups, +int check_in_list(SSL *s, uint16_t group_id, const uint16_t *groups, size_t num_groups, int checkallow); int create_synthetic_message_hash(SSL *s); int parse_ca_names(SSL *s, PACKET *pkt, int *al); diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index de13de6ba6..fd26595007 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c @@ -180,23 +180,23 @@ static const unsigned char ecformats_default[] = { }; /* The default curves */ -static const unsigned char eccurves_default[] = { - 0, 29, /* X25519 (29) */ - 0, 23, /* secp256r1 (23) */ - 0, 25, /* secp521r1 (25) */ - 0, 24, /* secp384r1 (24) */ +static const uint16_t eccurves_default[] = { + 29, /* X25519 (29) */ + 23, /* secp256r1 (23) */ + 25, /* secp521r1 (25) */ + 24, /* secp384r1 (24) */ }; -static const unsigned char suiteb_curves[] = { - 0, TLSEXT_curve_P_256, - 0, TLSEXT_curve_P_384 +static const uint16_t suiteb_curves[] = { + TLSEXT_curve_P_256, + TLSEXT_curve_P_384 }; -int tls1_ec_curve_id2nid(int curve_id, unsigned int *pflags) +int tls1_ec_curve_id2nid(uint16_t curve_id, unsigned int *pflags) { const tls_curve_info *cinfo; /* ECC curves from RFC 4492 and RFC 7027 */ - if ((curve_id < 1) || ((unsigned int)curve_id > OSSL_NELEM(nid_list))) + if (curve_id < 1 || curve_id > OSSL_NELEM(nid_list)) return 0; cinfo = nid_list + curve_id - 1; if (pflags) @@ -204,12 +204,12 @@ int tls1_ec_curve_id2nid(int curve_id, unsigned int *pflags) return cinfo->nid; } -int tls1_ec_nid2curve_id(int nid) +uint16_t tls1_ec_nid2curve_id(int nid) { size_t i; for (i = 0; i < OSSL_NELEM(nid_list); i++) { if (nid_list[i].nid == nid) - return (int)(i + 1); + return i + 1; } return 0; } @@ -222,11 +222,8 @@ int tls1_ec_nid2curve_id(int nid) * 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. - * TODO(emilia): we should really be storing the curves list in explicitly - * parsed form instead. (However, this would affect binary compatibility - * so cannot happen in the 1.0.x series.) */ -int tls1_get_curvelist(SSL *s, int sess, const unsigned char **pcurves, +int tls1_get_curvelist(SSL *s, int sess, const uint16_t **pcurves, size_t *num_curves) { size_t pcurveslen = 0; @@ -239,17 +236,17 @@ int tls1_get_curvelist(SSL *s, int sess, const unsigned char **pcurves, switch (tls1_suiteb(s)) { case SSL_CERT_FLAG_SUITEB_128_LOS: *pcurves = suiteb_curves; - pcurveslen = sizeof(suiteb_curves); + pcurveslen = OSSL_NELEM(suiteb_curves); break; case SSL_CERT_FLAG_SUITEB_128_LOS_ONLY: *pcurves = suiteb_curves; - pcurveslen = 2; + pcurveslen = 1; break; case SSL_CERT_FLAG_SUITEB_192_LOS: *pcurves = suiteb_curves + 2; - pcurveslen = 2; + pcurveslen = 1; break; default: *pcurves = s->ext.supportedgroups; @@ -257,63 +254,60 @@ int tls1_get_curvelist(SSL *s, int sess, const unsigned char **pcurves, } if (!*pcurves) { *pcurves = eccurves_default; - pcurveslen = sizeof(eccurves_default); + pcurveslen = OSSL_NELEM(eccurves_default); } } - /* We do not allow odd length arrays to enter the system. */ - if (pcurveslen & 1) { - SSLerr(SSL_F_TLS1_GET_CURVELIST, ERR_R_INTERNAL_ERROR); - *num_curves = 0; - return 0; - } - *num_curves = pcurveslen / 2; + *num_curves = pcurveslen; return 1; } /* See if curve is allowed by security callback */ -int tls_curve_allowed(SSL *s, const unsigned char *curve, int op) +int tls_curve_allowed(SSL *s, uint16_t curve, int op) { const tls_curve_info *cinfo; - if (curve[0]) + unsigned char ctmp[2]; + if (curve > 0xff) return 1; - if ((curve[1] < 1) || ((size_t)curve[1] > OSSL_NELEM(nid_list))) + if (curve < 1 || curve > OSSL_NELEM(nid_list)) return 0; - cinfo = &nid_list[curve[1] - 1]; + cinfo = &nid_list[curve - 1]; # ifdef OPENSSL_NO_EC2M if (cinfo->flags & TLS_CURVE_CHAR2) return 0; # endif - return ssl_security(s, op, cinfo->secbits, cinfo->nid, (void *)curve); + ctmp[0] = curve >> 8; + ctmp[1] = curve & 0xff; + return ssl_security(s, op, cinfo->secbits, cinfo->nid, (void *)ctmp); } /* Check a curve is one of our preferences */ int tls1_check_curve(SSL *s, const unsigned char *p, size_t len) { - const unsigned char *curves; + const uint16_t *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) { unsigned long cid = s->s3->tmp.new_cipher->id; - if (p[1]) - return 0; if (cid == TLS1_CK_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256) { - if (p[2] != TLSEXT_curve_P_256) + if (curve_id != TLSEXT_curve_P_256) return 0; } else if (cid == TLS1_CK_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384) { - if (p[2] != TLSEXT_curve_P_384) + if (curve_id != TLSEXT_curve_P_384) return 0; } else /* Should never happen */ return 0; } if (!tls1_get_curvelist(s, 0, &curves, &num_curves)) return 0; - for (i = 0; i < num_curves; i++, curves += 2) { - if (p[1] == curves[0] && p[2] == curves[1]) - return tls_curve_allowed(s, p + 1, SSL_SECOP_CURVE_CHECK); + 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; } @@ -327,7 +321,7 @@ int tls1_check_curve(SSL *s, const unsigned char *p, size_t len) */ int tls1_shared_group(SSL *s, int nmatch) { - const unsigned char *pref, *supp; + const uint16_t *pref, *supp; size_t num_pref, num_supp, i, j; int k; @@ -366,18 +360,15 @@ int tls1_shared_group(SSL *s, int nmatch) &pref, &num_pref)) return nmatch == -1 ? 0 : NID_undef; - for (k = 0, i = 0; i < num_pref; i++, pref += 2) { - const unsigned char *tsupp = supp; + for (k = 0, i = 0; i < num_pref; i++) { + uint16_t id = pref[i]; - for (j = 0; j < num_supp; j++, tsupp += 2) { - if (pref[0] == tsupp[0] && pref[1] == tsupp[1]) { - if (!tls_curve_allowed(s, pref, SSL_SECOP_CURVE_SHARED)) + for (j = 0; j < num_supp; j++) { + if (id == supp[j]) { + if (!tls_curve_allowed(s, id, SSL_SECOP_CURVE_SHARED)) continue; - if (nmatch == k) { - int id = (pref[0] << 8) | pref[1]; - + if (nmatch == k) return tls1_ec_curve_id2nid(id, NULL); - } k++; } } @@ -388,22 +379,22 @@ int tls1_shared_group(SSL *s, int nmatch) return NID_undef; } -int tls1_set_groups(unsigned char **pext, size_t *pextlen, +int tls1_set_groups(uint16_t **pext, size_t *pextlen, int *groups, size_t ngroups) { - unsigned char *glist, *p; + uint16_t *glist; size_t i; /* * Bitmap of groups included to detect duplicates: only works while group * ids < 32 */ unsigned long dup_list = 0; - glist = OPENSSL_malloc(ngroups * 2); + glist = OPENSSL_malloc(ngroups * sizeof(*glist)); if (glist == NULL) return 0; - for (i = 0, p = glist; i < ngroups; i++) { + for (i = 0; i < ngroups; i++) { unsigned long idmask; - int id; + uint16_t id; /* TODO(TLS1.3): Convert for DH groups */ id = tls1_ec_nid2curve_id(groups[i]); idmask = 1L << id; @@ -412,11 +403,11 @@ int tls1_set_groups(unsigned char **pext, size_t *pextlen, return 0; } dup_list |= idmask; - s2n(id, p); + glist[i] = id; } OPENSSL_free(*pext); *pext = glist; - *pextlen = ngroups * 2; + *pextlen = ngroups; return 1; } @@ -456,7 +447,7 @@ static int nid_cb(const char *elem, int len, void *arg) } /* Set groups based on a colon separate list */ -int tls1_set_groups_list(unsigned char **pext, size_t *pextlen, const char *str) +int tls1_set_groups_list(uint16_t **pext, size_t *pextlen, const char *str) { nid_cb_st ncb; ncb.nidcnt = 0; @@ -468,7 +459,7 @@ int tls1_set_groups_list(unsigned char **pext, size_t *pextlen, const char *str) } /* For an EC key set TLS id and required compression based on parameters */ -static int tls1_set_ec_id(unsigned char *curve_id, unsigned char *comp_id, +static int tls1_set_ec_id(uint16_t *pcurve_id, unsigned char *comp_id, EC_KEY *ec) { int id; @@ -481,12 +472,10 @@ static int tls1_set_ec_id(unsigned char *curve_id, unsigned char *comp_id, return 0; /* Determine curve ID */ id = EC_GROUP_get_curve_name(grp); - id = tls1_ec_nid2curve_id(id); + *pcurve_id = tls1_ec_nid2curve_id(id); /* If no id return error: we don't support arbitrary explicit curves */ - if (id == 0) + if (*pcurve_id == 0) return 0; - curve_id[0] = 0; - curve_id[1] = (unsigned char)id; if (comp_id) { if (EC_KEY_get0_public_key(ec) == NULL) return 0; @@ -503,10 +492,10 @@ static int tls1_set_ec_id(unsigned char *curve_id, unsigned char *comp_id, } /* Check an EC key is compatible with extensions */ -static int tls1_check_ec_key(SSL *s, - unsigned char *curve_id, unsigned char *comp_id) +static int tls1_check_ec_key(SSL *s, uint16_t curve_id, unsigned char *comp_id) { - const unsigned char *pformats, *pcurves; + const unsigned char *pformats; + const uint16_t *pcurves; size_t num_formats, num_curves, i; int j; /* @@ -523,7 +512,7 @@ static int tls1_check_ec_key(SSL *s, if (i == num_formats) return 0; } - if (!curve_id) + if (curve_id == 0) return 1; /* Check curve is consistent with client and server preferences */ for (j = 0; j <= 1; j++) { @@ -539,8 +528,8 @@ static int tls1_check_ec_key(SSL *s, */ break; } - for (i = 0; i < num_curves; i++, pcurves += 2) { - if (pcurves[0] == curve_id[0] && pcurves[1] == curve_id[1]) + for (i = 0; i < num_curves; i++) { + if (pcurves[i] == curve_id) break; } if (i == num_curves) @@ -577,7 +566,8 @@ 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, curve_id[2]; + unsigned char comp_id; + uint16_t curve_id; EVP_PKEY *pkey; int rv; pkey = X509_get0_pubkey(x); @@ -586,14 +576,14 @@ static int tls1_check_cert_param(SSL *s, X509 *x, int check_ee_md) /* 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)); + rv = tls1_set_ec_id(&curve_id, &comp_id, EVP_PKEY_get0_EC_KEY(pkey)); if (!rv) 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 : NULL, &comp_id); + rv = tls1_check_ec_key(s, s->server ? curve_id : 0, &comp_id); if (!rv) return 0; /* @@ -604,12 +594,11 @@ static int tls1_check_cert_param(SSL *s, X509 *x, int check_ee_md) int check_md; size_t i; CERT *c = s->cert; - if (curve_id[0]) - return 0; + /* Check to see we have necessary signing algorithm */ - if (curve_id[1] == TLSEXT_curve_P_256) + if (curve_id == TLSEXT_curve_P_256) check_md = NID_ecdsa_with_SHA256; - else if (curve_id[1] == TLSEXT_curve_P_384) + else if (curve_id == TLSEXT_curve_P_384) check_md = NID_ecdsa_with_SHA384; else return 0; /* Should never happen */ @@ -639,15 +628,15 @@ int tls1_check_ec_tmp_key(SSL *s, unsigned long cid) * curves permitted. */ if (tls1_suiteb(s)) { - unsigned char curve_id[2]; + uint16_t curve_id; + /* Curve to check determined by ciphersuite */ if (cid == TLS1_CK_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256) - curve_id[1] = TLSEXT_curve_P_256; + curve_id = TLSEXT_curve_P_256; else if (cid == TLS1_CK_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384) - curve_id[1] = TLSEXT_curve_P_384; + curve_id = TLSEXT_curve_P_384; else return 0; - curve_id[0] = 0; /* Check this curve is acceptable */ if (!tls1_check_ec_key(s, curve_id, NULL)) return 0; @@ -983,10 +972,11 @@ int tls12_check_peer_sigalg(SSL *s, uint16_t sig, EVP_PKEY *pkey) return 0; } } else { - unsigned char curve_id[2], comp_id; + unsigned char comp_id; + uint16_t curve_id; /* Check compression and curve matches extensions */ - if (!tls1_set_ec_id(curve_id, &comp_id, ec)) + if (!tls1_set_ec_id(&curve_id, &comp_id, ec)) return 0; if (!s->server && !tls1_check_ec_key(s, curve_id, &comp_id)) { SSLerr(SSL_F_TLS12_CHECK_PEER_SIGALG, SSL_R_WRONG_CURVE); @@ -1584,20 +1574,11 @@ static int tls1_set_shared_sigalgs(SSL *s) return 1; } -/* Set preferred digest for each key type */ - -int tls1_save_sigalgs(SSL *s, PACKET *pkt) +int tls1_save_u16(PACKET *pkt, uint16_t **pdest, size_t *pdestlen) { - CERT *c = s->cert; unsigned int stmp; size_t size, i; - - /* Extension ignored for inappropriate versions */ - if (!SSL_USE_SIGALGS(s)) - return 1; - /* Should never happen */ - if (!c) - return 0; + uint16_t *buf; size = PACKET_remaining(pkt); @@ -1607,21 +1588,41 @@ int tls1_save_sigalgs(SSL *s, PACKET *pkt) size >>= 1; - OPENSSL_free(s->s3->tmp.peer_sigalgs); - s->s3->tmp.peer_sigalgs = OPENSSL_malloc(size - * sizeof(*s->s3->tmp.peer_sigalgs)); - if (s->s3->tmp.peer_sigalgs == NULL) + buf = OPENSSL_malloc(size * sizeof(*buf)); + if (buf == NULL) return 0; - s->s3->tmp.peer_sigalgslen = size; for (i = 0; i < size && PACKET_get_net_2(pkt, &stmp); i++) - s->s3->tmp.peer_sigalgs[i] = stmp; + buf[i] = stmp; - if (i != size) + if (i != size) { + OPENSSL_free(buf); return 0; + } + + OPENSSL_free(*pdest); + *pdest = buf; + *pdestlen = size; return 1; } +int tls1_save_sigalgs(SSL *s, PACKET *pkt) +{ + /* Extension ignored for inappropriate versions */ + if (!SSL_USE_SIGALGS(s)) + return 1; + /* Should never happen */ + if (s->cert == NULL) + return 0; + + return tls1_save_u16(pkt, &s->s3->tmp.peer_sigalgs, + &s->s3->tmp.peer_sigalgslen); + + return 1; +} + +/* Set preferred digest for each key type */ + int tls1_process_sigalgs(SSL *s) { size_t i; -- 2.25.1