RepoMirrors/haproxy
1
0
Fork 0
mirror of http://git.haproxy.org/git/haproxy.git/ synced 2024-12-27 23:22:09 +00:00
Code Issues Packages Projects Releases Wiki Activity

BUG/MINOR: ssl: Store client SNI in SSL context in case of ClientHello error

Browse Source 
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.
This commit is contained in:
Remi Tricot-Le Breton 2022-01-07 17:12:01 +01:00 committed by William Lallemand
parent f82afbb9cd
commit a996763619
4 changed files with 94 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
include/haproxy/ssl_sock.h
View File

@ -47,6 +47,7 @@ extern int nb_engines;
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;

78
reg-tests/ssl/ssl_errors.vtc
View File

@ -63,24 +63,24 @@ syslog Slg_cust_fmt -level info {
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 @@ syslog Slg_bcknd -level info {
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 @@ syslog Slg_bcknd -level info {
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 @@ haproxy h1 -conf {
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 @@ haproxy h1 -conf {
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 @@ haproxy h1 -conf {
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 @@ haproxy h1 -conf {
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 -wait
syslog Slg_https_fmt_err -wait
syslog Slg_logconnerror -wait
syslog Slg_bcknd -wait
syslog Slg_bcknd_fe -wait

12
src/ssl_sample.c
View File

@ -1565,10 +1565,18 @@ smp_fetch_ssl_fc_sni(const struct arg *args, struct sample *smp, const char *kw,
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 */

26
src/ssl_sock.c
View File

@ -134,6 +134,8 @@ static BIO_METHOD *ha_meth;
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 @@ struct pool_head *pool_head_ssl_keylog_str __read_mostly = NULL;
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 @@ int ssl_sock_switchctx_cbk(SSL *ssl, int *al, void *arg)
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 @@ static void ssl_sock_clt_crt_free_func(void *parent, void *ptr, CRYPTO_EX_DATA *
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 @@ static void __ssl_sock_init(void)
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);