BUG/MEDIUM: ssl: crt-list ciphers ignored with AWS-LC

AWS-LC's SSL_new() copies the cipher lists of the SSL_CTX into the SSL
object, and SSL_set_SSL_CTX() does not update this copy while the server
handshake uses it. The "ciphers" and "ciphersuites" options of a
crt-list line were thus silently ignored: a client could negotiate a cipher
excluded by the certificate as long as the bind line allowed it. The same
configuration built against OpenSSL rejects such a connection.

The SNI callback now sets both lists again on the SSL object after the
context switch when built with AWS-LC API versions greater than 30.
API version 30 (AWS-LC 3.x) uses the selected SSL_CTX cipher lists
directly and does not need this workaround.

These setters are inefficient here: they parse the cipher configuration
strings for each connection.

This adds the ssl_crt-list_ciphers reg-test for TLSv1.2 and the
ssl_crt-list_ciphersuites ones for TLSv1.3, over TCP and QUIC.

Tested with AWS-LC LTS versions 3.6.0 and 4.2.0.

Thank you to @ygkat for having reported this issue in GH #3500

This should be backported as far as 3.0. Note the code is in ssl_sock.c
before 3.1.

Co-authored-by: William Lallemand <wlallemand@haproxy.com>

diff --git a/reg-tests/quic/ssl_crt-list_ciphersuites.vtc b/reg-tests/quic/ssl_crt-list_ciphersuites.vtc
new file mode 100644 (file)
index 0000000..e69d539
--- /dev/null
+++ b/reg-tests/quic/ssl_crt-list_ciphersuites.vtc
@@ -0,0 +1,10 @@
+#REGTEST_TYPE=bug
+varnishtest "Test for per-certificate ciphersuites in a crt-list over QUIC"
+feature cmd "$HAPROXY_PROGRAM -cc 'version_atleast(2.8)'"
+feature cmd "$HAPROXY_PROGRAM -cc 'feature(QUIC) && !feature(QUIC_OPENSSL_COMPAT) && !feature(OPENSSL_WOLFSSL) && !ssllib_name_startswith(LibreSSL) && !ssllib_name_startswith(BoringSSL)'"
+# This test checks that the ciphersuites set on a crt-list line are enforced
+# over QUIC, even when the bind line allows more of them.
+# See GitHub issue #3500.
+
+setenv VTC_SOCK_TYPE quic
+include ${testdir}/../ssl/ssl_crt-list_ciphersuites.vtci

diff --git a/reg-tests/ssl/certs/ciphers.crt-list b/reg-tests/ssl/certs/ciphers.crt-list
new file mode 100644 (file)
index 0000000..57c2141
--- /dev/null
+++ b/reg-tests/ssl/certs/ciphers.crt-list
@@ -0,0 +1,2 @@
+common.pem [ciphers ECDHE-RSA-AES128-GCM-SHA256] restricted.bug3500.domain.tld
+common.pem open.bug3500.domain.tld

diff --git a/reg-tests/ssl/certs/ciphersuites.crt-list b/reg-tests/ssl/certs/ciphersuites.crt-list
new file mode 100644 (file)
index 0000000..62ec8dc
--- /dev/null
+++ b/reg-tests/ssl/certs/ciphersuites.crt-list
@@ -0,0 +1,2 @@
+common.pem [ciphersuites TLS_AES_128_GCM_SHA256] restricted.bug3500.domain.tld
+common.pem open.bug3500.domain.tld

