Subtle bug in certificate selection (?)

[martinschmatz]

Detailed Description of the Problem

This is a follow up to #394.

We have a domain, and a subdomain, and a certificate for each:

• domain: RSA, CN=my.domain.com, DNS=*.my.domain.com
• subdomain: EC, CN=my.domain.com, DNS=subdomain.my.domain.com

When issuing a cURL GET request to the subdomain with --tlsv1.3, the domain certificate is (incorrectly) presented. Without --tlsv1.3 (implying the the client hello offers TLSv1.2 and TLSv1.3) the subdomain certificate is (correctly) presented.

Expected Behavior

When issuing a cURL GET request to the subdomain, I expect that HAproxy presents the subdomain certificate - irrespective of whether or not the request is limited to TLS1.3.

Steps to Reproduce the Behavior

Create an RSA certificate for a domain, and an EC certificate for a related subdomain. Use a DNS wildcard certificate extension for the domain, and subdomain DNS extension for the subdomain.

Issue a cURL GET request to the subdomain, once with --tlsv1.3 and once without.

Do you have any idea what may have caused this?

The client hello for such GET requests with and without --tlsv1.3 differ (obviously) in the extension supported versions, but also in the list of Cipher Suites: The former has only TLS1.3 ciphersuites, while the latter also has TLS1.2 ciphers e.g. TLS_ECDHE_ECDSA_AES_256_GCM_SHA384 (0xc02c).

OpenSSL specifies for SSL_CIPHER_get_auth_nid() that if "any appropriate authentication algorithm can be used (as in the case of TLS 1.3 cipher suites) NID_auth_any is returned".

The code includes a check for the "very rare case: has ecdsa sign but not a ECDSA cipher".

This leads to the effect, that if only TLSv1.3 is offered in the client hello, the variable has_ecdsa_sig is not (re-)set to 1, because TLSv1.3 ciphers will not be qualified via NID_auth_ecdsa.

After I changed the line

