NLnetLabs / unbound

Unbound is a validating, recursive, and caching DNS resolver.
https://nlnetlabs.nl/unbound
BSD 3-Clause "New" or "Revised" License
3.13k stars 359 forks source link

Empty view is never matched #1083

Closed aaliddell closed 5 months ago

aaliddell commented 5 months ago

Describe the bug If you have a view with only a name but no internal zones/data, it will be silently ignored and any access-control-view matching that view name appears to be dropped. An empty view can be useful to match a more specific IP block that otherwise matches a different view or can come about from automated tools. This silent ignoring leads to the wrong view being matched for an IP block and therefore unexpected exposure of data within that view.

To reproduce Steps to reproduce the behavior:

  1. Create some sample local-zone/local-data at the server top level (used to demonstrate bug)
  2. Create an empty named view with view-first: no. Queries hitting this should not return the data set in step 1 above
  3. Add an access-control-view that selects a relevant IP range and points to the empty view created in step 2
  4. Query from specified IP range for the zone data setup in step 1, where I'd expect to match the empty view and therefore not to see the data from step 1 (due to view-first: no).
  5. Now add a single local-zone entry to the otherwise empty view (with any content, it does not need to be something you will query for, only its presence matters)
  6. Repeat the query in step 4 and see that the data from step 1 is now not returned, as the view is correctly matching

Expected behavior An empty view should still match an ACL and not be silently ignored. The behaviour should match what is observed in step 6 above, where a dummy zone has been added to force the view to exist. From a quick browse of code, this could be due to:

https://github.com/NLnetLabs/unbound/blob/86fe9cbce533549e4310bd3fc7c1b89df70a33d4/services/view.c#L161

System:

Version 1.17.1

Configure line: --build=x86_64-linux-gnu --prefix=/usr --includedir=${prefix}/include --mandir=${prefix}/share/man --infodir=${prefix}/share/info --sysconfdir=/etc --localstatedir=/var --disable-option-checking --disable-silent-rules --libdir=${prefix}/lib/x86_64-linux-gnu --runstatedir=/run --disable-maintainer-mode --disable-dependency-tracking --with-pythonmodule --with-pyunbound --enable-subnet --enable-dnstap --enable-systemd --with-libnghttp2 --with-chroot-dir= --with-dnstap-socket-path=/run/dnstap.sock --disable-rpath --with-pidfile=/run/unbound.pid --with-libevent --enable-tfo-client --with-rootkey-file=/usr/share/dns/root.key --enable-tfo-server
Linked libs: libevent 2.1.12-stable (it uses epoll), OpenSSL 3.0.11 19 Sep 2023
Linked modules: dns64 python subnetcache respip validator iterator
TCP Fastopen feature available
gthess commented 5 months ago

This is explicit behavior in the code https://github.com/NLnetLabs/unbound/blob/86fe9cbce533549e4310bd3fc7c1b89df70a33d4/services/localzone.c#L1873 and the man page https://unbound.docs.nlnetlabs.nl/en/latest/manpages/unbound.conf.html#unbound-conf-view-local-zone.

aaliddell commented 5 months ago

Ok, sorry I missed that. I would suggest it’s somewhat surprising behaviour, but can’t argue with the docs 😂