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.
This commit is contained in:
Frdric Lcaille 2022-09-06 19:37:08 +02:00 committed by Willy Tarreau
parent a9e934bbd1
commit 2be0ac55e1

View File

@ -638,7 +638,8 @@ SSL *ssl_sock_get_ssl_object(struct connection *conn)
* 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 @@ static forceinline void ssl_sock_dump_errors(struct connection *conn)
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 @@ int ssl_sock_bind_verifycbk(int ok, X509_STORE_CTX *x_store)
{
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);
ctx = __conn_get_ssl_sock_ctx(conn);
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->xprt_st |= SSL_SOCK_ST_FL_VERIFY_DONE;
depth = X509_STORE_CTX_get_error_depth(x_store);
@ -1751,13 +1781,12 @@ int ssl_sock_bind_verifycbk(int ok, X509_STORE_CTX *x_store)
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 @@ int ssl_sock_bind_verifycbk(int ok, X509_STORE_CTX *x_store)
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 @@ reneg_ok:
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 @@ static size_t ssl_sock_to_buf(struct connection *conn, void *xprt_ctx, struct bu
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 @@ static size_t ssl_sock_to_buf(struct connection *conn, void *xprt_ctx, struct bu
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 @@ static size_t ssl_sock_from_buf(struct connection *conn, void *xprt_ctx, const s
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 @@ static void ssl_sock_shutw(struct connection *conn, void *xprt_ctx, int clean)
/* 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();
}
}