if (cipher && SSL_CIPHER_get_auth_nid(cipher) == NID_auth_ecdsa) {

to

if (cipher && (SSL_CIPHER_get_auth_nid(cipher) == NID_auth_ecdsa || SSL_CIPHER_get_auth_nid(cipher) == NID_auth_any) ) {

the certificate for the subdomain is (correctly) presented whether --tlsv1.3 is enforced in the cURL GET request or not.

==> This solved my issue.

Remarks:

• I don't know why this check is performed because trying to get insights on the code history via git blame takes to long to load.
• As a consequence, I also don't know if the above code change is having any undesired side effects, and I'm not 100% certain that my code changes addressed the root cause or only cured a symptom.

Do you have an idea how to solve the issue?

See above: Change this line to also accept NID_auth_any.

What is your configuration?

Used in a `k8s` cluster with the standard [HAproxy config file](https://github.com/haproxytech/kubernetes-ingress/blob/master/fs/usr/local/etc/haproxy/haproxy.cfg).

Output of haproxy -vv

HAProxy version 2.8.3-86e043a 2023/09/07 - https://haproxy.org/
Status: long-term supported branch - will stop receiving fixes around Q2 2028.
Known bugs: http://www.haproxy.org/bugs/bugs-2.8.3.html
Running on: Linux 6.1.19 #1 SMP PREEMPT_DYNAMIC Sat Mar 25 09:19:55 UTC 2023 x86_64
Build options :
  TARGET  = linux-glibc
  CPU     = generic
  CC      = cc
  CFLAGS  = -O3 -fno-omit-frame-pointer -DTCP_USER_TIMEOUT=18 -Wall -Wextra -Wundef -Wdeclaration-after-statement -Wfatal-errors -Wtype-limits -Wshift-negative-value -Wshift-overflow=2 -Wduplicated-cond -Wnull-dereference -fwrapv -Wno-address-of-packed-member -Wno-unused-label -Wno-sign-compare -Wno-unused-parameter -Wno-clobbered -Wno-missing-field-initializers -Wno-cast-function-type -Wno-string-plus-int -Wno-atomic-alignment
  OPTIONS = USE_THREAD=1 USE_LINUX_TPROXY=1 USE_GETADDRINFO=1 USE_OPENSSL=1 USE_LUA=1 USE_SLZ=1 USE_TFO=1 USE_PROMEX=1 USE_PCRE2=1 USE_PCRE2_JIT=1
  DEBUG   = -DDEBUG_STRICT -DDEBUG_MEMORY_POOLS

Feature list : -51DEGREES +ACCEPT4 +BACKTRACE -CLOSEFROM +CPU_AFFINITY +CRYPT_H -DEVICEATLAS +DL -ENGINE +EPOLL -EVPORTS +GETADDRINFO -KQUEUE -LIBATOMIC +LIBCRYPT +LINUX_CAP +LINUX_SPLICE +LINUX_TPROXY +LUA +MATH -MEMORY_PROFILING +NETFILTER +NS -OBSOLETE_LINKER +OPENSSL -OPENSSL_WOLFSSL -OT -PCRE +PCRE2 +PCRE2_JIT -PCRE_JIT +POLL +PRCTL -PROCCTL +PROMEX -PTHREAD_EMULATION -QUIC +RT +SHM_OPEN +SLZ +SSL -STATIC_PCRE -STATIC_PCRE2 -SYSTEMD +TFO +THREAD +THREAD_DUMP +TPROXY -WURFL -ZLIB

Default settings :
  bufsize = 16384, maxrewrite = 1024, maxpollevents = 200

Built with multi-threading support (MAX_TGROUPS=16, MAX_THREADS=256, default=128).
Built with OpenSSL version : OpenSSL 3.2.0-dev
Running on OpenSSL version : OpenSSL 3.2.0-dev
OpenSSL library supports TLS extensions : yes
OpenSSL library supports SNI : yes
OpenSSL library supports : TLSv1.0 TLSv1.1 TLSv1.2 TLSv1.3
OpenSSL providers loaded : default
Built with Lua version : Lua 5.4.6
Built with the Prometheus exporter as a service
Built with network namespace support.
Running with a replaced memory allocator (e.g. via LD_PRELOAD).
Built with libslz for stateless compression.
Compression algorithms supported : identity("identity"), deflate("deflate"), raw-deflate("deflate"), gzip("gzip")
Built with transparent proxy support using: IP_TRANSPARENT IPV6_TRANSPARENT IP_FREEBIND
Built with PCRE2 version : 10.42 2022-12-11
PCRE2 library supports JIT : yes
Encrypted password support via crypt(3): yes
Built with gcc compiler version 11.3.1 20221121 (Red Hat 11.3.1-4)

Available polling systems :
      epoll : pref=300,  test result OK
       poll : pref=200,  test result OK
     select : pref=150,  test result OK
Total: 3 (3 usable), will use epoll.

Available multiplexer protocols :
(protocols marked as <default> cannot be specified using 'proto' keyword)
         h2 : mode=HTTP  side=FE|BE  mux=H2    flags=HTX|HOL_RISK|NO_UPG
       fcgi : mode=HTTP  side=BE     mux=FCGI  flags=HTX|HOL_RISK|NO_UPG
  <default> : mode=HTTP  side=FE|BE  mux=H1    flags=HTX
         h1 : mode=HTTP  side=FE|BE  mux=H1    flags=HTX|NO_UPG
  <default> : mode=TCP   side=FE|BE  mux=PASS  flags=
       none : mode=TCP   side=FE|BE  mux=PASS  flags=NO_UPG

Available services : prometheus-exporter
Available filters :
        [BWLIM] bwlim-in
        [BWLIM] bwlim-out
        [CACHE] cache
        [COMP] compression
        [FCGI] fcgi-app
        [SPOE] spoe
        [TRACE] trace

Last Outputs and Backtraces

No response

Additional Information

No response

[wtarreau]

Many thanks Martin for this very detailed analysis, it's quite interesting! We'll have a look soon (the SSL team is still at kernel recipes today ;-)).

[wlallemand]

Hello, sorry for the late reply. I have to made more tests to be sure that It won't break other things. I'll keep you updated.

[martinschmatz]

@wlallemand No worries, take your time: Better safe than sorry!

Btw, the PR to enhance the verbose mode of cURL was instrumental to detect this bug <-- @ajbozarth for awareness.

[wlallemand]

The problem should be fixed by the latest patch above, however I won't merge it yet., I made a lot of tests and discovered another subtle stupidity of the TLS stack, some signaling ciphers (RFC 7507) could pop in the ciphers list and always returns NID_auth_any so I had to make an exception for them. I have some reg-tests that I need to finish to be sure we won't break this anymore.

Also I think we still have another problem in this part of the code because we never check the configured ciphers of haproxy. It could potentially result in handshake failures when restricting the ciphers manually in cases that should work. But I need to dig that.

[wlallemand]

The patch seems to fix correctly the issue, I just pushed it into the master branch along with the update of the reg-tests.
Thanks for the report and analyze it was really useful, we are going to backport this in every stable versions.