diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c index c8ed722ab157144b187091c16ac972041b0be50e..54075ae0e69bda34e69dab69d369799a05306637 100644 --- a/ssl/statem/extensions.c +++ b/ssl/statem/extensions.c @@ -421,8 +421,9 @@ int extension_is_relevant(SSL *s, unsigned int extctx, unsigned int thisctx) * stored in |*res| on success. In the event of an error the alert type to use * is stored in |*al|. We don't actually process the content of the extensions * yet, except to check their types. This function also runs the initialiser - * functions for all known extensions (whether we have collected them or not). - * If successful the caller is responsible for freeing the contents of |*res|. + * functions for all known extensions if |init| is nonzero (whether we have + * collected them or not). If successful the caller is responsible for freeing + * the contents of |*res|. * * Per http://tools.ietf.org/html/rfc5246#section-7.4.1.4, there may not be * more than one extension of the same type in a ClientHello or ServerHello. @@ -432,7 +433,8 @@ int extension_is_relevant(SSL *s, unsigned int extctx, unsigned int thisctx) * extensions that we know about. We ignore others. */ int tls_collect_extensions(SSL *s, PACKET *packet, unsigned int context, - RAW_EXTENSION **res, int *al, size_t *len) + RAW_EXTENSION **res, int *al, size_t *len, + int init) { PACKET extensions = *packet; size_t i = 0; @@ -490,16 +492,19 @@ int tls_collect_extensions(SSL *s, PACKET *packet, unsigned int context, } } - /* - * Initialise all known extensions relevant to this context, whether we have - * found them or not - */ - for (thisexd = ext_defs, i = 0; i < OSSL_NELEM(ext_defs); i++, thisexd++) { - if(thisexd->init != NULL && (thisexd->context & context) != 0 - && extension_is_relevant(s, thisexd->context, context) - && !thisexd->init(s, context)) { - *al = SSL_AD_INTERNAL_ERROR; - goto err; + if (init) { + /* + * Initialise all known extensions relevant to this context, + * whether we have found them or not + */ + for (thisexd = ext_defs, i = 0; i < OSSL_NELEM(ext_defs); + i++, thisexd++) { + if (thisexd->init != NULL && (thisexd->context & context) != 0 && + extension_is_relevant(s, thisexd->context, context) && + !thisexd->init(s, context)) { + *al = SSL_AD_INTERNAL_ERROR; + goto err; + } } } @@ -578,14 +583,14 @@ int tls_parse_extension(SSL *s, TLSEXT_INDEX idx, int context, /* * Parse all remaining extensions that have not yet been parsed. Also calls the - * finalisation for all extensions at the end, whether we collected them or not. - * Returns 1 for success or 0 for failure. If we are working on a Certificate - * message then we also pass the Certificate |x| and its position in the - * |chainidx|, with 0 being the first certificate. On failure, |*al| is - * populated with a suitable alert code. + * finalisation for all extensions at the end if |fin| is nonzero, whether we + * collected them or not. Returns 1 for success or 0 for failure. If we are + * working on a Certificate message then we also pass the Certificate |x| and + * its position in the |chainidx|, with 0 being the first certificate. On + * failure, |*al| is populated with a suitable alert code. */ int tls_parse_all_extensions(SSL *s, int context, RAW_EXTENSION *exts, X509 *x, - size_t chainidx, int *al) + size_t chainidx, int *al, int fin) { size_t i, numexts = OSSL_NELEM(ext_defs); const EXTENSION_DEFINITION *thisexd; @@ -599,15 +604,17 @@ int tls_parse_all_extensions(SSL *s, int context, RAW_EXTENSION *exts, X509 *x, return 0; } - /* - * Finalise all known extensions relevant to this context, whether we have - * found them or not - */ - for (i = 0, thisexd = ext_defs; i < OSSL_NELEM(ext_defs); i++, thisexd++) { - if(thisexd->final != NULL - && (thisexd->context & context) != 0 - && !thisexd->final(s, context, exts[i].present, al)) - return 0; + if (fin) { + /* + * Finalise all known extensions relevant to this context, + * whether we have found them or not + */ + for (i = 0, thisexd = ext_defs; i < OSSL_NELEM(ext_defs); + i++, thisexd++) { + if (thisexd->final != NULL && (thisexd->context & context) != 0 && + !thisexd->final(s, context, exts[i].present, al)) + return 0; + } } return 1; diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c index ab77ba05e9ff4416e56f36f3a3c44bb1a84131ee..0b4931d6d0812d9644258851e0218f8d78206d40 100644 --- a/ssl/statem/statem_clnt.c +++ b/ssl/statem/statem_clnt.c @@ -1373,7 +1373,7 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL *s, PACKET *pkt) 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, &al, NULL)) + if (!tls_collect_extensions(s, &extpkt, context, &extensions, &al, NULL, 1)) goto f_err; s->hit = 0; @@ -1525,7 +1525,7 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL *s, PACKET *pkt) } #endif - if (!tls_parse_all_extensions(s, context, extensions, NULL, 0, &al)) + if (!tls_parse_all_extensions(s, context, extensions, NULL, 0, &al, 1)) goto f_err; #ifndef OPENSSL_NO_SCTP @@ -1616,9 +1616,9 @@ static MSG_PROCESS_RETURN tls_process_hello_retry_request(SSL *s, PACKET *pkt) } if (!tls_collect_extensions(s, &extpkt, SSL_EXT_TLS1_3_HELLO_RETRY_REQUEST, - &extensions, &al, NULL) + &extensions, &al, NULL, 1) || !tls_parse_all_extensions(s, SSL_EXT_TLS1_3_HELLO_RETRY_REQUEST, - extensions, NULL, 0, &al)) + extensions, NULL, 0, &al, 1)) goto f_err; OPENSSL_free(extensions); @@ -1711,9 +1711,10 @@ MSG_PROCESS_RETURN tls_process_server_certificate(SSL *s, PACKET *pkt) } if (!tls_collect_extensions(s, &extensions, SSL_EXT_TLS1_3_CERTIFICATE, &rawexts, - &al, NULL) + &al, NULL, chainidx == 0) || !tls_parse_all_extensions(s, SSL_EXT_TLS1_3_CERTIFICATE, - rawexts, x, chainidx, &al)) { + rawexts, x, chainidx, &al, + !PACKET_remaining(pkt))) { OPENSSL_free(rawexts); goto f_err; } @@ -2340,9 +2341,9 @@ MSG_PROCESS_RETURN tls_process_certificate_request(SSL *s, PACKET *pkt) } if (!tls_collect_extensions(s, &extensions, SSL_EXT_TLS1_3_CERTIFICATE_REQUEST, - &rawexts, &al, NULL) + &rawexts, &al, NULL, 1) || !tls_parse_all_extensions(s, SSL_EXT_TLS1_3_CERTIFICATE_REQUEST, - rawexts, NULL, 0, &al)) { + rawexts, NULL, 0, &al, 1)) { OPENSSL_free(rawexts); goto err; } @@ -2501,10 +2502,10 @@ MSG_PROCESS_RETURN tls_process_new_session_ticket(SSL *s, PACKET *pkt) if (!PACKET_as_length_prefixed_2(pkt, &extpkt) || !tls_collect_extensions(s, &extpkt, SSL_EXT_TLS1_3_NEW_SESSION_TICKET, - &exts, &al, NULL) + &exts, &al, NULL, 1) || !tls_parse_all_extensions(s, SSL_EXT_TLS1_3_NEW_SESSION_TICKET, - exts, NULL, 0, &al)) { + exts, NULL, 0, &al, 1)) { SSLerr(SSL_F_TLS_PROCESS_NEW_SESSION_TICKET, SSL_R_BAD_EXTENSION); goto f_err; } @@ -3464,9 +3465,9 @@ static MSG_PROCESS_RETURN tls_process_encrypted_extensions(SSL *s, PACKET *pkt) if (!tls_collect_extensions(s, &extensions, SSL_EXT_TLS1_3_ENCRYPTED_EXTENSIONS, &rawexts, - &al, NULL) + &al, NULL, 1) || !tls_parse_all_extensions(s, SSL_EXT_TLS1_3_ENCRYPTED_EXTENSIONS, - rawexts, NULL, 0, &al)) + rawexts, NULL, 0, &al, 1)) goto err; OPENSSL_free(rawexts); diff --git a/ssl/statem/statem_locl.h b/ssl/statem/statem_locl.h index 2352c6a11ea76585eade8364cb2ef8cd0ebe166e..3b9311e02f5b1e7065c7c51222c947a9b95f3f24 100644 --- a/ssl/statem/statem_locl.h +++ b/ssl/statem/statem_locl.h @@ -156,12 +156,13 @@ MSG_PROCESS_RETURN tls_process_end_of_early_data(SSL *s, PACKET *pkt); __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, - RAW_EXTENSION **res, int *al, size_t *len); + RAW_EXTENSION **res, int *al, size_t *len, + int init); __owur int tls_parse_extension(SSL *s, TLSEXT_INDEX idx, int context, RAW_EXTENSION *exts, X509 *x, size_t chainidx, int *al); __owur int tls_parse_all_extensions(SSL *s, int context, RAW_EXTENSION *exts, - X509 *x, size_t chainidx, int *al); + X509 *x, size_t chainidx, int *al, int fin); __owur int should_add_extension(SSL *s, unsigned int extctx, unsigned int thisctx, int max_version); __owur int tls_construct_extensions(SSL *s, WPACKET *pkt, unsigned int context, diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c index d751502aba726fa2647c637d7c6e5f52ae8bc1c8..f6ecbf700654643553600d27243882cda95919de 100644 --- a/ssl/statem/statem_srvr.c +++ b/ssl/statem/statem_srvr.c @@ -1426,7 +1426,7 @@ MSG_PROCESS_RETURN tls_process_client_hello(SSL *s, PACKET *pkt) extensions = clienthello->extensions; if (!tls_collect_extensions(s, &extensions, SSL_EXT_CLIENT_HELLO, &clienthello->pre_proc_exts, &al, - &clienthello->pre_proc_exts_len)) { + &clienthello->pre_proc_exts_len, 1)) { /* SSLerr already been called */ goto f_err; } @@ -1690,7 +1690,7 @@ static int tls_early_post_process_client_hello(SSL *s, int *pal) /* TLS extensions */ if (!tls_parse_all_extensions(s, SSL_EXT_CLIENT_HELLO, - clienthello->pre_proc_exts, NULL, 0, &al)) { + clienthello->pre_proc_exts, NULL, 0, &al, 1)) { SSLerr(SSL_F_TLS_EARLY_POST_PROCESS_CLIENT_HELLO, SSL_R_PARSE_TLSEXT); goto err; } @@ -3217,9 +3217,10 @@ MSG_PROCESS_RETURN tls_process_client_certificate(SSL *s, PACKET *pkt) } if (!tls_collect_extensions(s, &extensions, SSL_EXT_TLS1_3_CERTIFICATE, &rawexts, - &al, NULL) + &al, NULL, chainidx == 0) || !tls_parse_all_extensions(s, SSL_EXT_TLS1_3_CERTIFICATE, - rawexts, x, chainidx, &al)) { + rawexts, x, chainidx, &al, + !PACKET_remaining(&spkt))) { OPENSSL_free(rawexts); goto f_err; }