mirrors/haproxy
1
0
Fork 0
mirror of https://git.haproxy.org/git/haproxy.git/ synced 2025-11-30 07:11:04 +01:00
Code Issues Projects Releases Wiki Activity

BUG/MINOR: quic-be: handshake errors without connection stream closure

Browse Source 
This bug was revealed on backend side by reg-tests/ssl/del_ssl_crt-list.vtc when
run wich QUIC connections. As expected by the test, a TLS alert is generated on
servsr side. This latter sands a CONNECTION_CLOSE frame with a CRYPTO error
(>= 0x100). In this case the client closes its QUIC connection. But
the stream connection was not informed. This leads the connection to
be closed after the server timeout expiration. It shouls be closed asap.
This is the reason why reg-tests/ssl/del_ssl_crt-list.vtc could succeeds
or failed, but only after a 5 seconds delay.

To fix this, mimic the ssl_sock_io_cb() for TCP/SSL connections. Call
the same code this patch implements with ssl_sock_handle_hs_error()
to correctly handle the handshake errors. Note that some SSL counters
were not incremented for both the backends and frontends. After such
errors, ssl_sock_io_cb() start the mux after the connection has been
flagged in error. This has as side effect to close the stream
in conn_create_mux().

Must be backported to 3.3 only for backends. This is not sure at this time
if this bug may impact the frontends.
This commit is contained in:
Frederic Lecaille 2025-11-28 11:27:54 +01:00
parent dc13068eb4
commit 6c33c6d262
3 changed files with 79 additions and 50 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
include/haproxy/ssl_sock.h
View File

@ -30,6 +30,7 @@
#include <haproxy/proxy-t.h>
#include <haproxy/quic_conn-t.h>
#include <haproxy/ssl_sock-t.h>
#include <haproxy/stats.h>
#include <haproxy/thread.h>
extern struct list tlskeys_reference;
@ -89,6 +90,7 @@ unsigned int ssl_sock_get_verify_result(struct connection *conn);
void ssl_sock_update_counters(SSL *ssl,
struct ssl_counters *counters,
struct ssl_counters *counters_px, int backend);
void ssl_sock_handle_hs_error(struct connection *conn);
#if (defined SSL_CTRL_SET_TLSEXT_TICKET_KEY_CB && TLS_TICKETS_NO > 0)
int ssl_sock_update_tlskey_ref(struct tls_keys_ref *ref,
struct buffer *tlskey);
@ -241,6 +243,30 @@ static inline struct connection *ssl_sock_get_conn(const SSL *s, struct ssl_sock
return ret;
}
/* Set at <counters> and <counters_px> addresses the SSL statistical counters */
static inline void ssl_sock_get_stats_counters(struct connection *conn,
struct ssl_counters **counters,
struct ssl_counters **counters_px)
{
switch (obj_type(conn->target)) {
case OBJ_TYPE_LISTENER: {
struct listener *li = __objt_listener(conn->target);
*counters = EXTRA_COUNTERS_GET(li->extra_counters, &ssl_stats_module);
*counters_px = EXTRA_COUNTERS_GET(li->bind_conf->frontend->extra_counters_fe,
&ssl_stats_module);
break;
}
case OBJ_TYPE_SERVER: {
struct server *srv = __objt_server(conn->target);
*counters = EXTRA_COUNTERS_GET(srv->extra_counters, &ssl_stats_module);
*counters_px = EXTRA_COUNTERS_GET(srv->proxy->extra_counters_be,
&ssl_stats_module);
break;
}
default:
break;
}
}
#endif /* USE_OPENSSL */
#endif /* _HAPROXY_SSL_SOCK_H */

10
src/quic_rx.c
View File

