From 4b299b8e174cd58f762f0f184ceac7955e4227c4 Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Thu, 24 Nov 2016 18:25:10 +0000 Subject: [PATCH] Add extensions construction support Perl changes reviewed by Richard Levitte. Non-perl changes reviewed by Rich Salz Reviewed-by: Rich Salz Reviewed-by: Richard Levitte --- include/openssl/ssl.h | 4 +++ ssl/ssl_err.c | 7 +++++ ssl/statem/extensions.c | 63 ++++++++++++++++++++++++---------------- ssl/statem/statem_locl.h | 8 +++-- ssl/statem/statem_srvr.c | 9 +++--- 5 files changed, 59 insertions(+), 32 deletions(-) diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index b500446b1b..95b53925ac 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h @@ -2270,6 +2270,7 @@ int ERR_load_SSL_strings(void); # define SSL_F_TLS_CONSTRUCT_CLIENT_KEY_EXCHANGE 357 # define SSL_F_TLS_CONSTRUCT_CLIENT_VERIFY 358 # define SSL_F_TLS_CONSTRUCT_ENCRYPTED_EXTENSIONS 443 +# 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_NEW_SESSION_TICKET 428 @@ -2281,6 +2282,8 @@ int ERR_load_SSL_strings(void); # define SSL_F_TLS_GET_MESSAGE_BODY 351 # define SSL_F_TLS_GET_MESSAGE_HEADER 387 # define SSL_F_TLS_PARSE_CLIENTHELLO_KEY_SHARE 445 +# define SSL_F_TLS_PARSE_CLIENTHELLO_RENEGOTIATE 448 +# define SSL_F_TLS_PARSE_CLIENTHELLO_TLSEXT 449 # define SSL_F_TLS_PARSE_CLIENTHELLO_USE_SRTP 446 # define SSL_F_TLS_POST_PROCESS_CLIENT_HELLO 378 # define SSL_F_TLS_POST_PROCESS_CLIENT_KEY_EXCHANGE 384 @@ -2311,6 +2314,7 @@ int ERR_load_SSL_strings(void); # define SSL_F_TLS_PROCESS_SKE_ECDHE 420 # define SSL_F_TLS_PROCESS_SKE_PSK_PREAMBLE 421 # define SSL_F_TLS_PROCESS_SKE_SRP 422 +# define SSL_F_TLS_SCAN_CLIENTHELLO_TLSEXT 450 # define SSL_F_USE_CERTIFICATE_CHAIN_FILE 220 /* Reason codes. */ diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c index b59da2b263..b5b9857ebd 100644 --- a/ssl/ssl_err.c +++ b/ssl/ssl_err.c @@ -275,6 +275,7 @@ static ERR_STRING_DATA SSL_str_functs[] = { "tls_construct_client_verify"}, {ERR_FUNC(SSL_F_TLS_CONSTRUCT_ENCRYPTED_EXTENSIONS), "tls_construct_encrypted_extensions"}, + {ERR_FUNC(SSL_F_TLS_CONSTRUCT_EXTENSIONS), "tls_construct_extensions"}, {ERR_FUNC(SSL_F_TLS_CONSTRUCT_FINISHED), "tls_construct_finished"}, {ERR_FUNC(SSL_F_TLS_CONSTRUCT_HELLO_REQUEST), "tls_construct_hello_request"}, @@ -292,6 +293,10 @@ static ERR_STRING_DATA SSL_str_functs[] = { {ERR_FUNC(SSL_F_TLS_GET_MESSAGE_HEADER), "tls_get_message_header"}, {ERR_FUNC(SSL_F_TLS_PARSE_CLIENTHELLO_KEY_SHARE), "tls_parse_clienthello_key_share"}, + {ERR_FUNC(SSL_F_TLS_PARSE_CLIENTHELLO_RENEGOTIATE), + "tls_parse_clienthello_renegotiate"}, + {ERR_FUNC(SSL_F_TLS_PARSE_CLIENTHELLO_TLSEXT), + "tls_parse_clienthello_tlsext"}, {ERR_FUNC(SSL_F_TLS_PARSE_CLIENTHELLO_USE_SRTP), "tls_parse_clienthello_use_srtp"}, {ERR_FUNC(SSL_F_TLS_POST_PROCESS_CLIENT_HELLO), @@ -336,6 +341,8 @@ static ERR_STRING_DATA SSL_str_functs[] = { {ERR_FUNC(SSL_F_TLS_PROCESS_SKE_PSK_PREAMBLE), "tls_process_ske_psk_preamble"}, {ERR_FUNC(SSL_F_TLS_PROCESS_SKE_SRP), "tls_process_ske_srp"}, + {ERR_FUNC(SSL_F_TLS_SCAN_CLIENTHELLO_TLSEXT), + "tls_scan_clienthello_tlsext"}, {ERR_FUNC(SSL_F_USE_CERTIFICATE_CHAIN_FILE), "use_certificate_chain_file"}, {0, NULL} diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c index 82f1565a95..c98a2055c2 100644 --- a/ssl/statem/extensions.c +++ b/ssl/statem/extensions.c @@ -45,6 +45,11 @@ typedef struct { unsigned int context; } EXTENSION_DEFINITION; +/* + * TODO(TLS1.3): Temporarily modified the definitions below to put all TLS1.3 + * extensions in the ServerHello for now. That needs to be put back to correct + * setting once encrypted extensions is working properly. + */ static const EXTENSION_DEFINITION ext_defs[] = { { TLSEXT_TYPE_renegotiate, @@ -62,7 +67,7 @@ static const EXTENSION_DEFINITION ext_defs[] = { NULL, NULL, EXT_CLIENT_HELLO | EXT_TLS1_2_SERVER_HELLO - | EXT_TLS1_3_ENCRYPTED_EXTENSIONS + | /*EXT_TLS1_3_ENCRYPTED_EXTENSIONS*/EXT_TLS1_3_SERVER_HELLO }, #ifndef OPENSSL_NO_SRP { @@ -89,7 +94,8 @@ static const EXTENSION_DEFINITION ext_defs[] = { NULL, NULL, NULL, - EXT_CLIENT_HELLO | EXT_TLS1_3_ENCRYPTED_EXTENSIONS + EXT_CLIENT_HELLO + | /*EXT_TLS1_3_ENCRYPTED_EXTENSIONS*/EXT_TLS1_3_SERVER_HELLO }, #endif { @@ -114,7 +120,8 @@ static const EXTENSION_DEFINITION ext_defs[] = { NULL, NULL, NULL, - EXT_CLIENT_HELLO | EXT_TLS1_2_SERVER_HELLO | EXT_TLS1_3_CERTIFICATE + EXT_CLIENT_HELLO | EXT_TLS1_2_SERVER_HELLO + | /*EXT_TLS1_3_CERTIFICATE*/EXT_TLS1_3_SERVER_HELLO }, #ifndef OPENSSL_NO_NEXTPROTONEG { @@ -133,7 +140,7 @@ static const EXTENSION_DEFINITION ext_defs[] = { NULL, NULL, EXT_CLIENT_HELLO | EXT_TLS1_2_SERVER_HELLO - | EXT_TLS1_3_ENCRYPTED_EXTENSIONS + | /*EXT_TLS1_3_ENCRYPTED_EXTENSIONS*/EXT_TLS1_3_SERVER_HELLO }, { TLSEXT_TYPE_use_srtp, @@ -163,12 +170,15 @@ static const EXTENSION_DEFINITION ext_defs[] = { NULL, NULL, NULL, - EXT_CLIENT_HELLO | EXT_TLS1_2_SERVER_HELLO | EXT_TLS1_3_CERTIFICATE + EXT_CLIENT_HELLO | EXT_TLS1_2_SERVER_HELLO + | /*EXT_TLS1_3_CERTIFICATE*/EXT_TLS1_3_SERVER_HELLO }, { TLSEXT_TYPE_extended_master_secret, tls_parse_clienthello_ems, NULL, + NULL, + NULL, EXT_CLIENT_HELLO | EXT_TLS1_2_SERVER_HELLO | EXT_TLS1_2_AND_BELOW_ONLY }, { @@ -365,8 +375,8 @@ int tls_collect_extensions(SSL *s, PACKET *packet, unsigned int context, return 0; } -int tls_parse_all_extensions(SSL *s, RAW_EXTENSION *exts, size_t numexts, - int *al) +int tls_parse_all_extensions(SSL *s, int context, RAW_EXTENSION *exts, + size_t numexts, int *al) { size_t loop; @@ -437,15 +447,15 @@ int tls_parse_all_extensions(SSL *s, RAW_EXTENSION *exts, size_t numexts, * failure. If a failure has occurred then |*al| will also be set to the alert * to be sent. */ -int tls_parse_extension(SSL *s, int type, RAW_EXTENSION *exts, size_t numexts, - int *al) +int tls_parse_extension(SSL *s, int type, int context, RAW_EXTENSION *exts, + size_t numexts, int *al) { RAW_EXTENSION *ext = tls_get_extension_by_type(exts, numexts, type); if (ext == NULL) return 1; - return tls_parse_all_extensions(s, ext, 1, al); + return tls_parse_all_extensions(s, context, ext, 1, al); } int tls_construct_extensions(SSL *s, WPACKET *pkt, unsigned int context, @@ -468,23 +478,27 @@ int tls_construct_extensions(SSL *s, WPACKET *pkt, unsigned int context, } for (loop = 0; loop < OSSL_NELEM(ext_defs); loop++) { + int (*construct)(SSL *s, WPACKET *pkt, int *al); + /* Skip if not relevant for our context */ if ((ext_defs[loop].context & context) == 0) continue; - construct = s->server ? extdef->server_construct - : extdef->client_construct; + construct = s->server ? ext_defs[loop].server_construct + : ext_defs[loop].client_construct; /* Check if this extension is defined for our protocol. If not, skip */ if ((SSL_IS_DTLS(s) - && (extdef->context & EXT_TLS_IMPLEMENTATION_ONLY) != 0) + && (ext_defs[loop].context & EXT_TLS_IMPLEMENTATION_ONLY) + != 0) || (s->version == SSL3_VERSION - && (extdef->context & EXT_SSL3_ALLOWED) == 0) + && (ext_defs[loop].context & EXT_SSL3_ALLOWED) == 0) || (SSL_IS_TLS13(s) - && (extdef->context & EXT_TLS1_2_AND_BELOW_ONLY) != 0) + && (ext_defs[loop].context & EXT_TLS1_2_AND_BELOW_ONLY) + != 0) || (!SSL_IS_TLS13(s) - && ((extdef->context & EXT_TLS1_3_ONLY) != 0 - || (context & EXT_CLIENT_HELLO) != 0)) + && (ext_defs[loop].context & EXT_TLS1_3_ONLY) != 0 + && (context & EXT_CLIENT_HELLO) == 0) || construct == NULL) continue; @@ -492,12 +506,11 @@ int tls_construct_extensions(SSL *s, WPACKET *pkt, unsigned int context, return 0; } - /* Add custom extensions */ if ((context & EXT_CLIENT_HELLO) != 0) { custom_ext_init(&s->cert->cli_ext); addcustom = 1; - } else if (context & (EXT_TLS1_2_SERVER_HELLO) { + } else if ((context & EXT_TLS1_2_SERVER_HELLO) != 0) { /* * We already initialised the custom extensions during ClientHello * parsing. @@ -508,13 +521,14 @@ int tls_construct_extensions(SSL *s, WPACKET *pkt, unsigned int context, */ addcustom = 1; } + if (addcustom && !custom_ext_add(s, s->server, pkt, al)) { - SSLerr(SSL_F_SSL_ADD_CLIENTHELLO_TLSEXT, ERR_R_INTERNAL_ERROR); + SSLerr(SSL_F_TLS_CONSTRUCT_EXTENSIONS, ERR_R_INTERNAL_ERROR); return 0; } if (!WPACKET_close(pkt)) { - *sl = SSL_AD_INTERNAL_ERROR; + *al = SSL_AD_INTERNAL_ERROR; SSLerr(SSL_F_TLS_CONSTRUCT_EXTENSIONS, ERR_R_INTERNAL_ERROR); return 0; } @@ -522,7 +536,6 @@ int tls_construct_extensions(SSL *s, WPACKET *pkt, unsigned int context, return 1; } - /* * Parse the client's renegotiation binding and abort if it's not right */ @@ -534,7 +547,7 @@ static int tls_parse_clienthello_renegotiate(SSL *s, PACKET *pkt, int *al) /* Parse the length byte */ if (!PACKET_get_1(pkt, &ilen) || !PACKET_get_bytes(pkt, &data, ilen)) { - SSLerr(SSL_F_SSL_PARSE_CLIENTHELLO_RENEGOTIATE_EXT, + SSLerr(SSL_F_TLS_PARSE_CLIENTHELLO_RENEGOTIATE, SSL_R_RENEGOTIATION_ENCODING_ERR); *al = SSL_AD_ILLEGAL_PARAMETER; return 0; @@ -542,7 +555,7 @@ static int tls_parse_clienthello_renegotiate(SSL *s, PACKET *pkt, int *al) /* Check that the extension matches */ if (ilen != s->s3->previous_client_finished_len) { - SSLerr(SSL_F_SSL_PARSE_CLIENTHELLO_RENEGOTIATE_EXT, + SSLerr(SSL_F_TLS_PARSE_CLIENTHELLO_RENEGOTIATE, SSL_R_RENEGOTIATION_MISMATCH); *al = SSL_AD_HANDSHAKE_FAILURE; return 0; @@ -550,7 +563,7 @@ static int tls_parse_clienthello_renegotiate(SSL *s, PACKET *pkt, int *al) if (memcmp(data, s->s3->previous_client_finished, s->s3->previous_client_finished_len)) { - SSLerr(SSL_F_SSL_PARSE_CLIENTHELLO_RENEGOTIATE_EXT, + SSLerr(SSL_F_TLS_PARSE_CLIENTHELLO_RENEGOTIATE, SSL_R_RENEGOTIATION_MISMATCH); *al = SSL_AD_HANDSHAKE_FAILURE; return 0; diff --git a/ssl/statem/statem_locl.h b/ssl/statem/statem_locl.h index 42d5a8c5c6..481336d47e 100644 --- a/ssl/statem/statem_locl.h +++ b/ssl/statem/statem_locl.h @@ -149,7 +149,9 @@ __owur MSG_PROCESS_RETURN tls_process_next_proto(SSL *s, PACKET *pkt); #endif __owur int tls_construct_new_session_ticket(SSL *s, WPACKET *pkt); -__owur int tls_parse_all_extensions(SSL *s, RAW_EXTENSION *exts, size_t numexts, - int *al); -__owur int tls_parse_extension(SSL *s, int type, RAW_EXTENSION *exts, +__owur int tls_parse_all_extensions(SSL *s, int context, RAW_EXTENSION *exts, + size_t numexts, int *al); +__owur int tls_parse_extension(SSL *s, int type, int context, RAW_EXTENSION *exts, size_t numexts, int *al); +__owur int tls_construct_extensions(SSL *s, WPACKET *pkt, unsigned int context, + int *al); diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c index 694f83e157..773e732642 100644 --- a/ssl/statem/statem_srvr.c +++ b/ssl/statem/statem_srvr.c @@ -1173,7 +1173,7 @@ static int tls_scan_clienthello_tlsext(SSL *s, CLIENTHELLO_MSG *hello, int *al) * We process the supported_groups extension first so that is done before * we get to key_share which needs to use the information in it. */ - if (!tls_parse_extension(s, TLSEXT_TYPE_supported_groups, + if (!tls_parse_extension(s, TLSEXT_TYPE_supported_groups, EXT_CLIENT_HELLO, hello->pre_proc_exts, hello->num_extensions, al)) { return 0; } @@ -1185,12 +1185,12 @@ static int tls_scan_clienthello_tlsext(SSL *s, CLIENTHELLO_MSG *hello, int *al) hello->num_extensions, TLSEXT_TYPE_renegotiate) == NULL) { *al = SSL_AD_HANDSHAKE_FAILURE; - SSLerr(SSL_F_SSL_SCAN_CLIENTHELLO_TLSEXT, + SSLerr(SSL_F_TLS_SCAN_CLIENTHELLO_TLSEXT, SSL_R_UNSAFE_LEGACY_RENEGOTIATION_DISABLED); return 0; } - return tls_parse_all_extensions(s, hello->pre_proc_exts, + return tls_parse_all_extensions(s, EXT_CLIENT_HELLO, hello->pre_proc_exts, hello->num_extensions, al); } @@ -1245,7 +1245,7 @@ static int tls_parse_clienthello_tlsext(SSL *s, CLIENTHELLO_MSG *hello) } if (!tls_check_clienthello_tlsext(s)) { - SSLerr(SSL_F_SSL_PARSE_CLIENTHELLO_TLSEXT, SSL_R_CLIENTHELLO_TLSEXT); + SSLerr(SSL_F_TLS_PARSE_CLIENTHELLO_TLSEXT, SSL_R_CLIENTHELLO_TLSEXT); return 0; } @@ -1527,6 +1527,7 @@ MSG_PROCESS_RETURN tls_process_client_hello(SSL *s, PACKET *pkt) /* We need to do this before getting the session */ if (!tls_parse_extension(s, TLSEXT_TYPE_extended_master_secret, + EXT_CLIENT_HELLO, clienthello.pre_proc_exts, clienthello.num_extensions, &al)) { SSLerr(SSL_F_TLS_PROCESS_CLIENT_HELLO, SSL_R_CLIENTHELLO_TLSEXT); -- 2.25.1