diff --git a/crypto/err/openssl.txt b/crypto/err/openssl.txt index 308abae13ed6cb49c30a80ad113b52853691bdf5..5c4ca2d6d0d3a13b37aaf770ba3fbd3cbfc2608d 100644 --- a/crypto/err/openssl.txt +++ b/crypto/err/openssl.txt @@ -1292,6 +1292,8 @@ SSL_F_TLS_CONSTRUCT_STOC_SESSION_TICKET:460:tls_construct_stoc_session_ticket SSL_F_TLS_CONSTRUCT_STOC_STATUS_REQUEST:461:tls_construct_stoc_status_request SSL_F_TLS_CONSTRUCT_STOC_SUPPORTED_GROUPS:544:\ tls_construct_stoc_supported_groups +SSL_F_TLS_CONSTRUCT_STOC_SUPPORTED_VERSIONS:608:\ + tls_construct_stoc_supported_versions SSL_F_TLS_CONSTRUCT_STOC_USE_SRTP:462:tls_construct_stoc_use_srtp SSL_F_TLS_EARLY_POST_PROCESS_CLIENT_HELLO:521:\ tls_early_post_process_client_hello @@ -1332,6 +1334,7 @@ SSL_F_TLS_PARSE_STOC_SCT:564:tls_parse_stoc_sct SSL_F_TLS_PARSE_STOC_SERVER_NAME:583:tls_parse_stoc_server_name SSL_F_TLS_PARSE_STOC_SESSION_TICKET:584:tls_parse_stoc_session_ticket SSL_F_TLS_PARSE_STOC_STATUS_REQUEST:585:tls_parse_stoc_status_request +SSL_F_TLS_PARSE_STOC_SUPPORTED_VERSIONS:609:tls_parse_stoc_supported_versions SSL_F_TLS_PARSE_STOC_USE_SRTP:446:tls_parse_stoc_use_srtp SSL_F_TLS_POST_PROCESS_CLIENT_HELLO:378:tls_post_process_client_hello SSL_F_TLS_POST_PROCESS_CLIENT_KEY_EXCHANGE:384:\ diff --git a/include/openssl/sslerr.h b/include/openssl/sslerr.h index b54459b0d07b9da2e0b8c9d1e9f40485e77a474d..364b19809e1a20c341be9922ee35e6c62ceb5bbe 100644 --- a/include/openssl/sslerr.h +++ b/include/openssl/sslerr.h @@ -340,6 +340,7 @@ int ERR_load_SSL_strings(void); # define SSL_F_TLS_CONSTRUCT_STOC_SESSION_TICKET 460 # define SSL_F_TLS_CONSTRUCT_STOC_STATUS_REQUEST 461 # define SSL_F_TLS_CONSTRUCT_STOC_SUPPORTED_GROUPS 544 +# define SSL_F_TLS_CONSTRUCT_STOC_SUPPORTED_VERSIONS 608 # define SSL_F_TLS_CONSTRUCT_STOC_USE_SRTP 462 # define SSL_F_TLS_EARLY_POST_PROCESS_CLIENT_HELLO 521 # define SSL_F_TLS_FINISH_HANDSHAKE 597 @@ -379,6 +380,7 @@ int ERR_load_SSL_strings(void); # define SSL_F_TLS_PARSE_STOC_SERVER_NAME 583 # define SSL_F_TLS_PARSE_STOC_SESSION_TICKET 584 # define SSL_F_TLS_PARSE_STOC_STATUS_REQUEST 585 +# define SSL_F_TLS_PARSE_STOC_SUPPORTED_VERSIONS 609 # define SSL_F_TLS_PARSE_STOC_USE_SRTP 446 # define SSL_F_TLS_POST_PROCESS_CLIENT_HELLO 378 # define SSL_F_TLS_POST_PROCESS_CLIENT_KEY_EXCHANGE 384 diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c index 7b201e0260cb084752ff5f466b2dbb6b7e262c6d..d608daff072454b98b1e61ce9775caf6aca8a2aa 100644 --- a/ssl/ssl_err.c +++ b/ssl/ssl_err.c @@ -524,6 +524,8 @@ static const ERR_STRING_DATA SSL_str_functs[] = { "tls_construct_stoc_status_request"}, {ERR_PACK(ERR_LIB_SSL, SSL_F_TLS_CONSTRUCT_STOC_SUPPORTED_GROUPS, 0), "tls_construct_stoc_supported_groups"}, + {ERR_PACK(ERR_LIB_SSL, SSL_F_TLS_CONSTRUCT_STOC_SUPPORTED_VERSIONS, 0), + "tls_construct_stoc_supported_versions"}, {ERR_PACK(ERR_LIB_SSL, SSL_F_TLS_CONSTRUCT_STOC_USE_SRTP, 0), "tls_construct_stoc_use_srtp"}, {ERR_PACK(ERR_LIB_SSL, SSL_F_TLS_EARLY_POST_PROCESS_CLIENT_HELLO, 0), @@ -593,6 +595,8 @@ static const ERR_STRING_DATA SSL_str_functs[] = { "tls_parse_stoc_session_ticket"}, {ERR_PACK(ERR_LIB_SSL, SSL_F_TLS_PARSE_STOC_STATUS_REQUEST, 0), "tls_parse_stoc_status_request"}, + {ERR_PACK(ERR_LIB_SSL, SSL_F_TLS_PARSE_STOC_SUPPORTED_VERSIONS, 0), + "tls_parse_stoc_supported_versions"}, {ERR_PACK(ERR_LIB_SSL, SSL_F_TLS_PARSE_STOC_USE_SRTP, 0), "tls_parse_stoc_use_srtp"}, {ERR_PACK(ERR_LIB_SSL, SSL_F_TLS_POST_PROCESS_CLIENT_HELLO, 0), diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h index 952a8f9ccca7a84dc36f207287a8e0a8c4227efa..ed6b9a8fd43b5256d0d29e533dd6df4e3aef9374 100644 --- a/ssl/ssl_locl.h +++ b/ssl/ssl_locl.h @@ -2266,7 +2266,8 @@ __owur int ssl_check_version_downgrade(SSL *s); __owur int ssl_set_version_bound(int method_version, int version, int *bound); __owur int ssl_choose_server_version(SSL *s, CLIENTHELLO_MSG *hello, DOWNGRADE *dgrd); -__owur int ssl_choose_client_version(SSL *s, int version, int checkdgrd); +__owur int ssl_choose_client_version(SSL *s, int version, + RAW_EXTENSION *extensions); int ssl_get_min_max_version(const SSL *s, int *min_version, int *max_version); __owur long tls1_default_timeout(void); diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c index 464a5ef5eb4052befea48735c774ffc9bc7d698c..988e91904438e6cfa16e94c650d820ce8f1eaf1e 100644 --- a/ssl/statem/extensions.c +++ b/ssl/statem/extensions.c @@ -9,6 +9,7 @@ #include #include "internal/nelem.h" +#include "internal/cryptlib.h" #include "../ssl_locl.h" #include "statem_locl.h" @@ -261,11 +262,13 @@ static const EXTENSION_DEFINITION ext_defs[] = { }, { TLSEXT_TYPE_supported_versions, - SSL_EXT_CLIENT_HELLO | SSL_EXT_TLS_IMPLEMENTATION_ONLY - | SSL_EXT_TLS1_3_ONLY, + SSL_EXT_CLIENT_HELLO | SSL_EXT_TLS1_2_SERVER_HELLO + | SSL_EXT_TLS1_3_SERVER_HELLO | SSL_EXT_TLS_IMPLEMENTATION_ONLY, NULL, /* Processed inline as part of version selection */ - NULL, NULL, NULL, tls_construct_ctos_supported_versions, NULL + NULL, tls_parse_stoc_supported_versions, + tls_construct_stoc_supported_versions, + tls_construct_ctos_supported_versions, NULL }, { TLSEXT_TYPE_psk_kex_modes, @@ -357,6 +360,44 @@ static int validate_context(SSL *s, unsigned int extctx, unsigned int thisctx) return 1; } +int tls_validate_all_contexts(SSL *s, unsigned int thisctx, RAW_EXTENSION *exts) +{ + size_t i, num_exts, builtin_num = OSSL_NELEM(ext_defs), offset; + RAW_EXTENSION *thisext; + unsigned int context; + ENDPOINT role = ENDPOINT_BOTH; + + if ((thisctx & SSL_EXT_CLIENT_HELLO) != 0) + role = ENDPOINT_SERVER; + else if ((thisctx & SSL_EXT_TLS1_2_SERVER_HELLO) != 0) + role = ENDPOINT_CLIENT; + + /* Calculate the number of extensions in the extensions list */ + num_exts = builtin_num + s->cert->custext.meths_count; + + for (thisext = exts, i = 0; i < num_exts; i++, thisext++) { + if (!thisext->present) + continue; + + if (i < builtin_num) { + context = ext_defs[i].context; + } else { + custom_ext_method *meth = NULL; + + meth = custom_ext_find(&s->cert->custext, role, thisext->type, + &offset); + if (!ossl_assert(meth != NULL)) + return 0; + context = meth->context; + } + + if (!validate_context(s, context, thisctx)) + return 0; + } + + return 1; +} + /* * Verify whether we are allowed to use the extension |type| in the current * |context|. Returns 1 to indicate the extension is allowed or unknown or 0 to diff --git a/ssl/statem/extensions_clnt.c b/ssl/statem/extensions_clnt.c index b7ef54e8b73ddb16698609878029dba966b60f71..2640756134f14323dd74539a358fe5fa08eb279a 100644 --- a/ssl/statem/extensions_clnt.c +++ b/ssl/statem/extensions_clnt.c @@ -507,6 +507,20 @@ EXT_RETURN tls_construct_ctos_supported_versions(SSL *s, WPACKET *pkt, { int currv, min_version, max_version, reason; + reason = ssl_get_min_max_version(s, &min_version, &max_version); + if (reason != 0) { + SSLfatal(s, SSL_AD_INTERNAL_ERROR, + SSL_F_TLS_CONSTRUCT_CTOS_SUPPORTED_VERSIONS, reason); + return EXT_RETURN_FAIL; + } + + /* + * Don't include this if we can't negotiate TLSv1.3. We can do a straight + * comparison here because we will never be called in DTLS. + */ + if (max_version < TLS1_3_VERSION) + return EXT_RETURN_NOT_SENT; + if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_supported_versions) || !WPACKET_start_sub_packet_u16(pkt) || !WPACKET_start_sub_packet_u8(pkt)) { @@ -516,13 +530,6 @@ EXT_RETURN tls_construct_ctos_supported_versions(SSL *s, WPACKET *pkt, return EXT_RETURN_FAIL; } - reason = ssl_get_min_max_version(s, &min_version, &max_version); - if (reason != 0) { - SSLfatal(s, SSL_AD_INTERNAL_ERROR, - SSL_F_TLS_CONSTRUCT_CTOS_SUPPORTED_VERSIONS, reason); - return EXT_RETURN_FAIL; - } - /* * TODO(TLS1.3): There is some discussion on the TLS list as to whether * we should include versions version = version; + + return 1; +} + int tls_parse_stoc_key_share(SSL *s, PACKET *pkt, unsigned int context, X509 *x, size_t chainidx) { diff --git a/ssl/statem/extensions_srvr.c b/ssl/statem/extensions_srvr.c index b07376fe9e6a7387c956bb704d6f32c3bd12b2b0..93ac98f11608bd791248c3a6974ace4b2940b012 100644 --- a/ssl/statem/extensions_srvr.c +++ b/ssl/statem/extensions_srvr.c @@ -1213,6 +1213,27 @@ EXT_RETURN tls_construct_stoc_ems(SSL *s, WPACKET *pkt, unsigned int context, return EXT_RETURN_SENT; } +EXT_RETURN tls_construct_stoc_supported_versions(SSL *s, WPACKET *pkt, + unsigned int context, X509 *x, + size_t chainidx) +{ + if (!SSL_IS_TLS13(s)) + return EXT_RETURN_NOT_SENT; + + if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_supported_versions) + || !WPACKET_start_sub_packet_u16(pkt) + /* TODO(TLS1.3): Update to remove the TLSv1.3 draft indicator */ + || !WPACKET_put_bytes_u16(pkt, TLS1_3_VERSION_DRAFT) + || !WPACKET_close(pkt)) { + SSLfatal(s, SSL_AD_INTERNAL_ERROR, + SSL_F_TLS_CONSTRUCT_STOC_SUPPORTED_VERSIONS, + ERR_R_INTERNAL_ERROR); + return EXT_RETURN_FAIL; + } + + return EXT_RETURN_SENT; +} + EXT_RETURN tls_construct_stoc_key_share(SSL *s, WPACKET *pkt, unsigned int context, X509 *x, size_t chainidx) diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c index a7a5a1723b8a2e06b24a0b284dc8307f8d5d6ea6..3c628bdd99e42a29589fa668c56ba089efa605e1 100644 --- a/ssl/statem/statem_clnt.c +++ b/ssl/statem/statem_clnt.c @@ -1332,60 +1332,30 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL *s, PACKET *pkt) goto err; } - /* - * We do this immediately so we know what format the ServerHello is in. - * Must be done after reading the random data so we can check for the - * TLSv1.3 downgrade sentinels - */ - if (!ssl_choose_client_version(s, sversion, 1)) { - /* SSLfatal() already called */ + /* Get the session-id. */ + if (!PACKET_get_length_prefixed_1(pkt, &session_id)) { + SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_F_TLS_PROCESS_SERVER_HELLO, + SSL_R_LENGTH_MISMATCH); goto err; } - - /* - * In TLSv1.3 a ServerHello message signals a key change so the end of the - * message must be on a record boundary. - */ - if (SSL_IS_TLS13(s) && RECORD_LAYER_processed_read_pending(&s->rlayer)) { - SSLfatal(s, SSL_AD_UNEXPECTED_MESSAGE, SSL_F_TLS_PROCESS_SERVER_HELLO, - SSL_R_NOT_ON_RECORD_BOUNDARY); + session_id_len = PACKET_remaining(&session_id); + if (session_id_len > sizeof(s->session->session_id) + || session_id_len > SSL3_SESSION_ID_SIZE) { + SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_F_TLS_PROCESS_SERVER_HELLO, + SSL_R_SSL3_SESSION_ID_TOO_LONG); goto err; } - /* Get the session-id. */ - if (!SSL_IS_TLS13(s)) { - if (!PACKET_get_length_prefixed_1(pkt, &session_id)) { - SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_F_TLS_PROCESS_SERVER_HELLO, - SSL_R_LENGTH_MISMATCH); - goto err; - } - session_id_len = PACKET_remaining(&session_id); - if (session_id_len > sizeof(s->session->session_id) - || session_id_len > SSL3_SESSION_ID_SIZE) { - SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, - SSL_F_TLS_PROCESS_SERVER_HELLO, - SSL_R_SSL3_SESSION_ID_TOO_LONG); - goto err; - } - } else { - PACKET_null_init(&session_id); - session_id_len = 0; - } - if (!PACKET_get_bytes(pkt, &cipherchars, TLS_CIPHER_LEN)) { SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_F_TLS_PROCESS_SERVER_HELLO, SSL_R_LENGTH_MISMATCH); goto err; } - if (!SSL_IS_TLS13(s)) { - if (!PACKET_get_1(pkt, &compression)) { - SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_F_TLS_PROCESS_SERVER_HELLO, - SSL_R_LENGTH_MISMATCH); - goto err; - } - } else { - compression = 0; + if (!PACKET_get_1(pkt, &compression)) { + SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_F_TLS_PROCESS_SERVER_HELLO, + SSL_R_LENGTH_MISMATCH); + goto err; } /* TLS extensions */ @@ -1398,10 +1368,44 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL *s, PACKET *pkt) goto err; } + if (!tls_collect_extensions(s, &extpkt, + SSL_EXT_TLS1_2_SERVER_HELLO + | SSL_EXT_TLS1_3_SERVER_HELLO, + &extensions, NULL, 1)) { + /* SSLfatal() already called */ + goto err; + } + + if (!ssl_choose_client_version(s, sversion, extensions)) { + /* SSLfatal() already called */ + goto err; + } + + /* + * Now we have chosen the version we need to check again that the extensions + * are appropriate for this version. + */ context = SSL_IS_TLS13(s) ? SSL_EXT_TLS1_3_SERVER_HELLO : SSL_EXT_TLS1_2_SERVER_HELLO; - if (!tls_collect_extensions(s, &extpkt, context, &extensions, NULL, 1)) { - /* SSLfatal() already called */ + if (!tls_validate_all_contexts(s, context, extensions)) { + SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_F_TLS_PROCESS_SERVER_HELLO, + SSL_R_BAD_EXTENSION); + goto err; + } + + /* + * In TLSv1.3 a ServerHello message signals a key change so the end of the + * message must be on a record boundary. + */ + if (SSL_IS_TLS13(s) && RECORD_LAYER_processed_read_pending(&s->rlayer)) { + SSLfatal(s, SSL_AD_UNEXPECTED_MESSAGE, SSL_F_TLS_PROCESS_SERVER_HELLO, + SSL_R_NOT_ON_RECORD_BOUNDARY); + goto err; + } + + if (SSL_IS_TLS13(s) && compression != 0) { + SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_F_TLS_PROCESS_SERVER_HELLO, + SSL_R_INVALID_COMPRESSION_ALGORITHM); goto err; } @@ -1411,7 +1415,7 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL *s, PACKET *pkt) /* This will set s->hit if we are resuming */ if (!tls_parse_extension(s, TLSEXT_IDX_psk, SSL_EXT_TLS1_3_SERVER_HELLO, - extensions, NULL, 0l)) { + extensions, NULL, 0)) { /* SSLfatal() already called */ goto err; } diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c index b8e094bdc46edab8c64640604f3d8f420a1cf10c..7b1811543cc94269ad1adb966f8b68e81a1acc31 100644 --- a/ssl/statem/statem_lib.c +++ b/ssl/statem/statem_lib.c @@ -1739,21 +1739,31 @@ int ssl_choose_server_version(SSL *s, CLIENTHELLO_MSG *hello, DOWNGRADE *dgrd) * * @s: client SSL handle. * @version: The proposed version from the server's HELLO. - * @checkdgrd: Whether to check the downgrade sentinels in the server_random + * @extensions: The extensions received * * Returns 1 on success or 0 on error. */ -int ssl_choose_client_version(SSL *s, int version, int checkdgrd) +int ssl_choose_client_version(SSL *s, int version, RAW_EXTENSION *extensions) { const version_info *vent; const version_info *table; int highver = 0; + int origv; - /* TODO(TLS1.3): Remove this before release */ - if (version == TLS1_3_VERSION_DRAFT) - version = TLS1_3_VERSION; + origv = s->version; + s->version = version; - if (s->hello_retry_request && version != TLS1_3_VERSION) { + /* This will overwrite s->version if the extension is present */ + if (!tls_parse_extension(s, TLSEXT_IDX_supported_versions, + SSL_EXT_TLS1_2_SERVER_HELLO + | SSL_EXT_TLS1_3_SERVER_HELLO, extensions, + NULL, 0)) { + s->version = origv; + return 0; + } + + if (s->hello_retry_request && s->version != TLS1_3_VERSION) { + s->version = origv; SSLfatal(s, SSL_AD_PROTOCOL_VERSION, SSL_F_SSL_CHOOSE_CLIENT_VERSION, SSL_R_WRONG_SSL_VERSION); return 0; @@ -1761,7 +1771,8 @@ int ssl_choose_client_version(SSL *s, int version, int checkdgrd) switch (s->method->version) { default: - if (version != s->version) { + if (s->version != s->method->version) { + s->version = origv; SSLfatal(s, SSL_AD_PROTOCOL_VERSION, SSL_F_SSL_CHOOSE_CLIENT_VERSION, SSL_R_WRONG_SSL_VERSION); @@ -1790,13 +1801,14 @@ int ssl_choose_client_version(SSL *s, int version, int checkdgrd) if (vent->cmeth == NULL) continue; - if (highver != 0 && version != vent->version) + if (highver != 0 && s->version != vent->version) continue; method = vent->cmeth(); err = ssl_method_error(s, method); if (err != 0) { - if (version == vent->version) { + if (s->version == vent->version) { + s->version = origv; SSLfatal(s, SSL_AD_PROTOCOL_VERSION, SSL_F_SSL_CHOOSE_CLIENT_VERSION, err); return 0; @@ -1807,43 +1819,43 @@ int ssl_choose_client_version(SSL *s, int version, int checkdgrd) if (highver == 0) highver = vent->version; - if (version != vent->version) + if (s->version != vent->version) continue; #ifndef OPENSSL_NO_TLS13DOWNGRADE /* Check for downgrades */ - if (checkdgrd) { - if (version == TLS1_2_VERSION && highver > version) { - if (memcmp(tls12downgrade, - s->s3->server_random + SSL3_RANDOM_SIZE - - sizeof(tls12downgrade), - sizeof(tls12downgrade)) == 0) { - SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, - SSL_F_SSL_CHOOSE_CLIENT_VERSION, - SSL_R_INAPPROPRIATE_FALLBACK); - return 0; - } - } else if (!SSL_IS_DTLS(s) - && version < TLS1_2_VERSION - && highver > version) { - if (memcmp(tls11downgrade, - s->s3->server_random + SSL3_RANDOM_SIZE - - sizeof(tls11downgrade), - sizeof(tls11downgrade)) == 0) { - SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, - SSL_F_SSL_CHOOSE_CLIENT_VERSION, - SSL_R_INAPPROPRIATE_FALLBACK); - return 0; - } + if (s->version == TLS1_2_VERSION && highver > s->version) { + if (memcmp(tls12downgrade, + s->s3->server_random + SSL3_RANDOM_SIZE + - sizeof(tls12downgrade), + sizeof(tls12downgrade)) == 0) { + s->version = origv; + SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, + SSL_F_SSL_CHOOSE_CLIENT_VERSION, + SSL_R_INAPPROPRIATE_FALLBACK); + return 0; + } + } else if (!SSL_IS_DTLS(s) + && s->version < TLS1_2_VERSION + && highver > s->version) { + if (memcmp(tls11downgrade, + s->s3->server_random + SSL3_RANDOM_SIZE + - sizeof(tls11downgrade), + sizeof(tls11downgrade)) == 0) { + s->version = origv; + SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, + SSL_F_SSL_CHOOSE_CLIENT_VERSION, + SSL_R_INAPPROPRIATE_FALLBACK); + return 0; } } #endif s->method = method; - s->version = version; return 1; } + s->version = origv; SSLfatal(s, SSL_AD_PROTOCOL_VERSION, SSL_F_SSL_CHOOSE_CLIENT_VERSION, SSL_R_UNSUPPORTED_PROTOCOL); return 0; diff --git a/ssl/statem/statem_locl.h b/ssl/statem/statem_locl.h index 888c0b512f4372744e84cf9453f13cd0fb6582f7..ad5b028da76ca73e06901f5300abeb37019f539e 100644 --- a/ssl/statem/statem_locl.h +++ b/ssl/statem/statem_locl.h @@ -161,6 +161,8 @@ typedef enum ext_return_en { EXT_RETURN_NOT_SENT } EXT_RETURN; +__owur int tls_validate_all_contexts(SSL *s, unsigned int thisctx, + RAW_EXTENSION *exts); __owur int extension_is_relevant(SSL *s, unsigned int extctx, unsigned int thisctx); __owur int tls_collect_extensions(SSL *s, PACKET *packet, unsigned int context, @@ -271,6 +273,9 @@ EXT_RETURN tls_construct_stoc_etm(SSL *s, WPACKET *pkt, unsigned int context, X509 *x, size_t chainidx); EXT_RETURN tls_construct_stoc_ems(SSL *s, WPACKET *pkt, unsigned int context, X509 *x, size_t chainidx); +EXT_RETURN tls_construct_stoc_supported_versions(SSL *s, WPACKET *pkt, + unsigned int context, X509 *x, + size_t chainidx); EXT_RETURN tls_construct_stoc_key_share(SSL *s, WPACKET *pkt, unsigned int context, X509 *x, size_t chainidx); @@ -388,6 +393,8 @@ int tls_parse_stoc_etm(SSL *s, PACKET *pkt, unsigned int context, X509 *x, size_t chainidx); int tls_parse_stoc_ems(SSL *s, PACKET *pkt, unsigned int context, X509 *x, size_t chainidx); +int tls_parse_stoc_supported_versions(SSL *s, PACKET *pkt, unsigned int context, + X509 *x, size_t chainidx); int tls_parse_stoc_key_share(SSL *s, PACKET *pkt, unsigned int context, X509 *x, size_t chainidx); int tls_parse_stoc_cookie(SSL *s, PACKET *pkt, unsigned int context, X509 *x, diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c index f95c19b41bf7c7071f8147252222f7940d1f0355..e91bba4c0f82af71f4c2d52f5573f3004fdd9ffb 100644 --- a/ssl/statem/statem_srvr.c +++ b/ssl/statem/statem_srvr.c @@ -2193,8 +2193,7 @@ int tls_construct_server_hello(SSL *s, WPACKET *pkt) size_t sl, len; int version; - /* TODO(TLS1.3): Remove the DRAFT conditional before release */ - version = SSL_IS_TLS13(s) ? TLS1_3_VERSION_DRAFT : s->version; + version = SSL_IS_TLS13(s) ? TLS1_2_VERSION : s->version; if (!WPACKET_put_bytes_u16(pkt, version) /* * Random stuff. Filling of the server_random takes place in @@ -2234,21 +2233,23 @@ int tls_construct_server_hello(SSL *s, WPACKET *pkt) return 0; } + /* Never send a session_id back in the ServerHello */ + if (SSL_IS_TLS13(s)) + sl = 0; + /* set up the compression method */ #ifdef OPENSSL_NO_COMP compm = 0; #else - if (s->s3->tmp.new_compression == NULL) + if (SSL_IS_TLS13(s) || s->s3->tmp.new_compression == NULL) compm = 0; else compm = s->s3->tmp.new_compression->id; #endif - if ((!SSL_IS_TLS13(s) - && !WPACKET_sub_memcpy_u8(pkt, s->session->session_id, sl)) + if (!WPACKET_sub_memcpy_u8(pkt, s->session->session_id, sl) || !s->method->put_cipher_by_char(s->s3->tmp.new_cipher, pkt, &len) - || (!SSL_IS_TLS13(s) - && !WPACKET_put_bytes_u8(pkt, compm)) + || !WPACKET_put_bytes_u8(pkt, compm) || !tls_construct_extensions(s, pkt, SSL_IS_TLS13(s) ? SSL_EXT_TLS1_3_SERVER_HELLO diff --git a/ssl/t1_trce.c b/ssl/t1_trce.c index 13e306251a08fa575412d2a9d45afc2c92f463ae..6a8314bf7373f3d8ede3691bf4f2afe0e406df6b 100644 --- a/ssl/t1_trce.c +++ b/ssl/t1_trce.c @@ -823,6 +823,17 @@ static int ssl_print_extension(BIO *bio, int indent, int server, break; case TLSEXT_TYPE_supported_versions: + if (server) { + int version; + + if (extlen != 2) + return 0; + version = (ext[0] << 8) | ext[1]; + BIO_indent(bio, indent + 4, 80); + BIO_printf(bio, "%s (%d)\n", + ssl_trace_str(version, ssl_version_tbl), version); + break; + } if (extlen < 1) return 0; xlen = ext[0]; diff --git a/test/asynciotest.c b/test/asynciotest.c index fdb977025128cba6da17e40a0c3afa513504724c..179fe2620d13221b5d21305449133e5ed2a03a5f 100644 --- a/test/asynciotest.c +++ b/test/asynciotest.c @@ -146,7 +146,7 @@ static int async_write(BIO *bio, const char *in, int inl) return -1; while (PACKET_remaining(&pkt) > 0) { - PACKET payload, wholebody; + PACKET payload, wholebody, sessionid, extensions; unsigned int contenttype, versionhi, versionlo, data; unsigned int msgtype = 0, negversion = 0; @@ -164,11 +164,43 @@ static int async_write(BIO *bio, const char *in, int inl) && !PACKET_get_1(&wholebody, &msgtype)) return -1; - if (msgtype == SSL3_MT_SERVER_HELLO - && (!PACKET_forward(&wholebody, + if (msgtype == SSL3_MT_SERVER_HELLO) { + if (!PACKET_forward(&wholebody, SSL3_HM_HEADER_LENGTH - 1) - || !PACKET_get_net_2(&wholebody, &negversion))) - return -1; + || !PACKET_get_net_2(&wholebody, &negversion) + /* Skip random (32 bytes) */ + || !PACKET_forward(&wholebody, 32) + /* Skip session id */ + || !PACKET_get_length_prefixed_1(&wholebody, + &sessionid) + /* + * Skip ciphersuite (2 bytes) and compression + * method (1 byte) + */ + || !PACKET_forward(&wholebody, 2 + 1) + || !PACKET_get_length_prefixed_2(&wholebody, + &extensions)) + return -1; + + /* + * Find the negotiated version in supported_versions + * extension, if present. + */ + while (PACKET_remaining(&extensions)) { + unsigned int type; + PACKET extbody; + + if (!PACKET_get_net_2(&extensions, &type) + || !PACKET_get_length_prefixed_2(&extensions, + &extbody)) + return -1; + + if (type == TLSEXT_TYPE_supported_versions + && (!PACKET_get_net_2(&extbody, &negversion) + || PACKET_remaining(&extbody) != 0)) + return -1; + } + } while (PACKET_get_1(&payload, &data)) { /* Create a new one byte long record for each byte in the diff --git a/test/recipes/70-test_tls13kexmodes.t b/test/recipes/70-test_tls13kexmodes.t index 97cbf7676b82048aa8a7ab47bf5bb4b4004499e3..dcc51aecdd97ffd7d31a0274d7d67ddcb9b70830 100644 --- a/test/recipes/70-test_tls13kexmodes.t +++ b/test/recipes/70-test_tls13kexmodes.t @@ -122,6 +122,8 @@ $ENV{CTLOG_FILE} = srctop_file("test", "ct", "log_list.conf"); [TLSProxy::Message::MT_CLIENT_HELLO, TLSProxy::Message::EXT_PSK, checkhandshake::PSK_CLI_EXTENSION], + [TLSProxy::Message::MT_SERVER_HELLO, TLSProxy::Message::EXT_SUPPORTED_VERSIONS, + checkhandshake::DEFAULT_EXTENSIONS], [TLSProxy::Message::MT_SERVER_HELLO, TLSProxy::Message::EXT_KEY_SHARE, checkhandshake::KEY_SHARE_SRV_EXTENSION], [TLSProxy::Message::MT_SERVER_HELLO, TLSProxy::Message::EXT_PSK, diff --git a/test/recipes/70-test_tls13messages.t b/test/recipes/70-test_tls13messages.t index 5bd2a966c7208ad3278a02ca1d62fb6115db9eb1..9319e8492d0e33d5d8f2cc47fb69e99412df8d8f 100644 --- a/test/recipes/70-test_tls13messages.t +++ b/test/recipes/70-test_tls13messages.t @@ -122,6 +122,8 @@ $ENV{CTLOG_FILE} = srctop_file("test", "ct", "log_list.conf"); [TLSProxy::Message::MT_CLIENT_HELLO, TLSProxy::Message::EXT_PSK, checkhandshake::PSK_CLI_EXTENSION], + [TLSProxy::Message::MT_SERVER_HELLO, TLSProxy::Message::EXT_SUPPORTED_VERSIONS, + checkhandshake::DEFAULT_EXTENSIONS], [TLSProxy::Message::MT_SERVER_HELLO, TLSProxy::Message::EXT_KEY_SHARE, checkhandshake::DEFAULT_EXTENSIONS], [TLSProxy::Message::MT_SERVER_HELLO, TLSProxy::Message::EXT_PSK, diff --git a/util/perl/TLSProxy/ServerHello.pm b/util/perl/TLSProxy/ServerHello.pm index 1abdd053e152aa71de8891fc3ddbc2ede7af316c..3e403e52d88c644a0e62a9526353c45469fd0db1 100644 --- a/util/perl/TLSProxy/ServerHello.pm +++ b/util/perl/TLSProxy/ServerHello.pm @@ -46,29 +46,21 @@ sub parse my $ptr = 2; my ($server_version) = unpack('n', $self->data); - # TODO(TLS1.3): Replace this reference to draft version before release - if ($server_version == TLSProxy::Record::VERS_TLS_1_3_DRAFT) { - $server_version = TLSProxy::Record::VERS_TLS_1_3; - TLSProxy::Proxy->is_tls13(1); - } - my $random = substr($self->data, $ptr, 32); $ptr += 32; my $session_id_len = 0; my $session = ""; - if (!TLSProxy::Proxy->is_tls13()) { - $session_id_len = unpack('C', substr($self->data, $ptr)); - $ptr++; - $session = substr($self->data, $ptr, $session_id_len); - $ptr += $session_id_len; - } + $session_id_len = unpack('C', substr($self->data, $ptr)); + $ptr++; + $session = substr($self->data, $ptr, $session_id_len); + $ptr += $session_id_len; + my $ciphersuite = unpack('n', substr($self->data, $ptr)); $ptr += 2; my $comp_meth = 0; - if (!TLSProxy::Proxy->is_tls13()) { - $comp_meth = unpack('C', substr($self->data, $ptr)); - $ptr++; - } + $comp_meth = unpack('C', substr($self->data, $ptr)); + $ptr++; + my $extensions_len = unpack('n', substr($self->data, $ptr)); if (!defined $extensions_len) { $extensions_len = 0; @@ -96,6 +88,15 @@ sub parse my $extdata = substr($extension_data, 4, $size); $extension_data = substr($extension_data, 4 + $size); $extensions{$type} = $extdata; + if ($type == TLSProxy::Message::EXT_SUPPORTED_VERSIONS) { + $server_version = unpack('n', $extdata); + } + } + + # TODO(TLS1.3): Replace this reference to draft version before release + if ($server_version == TLSProxy::Record::VERS_TLS_1_3_DRAFT) { + $server_version = TLSProxy::Record::VERS_TLS_1_3; + TLSProxy::Proxy->is_tls13(1); } $self->server_version($server_version); @@ -138,14 +139,10 @@ sub set_message_contents $data = pack('n', $self->server_version); $data .= $self->random; - if (!TLSProxy::Proxy->is_tls13()) { - $data .= pack('C', $self->session_id_len); - $data .= $self->session; - } + $data .= pack('C', $self->session_id_len); + $data .= $self->session; $data .= pack('n', $self->ciphersuite); - if (!TLSProxy::Proxy->is_tls13()) { - $data .= pack('C', $self->comp_meth); - } + $data .= pack('C', $self->comp_meth); foreach my $key (keys %{$self->extension_data}) { my $extdata = ${$self->extension_data}{$key};