From db80cf4eb19af51415bb2f3714765b92864e3924 Mon Sep 17 00:00:00 2001 From: jsing Date: Sun, 17 Jul 2022 14:54:10 +0000 Subject: [PATCH] Correct handling of QUIC transport parameters extension. Remove duplicate U16 length prefix, since tlsext_build() already adds this for us. Condition on SSL_is_quic() rather than TLS version - RFC 9001 is clear that this extension is only permitted on QUIC transport and an fatal unsupported extension alert is required if used elsewhere. Additionally, at the point where extensions are parsed, we do not necessarily know what TLS version has been negotiated. ok beck@ tb@ --- lib/libssl/ssl_tlsext.c | 64 +++++++++++------------------------------ 1 file changed, 16 insertions(+), 48 deletions(-) diff --git a/lib/libssl/ssl_tlsext.c b/lib/libssl/ssl_tlsext.c index a7c8f2d61d0..6063991306f 100644 --- a/lib/libssl/ssl_tlsext.c +++ b/lib/libssl/ssl_tlsext.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssl_tlsext.c,v 1.120 2022/07/17 14:41:27 jsing Exp $ */ +/* $OpenBSD: ssl_tlsext.c,v 1.121 2022/07/17 14:54:10 jsing Exp $ */ /* * Copyright (c) 2016, 2017, 2019 Joel Sing * Copyright (c) 2017 Doug Hogan @@ -1950,32 +1950,23 @@ tlsext_psk_server_parse(SSL *s, uint16_t msg_type, CBS *cbs, int *alert) } /* - * QUIC transport parameters extension. + * QUIC transport parameters extension - RFC 9001 section 8.2. */ int tlsext_quic_transport_parameters_client_needs(SSL *s, uint16_t msg_type) { - return (s->internal->quic_transport_params_len > 0 && - s->s3->hs.our_max_tls_version >= TLS1_3_VERSION); + return SSL_is_quic(s) && s->internal->quic_transport_params_len > 0; } int tlsext_quic_transport_parameters_client_build(SSL *s, uint16_t msg_type, CBB *cbb) { - CBB contents; - - if (!CBB_add_u16_length_prefixed(cbb, &contents)) - return 0; - - if (!CBB_add_bytes(&contents, s->internal->quic_transport_params, + if (!CBB_add_bytes(cbb, s->internal->quic_transport_params, s->internal->quic_transport_params_len)) return 0; - if (!CBB_flush(cbb)) - return 0; - return 1; } @@ -1983,20 +1974,16 @@ int tlsext_quic_transport_parameters_client_parse(SSL *s, uint16_t msg_type, CBS *cbs, int *alert) { - CBS transport_data; - - /* QUIC requires TLS 1.3. */ - if (ssl_effective_tls_version(s) < TLS1_3_VERSION) { + if (!SSL_is_quic(s)) { *alert = SSL_AD_UNSUPPORTED_EXTENSION; return 0; } - if (!CBS_get_u16_length_prefixed(cbs, &transport_data)) - return 0; - - if (!CBS_stow(&transport_data, &s->s3->peer_quic_transport_params, + if (!CBS_stow(cbs, &s->s3->peer_quic_transport_params, &s->s3->peer_quic_transport_params_len)) return 0; + if (!CBS_skip(cbs, s->s3->peer_quic_transport_params_len)) + return 0; return 1; } @@ -2004,25 +1991,17 @@ tlsext_quic_transport_parameters_client_parse(SSL *s, uint16_t msg_type, int tlsext_quic_transport_parameters_server_needs(SSL *s, uint16_t msg_type) { - return s->internal->quic_transport_params_len > 0; + return SSL_is_quic(s) && s->internal->quic_transport_params_len > 0; } int tlsext_quic_transport_parameters_server_build(SSL *s, uint16_t msg_type, CBB *cbb) { - CBB contents; - - if (!CBB_add_u16_length_prefixed(cbb, &contents)) - return 0; - - if (!CBB_add_bytes(&contents, s->internal->quic_transport_params, + if (!CBB_add_bytes(cbb, s->internal->quic_transport_params, s->internal->quic_transport_params_len)) return 0; - if (!CBB_flush(cbb)) - return 0; - return 1; } @@ -2030,27 +2009,16 @@ int tlsext_quic_transport_parameters_server_parse(SSL *s, uint16_t msg_type, CBS *cbs, int *alert) { - CBS transport_data; - - /* - * Ignore this extension if we don't have configured quic transport data - * or if we are not TLS 1.3. - */ - if (s->internal->quic_transport_params_len == 0 || - ssl_effective_tls_version(s) < TLS1_3_VERSION) { - if (!CBS_skip(cbs, CBS_len(cbs))) { - *alert = SSL_AD_INTERNAL_ERROR; - return 0; - } - return 1; - } - - if (!CBS_get_u16_length_prefixed(cbs, &transport_data)) + if (!SSL_is_quic(s)) { + *alert = SSL_AD_UNSUPPORTED_EXTENSION; return 0; + } - if (!CBS_stow(&transport_data, &s->s3->peer_quic_transport_params, + if (!CBS_stow(cbs, &s->s3->peer_quic_transport_params, &s->s3->peer_quic_transport_params_len)) return 0; + if (!CBS_skip(cbs, s->s3->peer_quic_transport_params_len)) + return 0; return 1; } -- 2.20.1