From 8d2db5d964ea2171b1d1c024de0a18c1f3653097 Mon Sep 17 00:00:00 2001 From: Frederic Lecaille Date: Fri, 30 Aug 2024 15:38:54 +0200 Subject: [PATCH] BUG/MEDIUM: quic: 0-RTT securing It has been reported by Wedl Michael, a student at the University of Applied Sciences St. Poelten, a potential vulnerability into haproxy as described below. An attacker could have obtained a TLS session ticket after having established a connection to an haproxy QUIC listener, using its real IP address. The attacker has not even to send a application level request (HTTP3). Then the attacker could open a 0-RTT session with a spoofed IP address trusted by the QUIC listen to bypass IP allow/block list and send HTTP3 requests. To mitigate this vulnerability, one decided to use a token which can be provided to the client each time it successfully managed to connect to haproxy. These tokens may be reused for future connections to validate the address/path of the remote peer as this is done with the Retry token which is used for the current connection, not the next one. Such tokens are transported by NEW_TOKEN frames which was not used at this time by haproxy. So, each time a client connect to an haproxy QUIC listener with 0-RTT enabled, it is provided with such a token which can be reused for the next 0-RTT session. If no such a token is presented by the client, haproxy checks if the session is a 0-RTT one, so with early-data presented by the client. Contrary to the Retry token, the decision to refuse the connection is made only when the TLS stack has been provided with enough early-data from the Initial ClientHello TLS message and when these data have been accepted. Hopefully, this event arrives fast enough to allow haproxy to kill the connection if some early-data have been accepted without token presented by the client. quic_build_post_handshake_frames() has been modified to build a NEW_TOKEN frame with this newly implemented token to be transported inside. quic_tls_derive_retry_token_secret() was renamed to quic_do_tls_derive_token_secre() and modified to be reused and derive the secret for the new token implementation. quic_token_validate() has been implemented to validate both the Retry and the new token implemented by this patch. When this is a non-retry token which could not be validated, the datagram received is marked as requiring a Retry packet to be sent, and no connection is created. When the Initial packet does not embed any non-retry token and if 0-RTT is enabled the connection is marked with this new flag: QUIC_FL_CONN_NO_TOKEN_RCVD. As soon as the TLS stack detects that some early-data have been provided and accepted by the client, the connection is marked to be killed (QUIC_FL_CONN_TO_KILL) from ha_quic_add_handshake_data(). This is done calling qc_ssl_eary_data_accepted() new function. The secret TLS handshake is interrupted as soon as possible returnin 0 from ha_quic_add_handshake_data(). The connection is also marked as requiring a Retry packet to be sent (QUIC_FL_CONN_SEND_RETRY) from ha_quic_add_handshake_data(). The the handshake I/O handler (quic_conn_io_cb()) knows how to behave: kill the connection after having sent a Retry packet. About TLS stack compatibility, this patch is supported by aws-lc. It is disabled for wolfssl which does not support 0-RTT at this time thanks to HAVE_SSL_0RTT_QUIC. This patch depends on these commits: MINOR: quic: Add trace for QUIC_EV_CONN_IO_CB event. MINOR: quic: Implement qc_ssl_eary_data_accepted(). MINOR: quic: Modify NEW_TOKEN frame structure (qf_new_token struct) BUG/MINOR: quic: Missing incrementation in NEW_TOKEN frame builder MINOR: quic: Token for future connections implementation. MINOR: quic: Implement quic_tls_derive_token_secret(). MINOR: tools: Implement ipaddrcpy(). Must be backported as far as 2.6. --- include/haproxy/quic_conn-t.h | 2 ++ src/quic_conn.c | 65 ++++++++++++++++++++++++++++++++--- src/quic_retry.c | 8 +---- src/quic_rx.c | 62 +++++++++++++++++++++++++++++++-- src/quic_ssl.c | 20 ++++++++++- 5 files changed, 142 insertions(+), 15 deletions(-) diff --git a/include/haproxy/quic_conn-t.h b/include/haproxy/quic_conn-t.h index 8a4ed58be..a85606ffc 100644 --- a/include/haproxy/quic_conn-t.h +++ b/include/haproxy/quic_conn-t.h @@ -445,6 +445,8 @@ struct quic_conn_closed { #define QUIC_FL_CONN_IPKTNS_DCD (1U << 15) /* Initial packet number space discarded */ #define QUIC_FL_CONN_HPKTNS_DCD (1U << 16) /* Handshake packet number space discarded */ #define QUIC_FL_CONN_PEER_VALIDATED_ADDR (1U << 17) /* Peer address is considered as validated for this connection. */ +#define QUIC_FL_CONN_NO_TOKEN_RCVD (1U << 18) /* Client dit not send any token */ +#define QUIC_FL_CONN_SEND_RETRY (1U << 19) /* A send retry packet must be sent */ /* gap here */ #define QUIC_FL_CONN_TO_KILL (1U << 24) /* Unusable connection, to be killed */ #define QUIC_FL_CONN_TX_TP_RECEIVED (1U << 25) /* Peer transport parameters have been received (used for the transmitting part) */ diff --git a/src/quic_conn.c b/src/quic_conn.c index 80401fe27..37eaf940b 100644 --- a/src/quic_conn.c +++ b/src/quic_conn.c @@ -57,6 +57,7 @@ #include #include #include +#include #include #include #include @@ -479,6 +480,30 @@ int quic_build_post_handshake_frames(struct quic_conn *qc) } LIST_APPEND(&frm_list, &frm->list); + +#ifdef HAVE_SSL_0RTT_QUIC + if (qc->li->bind_conf->ssl_conf.early_data) { + size_t new_token_frm_len; + + frm = qc_frm_alloc(QUIC_FT_NEW_TOKEN); + if (!frm) { + TRACE_ERROR("frame allocation error", QUIC_EV_CONN_IO_CB, qc); + goto leave; + } + + new_token_frm_len = + quic_generate_token(frm->new_token.data, + sizeof(frm->new_token.data), &qc->peer_addr); + if (!new_token_frm_len) { + TRACE_ERROR("token generation failed", QUIC_EV_CONN_IO_CB, qc); + goto leave; + } + + BUG_ON(new_token_frm_len != sizeof(frm->new_token.data)); + frm->new_token.len = new_token_frm_len; + LIST_APPEND(&frm_list, &frm->list); + } +#endif } /* Initialize connection IDs minus one: there is @@ -761,6 +786,11 @@ struct task *quic_conn_io_cb(struct task *t, void *context, unsigned int state) HA_ATOMIC_AND(&tl->state, ~TASK_HEAVY); } + if (qc->flags & QUIC_FL_CONN_TO_KILL) { + TRACE_DEVEL("connection to be killed", QUIC_EV_CONN_PHPKTS, qc); + goto out; + } + /* Retranmissions */ if (qc->flags & QUIC_FL_CONN_RETRANS_NEEDED) { TRACE_DEVEL("retransmission needed", QUIC_EV_CONN_PHPKTS, qc); @@ -862,7 +892,25 @@ struct task *quic_conn_io_cb(struct task *t, void *context, unsigned int state) quic_nictx_free(qc); } - if ((qc->flags & QUIC_FL_CONN_CLOSING) && qc->mux_state != QC_MUX_READY) { + if (qc->flags & QUIC_FL_CONN_SEND_RETRY) { + struct quic_counters *prx_counters; + struct proxy *prx = qc->li->bind_conf->frontend; + struct quic_rx_packet pkt = { + .scid = qc->dcid, + .dcid = qc->odcid, + }; + + prx_counters = EXTRA_COUNTERS_GET(prx->extra_counters_fe, &quic_stats_module); + if (send_retry(qc->li->rx.fd, &qc->peer_addr, &pkt, qc->original_version)) { + TRACE_ERROR("Error during Retry generation", + QUIC_EV_CONN_LPKT, NULL, NULL, NULL, qc->original_version); + } + else + HA_ATOMIC_INC(&prx_counters->retry_sent); + } + + if ((qc->flags & (QUIC_FL_CONN_CLOSING|QUIC_FL_CONN_TO_KILL)) && + qc->mux_state != QC_MUX_READY) { quic_conn_release(qc); qc = NULL; } @@ -969,11 +1017,15 @@ struct task *qc_process_timer(struct task *task, void *ctx, unsigned int state) * for QUIC servers (or haproxy listeners). * is the destination connection ID, is the source connection ID. * This latter CID as the same value on the wire as the one for - * which is the first CID of this connection but a different internal representation used to build + * which is the first CID of this connection but a different internal + * representation used to build * NEW_CONNECTION_ID frames. This is the responsibility of the caller to insert * in the CIDs tree for this connection (qc->cids). - * is the token found to be used for this connection with as - * length. Endpoints addresses are specified via and . + * is a boolean denoting if a token was received for this connection + * from an Initial packet. + * is the original destination connection ID which was embedded + * into the Retry token sent to the client before instantiated this connection. + * Endpoints addresses are specified via and . * Returns the connection if succeeded, NULL if not. */ struct quic_conn *qc_new_conn(const struct quic_version *qv, int ipv4, @@ -1080,6 +1132,9 @@ struct quic_conn *qc_new_conn(const struct quic_version *qv, int ipv4, qc->prx_counters = EXTRA_COUNTERS_GET(prx->extra_counters_fe, &quic_stats_module); qc->flags = QUIC_FL_CONN_LISTENER; + /* Mark this connection as having not received any token when 0-RTT is enabled. */ + if (l->bind_conf->ssl_conf.early_data && !token) + qc->flags |= QUIC_FL_CONN_NO_TOKEN_RCVD; qc->state = QUIC_HS_ST_SERVER_INITIAL; /* Copy the client original DCID. */ qc->odcid = *dcid; @@ -1102,7 +1157,7 @@ struct quic_conn *qc_new_conn(const struct quic_version *qv, int ipv4, /* If connection is instantiated due to an INITIAL packet with an * already checked token, consider the peer address as validated. */ - if (token_odcid->len) { + if (token) { TRACE_STATE("validate peer address due to initial token", QUIC_EV_CONN_INIT, qc); qc->flags |= QUIC_FL_CONN_PEER_VALIDATED_ADDR; diff --git a/src/quic_retry.c b/src/quic_retry.c index 2d6ea31ac..78ef88a76 100644 --- a/src/quic_retry.c +++ b/src/quic_retry.c @@ -258,17 +258,11 @@ int quic_retry_token_check(struct quic_rx_packet *pkt, TRACE_ENTER(QUIC_EV_CONN_LPKT, qc); /* The caller must ensure this. */ - BUG_ON(!pkt->token_len); + BUG_ON(!pkt->token_len || *pkt->token != QUIC_TOKEN_FMT_RETRY); prx = l->bind_conf->frontend; prx_counters = EXTRA_COUNTERS_GET(prx->extra_counters_fe, &quic_stats_module); - if (*pkt->token != QUIC_TOKEN_FMT_RETRY) { - /* TODO: New token check */ - TRACE_PROTO("Packet dropped", QUIC_EV_CONN_LPKT, qc, NULL, NULL, pkt->version); - goto leave; - } - if (sizeof buf < tokenlen) { TRACE_ERROR("too short buffer", QUIC_EV_CONN_LPKT, qc); goto err; diff --git a/src/quic_rx.c b/src/quic_rx.c index 6e21958cc..82a4b5d89 100644 --- a/src/quic_rx.c +++ b/src/quic_rx.c @@ -27,6 +27,7 @@ #include #include #include +#include #include #include #include @@ -1522,6 +1523,47 @@ static inline int quic_padding_check(const unsigned char *pos, return pos == end; } +/* Validate the token, retry or not (provided by NEW_TOKEN) parsed into + * RX packet from datagram. + * Return 1 if succeded, 0 if not. + */ +static inline int quic_token_validate(struct quic_rx_packet *pkt, + struct quic_dgram *dgram, + struct listener *l, struct quic_conn *qc, + struct quic_cid *odcid) +{ + int ret = 0; + + TRACE_ENTER(QUIC_EV_CONN_LPKT, qc); + + switch (*pkt->token) { + case QUIC_TOKEN_FMT_RETRY: + ret = quic_retry_token_check(pkt, dgram, l, qc, odcid); + break; + case QUIC_TOKEN_FMT_NEW: + ret = quic_token_check(pkt, dgram, qc); + if (!ret) { + /* Fallback to a retry token in case of any error. */ + dgram->flags |= QUIC_DGRAM_FL_SEND_RETRY; + } + break; + default: + TRACE_PROTO("Packet dropped", QUIC_EV_CONN_LPKT, qc, NULL, NULL, pkt->version); + break; + } + + if (!ret) + goto err; + + ret = 1; + leave: + TRACE_LEAVE(QUIC_EV_CONN_LPKT, qc); + return ret; + err: + TRACE_DEVEL("leaving in error", QUIC_EV_CONN_LPKT, qc); + goto leave; +} + /* Find the associated connection to the packet or create a new one if * this is an Initial packet. is the datagram containing the packet and * is the listener instance on which it was received. @@ -1581,9 +1623,25 @@ static struct quic_conn *quic_rx_pkt_retrieve_conn(struct quic_rx_packet *pkt, } if (pkt->token_len) { - /* Validate the token only when connection is unknown. */ - if (!quic_retry_token_check(pkt, dgram, l, qc, &token_odcid)) + TRACE_PROTO("Initial with token", QUIC_EV_CONN_LPKT, NULL, NULL, NULL, pkt->version); + /* Validate the token, retry or not only when connection is unknown. */ + if (!quic_token_validate(pkt, dgram, l, qc, &token_odcid)) { + if (dgram->flags & QUIC_DGRAM_FL_SEND_RETRY) { + if (send_retry(l->rx.fd, &dgram->saddr, pkt, pkt->version)) { + TRACE_ERROR("Error during Retry generation", + QUIC_EV_CONN_LPKT, NULL, NULL, NULL, pkt->version); + } + else + HA_ATOMIC_INC(&prx_counters->retry_sent); + + goto out; + } + goto err; + } + } + else { + TRACE_PROTO("Initial without token", QUIC_EV_CONN_LPKT, NULL, NULL, NULL, pkt->version); } if (!quic_init_exec_rules(l, dgram)) { diff --git a/src/quic_ssl.c b/src/quic_ssl.c index 3d3f8e77b..9dba4212f 100644 --- a/src/quic_ssl.c +++ b/src/quic_ssl.c @@ -353,6 +353,23 @@ static int ha_quic_add_handshake_data(SSL *ssl, enum ssl_encryption_level_t leve TRACE_ENTER(QUIC_EV_CONN_ADDDATA, qc); + TRACE_PROTO("ha_quic_add_handshake_data() called", QUIC_EV_CONN_IO_CB, qc, NULL, ssl); + +#ifdef HAVE_SSL_0RTT_QUIC + /* Detect asap if some 0-RTT data were accepted for this connection. + * If this is the case and no token was provided, interrupt the useless + * secrets derivations. A Retry packet must be sent, and this connection + * must be killed. + * Note that QUIC_FL_CONN_NO_TOKEN_RCVD is possibly set only for when 0-RTT is + * enabled for the connection. + */ + if ((qc->flags & QUIC_FL_CONN_NO_TOKEN_RCVD) && qc_ssl_eary_data_accepted(ssl)) { + TRACE_PROTO("connection to be killed", QUIC_EV_CONN_ADDDATA, qc); + qc->flags |= QUIC_FL_CONN_TO_KILL|QUIC_FL_CONN_SEND_RETRY; + goto leave; + } +#endif + if (qc->flags & QUIC_FL_CONN_TO_KILL) { TRACE_PROTO("connection to be killed", QUIC_EV_CONN_ADDDATA, qc); goto out; @@ -533,9 +550,10 @@ static int qc_ssl_provide_quic_data(struct ncbuf *ncbuf, state = qc->state; if (state < QUIC_HS_ST_COMPLETE) { ssl_err = SSL_do_handshake(ctx->ssl); + TRACE_PROTO("SSL_do_handshake() called", QUIC_EV_CONN_IO_CB, qc, NULL, ctx->ssl); if (qc->flags & QUIC_FL_CONN_TO_KILL) { - TRACE_DEVEL("connection to be killed", QUIC_EV_CONN_IO_CB, qc); + TRACE_DEVEL("connection to be killed", QUIC_EV_CONN_IO_CB, qc, &state, ctx->ssl); goto leave; }