haproxy icon indicating copy to clipboard operation
haproxy copied to clipboard

allow-0rtt ineffective when used in crt-list

Open theothergraham opened this issue 3 years ago • 2 comments

Detailed Description of the Problem

When allow-0rtt is configured in the bind config of a cert-list entry, is does not allow early data. When moved to the bind config that references the crt-list file, then it works.

For example:

vagrant@ubuntu-focal:~$ grep bind hap_test/conf/haproxy.conf
  bind 127.0.0.1:8443 ssl crt-list conf/localhost.crts alpn h2,http/1.1
vagrant@ubuntu-focal:~$ cat hap_test/conf/localhost.crts
conf/test.hap.pem [allow-0rtt]
vagrant@ubuntu-focal:~$ openssl s_client -sess_out test.hap.sess -connect 127.0.0.1:8443 -servername test.hap 2>&1 | grep 'Max Early Data'
    Max Early Data: 0
    Max Early Data: 0

vs

vagrant@ubuntu-focal:~$ grep bind hap_test/conf/haproxy.conf
  bind 127.0.0.1:8443 ssl crt-list conf/localhost.crts alpn h2,http/1.1 allow-0rtt
vagrant@ubuntu-focal:~$ cat hap_test/conf/localhost.crts
conf/test.hap.pem
vagrant@ubuntu-focal:~$ openssl s_client -sess_out test.hap.sess -connect 127.0.0.1:8443 -servername test.hap 2>&1 | grep 'Max Early Data'
    Max Early Data: 15360
    Max Early Data: 15360

Expected Behavior

Early data should be configurable on a per-certificate basis when allow-0rtt is applied in the bind config in the crt-list file.

Steps to Reproduce the Behavior

NOTE: this is shown in the detailed description above

  1. Configure an ssl bind with a crt-list file.
  2. In the crt-list file, configure allow-0rtt on one or more certificates.
  3. Using openssl s_client, see if a newly established session allows early data.

Do you have any idea what may have caused this?

The preparatory work to allow early data to be accepted looks like it is only performed when the bind conf has early_data set. This includes:

  • allocation of a buffer in ctx->early_buf
  • setting of amount allowed in that buffer by calling SSL_set_max_early_data()
  • setting of option SSL_OP_NO_ANTI_REPLAY

It appears that it may not be possible to perform these steps after the certificate to serve has been chosen in ssl_sock_switchctx_cbk(), as this gets called from within the call to SSL_read_early_data(), where we must pass in the buffer pointer and max length.

Do you have an idea how to solve the issue?

I was able to patch the code to always perform the steps to prepare for early data. This enables allow-0rtt to work in both configuration locations.

vagrant@ubuntu-focal:~/haproxy-2.4$ git diff
diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index c725f1ff2..587ec4275 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -3913,8 +3913,7 @@ ssl_sock_initial_ctx(struct bind_conf *bind_conf)
        SSL_CTX_set_select_certificate_cb(ctx, ssl_sock_switchctx_cbk);
        SSL_CTX_set_tlsext_servername_callback(ctx, ssl_sock_switchctx_err_cbk);
 #elif defined(SSL_OP_NO_ANTI_REPLAY)
-       if (bind_conf->ssl_conf.early_data)
-               SSL_CTX_set_options(ctx, SSL_OP_NO_ANTI_REPLAY);
+       SSL_CTX_set_options(ctx, SSL_OP_NO_ANTI_REPLAY);
        SSL_CTX_set_client_hello_cb(ctx, ssl_sock_switchctx_cbk, NULL);
        SSL_CTX_set_tlsext_servername_callback(ctx, ssl_sock_switchctx_err_cbk);
 #else
@@ -5332,15 +5331,13 @@ static int ssl_sock_init(struct connection *conn, void **xprt_ctx)
                        goto err;

 #ifdef SSL_READ_EARLY_DATA_SUCCESS
-               if (bc->ssl_conf.early_data) {
-                       b_alloc(&ctx->early_buf);
-                       SSL_set_max_early_data(ctx->ssl,
-                           /* Only allow early data if we managed to allocate
-                            * a buffer.
-                            */
-                           (!b_is_null(&ctx->early_buf)) ?
-                           global.tune.bufsize - global.tune.maxrewrite : 0);
-               }
+               /* be prepared to accept early data, but we may revise this
+                * later in ssl_sock_switchctx_cbk() if allow-0rtt is unset */
+               b_alloc(&ctx->early_buf);
+               SSL_set_max_early_data(ctx->ssl,
+                   /* Only allow early data if we allocate a buffer. */
+                   (!b_is_null(&ctx->early_buf)) ?
+                   global.tune.bufsize - global.tune.maxrewrite : 0);
 #endif

                SSL_set_accept_state(ctx->ssl);
