[PATCH v2 0/2] Broaden DNS forwarding

newer
[PATCH] test: Fix spurious test...

David Gibson

24 Jul 2024 24 Jul '24

9:51 a.m.

When looking through outstanding bugs in various trackers, I noticed this one: https://github.com/containers/podman/issues/23239 Now that the flow table is merged, this is very easy to fix, so, here's a fix. While we're at it, handle encrypted DNS on port 853 as well, which Red Hat certainly seems to be interested in for one. Changes since v1: * Fix some stylistic errors in 1/2 * Update man page to reflect new behaviour in 2/2 David Gibson (2): fwd: Refactor tests in fwd_nat_from_tap() for clarity fwd: Broaden what we consider for DNS specific forwarding rules fwd.c | 39 ++++++++++++++++++++++++++------------- passt.1 | 10 +++++----- 2 files changed, 31 insertions(+), 18 deletions(-) -- 2.45.2

Show replies by date

David Gibson

24 Jul 24 Jul

9:51 a.m.

New subject: [PATCH v2 1/2] fwd: Refactor tests in fwd_nat_from_tap() for clarity

Currently, we start by handling the common case, where we don't translate the destination address, then we modify the tgt side for the special cases. In the process we do comparisons on the tentatively set fields in tgt, which obscures the fact that tgt should be an essentially pure function of ini, and risks people examining fields of tgt that are not yet initialized. To make this clearer, do all our tests on 'ini', constructing tgt from scratch on that basis. Signed-off-by: David Gibson --- fwd.c | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/fwd.c b/fwd.c index 8c1f3d91..c323aba7 100644 --- a/fwd.c +++ b/fwd.c @@ -169,21 +169,20 @@ void fwd_scan_ports_init(struct ctx *c) uint8_t fwd_nat_from_tap(const struct ctx *c, uint8_t proto, const struct flowside *ini, struct flowside *tgt) { - tgt->eaddr = ini->faddr; - tgt->eport = ini->fport; - - if (proto == IPPROTO_UDP && tgt->eport == 53 && - inany_equals4(&tgt->eaddr, &c->ip4.dns_match)) { + if (proto == IPPROTO_UDP && ini->fport == 53 && + inany_equals4(&ini->faddr, &c->ip4.dns_match)) tgt->eaddr = inany_from_v4(c->ip4.dns_host); - } else if (proto == IPPROTO_UDP && tgt->eport == 53 && - inany_equals6(&tgt->eaddr, &c->ip6.dns_match)) { + else if (proto == IPPROTO_UDP && ini->fport == 53 && + inany_equals6(&ini->faddr, &c->ip6.dns_match)) tgt->eaddr.a6 = c->ip6.dns_host; - } else if (!c->no_map_gw) { - if (inany_equals4(&tgt->eaddr, &c->ip4.gw)) - tgt->eaddr = inany_loopback4; - else if (inany_equals6(&tgt->eaddr, &c->ip6.gw)) - tgt->eaddr = inany_loopback6; - } + else if (!c->no_map_gw && inany_equals4(&ini->faddr, &c->ip4.gw)) + tgt->eaddr = inany_loopback4; + else if (!c->no_map_gw && inany_equals6(&ini->faddr, &c->ip6.gw)) + tgt->eaddr = inany_loopback6; + else + tgt->eaddr = ini->faddr; + + tgt->eport = ini->fport; /* The relevant addr_out controls the host side source address. This * may be unspecified, which allows the kernel to pick an address. -- 2.45.2

David Gibson

9:51 a.m.

New subject: [PATCH v2 2/2] fwd: Broaden what we consider for DNS specific forwarding rules

