From: David Gibson <david@gibson.dropbear.id.au>
To: Stefano Brivio <sbrivio@redhat.com>,
Jon Maloy <jmaloy@redhat.com>,
passt-dev@passt.top
Cc: David Gibson <david@gibson.dropbear.id.au>
Subject: [PATCH 4/4] udp: Translate offender addresses for ICMP messages
Date: Wed, 16 Apr 2025 19:07:07 +1000 [thread overview]
Message-ID: <20250416090707.393497-5-david@gibson.dropbear.id.au> (raw)
In-Reply-To: <20250416090707.393497-1-david@gibson.dropbear.id.au>
We've recently added support for propagating ICMP errors related to a UDP
flow from the host to the guest, by handling the extended UDP error on the
socket and synthesizing a suitable ICMP on the tap interface.
Currently we create that ICMP with a source address of the "offender" from
the extended error information - the source of the ICMP error received on
the host. However, we don't translate this address for cases where we NAT
between host and guest. This means (amongst other things) that we won't
get a "Connection refused" error as expected if send data from the guest to
the --map-host-loopback address. The error comes from 127.0.0.1 on the
host, which doesn't make sense on the tap interface and will be discarded
by the guest.
Because ICMP errors can be sent by an intermediate host, not just by the
endpoints of the flow, we can't handle this translation purely with the
information in the flow table entry. We need to explicitly translate this
address by our NAT rules, which we can do with the nat_inbound() helper.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
fwd.c | 4 ++--
fwd.h | 3 +++
udp.c | 18 ++++++++++++++----
3 files changed, 19 insertions(+), 6 deletions(-)
diff --git a/fwd.c b/fwd.c
index 5c70e834..b73c2c8d 100644
--- a/fwd.c
+++ b/fwd.c
@@ -450,8 +450,8 @@ uint8_t fwd_nat_from_splice(const struct ctx *c, uint8_t proto,
* Only handles translations that depend *only* on the address. Anything
* related to specific ports or flows is handled elsewhere.
*/
-static bool nat_inbound(const struct ctx *c, const union inany_addr *addr,
- union inany_addr *translated)
+bool nat_inbound(const struct ctx *c, const union inany_addr *addr,
+ union inany_addr *translated)
{
if (!IN4_IS_ADDR_UNSPECIFIED(&c->ip4.map_host_loopback) &&
inany_equals4(addr, &in4addr_loopback)) {
diff --git a/fwd.h b/fwd.h
index 3562f3ca..0458a3c5 100644
--- a/fwd.h
+++ b/fwd.h
@@ -7,6 +7,7 @@
#ifndef FWD_H
#define FWD_H
+union inany_addr;
struct flowside;
/* Number of ports for both TCP and UDP */
@@ -47,6 +48,8 @@ void fwd_scan_ports_udp(struct fwd_ports *fwd, const struct fwd_ports *rev,
const struct fwd_ports *tcp_rev);
void fwd_scan_ports_init(struct ctx *c);
+bool nat_inbound(const struct ctx *c, const union inany_addr *addr,
+ union inany_addr *translated);
uint8_t fwd_nat_from_tap(const struct ctx *c, uint8_t proto,
const struct flowside *ini, struct flowside *tgt);
uint8_t fwd_nat_from_splice(const struct ctx *c, uint8_t proto,
diff --git a/udp.c b/udp.c
index 4352520e..ebee1d14 100644
--- a/udp.c
+++ b/udp.c
@@ -539,10 +539,10 @@ static int udp_sock_recverr(const struct ctx *c, int s, flow_sidx_t sidx,
.msg_control = buf,
.msg_controllen = sizeof(buf),
};
- const struct flowside *toside;
+ const struct flowside *fromside, *toside;
+ union inany_addr offender, otap;
char astr[INANY_ADDRSTRLEN];
char sastr[SOCKADDR_STRLEN];
- union inany_addr offender;
const struct in_addr *o4;
in_port_t offender_port;
uint8_t topif;
@@ -599,6 +599,7 @@ static int udp_sock_recverr(const struct ctx *c, int s, flow_sidx_t sidx,
pif = pif_at_sidx(sidx);
}
+ fromside = flowside_at_sidx(sidx);
toside = flowside_at_sidx(flow_sidx_opposite(sidx));
topif = pif_at_sidx(flow_sidx_opposite(sidx));
dlen = rc;
@@ -611,15 +612,24 @@ static int udp_sock_recverr(const struct ctx *c, int s, flow_sidx_t sidx,
/* XXX Can we support any other cases? */
goto fail;
+ /* If the offender *is* the endpoint, make sure our translation is
+ * consistent with the flow's translation. This matters if the flow
+ * endpoint has a port specific translation (like --dns-match).
+ */
+ if (inany_equals(&offender, &fromside->eaddr))
+ otap = toside->oaddr;
+ else if (!nat_inbound(c, &offender, &otap))
+ goto fail;
+
if (hdr->cmsg_level == IPPROTO_IP &&
- (o4 = inany_v4(&offender)) && inany_v4(&toside->eaddr)) {
+ (o4 = inany_v4(&otap)) && inany_v4(&toside->eaddr)) {
dlen = MIN(dlen, ICMP4_MAX_DLEN);
udp_send_tap_icmp4(c, ee, toside, *o4, data, dlen);
return 1;
}
if (hdr->cmsg_level == IPPROTO_IPV6 && !inany_v4(&toside->eaddr)) {
- udp_send_tap_icmp6(c, ee, toside, &offender.a6, data, dlen,
+ udp_send_tap_icmp6(c, ee, toside, &otap.a6, data, dlen,
sidx.flowi);
return 1;
}
--
@@ -539,10 +539,10 @@ static int udp_sock_recverr(const struct ctx *c, int s, flow_sidx_t sidx,
.msg_control = buf,
.msg_controllen = sizeof(buf),
};
- const struct flowside *toside;
+ const struct flowside *fromside, *toside;
+ union inany_addr offender, otap;
char astr[INANY_ADDRSTRLEN];
char sastr[SOCKADDR_STRLEN];
- union inany_addr offender;
const struct in_addr *o4;
in_port_t offender_port;
uint8_t topif;
@@ -599,6 +599,7 @@ static int udp_sock_recverr(const struct ctx *c, int s, flow_sidx_t sidx,
pif = pif_at_sidx(sidx);
}
+ fromside = flowside_at_sidx(sidx);
toside = flowside_at_sidx(flow_sidx_opposite(sidx));
topif = pif_at_sidx(flow_sidx_opposite(sidx));
dlen = rc;
@@ -611,15 +612,24 @@ static int udp_sock_recverr(const struct ctx *c, int s, flow_sidx_t sidx,
/* XXX Can we support any other cases? */
goto fail;
+ /* If the offender *is* the endpoint, make sure our translation is
+ * consistent with the flow's translation. This matters if the flow
+ * endpoint has a port specific translation (like --dns-match).
+ */
+ if (inany_equals(&offender, &fromside->eaddr))
+ otap = toside->oaddr;
+ else if (!nat_inbound(c, &offender, &otap))
+ goto fail;
+
if (hdr->cmsg_level == IPPROTO_IP &&
- (o4 = inany_v4(&offender)) && inany_v4(&toside->eaddr)) {
+ (o4 = inany_v4(&otap)) && inany_v4(&toside->eaddr)) {
dlen = MIN(dlen, ICMP4_MAX_DLEN);
udp_send_tap_icmp4(c, ee, toside, *o4, data, dlen);
return 1;
}
if (hdr->cmsg_level == IPPROTO_IPV6 && !inany_v4(&toside->eaddr)) {
- udp_send_tap_icmp6(c, ee, toside, &offender.a6, data, dlen,
+ udp_send_tap_icmp6(c, ee, toside, &otap.a6, data, dlen,
sidx.flowi);
return 1;
}
--
2.49.0
next prev parent reply other threads:[~2025-04-16 9:07 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-04-16 9:07 [PATCH 0/4] Translate source addresses for ICMP errors David Gibson
2025-04-16 9:07 ` [PATCH 1/4] fwd: Split out helpers for port-independent NAT David Gibson
2025-04-16 9:07 ` [PATCH 2/4] treewide: Improve robustness against sockaddrs of unexpected family David Gibson
2025-04-16 9:41 ` Stefano Brivio
2025-04-17 1:14 ` David Gibson
2025-04-16 9:07 ` [PATCH 3/4] udp: Rework offender address handling in udp_sock_recverr() David Gibson
2025-04-16 14:27 ` Stefano Brivio
2025-04-17 1:33 ` David Gibson
2025-04-16 9:07 ` David Gibson [this message]
2025-04-16 14:27 ` [PATCH 0/4] Translate source addresses for ICMP errors Stefano Brivio
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250416090707.393497-5-david@gibson.dropbear.id.au \
--to=david@gibson.dropbear.id.au \
--cc=jmaloy@redhat.com \
--cc=passt-dev@passt.top \
--cc=sbrivio@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://passt.top/passt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).