From 7d061fced39d72bd664d04e254c1e3ba6cf99fbc Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Mon, 30 Jan 2017 16:16:28 +0000 Subject: [PATCH] Add server side support for creating the Hello Retry Request message Reviewed-by: Richard Levitte (Merged from https://github.com/openssl/openssl/pull/2341) --- include/openssl/ssl.h | 5 ++- include/openssl/ssl3.h | 1 + ssl/ssl_err.c | 3 ++ ssl/ssl_locl.h | 3 ++ ssl/statem/extensions.c | 8 ++++- ssl/statem/extensions_clnt.c | 2 +- ssl/statem/extensions_srvr.c | 68 +++++++++++++++++++++++++++++++++++- ssl/statem/statem_lib.c | 36 ++++++++++++------- ssl/statem/statem_srvr.c | 68 ++++++++++++++++++++++++++++++++---- 9 files changed, 170 insertions(+), 24 deletions(-) diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index aa3bcc6af0..78acf60093 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h @@ -880,7 +880,8 @@ typedef enum { TLS_ST_CR_ENCRYPTED_EXTENSIONS, TLS_ST_CR_CERT_VRFY, TLS_ST_SW_CERT_VRFY, - TLS_ST_CR_HELLO_REQ + TLS_ST_CR_HELLO_REQ, + TLS_ST_SW_HELLO_RETRY_REQUEST } OSSL_HANDSHAKE_STATE; /* @@ -2300,6 +2301,7 @@ int ERR_load_SSL_strings(void); # define SSL_F_TLS_CONSTRUCT_EXTENSIONS 447 # define SSL_F_TLS_CONSTRUCT_FINISHED 359 # define SSL_F_TLS_CONSTRUCT_HELLO_REQUEST 373 +# define SSL_F_TLS_CONSTRUCT_HELLO_RETRY_REQUEST 510 # define SSL_F_TLS_CONSTRUCT_NEW_SESSION_TICKET 428 # define SSL_F_TLS_CONSTRUCT_NEXT_PROTO 426 # define SSL_F_TLS_CONSTRUCT_SERVER_CERTIFICATE 490 @@ -2502,6 +2504,7 @@ int ERR_load_SSL_strings(void); # define SSL_R_NO_RENEGOTIATION 339 # define SSL_R_NO_REQUIRED_DIGEST 324 # define SSL_R_NO_SHARED_CIPHER 193 +# define SSL_R_NO_SHARED_GROUPS 410 # define SSL_R_NO_SHARED_SIGNATURE_ALGORITHMS 376 # define SSL_R_NO_SRTP_PROFILES 359 # define SSL_R_NO_SUITABLE_KEY_SHARE 101 diff --git a/include/openssl/ssl3.h b/include/openssl/ssl3.h index 8d146be19b..d76236ab8d 100644 --- a/include/openssl/ssl3.h +++ b/include/openssl/ssl3.h @@ -274,6 +274,7 @@ extern "C" { # define SSL3_MT_CLIENT_HELLO 1 # define SSL3_MT_SERVER_HELLO 2 # define SSL3_MT_NEWSESSION_TICKET 4 +# define SSL3_MT_HELLO_RETRY_REQUEST 6 # define SSL3_MT_ENCRYPTED_EXTENSIONS 8 # define SSL3_MT_CERTIFICATE 11 # define SSL3_MT_SERVER_KEY_EXCHANGE 12 diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c index a6d3412efa..705252c763 100644 --- a/ssl/ssl_err.c +++ b/ssl/ssl_err.c @@ -329,6 +329,8 @@ static ERR_STRING_DATA SSL_str_functs[] = { {ERR_FUNC(SSL_F_TLS_CONSTRUCT_FINISHED), "tls_construct_finished"}, {ERR_FUNC(SSL_F_TLS_CONSTRUCT_HELLO_REQUEST), "tls_construct_hello_request"}, + {ERR_FUNC(SSL_F_TLS_CONSTRUCT_HELLO_RETRY_REQUEST), + "tls_construct_hello_retry_request"}, {ERR_FUNC(SSL_F_TLS_CONSTRUCT_NEW_SESSION_TICKET), "tls_construct_new_session_ticket"}, {ERR_FUNC(SSL_F_TLS_CONSTRUCT_NEXT_PROTO), "tls_construct_next_proto"}, @@ -603,6 +605,7 @@ static ERR_STRING_DATA SSL_str_reasons[] = { {ERR_REASON(SSL_R_NO_RENEGOTIATION), "no renegotiation"}, {ERR_REASON(SSL_R_NO_REQUIRED_DIGEST), "no required digest"}, {ERR_REASON(SSL_R_NO_SHARED_CIPHER), "no shared cipher"}, + {ERR_REASON(SSL_R_NO_SHARED_GROUPS), "no shared groups"}, {ERR_REASON(SSL_R_NO_SHARED_SIGNATURE_ALGORITHMS), "no shared signature algorithms"}, {ERR_REASON(SSL_R_NO_SRTP_PROFILES), "no srtp profiles"}, diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h index 2a23007393..099f8ccadc 100644 --- a/ssl/ssl_locl.h +++ b/ssl/ssl_locl.h @@ -1005,6 +1005,9 @@ struct ssl_st { unsigned char cert_verify_hash[EVP_MAX_MD_SIZE]; size_t cert_verify_hash_len; + /* Flag to indicate whether we should send a HelloRetryRequest or not */ + int hello_retry_request; + /* * the session_id_context is used to ensure sessions are only reused in * the appropriate context diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c index 4137f0333a..fd050f0d2e 100644 --- a/ssl/statem/extensions.c +++ b/ssl/statem/extensions.c @@ -979,12 +979,18 @@ 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) == 0)) { /* No suitable share */ - /* TODO(TLS1.3): Send a HelloRetryRequest */ + if (s->server && s->hello_retry_request == 0 && sent) { + s->hello_retry_request = 1; + return 1; + } + + /* Nothing left we can do - just fail */ *al = SSL_AD_HANDSHAKE_FAILURE; SSLerr(SSL_F_FINAL_KEY_SHARE, SSL_R_NO_SUITABLE_KEY_SHARE); return 0; } + s->hello_retry_request = 0; /* * For a client side resumption with no key_share we need to generate * the handshake secret (otherwise this is done during key_share diff --git a/ssl/statem/extensions_clnt.c b/ssl/statem/extensions_clnt.c index efae91692d..003874507d 100644 --- a/ssl/statem/extensions_clnt.c +++ b/ssl/statem/extensions_clnt.c @@ -164,7 +164,7 @@ int tls_construct_ctos_supported_groups(SSL *s, WPACKET *pkt, } /* Copy curve ID if supported */ for (i = 0; i < num_curves; i++, pcurvestmp += 2) { - if (tls_curve_allowed(s, pcurves, SSL_SECOP_CURVE_SUPPORTED)) { + 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])) { SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_SUPPORTED_GROUPS, diff --git a/ssl/statem/extensions_srvr.c b/ssl/statem/extensions_srvr.c index c4d20e576a..f7b799030d 100644 --- a/ssl/statem/extensions_srvr.c +++ b/ssl/statem/extensions_srvr.c @@ -116,6 +116,8 @@ int tls_parse_ctos_server_name(SSL *s, PACKET *pkt, unsigned int context, return 0; } + OPENSSL_free(s->session->ext.hostname); + s->session->ext.hostname = NULL; if (!PACKET_strndup(&hostname, &s->session->ext.hostname)) { *al = TLS1_AD_INTERNAL_ERROR; return 0; @@ -361,6 +363,9 @@ int tls_parse_ctos_alpn(SSL *s, PACKET *pkt, unsigned int context, X509 *x, } } while (PACKET_remaining(&protocol_list) != 0); + OPENSSL_free(s->s3->alpn_proposed); + s->s3->alpn_proposed = NULL; + s->s3->alpn_proposed_len = 0; if (!PACKET_memdup(&save_protocol_list, &s->s3->alpn_proposed, &s->s3->alpn_proposed_len)) { *al = TLS1_AD_INTERNAL_ERROR; @@ -659,6 +664,9 @@ int tls_parse_ctos_supported_groups(SSL *s, PACKET *pkt, unsigned int context, return 0; } + OPENSSL_free(s->session->ext.supportedgroups); + s->session->ext.supportedgroups = NULL; + s->session->ext.supportedgroups_len = 0; if (!PACKET_memdup(&supported_groups_list, &s->session->ext.supportedgroups, &s->session->ext.supportedgroups_len)) { @@ -1024,7 +1032,65 @@ int tls_construct_stoc_key_share(SSL *s, WPACKET *pkt, unsigned int context, EVP_PKEY *ckey = s->s3->peer_tmp, *skey = NULL; if (ckey == NULL) { - /* No key_share received from client; must be resuming. */ + /* No key_share received from client */ + if (s->hello_retry_request) { + const unsigned char *pcurves, *pcurvestmp, *clntcurves; + size_t num_curves, clnt_num_curves, i; + + /* 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_TLS_CONSTRUCT_STOC_KEY_SHARE, + ERR_R_INTERNAL_ERROR); + return 0; + } + + /* Get our list of available groups */ + if (!tls1_get_curvelist(s, 0, &pcurves, &num_curves)) { + SSLerr(SSL_F_TLS_CONSTRUCT_STOC_KEY_SHARE, + ERR_R_INTERNAL_ERROR); + return 0; + } + + if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_key_share) + || !WPACKET_start_sub_packet_u16(pkt)) { + SSLerr(SSL_F_TLS_CONSTRUCT_STOC_KEY_SHARE, + ERR_R_INTERNAL_ERROR); + return 0; + } + + /* Find first group we allow that is also in client's list */ + for (i = 0, pcurvestmp = pcurves; i < num_curves; + i++, pcurvestmp += 2) { + unsigned int group_id = pcurvestmp[0] << 8 | pcurvestmp[1]; + + if (check_in_list(s, group_id, clntcurves, clnt_num_curves, + 1)) { + if (!WPACKET_put_bytes_u16(pkt, group_id)) { + SSLerr(SSL_F_TLS_CONSTRUCT_STOC_KEY_SHARE, + ERR_R_INTERNAL_ERROR); + return 0; + } + break; + } + } + if (i == num_curves) { + /* No common groups */ + *al = SSL_AD_HANDSHAKE_FAILURE; + SSLerr(SSL_F_TLS_CONSTRUCT_STOC_KEY_SHARE, + SSL_R_NO_SHARED_GROUPS); + return 0; + } + if (!WPACKET_close(pkt)) { + SSLerr(SSL_F_TLS_CONSTRUCT_STOC_KEY_SHARE, + ERR_R_INTERNAL_ERROR); + return 0; + } + + return 1; + } + + /* Must be resuming. */ if (!s->hit || !tls13_generate_handshake_secret(s, NULL, 0)) { *al = SSL_AD_INTERNAL_ERROR; SSLerr(SSL_F_TLS_CONSTRUCT_STOC_KEY_SHARE, ERR_R_INTERNAL_ERROR); diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c index 8e7245b856..13174abb17 100644 --- a/ssl/statem/statem_lib.c +++ b/ssl/statem/statem_lib.c @@ -1434,21 +1434,22 @@ int ssl_choose_server_version(SSL *s, CLIENTHELLO_MSG *hello) switch (server_version) { default: + if (!SSL_IS_TLS13(s)) { + if (version_cmp(s, client_version, s->version) < 0) + return SSL_R_WRONG_SSL_VERSION; + /* + * If this SSL handle is not from a version flexible method we don't + * (and never did) check min/max FIPS or Suite B constraints. Hope + * that's OK. It is up to the caller to not choose fixed protocol + * versions they don't want. If not, then easy to fix, just return + * ssl_method_error(s, s->method) + */ + return 0; + } /* - * TODO(TLS1.3): This check will fail if someone attempts to do - * renegotiation in TLS1.3 at the moment. We need to ensure we disable - * renegotiation for TLS1.3 + * Fall through if we are TLSv1.3 already (this means we must be after + * a HelloRetryRequest */ - if (version_cmp(s, client_version, s->version) < 0) - return SSL_R_WRONG_SSL_VERSION; - /* - * If this SSL handle is not from a version flexible method we don't - * (and never did) check min/max FIPS or Suite B constraints. Hope - * that's OK. It is up to the caller to not choose fixed protocol - * versions they don't want. If not, then easy to fix, just return - * ssl_method_error(s, s->method) - */ - return 0; case TLS_ANY_VERSION: table = tls_version_table; break; @@ -1503,6 +1504,15 @@ int ssl_choose_server_version(SSL *s, CLIENTHELLO_MSG *hello) } if (best_vers > 0) { + if (SSL_IS_TLS13(s)) { + /* + * We get here if this is after a HelloRetryRequest. In this + * case we just check that we still negotiated TLSv1.3 + */ + if (best_vers != TLS1_3_VERSION) + return SSL_R_UNSUPPORTED_PROTOCOL; + return 0; + } s->version = best_vers; s->method = best_method; return 0; diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c index de0fcc0bbd..8aba6697f3 100644 --- a/ssl/statem/statem_srvr.c +++ b/ssl/statem/statem_srvr.c @@ -62,6 +62,7 @@ #include static int tls_construct_encrypted_extensions(SSL *s, WPACKET *pkt); +static int tls_construct_hello_retry_request(SSL *s, WPACKET *pkt); static STACK_OF(SSL_CIPHER) *ssl_bytes_to_cipher_list(SSL *s, PACKET *cipher_suites, STACK_OF(SSL_CIPHER) @@ -81,11 +82,6 @@ static int ossl_statem_server13_read_transition(SSL *s, int mt) { OSSL_STATEM *st = &s->statem; - /* - * TODO(TLS1.3): This is still based on the TLSv1.2 state machine. Over time - * we will update this to look more like real TLSv1.3 - */ - /* * Note: There is no case for TLS_ST_BEFORE because at that stage we have * not negotiated TLSv1.3 yet, so that case is handled by @@ -95,6 +91,13 @@ static int ossl_statem_server13_read_transition(SSL *s, int mt) default: break; + case TLS_ST_SW_HELLO_RETRY_REQUEST: + if (mt == SSL3_MT_CLIENT_HELLO) { + st->hand_state = TLS_ST_SR_CLNT_HELLO; + return 1; + } + break; + case TLS_ST_SW_FINISHED: if (s->s3->tmp.cert_request) { if (mt == SSL3_MT_CERTIFICATE) { @@ -406,9 +409,15 @@ static WRITE_TRAN ossl_statem_server13_write_transition(SSL *s) return WRITE_TRAN_ERROR; case TLS_ST_SR_CLNT_HELLO: - st->hand_state = TLS_ST_SW_SRVR_HELLO; + if (s->hello_retry_request) + st->hand_state = TLS_ST_SW_HELLO_RETRY_REQUEST; + else + st->hand_state = TLS_ST_SW_SRVR_HELLO; return WRITE_TRAN_CONTINUE; + case TLS_ST_SW_HELLO_RETRY_REQUEST: + return WRITE_TRAN_FINISHED; + case TLS_ST_SW_SRVR_HELLO: st->hand_state = TLS_ST_SW_ENCRYPTED_EXTENSIONS; return WRITE_TRAN_CONTINUE; @@ -693,6 +702,11 @@ WORK_STATE ossl_statem_server_post_work(SSL *s, WORK_STATE wst) /* No post work to be done */ break; + case TLS_ST_SW_HELLO_RETRY_REQUEST: + if (statem_flush(s) != 1) + return WORK_MORE_A; + break; + case TLS_ST_SW_HELLO_REQ: if (statem_flush(s) != 1) return WORK_MORE_A; @@ -904,6 +918,11 @@ int ossl_statem_server_construct_message(SSL *s, WPACKET *pkt, *confunc = tls_construct_encrypted_extensions; *mt = SSL3_MT_ENCRYPTED_EXTENSIONS; break; + + case TLS_ST_SW_HELLO_RETRY_REQUEST: + *confunc = tls_construct_hello_retry_request; + *mt = SSL3_MT_HELLO_RETRY_REQUEST; + break; } return 1; @@ -1200,6 +1219,12 @@ MSG_PROCESS_RETURN tls_process_client_hello(SSL *s, PACKET *pkt) if (clienthello.isv2) { unsigned int mt; + if (!SSL_IS_FIRST_HANDSHAKE(s) || s->hello_retry_request) { + al = SSL_AD_HANDSHAKE_FAILURE; + SSLerr(SSL_F_TLS_PROCESS_CLIENT_HELLO, SSL_R_UNEXPECTED_MESSAGE); + goto f_err; + } + /*- * An SSLv3/TLSv1 backwards-compatible CLIENT-HELLO in an SSLv2 * header is sent directly on the wire, not wrapped as a TLS @@ -1402,7 +1427,7 @@ MSG_PROCESS_RETURN tls_process_client_hello(SSL *s, PACKET *pkt) if (protverr) { SSLerr(SSL_F_TLS_PROCESS_CLIENT_HELLO, protverr); - if ((!s->enc_write_ctx && !s->write_hash)) { + if (SSL_IS_FIRST_HANDSHAKE(s)) { /* like ssl3_get_record, send alert using remote version number */ s->version = s->client_version = clienthello.legacy_version; } @@ -3502,6 +3527,10 @@ STACK_OF(SSL_CIPHER) *ssl_bytes_to_cipher_list(SSL *s, return NULL; } + OPENSSL_free(s->s3->tmp.ciphers_raw); + s->s3->tmp.ciphers_raw = NULL; + s->s3->tmp.ciphers_rawlen = 0; + if (sslv2format) { size_t numciphers = PACKET_remaining(cipher_suites) / n; PACKET sslv2ciphers = *cipher_suites; @@ -3607,3 +3636,28 @@ STACK_OF(SSL_CIPHER) *ssl_bytes_to_cipher_list(SSL *s, sk_SSL_CIPHER_free(sk); return NULL; } + +static int tls_construct_hello_retry_request(SSL *s, WPACKET *pkt) +{ + int al; + + /* + * TODO(TLS1.3): Remove the DRAFT version before release + * (should be s->version) + */ + if (!WPACKET_put_bytes_u16(pkt, TLS1_3_VERSION_DRAFT) + || !tls_construct_extensions(s, pkt, EXT_TLS1_3_HELLO_RETRY_REQUEST, + NULL, 0, &al)) { + ssl3_send_alert(s, SSL3_AL_FATAL, al); + SSLerr(SSL_F_TLS_CONSTRUCT_HELLO_RETRY_REQUEST, ERR_R_INTERNAL_ERROR); + ssl3_send_alert(s, SSL3_AL_FATAL, al); + return 0; + } + + /* Ditch the session. We'll create a new one next time around */ + SSL_SESSION_free(s->session); + s->session = NULL; + s->hit = 0; + + return 1; +}