@@ -5348,8 +5345,8 @@ static int ssl_sock_init(struct connection *conn, void **xprt_ctx)
                /* leave init state and start handshake */
                conn->flags |= CO_FL_SSL_WAIT_HS | CO_FL_WAIT_L6_CONN;
 #ifdef SSL_READ_EARLY_DATA_SUCCESS
-               if (bc->ssl_conf.early_data)
-                       conn->flags |= CO_FL_EARLY_SSL_HS;
+               /* be prepared to accept early data */
+               conn->flags |= CO_FL_EARLY_SSL_HS;
 #endif

                _HA_ATOMIC_INC(&sslconns);

This does, of course, mean we are doing a useless buffer allocation when early data is ultimately not allowed based on the chosen certificate. Not to mention that it will not be used unless early data is both allowed and provided. If it's worth optimizing, we could allocate a thread-local buffer for it. When early data is allowed and successfully read, this buffer can be assigned to the ctx and a new one allocated.

What is your configuration?

vagrant@ubuntu-focal:~/hap_test$ cat conf/haproxy.conf
global
  nbthread 2
  master-worker

defaults
  mode http
  timeout connect 5s
  timeout client 5s
  timeout server 5s

frontend test-0rtt
  bind 127.0.0.1:8443 ssl crt-list conf/localhost.crts alpn h2,http/1.1
  default_backend test_be

backend test_be
  balance roundrobin
  server default [email protected]:8000
vagrant@ubuntu-focal:~/hap_test$ cat conf/localhost.crts
conf/test.hap.pem [allow-0rtt]

Output of haproxy -vv

vagrant@ubuntu-focal:~/hap_test$ bin/haproxy -vv
HAProxy version 2.4.8 2021/11/03 - https://haproxy.org/
Status: long-term supported branch - will stop receiving fixes around Q2 2026.
Known bugs: http://www.haproxy.org/bugs/bugs-2.4.8.html
Running on: Linux 5.4.0-90-generic #101-Ubuntu SMP Fri Oct 15 20:00:55 UTC 2021 x86_64
Build options :
  TARGET  = linux-glibc
  CPU     = generic
  CC      = cc
  CFLAGS  = -O2 -g -Wall -Wextra -Wdeclaration-after-statement -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 -Wtype-limits -Wshift-negative-value -Wshift-overflow=2 -Wduplicated-cond -Wnull-dereference
  OPTIONS = USE_PCRE2_JIT=1 USE_STATIC_PCRE2=1 USE_TPROXY= USE_LINUX_SPLICE=1 USE_LIBCRYPT= USE_OPENSSL=1 USE_SYSTEMD=1 USE_PROMEX=1
  DEBUG   =

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

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

Built with multi-threading support (MAX_THREADS=64, default=2).
Built with OpenSSL version : OpenSSL 1.1.1f  31 Mar 2020
Running on OpenSSL version : OpenSSL 1.1.1f  31 Mar 2020
OpenSSL library supports TLS extensions : yes
OpenSSL library supports SNI : yes
OpenSSL library supports : TLSv1.0 TLSv1.1 TLSv1.2 TLSv1.3
Built with the Prometheus exporter as a service
Built with network namespace support.
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.34 2019-11-21
PCRE2 library supports JIT : yes
Encrypted password support via crypt(3): no
Built with gcc compiler version 9.3.0

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|CLEAN_ABRT|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 :
	[SPOE] spoe
	[CACHE] cache
	[FCGI] fcgi-app
	[COMP] compression
	[TRACE] trace

Last Outputs and Backtraces

No response

Additional Information

The examples here use a build of tag v2.4.8 without local patches or other modifications, on a stock Ubuntu 20.04 install. However, I can reproduce it in the same manner on 2.4.7 or 2.3.14, on Linux or FreeBSD.

theothergraham avatar Nov 15 '21 20:11 theothergraham

Thanks for your report, I'll take a look at it. Regarding the buffer allocation, it could probably be done in a postparser operation once the crt-list are allocated. But there could be other issues in this code, I'm surprised it was ever activated in the crt-list. We should probably make a reg-test for this.

wlallemand avatar Nov 22 '21 09:11 wlallemand

What is the status of this issue ?

capflam avatar May 24 '22 13:05 capflam

ping ?

capflam avatar Jan 23 '23 07:01 capflam