@ -1099,6 +1099,16 @@ static int qc_parse_pkt_frms(struct quic_conn *qc, struct quic_rx_packet *pkt,
/* TODO */
break;
case QUIC_FT_CONNECTION_CLOSE:
/* Nothing to do for non crypto errors */
if ((frm->connection_close.error_code & QC_ERR_CRYPTO_ERROR) && qc->conn) {
ssl_sock_handle_hs_error(qc->conn);
if (objt_server(qc->conn->target) && !qc->conn->mux) {
/* This has as side effect to close the connection stream */
if (conn_create_mux(qc->conn, NULL) >= 0)
qc->conn->mux->wake(qc->conn);
}
}
__fallthrough;
case QUIC_FT_CONNECTION_CLOSE_APP:
/* Increment the error counters */
quic_conn_closed_err_count_inc(qc, frm);

93
src/ssl_sock.c
View File

@ -5963,6 +5963,45 @@ void ssl_sock_update_counters(SSL *ssl,
}
}
/* Handle the handshake error for <conn> connection.
* Also used by QUIC.
*/
void ssl_sock_handle_hs_error(struct connection *conn)
{
struct ssl_counters *counters = NULL;
struct ssl_counters *counters_px = NULL;
/* get counters */
ssl_sock_get_stats_counters(conn, &counters, &counters_px);
/* free resumed session if exists */
if (objt_server(conn->target)) {
struct server *s = __objt_server(conn->target);
/* RWLOCK: only rdlock the SSL cache even when writing in it because there is
* one cache per thread, it only prevents to flush it from the CLI in
* another thread */
HA_RWLOCK_RDLOCK(SSL_SERVER_LOCK, &s->ssl_ctx.lock);
if (s->ssl_ctx.reused_sess[tid].ptr)
ha_free(&s->ssl_ctx.reused_sess[tid].ptr);
HA_RWLOCK_RDUNLOCK(SSL_SERVER_LOCK, &s->ssl_ctx.lock);
}
if (counters) {
HA_ATOMIC_INC(&counters->failed_handshake);
HA_ATOMIC_INC(&counters_px->failed_handshake);
}
/* Report an HS error only on SSL error */
if (!(conn->flags & (CO_FL_ERROR | CO_FL_SOCK_RD_SH | CO_FL_SOCK_WR_SH)))
conn_report_term_evt(conn, tevt_loc_hs, hs_tevt_type_truncated_rcv_err);
/* Fail on all other handshake errors */
conn->flags |= CO_FL_ERROR;
if (!conn->err_code)
conn->err_code = CO_ER_SSL_HANDSHAKE;
}
/* This is the callback which is used when an SSL handshake is pending. It
* updates the FD status if it wants some polling before being called again.
* It returns 0 if it fails in a fatal way or needs to poll to go further,
@ -5975,8 +6014,6 @@ static int ssl_sock_handshake(struct connection *conn, unsigned int flag)
int ret;
struct ssl_counters *counters = NULL;
struct ssl_counters *counters_px = NULL;
struct listener *li;
struct server *srv = NULL;
socklen_t lskerr;
int skerr;
@ -5985,26 +6022,6 @@ static int ssl_sock_handshake(struct connection *conn, unsigned int flag)
if (!conn_ctrl_ready(conn))
return 0;
/* get counters */
switch (obj_type(conn->target)) {
case OBJ_TYPE_LISTENER:
li = __objt_listener(conn->target);
counters = EXTRA_COUNTERS_GET(li->extra_counters, &ssl_stats_module);
counters_px = EXTRA_COUNTERS_GET(li->bind_conf->frontend->extra_counters_fe,
&ssl_stats_module);
break;
case OBJ_TYPE_SERVER:
srv = __objt_server(conn->target);
counters = EXTRA_COUNTERS_GET(srv->extra_counters, &ssl_stats_module);
counters_px = EXTRA_COUNTERS_GET(srv->proxy->extra_counters_be,
&ssl_stats_module);
break;
default:
break;
}
if (!ctx)
goto out_error;
@ -6294,8 +6311,10 @@ reneg_ok:
if (global_ssl.async)
SSL_clear_mode(ctx->ssl, SSL_MODE_ASYNC);
#endif
ssl_sock_get_stats_counters(conn, &counters, &counters_px);
/* Handshake succeeded */
ssl_sock_update_counters(ctx->ssl, counters, counters_px, !!srv);
ssl_sock_update_counters(ctx->ssl, counters, counters_px,
!!objt_server(conn->target));
TRACE_LEAVE(SSL_EV_CONN_HNDSHK, conn, ctx->ssl);
@ -6307,33 +6326,7 @@ reneg_ok:
/* Clear openssl global errors stack */
ssl_sock_dump_errors(conn, NULL);
ERR_clear_error();
/* free resumed session if exists */
if (objt_server(conn->target)) {
struct server *s = __objt_server(conn->target);
/* RWLOCK: only rdlock the SSL cache even when writing in it because there is
* one cache per thread, it only prevents to flush it from the CLI in
* another thread */
HA_RWLOCK_RDLOCK(SSL_SERVER_LOCK, &s->ssl_ctx.lock);
if (s->ssl_ctx.reused_sess[tid].ptr)
ha_free(&s->ssl_ctx.reused_sess[tid].ptr);
HA_RWLOCK_RDUNLOCK(SSL_SERVER_LOCK, &s->ssl_ctx.lock);
}
if (counters) {
HA_ATOMIC_INC(&counters->failed_handshake);
HA_ATOMIC_INC(&counters_px->failed_handshake);
}
/* Report an HS error only on SSL error */
if (!(conn->flags & (CO_FL_ERROR | CO_FL_SOCK_RD_SH | CO_FL_SOCK_WR_SH)))
conn_report_term_evt(conn, tevt_loc_hs, hs_tevt_type_truncated_rcv_err);
/* Fail on all other handshake errors */
conn->flags |= CO_FL_ERROR;
if (!conn->err_code)
conn->err_code = CO_ER_SSL_HANDSHAKE;
ssl_sock_handle_hs_error(conn);
TRACE_ERROR("handshake error", SSL_EV_CONN_HNDSHK|SSL_EV_CONN_ERR, conn, (ctx ? ctx->ssl : NULL), &conn->err_code, (ctx ? &ctx->error_code : NULL));
return 0;