passt/pasta has options to redirect DNS requests from the guest to a different server address on the host side. Currently, however, only UDP packets to port 53 are considered "DNS requests". This ignores DNS requests over TCP - less common, but certainly possible. It also ignores encrypted DNS requests on port 853. Extend the DNS forwarding logic to handle both of those cases. Link: https://github.com/containers/podman/issues/23239 Signed-off-by: David Gibson --- fwd.c | 18 ++++++++++++++++-- passt.1 | 10 +++++----- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/fwd.c b/fwd.c index c323aba7..dea36f6c 100644 --- a/fwd.c +++ b/fwd.c @@ -156,6 +156,20 @@ void fwd_scan_ports_init(struct ctx *c) } } +/** + * is_dns_flow() - Determine if flow appears to be a DNS request + * @proto: Protocol (IP L4 protocol number) + * @ini: Flow address information of the initiating side + * + * Return: true if the flow appears to be directed at a dns server, that is a + * TCP or UDP flow to port 53 (domain) or port 853 (domain-s) + */ +static bool is_dns_flow(uint8_t proto, const struct flowside *ini) +{ + return ((proto == IPPROTO_UDP) || (proto == IPPROTO_TCP)) && + ((ini->fport == 53) || (ini->fport == 853)); +} + /** * fwd_nat_from_tap() - Determine to forward a flow from the tap interface * @c: Execution context @@ -169,10 +183,10 @@ void fwd_scan_ports_init(struct ctx *c) uint8_t fwd_nat_from_tap(const struct ctx *c, uint8_t proto, const struct flowside *ini, struct flowside *tgt) { - if (proto == IPPROTO_UDP && ini->fport == 53 && + if (is_dns_flow(proto, ini) && inany_equals4(&ini->faddr, &c->ip4.dns_match)) tgt->eaddr = inany_from_v4(c->ip4.dns_host); - else if (proto == IPPROTO_UDP && ini->fport == 53 && + else if (is_dns_flow(proto, ini) && inany_equals6(&ini->faddr, &c->ip6.dns_match)) tgt->eaddr.a6 = c->ip6.dns_host; else if (!c->no_map_gw && inany_equals4(&ini->faddr, &c->ip4.gw)) diff --git a/passt.1 b/passt.1 index abc13d51..81789cc4 100644 --- a/passt.1 +++ b/passt.1 @@ -244,11 +244,11 @@ usage of DNS addresses altogether. .TP .BR \-\-dns-forward " " \fIaddr -Map \fIaddr\fR (IPv4 or IPv6) as seen from guest or namespace to the first -configured DNS resolver (with corresponding IP version). Mapping is limited to -UDP traffic directed to port 53, and DNS answers are translated back with a -reverse mapping. -This option can be specified zero to two times (once for IPv4, once for IPv6). +Map \fIaddr\fR (IPv4 or IPv6) as seen from guest or namespace to the +first configured DNS resolver (with corresponding IP version). Maps +only UDP and TCP traffic to port 53 or port 853. Replies are +translated back with a reverse mapping. This option can be specified +zero to two times (once for IPv4, once for IPv6). .TP .BR \-S ", " \-\-search " " \fIlist -- 2.45.2

Paul Holzinger

11:41 a.m.

New subject: [PATCH v2 2/2] fwd: Broaden what we consider for DNS specific forwarding rules

Hi, On 24/07/2024 09:51, David Gibson wrote:

...

passt/pasta has options to redirect DNS requests from the guest to a different server address on the host side. Currently, however, only UDP packets to port 53 are considered "DNS requests". This ignores DNS requests over TCP - less common, but certainly possible. It also ignores encrypted DNS requests on port 853.

Extend the DNS forwarding logic to handle both of those cases. The question here is if it handles DoT should it handle DoH as well, i.e. https (443)?

Link: https://github.com/containers/podman/issues/23239

Signed-off-by: David Gibson

Tested-by: Paul Holzinger I tested both dns over tcp and dns over tls with dig. -- Paul

Stefano Brivio

4:30 p.m.

New subject: [PATCH v2 2/2] fwd: Broaden what we consider for DNS specific forwarding rules

On Wed, 24 Jul 2024 11:41:44 +0200 Paul Holzinger wrote:

...

Hi,

On 24/07/2024 09:51, David Gibson wrote:

...
passt/pasta has options to redirect DNS requests from the guest to a different server address on the host side. Currently, however, only UDP packets to port 53 are considered "DNS requests". This ignores DNS requests over TCP - less common, but certainly possible. It also ignores encrypted DNS requests on port 853.

Extend the DNS forwarding logic to handle both of those cases.

The question here is if it handles DoT should it handle DoH as well, i.e. https (443)?

