public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
From: David Gibson <david@gibson.dropbear.id.au>
To: passt-dev@passt.top, Stefano Brivio <sbrivio@redhat.com>
Cc: David Gibson <david@gibson.dropbear.id.au>
Subject: [PATCH 3/3] fwd: Rework inconsistencies in translation of inbound flows
Date: Mon, 12 Aug 2024 19:53:55 +1000	[thread overview]
Message-ID: <20240812095355.1721876-4-david@gibson.dropbear.id.au> (raw)
In-Reply-To: <20240812095355.1721876-1-david@gibson.dropbear.id.au>

We usually avoid NAT, but in a few cases we need to apply address
translations.  The current logic for this on inbound flows has some
inconsistencies:

 * For IPv4 (but not IPv6) we translated unspecified source addresses

While an unspecified source probably doesn't make sense, it makes no less
sense in the guest context than it does for the host, it's possible there
could be protocols / flow types where this works.  At the moment, this
should never happen since the protocols will drop the flow before getting
here.  So, don't alter unspecified addresses for either IP version.

 * For IPv6 (but not IPv4) we translated the guest's assigned address

We always translated the guest's observed address, but for the case where
the assigned address different we should be consistent.  Translate the
guest assigned address for IPv4 as well.


The overall point is that we need to translate here if and only if the host
address is one that's not accessible to the guest.  Create some helper
functions to determine this, which will have additional uses later.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 fwd.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 77 insertions(+), 11 deletions(-)

diff --git a/fwd.c b/fwd.c
index dea36f6c..24700998 100644
--- a/fwd.c
+++ b/fwd.c
@@ -170,6 +170,73 @@ static bool is_dns_flow(uint8_t proto, const struct flowside *ini)
 		((ini->fport == 53) || (ini->fport == 853));
 }
 
+/**
+ * fwd_guest_accessible4() - Is IPv4 address guest accessible
+ * @c:		Execution context
+ * @addr:	Host visible IPv4 address
+ *
+ * Return: true if @addr on the host is accessible to the guest without
+ *         translation, false otherwise
+ */
+static bool fwd_guest_accessible4(const struct ctx *c,
+				    const struct in_addr *addr)
+{
+	if (IN4_IS_ADDR_LOOPBACK(addr))
+		return false;
+
+	if (IN4_ARE_ADDR_EQUAL(addr, &c->ip4.addr))
+		return false;
+
+	if (!IN4_IS_ADDR_UNSPECIFIED(&c->ip4.addr_seen) &&
+	    IN4_ARE_ADDR_EQUAL(addr, &c->ip4.addr_seen))
+		return false;
+
+	return true;
+}
+
+/**
+ * fwd_guest_accessible6() - Is IPv6 address guest accessible
+ * @c:		Execution context
+ * @addr:	Host visible IPv6 address
+ *
+ * Return: true if @addr on the host is accessible to the guest without
+ *         translation, false otherwise
+ */
+static bool fwd_guest_accessible6(const struct ctx *c,
+				  const struct in6_addr *addr)
+{
+	if (IN6_IS_ADDR_LOOPBACK(addr))
+		return false;
+
+	if (IN6_ARE_ADDR_EQUAL(addr, &c->ip6.addr))
+		return false;
+
+	if (!IN6_IS_ADDR_UNSPECIFIED(&c->ip6.addr_seen) &&
+	    IN6_ARE_ADDR_EQUAL(addr, &c->ip6.addr_seen))
+		return false;
+
+	return true;
+}
+
+/**
+ * fwd_guest_accessible() - Is IPv[46] address guest accessible
+ * @c:		Execution context
+ * @addr:	Host visible IPv[46] address
+ *
+ * Return: true if @addr on the host is accessible to the guest without
+ *         translation, false otherwise
+ */
+static bool fwd_guest_accessible(const struct ctx *c,
+				 const union inany_addr *addr)
+{
+	const struct in_addr *a4 = inany_v4(addr);
+
+	if (a4)
+		return fwd_guest_accessible4(c, a4);
+
+	return fwd_guest_accessible6(c, &addr->a6);
+}
+
 /**
  * fwd_nat_from_tap() - Determine to forward a flow from the tap interface
  * @c:		Execution context
@@ -307,21 +374,20 @@ uint8_t fwd_nat_from_host(const struct ctx *c, uint8_t proto,
 		return PIF_SPLICE;
 	}
 
-	tgt->faddr = ini->eaddr;
-	tgt->fport = ini->eport;
-
-	if (inany_is_loopback4(&tgt->faddr) ||
-	    inany_is_unspecified4(&tgt->faddr) ||
-	    inany_equals4(&tgt->faddr, &c->ip4.addr_seen)) {
-		tgt->faddr = inany_from_v4(c->ip4.gw);
-	} else if (inany_is_loopback6(&tgt->faddr) ||
-		   inany_equals6(&tgt->faddr, &c->ip6.addr_seen) ||
-		   inany_equals6(&tgt->faddr, &c->ip6.addr)) {
-		if (IN6_IS_ADDR_LINKLOCAL(&c->ip6.gw))
+	/* These host-visible addresses aren't visible to the guest, so need
+	 * translation
+	 */
+	if (!fwd_guest_accessible(c, &ini->eaddr)) {
+		if (inany_v4(&ini->eaddr))
+			tgt->faddr = inany_from_v4(c->ip4.gw);
+		else if (IN6_IS_ADDR_LINKLOCAL(&c->ip6.gw))
 			tgt->faddr.a6 = c->ip6.gw;
 		else
 			tgt->faddr.a6 = c->ip6.addr_ll;
+	} else {
+		tgt->faddr = ini->eaddr;
 	}
