MEDIUM: ssl: Keep a reference to the client's certificate for use in logs
Most of the SSL sample fetches related to the client certificate were
based on the SSL_get_peer_certificate function which returns NULL when
the verification process failed. This made it impossible to use those
fetches in a log format since they would always be empty.
The patch adds a reference to the X509 object representing the client
certificate in the SSL structure and makes use of this reference in the
fetches.
The reference can only be obtained in ssl_sock_bind_verifycbk which
means that in case of an SSL error occurring before the verification
process ("no shared cipher" for instance, which happens while processing
the Client Hello), we won't ever start the verification process and it
will be impossible to get information about the client certificate.
This patch also allows most of the ssl_c_XXX fetches to return a usable
value in case of connection failure (because of a verification error for
instance) by making the "conn->flags & CO_FL_WAIT_XPRT" test (which
requires a connection to be established) less strict.
Thanks to this patch, a log-format such as the following should return
usable information in case of an error occurring during the verification
process :
log-format "DN=%{+Q}[ssl_c_s_dn] serial=%[ssl_c_serial,hex] \
hash=%[ssl_c_sha1,hex]"
It should answer to GitHub issue #693.
diff --git a/include/haproxy/ssl_utils.h b/include/haproxy/ssl_utils.h
index 2ebe034..28c5a84 100644
--- a/include/haproxy/ssl_utils.h
+++ b/include/haproxy/ssl_utils.h
@@ -39,6 +39,8 @@
int ssl_sock_get_dn_formatted(X509_NAME *a, const struct buffer *format, struct buffer *out);
int ssl_sock_get_dn_oneline(X509_NAME *a, struct buffer *out);
+X509* ssl_sock_get_peer_certificate(SSL *ssl);
+
#endif /* _HAPROXY_SSL_UTILS_H */
#endif /* USE_OPENSSL */
diff --git a/reg-tests/ssl/ssl_errors.vtc b/reg-tests/ssl/ssl_errors.vtc
index f138249..0d652d4 100644
--- a/reg-tests/ssl/ssl_errors.vtc
+++ b/reg-tests/ssl/ssl_errors.vtc
@@ -39,22 +39,25 @@
syslog Slg_cust_fmt -level info {
recv
- expect ~ ".*conn_status:\"0:Success\" hsk_err:\"0:-\""
+ expect ~ ".*conn_status:\"0:Success\" hsk_err:\"0:-\" CN=\"/C=FR/O=HAProxy Technologies/CN=Client\",serial=1007,hash=063DCC2E6A9159E66994B325D6D2EF3D17A75B6F"
barrier b1 sync
recv
- expect ~ ".*conn_status:\"30:SSL client CA chain cannot be verified\" hsk_err:\"337100934:error:1417C086:SSL routines:tls_process_client_certificate:certificate verify failed\""
+ expect ~ ".*conn_status:\"30:SSL client CA chain cannot be verified\" hsk_err:\"337100934:error:1417C086:SSL routines:tls_process_client_certificate:certificate verify failed\" CN=\"/C=FR/O=HAProxy Technologies/CN=Client\",serial=1007,hash=063DCC2E6A9159E66994B325D6D2EF3D17A75B6F"
barrier b1 sync
recv
- expect ~ ".*conn_status:\"31:SSL client certificate not trusted\" hsk_err:\"337100934:error:1417C086:SSL routines:tls_process_client_certificate:certificate verify failed\""
+ expect ~ ".*conn_status:\"31:SSL client certificate not trusted\" hsk_err:\"337100934:error:1417C086:SSL routines:tls_process_client_certificate:certificate verify failed\" CN=\"/C=FR/O=HAProxy Technologies/CN=Client\",serial=1007,hash=063DCC2E6A9159E66994B325D6D2EF3D17A75B6F"
barrier b1 sync
+ # In case of an error occuring before the certificate verification process,
+ # the client certificate chain is never parsed and verified so we can't
+ # have information about the client's certificate.
recv
- expect ~ ".*conn_status:\"34:SSL handshake failure\" hsk_err:\"337678529:error:142090C1:SSL routines:tls_early_post_process_client_hello:no shared cipher\""
+ expect ~ ".*conn_status:\"34:SSL handshake failure\" hsk_err:\"337678529:error:142090C1:SSL routines:tls_early_post_process_client_hello:no shared cipher\" CN=\"\",serial=-,hash=-"
} -start
syslog Slg_https_fmt -level info {
@@ -136,7 +139,7 @@
log ${Slg_cust_fmt_addr}:${Slg_cust_fmt_port} local0
option log-error-via-logformat
mode http
- log-format "conn_status:\"%[fc_conn_err]:%[fc_conn_err_str]\" hsk_err:\"%[ssl_fc_hsk_err]:%[ssl_fc_hsk_err_str]\""
+ log-format "conn_status:\"%[fc_conn_err]:%[fc_conn_err_str]\" hsk_err:\"%[ssl_fc_hsk_err]:%[ssl_fc_hsk_err_str]\" CN=%{+Q}[ssl_c_s_dn],serial=%[ssl_c_serial,hex],hash=%[ssl_c_sha1,hex]"
bind "${tmpdir}/cust_logfmt_ssl.sock" ssl crt ${testdir}/set_cafile_server.pem ca-verify-file ${testdir}/set_cafile_rootCA.crt ca-file ${testdir}/set_cafile_interCA1.crt verify required ciphersuites "TLS_AES_256_GCM_SHA384"
server s1 ${s1_addr}:${s1_port}
diff --git a/src/ssl_sample.c b/src/ssl_sample.c
index 8bfea07..f4e0c93 100644
--- a/src/ssl_sample.c
+++ b/src/ssl_sample.c
@@ -109,13 +109,13 @@
if (!ssl)
return 0;
- if (conn->flags & CO_FL_WAIT_XPRT) {
+ if (conn->flags & CO_FL_WAIT_XPRT && !conn->err_code) {
smp->flags |= SMP_F_MAY_CHANGE;
return 0;
}
if (cert_peer)
- crt = SSL_get_peer_certificate(ssl);
+ crt = ssl_sock_get_peer_certificate(ssl);
else
crt = SSL_get_certificate(ssl);
@@ -226,13 +226,13 @@
if (!ssl)
return 0;
- if (conn->flags & CO_FL_WAIT_XPRT) {
+ if (conn->flags & CO_FL_WAIT_XPRT && !conn->err_code) {
smp->flags |= SMP_F_MAY_CHANGE;
return 0;
}
if (cert_peer)
- crt = SSL_get_peer_certificate(ssl);
+ crt = ssl_sock_get_peer_certificate(ssl);
else
crt = SSL_get_certificate(ssl);
@@ -280,13 +280,13 @@
if (!ssl)
return 0;
- if (conn->flags & CO_FL_WAIT_XPRT) {
+ if (conn->flags & CO_FL_WAIT_XPRT && !conn->err_code) {
smp->flags |= SMP_F_MAY_CHANGE;
return 0;
}
if (cert_peer)
- crt = SSL_get_peer_certificate(ssl);
+ crt = ssl_sock_get_peer_certificate(ssl);
else
crt = SSL_get_certificate(ssl);
if (!crt)
@@ -331,13 +331,13 @@
if (!ssl)
return 0;
- if (conn->flags & CO_FL_WAIT_XPRT) {
+ if (conn->flags & CO_FL_WAIT_XPRT && !conn->err_code) {
smp->flags |= SMP_F_MAY_CHANGE;
return 0;
}
if (cert_peer)
- crt = SSL_get_peer_certificate(ssl);
+ crt = ssl_sock_get_peer_certificate(ssl);
else
crt = SSL_get_certificate(ssl);
if (!crt)
@@ -383,13 +383,13 @@
if (!ssl)
return 0;
- if (conn->flags & CO_FL_WAIT_XPRT) {
+ if (conn->flags & CO_FL_WAIT_XPRT && !conn->err_code) {
smp->flags |= SMP_F_MAY_CHANGE;
return 0;
}
if (cert_peer)
- crt = SSL_get_peer_certificate(ssl);
+ crt = ssl_sock_get_peer_certificate(ssl);
else
crt = SSL_get_certificate(ssl);
if (!crt)
@@ -451,13 +451,13 @@
if (!ssl)
return 0;
- if (conn->flags & CO_FL_WAIT_XPRT) {
+ if (conn->flags & CO_FL_WAIT_XPRT && !conn->err_code) {
smp->flags |= SMP_F_MAY_CHANGE;
return 0;
}
if (cert_peer)
- crt = SSL_get_peer_certificate(ssl);
+ crt = ssl_sock_get_peer_certificate(ssl);
else
crt = SSL_get_certificate(ssl);
if (!crt)
@@ -503,13 +503,13 @@
if (!ssl)
return 0;
- if (conn->flags & CO_FL_WAIT_XPRT) {
+ if (conn->flags & CO_FL_WAIT_XPRT && !conn->err_code) {
smp->flags |= SMP_F_MAY_CHANGE;
return 0;
}
if (cert_peer)
- crt = SSL_get_peer_certificate(ssl);
+ crt = ssl_sock_get_peer_certificate(ssl);
else
crt = SSL_get_certificate(ssl);
if (!crt)
@@ -560,13 +560,13 @@
if (!ssl)
return 0;
- if (conn->flags & CO_FL_WAIT_XPRT) {
+ if (conn->flags & CO_FL_WAIT_XPRT && !conn->err_code) {
smp->flags |= SMP_F_MAY_CHANGE;
return 0;
}
/* SSL_get_peer_certificate returns a ptr on allocated X509 struct */
- crt = SSL_get_peer_certificate(ssl);
+ crt = ssl_sock_get_peer_certificate(ssl);
if (crt) {
X509_free(crt);
}
@@ -599,13 +599,13 @@
if (!ssl)
return 0;
- if (conn->flags & CO_FL_WAIT_XPRT) {
+ if (conn->flags & CO_FL_WAIT_XPRT && !conn->err_code) {
smp->flags |= SMP_F_MAY_CHANGE;
return 0;
}
if (cert_peer)
- crt = SSL_get_peer_certificate(ssl);
+ crt = ssl_sock_get_peer_certificate(ssl);
else
crt = SSL_get_certificate(ssl);
if (!crt)
@@ -645,13 +645,13 @@
if (!ssl)
return 0;
- if (conn->flags & CO_FL_WAIT_XPRT) {
+ if (conn->flags & CO_FL_WAIT_XPRT && !conn->err_code) {
smp->flags |= SMP_F_MAY_CHANGE;
return 0;
}
if (cert_peer)
- crt = SSL_get_peer_certificate(ssl);
+ crt = ssl_sock_get_peer_certificate(ssl);
else
crt = SSL_get_certificate(ssl);
if (!crt)
@@ -701,13 +701,13 @@
if (!ssl)
return 0;
- if (conn->flags & CO_FL_WAIT_XPRT) {
+ if (conn->flags & CO_FL_WAIT_XPRT && !conn->err_code) {
smp->flags |= SMP_F_MAY_CHANGE;
return 0;
}
if (cert_peer)
- crt = SSL_get_peer_certificate(ssl);
+ crt = ssl_sock_get_peer_certificate(ssl);
else
crt = SSL_get_certificate(ssl);
if (!crt)
diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index ba61243..bc82783 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -443,6 +443,8 @@
struct pool_head *pool_head_ssl_keylog_str __read_mostly = NULL;
#endif
+int ssl_client_crt_ref_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
@@ -1557,19 +1559,51 @@
struct connection *conn;
struct ssl_sock_ctx *ctx;
int err, depth;
+ X509 *client_crt;
+ STACK_OF(X509) *certs;
ssl = X509_STORE_CTX_get_ex_data(x_store, SSL_get_ex_data_X509_STORE_CTX_idx());
conn = SSL_get_ex_data(ssl, ssl_app_data_index);
+ client_crt = SSL_get_ex_data(ssl, ssl_client_crt_ref_index);
ctx = conn->xprt_ctx;
ctx->xprt_st |= SSL_SOCK_ST_FL_VERIFY_DONE;
+ depth = X509_STORE_CTX_get_error_depth(x_store);
+ err = X509_STORE_CTX_get_error(x_store);
+
if (ok) /* no errors */
return ok;
- depth = X509_STORE_CTX_get_error_depth(x_store);
- err = X509_STORE_CTX_get_error(x_store);
+ /* Keep a reference to the client's certificate in order to be able to
+ * dump some fetches values in a log even when the verification process
+ * fails. */
+ if (depth == 0) {
+ X509_free(client_crt);
+ client_crt = X509_STORE_CTX_get0_cert(x_store);
+ if (client_crt) {
+ X509_up_ref(client_crt);
+ SSL_set_ex_data(ssl, ssl_client_crt_ref_index, client_crt);
+ }
+ }
+ else {
+ /* An error occurred on a CA certificate of the certificate
+ * chain, we might never call this verify callback on the client
+ * certificate's depth (which is 0) so we try to store the
+ * reference right now. */
+ if (X509_STORE_CTX_get0_chain(x_store) != NULL) {
+ certs = X509_STORE_CTX_get1_chain(x_store);
+ if (certs) {
+ client_crt = sk_X509_value(certs, 0);
+ if (client_crt) {
+ X509_up_ref(client_crt);
+ SSL_set_ex_data(ssl, ssl_client_crt_ref_index, client_crt);
+ }
+ }
+ sk_X509_pop_free(certs, X509_free);
+ }
+ }
/* check if CA error needs to be ignored */
if (depth > 0) {
@@ -7330,6 +7364,14 @@
}
#endif
+static void ssl_sock_clt_crt_free_func(void *parent, void *ptr, CRYPTO_EX_DATA *ad, int idx, long argl, void *argp)
+{
+ if (!ptr)
+ return;
+
+ X509_free((X509*)ptr);
+}
+
__attribute__((constructor))
static void __ssl_sock_init(void)
{
@@ -7377,6 +7419,7 @@
#ifdef HAVE_SSL_KEYLOG
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);
#ifndef OPENSSL_NO_ENGINE
ENGINE_load_builtin_engines();
hap_register_post_check(ssl_check_async_engine_count);
diff --git a/src/ssl_utils.c b/src/ssl_utils.c
index cb94140..ce17e95 100644
--- a/src/ssl_utils.c
+++ b/src/ssl_utils.c
@@ -287,3 +287,31 @@
return 1;
}
+
+extern int ssl_client_crt_ref_index;
+
+/*
+ * This function fetches the SSL certificate for a specific connection (either
+ * client certificate or server certificate depending on the cert_peer
+ * parameter).
+ * When trying to get the peer certificate from the server side, we first try to
+ * use the dedicated SSL_get_peer_certificate function, but we fall back to
+ * trying to get the client certificate reference that might have been stored in
+ * the SSL structure's ex_data during the verification process.
+ * Returns NULL in case of failure.
+ */
+X509* ssl_sock_get_peer_certificate(SSL *ssl)
+{
+ X509* cert;
+
+ cert = SSL_get_peer_certificate(ssl);
+ /* Get the client certificate reference stored in the SSL
+ * structure's ex_data during the verification process. */
+ if (!cert) {
+ cert = SSL_get_ex_data(ssl, ssl_client_crt_ref_index);
+ if (cert)
+ X509_up_ref(cert);
+ }
+
+ return cert;
+}