We don't have a flexible interface, yet, to finely configure outbound traffic redirections, so the user couldn't enable or disable this at will. So I'm wondering if there's any use case that we risk breaking with that. The most confusing case I can think of is a host with a local resolver with a loopback address (for example, the usual 127.0.0.53 from systemd-resolved). Without --no-map-gw (or with Podman's --map-gw), we will, by default, use the address of the default gateway (which maps to the host) as implied --dns-forward option. If we now match on HTTPS as well, HTTPS traffic that's supposed to reach the host (because there's an HTTPS server there) will anyway reach the host, even if we mishandle it as DNS traffic somehow. So I don't actually see an issue with that, but given that users can't disable just HTTPS (this should be easier to implement with the flow table, but it will surely be a while before we get to that), we should think quite hard if there's any possibility of breakage before going ahead with it.

...

...
Link: https://github.com/containers/podman/issues/23239

Signed-off-by: David Gibson

Tested-by: Paul Holzinger

I tested both dns over tcp and dns over tls with dig.

Thanks! -- Stefano

David Gibson

25 Jul 25 Jul

6:44 a.m.

New subject: [PATCH v2 2/2] fwd: Broaden what we consider for DNS specific forwarding rules

On Wed, Jul 24, 2024 at 04:30:50PM +0200, Stefano Brivio wrote:

...

On Wed, 24 Jul 2024 11:41:44 +0200 Paul Holzinger wrote:

...
Hi,

On 24/07/2024 09:51, David Gibson wrote:

...
passt/pasta has options to redirect DNS requests from the guest to a different server address on the host side. Currently, however, only UDP packets to port 53 are considered "DNS requests". This ignores DNS requests over TCP - less common, but certainly possible. It also ignores encrypted DNS requests on port 853.

Extend the DNS forwarding logic to handle both of those cases.

The question here is if it handles DoT should it handle DoH as well, i.e. https (443)?

My first inclination was, no, because for traffic to port 443 we can't be confident it's actually DNS. But, then again, maybe going to an address marked as a DNS server address is good enough? I'm not sure.

...

We don't have a flexible interface, yet, to finely configure outbound traffic redirections, so the user couldn't enable or disable this at will. So I'm wondering if there's any use case that we risk breaking with that.

The most confusing case I can think of is a host with a local resolver with a loopback address (for example, the usual 127.0.0.53 from systemd-resolved). Without --no-map-gw (or with Podman's --map-gw), we will, by default, use the address of the default gateway (which maps to the host) as implied --dns-forward option.

If we now match on HTTPS as well, HTTPS traffic that's supposed to reach the host (because there's an HTTPS server there) will anyway reach the host, even if we mishandle it as DNS traffic somehow.

So I don't actually see an issue with that, but given that users can't disable just HTTPS (this should be easier to implement with the flow table, but it will surely be a while before we get to that), we should think quite hard if there's any possibility of breakage before going ahead with it.

Yeah, that argument inclines me back towards "no" for DoH, at least for the time being.

...

...
...
Link: https://github.com/containers/podman/issues/23239

Signed-off-by: David Gibson

Tested-by: Paul Holzinger

I tested both dns over tcp and dns over tls with dig.

Thanks!

-- David Gibson (he or they) | 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

Paul Holzinger

10:39 a.m.

New subject: [PATCH v2 2/2] fwd: Broaden what we consider for DNS specific forwarding rules

On 25/07/2024 06:44, David Gibson wrote:

...

On Wed, Jul 24, 2024 at 04:30:50PM +0200, Stefano Brivio wrote:

...
On Wed, 24 Jul 2024 11:41:44 +0200 Paul Holzinger wrote:

...
Hi,

On 24/07/2024 09:51, David Gibson wrote:

...
passt/pasta has options to redirect DNS requests from the guest to a different server address on the host side. Currently, however, only UDP packets to port 53 are considered "DNS requests". This ignores DNS requests over TCP - less common, but certainly possible. It also ignores encrypted DNS requests on port 853.

Extend the DNS forwarding logic to handle both of those cases. The question here is if it handles DoT should it handle DoH as well, i.e. https (443)? My first inclination was, no, because for traffic to port 443 we can't be confident it's actually DNS. But, then again, maybe going to an address marked as a DNS server address is good enough? I'm not sure.

...
We don't have a flexible interface, yet, to finely configure outbound traffic redirections, so the user couldn't enable or disable this at will. So I'm wondering if there's any use case that we risk breaking with that.

The most confusing case I can think of is a host with a local resolver with a loopback address (for example, the usual 127.0.0.53 from systemd-resolved). Without --no-map-gw (or with Podman's --map-gw), we will, by default, use the address of the default gateway (which maps to the host) as implied --dns-forward option.

If we now match on HTTPS as well, HTTPS traffic that's supposed to reach the host (because there's an HTTPS server there) will anyway reach the host, even if we mishandle it as DNS traffic somehow.

So I don't actually see an issue with that, but given that users can't disable just HTTPS (this should be easier to implement with the flow table, but it will surely be a while before we get to that), we should think quite hard if there's any possibility of breakage before going ahead with it. Yeah, that argument inclines me back towards "no" for DoH, at least for the time being. Ok, I agree.

...
...
...
Link: https://github.com/containers/podman/issues/23239

Signed-off-by: David Gibson Tested-by: Paul Holzinger

I tested both dns over tcp and dns over tls with dig. Thanks!

-- Paul Holzinger

Stefano Brivio

1:27 p.m.

On Wed, 24 Jul 2024 17:51:10 +1000 David Gibson wrote:

...

When looking through outstanding bugs in various trackers, I noticed this one: https://github.com/containers/podman/issues/23239

Now that the flow table is merged, this is very easy to fix, so, here's a fix. While we're at it, handle encrypted DNS on port 853 as well, which Red Hat certainly seems to be interested in for one.

Changes since v1: * Fix some stylistic errors in 1/2 * Update man page to reflect new behaviour in 2/2

David Gibson (2): fwd: Refactor tests in fwd_nat_from_tap() for clarity fwd: Broaden what we consider for DNS specific forwarding rules

Applied. -- Stefano

314

Age (days ago)

315

Last active (days ago)

List overview

Download

7 comments

3 participants

participants (3)

David Gibson
Paul Holzinger
Stefano Brivio

[PATCH v2 0/2] Broaden DNS forwarding

tags

participants (3)