public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
* [PATCH] udp: Translate source address of resolver only for DNS remapped queries
@ 2024-03-15 14:26 Stefano Brivio
  2024-03-18  3:26 ` David Gibson
  0 siblings, 1 reply; 2+ messages in thread
From: Stefano Brivio @ 2024-03-15 14:26 UTC (permalink / raw)
  To: passt-dev; +Cc: Paul Holzinger, David Gibson

Paul reports that if pasta is configured with --dns-forward, and the
container queries a resolver which is configured on the host directly,
without using the address given for --dns-forward, we'll translate
the source address of the response pretending it's coming from the
address passed as --dns-forward, and the client will discard the
reply.

That is,

  $ cat /etc/resolv.conf
  198.51.100.1
  $ pasta --config-net --dns-forward 192.0.2.1 nslookup passt.top

will not work, because we change the source address of the reply from
198.51.100.1 to 192.0.2.1. But the client contacted 198.51.100.1, and
it's from that address that it expects an answer.

Add a PORT_DNS_FWD flag for tap-facing ports, which is triggered by
activity in the opposite direction as the other flags. If the
tap-facing port was seen sending a DNS query that was remapped, we'll
remap the source address of the response, otherwise we'll leave it
unaffected.

Reported-by: Paul Holzinger <pholzing@redhat.com>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
 udp.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/udp.c b/udp.c
index 0a7f3b7..694424a 100644
--- a/udp.c
+++ b/udp.c
@@ -127,15 +127,16 @@
 /**
  * struct udp_tap_port - Port tracking based on tap-facing source port
  * @sock:	Socket bound to source port used as index
- * @flags:	Flags for local bind, loopback address/unicast address as source
+ * @flags:	Flags for recent activity type seen from/to port
  * @ts:		Activity timestamp from tap, used for socket aging
  */
 struct udp_tap_port {
 	int sock;
 	uint8_t flags;
-#define PORT_LOCAL	BIT(0)
-#define PORT_LOOPBACK	BIT(1)
-#define PORT_GUA	BIT(2)
+#define PORT_LOCAL	BIT(0)	/* Port was contacted from local address */
+#define PORT_LOOPBACK	BIT(1)	/* Port was contacted from loopback address */
+#define PORT_GUA	BIT(2)	/* Port was contacted from global unicast */
+#define PORT_DNS_FWD	BIT(3)	/* Port used as source for DNS remapped query */
 
 	time_t ts;
 };
@@ -579,7 +580,8 @@ static size_t udp_update_hdr4(const struct ctx *c, struct udp4_l2_buf_t *b,
 	struct in_addr src = b->s_in.sin_addr;
 
 	if (!IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_match) &&
-	    IN4_ARE_ADDR_EQUAL(&src, &c->ip4.dns_host) && srcport == 53) {
+	    IN4_ARE_ADDR_EQUAL(&src, &c->ip4.dns_host) && srcport == 53 &&
+	    (udp_tap_map[V4][dstport].flags & PORT_DNS_FWD)) {
 		src = c->ip4.dns_match;
 	} else if (IN4_IS_ADDR_LOOPBACK(&src) ||
 		   IN4_ARE_ADDR_EQUAL(&src, &c->ip4.addr_seen)) {
@@ -632,7 +634,8 @@ static size_t udp_update_hdr6(const struct ctx *c, struct udp6_l2_buf_t *b,
 		dst = &c->ip6.addr_ll_seen;
 	} else if (!IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_match) &&
 		   IN6_ARE_ADDR_EQUAL(src, &c->ip6.dns_host) &&
-		   srcport == 53) {
+		   srcport == 53 &&
+		   (udp_tap_map[V4][dstport].flags & PORT_DNS_FWD)) {
 		src = &c->ip6.dns_match;
 	} else if (IN6_IS_ADDR_LOOPBACK(src)			||
 		   IN6_ARE_ADDR_EQUAL(src, &c->ip6.addr_seen)	||
@@ -841,6 +844,9 @@ int udp_tap_handler(struct ctx *c, uint8_t pif,
 		if (IN4_ARE_ADDR_EQUAL(&s_in.sin_addr, &c->ip4.dns_match) &&
 		    ntohs(s_in.sin_port) == 53) {
 			s_in.sin_addr = c->ip4.dns_host;
+			udp_tap_map[V4][src].ts = now->tv_sec;
+			udp_tap_map[V4][src].flags |= PORT_DNS_FWD;
+			bitmap_set(udp_act[V4][UDP_ACT_TAP], src);
 		} else if (IN4_ARE_ADDR_EQUAL(&s_in.sin_addr, &c->ip4.gw) &&
 			   !c->no_map_gw) {
 			if (!(udp_tap_map[V4][dst].flags & PORT_LOCAL) ||
@@ -890,6 +896,9 @@ int udp_tap_handler(struct ctx *c, uint8_t pif,
 		if (IN6_ARE_ADDR_EQUAL(daddr, &c->ip6.dns_match) &&
 		    ntohs(s_in6.sin6_port) == 53) {
 			s_in6.sin6_addr = c->ip6.dns_host;
+			udp_tap_map[V6][src].ts = now->tv_sec;
+			udp_tap_map[V6][src].flags |= PORT_DNS_FWD;
+			bitmap_set(udp_act[V6][UDP_ACT_TAP], src);
 		} else if (IN6_ARE_ADDR_EQUAL(daddr, &c->ip6.gw) &&
 			   !c->no_map_gw) {
 			if (!(udp_tap_map[V6][dst].flags & PORT_LOCAL) ||
-- 
@@ -127,15 +127,16 @@
 /**
  * struct udp_tap_port - Port tracking based on tap-facing source port
  * @sock:	Socket bound to source port used as index
- * @flags:	Flags for local bind, loopback address/unicast address as source
+ * @flags:	Flags for recent activity type seen from/to port
  * @ts:		Activity timestamp from tap, used for socket aging
  */
 struct udp_tap_port {
 	int sock;
 	uint8_t flags;
-#define PORT_LOCAL	BIT(0)
-#define PORT_LOOPBACK	BIT(1)
-#define PORT_GUA	BIT(2)
+#define PORT_LOCAL	BIT(0)	/* Port was contacted from local address */
+#define PORT_LOOPBACK	BIT(1)	/* Port was contacted from loopback address */
+#define PORT_GUA	BIT(2)	/* Port was contacted from global unicast */
+#define PORT_DNS_FWD	BIT(3)	/* Port used as source for DNS remapped query */
 
 	time_t ts;
 };
@@ -579,7 +580,8 @@ static size_t udp_update_hdr4(const struct ctx *c, struct udp4_l2_buf_t *b,
 	struct in_addr src = b->s_in.sin_addr;
 
 	if (!IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_match) &&
-	    IN4_ARE_ADDR_EQUAL(&src, &c->ip4.dns_host) && srcport == 53) {
+	    IN4_ARE_ADDR_EQUAL(&src, &c->ip4.dns_host) && srcport == 53 &&
+	    (udp_tap_map[V4][dstport].flags & PORT_DNS_FWD)) {
 		src = c->ip4.dns_match;
 	} else if (IN4_IS_ADDR_LOOPBACK(&src) ||
 		   IN4_ARE_ADDR_EQUAL(&src, &c->ip4.addr_seen)) {
@@ -632,7 +634,8 @@ static size_t udp_update_hdr6(const struct ctx *c, struct udp6_l2_buf_t *b,
 		dst = &c->ip6.addr_ll_seen;
 	} else if (!IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_match) &&
 		   IN6_ARE_ADDR_EQUAL(src, &c->ip6.dns_host) &&
-		   srcport == 53) {
+		   srcport == 53 &&
+		   (udp_tap_map[V4][dstport].flags & PORT_DNS_FWD)) {
 		src = &c->ip6.dns_match;
 	} else if (IN6_IS_ADDR_LOOPBACK(src)			||
 		   IN6_ARE_ADDR_EQUAL(src, &c->ip6.addr_seen)	||
@@ -841,6 +844,9 @@ int udp_tap_handler(struct ctx *c, uint8_t pif,
 		if (IN4_ARE_ADDR_EQUAL(&s_in.sin_addr, &c->ip4.dns_match) &&
 		    ntohs(s_in.sin_port) == 53) {
 			s_in.sin_addr = c->ip4.dns_host;
+			udp_tap_map[V4][src].ts = now->tv_sec;
+			udp_tap_map[V4][src].flags |= PORT_DNS_FWD;
+			bitmap_set(udp_act[V4][UDP_ACT_TAP], src);
 		} else if (IN4_ARE_ADDR_EQUAL(&s_in.sin_addr, &c->ip4.gw) &&
 			   !c->no_map_gw) {
 			if (!(udp_tap_map[V4][dst].flags & PORT_LOCAL) ||
@@ -890,6 +896,9 @@ int udp_tap_handler(struct ctx *c, uint8_t pif,
 		if (IN6_ARE_ADDR_EQUAL(daddr, &c->ip6.dns_match) &&
 		    ntohs(s_in6.sin6_port) == 53) {
 			s_in6.sin6_addr = c->ip6.dns_host;
+			udp_tap_map[V6][src].ts = now->tv_sec;
+			udp_tap_map[V6][src].flags |= PORT_DNS_FWD;
+			bitmap_set(udp_act[V6][UDP_ACT_TAP], src);
 		} else if (IN6_ARE_ADDR_EQUAL(daddr, &c->ip6.gw) &&
 			   !c->no_map_gw) {
 			if (!(udp_tap_map[V6][dst].flags & PORT_LOCAL) ||
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] udp: Translate source address of resolver only for DNS remapped queries
  2024-03-15 14:26 [PATCH] udp: Translate source address of resolver only for DNS remapped queries Stefano Brivio
@ 2024-03-18  3:26 ` David Gibson
  0 siblings, 0 replies; 2+ messages in thread
