From: Frédéric Lécaille Date: Tue, 6 Sep 2022 17:37:08 +0000 (+0200) Subject: BUG/MINOR: quic: Possible crash when verifying certificates X-Git-Tag: v2.7-dev6~102 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2be0ac55e10aec6ba3eaef4ccd7d6fe3fe10633c;p=thirdparty%2Fhaproxy.git 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 0edad3b082..caf41eb9a0 100644 --- a/src/ssl_sock.c +++ b/src/ssl_sock.c @@ -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 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(); } }