Skip to content

dns/bind: add DNS-over-TLS forwarder configuration - #5545

Open
bryanwieg wants to merge 1 commit into
opnsense:masterfrom
bryanwieg:dns-bind-forwarders-dot
Open

dns/bind: add DNS-over-TLS forwarder configuration#5545
bryanwieg wants to merge 1 commit into
opnsense:masterfrom
bryanwieg:dns-bind-forwarders-dot

Conversation

@bryanwieg

@bryanwieg bryanwieg commented Jul 10, 2026

Copy link
Copy Markdown

Important notices

If AI was used, please disclose:

  • Models used: GPT-5.6 Terra
  • Extent of AI involvement: I handled design and decisions. It assisted with testing, investigation, code generation, review, commits, commit messages, and PR generation.

Summary

BIND supports native DNS-over-TLS (DoT) forwarders, but the plugin previously exposed only a legacy comma-separated forwarder list. That format cannot represent a destination port or TLS hostname per forwarder.

This PR adds a dedicated Query Forwarding page with separate grids for plain DNS and DoT upstreams. Each forwarder has its own address and port; DoT forwarders may also specify a TLS hostname.

Behaviour

  • With a TLS hostname, BIND verifies the upstream certificate against that hostname using the system CA bundle.
  • Without a TLS hostname, BIND verifies the certificate against the configured forwarder's IP address; the certificate must include that IP address in its Subject Alternative Name.
  • The legacy forwarder CSV is migrated into the plain-DNS grid by the Forwarder model.
  • The General page retains the Forward First and Forward Only policy choice.

Scope

This PR is intentionally limited to forwarder configuration and DNS-over-TLS support. The unrelated DNSBL, dynamic DNS, reverse-zone, listener-interface, zone-NOTIFY, and UI restructuring work has been split into separate branches for follow-up PRs.

BIND compatibility

BIND 9.20.24 introduced an upstream regression that breaks DNS-over-TLS forwarders when BIND is configured with Forward Only. The fix is upstream in fb0ae8a5d6b2abb8b6ad3529e205caacbe4e70d1 and was released in BIND 9.20.26.

Users who need DoT with Forward Only should use BIND 9.20.26 or later.

Validation

  • Testing and running all of this on my CARP enabled cluster, on OPNsense 26.1.11_6-amd64.
  • PHP syntax checks pass for the BIND plugin.
  • Tested on OPNsense with BIND 9.20.26, a configured DoT upstream, and Forward Only: queries for example.com, cloudflare.com, and openai.com returned NOERROR.

Related issue

Closes #5546

@bryanwieg
bryanwieg marked this pull request as draft July 10, 2026 19:53
@bryanwieg

Copy link
Copy Markdown
Author

I have submitted this pr now to invite any feedback or guidance from the maintainers.

I still have to create an issue, and I also am in the process of more thoroughly testing these changes in a working environment.

@bryanwieg
bryanwieg force-pushed the dns-bind-forwarders-dot branch 2 times, most recently from 315470c to 2c4fd3f Compare July 14, 2026 03:31
@bryanwieg
bryanwieg force-pushed the dns-bind-forwarders-dot branch 2 times, most recently from a63969b to 1d6e62e Compare July 18, 2026 03:22
@bryanwieg

Copy link
Copy Markdown
Author

got this running on my ha setup, with a dot upstream service. so far so good.

yes, I added more stuff to make it parity with unbound. please let me know if some of this is better suited for separate mr's, or other changes i need to make instead.

I've made quite a few assumptions, so i welcome feedback. the commits are split per feature.

@bryanwieg
bryanwieg force-pushed the dns-bind-forwarders-dot branch 10 times, most recently from d795063 to 2bcaeb8 Compare July 24, 2026 15:55
@bryanwieg
bryanwieg force-pushed the dns-bind-forwarders-dot branch 6 times, most recently from e97697f to 6bafb48 Compare July 28, 2026 23:41
@mimugmail

Copy link
Copy Markdown
Member

Hi,

