BUG/MINOR: quic: Possible crash when verifying certificates
This verification is done by ssl_sock_bind_verifycbk() which is set at different
locations in the ssl_sock.c code . About QUIC connections, there are a lot of chances
the connection object is not initialized when entering this function. What must
be accessed is the SSL object to retrieve the connection or quic_conn objects,
then the bind_conf object of the listener. If the connection object is not found,
we try to find the quic_conn object.
Modify ssl_sock_dump_errors() interface which takes a connection object as parameter
to also passed a quic_conn object as parameter. Again this function try first
to access the connection object if not NULL or the quic_conn object if not.
There is a remaining thing to do for QUIC: store the certificate verification error
code as it is currently stored in the connection object. This error code is at least
used by the "bc_err" and "fc_err" sample fetches.
There are chances this bug is in relation with GH #1851. Thank you to @tasavis
for the report.
Must be merged into 2.6.
diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index 0edad3b..caf41eb 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -638,7 +638,8 @@
* if the debug mode and the verbose mode are activated. It dump all
* the SSL error until the stack was empty.
*/
-static forceinline void ssl_sock_dump_errors(struct connection *conn)
+static forceinline void ssl_sock_dump_errors(struct connection *conn,
+ struct quic_conn *qc)
{
unsigned long ret;
@@ -650,9 +651,18 @@
ret = ERR_get_error();
if (ret == 0)
return;
- fprintf(stderr, "fd[%#x] OpenSSL error[0x%lx] %s: %s\n",
- conn_fd(conn), ret,
- func, ERR_reason_error_string(ret));
+ if (conn) {
+ fprintf(stderr, "fd[%#x] OpenSSL error[0x%lx] %s: %s\n",
+ conn_fd(conn), ret,
+ func, ERR_reason_error_string(ret));
+ }
+#ifdef USE_QUIC
+ else {
+ /* TODO: we are not sure <conn> is always initialized for QUIC connections */
+ fprintf(stderr, "qc @%p OpenSSL error[0x%lx] %s: %s\n", qc, ret,
+ func, ERR_reason_error_string(ret));
+ }
+#endif
}
}
}
@@ -1699,16 +1709,36 @@
{
SSL *ssl;
struct connection *conn;
- struct ssl_sock_ctx *ctx;
+ struct ssl_sock_ctx *ctx = NULL;
int err, depth;
X509 *client_crt;
STACK_OF(X509) *certs;
+ struct bind_conf *bind_conf;
+ struct quic_conn *qc = NULL;
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);
+ if (conn) {
+ bind_conf = __objt_listener(conn->target)->bind_conf;
+ ctx = __conn_get_ssl_sock_ctx(conn);
+ }
+#ifdef USE_QUIC
+ else {
+ qc = SSL_get_ex_data(ssl, ssl_qc_app_data_index);
+ if (qc) {
+ bind_conf = qc->li->bind_conf;
+ ctx = qc->xprt_ctx;
+ }
+ }
+#endif
+
+ if (!ctx || !bind_conf) {
+ /* Must never happen */
+ ABORT_NOW();
+ }
+
- ctx = __conn_get_ssl_sock_ctx(conn);
ctx->xprt_st |= SSL_SOCK_ST_FL_VERIFY_DONE;
depth = X509_STORE_CTX_get_error_depth(x_store);
@@ -1751,13 +1781,12 @@
ctx->xprt_st |= SSL_SOCK_CAEDEPTH_TO_ST(depth);
}
- if (err < 64 && __objt_listener(conn->target)->bind_conf->ca_ignerr & (1ULL << err)) {
- ssl_sock_dump_errors(conn);
- ERR_clear_error();
- return 1;
- }
+ if (err < 64 && bind_conf->ca_ignerr & (1ULL << err))
+ goto err_ignored;
- conn->err_code = CO_ER_SSL_CA_FAIL;
+ /* TODO: for QUIC connection, this error code is lost */
+ if (conn)
+ conn->err_code = CO_ER_SSL_CA_FAIL;
return 0;
}
@@ -1765,14 +1794,18 @@
ctx->xprt_st |= SSL_SOCK_CRTERROR_TO_ST(err);
/* check if certificate error needs to be ignored */
- if (err < 64 && __objt_listener(conn->target)->bind_conf->crt_ignerr & (1ULL << err)) {
- ssl_sock_dump_errors(conn);
- ERR_clear_error();
- return 1;
- }
+ if (err < 64 && bind_conf->crt_ignerr & (1ULL << err))
+ goto err_ignored;
- conn->err_code = CO_ER_SSL_CRT_FAIL;
+ /* TODO: for QUIC connection, this error code is lost */
+ if (conn)
+ conn->err_code = CO_ER_SSL_CRT_FAIL;
return 0;
+
+ err_ignored:
+ ssl_sock_dump_errors(conn, qc);
+ ERR_clear_error();
+ return 1;
}
#ifdef TLS1_RT_HEARTBEAT
@@ -6229,7 +6262,7 @@
out_error:
/* Clear openssl global errors stack */
- ssl_sock_dump_errors(conn);
+ ssl_sock_dump_errors(conn, NULL);
ERR_clear_error();
/* free resumed session if exists */
@@ -6594,7 +6627,7 @@
clear_ssl_error:
/* Clear openssl global errors stack */
- ssl_sock_dump_errors(conn);
+ ssl_sock_dump_errors(conn, NULL);
ERR_clear_error();
read0:
conn_sock_read0(conn);
@@ -6603,7 +6636,7 @@
out_error:
conn->flags |= CO_FL_ERROR;
/* Clear openssl global errors stack */
- ssl_sock_dump_errors(conn);
+ ssl_sock_dump_errors(conn, NULL);
ERR_clear_error();
goto leave;
}
@@ -6759,7 +6792,7 @@
out_error:
/* Clear openssl global errors stack */
- ssl_sock_dump_errors(conn);
+ ssl_sock_dump_errors(conn, NULL);
ERR_clear_error();
conn->flags |= CO_FL_ERROR;
@@ -6858,7 +6891,7 @@
/* no handshake was in progress, try a clean ssl shutdown */
if (SSL_shutdown(ctx->ssl) <= 0) {
/* Clear openssl global errors stack */
- ssl_sock_dump_errors(conn);
+ ssl_sock_dump_errors(conn, NULL);
ERR_clear_error();
}
}