diff --git a/reg-tests/ssl/ssl_crt-list_ciphers.vtc b/reg-tests/ssl/ssl_crt-list_ciphers.vtc
new file mode 100644 (file)
index 0000000..70f65f7
--- /dev/null
+++ b/reg-tests/ssl/ssl_crt-list_ciphers.vtc
@@ -0,0 +1,75 @@
+#REGTEST_TYPE=bug
+varnishtest "Test for per-certificate ciphers in a crt-list"
+feature cmd "$HAPROXY_PROGRAM -cc 'version_atleast(2.8)'"
+feature cmd "$HAPROXY_PROGRAM -cc 'feature(OPENSSL) && !feature(OPENSSL_WOLFSSL)'"
+# This test checks that the ciphers set on a crt-list line are enforced, even
+# when the bind line allows more of them. It uses TLSv1.2, so there is no QUIC
+# variant.
+# See GitHub issue #3500.
+feature ignore_unknown_macro
+
+server s1 -repeat 2 {
+    rxreq
+    txresp
+} -start
+
+haproxy h1 -conf {
+    global
+    .if feature(THREAD)
+        thread-groups 1
+    .endif
+        crt-base ${testdir}/certs
+
+    defaults
+        mode http
+        option httplog
+        retries 0
+        log stderr local0 debug err
+        option logasap
+        timeout connect "${HAPROXY_TEST_TIMEOUT-5s}"
+        timeout client  "${HAPROXY_TEST_TIMEOUT-5s}"
+        timeout server  "${HAPROXY_TEST_TIMEOUT-5s}"
+
+    listen clear-lst
+        bind "fd@${clearlst}"
+        use_backend allowed  if { path /allowed }
+        use_backend excluded if { path /excluded }
+        use_backend open     if { path /open }
+
+    # cipher allowed by the crt-list line
+    backend allowed
+        server s1 ${h1_ssllst_sock} ssl verify none sni str(restricted.bug3500.domain.tld) ssl-min-ver TLSv1.2 ssl-max-ver TLSv1.2 ciphers ECDHE-RSA-AES128-GCM-SHA256
+
+    # cipher allowed by the bind line but excluded by the crt-list line
+    backend excluded
+        server s1 ${h1_ssllst_sock} ssl verify none sni str(restricted.bug3500.domain.tld) ssl-min-ver TLSv1.2 ssl-max-ver TLSv1.2 ciphers ECDHE-RSA-AES256-GCM-SHA384
+
+    # same excluded cipher, but for a certificate without restriction
+    backend open
+        server s1 ${h1_ssllst_sock} ssl verify none sni str(open.bug3500.domain.tld) ssl-min-ver TLSv1.2 ssl-max-ver TLSv1.2 ciphers ECDHE-RSA-AES256-GCM-SHA384
+
+    listen ssl-lst
+        http-response add-header x-cipher %[ssl_fc_cipher]
+        bind "fd@${ssllst}" ssl strict-sni crt-list ${testdir}/certs/ciphers.crt-list ciphers ECDHE-RSA-AES128-GCM-SHA256:ECDHE-RSA-AES256-GCM-SHA384
+        server s1 ${s1_addr}:${s1_port}
+} -start
+
+client c1 -connect ${h1_clearlst_sock} {
+    txreq -url "/allowed"
+    rxresp
+    expect resp.status == 200
+    expect resp.http.x-cipher == ECDHE-RSA-AES128-GCM-SHA256
+} -run
+
+client c1 -connect ${h1_clearlst_sock} {
+    txreq -url "/excluded"
+    rxresp
+    expect resp.status == 503
+} -run
+
+client c1 -connect ${h1_clearlst_sock} {
+    txreq -url "/open"
+    rxresp
+    expect resp.status == 200
+    expect resp.http.x-cipher == ECDHE-RSA-AES256-GCM-SHA384
+} -run

diff --git a/reg-tests/ssl/ssl_crt-list_ciphersuites.vtc b/reg-tests/ssl/ssl_crt-list_ciphersuites.vtc
new file mode 100644 (file)
index 0000000..f622f63
--- /dev/null
+++ b/reg-tests/ssl/ssl_crt-list_ciphersuites.vtc
@@ -0,0 +1,10 @@
+#REGTEST_TYPE=bug
+varnishtest "Test for per-certificate ciphersuites in a crt-list"
+feature cmd "$HAPROXY_PROGRAM -cc 'version_atleast(2.8)'"
+feature cmd "$HAPROXY_PROGRAM -cc 'feature(OPENSSL) && !ssllib_name_startswith(LibreSSL) && !ssllib_name_startswith(BoringSSL) && !feature(OPENSSL_WOLFSSL)'"
+# This test checks that the ciphersuites set on a crt-list line are enforced,
+# even when the bind line allows more of them.
+# See GitHub issue #3500.
+
+setenv VTC_SOCK_TYPE stream
+include ${testdir}/ssl_crt-list_ciphersuites.vtci

