resolved: don't query domain-limited DNS servers for other domains (#3621)

DNS servers which have route-only domains should only be used for
the specified domains. Routing queries about other domains there is a privacy
violation, prone to fail (as that DNS server was not meant to be used for other
domains), and puts unnecessary load onto that server.

Introduce a new helper function dns_server_limited_domains() that checks if the
DNS server should only be used for some selected domains, i. e. has some
route-only domains without "~.". Use that when determining whether to query it
in the scope, and when writing resolv.conf.

Extend the test_route_only_dns() case to ensure that the DNS server limited to
~company does not appear in resolv.conf. Add test_route_only_dns_all_domains()
to ensure that a server that also has ~. does appear in resolv.conf as global
name server. These reproduce #3420.

Add a new test_resolved_domain_restricted_dns() test case that verifies that
domain-limited DNS servers are only being used for those domains. This
reproduces #3421.

Clarify what a "routing domain" is in the manpage.

Fixes #3420
Fixes #3421

6 files changed, 152 insertions, 3 deletions

diff --git a/man/systemd.network.xml b/man/systemd.network.xml
index 9c1b10fc5c..08dd157e31 100644
--- a/man/systemd.network.xml
+++ b/man/systemd.network.xml
@@ -475,8 +475,8 @@
 
             <para>The specified domains are also used for routing of DNS queries: look-ups for host names ending in the
             domains specified here are preferably routed to the DNS servers configured for this interface. If a domain
-            name is prefixed with <literal>~</literal>, the domain name becomes a pure "routing" domain, is used for
-            DNS query routing purposes only and is not used in the described domain search logic. By specifying a
+            name is prefixed with <literal>~</literal>, the domain name becomes a pure "routing" domain, the DNS server
+            is used for the given domain names only and is not used in the described domain search logic. By specifying a
             routing domain of <literal>~.</literal> (the tilde indicating definition of a routing domain, the dot
             referring to the DNS root domain which is the implied suffix of all valid DNS names) it is possible to
             route all DNS traffic preferably to the DNS server specified for this interface. The route domain logic is
