From: Matt Caswell Date: Fri, 24 Feb 2017 11:13:25 +0000 (+0000) Subject: Don't accept early_data if we are going to issue a HelloRetryRequest X-Git-Tag: OpenSSL_1_1_1-pre1~2151 X-Git-Url: https://git.openssl.org/?p=openssl.git;a=commitdiff_plain;h=38df5a452777b612f75796531c0b2629da6aa550;hp=538bea6c8184670a8d1608ef288a4e1813dcefa6 Don't accept early_data if we are going to issue a HelloRetryRequest Reviewed-by: Rich Salz (Merged from https://github.com/openssl/openssl/pull/2737) --- diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h index 52515cb589..9e71768903 100644 --- a/ssl/ssl_locl.h +++ b/ssl/ssl_locl.h @@ -1783,7 +1783,6 @@ typedef enum tlsext_index_en { TLSEXT_IDX_server_name, TLSEXT_IDX_srp, TLSEXT_IDX_early_data_info, - TLSEXT_IDX_early_data, TLSEXT_IDX_ec_point_formats, TLSEXT_IDX_supported_groups, TLSEXT_IDX_session_ticket, @@ -1799,6 +1798,7 @@ typedef enum tlsext_index_en { TLSEXT_IDX_psk_kex_modes, TLSEXT_IDX_key_share, TLSEXT_IDX_cryptopro_bug, + TLSEXT_IDX_early_data, TLSEXT_IDX_padding, TLSEXT_IDX_psk } TLSEXT_INDEX; diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c index 2b137701f0..fa6221f6e2 100644 --- a/ssl/statem/extensions.c +++ b/ssl/statem/extensions.c @@ -43,6 +43,7 @@ static int final_key_share(SSL *s, unsigned int context, int sent, int *al); static int init_srtp(SSL *s, unsigned int context); #endif static int final_sig_algs(SSL *s, unsigned int context, int sent, int *al); +static int final_early_data(SSL *s, unsigned int context, int sent, int *al); /* Structure to define a built-in extension */ typedef struct extensions_definition_st { @@ -135,12 +136,6 @@ static const EXTENSION_DEFINITION ext_defs[] = { NULL, NULL, tls_parse_stoc_early_data_info, tls_construct_stoc_early_data_info, NULL, NULL }, - { - TLSEXT_TYPE_early_data, - EXT_CLIENT_HELLO | EXT_TLS1_3_ENCRYPTED_EXTENSIONS, - NULL, tls_parse_ctos_early_data, tls_parse_stoc_early_data, - tls_construct_stoc_early_data, tls_construct_ctos_early_data, NULL - }, #ifndef OPENSSL_NO_EC { TLSEXT_TYPE_ec_point_formats, @@ -282,6 +277,13 @@ static const EXTENSION_DEFINITION ext_defs[] = { EXT_TLS1_2_SERVER_HELLO | EXT_TLS1_2_AND_BELOW_ONLY, NULL, NULL, NULL, tls_construct_stoc_cryptopro_bug, NULL, NULL }, + { + TLSEXT_TYPE_early_data, + EXT_CLIENT_HELLO | EXT_TLS1_3_ENCRYPTED_EXTENSIONS, + NULL, tls_parse_ctos_early_data, tls_parse_stoc_early_data, + tls_construct_stoc_early_data, tls_construct_ctos_early_data, + final_early_data + }, { /* Must be immediately before pre_shared_key */ /* TODO(TLS1.3): Fix me */ @@ -1229,3 +1231,28 @@ int tls_psk_do_binder(SSL *s, const EVP_MD *md, const unsigned char *msgstart, return ret; } + +static int final_early_data(SSL *s, unsigned int context, int sent, int *al) +{ + if (!s->server || !sent) + return 1; + + if (s->max_early_data == 0 + || !s->hit + || s->session->ext.tick_identity != 0 + || s->early_data_state != SSL_EARLY_DATA_ACCEPTING + || !s->ext.early_data_ok + || s->hello_retry_request) { + s->ext.early_data = SSL_EARLY_DATA_REJECTED; + } else { + s->ext.early_data = SSL_EARLY_DATA_ACCEPTED; + + if (!tls13_change_cipher_state(s, + SSL3_CC_EARLY | SSL3_CHANGE_CIPHER_SERVER_READ)) { + *al = SSL_AD_INTERNAL_ERROR; + return 0; + } + } + + return 1; +} diff --git a/ssl/statem/extensions_clnt.c b/ssl/statem/extensions_clnt.c index 778d2c87e3..0af4d1b588 100644 --- a/ssl/statem/extensions_clnt.c +++ b/ssl/statem/extensions_clnt.c @@ -108,32 +108,6 @@ static int use_ecc(SSL *s) return i < end; } -int tls_construct_ctos_early_data(SSL *s, WPACKET *pkt, unsigned int context, - X509 *x, size_t chainidx, int *al) -{ - if (s->early_data_state != SSL_EARLY_DATA_CONNECTING - || s->session->ext.max_early_data == 0) { - s->max_early_data = 0; - return 1; - } - s->max_early_data = s->session->ext.max_early_data; - - if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_early_data) - || !WPACKET_start_sub_packet_u16(pkt) - || !WPACKET_close(pkt)) { - SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_EARLY_DATA, ERR_R_INTERNAL_ERROR); - return 0; - } - - /* - * We set this to rejected here. Later, if the server acknowledges the - * extension, we set it to accepted. - */ - s->ext.early_data = SSL_EARLY_DATA_REJECTED; - - return 1; -} - int tls_construct_ctos_ec_pt_formats(SSL *s, WPACKET *pkt, unsigned int context, X509 *x, size_t chainidx, int *al) { @@ -662,6 +636,32 @@ int tls_construct_ctos_key_share(SSL *s, WPACKET *pkt, unsigned int context, return 1; } +int tls_construct_ctos_early_data(SSL *s, WPACKET *pkt, unsigned int context, + X509 *x, size_t chainidx, int *al) +{ + if (s->early_data_state != SSL_EARLY_DATA_CONNECTING + || s->session->ext.max_early_data == 0) { + s->max_early_data = 0; + return 1; + } + s->max_early_data = s->session->ext.max_early_data; + + if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_early_data) + || !WPACKET_start_sub_packet_u16(pkt) + || !WPACKET_close(pkt)) { + SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_EARLY_DATA, ERR_R_INTERNAL_ERROR); + return 0; + } + + /* + * We set this to rejected here. Later, if the server acknowledges the + * extension, we set it to accepted. + */ + s->ext.early_data = SSL_EARLY_DATA_REJECTED; + + return 1; +} + #define F5_WORKAROUND_MIN_MSG_LEN 0xff #define F5_WORKAROUND_MAX_MSG_LEN 0x200 @@ -923,30 +923,6 @@ int tls_parse_stoc_early_data_info(SSL *s, PACKET *pkt, unsigned int context, return 1; } -int tls_parse_stoc_early_data(SSL *s, PACKET *pkt, unsigned int context, - X509 *x, size_t chainidx, int *al) -{ - if (PACKET_remaining(pkt) != 0) { - *al = SSL_AD_DECODE_ERROR; - return 0; - } - - if (s->ext.early_data != SSL_EARLY_DATA_REJECTED - || !s->hit - || s->session->ext.tick_identity != 0) { - /* - * If we get here then we didn't send early data, or we didn't resume - * using the first identity so the server should not be accepting it. - */ - *al = SSL_AD_ILLEGAL_PARAMETER; - return 0; - } - - s->ext.early_data = SSL_EARLY_DATA_ACCEPTED; - - return 1; -} - #ifndef OPENSSL_NO_EC int tls_parse_stoc_ec_pt_formats(SSL *s, PACKET *pkt, unsigned int context, X509 *x, size_t chainidx, int *al) @@ -1362,6 +1338,30 @@ int tls_parse_stoc_key_share(SSL *s, PACKET *pkt, unsigned int context, X509 *x, return 1; } +int tls_parse_stoc_early_data(SSL *s, PACKET *pkt, unsigned int context, + X509 *x, size_t chainidx, int *al) +{ + if (PACKET_remaining(pkt) != 0) { + *al = SSL_AD_DECODE_ERROR; + return 0; + } + + if (s->ext.early_data != SSL_EARLY_DATA_REJECTED + || !s->hit + || s->session->ext.tick_identity != 0) { + /* + * If we get here then we didn't send early data, or we didn't resume + * using the first identity so the server should not be accepting it. + */ + *al = SSL_AD_ILLEGAL_PARAMETER; + return 0; + } + + s->ext.early_data = SSL_EARLY_DATA_ACCEPTED; + + return 1; +} + int tls_parse_stoc_psk(SSL *s, PACKET *pkt, unsigned int context, X509 *x, size_t chainidx, int *al) { diff --git a/ssl/statem/extensions_srvr.c b/ssl/statem/extensions_srvr.c index 99ce6ce80d..c613143d88 100644 --- a/ssl/statem/extensions_srvr.c +++ b/ssl/statem/extensions_srvr.c @@ -162,31 +162,6 @@ int tls_parse_ctos_srp(SSL *s, PACKET *pkt, unsigned int context, X509 *x, } #endif -int tls_parse_ctos_early_data(SSL *s, PACKET *pkt, unsigned int context, - X509 *x, size_t chainidx, int *al) -{ - if (PACKET_remaining(pkt) != 0) { - *al = SSL_AD_DECODE_ERROR; - return 0; - } - - if (s->max_early_data == 0 || !s->hit || s->session->ext.tick_identity != 0 - || s->early_data_state != SSL_EARLY_DATA_ACCEPTING - || !s->ext.early_data_ok) { - s->ext.early_data = SSL_EARLY_DATA_REJECTED; - } else { - s->ext.early_data = SSL_EARLY_DATA_ACCEPTED; - - if (!tls13_change_cipher_state(s, - SSL3_CC_EARLY | SSL3_CHANGE_CIPHER_SERVER_READ)) { - *al = SSL_AD_INTERNAL_ERROR; - return 0; - } - } - - return 1; -} - #ifndef OPENSSL_NO_EC int tls_parse_ctos_ec_pt_formats(SSL *s, PACKET *pkt, unsigned int context, X509 *x, size_t chainidx, int *al) @@ -686,6 +661,18 @@ int tls_parse_ctos_ems(SSL *s, PACKET *pkt, unsigned int context, X509 *x, return 1; } + +int tls_parse_ctos_early_data(SSL *s, PACKET *pkt, unsigned int context, + X509 *x, size_t chainidx, int *al) +{ + if (PACKET_remaining(pkt) != 0) { + *al = SSL_AD_DECODE_ERROR; + return 0; + } + + return 1; +} + int tls_parse_ctos_psk(SSL *s, PACKET *pkt, unsigned int context, X509 *x, size_t chainidx, int *al) { @@ -871,22 +858,6 @@ int tls_construct_stoc_early_data_info(SSL *s, WPACKET *pkt, return 1; } -int tls_construct_stoc_early_data(SSL *s, WPACKET *pkt, unsigned int context, - X509 *x, size_t chainidx, int *al) -{ - if (s->ext.early_data != SSL_EARLY_DATA_ACCEPTED) - return 1; - - if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_early_data) - || !WPACKET_start_sub_packet_u16(pkt) - || !WPACKET_close(pkt)) { - SSLerr(SSL_F_TLS_CONSTRUCT_STOC_EARLY_DATA, ERR_R_INTERNAL_ERROR); - return 0; - } - - return 1; -} - #ifndef OPENSSL_NO_EC int tls_construct_stoc_ec_pt_formats(SSL *s, WPACKET *pkt, unsigned int context, X509 *x, size_t chainidx, int *al) @@ -1176,6 +1147,22 @@ int tls_construct_stoc_cryptopro_bug(SSL *s, WPACKET *pkt, unsigned int context, return 1; } +int tls_construct_stoc_early_data(SSL *s, WPACKET *pkt, unsigned int context, + X509 *x, size_t chainidx, int *al) +{ + if (s->ext.early_data != SSL_EARLY_DATA_ACCEPTED) + return 1; + + if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_early_data) + || !WPACKET_start_sub_packet_u16(pkt) + || !WPACKET_close(pkt)) { + SSLerr(SSL_F_TLS_CONSTRUCT_STOC_EARLY_DATA, ERR_R_INTERNAL_ERROR); + return 0; + } + + return 1; +} + int tls_construct_stoc_psk(SSL *s, WPACKET *pkt, unsigned int context, X509 *x, size_t chainidx, int *al) {