Hi, > I take it that means theres no internal debug logging for the tls errors that > we can just expose via logfile?
Proof of concept patches are attached with build instructions. You may wish to edit the haproxy-2.2.3/rules/debian folder to increase the -j setting to your current number of cores. The "disambiguate-ssl-handshake-errors-1.patch" only adds additional error messages for the initial ClientHello processing - realistically, it's only useful to see if there is no SNI being sent (bots, healthchecks are the usual offenders). The "disambiguate-ssl-handshake-errors-2.patch" implements everything the first patch implements, adds a trash chunk for logging additional error data to the conn structure, and reuses the SSL error logging logic from ssl_sock_dump_errors Practically, this means that memory usage is higher - if I recall correctly (and it's way to late/early at this point) it's a 16KB overhead per connection (echo "show pools" | socat stdio /var/run/haproxy.sock will have a more detailed breakdown). Watching the output of show pools is recommended - while I haven't noticed a memory leak, keeping an eye on the trash pool is a good idea. The fcgi protocol is also affected by the addition of the extra_err_data. I have to do a smoke test if "proto fcgi" behaves as expected, or if there's a potential segfault. The patch works, but it requires more extensive testing. Sharing it as-is since I might not be able to pursue this further in a significant way for some time. The mapping between the error messages and the potential causes can be a bit obscure, but it's still useful. E.g. an invalid SNI when using strict-sni maps to: tls_post_process_client_hello: no shared cipher If there's a cipher mismatch, this also maps to the above error message. A protocol version mismatch (e.g. trying TLS1 when only TLS1.2 is supported) results in: tls_early_post_process_client_hello: unsupported protocol. The list of error codes is available upstream at https://github.com/openssl/openssl/blob/OpenSSL_1_1_1-stable/crypto/err/openssl.txt#L2774 . Regarding the packet capture question - exporting libpcap data via SPOE might be possible. It's an ongoing topic. Regards, Bruno
diff -ur a/include/haproxy/connection.h b/include/haproxy/connection.h
--- a/include/haproxy/connection.h 2020-09-10 05:49:02.705917730 +0200
+++ b/include/haproxy/connection.h 2020-09-10 05:12:30.287797636 +0200
@@ -382,8 +382,10 @@
struct connection *conn;
conn = pool_alloc(pool_head_connection);
+ conn->extra_err_data = alloc_trash_chunk();
if (likely(conn != NULL))
- conn_init(conn);
+ if(likely(conn->extra_err_data != NULL))
+ conn_init(conn);
return conn;
}
@@ -458,7 +460,7 @@
sess->idle_conns--;
session_unown_conn(sess, conn);
}
-
+ free_trash_chunk(conn->extra_err_data);
sockaddr_free(&conn->src);
sockaddr_free(&conn->dst);
@@ -697,8 +699,12 @@
case CO_ER_SSL_CRT_FAIL: return "SSL client certificate not trusted";
case CO_ER_SSL_MISMATCH: return "Server presented an SSL certificate different from the configured one";
case CO_ER_SSL_MISMATCH_SNI: return "Server presented an SSL certificate different from the expected one";
- case CO_ER_SSL_HANDSHAKE: return "SSL handshake failure";
+ case CO_ER_SSL_HANDSHAKE: return "SSL handshake failure";
case CO_ER_SSL_HANDSHAKE_HB: return "SSL handshake failure after heartbeat";
+ case CO_ER_SSL_HSHK_CL_SNI_GBRSH: return "SSL handshake failure: ClientHello server name (SNI) null, too long, or invalid";
+ case CO_ER_SSL_HSHK_CL_S_SNI: return "SSL handshake failure: ClientHello server name (SNI) missing. SNI is required when strict-sni is used";
+ case CO_ER_SSL_HSHK_CL_CIPHERS: return "SSL handshake failure: ClientHello ciphers are invalid";
+ case CO_ER_SSL_HSHK_CL_SIGALGS: return "SSL handshake failure: ClientHello signature algorithms are invalid";
case CO_ER_SSL_KILLED_HB: return "Stopped a TLSv1 heartbeat attack (CVE-2014-0160)";
case CO_ER_SSL_NO_TARGET: return "Attempt to use SSL on an unknown target (internal error)";
diff -ur a/include/haproxy/connection-t.h b/include/haproxy/connection-t.h
--- a/include/haproxy/connection-t.h 2020-09-10 05:49:02.705917730 +0200
+++ b/include/haproxy/connection-t.h 2020-09-10 05:13:00.007799264 +0200
@@ -233,6 +233,10 @@
CO_ER_SSL_MISMATCH_SNI, /* Server presented an SSL certificate different from the expected one */
CO_ER_SSL_HANDSHAKE, /* SSL error during handshake */
CO_ER_SSL_HANDSHAKE_HB, /* SSL error during handshake with heartbeat present */
+ CO_ER_SSL_HSHK_CL_SNI_GBRSH, /* SSL handshake failure: ClientHello server name (SNI) null, too long, or invalid */
+ CO_ER_SSL_HSHK_CL_S_SNI, /* SSL handshake failure: ClientHello server name (SNI) missing - SNI is required if strict-sni is enabled */
+ CO_ER_SSL_HSHK_CL_CIPHERS, /* SSL handshake failure: ClientHello ciphers are invalid */
+ CO_ER_SSL_HSHK_CL_SIGALGS, /* SSL handshake failure: ClientHello signature algorithms are invalid */
CO_ER_SSL_KILLED_HB, /* Stopped a TLSv1 heartbeat attack (CVE-2014-0160) */
CO_ER_SSL_NO_TARGET, /* unknown target (not client nor server) */
CO_ER_SSL_EARLY_FAILED, /* Server refused early data */
@@ -467,6 +471,7 @@
/* first cache line */
enum obj_type obj_type; /* differentiates connection from applet context */
unsigned char err_code; /* CO_ER_* */
+ struct buffer *extra_err_data; /* For storing (usually SSL-related) extra data */
signed short send_proxy_ofs; /* <0 = offset to (re)send from the end, >0 = send all (reused for SOCKS4) */
unsigned int flags; /* CO_FL_* */
const struct protocol *ctrl; /* operations at the socket layer */
diff -ur a/src/mux_fcgi.c b/src/mux_fcgi.c
--- a/src/mux_fcgi.c 2020-09-10 05:49:02.713917730 +0200
+++ b/src/mux_fcgi.c 2020-09-10 03:56:42.229548508 +0200
@@ -783,6 +783,7 @@
task_destroy(t);
if (fconn->wait_event.tasklet)
tasklet_free(fconn->wait_event.tasklet);
+ free_trash_chunk(fconn->conn->extra_err_data);
pool_free(pool_head_fcgi_conn, fconn);
fail_conn:
conn->ctx = conn_ctx; // restore saved ctx
@@ -853,6 +854,7 @@
conn->xprt->unsubscribe(conn, conn->xprt_ctx, fconn->wait_event.events,
&fconn->wait_event);
+ free_trash_chunk(fconn->conn->extra_err_data);
pool_free(pool_head_fcgi_conn, fconn);
}
diff -ur a/src/session.c b/src/session.c
--- a/src/session.c 2020-09-10 05:49:02.713917730 +0200
+++ b/src/session.c 2020-09-10 05:44:14.149901924 +0200
@@ -381,12 +381,13 @@
session_prepare_log_prefix(sess);
err_msg = conn_err_code_str(conn);
+
if (err_msg)
- send_log(sess->fe, level, "%s: %s\n", trash.area,
- err_msg);
+ send_log(sess->fe, level, "%s: %s %s\n", trash.area,
+ err_msg, conn->extra_err_data->area);
else
- send_log(sess->fe, level, "%s: unknown connection error (code=%d flags=%08x)\n",
- trash.area, conn->err_code, conn->flags);
+ send_log(sess->fe, level, "%s: unknown connection error (code=%d flags=%08x) %s\n",
+ trash.area, conn->err_code, conn->flags, conn->extra_err_data->area);
}
/* kill the connection now */
diff -ur a/src/ssl_sock.c b/src/ssl_sock.c
--- a/src/ssl_sock.c 2020-09-10 05:49:02.711917730 +0200
+++ b/src/ssl_sock.c 2020-09-10 05:34:41.710870567 +0200
@@ -2240,28 +2240,38 @@
* was written, so parsing the normal case is a bit complex.
*/
size_t len;
- if (extension_len <= 2)
+ if (extension_len <= 2) {
+ conn->err_code = CO_ER_SSL_HSHK_CL_SNI_GBRSH;
goto abort;
+ }
/* Extract the length of the supplied list of names. */
len = (*extension_data++) << 8;
len |= *extension_data++;
- if (len + 2 != extension_len)
+ if (len + 2 != extension_len) {
+ conn->err_code = CO_ER_SSL_HSHK_CL_SNI_GBRSH;
goto abort;
+ }
/*
* The list in practice only has a single element, so we only consider
* the first one.
*/
- if (len == 0 || *extension_data++ != TLSEXT_NAMETYPE_host_name)
+ if (len == 0 || *extension_data++ != TLSEXT_NAMETYPE_host_name) {
+ conn->err_code = CO_ER_SSL_HSHK_CL_SNI_GBRSH;
goto abort;
+ }
extension_len = len - 1;
/* Now we can finally pull out the byte array with the actual hostname. */
- if (extension_len <= 2)
+ if (extension_len <= 2) {
+ conn->err_code = CO_ER_SSL_HSHK_CL_SNI_GBRSH;
goto abort;
+ }
len = (*extension_data++) << 8;
len |= *extension_data++;
if (len == 0 || len + 2 > extension_len || len > TLSEXT_MAXLEN_host_name
- || memchr(extension_data, 0, len) != NULL)
+ || memchr(extension_data, 0, len) != NULL) {
+ conn->err_code = CO_ER_SSL_HSHK_CL_SNI_GBRSH;
goto abort;
+ }
servername = extension_data;
servername_len = len;
} else {
@@ -2277,6 +2287,7 @@
HA_RWLOCK_RDUNLOCK(SNI_LOCK, &s->sni_lock);
goto allow_early;
}
+ conn->err_code = CO_ER_SSL_HSHK_CL_S_SNI;
goto abort;
}
@@ -2288,14 +2299,20 @@
#endif
uint8_t sign;
size_t len;
- if (extension_len < 2)
+ if (extension_len < 2) {
+ conn->err_code = CO_ER_SSL_HSHK_CL_SIGALGS;
goto abort;
+ }
len = (*extension_data++) << 8;
len |= *extension_data++;
- if (len + 2 != extension_len)
+ if (len + 2 != extension_len) {
+ conn->err_code = CO_ER_SSL_HSHK_CL_SIGALGS;
goto abort;
- if (len % 2 != 0)
+ }
+ if (len % 2 != 0) {
+ conn->err_code = CO_ER_SSL_HSHK_CL_SIGALGS;
goto abort;
+ }
for (; len > 0; len -= 2) {
extension_data++; /* hash */
sign = *extension_data++;
@@ -2327,8 +2344,10 @@
#else
len = SSL_client_hello_get0_ciphers(ssl, &cipher_suites);
#endif
- if (len % 2 != 0)
+ if (len % 2 != 0) {
+ conn->err_code = CO_ER_SSL_HSHK_CL_CIPHERS;
goto abort;
+ }
for (; len != 0; len -= 2, cipher_suites += 2) {
#ifdef OPENSSL_IS_BORINGSSL
uint16_t cipher_suite = (cipher_suites[0] << 8) | cipher_suites[1];
@@ -2446,7 +2465,8 @@
return 1;
abort:
/* abort handshake (was SSL_TLSEXT_ERR_ALERT_FATAL) */
- conn->err_code = CO_ER_SSL_HANDSHAKE;
+ if (!conn->err_code)
+ conn->err_code = CO_ER_SSL_HANDSHAKE;
#ifdef OPENSSL_IS_BORINGSSL
return ssl_select_cert_error;
#else
@@ -5554,6 +5574,10 @@
out_error:
/* Clear openssl global errors stack */
+ ret = ERR_get_error();
+ chunk_printf(conn->extra_err_data, "fd[%04x] OpenSSL error[0x%lx] %s: %s",
+ (unsigned short)conn->handle.fd, ret,
+ ERR_func_error_string(ret), ERR_reason_error_string(ret));
ssl_sock_dump_errors(conn);
ERR_clear_error();
diff -ur a/include/haproxy/connection-t.h b/include/haproxy/connection-t.h
--- a/include/haproxy/connection-t.h 2020-07-31 09:54:32.000000000 +0000
+++ b/include/haproxy/connection-t.h 2020-09-02 04:01:41.287339829 +0000
@@ -233,6 +233,10 @@
CO_ER_SSL_MISMATCH_SNI, /* Server presented an SSL certificate different from the expected one */
CO_ER_SSL_HANDSHAKE, /* SSL error during handshake */
CO_ER_SSL_HANDSHAKE_HB, /* SSL error during handshake with heartbeat present */
+ CO_ER_SSL_HSHK_CL_SNI_GBRSH, /* SSL handshake failure: ClientHello server name (SNI) null, too long, or invalid */
+ CO_ER_SSL_HSHK_CL_S_SNI, /* SSL handshake failure: ClientHello server name (SNI) invalid in strict-sni context */
+ CO_ER_SSL_HSHK_CL_CIPHERS, /* SSL handshake failure: ClientHello ciphers are invalid */
+ CO_ER_SSL_HSHK_CL_SIGALGS, /* SSL handshake failure: ClientHello signature algorithms are invalid */
CO_ER_SSL_KILLED_HB, /* Stopped a TLSv1 heartbeat attack (CVE-2014-0160) */
CO_ER_SSL_NO_TARGET, /* unknown target (not client nor server) */
CO_ER_SSL_EARLY_FAILED, /* Server refused early data */
diff -ur a/include/haproxy/connection.h b/include/haproxy/connection.h
--- a/include/haproxy/connection.h 2020-07-31 09:54:32.000000000 +0000
+++ b/include/haproxy/connection.h 2020-09-02 04:01:41.295339815 +0000
@@ -697,8 +697,12 @@
case CO_ER_SSL_CRT_FAIL: return "SSL client certificate not trusted";
case CO_ER_SSL_MISMATCH: return "Server presented an SSL certificate different from the configured one";
case CO_ER_SSL_MISMATCH_SNI: return "Server presented an SSL certificate different from the expected one";
- case CO_ER_SSL_HANDSHAKE: return "SSL handshake failure";
+ case CO_ER_SSL_HANDSHAKE: return "SSL handshake failure";
case CO_ER_SSL_HANDSHAKE_HB: return "SSL handshake failure after heartbeat";
+ case CO_ER_SSL_HSHK_CL_SNI_GBRSH: return "SSL handshake failure: ClientHello server name (SNI) null, too long, or invalid";
+ case CO_ER_SSL_HSHK_CL_S_SNI: return "SSL handshake failure: ClientHello server name (SNI) invalid in strict-sni context";
+ case CO_ER_SSL_HSHK_CL_CIPHERS: return "SSL handshake failure: ClientHello ciphers are invalid";
+ case CO_ER_SSL_HSHK_CL_SIGALGS: return "SSL handshake failure: ClientHello signature algorithms are invalid";
case CO_ER_SSL_KILLED_HB: return "Stopped a TLSv1 heartbeat attack (CVE-2014-0160)";
case CO_ER_SSL_NO_TARGET: return "Attempt to use SSL on an unknown target (internal error)";
diff -ur a/src/ssl_sock.c b/src/ssl_sock.c
--- a/src/ssl_sock.c 2020-07-31 09:54:32.000000000 +0000
+++ b/src/ssl_sock.c 2020-09-02 04:01:41.315339779 +0000
@@ -2207,28 +2207,38 @@
* was written, so parsing the normal case is a bit complex.
*/
size_t len;
- if (extension_len <= 2)
+ if (extension_len <= 2) {
+ conn->err_code = CO_ER_SSL_HSHK_CL_SNI_GBRSH;
goto abort;
+ }
/* Extract the length of the supplied list of names. */
len = (*extension_data++) << 8;
len |= *extension_data++;
- if (len + 2 != extension_len)
+ if (len + 2 != extension_len) {
+ conn->err_code = CO_ER_SSL_HSHK_CL_SNI_GBRSH;
goto abort;
+ }
/*
* The list in practice only has a single element, so we only consider
* the first one.
*/
- if (len == 0 || *extension_data++ != TLSEXT_NAMETYPE_host_name)
+ if (len == 0 || *extension_data++ != TLSEXT_NAMETYPE_host_name) {
+ conn->err_code = CO_ER_SSL_HSHK_CL_SNI_GBRSH;
goto abort;
+ }
extension_len = len - 1;
/* Now we can finally pull out the byte array with the actual hostname. */
- if (extension_len <= 2)
+ if (extension_len <= 2) {
+ conn->err_code = CO_ER_SSL_HSHK_CL_SNI_GBRSH;
goto abort;
+ }
len = (*extension_data++) << 8;
len |= *extension_data++;
if (len == 0 || len + 2 > extension_len || len > TLSEXT_MAXLEN_host_name
- || memchr(extension_data, 0, len) != NULL)
+ || memchr(extension_data, 0, len) != NULL) {
+ conn->err_code = CO_ER_SSL_HSHK_CL_SNI_GBRSH;
goto abort;
+ }
servername = extension_data;
servername_len = len;
} else {
@@ -2244,6 +2254,7 @@
HA_RWLOCK_RDUNLOCK(SNI_LOCK, &s->sni_lock);
goto allow_early;
}
+ conn->err_code = CO_ER_SSL_HSHK_CL_S_SNI;
goto abort;
}
@@ -2255,14 +2266,20 @@
#endif
uint8_t sign;
size_t len;
- if (extension_len < 2)
+ if (extension_len < 2) {
+ conn->err_code = CO_ER_SSL_HSHK_CL_SIGALGS;
goto abort;
+ }
len = (*extension_data++) << 8;
len |= *extension_data++;
- if (len + 2 != extension_len)
+ if (len + 2 != extension_len) {
+ conn->err_code = CO_ER_SSL_HSHK_CL_SIGALGS;
goto abort;
- if (len % 2 != 0)
+ }
+ if (len % 2 != 0) {
+ conn->err_code = CO_ER_SSL_HSHK_CL_SIGALGS;
goto abort;
+ }
for (; len > 0; len -= 2) {
extension_data++; /* hash */
sign = *extension_data++;
@@ -2294,8 +2311,10 @@
#else
len = SSL_client_hello_get0_ciphers(ssl, &cipher_suites);
#endif
- if (len % 2 != 0)
+ if (len % 2 != 0) {
+ conn->err_code = CO_ER_SSL_HSHK_CL_CIPHERS;
goto abort;
+ }
for (; len != 0; len -= 2, cipher_suites += 2) {
#ifdef OPENSSL_IS_BORINGSSL
uint16_t cipher_suite = (cipher_suites[0] << 8) | cipher_suites[1];
@@ -2391,7 +2410,8 @@
return 1;
abort:
/* abort handshake (was SSL_TLSEXT_ERR_ALERT_FATAL) */
- conn->err_code = CO_ER_SSL_HANDSHAKE;
+ if (!conn->err_code)
+ conn->err_code = CO_ER_SSL_HANDSHAKE;
#ifdef OPENSSL_IS_BORINGSSL
return ssl_select_cert_error;
#else
apt-debuild
Description: Binary data
