From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id ABBAE5A031A for ; Mon, 12 Aug 2024 11:54:03 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202312; t=1723456436; bh=VBNOK73S1jmbUAbOzpro4T1epBmH/q/QmbKTE93vh10=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=VTa1lD9R1FoNmwGNBK6JlI4MQOFM5SL7Tm8Mo4QmDrsGvVY1Jbp6McmI/Ei2pi7qn QUUJ6/Rcs2iCmDu0rlrPHVXsjI7lii71jhuMEIhGVsEzec1iuAYrIezIwbWJMlVckJ tE4/9xOF0+rzDfsF8/5evlThJYyptYwbGrgpxmPXTe4oxlNxrwy/TX3M7IQe40OA+n 1lPt9jMWW1ib7wXifZyqzZ9ktEkNBtaFakyD8kN5vv1U8RwnD3uPNHQXz8ZpEmJRtU 7OiFLEsdz4JofJPhclPIvu78Dv7OiyP0bYUUz1PPW0SakFAYOu5eBdOrhATRC33p6S P7xT+KBn0nKnw== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4Wj8x05SwCz4xCT; Mon, 12 Aug 2024 19:53:56 +1000 (AEST) From: David Gibson To: passt-dev@passt.top, Stefano Brivio Subject: [PATCH 3/3] fwd: Rework inconsistencies in translation of inbound flows Date: Mon, 12 Aug 2024 19:53:55 +1000 Message-ID: <20240812095355.1721876-4-david@gibson.dropbear.id.au> X-Mailer: git-send-email 2.46.0 In-Reply-To: <20240812095355.1721876-1-david@gibson.dropbear.id.au> References: <20240812095355.1721876-1-david@gibson.dropbear.id.au> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Message-ID-Hash: H2DKT7M5EA3MSCOU5Y3OI5V2JJG3QQZA X-Message-ID-Hash: H2DKT7M5EA3MSCOU5Y3OI5V2JJG3QQZA X-MailFrom: dgibson@gandalf.ozlabs.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: David Gibson X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: 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 --- 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); -- 2.46.0