From d88b63d19284ee98f3df1731a40a894d9d22878c Mon Sep 17 00:00:00 2001 From: Handa Wang <7058128+superwhd@users.noreply.github.com> Date: Tue, 29 Oct 2024 22:54:55 +0800 Subject: [PATCH] [posix] bind the resolver's UDP socket to the infra network interface (#10864) This commit fixes an issue in DNS recursive resolver that it didn't bind its socket to the infra network interface. This may cause the DNS message to be sent on an unexpected network interface, depending on the routing table of the platform. This commit also updates the test case `test_upstream_dns.py` to make the upstream DNS server run on a different node. Previously the upstream DNS server ran on the same node as the BR which is a limitation of this test case. --- src/posix/platform/openthread-posix-config.h | 9 ++++ src/posix/platform/resolver.cpp | 23 ++++++++++- src/posix/platform/resolver.hpp | 2 + .../internet/test_upstream_dns.py | 41 +++++++++++-------- tests/scripts/thread-cert/node.py | 24 +++++++---- 5 files changed, 72 insertions(+), 27 deletions(-) diff --git a/src/posix/platform/openthread-posix-config.h b/src/posix/platform/openthread-posix-config.h index 2a4cdf7af..20411d9ea 100644 --- a/src/posix/platform/openthread-posix-config.h +++ b/src/posix/platform/openthread-posix-config.h @@ -407,4 +407,13 @@ #define OPENTHREAD_POSIX_CONFIG_RCP_CAPS_DIAG_ENABLE OPENTHREAD_CONFIG_DIAG_ENABLE #endif +/** + * @def OPENTHREAD_POSIX_CONFIG_UPSTREAM_DNS_BIND_TO_INFRA_NETIF + * + * Define as 1 to let the upstream DNS bind the socket to infra network interface. + */ +#ifndef OPENTHREAD_POSIX_CONFIG_UPSTREAM_DNS_BIND_TO_INFRA_NETIF +#define OPENTHREAD_POSIX_CONFIG_UPSTREAM_DNS_BIND_TO_INFRA_NETIF 1 +#endif + #endif // OPENTHREAD_PLATFORM_POSIX_CONFIG_H_ diff --git a/src/posix/platform/resolver.cpp b/src/posix/platform/resolver.cpp index 9aa53196a..0e16e6fab 100644 --- a/src/posix/platform/resolver.cpp +++ b/src/posix/platform/resolver.cpp @@ -175,7 +175,7 @@ Resolver::Transaction *Resolver::AllocateTransaction(otPlatDnsUpstreamQuery *aTh { if (txn.mThreadTxn == nullptr) { - fdOrError = socket(AF_INET, SOCK_DGRAM, 0); + fdOrError = CreateUdpSocket(); if (fdOrError < 0) { LogInfo("Failed to create socket for upstream resolver: %d", fdOrError); @@ -313,6 +313,27 @@ void Resolver::SetUpstreamDnsServers(const otIp6Address *aUpstreamDnsServers, in } } +int Resolver::CreateUdpSocket(void) +{ + int fd = -1; + + VerifyOrExit(otSysGetInfraNetifName() != nullptr, LogDebg("No infra network interface available")); + fd = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP); + VerifyOrExit(fd >= 0, LogDebg("Failed to create the UDP socket: %s", strerror(errno))); +#if OPENTHREAD_POSIX_CONFIG_UPSTREAM_DNS_BIND_TO_INFRA_NETIF + if (setsockopt(fd, SOL_SOCKET, SO_BINDTODEVICE, otSysGetInfraNetifName(), strlen(otSysGetInfraNetifName())) < 0) + { + LogDebg("Failed to bind the UDP socket to infra interface %s: %s", otSysGetInfraNetifName(), strerror(errno)); + close(fd); + fd = -1; + ExitNow(); + } +#endif + +exit: + return fd; +} + } // namespace Posix } // namespace ot diff --git a/src/posix/platform/resolver.hpp b/src/posix/platform/resolver.hpp index f935ebc76..d1d0e2cfc 100644 --- a/src/posix/platform/resolver.hpp +++ b/src/posix/platform/resolver.hpp @@ -111,6 +111,8 @@ private: int mUdpFd; }; + static int CreateUdpSocket(void); + Transaction *GetTransaction(int aFd); Transaction *GetTransaction(otPlatDnsUpstreamQuery *aThreadTxn); Transaction *AllocateTransaction(otPlatDnsUpstreamQuery *aThreadTxn); diff --git a/tests/scripts/thread-cert/border_router/internet/test_upstream_dns.py b/tests/scripts/thread-cert/border_router/internet/test_upstream_dns.py index 84707d280..acd525609 100755 --- a/tests/scripts/thread-cert/border_router/internet/test_upstream_dns.py +++ b/tests/scripts/thread-cert/border_router/internet/test_upstream_dns.py @@ -41,14 +41,14 @@ import shlex # Topology: # ----------------(eth)-------------------- # | | -# BR (Leader) HOST +# BR (Leader) DNS SERVER # | # ROUTER # BR = 1 ROUTER = 2 -HOST = 3 +DNS_SERVER = 3 TEST_DOMAIN = 'test.domain' TEST_DOMAIN_IP6_ADDRESSES = {'2001:db8::1'} @@ -70,17 +70,15 @@ class UpstreamDns(thread_cert.TestCase): TOPOLOGY = { BR: { 'name': 'BR', - 'allowlist': [ROUTER], 'is_otbr': True, 'version': '1.4', }, ROUTER: { 'name': 'Router', - 'allowlist': [BR], 'version': '1.4', }, - HOST: { - 'name': 'Host', + DNS_SERVER: { + 'name': 'DNS Server', 'is_host': True }, } @@ -88,27 +86,25 @@ class UpstreamDns(thread_cert.TestCase): def test(self): br = self.nodes[BR] router = self.nodes[ROUTER] - host = self.nodes[HOST] + dns_server = self.nodes[DNS_SERVER] - host.start(start_radvd=False) - self.simulator.go(5) + self._start_dns_server(dns_server) + dns_server_addr = dns_server.get_ether_addrs(ipv4=True, ipv6=False)[0] + + # Update BR's /etc/resolv.conf and force BR to reload it + br.bash(shlex.join(['echo', 'nameserver ' + dns_server_addr]) + ' >> /etc/resolv.conf') + br.stop_otbr_service() + br.start_otbr_service() br.start() - # When feature flag is enabled, NAT64 might be disabled by default. So - # ensure NAT64 is enabled here. self.simulator.go(config.LEADER_STARTUP_DELAY) self.assertEqual('leader', br.get_state()) + # When feature flag is enabled, NAT64 might be disabled by default. So + # ensure NAT64 is enabled here. br.nat64_set_enabled(True) br.srp_server_set_enabled(True) - br.bash('service bind9 stop') - - br.bash(shlex.join(['echo', TEST_DOMAIN_BIND_CONF]) + ' >> /etc/bind/named.conf.local') - br.bash(shlex.join(['echo', TEST_DOMAIN_BIND_ZONE]) + ' >> /etc/bind/db.test.domain') - - br.bash('service bind9 start') - router.start() self.simulator.go(config.ROUTER_STARTUP_DELAY) self.assertEqual('router', router.get_state()) @@ -130,6 +126,15 @@ class UpstreamDns(thread_cert.TestCase): for record in resolved_names: self.assertIn(ipaddress.IPv6Address(record[0]).compressed, TEST_DOMAIN_IP6_ADDRESSES) + def _start_dns_server(self, dns_server): + dns_server.start(start_radvd=False) + dns_server.bash('service bind9 stop') + + dns_server.bash(shlex.join(['echo', TEST_DOMAIN_BIND_CONF]) + ' >> /etc/bind/named.conf.local') + dns_server.bash(shlex.join(['echo', TEST_DOMAIN_BIND_ZONE]) + ' >> /etc/bind/db.test.domain') + + dns_server.bash('service bind9 start') + if __name__ == '__main__': unittest.main() diff --git a/tests/scripts/thread-cert/node.py b/tests/scripts/thread-cert/node.py index b64b99d72..a9d838259 100755 --- a/tests/scripts/thread-cert/node.py +++ b/tests/scripts/thread-cert/node.py @@ -3818,19 +3818,27 @@ class LinuxHost(): self.bash(f'ip link set {self.ETH_DEV} down') - def get_ether_addrs(self): - output = self.bash(f'ip -6 addr list dev {self.ETH_DEV}') + def get_ether_addrs(self, ipv4=False, ipv6=True): + output = self.bash(f'ip addr list dev {self.ETH_DEV}') addrs = [] for line in output: - # line example: "inet6 fe80::42:c0ff:fea8:903/64 scope link" + # line examples: + # "inet6 fe80::42:c0ff:fea8:903/64 scope link" + # "inet 192.168.9.1/24 brd 192.168.9.255 scope global eth0" line = line.strip().split() - if line and line[0] == 'inet6': - addr = line[1] - if '/' in addr: - addr = addr.split('/')[0] - addrs.append(addr) + if not line or not line[0].startswith('inet'): + continue + if line[0] == 'inet' and not ipv4: + continue + if line[0] == 'inet6' and not ipv6: + continue + + addr = line[1] + if '/' in addr: + addr = addr.split('/')[0] + addrs.append(addr) logging.debug('%s: get_ether_addrs: %r', self, addrs) return addrs