There are so many changes in this PR, its impossible to review (for a human). It looks like a complete rewrite by AI and I have no idea how to maintain this in future. :(

@maloo1

maloo1 commented Jul 29, 2026

Copy link
Copy Markdown

Hi @mimugmail and @bryanwieg I am trying to achieve the same thing (want to add DoT forwarders) in bind on opnsense.

Can I suggest that we just add another directory to add custom options as suggested in here: #5546?

I can write this up as a pull request (as a human as it looks pretty easy based on what was done for #4773) if you would be open to that?

Thanks for your consideration!!
Paul

@mimugmail

Copy link
Copy Markdown
Member

@maloo1 if it's just a couple of lines (it think it shouldnt be more), sure. Problem is, real humans can't audit such code in spare time. I ran this PR against my Claude Pro for fun, it's too big :)

This is a big one — ~7,000 added lines across 83 files. Let me see the whole picture.

Synthesized comprehensive security and architecture critique across multiple layers
Synthesized comprehensive security and architecture critique across multiple layers

I pulled the PR down over git (opnsense/plugins#5545, 8 commits by Bryan on top of b57967f, 83 files, +7047/−903) and read through it end to end. I also fetched core's interfaces.inc and unbound.inc to check a few assumptions. Here's the review.

Overall

Four substantial features in one PR: DoT/plain forwarder grids, reverse zones, DHCP-lease DDNS watcher, DNSBL rework, plus a listener-selection rewrite and a menu restructure. CONTRIBUTING.md's "we can't review large initial codebases" applies in spirit — this really wants to be four PRs against the maintainer (m.muenz), landing in order: forwarders → reverse zones → DNSBL → watcher. Nothing below is meant as discouragement; the watcher's reconcile design in particular is thoughtful. But there are a few things that will break installs.

Likely to break things

1. Link-local addresses will be emitted into listen-on-v6. bind_generate_listen_config() copies core's Unbound loop verbatim (compare unbound.inc lines 245–256), but interfaces_addresses() keys link-local entries as fe80::1%igb0 — the array key includes the %device scope. Unbound accepts that syntax; BIND does not. Since practically every up interface has a link-local, selecting an interface will produce a named.conf that named-checkconf rejects and named won't start. Skip entries where $info['scope'] is set, or strip at %.

2. Patching named.conf after template render is fail-open. The template emits # bind-listener-directives and PHP string-replaces it. named.conf is a configd template target, so any plain configctl template reload OPNsense/Bind puts the comment back — and because it's a comment, named starts fine with BIND's default of listen-on { any; }. An admin who restricted BIND to LAN silently ends up answering on WAN. dnsblMemoryRecovery.sh does exactly this (template reload then named start, no re-patch). Fix: put include "/usr/local/etc/namedb/listen.conf"; inside the options {} block in the template and have PHP write only that file — the same separate-include pattern core Unbound uses. (I assume the reason you moved off named.conf.d/bind-listen.conf is that it's included at top level and listen-on must be inside options; an explicit include inside the block solves that.)

3. configctl bind restart fails when named is already stopped. The action is bindStop.py && pluginctl -c bind_start. bindStop.py returns 1 if rc.d/named stop fails, and FreeBSD's rc returns 1 for "not running" — so the && short-circuits and named never starts. Don't gate the start on the stop's exit status.

4. update-policy leaks onto every zone. The condition is allowrndcupdate == "1" or helpers.exists('...watcher.mappings.mapping') — one watcher mapping anywhere gives every primary and reverse zone an update-policy block. That converts all of them into dynamic zones (journals, freeze/thaw needed for manual edits), which is presumably why you now delete journals on stop. Also (domain.type == 'reverse' and not w_reverse) grants a mapping's key zonesub ANY on all reverse zones. Scope it to zones actually targeted by a mapping, and constrain reverse grants to zones whose network intersects the mapping's lease scope.

5. TSIG secrets land in a world-readable file. /usr/local/etc/bind/dhcpwatcher.conf is a configd template target, so it's written 0644 with the raw tsigkey_secret. Please chmod 600 / root-own it (in setup.sh or bindStart.sh). Worth double-checking named.conf too now that user TSIG keys go in there alongside the existing rndc key.

6. DoT verification isn't actually enforced. tlshostname is Required=N, and a tls block with only ca-file validates the chain but not the peer name — yet the changelog says "DNS-over-TLS (DoT) forwarders with TLS hostname verification." Make it required, or reword. Also /usr/local/etc/ssl/cert.pem is hardcoded; worth confirming that path on OPNsense rather than /etc/ssl/cert.pem.

7. The memory guard is likely to misfire, and it rewrites the user's config. Two separate concerns:

vm.stats.vm.v_free_count excludes inactive/laundry/cache pages, which are readily reclaimable. A healthy FreeBSD box routinely sits with a low free count, so a 300 MB floor will trip on machines that were never short of memory. Include inactive (and laundry) or use a different signal.
On tripping, dnsblDisableOnMemoryGuard.php calls write_config() from a background process to flip dnsbl.enabled off permanently. That's surprising behaviour (a watchdog silently editing config), it uses the legacy $config array rather than the model API, and it can race a concurrent GUI save. You already have a status file — consider skipping the RPZ zone for this boot only and surfacing a warning, leaving config alone.

8. DNSBL shortcodes have no migration. Dnsbl.xml drops its OptionValues and Dnsbl::init() injects them from core's Unbound.xml. Any code an upgrading user has selected that no longer exists upstream (the dead hpHosts entries, for instance) becomes an invalid OptionField value — validation errors on the next save plus "unknown shortcode" in the log. Add a migration that prunes unknown codes. Also, init() runs simplexml_load_file on every instantiation of the model; worth caching.

Design and convention

Cross-plugin coupling into core internals. Two hardcoded reads: dnsbl.py regexes URLs out of /usr/local/opnsense/service/templates/OPNsense/Unbound/core/blocklists.conf, and Dnsbl::init() parses Unbound.xml. Reformatting either file in core silently breaks BIND's DNSBL. The maintainable path is a small core PR that factors the shared blocklist catalogue into one consumable location, then depend on that.

bindStop.py parses /conf/config.xml directly with ElementTree, and bind_config() guesses among four possible paths. You already render the authoritative zone list into dhcpwatcher.conf and read it in watcher_zones() — make that the single source of truth and drop model_zones().

Migrations. M1_0_13 hand-generates UUIDv4s and hand-builds <dns> rows in another model's subtree from General's migration; the comment itself flags how awkward this is. Move it to a Forwarder migration and use `
𝑚
𝑜
𝑑
𝑒
𝑙
−
>
𝑓
𝑜
𝑟
𝑤
𝑎
𝑟
𝑑
𝑒
𝑟
𝑠
−
>
𝑑
𝑛
𝑠
−
>
𝑎
𝑑
𝑑
(
)
‘
+
𝑡
ℎ
𝑒
𝑛
𝑜
𝑟
𝑚
𝑎
𝑙
𝑠
𝑒
𝑟
𝑖
𝑎
𝑙
𝑖
𝑧
𝑒
𝑝
𝑎
𝑡
ℎ
.
𝑀
𝑖
𝑛
𝑜
𝑟
𝑏
𝑢
𝑔
𝑖
𝑛
𝑡
ℎ
𝑒
𝑠
𝑎
𝑚
𝑒
𝑓
𝑖
𝑙
𝑒
:
‘
𝑠
𝑡
𝑟
𝑝
𝑜
𝑠
(
model−>forwarders−>dns−>add()‘+thenormalserializepath.Minorbuginthesamefile:‘strpos(token, ':') !== false` treats a bare 2001:db8::1 as ip:port and logs a spurious "no valid port found" warning — test with filter_var(..., FILTER_VALIDATE_IP) first.

BindInterfaceField duplicates core's InterfaceField, which already handles this (and is what Unbound uses). If you keep it: $config->openvpn->children() ?? [] doesn't protect against openvpn being absent — ?? fires after the method call, so that's a fatal on a box with no OpenVPN config.

Validation placement. hasForwarderWhenForwardOnly() in reconfigureAction() lets the config save in a broken state and then fails at apply; it belongs in General::performValidation(). It also reads raw Config instead of the Forwarder model. In Domain::performValidation(), the message references should be $domain->__reference . '.domainname' (see www/caddy's model for the house style) — getInternalXMLTagName() returns a bare domainname and won't highlight the field. And since setBaseHook() runs before validateAndSave(), you can keep domainname as Required=Y rather than relaxing it in XML and re-adding the check in PHP.

Reconfigure is much heavier than before. bind_configure_do() is now stop → template reload → start on every dns event (which fires for plenty of unrelated changes), and bindStop.py also wipes journals and deletes the watcher state file. Every such event drops all dynamic records and rebuilds them. Consider keeping a reload/restart fast path and only doing the journal dance when the zone files actually changed. Relatedly, the [start] and [restart] actions now call pluginctl -c bind_start, which calls configd_run('bind stop') back into configd — re-entrancy that's worth avoiding.

Small but real:

bindStart.sh backgrounds the guard with & but inherits stdout, so a configd action can block on the pipe for up to 90 s. Redirect to /dev/null.
namedMemoryGuard.py forks sleep/pgrep/sysctl/ps ~900 times per named start. time.sleep() and fewer subprocesses would be much lighter. Its SIGKILL fallback also checks pgrep -o named, which may be a different pid than the one it's killing.
dhcplease_watcher.py registers the SIGTERM handler before Daemonize(...).start(); daemonize installs its own SIGTERM handler, so shutdown_flag likely never gets set and the graceful path never runs. Register inside run_watcher().
Watcher._check_health() exits when named goes away, and nothing brings the watcher back if named is restarted outside the plugin. bind_cron() returning [] is dead code — either drop it or use it for a health check.
_reconcile(replay=True) skips the delete branch, so if a host's DHCP hostname changed while the watcher was down, the old A record is orphaned indefinitely. Attempt the delete when _state_matches() is false, even on replay.
IscDhcpSource.fetch_all() calls core's private watchers.dhcpd.DHCPDLease._open().
dnsbl.py has no download size cap and holds every domain in a set — an unbounded list can balloon RSS on a small appliance, which is the very thing the guard exists for.
searchReverseDomainAction() calls configdRun('interface list ifconfig') once per row via getInterfaceFromSubnet(). Fetch it once. networkAddress() is also copy-pasted into two controllers — put it on the Domain model.
config.py: delimiters=('=') is a string, not a tuple; works by accident, should be ('=',).
Watcher.xml is <version>1.1.7</version> for a brand-new model with no migrations — probably should be 1.0.0.
The mapping→lease-scope match in dhcpwatcher.conf depends on the DHCP scope's domain option exactly equalling the zone name; if it doesn't, the section is omitted entirely and the watcher just logs "no enabled watcher mappings" and exits. Silent failure with no GUI signal — worth a model constraint or a status readout.

Tests. ~1,730 lines under src/opnsense/scripts/OPNsense/Bind/tests/ — there's no precedent for that in this repo (I checked: zero tests/ dirs across all plugins), and everything under src/ gets packaged onto the firewall. Move them somewhere that isn't installed. Also, several (test_listener_lifecycle.py, test_reverse_zones.py) assert literal substrings of the very files they accompany, including the # bind-listener-directives marker — they encode the implementation rather than testing behaviour and will break on any refactor. The watcher/DNSBL/state tests are the ones worth keeping.

Version bump. PLUGIN_VERSION 1.34→1.36 plus PLUGIN_REVISION=1 — on a version bump the revision line is normally dropped, and version bumps are usually the maintainer's call rather than part of a feature PR.

Worth keeping as-is

zoneCheck.sh gained argument validation and -j journal handling — a straight improvement. StateManager.save() is properly atomic with 0600 perms. is_valid_hostname() gates DHCP-supplied hostnames before they reach nsupdate, which closes the obvious injection path. Passing the TSIG key via a 0600 temp keyfile rather than argv is the right call. And DoT forwarding is a genuine gap in the plugin.

Two caveats on my end: I couldn't run this against a live BIND, so items 1, 3 and 4 are reasoned from the config semantics and want a quick named-checkconf on a box with IPv6 enabled. Want me to write this up as a comment-ready markdown file, or narrow it down to just the blocking items?

@maloo1

maloo1 commented Jul 29, 2026

Copy link
Copy Markdown

Thanks @mimugmail - I have created a pull request in #5588 - I think its pretty simple and i have tested it on my setup and is working well.
let me know if you need me to change anything:)

Thanks again

@bryanwieg

Copy link
Copy Markdown
Author

I agree. I got carried away working for parity with unbound. And the PR is too large.

I haven’t updated comments yet as I continue to run it and look for bugs. One of, if not the last, thing I need to do is doublecheck migration scenarios.

I had just started to write last night actually, I know it is large. Right now the PR is split into eight commits per future change. But I wanted feedback about splitting up the PR, or just a different approach in general.

I don’t know. You guys let me know what you would like. I’ve been running it for about a week or two. It’s doing what I need and it nearly completely replaces unbound.

@bryanwieg

Copy link
Copy Markdown
Author

@maloo1 you may want to verify at home that the DOT forwarders work. There’s a bug report in BIND about a regression introduced recently breaking DOT.

At home I reverted to an earlier version of bind for it to work. I’m at the doctor this morning, so I’ll have to get you details later.

@bryanwieg

Copy link
Copy Markdown
Author

One option is that we pull out the other commits and leave this one focusing on the DOT.

@bryanwieg
bryanwieg force-pushed the dns-bind-forwarders-dot branch from 6bafb48 to 0a52363 Compare July 29, 2026 16:39
@bryanwieg bryanwieg changed the title wip: Dns bind forwarders dot dns/bind: add DNS-over-TLS forwarder configuration Jul 29, 2026
@bryanwieg
bryanwieg force-pushed the dns-bind-forwarders-dot branch from ae9890d to 99d94e4 Compare July 29, 2026 17:21
@bryanwieg

bryanwieg commented Jul 29, 2026

Copy link
Copy Markdown
Author

@mimugmail Thanks for the forwarder-specific observations. I have narrowed this PR to a single DoT/forwarder commit and addressed the two items below.

  • Verification wording and CA path: tlshostname remains optional. I confirmed that /usr/local/etc/ssl/cert.pem exists on OPNsense (and /etc/ssl/cert.pem does not). The UI and PR description now state the actual BIND behavior: with a TLS hostname, the certificate is verified against that hostname using the system CA bundle; without one, BIND verifies the certificate against the configured forwarder IP address, which must be in the certificate's IP Subject Alternative Name. It no longer calls this opportunistic TLS.
  • Migration ownership and IPv6: the old General M1_0_13 migration was removed. The new Forwarder M1_0_1 migration uses $model->forwarders->dns->add() so UUID/default handling and serialization stay within the target model. It first recognizes a complete IP literal with filter_var(..., FILTER_VALIDATE_IP), so a bare IPv6 address is not treated as ip:port or logged as an invalid port.

I also performed a migration check: restored the pre-development plugin, saved two legacy CSV forwarders, deployed this branch, and ran the normal model migration. Both entries became enabled plain-DNS rows with framework UUIDv4s, the legacy CSV was cleared, and the rendered named.conf passed named-checkconf.

@bryanwieg
bryanwieg marked this pull request as ready for review July 29, 2026 17:24
@bryanwieg
bryanwieg force-pushed the dns-bind-forwarders-dot branch from 99d94e4 to 90b5c21 Compare July 29, 2026 17:26
@maloo1

maloo1 commented Jul 29, 2026

Copy link
Copy Markdown

@maloo1 you may want to verify at home that the DOT forwarders work. There’s a bug report in BIND about a regression introduced recently breaking DOT.

At home I reverted to an earlier version of bind for it to work. I’m at the doctor this morning, so I’ll have to get you details later.

DoT is working fine for me querying Cloudflare with the version of bind that comes with opnsense: v26.1.11_10.

To configure DoT you need to add the following config to /usr/local/etc/namedb/named.conf.d/DoT_forwarders.conf:
tls Cloudflare-DoT { ca-file "/usr/local/etc/namedb/named.conf.d/ca-bundle.trust.crt"; remote-hostname "one.one.one.one"; };

I have also copied in ca trusts to this folder although there is probably a better way to use the ca trusts already within opnsense.

The last thing that is needed (and hence my PR #5588 ) is to add custom forwarders with the following syntax to the bind options:
forwarders port 853 tls Cloudflare-DoT { 1.1.1.2; 1.0.0.2; };

IMO I think both PRs are fine - being able to add TLS forwarders from the GUI would be great, and allowing custom bind options for edge cases would mean we don't need to write code for every bind option and can just let people manage the edge cases themselves.

I guess I am just giving a simpler path to achieve DoT and also other custom config (There still seems to be a lot of changes for something that I would have thought would be simple - but I do find that with AI unfortunately).

@bryanwieg

Copy link
Copy Markdown
Author

Good point on the opnsense version, I am slightly behind you, which may explain why your BIND DoT is working.

Also, yes, I'm attempting to add a lot of work to get this stuff in the gui. In particular, like how unbound is set up in the gui.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

dns/bind: add DNS-over-TLS forwarder support and sync DNSBL from Unbound

3 participants