From: Yeonggi Kim <[email protected]>

When a QUIC listener is bound on a wildcard address, the local address
targeted by the client is retrieved on reception via IP_PKTINFO or
equivalent. It is then used as source address for all datagrams of the
connection, either by binding the connection socket on it, or via
ancillary data when the listener socket is used. Without it, the kernel
selects the source address from the route to the client, which may
differ from the one targeted by the client. This is typically the case
when several addresses are assigned to the host, for example VIPs
configured on a loopback interface behind a load balancer in direct
routing mode. As QUIC clients usually rely on connected UDP sockets,
datagrams from another address are then silently dropped by their
kernel.

However, Retry packets are emitted before any connection exists, via
send_retry() which used a bare sendto() on the listener socket without
source address. In the above conditions, the client never receives the
Retry and keeps retransmitting its Initial packet until it gives up.
Retry is emitted in particular when quic-force-retry is set on the bind
line, when a quic-initial send-retry rule matches, or when the number of
half-open connections reaches tune.quic.fe.sec.retry-threshold, so
handshakes may also fail only intermittently under load. Apart from not
using Retry at all, which removes client address validation, the usual
workaround is to explicitly bind each address, in addition to or instead
of the wildcard one.

Fix this by adding a source address parameter to send_retry(). Its
callers set it to the datagram destination address, only when the
listener is bound on a wildcard address. Emission now relies on
quic_sock_sendto(). Apart from the source address, the emission is
unchanged, except that the MSG_DONTWAIT|MSG_NOSIGNAL flags are now set
and EINTR is retried, as in qc_snd_buf(). Also fix the trace emitted on
send failure, which was a copy of the previous one.

This should fix github issue #3503.

This should be backported up to 2.8. Note that it relies on previous
patch "MINOR: quic: define a function to emit a datagram without
connection". Prior to 2.9, send_retry() is a static function defined in
quic_conn.c.
---
 include/haproxy/quic_tx.h |  1 +
 src/quic_rx.c             |  9 +++++++--
 src/quic_tx.c             | 10 ++++++----
 3 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/include/haproxy/quic_tx.h b/include/haproxy/quic_tx.h
index 60d567a72..e85125470 100644
--- a/include/haproxy/quic_tx.h
+++ b/include/haproxy/quic_tx.h
@@ -48,6 +48,7 @@ int qc_dgrams_retransmit(struct quic_conn *qc);
 void qc_prep_hdshk_fast_retrans(struct quic_conn *qc,
                                 struct list *ifrms, struct list *hfrms);
 int send_retry(int fd, struct sockaddr_storage *addr,
+               struct sockaddr_storage *src,
                struct quic_rx_packet *pkt, const struct quic_version *qv);
 int send_stateless_reset(struct listener *l, struct sockaddr_storage *dstaddr,
                          struct quic_rx_packet *rxpkt);
diff --git a/src/quic_rx.c b/src/quic_rx.c
index 1bfe78fbb..4c665ab6d 100644
--- a/src/quic_rx.c
+++ b/src/quic_rx.c
@@ -1793,6 +1793,7 @@ static struct quic_conn *quic_rx_pkt_retrieve_conn(struct 
quic_rx_packet *pkt,
        struct quic_conn *qc = NULL;
        struct proxy *prx;
        struct quic_counters *prx_counters;
+       struct sockaddr_storage laddr;
 
        TRACE_ENTER(QUIC_EV_CONN_LPKT);
 
@@ -1836,7 +1837,9 @@ static struct quic_conn *quic_rx_pkt_retrieve_conn(struct 
quic_rx_packet *pkt,
                                /* 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, 
(struct sockaddr_storage *)&dgram->saddr, pkt, pkt->version)) {
+                                               if (send_retry(l->rx.fd, 
(struct sockaddr_storage *)&dgram->saddr,
+                                                              
quic_dgram_reply_src(dgram, l, &laddr),
+                                                              pkt, 
pkt->version)) {
                                                        TRACE_ERROR("Error 
during Retry generation",
                                                                    
QUIC_EV_CONN_LPKT, NULL, NULL, NULL, pkt->version);
                                                }
@@ -1866,7 +1869,9 @@ static struct quic_conn *quic_rx_pkt_retrieve_conn(struct 
quic_rx_packet *pkt,
 
                                        TRACE_PROTO("Initial without token, 
sending retry",
                                                    QUIC_EV_CONN_LPKT, NULL, 
NULL, NULL, pkt->version);
-                                       if (send_retry(l->rx.fd, (struct 
sockaddr_storage *)&dgram->saddr, pkt, pkt->version)) {
+                                       if (send_retry(l->rx.fd, (struct 
sockaddr_storage *)&dgram->saddr,
+                                                      
quic_dgram_reply_src(dgram, l, &laddr),
+                                                      pkt, pkt->version)) {
                                                TRACE_ERROR("Error during Retry 
generation",
                                                            QUIC_EV_CONN_LPKT, 
NULL, NULL, NULL, pkt->version);
                                                goto out;
diff --git a/src/quic_tx.c b/src/quic_tx.c
index 43ce8fc57..2023b7fdd 100644
--- a/src/quic_tx.c
+++ b/src/quic_tx.c
@@ -1306,17 +1306,19 @@ static inline int quic_pkt_type(int type, uint32_t 
version)
 
 
 /* Generate a Retry packet and send it on <fd> socket to <addr> in response to
- * the Initial <pkt> packet.
+ * the Initial <pkt> packet. <src> is the local address to use as datagram
+ * source, or NULL to let the kernel select it. It must be NULL when <fd> is
+ * bound on a specific address (see quic_lstnr_may_set_src()).
  *
  * Returns 0 on success else non-zero.
  */
 int send_retry(int fd, struct sockaddr_storage *addr,
+               struct sockaddr_storage *src,
                struct quic_rx_packet *pkt, const struct quic_version *qv)
 {
        int ret = 0;
        unsigned char buf[128];
        int i = 0, token_len;
-       const socklen_t addrlen = get_addr_len(addr);
        struct quic_cid scid;
 
        TRACE_ENTER(QUIC_EV_CONN_TXPKT);
@@ -1365,8 +1367,8 @@ int send_retry(int fd, struct sockaddr_storage *addr,
 
        i += QUIC_TLS_TAG_LEN;
 
-       if (sendto(fd, buf, i, 0, (struct sockaddr *)addr, addrlen) < 0) {
-               TRACE_ERROR("quic_tls_generate_retry_integrity_tag() failed", 
QUIC_EV_CONN_TXPKT);
+       if (quic_sock_sendto(fd, buf, i, addr, src) < 0) {
+               TRACE_ERROR("Retry emission failed", QUIC_EV_CONN_TXPKT);
                goto out;
        }
 
-- 
2.50.1 (Apple Git-155)



Reply via email to