diff --git a/src/resolve/resolved-dns-scope.c b/src/resolve/resolved-dns-scope.c
index ed0c6aa105..03811ac8e7 100644
--- a/src/resolve/resolved-dns-scope.c
+++ b/src/resolve/resolved-dns-scope.c
@@ -407,6 +407,7 @@ int dns_scope_socket_tcp(DnsScope *s, int family, const union in_addr_union *add
 
 DnsScopeMatch dns_scope_good_domain(DnsScope *s, int ifindex, uint64_t flags, const char *domain) {
         DnsSearchDomain *d;
+        DnsServer *dns_server;
 
         assert(s);
         assert(domain);
@@ -447,6 +448,13 @@ DnsScopeMatch dns_scope_good_domain(DnsScope *s, int ifindex, uint64_t flags, co
                 if (dns_name_endswith(domain, d->name) > 0)
                         return DNS_SCOPE_YES;
 
+        /* If the DNS server has route-only domains, don't send other requests
+         * to it. This would be a privacy violation, will most probably fail
+         * anyway, and adds unnecessary load. */
+        dns_server = dns_scope_get_dns_server(s);
+        if (dns_server && dns_server_limited_domains(dns_server))
+                return DNS_SCOPE_NO;
+
         switch (s->protocol) {
 
         case DNS_PROTOCOL_DNS:
diff --git a/src/resolve/resolved-dns-server.c b/src/resolve/resolved-dns-server.c
index 9b7b471600..97cc8c0e09 100644
--- a/src/resolve/resolved-dns-server.c
+++ b/src/resolve/resolved-dns-server.c
@@ -576,6 +576,27 @@ void dns_server_warn_downgrade(DnsServer *server) {
         server->warned_downgrade = true;
 }
 
+bool dns_server_limited_domains(DnsServer *server)
+{
+        DnsSearchDomain *domain;
+        bool domain_restricted = false;
+
+        /* Check if the server has route-only domains without ~., i. e. whether
+         * it should only be used for particular domains */
+        if (!server->link)
+                return false;
+
+        LIST_FOREACH(domains, domain, server->link->search_domains)
+                if (domain->route_only) {
+                        domain_restricted = true;
+                        /* ~. means "any domain", thus it is a global server */
+                        if (streq(DNS_SEARCH_DOMAIN_NAME(domain), "."))
+                                return false;
+                }
+
+        return domain_restricted;
+}
+
 static void dns_server_hash_func(const void *p, struct siphash *state) {
         const DnsServer *s = p;
 
diff --git a/src/resolve/resolved-dns-server.h b/src/resolve/resolved-dns-server.h
index c1732faffd..83e288a202 100644
--- a/src/resolve/resolved-dns-server.h
+++ b/src/resolve/resolved-dns-server.h
@@ -128,6 +128,8 @@ bool dns_server_dnssec_supported(DnsServer *server);
 
 void dns_server_warn_downgrade(DnsServer *server);
 
+bool dns_server_limited_domains(DnsServer *server);
+
 DnsServer *dns_server_find(DnsServer *first, int family, const union in_addr_union *in_addr, int ifindex);
 
 void dns_server_unlink_all(DnsServer *first);
diff --git a/src/resolve/resolved-resolv-conf.c b/src/resolve/resolved-resolv-conf.c
index 31b25ca50f..801014caf5 100644
--- a/src/resolve/resolved-resolv-conf.c
+++ b/src/resolve/resolved-resolv-conf.c
@@ -154,6 +154,16 @@ static void write_resolv_conf_server(DnsServer *s, FILE *f, unsigned *count) {
                 return;
         }
 
+        /* Check if the DNS server is limited to particular domains;
+         * resolv.conf does not have a syntax to express that, so it must not
+         * appear as a global name server to avoid routing unrelated domains to
+         * it (which is a privacy violation, will most probably fail anyway,
+         * and adds unnecessary load) */
+        if (dns_server_limited_domains(s)) {
+                log_debug("DNS server %s has route-only domains, not using as global name server", dns_server_string(s));
+                return;
+        }
+
         if (*count == MAXNS)
                 fputs("# Too many DNS servers configured, the following entries may be ignored.\n", f);
         (*count)++;
diff --git a/test/networkd-test.py b/test/networkd-test.py
index baa1dc2a47..3091722fc1 100755
--- a/test/networkd-test.py
+++ b/test/networkd-test.py
@@ -250,6 +250,38 @@ Domains= ~company''')
             self.assertNotRegex(contents, 'search.*company')
             # our global server should appear
             self.assertIn('nameserver 192.168.5.1\n', contents)
+            # should not have domain-restricted server as global server
+            self.assertNotIn('nameserver 192.168.42.1\n', contents)
+
+    def test_route_only_dns_all_domains(self):
+        with open('/run/systemd/network/myvpn.netdev', 'w') as f:
+            f.write('''[NetDev]
+Name=dummy0
+Kind=dummy
+MACAddress=12:34:56:78:9a:bc''')
+        with open('/run/systemd/network/myvpn.network', 'w') as f:
+            f.write('''[Match]
+Name=dummy0
+[Network]
+Address=192.168.42.100
+DNS=192.168.42.1
+Domains= ~company ~.''')
+        self.addCleanup(os.remove, '/run/systemd/network/myvpn.netdev')
+        self.addCleanup(os.remove, '/run/systemd/network/myvpn.network')
+
+        self.do_test(coldplug=True, ipv6=False,
+                     extra_opts='IPv6AcceptRouterAdvertisements=False')
+
+        with open(RESOLV_CONF) as f:
+            contents = f.read()
+
+        # ~company is not a search domain, only a routing domain
+        self.assertNotRegex(contents, 'search.*company')
+
+        # our global server should appear
+        self.assertIn('nameserver 192.168.5.1\n', contents)
+        # should have company server as global server due to ~.
+        self.assertIn('nameserver 192.168.42.1\n', contents)
 
 
 @unittest.skipUnless(have_dnsmasq, 'dnsmasq not installed')
@@ -260,7 +292,7 @@ class DnsmasqClientTest(ClientTestBase, unittest.TestCase):
         super().setUp()
         self.dnsmasq = None
 
-    def create_iface(self, ipv6=False):
+    def create_iface(self, ipv6=False, dnsmasq_opts=None):
         '''Create test interface with DHCP server behind it'''
 
         # add veth pair
@@ -281,6 +313,8 @@ class DnsmasqClientTest(ClientTestBase, unittest.TestCase):
             extra_opts = ['--enable-ra', '--dhcp-range=2600::10,2600::20']
         else:
             extra_opts = []
+        if dnsmasq_opts:
+            extra_opts += dnsmasq_opts
         self.dnsmasq = subprocess.Popen(
             ['dnsmasq', '--keep-in-foreground', '--log-queries',
              '--log-facility=' + self.dnsmasq_log, '--conf-file=/dev/null',
@@ -305,6 +339,80 @@ class DnsmasqClientTest(ClientTestBase, unittest.TestCase):
         with open(self.dnsmasq_log) as f:
             sys.stdout.write('\n\n---- dnsmasq log ----\n%s\n------\n\n' % f.read())
 
+    def test_resolved_domain_restricted_dns(self):
+        '''resolved: domain-restricted DNS servers'''
+
+        # create interface for generic connections; this will map all DNS names
+        # to 192.168.42.1
+        self.create_iface(dnsmasq_opts=['--address=/#/192.168.42.1'])
+        self.writeConfig('/run/systemd/network/general.network', '''\
+[Match]
+Name=%s
+[Network]
+DHCP=ipv4
+IPv6AcceptRA=False''' % self.iface)
+
+        # create second device/dnsmasq for a .company/.lab VPN interface
+        # static IPs for simplicity
+        subprocess.check_call(['ip', 'link', 'add', 'name', 'testvpnclient', 'type',
+                               'veth', 'peer', 'name', 'testvpnrouter'])
+        self.addCleanup(subprocess.call, ['ip', 'link', 'del', 'dev', 'testvpnrouter'])
+        subprocess.check_call(['ip', 'a', 'flush', 'dev', 'testvpnrouter'])
+        subprocess.check_call(['ip', 'a', 'add', '10.241.3.1/24', 'dev', 'testvpnrouter'])
+        subprocess.check_call(['ip', 'link', 'set', 'testvpnrouter', 'up'])
+
+        vpn_dnsmasq_log = os.path.join(self.workdir, 'dnsmasq-vpn.log')
+        vpn_dnsmasq = subprocess.Popen(
+            ['dnsmasq', '--keep-in-foreground', '--log-queries',
+             '--log-facility=' + vpn_dnsmasq_log, '--conf-file=/dev/null',
+             '--dhcp-leasefile=/dev/null', '--bind-interfaces',
+             '--interface=testvpnrouter', '--except-interface=lo',
+             '--address=/math.lab/10.241.3.3', '--address=/cantina.company/10.241.4.4'])
+        self.addCleanup(vpn_dnsmasq.wait)
+        self.addCleanup(vpn_dnsmasq.kill)
+
+        self.writeConfig('/run/systemd/network/vpn.network', '''\
+[Match]
+Name=testvpnclient
+[Network]
+IPv6AcceptRA=False
+Address=10.241.3.2/24
+DNS=10.241.3.1
+Domains= ~company ~lab''')
+
+        subprocess.check_call(['systemctl', 'start', 'systemd-networkd'])
+        subprocess.check_call([self.networkd_wait_online, '--interface', self.iface,
+                               '--interface=testvpnclient', '--timeout=20'])
+
+        # ensure we start fresh with every test
+        subprocess.check_call(['systemctl', 'restart', 'systemd-resolved'])
+
+        # test vpnclient specific domains; these should *not* be answered by
+        # the general DNS
+        out = subprocess.check_output(['systemd-resolve', 'math.lab'])
+        self.assertIn(b'math.lab: 10.241.3.3', out)
+        out = subprocess.check_output(['systemd-resolve', 'kettle.cantina.company'])
+        self.assertIn(b'kettle.cantina.company: 10.241.4.4', out)
+
+        # test general domains
+        out = subprocess.check_output(['systemd-resolve', 'megasearch.net'])
+        self.assertIn(b'megasearch.net: 192.168.42.1', out)
+
+        with open(self.dnsmasq_log) as f:
+            general_log = f.read()
+        with open(vpn_dnsmasq_log) as f:
+            vpn_log = f.read()
+
+        # VPN domains should only be sent to VPN DNS
+        self.assertRegex(vpn_log, 'query.*math.lab')
+        self.assertRegex(vpn_log, 'query.*cantina.company')
+        self.assertNotIn('lab', general_log)
+        self.assertNotIn('company', general_log)
+
+        # general domains should not be sent to the VPN DNS
+        self.assertRegex(general_log, 'query.*megasearch.net')
+        self.assertNotIn('megasearch.net', vpn_log)
+
 
 class NetworkdClientTest(ClientTestBase, unittest.TestCase):
     '''Test networkd client against networkd server'''