From 3847d426e3a530786b82fecfdbc9793b44b88cd3 Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Wed, 1 Feb 2017 13:31:27 +0000 Subject: [PATCH] Add client side support for parsing Hello Retry Request Reviewed-by: Richard Levitte (Merged from https://github.com/openssl/openssl/pull/2341) --- include/openssl/ssl.h | 5 +- ssl/ssl_err.c | 3 + ssl/statem/extensions_clnt.c | 157 ++++++++++++++++++++++++----------- ssl/statem/statem_clnt.c | 87 +++++++++++++++++-- ssl/statem/statem_lib.c | 3 + ssl/statem/statem_locl.h | 1 + 6 files changed, 203 insertions(+), 53 deletions(-) diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index 78acf60093..96a5558082 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h @@ -881,7 +881,8 @@ typedef enum { TLS_ST_CR_CERT_VRFY, TLS_ST_SW_CERT_VRFY, TLS_ST_CR_HELLO_REQ, - TLS_ST_SW_HELLO_RETRY_REQUEST + TLS_ST_SW_HELLO_RETRY_REQUEST, + TLS_ST_CR_HELLO_RETRY_REQUEST } OSSL_HANDSHAKE_STATE; /* @@ -2073,6 +2074,7 @@ int ERR_load_SSL_strings(void); /* Function codes. */ # define SSL_F_ADD_CLIENT_KEY_SHARE_EXT 438 +# define SSL_F_ADD_KEY_SHARE 512 # define SSL_F_CHECK_SUITEB_CIPHER_LIST 331 # define SSL_F_CT_MOVE_SCTS 345 # define SSL_F_CT_STRICT 349 @@ -2355,6 +2357,7 @@ int ERR_load_SSL_strings(void); # define SSL_F_TLS_PROCESS_ENCRYPTED_EXTENSIONS 444 # define SSL_F_TLS_PROCESS_FINISHED 364 # define SSL_F_TLS_PROCESS_HELLO_REQ 507 +# define SSL_F_TLS_PROCESS_HELLO_RETRY_REQUEST 511 # define SSL_F_TLS_PROCESS_INITIAL_SERVER_FLIGHT 442 # define SSL_F_TLS_PROCESS_KEY_EXCHANGE 365 # define SSL_F_TLS_PROCESS_NEW_SESSION_TICKET 366 diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c index 705252c763..ea5a7634bd 100644 --- a/ssl/ssl_err.c +++ b/ssl/ssl_err.c @@ -20,6 +20,7 @@ static ERR_STRING_DATA SSL_str_functs[] = { {ERR_FUNC(SSL_F_ADD_CLIENT_KEY_SHARE_EXT), "add_client_key_share_ext"}, + {ERR_FUNC(SSL_F_ADD_KEY_SHARE), "add_key_share"}, {ERR_FUNC(SSL_F_CHECK_SUITEB_CIPHER_LIST), "check_suiteb_cipher_list"}, {ERR_FUNC(SSL_F_CT_MOVE_SCTS), "ct_move_scts"}, {ERR_FUNC(SSL_F_CT_STRICT), "ct_strict"}, @@ -412,6 +413,8 @@ static ERR_STRING_DATA SSL_str_functs[] = { "tls_process_encrypted_extensions"}, {ERR_FUNC(SSL_F_TLS_PROCESS_FINISHED), "tls_process_finished"}, {ERR_FUNC(SSL_F_TLS_PROCESS_HELLO_REQ), "tls_process_hello_req"}, + {ERR_FUNC(SSL_F_TLS_PROCESS_HELLO_RETRY_REQUEST), + "tls_process_hello_retry_request"}, {ERR_FUNC(SSL_F_TLS_PROCESS_INITIAL_SERVER_FLIGHT), "tls_process_initial_server_flight"}, {ERR_FUNC(SSL_F_TLS_PROCESS_KEY_EXCHANGE), "tls_process_key_exchange"}, diff --git a/ssl/statem/extensions_clnt.c b/ssl/statem/extensions_clnt.c index 003874507d..2516ab9562 100644 --- a/ssl/statem/extensions_clnt.c +++ b/ssl/statem/extensions_clnt.c @@ -528,12 +528,57 @@ int tls_construct_ctos_psk_kex_modes(SSL *s, WPACKET *pkt, unsigned int context, return 1; } +#ifndef OPENSSL_NO_TLS1_3 +static int add_key_share(SSL *s, WPACKET *pkt, unsigned int curve_id) +{ + unsigned char *encodedPoint = NULL; + EVP_PKEY *key_share_key = NULL; + size_t encodedlen; + + key_share_key = ssl_generate_pkey_curve(curve_id); + if (key_share_key == NULL) { + SSLerr(SSL_F_ADD_KEY_SHARE, ERR_R_EVP_LIB); + return 0; + } + + /* Encode the public key. */ + encodedlen = EVP_PKEY_get1_tls_encodedpoint(key_share_key, + &encodedPoint); + if (encodedlen == 0) { + SSLerr(SSL_F_ADD_KEY_SHARE, ERR_R_EC_LIB); + EVP_PKEY_free(key_share_key); + return 0; + } + + /* Create KeyShareEntry */ + if (!WPACKET_put_bytes_u16(pkt, curve_id) + || !WPACKET_sub_memcpy_u16(pkt, encodedPoint, encodedlen)) { + SSLerr(SSL_F_ADD_KEY_SHARE, ERR_R_INTERNAL_ERROR); + EVP_PKEY_free(key_share_key); + OPENSSL_free(encodedPoint); + return 0; + } + + /* + * TODO(TLS1.3): When changing to send more than one key_share we're + * going to need to be able to save more than one EVP_PKEY. For now + * we reuse the existing tmp.pkey + */ + s->s3->tmp.pkey = key_share_key; + s->s3->group_id = curve_id; + OPENSSL_free(encodedPoint); + + return 1; +} +#endif + int tls_construct_ctos_key_share(SSL *s, WPACKET *pkt, unsigned int context, X509 *x, size_t chainidx, int *al) { #ifndef OPENSSL_NO_TLS1_3 - size_t i, sharessent = 0, num_curves = 0; + size_t i, num_curves = 0; const unsigned char *pcurves = NULL; + unsigned int curve_id = 0; /* key_share extension */ if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_key_share) @@ -551,62 +596,37 @@ int tls_construct_ctos_key_share(SSL *s, WPACKET *pkt, unsigned int context, return 0; } + if (s->s3->tmp.pkey != NULL) { + /* Shouldn't happen! */ + SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_KEY_SHARE, ERR_R_INTERNAL_ERROR); + return 0; + } + /* * TODO(TLS1.3): Make the number of key_shares sent configurable. For * now, just send one */ - for (i = 0; i < num_curves && sharessent < 1; i++, pcurves += 2) { - unsigned char *encodedPoint = NULL; - unsigned int curve_id = 0; - EVP_PKEY *key_share_key = NULL; - size_t encodedlen; - - if (!tls_curve_allowed(s, pcurves, SSL_SECOP_CURVE_SUPPORTED)) - continue; - - if (s->s3->tmp.pkey != NULL) { - /* Shouldn't happen! */ - SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_KEY_SHARE, ERR_R_INTERNAL_ERROR); - return 0; - } + if (s->s3->group_id != 0) { + curve_id = s->s3->group_id; + } else { + for (i = 0; i < num_curves; i++, pcurves += 2) { - /* Generate a key for this key_share */ - curve_id = (pcurves[0] << 8) | pcurves[1]; - key_share_key = ssl_generate_pkey_curve(curve_id); - if (key_share_key == NULL) { - SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_KEY_SHARE, ERR_R_EVP_LIB); - return 0; - } + if (!tls_curve_allowed(s, pcurves, SSL_SECOP_CURVE_SUPPORTED)) + continue; - /* Encode the public key. */ - encodedlen = EVP_PKEY_get1_tls_encodedpoint(key_share_key, - &encodedPoint); - if (encodedlen == 0) { - SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_KEY_SHARE, ERR_R_EC_LIB); - EVP_PKEY_free(key_share_key); - return 0; - } - - /* Create KeyShareEntry */ - if (!WPACKET_put_bytes_u16(pkt, curve_id) - || !WPACKET_sub_memcpy_u16(pkt, encodedPoint, encodedlen)) { - SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_KEY_SHARE, ERR_R_INTERNAL_ERROR); - EVP_PKEY_free(key_share_key); - OPENSSL_free(encodedPoint); - return 0; + curve_id = (pcurves[0] << 8) | pcurves[1]; + break; } + } - /* - * TODO(TLS1.3): When changing to send more than one key_share we're - * going to need to be able to save more than one EVP_PKEY. For now - * we reuse the existing tmp.pkey - */ - s->s3->group_id = curve_id; - s->s3->tmp.pkey = key_share_key; - sharessent++; - OPENSSL_free(encodedPoint); + if (curve_id == 0) { + SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_KEY_SHARE, SSL_R_NO_SUITABLE_KEY_SHARE); + return 0; } + if (!add_key_share(s, pkt, curve_id)) + return 0; + if (!WPACKET_close(pkt) || !WPACKET_close(pkt)) { SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_KEY_SHARE, ERR_R_INTERNAL_ERROR); return 0; @@ -1188,6 +1208,49 @@ int tls_parse_stoc_key_share(SSL *s, PACKET *pkt, unsigned int context, X509 *x, return 0; } + if ((context & EXT_TLS1_3_HELLO_RETRY_REQUEST) != 0) { + unsigned const char *pcurves = NULL; + size_t i, num_curves; + + if (PACKET_remaining(pkt) != 0) { + *al = SSL_AD_HANDSHAKE_FAILURE; + SSLerr(SSL_F_TLS_PARSE_STOC_KEY_SHARE, SSL_R_LENGTH_MISMATCH); + return 0; + } + + /* + * It is an error if the HelloRetryRequest wants a key_share that we + * already sent in the first ClientHello + */ + if (group_id == s->s3->group_id) { + *al = SSL_AD_ILLEGAL_PARAMETER; + SSLerr(SSL_F_TLS_PARSE_STOC_KEY_SHARE, SSL_R_BAD_KEY_SHARE); + return 0; + } + + /* Validate the selected group is one we support */ + pcurves = s->ext.supportedgroups; + if (!tls1_get_curvelist(s, 0, &pcurves, &num_curves)) { + 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 == (unsigned int)((pcurves[0] << 8) | pcurves[1])) + break; + } + if (i >= num_curves + || !tls_curve_allowed(s, pcurves, SSL_SECOP_CURVE_SUPPORTED)) { + *al = SSL_AD_ILLEGAL_PARAMETER; + SSLerr(SSL_F_TLS_PARSE_STOC_KEY_SHARE, SSL_R_BAD_KEY_SHARE); + return 0; + } + + s->s3->group_id = group_id; + EVP_PKEY_free(s->s3->tmp.pkey); + s->s3->tmp.pkey = NULL; + return 1; + } + if (group_id != s->s3->group_id) { /* * This isn't for the group that we sent in the original diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c index e5c60aee78..5860a2307d 100644 --- a/ssl/statem/statem_clnt.c +++ b/ssl/statem/statem_clnt.c @@ -60,6 +60,7 @@ #include #include +static MSG_PROCESS_RETURN tls_process_hello_retry_request(SSL *s, PACKET *pkt); static MSG_PROCESS_RETURN tls_process_encrypted_extensions(SSL *s, PACKET *pkt); static ossl_inline int cert_req_allowed(SSL *s); @@ -137,6 +138,17 @@ static int ossl_statem_client13_read_transition(SSL *s, int mt) default: break; + case TLS_ST_CW_CLNT_HELLO: + /* + * This must a ClientHello following a HelloRetryRequest, so the only + * thing we can get now is a ServerHello. + */ + if (mt == SSL3_MT_SERVER_HELLO) { + st->hand_state = TLS_ST_CR_SRVR_HELLO; + return 1; + } + break; + case TLS_ST_CR_SRVR_HELLO: if (mt == SSL3_MT_ENCRYPTED_EXTENSIONS) { st->hand_state = TLS_ST_CR_ENCRYPTED_EXTENSIONS; @@ -210,8 +222,8 @@ int ossl_statem_client_read_transition(SSL *s, int mt) int ske_expected; /* - * Note that after a ClientHello we don't know what version we are going - * to negotiate yet, so we don't take this branch until later + * Note that after writing the first ClientHello we don't know what version + * we are going to negotiate yet, so we don't take this branch until later. */ if (SSL_IS_TLS13(s)) { if (!ossl_statem_client13_read_transition(s, mt)) @@ -234,6 +246,11 @@ int ossl_statem_client_read_transition(SSL *s, int mt) st->hand_state = DTLS_ST_CR_HELLO_VERIFY_REQUEST; return 1; } + } else { + if (mt == SSL3_MT_HELLO_RETRY_REQUEST) { + st->hand_state = TLS_ST_CR_HELLO_RETRY_REQUEST; + return 1; + } } break; @@ -390,15 +407,23 @@ static WRITE_TRAN ossl_statem_client13_write_transition(SSL *s) */ /* - * Note: There are no cases for TLS_ST_BEFORE or TLS_ST_CW_CLNT_HELLO, - * because we haven't negotiated TLSv1.3 yet at that point. They are - * handled by ossl_statem_client_write_transition(). + * Note: There are no cases for TLS_ST_BEFORE because we haven't negotiated + * TLSv1.3 yet at that point. They are handled by + * ossl_statem_client_write_transition(). */ switch (st->hand_state) { default: /* Shouldn't happen */ return WRITE_TRAN_ERROR; + case TLS_ST_CW_CLNT_HELLO: + /* We only hit this in the case of HelloRetryRequest */ + return WRITE_TRAN_FINISHED; + + case TLS_ST_CR_HELLO_RETRY_REQUEST: + st->hand_state = TLS_ST_CW_CLNT_HELLO; + return WRITE_TRAN_CONTINUE; + case TLS_ST_CR_FINISHED: st->hand_state = (s->s3->tmp.cert_req != 0) ? TLS_ST_CW_CERT : TLS_ST_CW_FINISHED; @@ -779,6 +804,9 @@ size_t ossl_statem_client_max_message_size(SSL *s) case DTLS_ST_CR_HELLO_VERIFY_REQUEST: return HELLO_VERIFY_REQUEST_MAX_LENGTH; + case TLS_ST_CR_HELLO_RETRY_REQUEST: + return HELLO_RETRY_REQUEST_MAX_LENGTH; + case TLS_ST_CR_CERT: return s->max_cert_list; @@ -836,6 +864,9 @@ MSG_PROCESS_RETURN ossl_statem_client_process_message(SSL *s, PACKET *pkt) case DTLS_ST_CR_HELLO_VERIFY_REQUEST: return dtls_process_hello_verify(s, pkt); + case TLS_ST_CR_HELLO_RETRY_REQUEST: + return tls_process_hello_retry_request(s, pkt); + case TLS_ST_CR_CERT: return tls_process_server_certificate(s, pkt); @@ -1432,6 +1463,52 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL *s, PACKET *pkt) return MSG_PROCESS_ERROR; } +static MSG_PROCESS_RETURN tls_process_hello_retry_request(SSL *s, PACKET *pkt) +{ + unsigned int sversion; + int protverr; + RAW_EXTENSION *extensions = NULL; + int al; + PACKET extpkt; + + if (!PACKET_get_net_2(pkt, &sversion)) { + al = SSL_AD_DECODE_ERROR; + SSLerr(SSL_F_TLS_PROCESS_HELLO_RETRY_REQUEST, SSL_R_LENGTH_MISMATCH); + goto f_err; + } + + s->hello_retry_request = 1; + + /* This will fail if it doesn't choose TLSv1.3+ */ + protverr = ssl_choose_client_version(s, sversion); + if (protverr != 0) { + al = SSL_AD_PROTOCOL_VERSION; + SSLerr(SSL_F_TLS_PROCESS_HELLO_RETRY_REQUEST, protverr); + goto f_err; + } + + if (!PACKET_as_length_prefixed_2(pkt, &extpkt)) { + al = SSL_AD_DECODE_ERROR; + SSLerr(SSL_F_TLS_PROCESS_HELLO_RETRY_REQUEST, SSL_R_BAD_LENGTH); + goto f_err; + } + + if (!tls_collect_extensions(s, &extpkt, EXT_TLS1_3_HELLO_RETRY_REQUEST, + &extensions, &al) + || !tls_parse_all_extensions(s, EXT_TLS1_3_HELLO_RETRY_REQUEST, + extensions, NULL, 0, &al)) + goto f_err; + + OPENSSL_free(extensions); + + return MSG_PROCESS_FINISHED_READING; + f_err: + ssl3_send_alert(s, SSL3_AL_FATAL, al); + ossl_statem_set_error(s); + OPENSSL_free(extensions); + return MSG_PROCESS_ERROR; +} + MSG_PROCESS_RETURN tls_process_server_certificate(SSL *s, PACKET *pkt) { int al, i, ret = MSG_PROCESS_ERROR, exp_idx; diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c index 13174abb17..c3dd31a9ad 100644 --- a/ssl/statem/statem_lib.c +++ b/ssl/statem/statem_lib.c @@ -1595,6 +1595,9 @@ int ssl_choose_client_version(SSL *s, int version) continue; if (vent->cmeth == NULL) break; + if (s->hello_retry_request && version != TLS1_3_VERSION) + return SSL_R_WRONG_SSL_VERSION; + method = vent->cmeth(); err = ssl_method_error(s, method); if (err != 0) diff --git a/ssl/statem/statem_locl.h b/ssl/statem/statem_locl.h index 35d96cd78c..429d5c703f 100644 --- a/ssl/statem/statem_locl.h +++ b/ssl/statem/statem_locl.h @@ -19,6 +19,7 @@ /* The spec allows for a longer length than this, but we limit it */ #define HELLO_VERIFY_REQUEST_MAX_LENGTH 258 #define SERVER_HELLO_MAX_LENGTH 20000 +#define HELLO_RETRY_REQUEST_MAX_LENGTH 20000 #define ENCRYPTED_EXTENSIONS_MAX_LENGTH 20000 #define SERVER_KEY_EXCH_MAX_LENGTH 102400 #define SERVER_HELLO_DONE_MAX_LENGTH 0 -- 2.25.1