diff --git a/reg-tests/ssl/ssl_crt-list_ciphersuites.vtci b/reg-tests/ssl/ssl_crt-list_ciphersuites.vtci
new file mode 100644 (file)
index 0000000..ce49d54
--- /dev/null
+++ b/reg-tests/ssl/ssl_crt-list_ciphersuites.vtci
@@ -0,0 +1,71 @@
+feature ignore_unknown_macro
+
+server s1 -repeat 2 {
+    rxreq
+    txresp
+} -start
+
+haproxy h1 -conf {
+    global
+    .if streq("$VTC_SOCK_TYPE",quic)
+        # required for backend connections
+        expose-experimental-directives
+    .endif
+    .if feature(THREAD)
+        thread-groups 1
+    .endif
+        crt-base ${testdir}/certs
+
+    defaults
+        mode http
+        option httplog
+        retries 0
+        log stderr local0 debug err
+        option logasap
+        timeout connect "${HAPROXY_TEST_TIMEOUT-5s}"
+        timeout client  "${HAPROXY_TEST_TIMEOUT-5s}"
+        timeout server  "${HAPROXY_TEST_TIMEOUT-5s}"
+
+    listen clear-lst
+        bind "fd@${clearlst}"
+        use_backend allowed  if { path /allowed }
+        use_backend excluded if { path /excluded }
+        use_backend open     if { path /open }
+
+    # ciphersuite allowed by the crt-list line
+    backend allowed
+        server s1 ${VTC_SOCK_TYPE}+${h1_ssllst_sock} ssl verify none sni str(restricted.bug3500.domain.tld) ssl-min-ver TLSv1.3 ciphersuites TLS_AES_128_GCM_SHA256
+
+    # ciphersuite allowed by the bind line but excluded by the crt-list line
+    backend excluded
+        server s1 ${VTC_SOCK_TYPE}+${h1_ssllst_sock} ssl verify none sni str(restricted.bug3500.domain.tld) ssl-min-ver TLSv1.3 ciphersuites TLS_AES_256_GCM_SHA384
+
+    # same excluded ciphersuite, but for a certificate without restriction
+    backend open
+        server s1 ${VTC_SOCK_TYPE}+${h1_ssllst_sock} ssl verify none sni str(open.bug3500.domain.tld) ssl-min-ver TLSv1.3 ciphersuites TLS_AES_256_GCM_SHA384
+
+    listen ssl-lst
+        http-response add-header x-cipher %[ssl_fc_cipher]
+        bind ${VTC_SOCK_TYPE}+fd@${ssllst} ssl strict-sni crt-list ${testdir}/certs/ciphersuites.crt-list ciphersuites TLS_AES_128_GCM_SHA256:TLS_AES_256_GCM_SHA384
+        server s1 ${s1_addr}:${s1_port}
+} -start
+
+client c1 -connect ${h1_clearlst_sock} {
+    txreq -url "/allowed"
+    rxresp
+    expect resp.status == 200
+    expect resp.http.x-cipher == TLS_AES_128_GCM_SHA256
+} -run
+
+client c1 -connect ${h1_clearlst_sock} {
+    txreq -url "/excluded"
+    rxresp
+    expect resp.status == 503
+} -run
+
+client c1 -connect ${h1_clearlst_sock} {
+    txreq -url "/open"
+    rxresp
+    expect resp.status == 200
+    expect resp.http.x-cipher == TLS_AES_256_GCM_SHA384
+} -run

diff --git a/src/ssl_clienthello.c b/src/ssl_clienthello.c
index 5cb94e6855d18290b111b7efd2286a1844bfb5b6..c08e1ca328bd847201edde7fc45dc0e68a3e150a 100644 (file)
--- a/src/ssl_clienthello.c
+++ b/src/ssl_clienthello.c
@@ -461,6 +461,22 @@ sni_lookup:
                        methodVersions[conf->ssl_methods.max].ssl_set_version(ssl, SET_MAX);
                        if (conf->early_data)
                                allow_early = 1;
+#if defined(OPENSSL_IS_AWSLC) && AWSLC_API_VERSION > 30
+                       /* AWS-LC copies the cipher lists at SSL_new() time and does
+                        * not update them on SSL_set_SSL_CTX(). Reapply the crt-list
+                        * options. API version 30 (AWS-LC 3.x) uses the SSL_CTX
+                        * cipher lists directly and does not need this workaround.
+                        * These calls are inefficient as they parse the cipher
+                        * configuration strings for each connection.
+                        */
+                       if ((conf->ciphers && !SSL_set_cipher_list(ssl, conf->ciphers)) ||
+                           (conf->ciphersuites && !SSL_set_ciphersuites(ssl, conf->ciphersuites))) {
+                               HA_RWLOCK_RDUNLOCK(SNI_LOCK, &s->sni_lock);
+                               TRACE_ERROR("Cannot set the crt-list ciphers on the SSL object",
+                                           SSL_EV_CONN_SWITCHCTX_CB|SSL_EV_CONN_ERR, conn);
+                               goto abort;
+                       }
+#endif
                }
                HA_RWLOCK_RDUNLOCK(SNI_LOCK, &s->sni_lock);
                goto allow_early;