]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
BUG/MEDIUM: quic: always validate sender address on 0-RTT
authorFrederic Lecaille <flecaille@haproxy.com>
Fri, 30 Aug 2024 13:38:54 +0000 (15:38 +0200)
committerFrederic Lecaille <flecaille@haproxy.com>
Fri, 30 Aug 2024 15:04:09 +0000 (17:04 +0200)
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
src/quic_conn.c
src/quic_retry.c
src/quic_rx.c
src/quic_ssl.c

index 8a4ed58be14b9b841b8546ab5a9aa036e4d40303..a85606ffce4255df00b8f7228f2f06fb80de8c4e 100644 (file)
@@ -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) */
index 80401fe276035c9eab120c255a514687604fa9ff..37eaf940b20289b908c25b817230cce70b60d65c 100644 (file)
@@ -57,6 +57,7 @@
 #include <haproxy/quic_sock.h>
 #include <haproxy/quic_stats.h>
 #include <haproxy/quic_stream.h>
+#include <haproxy/quic_token.h>
 #include <haproxy/quic_tp.h>
 #include <haproxy/quic_trace.h>
 #include <haproxy/quic_tx.h>
@@ -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 <max> 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).
  * <dcid> is the destination connection ID, <scid> is the source connection ID.
  * This latter <scid> CID as the same value on the wire as the one for <conn_id>
- * 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
  * <conn_id> in the CIDs tree for this connection (qc->cids).
- * <token> is the token found to be used for this connection with <token_len> as
- * length. Endpoints addresses are specified via <local_addr> and <peer_addr>.
+ * <token> is a boolean denoting if a token was received for this connection
+ * from an Initial packet.
+ * <token_odcid> 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 <local_addr> and <peer_addr>.
  * 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;
index 2d6ea31ac4f714267c3fd274e2a7b42d12ff1b96..78ef88a76900711ea859e7e20b87d0e36534f57f 100644 (file)
@@ -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;
index 6e21958cc38a132933d0e56044858f3ac48b8552..82a4b5d895c7984f0af8504632c9ed6d056b7ca1 100644 (file)
@@ -27,6 +27,7 @@
 #include <haproxy/quic_stream.h>
 #include <haproxy/quic_ssl.h>
 #include <haproxy/quic_tls.h>
+#include <haproxy/quic_token.h>
 #include <haproxy/quic_trace.h>
 #include <haproxy/quic_tx.h>
 #include <haproxy/ssl_sock.h>
@@ -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
+ * <pkt> RX packet from <dgram> 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 <pkt> or create a new one if
  * this is an Initial packet. <dgram> is the datagram containing the packet and
  * <l> 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)) {
index 3d3f8e77b8996c59631488bf4bdfbc6b117f6e32..9dba4212fa843a0c23f2a19fe68dcf26e9c6f541 100644 (file)
@@ -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;
                }