Decouple TLS extension table order from tlsext_randomize_build_order()
authorjsing <jsing@openbsd.org>
Mon, 25 Mar 2024 03:23:59 +0000 (03:23 +0000)
committerjsing <jsing@openbsd.org>
Mon, 25 Mar 2024 03:23:59 +0000 (03:23 +0000)
The PSK extension must be the last extension in the client hello. This is
currently implemented by relying on the fact that it is the last extension
in the TLS extension table. Remove this dependency so that we can reorder
the table as needed.

ok tb@

lib/libssl/ssl_tlsext.c

index 5dd4b69..7b81643 100644 (file)
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_tlsext.c,v 1.137 2023/04/28 18:14:59 tb Exp $ */
+/* $OpenBSD: ssl_tlsext.c,v 1.138 2024/03/25 03:23:59 jsing Exp $ */
 /*
  * Copyright (c) 2016, 2017, 2019 Joel Sing <jsing@openbsd.org>
  * Copyright (c) 2017 Doug Hogan <doug@openbsd.org>
@@ -2185,8 +2185,6 @@ static const struct tls_extension tls_extensions[] = {
                },
        },
        {
-               /* MUST be last extension in CH per RFC 8446 section 4.2. */
-
                .type = TLSEXT_TYPE_pre_shared_key,
                .messages = SSL_TLSEXT_MSG_CH | SSL_TLSEXT_MSG_SH,
                .client = {
@@ -2250,6 +2248,7 @@ tlsext_funcs(const struct tls_extension *tlsext, int is_server)
 int
 tlsext_randomize_build_order(SSL *s)
 {
+       const struct tls_extension *psk_ext;
        size_t idx, new_idx, psk_idx;
        size_t alpn_idx = 0, sni_idx = 0;
 
@@ -2261,9 +2260,11 @@ tlsext_randomize_build_order(SSL *s)
                return 0;
        s->tlsext_build_order_len = N_TLS_EXTENSIONS;
 
-       /* RFC 8446, section 4.2: PSK must be the last extension in the CH. */
-       psk_idx = N_TLS_EXTENSIONS - 1;
-       s->tlsext_build_order[psk_idx] = &tls_extensions[psk_idx];
+       /* RFC 8446, section 4.2 - PSK MUST be the last extension in the CH. */
+       if ((psk_ext = tls_extension_find(TLSEXT_TYPE_pre_shared_key,
+           &psk_idx)) == NULL)
+               return 0;
+       s->tlsext_build_order[N_TLS_EXTENSIONS - 1] = psk_ext;
 
        /* Fisher-Yates shuffle with PSK fixed. */
        for (idx = 0; idx < psk_idx; idx++) {