From: David Gibson @ 2024-03-18  3:26 UTC (permalink / raw)
  To: Stefano Brivio; +Cc: passt-dev, Paul Holzinger

[-- Attachment #1: Type: text/plain, Size: 4681 bytes --]

On Fri, Mar 15, 2024 at 03:26:37PM +0100, Stefano Brivio wrote:
> Paul reports that if pasta is configured with --dns-forward, and the
> container queries a resolver which is configured on the host directly,
> without using the address given for --dns-forward, we'll translate
> the source address of the response pretending it's coming from the
> address passed as --dns-forward, and the client will discard the
> reply.
> 
> That is,
> 
>   $ cat /etc/resolv.conf
>   198.51.100.1
>   $ pasta --config-net --dns-forward 192.0.2.1 nslookup passt.top
> 
> will not work, because we change the source address of the reply from
> 198.51.100.1 to 192.0.2.1. But the client contacted 198.51.100.1, and
> it's from that address that it expects an answer.
> 
> Add a PORT_DNS_FWD flag for tap-facing ports, which is triggered by
> activity in the opposite direction as the other flags. If the
> tap-facing port was seen sending a DNS query that was remapped, we'll
> remap the source address of the response, otherwise we'll leave it
> unaffected.
> 
> Reported-by: Paul Holzinger <pholzing@redhat.com>
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

Blech.  Ok, I've really got to do my flow table implementation to
avoid more of this fragile nonsense.

I guess as a stop gap.

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  udp.c | 21 +++++++++++++++------
>  1 file changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/udp.c b/udp.c
> index 0a7f3b7..694424a 100644
> --- a/udp.c
> +++ b/udp.c
> @@ -127,15 +127,16 @@
>  /**
>   * struct udp_tap_port - Port tracking based on tap-facing source port
>   * @sock:	Socket bound to source port used as index
> - * @flags:	Flags for local bind, loopback address/unicast address as source
> + * @flags:	Flags for recent activity type seen from/to port
>   * @ts:		Activity timestamp from tap, used for socket aging
>   */
>  struct udp_tap_port {
>  	int sock;
>  	uint8_t flags;
> -#define PORT_LOCAL	BIT(0)
> -#define PORT_LOOPBACK	BIT(1)
> -#define PORT_GUA	BIT(2)
> +#define PORT_LOCAL	BIT(0)	/* Port was contacted from local address */
> +#define PORT_LOOPBACK	BIT(1)	/* Port was contacted from loopback address */
> +#define PORT_GUA	BIT(2)	/* Port was contacted from global unicast */
> +#define PORT_DNS_FWD	BIT(3)	/* Port used as source for DNS remapped query */
>  
>  	time_t ts;
>  };
> @@ -579,7 +580,8 @@ static size_t udp_update_hdr4(const struct ctx *c, struct udp4_l2_buf_t *b,
>  	struct in_addr src = b->s_in.sin_addr;
>  
>  	if (!IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns_match) &&
> -	    IN4_ARE_ADDR_EQUAL(&src, &c->ip4.dns_host) && srcport == 53) {
> +	    IN4_ARE_ADDR_EQUAL(&src, &c->ip4.dns_host) && srcport == 53 &&
> +	    (udp_tap_map[V4][dstport].flags & PORT_DNS_FWD)) {
>  		src = c->ip4.dns_match;
>  	} else if (IN4_IS_ADDR_LOOPBACK(&src) ||
>  		   IN4_ARE_ADDR_EQUAL(&src, &c->ip4.addr_seen)) {
> @@ -632,7 +634,8 @@ static size_t udp_update_hdr6(const struct ctx *c, struct udp6_l2_buf_t *b,
>  		dst = &c->ip6.addr_ll_seen;
>  	} else if (!IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns_match) &&
>  		   IN6_ARE_ADDR_EQUAL(src, &c->ip6.dns_host) &&
> -		   srcport == 53) {
> +		   srcport == 53 &&
> +		   (udp_tap_map[V4][dstport].flags & PORT_DNS_FWD)) {
>  		src = &c->ip6.dns_match;
>  	} else if (IN6_IS_ADDR_LOOPBACK(src)			||
>  		   IN6_ARE_ADDR_EQUAL(src, &c->ip6.addr_seen)	||
> @@ -841,6 +844,9 @@ int udp_tap_handler(struct ctx *c, uint8_t pif,
>  		if (IN4_ARE_ADDR_EQUAL(&s_in.sin_addr, &c->ip4.dns_match) &&
>  		    ntohs(s_in.sin_port) == 53) {
>  			s_in.sin_addr = c->ip4.dns_host;
> +			udp_tap_map[V4][src].ts = now->tv_sec;
> +			udp_tap_map[V4][src].flags |= PORT_DNS_FWD;
> +			bitmap_set(udp_act[V4][UDP_ACT_TAP], src);
>  		} else if (IN4_ARE_ADDR_EQUAL(&s_in.sin_addr, &c->ip4.gw) &&
>  			   !c->no_map_gw) {
>  			if (!(udp_tap_map[V4][dst].flags & PORT_LOCAL) ||
> @@ -890,6 +896,9 @@ int udp_tap_handler(struct ctx *c, uint8_t pif,
>  		if (IN6_ARE_ADDR_EQUAL(daddr, &c->ip6.dns_match) &&
>  		    ntohs(s_in6.sin6_port) == 53) {
>  			s_in6.sin6_addr = c->ip6.dns_host;
> +			udp_tap_map[V6][src].ts = now->tv_sec;
> +			udp_tap_map[V6][src].flags |= PORT_DNS_FWD;
> +			bitmap_set(udp_act[V6][UDP_ACT_TAP], src);
>  		} else if (IN6_ARE_ADDR_EQUAL(daddr, &c->ip6.gw) &&
>  			   !c->no_map_gw) {
>  			if (!(udp_tap_map[V6][dst].flags & PORT_LOCAL) ||

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2024-03-18  3:28 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-15 14:26 [PATCH] udp: Translate source address of resolver only for DNS remapped queries Stefano Brivio
2024-03-18  3:26 ` David Gibson

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).