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
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 <david(a)gibson.dropbear.id.au>
---
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
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 <david(a)gibson.dropbear.id.au> --- 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
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 <david(a)gibson.dropbear.id.au>Tested-by: Paul Holzinger <pholzing(a)redhat.com> I tested both dns over tcp and dns over tls with dig. -- Paul
On Wed, 24 Jul 2024 11:41:44 +0200 Paul Holzinger <pholzing(a)redhat.com> wrote:Hi, On 24/07/2024 09:51, David Gibson wrote: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.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)?Thanks! -- StefanoLink: https://github.com/containers/podman/issues/23239 Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>Tested-by: Paul Holzinger <pholzing(a)redhat.com> I tested both dns over tcp and dns over tls with dig.
On Wed, Jul 24, 2024 at 04:30:50PM +0200, Stefano Brivio wrote:On Wed, 24 Jul 2024 11:41:44 +0200 Paul Holzinger <pholzing(a)redhat.com> 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.-- 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/~dgibsonThanks!Link: https://github.com/containers/podman/issues/23239 Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>Tested-by: Paul Holzinger <pholzing(a)redhat.com> I tested both dns over tcp and dns over tls with dig.
On 25/07/2024 06:44, David Gibson wrote:On Wed, Jul 24, 2024 at 04:30:50PM +0200, Stefano Brivio wrote:Ok, I agree.On Wed, 24 Jul 2024 11:41:44 +0200 Paul Holzinger <pholzing(a)redhat.com> 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 <david(a)gibson.dropbear.id.au> >> Tested-by: Paul Holzinger <pholzing(a)redhat.com> >> >> I tested both dns over tcp and dns over tls with dig. > Thanks! >-- Paul Holzinger
On Wed, 24 Jul 2024 17:51:10 +1000 David Gibson <david(a)gibson.dropbear.id.au> 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 rulesApplied. -- Stefano