From 6d3643d599537b4bfb4a790e77fa7ae556d70b27 Mon Sep 17 00:00:00 2001 From: jsing <> Date: Sun, 17 Jul 2022 14:54:10 +0000 Subject: 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@ --- src/lib/libssl/ssl_tlsext.c | 64 ++++++++++++--------------------------------- 1 file changed, 16 insertions(+), 48 deletions(-) (limited to 'src') diff --git a/src/lib/libssl/ssl_tlsext.c b/src/lib/libssl/ssl_tlsext.c index a7c8f2d61d..6063991306 100644 --- a/src/lib/libssl/ssl_tlsext.c +++ b/src/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; } -- cgit v1.2.3-55-g6feb