Thanks a lot, this patch also fixes the problem in my setup with the TLS 1.2-only nginx config.

On 5/16/24 20:17, Theo Buehler wrote:
On Thu, May 16, 2024 at 04:41:05PM +0200, Andreas Bartelt wrote:
Hi,

I've chosen to report to ports@ first since I'm not yet sure what's the best
mailing list for the kind of problem I've been observing recently.

I've deliberately been using a TLS 1.2-only (i.e., no TLS 1.3 support)
configuration with nginx for multiple years now and my (previously working)
TLS 1.2 config from nginx.conf looks like the following:
         ssl_protocols TLSv1.2;
         ssl_ciphers 
ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384;
         ssl_prefer_server_ciphers   off;
         ssl_ecdh_curve prime256v1;

On OpenBSD current and with the following application versions (nginx
1.26.0, LibreSSL 3.9.0, chromium-124.0.6367.201), I've noticed that a
significant fraction of HTTPS connections via chrome fail via illegal
parameter alert (but not all of them). All HTTPS connections still work
perfectly fine via firefox-125.0.3 as well as SSL Labs site from Qualys.

I could get rid of these problems (with TLS access to nginx via chrome) by
also enabling TLS 1.3 in nginx.conf:
         ssl_protocols TLSv1.2 TLSv1.3;
         ssl_ciphers 
TLS_AES_128_GCM_SHA256:TLS_AES_256_GCM_SHA384:TLS_CHACHA20_POLY1305_SHA256:ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384;
         ssl_prefer_server_ciphers   off;
         ssl_ecdh_curve prime256v1;

It might be possible that also enabling TLS 1.3 simply masks the current
problem with TLS 1.2 support.

Can anybody reproduce this problem?
Any ideas on how to locate the actual source of this bug?

It's a side effect of

https://github.com/openbsd/src/commit/be89428ce5fb176ef7adb0cb4f297af5f2b0fb91

Specifically, this check is hit:

                if (!client_sent_group) {
                        *alert = SSL_AD_ILLEGAL_PARAMETER;
                        return 0;
                }

https://github.com/openbsd/src/blob/master/lib/libssl/ssl_tlsext.c#L1587-L1590

One workaround is to move this check a few lines down so it is only
triggered for TLSv1.3.

The problem is that with TLSv1.2 s->hit can be set and at the same time
no supported groups available in s->session, so

        tls1_get_group_list(s, 1, &client_groups, &client_groups_len);

returns NULL client_groups and length 0, so client_sent_group is never set.

I can no longer reproduce the problem with the diff below, but it seems
just as plausible that tls_keyshare_server_process() needs special
s->hit handling.

Index: ssl_tlsext.c
===================================================================
RCS file: /cvs/src/lib/libssl/ssl_tlsext.c,v
diff -u -p -r1.149 ssl_tlsext.c
--- ssl_tlsext.c        16 Apr 2024 17:46:30 -0000      1.149
+++ ssl_tlsext.c        16 May 2024 18:06:56 -0000
@@ -248,7 +248,7 @@ tlsext_supportedgroups_server_process(SS
        if (groups_len > TLSEXT_MAX_SUPPORTED_GROUPS)
                goto err;
- if (s->hit)
+       if (s->hit && s->session->tlsext_supportedgroups_length > 0)
                goto done;
if (s->s3->hs.tls13.hrr) {

Reply via email to