+	tgt->fport = ini->eport;
 
 	if (inany_v4(&tgt->faddr)) {
 		tgt->eaddr = inany_from_v4(c->ip4.addr_seen);
-- 
@@ -170,6 +170,73 @@ static bool is_dns_flow(uint8_t proto, const struct flowside *ini)
 		((ini->fport == 53) || (ini->fport == 853));
 }
 
+/**
+ * fwd_guest_accessible4() - Is IPv4 address guest accessible
+ * @c:		Execution context
+ * @addr:	Host visible IPv4 address
+ *
+ * Return: true if @addr on the host is accessible to the guest without
+ *         translation, false otherwise
+ */
+static bool fwd_guest_accessible4(const struct ctx *c,
+				    const struct in_addr *addr)
+{
+	if (IN4_IS_ADDR_LOOPBACK(addr))
+		return false;
+
+	if (IN4_ARE_ADDR_EQUAL(addr, &c->ip4.addr))
+		return false;
+
+	if (!IN4_IS_ADDR_UNSPECIFIED(&c->ip4.addr_seen) &&
+	    IN4_ARE_ADDR_EQUAL(addr, &c->ip4.addr_seen))
+		return false;
+
+	return true;
+}
+
+/**
+ * fwd_guest_accessible6() - Is IPv6 address guest accessible
+ * @c:		Execution context
+ * @addr:	Host visible IPv6 address
+ *
+ * Return: true if @addr on the host is accessible to the guest without
+ *         translation, false otherwise
+ */
+static bool fwd_guest_accessible6(const struct ctx *c,
+				  const struct in6_addr *addr)
+{
+	if (IN6_IS_ADDR_LOOPBACK(addr))
+		return false;
+
+	if (IN6_ARE_ADDR_EQUAL(addr, &c->ip6.addr))
+		return false;
+
+	if (!IN6_IS_ADDR_UNSPECIFIED(&c->ip6.addr_seen) &&
+	    IN6_ARE_ADDR_EQUAL(addr, &c->ip6.addr_seen))
+		return false;
+
+	return true;
+}
+
+/**
+ * fwd_guest_accessible() - Is IPv[46] address guest accessible
+ * @c:		Execution context
+ * @addr:	Host visible IPv[46] address
+ *
+ * Return: true if @addr on the host is accessible to the guest without
+ *         translation, false otherwise
+ */
+static bool fwd_guest_accessible(const struct ctx *c,
+				 const union inany_addr *addr)
+{
+	const struct in_addr *a4 = inany_v4(addr);
+
+	if (a4)
+		return fwd_guest_accessible4(c, a4);
+
+	return fwd_guest_accessible6(c, &addr->a6);
+}
+
 /**
  * fwd_nat_from_tap() - Determine to forward a flow from the tap interface
  * @c:		Execution context
@@ -307,21 +374,20 @@ uint8_t fwd_nat_from_host(const struct ctx *c, uint8_t proto,
 		return PIF_SPLICE;
 	}
 
-	tgt->faddr = ini->eaddr;
-	tgt->fport = ini->eport;
-
-	if (inany_is_loopback4(&tgt->faddr) ||
-	    inany_is_unspecified4(&tgt->faddr) ||
-	    inany_equals4(&tgt->faddr, &c->ip4.addr_seen)) {
-		tgt->faddr = inany_from_v4(c->ip4.gw);
-	} else if (inany_is_loopback6(&tgt->faddr) ||
-		   inany_equals6(&tgt->faddr, &c->ip6.addr_seen) ||
-		   inany_equals6(&tgt->faddr, &c->ip6.addr)) {
-		if (IN6_IS_ADDR_LINKLOCAL(&c->ip6.gw))
+	/* These host-visible addresses aren't visible to the guest, so need
+	 * translation
+	 */
+	if (!fwd_guest_accessible(c, &ini->eaddr)) {
+		if (inany_v4(&ini->eaddr))
+			tgt->faddr = inany_from_v4(c->ip4.gw);
+		else if (IN6_IS_ADDR_LINKLOCAL(&c->ip6.gw))
 			tgt->faddr.a6 = c->ip6.gw;
 		else
 			tgt->faddr.a6 = c->ip6.addr_ll;
+	} else {
+		tgt->faddr = ini->eaddr;
 	}
+	tgt->fport = ini->eport;
 
 	if (inany_v4(&tgt->faddr)) {
 		tgt->eaddr = inany_from_v4(c->ip4.addr_seen);
-- 
2.46.0


  parent reply	other threads:[~2024-08-12  9:54 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-12  9:53 [PATCH 0/3] Assorted fixes related to addressing David Gibson
2024-08-12  9:53 ` [PATCH 1/3] Correct inaccurate comments on ip[46]_ctx::addr David Gibson
2024-08-12  9:53 ` [PATCH 2/3] conf: Delay handling -D option until after addresses are configured David Gibson
2024-08-12  9:53 ` David Gibson [this message]
2024-08-12 21:51   ` [PATCH 3/3] fwd: Rework inconsistencies in translation of inbound flows Stefano Brivio
2024-08-15  5:01     ` David Gibson

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=20240812095355.1721876-4-david@gibson.dropbear.id.au \
    --to=david@gibson.dropbear.id.au \
    --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).