Recommit a better version of the removal of the F5 workaround

Unlike for previous TLS versions, TLSv1.3 servers can send the supported
groups extension to inform a client of the server's preferences. The
intention is that a client can adapt for subsequent commits. We ignore
this info for now, but sthen ran into java-based servers that do this.

Thus, rejecting the extension outright was incorrect. Instead, only allow
the extension in TLSv1.3 encrypted extensions. This way the F5 workaround
is also disabled, but we continue to interoperate with TLSv1.3 servers that
do follow the last paragraph of RFC 8446, section 4.2.7.

This mostly adjusts outdated/misleading comments.

ok jsing sthen

diff --git a/lib/libssl/ssl_tlsext.c b/lib/libssl/ssl_tlsext.c
index 9073445..6d8f518 100644 (file)
--- a/lib/libssl/ssl_tlsext.c
+++ b/lib/libssl/ssl_tlsext.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_tlsext.c,v 1.147 2024/04/02 22:50:54 sthen Exp $ */
+/* $OpenBSD: ssl_tlsext.c,v 1.148 2024/04/04 08:02:21 tb Exp $ */
 /*
  * Copyright (c) 2016, 2017, 2019 Joel Sing <jsing@openbsd.org>
  * Copyright (c) 2017 Doug Hogan <doug@openbsd.org>
@@ -325,14 +325,17 @@ tlsext_supportedgroups_client_process(SSL *s, uint16_t msg_type, CBS *cbs,
     int *alert)
 {
        /*
-        * Servers should not send this extension per the RFC.
-        *
-        * However, certain F5 BIG-IP systems incorrectly send it. This bug is
-        * from at least 2014 but as of 2017, there are still large sites with
-        * this unpatched in production. As a result, we need to currently skip
-        * over the extension and ignore its content:
-        *
-        *  https://support.f5.com/csp/article/K37345003
+        * This extension is only allowed in TLSv1.3 encrypted extensions.
+        * It is not permitted in a ServerHello in any version of TLS.
+        */
+       if (msg_type != SSL_TLSEXT_MSG_EE)
+               return 0;
+
+       /*
+        * RFC 8446, section 4.2.7: TLSv1.3 servers can send this extension but
+        * clients must not act on it during the handshake. This allows servers
+        * to advertise their preferences for subsequent handshakes. We ignore
+        * this complication.
         */
        if (!CBS_skip(cbs, CBS_len(cbs))) {
                *alert = SSL_AD_INTERNAL_ERROR;