feat(net): resolve device .local names, asking avahi first - #728
Conversation
HuggeK
changed the title
|
For the record: #746 is the alternative to this PR — the same The trade-off between them is not really the resolver, it is reach vs. agreement: this PR's avahi path makes |
483a5b8 to
9c0ce14
Compare
@ Go hands a ".local" name to libc only when cgo is available, and FTW builds CGO_ENABLED=0, so a configured zap.local became a unicast DNS query to the site router and failed. Shipping libnss-mdns does not change that: it changes what getent and curl resolve inside the image, not what this process resolves. Ask the machine that already knows. avahi-daemon answers over its simple-protocol socket -- one line out, one line back, and no DNS wire format decoded on that path. It is the same daemon over the same socket that libnss_mdns4_minimal.so.2 talks to, so FTW and an operator running `getent hosts zap.local` in the container cannot disagree about an address. Keep a direct query for where that socket cannot be reached. It has to be bind-mounted, and under the Home Assistant Supervisor an add-on cannot mount arbitrary host paths at all, so a resolver that required it would simply not work in the add-on FTW ships as. The lookup log says which backend answered. Wire every driver transport through it: Modbus TCP, MQTT for both the driver and the Home Assistant bridge, HTTP including the TLS-pinned client, WebSocket and raw TCP. Resolution runs per dial, which is what makes a name survive a DHCP move -- the reconnect path rebuilds from the configured address. Co-authored-by: HuggeK <48095810+HuggeK@users.noreply.github.com> @
9c0ce14 to
0f5635b
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0f5635b203
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Two fixes, both from evidence rather than reasoning.
A pilot Home Assistant install resolves ".local" today, on stock
v1.10.0-beta.1, with none of this code: Supervisor points every container at
its own CoreDNS, which carries an mdns plugin backed by systemd-resolved.
The failure text names it outright —
lookup ftw-no-such-device-xyz.local on 172.30.32.3:53: no such host
— and a probe against the real device returned its serial over that path
while the configured IP gave "no route to host". Returning the mDNS error
instead of trying the system resolver would therefore take an install that
works and break it. The dialer now falls through, and reports both causes so
neither hides the other.
Second, a truncated record after a valid one no longer keeps the addresses
read before it. `break parse` left them in place and handed them to
finishAnswer, so a malformed UDP response could populate the cache — the
opposite of what the comment above the loop claimed. Reported by Codex on
this PR.
Co-authored-by: HuggeK <48095810+HuggeK@users.noreply.github.com>
Tested on a real Home Assistant install, and it changed two things hereA pilot HA green (
Two conclusions, both load-bearing for this PR. 1. So the earlier claim in this PR that the add-on has no 2. As written, this PR would have broken that install. Also in that commit: the Codex P2 above. A truncated record after a valid one Third probe as a control — why the middle row is conclusiveThe serial One thing left, and it needs a decision rather than a patch
I have deliberately not touched that gate, because it was added on purpose Your call; I'd rather flag it than quietly widen a security gate. |
Tested on a pilot HA green instead of reasoning about it, and the earlier text was wrong in the way that matters. `.local` does not need anything from FTW here: Supervisor points every app at its own CoreDNS, which carries an mdns plugin backed by systemd-resolved, and that answers over ordinary unicast DNS. A lookup failure names it — "on 172.30.32.3:53". The avahi paragraph stands but for a further reason: HA OS runs no avahi at all. Its multicast plugin is mdns-repeater, a packet relay with no query API, so there is no socket to bind even in principle. Evidence: srcfl/ftw#728 (comment). Co-authored-by: HuggeK <48095810+HuggeK@users.noreply.github.com>
allow_unverified_local refused the dial outright. That broke two things that matter more than the risk it was guarding. srcfl#714 makes the setup wizard write `host: zap.local`, and nothing in web/ sets this flag — so every device installed by the wizard would have been refused on its first poll, usually the site meter, which stops dispatch. And a pilot Home Assistant install resolves .local today through Supervisor's DNS service, on stock v1.10.0-beta.1, with none of this code: refusing would take a working site and break it. The risk it guarded is also not one-sided. A raw IP is no more an identity on a LAN than a name — it can be claimed by ARP, and DHCP can hand it to another device with nobody attacking anything. Only the name was gated, and the fallback the gate pushed operators towards is the option with the extra failure mode. So the flag now gates what its own doc comment always said: whether FTW may use *its own* mDNS answer. Without it the name goes to the system resolver, exactly as before this package existed. With it, avahi and the LAN are queried too. Both causes are wrapped so errors.Is finds either. guardMDNSProxy goes with it. Where a proxy is configured FTW never resolves the destination — the proxy does — so there was nothing for it to gate, and its tests asserted a contract that no longer holds. The check that would actually help is on identity after connect: record make+serial or MAC on first success, fault when it changes. FTW already collects all three (state.ResolveDeviceID) and never compares them. Filed separately rather than smuggled in here. Co-authored-by: HuggeK <48095810+HuggeK@users.noreply.github.com>
Fixed: the gate now covers the resolver, not the connectionFollowing the mismatch flagged on #714 and the LAN-risk point that came with it, Why, in one line each:
Verified against a real Sourceful Zap:
On that host Tests changed with the contract: the deny test becomes "without the opt-in our own answer is ignored but the name still resolves", plus its opposite, and both proxy tests now assert a Not included, and it is the check that would actually help: verifying device identity after connect. |
Reverse lookups asked unicast DNS first and mDNS only as a fallback, so on any network whose router answers PTR the scan returned the router's label for the lease and never looked for a .local name. The setup wizard then fell back to the raw IP, which is the outcome it exists to avoid. Both queries now run together and a .local answer wins. That alone is still not enough: RFC 6762 leaves in-addr.arpa mapping optional and plenty of responders publish a forward A record without one. A Sourceful Zap answers zap.local all day and returns nothing for 141.1.168.192.in-addr.arpa. So where no reverse record exists, the label is re-asked forward as <label>.local and kept only when it resolves back to the same address -- the check is what keeps the router's naming from deciding what FTW dials. Co-authored-by: HuggeK <48095810+HuggeK@users.noreply.github.com>
Verified on a real device: the wizard was not choosing the mDNS nameAsked to confirm this PR does what it says, I checked against a live Sourceful
Measured against
Note rows 2 and 3 together: the device answers to a The fixIn #728, since it needs the forward resolver: 4d746ddd Both queries now run concurrently and a After the fix, same address, same code path: Why the round-trip check is there rather than trusting the labelWithout it the router's naming would decide what FTW dials: take whatever It also costs nothing when the device does publish a reverse mDNS record — that Scan latencyUnchanged in the common case and bounded in the worst. The two reverse queries Tests are in |
* build: follow Core and Optimizer onto the trixie base @ srcfl/ftw#731 moves the whole FTW stack onto Debian 13 "trixie". This repo inherits that automatically -- the add-on image is built FROM the Optimizer image -- so the change that matters here is the test fixtures, which stood in for Core and Optimizer on bookworm and would have gone on testing a base that no longer ships. Also say, once, why libnss-mdns is not installed here even though Core has it: it only forwards to avahi-daemon over a Unix socket, and Supervisor gives an add-on no way to bind an arbitrary host path. FTW resolves ".local" in-process instead, which is what host_network is for. Documented in DOCS.md for operators who hit a name that will not resolve. Co-authored-by: HuggeK <48095810+HuggeK@users.noreply.github.com> @ * docs: correct how `.local` resolves under Supervisor Tested on a pilot HA green instead of reasoning about it, and the earlier text was wrong in the way that matters. `.local` does not need anything from FTW here: Supervisor points every app at its own CoreDNS, which carries an mdns plugin backed by systemd-resolved, and that answers over ordinary unicast DNS. A lookup failure names it — "on 172.30.32.3:53". The avahi paragraph stands but for a further reason: HA OS runs no avahi at all. Its multicast plugin is mdns-repeater, a packet relay with no query API, so there is no socket to bind even in principle. Evidence: srcfl/ftw#728 (comment). Co-authored-by: HuggeK <48095810+HuggeK@users.noreply.github.com> * docs: say what `.local` resolution depends on, and what it does not Someone reading the previous version could reasonably ask which plugin they have to install. The answer is none, and that is worth stating rather than leaving implied. Supervisor hard-codes five built-in services and starts them itself (supervisor/plugins/manager.py: cli, dns, audio, observer, multicast). They are not add-ons — an app cannot install one, and cannot declare a dependency on one either. A failure there becomes a repairable system issue Supervisor raises on its own. The one real host requirement is systemd-resolved, which HA OS always has and which the Supervised installer's first step sets up. A host that skipped it resolves no .local names, so that case is called out with its workaround. Also documents the resolver address in the failure message, since seeing 172.30.32.3 is what separates "device is not there" from "resolution never got that far". Co-authored-by: HuggeK <48095810+HuggeK@users.noreply.github.com> * docs: qualify local-name network failures Signed-off-by: Fredrik Ahlgren <fredrik@sourceful-labs.com> --------- Signed-off-by: Fredrik Ahlgren <fredrik@sourceful-labs.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Fredrik Ahlgren <fredrik@sourceful-labs.com>
Co-authored-by: HuggeK <48095810+HuggeK@users.noreply.github.com>
3 participants
Note
There is no longer an alternative route. #746 offered one (Alpine base,
dependency-free Go resolver) and is now closed, because #731 merged.
#731 is already merged. This PR is based on
masterat980b247edb20f6b5b48fd217899efa3289d873e4. Since the previous review, thebase gained only #789's notifications-test synchronization; the four #728
commits are unchanged. The current head is
0f5635b2032daec33d6e2e2562a63c89e34a6548. Review the full PR diff.Important
#714 remains separate. It changes the setup wizard's device binding;
this PR supplies the resolver and transport policy needed for configured
.localnames. #714 is not changed here.What & why
FTW builds with
CGO_ENABLED=0, so the normal Go resolver cannot use the host'smDNS NSS module. A device configured as
zap.localtherefore needs an explicitmDNS path before FTW can connect to it.
Resolver and transport
The resolver asks Avahi first and uses a validated direct multicast query as a
fallback. Modbus TCP, MQTT (driver and Home Assistant bridge), HTTP, WebSocket,
and raw TCP use the shared resolver. Parser checks validate the DNS response,
source, answer name/type/class/family, and Avahi's interface and address fields.
IPv4/IPv6, interface, and IPv6 zone handling fail closed.
mDNS does not prove server identity.
capabilities.allow_unverified_localdefaults to
falseand must be enabled per driver before a.localtransportcan be used. The Home Assistant bridge has its own equivalent setting. A name
allow-list does not count as server identity.
HTTP and WebSocket transports check the original destination before selecting an
HTTP proxy, covering GET, POST, PATCH, credentials, and command payloads. A
TLS certificate pin does not bypass the
.localopt-in: all mDNS transportsuse the same default-deny rule. Literal IPs and ordinary DNS names keep their
existing proxy behavior.
Compose
docker-compose.ymlcarries the Avahi socket mount commented out.docs/operations.mddescribes when to enable it. Direct multicast is suitablefor host-networked Linux and Home Assistant add-on deployments; bridged macOS
deployments should use an IP address.
Verification
go test -race ./internal/mdnsresolve ./internal/drivers ./internal/config ./internal/configreload ./internal/modbus ./internal/mqtt ./internal/hamake verify-allopt-in, and ordinary-host proxy behavior
0f5635b2032daec33d6e2e2562a63c89e34a6548passed:test,
changeset,
repo hygiene,
brand cleanup, and
RPi installer
The fixed Home Link uplink endpoint also uses an environment proxy, but it is a
fixed ordinary-DNS endpoint and does not resolve driver
.localnames; it isoutside this policy change.
Not in scope
internal/scanner's reverse mDNS lookup remains unchanged. It serves discoveryrather than driver dialing and has its own tests.