From dfa14e2859418593b2f9bfae8936d780148c4e6a Mon Sep 17 00:00:00 2001 From: Benjamin Franzke Date: Tue, 31 May 2022 21:36:55 +0200 Subject: [PATCH 1/2] resolved: define source address for proxy-only stub replies DnsPacket.ifindex=1 (loopback) is normalized to 0 whenever a message is received on the loopback iface, so for both listeners, 127.0.0.53 and 127.0.0.54, the ifindex will be set to 0 by manager_recv() for queries that have a local origin. Replies to such local messages need to set a proper ifindex in any case, as the supplied source-address would otherwise be ignored in manager_ipv4_send() (CMSG generation is skipped due to ifindex > 0 check). Note that this change only forces `ifindex` to loopback if it was actually normalized to `0` before (due to a loopback detection) in order to keep the nat-to-127.0.0.54-from-another-interface usecase that was described in a8d09063447568d87288a8e868fe386c1da7ce09 intact. Also note that nat is not supported for the main stub 127.0.0.53 which is why forcing LOOPBACK_IFINDEX was/is fine for that case. Fixes #23495 --- src/resolve/resolved-dns-stub.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/resolve/resolved-dns-stub.c b/src/resolve/resolved-dns-stub.c index 9e34161eb3..89d1f1cdfc 100644 --- a/src/resolve/resolved-dns-stub.c +++ b/src/resolve/resolved-dns-stub.c @@ -528,18 +528,26 @@ static int dns_stub_send( if (s) r = dns_stream_write_packet(s, reply); else { - int fd; + int fd, ifindex; fd = find_socket_fd(m, l, p->family, &p->sender, SOCK_DGRAM); if (fd < 0) return fd; + if (address_is_proxy(p->family, &p->destination)) + /* Force loopback iface if this is the loopback proxy stub + * and ifindex was normalized to 0 by manager_recv(). */ + ifindex = p->ifindex ?: LOOPBACK_IFINDEX; + else + /* Force loopback iface if this is the main listener stub. */ + ifindex = l ? p->ifindex : LOOPBACK_IFINDEX; + /* Note that it is essential here that we explicitly choose the source IP address for this * packet. This is because otherwise the kernel will choose it automatically based on the - * routing table and will thus pick 127.0.0.1 rather than 127.0.0.53. */ + * routing table and will thus pick 127.0.0.1 rather than 127.0.0.53/54. */ r = manager_send(m, fd, - l || address_is_proxy(p->family, &p->destination) ? p->ifindex : LOOPBACK_IFINDEX, /* force loopback iface if this is the main listener stub */ + ifindex, p->family, &p->sender, p->sender_port, &p->destination, reply); } From de777ffae8683526d089a781a498af06f8c0aee7 Mon Sep 17 00:00:00 2001 From: Benjamin Franzke Date: Wed, 1 Jun 2022 14:40:51 +0200 Subject: [PATCH 2/2] resolved: choose correct file descriptor for proxy stub replies MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit find_socket_fd() does not expect the sender address, but the listen-address. This is in fact the destination of the DNS packet. Matching via sender address caused a fallback to the default stub listener in manager_dns_stub_fd() as the sender address can never match the proxy stub listen address. Note that manager_dns_stub_fd() is only used for the default listener stub and the proxy stub, that means *extra* listeners stubs (DNSStubListenerExtra=…) have not been affected as `struct DnsStubListenerExtra` provides a direct link to the event source. By using the correct fd we ensure the correct socket options (like TTL) are used and prevent issues like #23495 in case ifindex could not be determined. --- src/resolve/resolved-dns-stub.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/resolve/resolved-dns-stub.c b/src/resolve/resolved-dns-stub.c index 89d1f1cdfc..fdd589fa92 100644 --- a/src/resolve/resolved-dns-stub.c +++ b/src/resolve/resolved-dns-stub.c @@ -530,7 +530,7 @@ static int dns_stub_send( else { int fd, ifindex; - fd = find_socket_fd(m, l, p->family, &p->sender, SOCK_DGRAM); + fd = find_socket_fd(m, l, p->family, &p->destination, SOCK_DGRAM); if (fd < 0) return fd;