BUG/MINOR: ssl: Store client SNI in SSL context in case of ClientHello error
If an error is raised during the ClientHello callback on the server side
(ssl_sock_switchctx_cbk), the servername callback won't be called and
the client's SNI will not be saved in the SSL context. But since we use
the SSL_get_servername function to return this SNI in the ssl_fc_sni
sample fetch, that means that in case of error, such as an SNI mismatch
with a frontend having the strict-sni option enabled, the sample fetch
would not work (making strict-sni related errors hard to debug).
This patch fixes that by storing the SNI as an ex_data in the SSL
context in case the ClientHello callback returns an error. This way the
sample fetch can fallback to getting the SNI this way. It will still
first call the SSL_get_servername function first since it is the proper
way of getting a client's SNI when the handshake succeeded.
In order to avoid memory allocations are runtime into this highly used
runtime function, a new memory pool was created to store those client
SNIs. Its entry size is set to 256 bytes since SNIs can't be longer than
255 characters.
This fixes GitHub #1484.
It can be backported in 2.5.
diff --git a/include/haproxy/ssl_sock.h b/include/haproxy/ssl_sock.h
index cd8d3b6..fd45c4b 100644
--- a/include/haproxy/ssl_sock.h
+++ b/include/haproxy/ssl_sock.h
@@ -47,6 +47,7 @@
extern struct xprt_ops ssl_sock;
extern int ssl_capture_ptr_index;
extern int ssl_keylog_index;
+extern int ssl_client_sni_index;
extern struct pool_head *pool_head_ssl_keylog;
extern struct pool_head *pool_head_ssl_keylog_str;
diff --git a/reg-tests/ssl/ssl_errors.vtc b/reg-tests/ssl/ssl_errors.vtc
index 1ac6b08..6148a9d 100644
--- a/reg-tests/ssl/ssl_errors.vtc
+++ b/reg-tests/ssl/ssl_errors.vtc
@@ -63,24 +63,24 @@
syslog Slg_https_fmt -level info {
recv
- expect ~ ".*https_logfmt_ssl_lst~ https_logfmt_ssl_lst/s1.*0/0000000000000000/0/0/.? -/TLSv1.2/AES256-GCM-SHA384"
+ expect ~ ".*https_logfmt_ssl_lst~ https_logfmt_ssl_lst/s1.*0/0000000000000000/0/0/.? foo.com/TLSv1.2/AES256-GCM-SHA384"
barrier b1 sync
} -start
syslog Slg_https_fmt_err -level info {
recv
- expect ~ "ERROR.*https_logfmt_ssl_lst~ https_logfmt_ssl_lst/<NOSRV>.*30/0000000000000086/0/2/.? -/TLSv1.2/\\(NONE\\)"
+ expect ~ "ERROR.*https_logfmt_ssl_lst~ https_logfmt_ssl_lst/<NOSRV>.*30/0000000000000086/0/2/.? foo.com/TLSv1.2/\\(NONE\\)"
barrier b1 sync
recv
- expect ~ "ERROR.*https_logfmt_ssl_lst~ https_logfmt_ssl_lst/<NOSRV>.*31/0000000000000086/20/0/.? -/TLSv1.2/\\(NONE\\)"
+ expect ~ "ERROR.*https_logfmt_ssl_lst~ https_logfmt_ssl_lst/<NOSRV>.*31/0000000000000086/20/0/.? foo.com/TLSv1.2/\\(NONE\\)"
barrier b1 sync
recv
- expect ~ "ERROR.*https_logfmt_ssl_lst~ https_logfmt_ssl_lst/<NOSRV>.*34/00000000000000C1/0/0/.? -/TLSv1.2/\\(NONE\\)"
+ expect ~ "ERROR.*https_logfmt_ssl_lst~ https_logfmt_ssl_lst/<NOSRV>.*34/00000000000000C1/0/0/.? foo.com/TLSv1.2/\\(NONE\\)"
} -start
syslog Slg_logconnerror -level info {
@@ -115,7 +115,7 @@
barrier b2 sync
recv
- expect ~ ".*bc_err:32:\"Server presented an SSL certificate different from the configured one\" ssl_bc_err:134:.*:certificate verify failed"
+ expect ~ ".*bc_err:33:\"Server presented an SSL certificate different from the expected one\" ssl_bc_err:134:.*:certificate verify failed"
barrier b2 sync
@@ -134,6 +134,32 @@
expect ~ ".*bc_err:34:\"SSL handshake failure\" ssl_bc_err:1040:.*:sslv3 alert handshake failure"
} -start
+syslog Slg_bcknd_fe -level info {
+ # Client c13 - No error
+ recv
+ expect ~ ".* Server/TLSv1.3/TLS_AES_256_GCM_SHA384"
+
+ # Client c14 - Server certificate rejected
+ recv
+ expect ~ ".* foo.com/TLSv1.3/TLS_AES_256_GCM_SHA384"
+
+ # Client c15 - Server certificate mismatch (verifyhost option on backend)
+ recv
+ expect ~ ".* foo.com/TLSv1.3/TLS_AES_256_GCM_SHA384"
+
+ # Client c16 - Client certificate rejected
+ recv
+ expect ~ ".* foo.com/TLSv1.2/\\(NONE\\)"
+
+ # Client c17 - Wrong ciphers TLSv1.2
+ recv
+ expect ~ ".* foo.com/TLSv1.2/\\(NONE\\)"
+
+ # Client c18 - Wrong ciphers TLSv1.3 - the client does not get to send its certificate because the error happens before
+ recv
+ expect ~ ".* -/TLSv1.3/\\(NONE\\)"
+} -start
+
haproxy h1 -conf {
global
@@ -154,7 +180,7 @@
listen clear_lst
bind "fd@${clearlst}"
- default-server ssl crt ${testdir}/set_cafile_client.pem ca-file ${testdir}/set_cafile_interCA2.crt verify none no-ssl-reuse force-tlsv12
+ default-server ssl crt ${testdir}/set_cafile_client.pem ca-file ${testdir}/set_cafile_interCA2.crt verify none no-ssl-reuse force-tlsv12 sni str(foo.com)
balance roundrobin
server cust_fmt "${tmpdir}/cust_logfmt_ssl.sock"
@@ -164,7 +190,7 @@
listen clear_wrong_ciphers_lst
bind "fd@${wrongcipherslst}"
- default-server ssl crt ${testdir}/set_cafile_client.pem ca-file ${testdir}/set_cafile_interCA2.crt verify none no-ssl-reuse force-tlsv12 ciphers "aECDSA"
+ default-server ssl crt ${testdir}/set_cafile_client.pem ca-file ${testdir}/set_cafile_interCA2.crt verify none no-ssl-reuse force-tlsv12 ciphers "aECDSA" sni str(foo.com)
balance roundrobin
server cust_fmt "${tmpdir}/cust_logfmt_ssl.sock"
@@ -180,21 +206,20 @@
error-log-format "ERROR bc_err:%[bc_err]:%{+Q}[bc_err_str]\ ssl_bc_err:%[ssl_bc_err,and(proc.ssl_error_mask)]:%[ssl_bc_err_str]"
balance roundrobin
- server no_err "${tmpdir}/no_err_ssl.sock" ssl crt ${testdir}/set_cafile_client.pem ca-file ${testdir}/set_cafile_interCA2.crt verify required
- server srv_cert_rejected "${tmpdir}/srv_rejected_ssl.sock" ssl crt ${testdir}/set_cafile_client.pem ca-file ${testdir}/set_cafile_interCA1.crt verify required
- server mismatch_frontend "${tmpdir}/mismatch_fe_ssl.sock" ssl crt ${testdir}/set_cafile_client.pem ca-file ${testdir}/set_cafile_interCA2.crt verify required verifyhost str(toto)
- # We force TLSv1.2 for this specific case because server-side
+ server no_err "${tmpdir}/no_err_ssl.sock" ssl crt ${testdir}/set_cafile_client.pem ca-file ${testdir}/set_cafile_interCA2.crt verify required sni str(Server)
+ server srv_cert_rejected "${tmpdir}/srv_rejected_ssl.sock" ssl crt ${testdir}/set_cafile_client.pem ca-file ${testdir}/set_cafile_interCA1.crt verify required sni str(foo.com)
+ server mismatch_frontend "${tmpdir}/mismatch_fe_ssl.sock" ssl crt ${testdir}/set_cafile_client.pem ca-file ${testdir}/set_cafile_interCA2.crt verify required sni str(foo.com) verifyhost str(toto) # We force TLSv1.2 for this specific case because server-side
# verification errors cannot be caught by the backend fetches when
# using TLSv1.3
- server clt_cert_rejected "${tmpdir}/rejected_ssl.sock" ssl crt ${testdir}/set_cafile_client.pem ca-file ${testdir}/set_cafile_interCA2.crt verify none force-tlsv12
- server wrong_ciphers "${tmpdir}/wrong_ciphers_ssl.sock" ssl verify none crt ${testdir}/client1.pem ca-file ${testdir}/ca-auth.crt force-tlsv12 ciphers "aECDSA"
+ server clt_cert_rejected "${tmpdir}/rejected_ssl.sock" ssl crt ${testdir}/set_cafile_client.pem ca-file ${testdir}/set_cafile_interCA2.crt verify none force-tlsv12 sni str(foo.com)
+ server wrong_ciphers "${tmpdir}/wrong_ciphers_ssl.sock" ssl verify none crt ${testdir}/client1.pem ca-file ${testdir}/ca-auth.crt force-tlsv12 ciphers "aECDSA" sni str(foo.com)
# No TLSv1.3 support with OpenSSL 1.0.2 so we duplicate the previous
# wrong cipher test in this case so that the error log remains the same
.if openssl_version_before(1.1.1)
- server wrong_ciphers2 "${tmpdir}/wrong_ciphers_ssl.sock" ssl verify none crt ${testdir}/client1.pem ca-file ${testdir}/ca-auth.crt force-tlsv12 ciphers "aECDSA"
+ server wrong_ciphers2 "${tmpdir}/wrong_ciphers_ssl.sock" ssl verify none crt ${testdir}/client1.pem ca-file ${testdir}/ca-auth.crt force-tlsv12 ciphers "aECDSA" sni str(foo.com)
.else
- server wrong_ciphers_tls13 "${tmpdir}/wrong_ciphers_tls13_ssl.sock" ssl verify none crt ${testdir}/client1.pem ca-file ${testdir}/ca-auth.crt ciphersuites "TLS_AES_256_GCM_SHA384:TLS_CHACHA20_POLY1305_SHA256" force-tlsv13
+ server wrong_ciphers_tls13 "${tmpdir}/wrong_ciphers_tls13_ssl.sock" ssl verify none crt ${testdir}/client1.pem ca-file ${testdir}/ca-auth.crt ciphersuites "TLS_AES_256_GCM_SHA384:TLS_CHACHA20_POLY1305_SHA256" force-tlsv13 sni str(foo.com)
.endif
@@ -226,29 +251,39 @@
server s1 ${s1_addr}:${s1_port}
+
+ defaults bknd_err_dflt
+ timeout connect "${HAPROXY_TEST_TIMEOUT-5s}"
+ timeout client "${HAPROXY_TEST_TIMEOUT-5s}"
+ timeout server "${HAPROXY_TEST_TIMEOUT-5s}"
+ retries 0
+ log ${Slg_bcknd_fe_addr}:${Slg_bcknd_fe_port} local0
+ log-format "%ci:%cp %[ssl_fc_sni]/%sslv/%sslc"
+ error-log-format "ERROR %ci:%cp %[ssl_fc_sni]/%sslv/%sslc"
+
# The following listeners allow to test backend error fetches
- listen no_backend_err_ssl_lst
+ listen no_backend_err_ssl_lst from bknd_err_dflt
bind "${tmpdir}/no_err_ssl.sock" ssl crt ${testdir}/set_cafile_server.pem ca-file ${testdir}/set_cafile_interCA2.crt verify none
server s1 ${s1_addr}:${s1_port}
- listen srv_rejected_ssl_lst
+ listen srv_rejected_ssl_lst from bknd_err_dflt
bind "${tmpdir}/srv_rejected_ssl.sock" ssl crt ${testdir}/set_cafile_server.pem ca-file ${testdir}/set_cafile_interCA2.crt verify none
server s1 ${s1_addr}:${s1_port}
- listen mismatch_fe_ssl_lst
+ listen mismatch_fe_ssl_lst from bknd_err_dflt
bind "${tmpdir}/mismatch_fe_ssl.sock" ssl crt ${testdir}/set_cafile_server.pem ca-file ${testdir}/set_cafile_interCA2.crt verify none
server s1 ${s1_addr}:${s1_port}
- listen rejected_clt_ssl_lst
+ listen rejected_clt_ssl_lst from bknd_err_dflt
bind "${tmpdir}/rejected_ssl.sock" ssl crt ${testdir}/set_cafile_server.pem ca-file ${testdir}/set_cafile_interCA2.crt verify required
server s1 ${s1_addr}:${s1_port}
- listen wrong_ciphers_ssl_lst
+ listen wrong_ciphers_ssl_lst from bknd_err_dflt
bind "${tmpdir}/wrong_ciphers_ssl.sock" ssl crt ${testdir}/common.pem ca-file ${testdir}/ca-auth.crt verify none force-tlsv12 ciphers "kRSA"
server s1 ${s1_addr}:${s1_port}
.if openssl_version_atleast(1.1.1)
- listen wrong_ciphers_tls13_ssl_lst
+ listen wrong_ciphers_tls13_ssl_lst from bknd_err_dflt
bind "${tmpdir}/wrong_ciphers_tls13_ssl.sock" ssl crt ${testdir}/common.pem ca-file ${testdir}/ca-auth.crt verify none force-tlsv13 ciphersuites "TLS_AES_128_GCM_SHA256"
server s1 ${s1_addr}:${s1_port}
.endif
@@ -382,3 +417,4 @@
syslog Slg_https_fmt_err -wait
syslog Slg_logconnerror -wait
syslog Slg_bcknd -wait
+syslog Slg_bcknd_fe -wait
diff --git a/src/ssl_sample.c b/src/ssl_sample.c
index 8aaead2..ca09829 100644
--- a/src/ssl_sample.c
+++ b/src/ssl_sample.c
@@ -1565,10 +1565,18 @@
return 0;
smp->data.u.str.area = (char *)SSL_get_servername(ssl, TLSEXT_NAMETYPE_host_name);
- if (!smp->data.u.str.area)
- return 0;
+ if (!smp->data.u.str.area) {
+ /* We might have stored the SNI ourselves, look for it in the
+ * context's ex_data.
+ */
+ smp->data.u.str.area = SSL_get_ex_data(ssl, ssl_client_sni_index);
+
+ if (!smp->data.u.str.area)
+ return 0;
+ }
smp->data.u.str.data = strlen(smp->data.u.str.area);
+
return 1;
#else
/* SNI not supported */
diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index 47bbf1d..576b21d 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -134,6 +134,8 @@
DECLARE_STATIC_POOL(ssl_sock_ctx_pool, "ssl_sock_ctx_pool", sizeof(struct ssl_sock_ctx));
+DECLARE_STATIC_POOL(ssl_sock_client_sni_pool, "ssl_sock_client_sni_pool", TLSEXT_MAXLEN_host_name + 1);
+
/* ssl stats module */
enum {
SSL_ST_SESS,
@@ -443,6 +445,9 @@
int ssl_client_crt_ref_index = -1;
+/* Used to store the client's SNI in case of ClientHello callback error */
+int ssl_client_sni_index = -1;
+
#if (defined SSL_CTRL_SET_TLSEXT_TICKET_KEY_CB && TLS_TICKETS_NO > 0)
struct list tlskeys_reference = LIST_HEAD_INIT(tlskeys_reference);
#endif
@@ -2705,6 +2710,21 @@
goto allow_early;
}
+ /* We are about to raise an handshake error so the servername extension
+ * callback will never be called and the SNI will never be stored in the
+ * SSL context. In order for the ssl_fc_sni sample fetch to still work
+ * in such a case, we store the SNI ourselves as an ex_data information
+ * in the SSL context.
+ */
+ {
+ char *client_sni = pool_alloc(ssl_sock_client_sni_pool);
+ if (client_sni) {
+ strncpy(client_sni, trash.area, TLSEXT_MAXLEN_host_name);
+ client_sni[TLSEXT_MAXLEN_host_name] = '\0';
+ SSL_set_ex_data(ssl, ssl_client_sni_index, client_sni);
+ }
+ }
+
/* other cases fallback on abort, if strict-sni is set but no node was found */
abort:
@@ -7584,6 +7604,11 @@
X509_free((X509*)ptr);
}
+static void ssl_sock_clt_sni_free_func(void *parent, void *ptr, CRYPTO_EX_DATA *ad, int idx, long argl, void *argp)
+{
+ pool_free(ssl_sock_client_sni_pool, ptr);
+}
+
__attribute__((constructor))
static void __ssl_sock_init(void)
{
@@ -7632,6 +7657,7 @@
ssl_keylog_index = SSL_get_ex_new_index(0, NULL, NULL, NULL, ssl_sock_keylog_free_func);
#endif
ssl_client_crt_ref_index = SSL_get_ex_new_index(0, NULL, NULL, NULL, ssl_sock_clt_crt_free_func);
+ ssl_client_sni_index = SSL_get_ex_new_index(0, NULL, NULL, NULL, ssl_sock_clt_sni_free_func);
#ifndef OPENSSL_NO_ENGINE
ENGINE_load_builtin_engines();
hap_register_post_check(ssl_check_async_engine_count);