From e3ff8759acb622df8d538896e3ab5ccc853c3253 Mon Sep 17 00:00:00 2001 From: Claude Opus 5 Date: Fri, 31 Jul 2026 14:17:41 +0200 Subject: [PATCH 1/7] feat(net): resolve device .local names, asking avahi first @ 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> @ --- .changeset/resolve-local-device-names.md | 37 +++ config.example.yaml | 2 +- docker-compose.yml | 15 + docs/operations.md | 59 ++-- docs/sourceful-zap.md | 9 + go/internal/drivers/lua.go | 17 +- go/internal/drivers/tcp_cap.go | 4 +- go/internal/drivers/ws_cap.go | 4 + go/internal/ha/bridge.go | 10 + go/internal/mdnsresolve/avahi.go | 127 +++++++++ go/internal/mdnsresolve/avahi_test.go | 214 +++++++++++++++ go/internal/mdnsresolve/mdnsresolve.go | 231 ++++++++++++++++ go/internal/mdnsresolve/mdnsresolve_test.go | 290 ++++++++++++++++++++ go/internal/mdnsresolve/multicast.go | 161 +++++++++++ go/internal/modbus/tcp_client.go | 9 +- go/internal/mqtt/client.go | 11 + 16 files changed, 1178 insertions(+), 22 deletions(-) create mode 100644 .changeset/resolve-local-device-names.md create mode 100644 go/internal/mdnsresolve/avahi.go create mode 100644 go/internal/mdnsresolve/avahi_test.go create mode 100644 go/internal/mdnsresolve/mdnsresolve.go create mode 100644 go/internal/mdnsresolve/mdnsresolve_test.go create mode 100644 go/internal/mdnsresolve/multicast.go diff --git a/.changeset/resolve-local-device-names.md b/.changeset/resolve-local-device-names.md new file mode 100644 index 00000000..3f496148 --- /dev/null +++ b/.changeset/resolve-local-device-names.md @@ -0,0 +1,37 @@ +--- +"ftw": minor +--- + +Resolve device `.local` names, so devices can be configured by name instead of a DHCP-assigned IP. + +Go never resolves `.local` itself: it hands those names to libc only when cgo is +available, and FTW builds with `CGO_ENABLED=0`, so a configured `zap.local` +became a unicast DNS query to the site router and failed. That is true on every +base image and every libc — shipping `libnss-mdns` changes what `getent` and +`curl` resolve inside the container, not what this process resolves. + +FTW now asks the host's own mDNS responder, `avahi-daemon`, over its +simple-protocol socket — the same daemon and the same socket +`libnss_mdns4_minimal.so.2` uses, so a name resolves identically whether FTW +dials it or an operator checks it from a shell in the container. Where that +socket cannot be reached, FTW queries the LAN directly instead. The socket has +to be bind-mounted, and under the Home Assistant Supervisor an add-on cannot +mount arbitrary host paths at all, so the direct path is what makes the feature +work there; it is also the default in Compose, where mounting a host runtime +directory is left to the operator. A successful lookup logs which one answered. + +Every driver transport uses it — Modbus TCP, MQTT (driver and Home Assistant +bridge), HTTP including TLS-pinned clients, WebSocket and raw TCP. + +Resolution happens per dial rather than once at startup, so a device that moves +to a new DHCP lease is found again on the next reconnect without a config edit. +Answers are cached (30–120 s, following the record TTL where there is one) so +reconnect loops do not flood the LAN, and failures are cached briefly so a +device that is still booting is retried soon. A failed resolution logs +`mDNS resolution failed` and names the mechanism, instead of surfacing as a +generic dial error. + +Only `.local` names take this path; literal IPs and ordinary DNS names dial +exactly as before. Multicast still has to reach the LAN, which the Linux +Compose topology has via `network_mode: host`. Under `docker-compose.macos.yml` +the container is bridged, so configure devices by IP there. diff --git a/config.example.yaml b/config.example.yaml index 0999020f..ff047809 100644 --- a/config.example.yaml +++ b/config.example.yaml @@ -91,7 +91,7 @@ drivers: # battery_telemetry_only: true # capabilities: # http: - # allowed_hosts: ["zap.local"] # use the LAN IP if mDNS is unavailable + # allowed_hosts: ["zap.local"] # .local is resolved by FTW over mDNS # config: # host: zap.local # # meter_serial: p1m-... # optional; P1 is auto-selected diff --git a/docker-compose.yml b/docker-compose.yml index 414b4fab..a4c75a2e 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -73,6 +73,21 @@ services: # here to render update progress in the UI; it never writes to it. - update-ipc:/run/ftw-update - optimizer-ipc:/run/ftw-optimizer + # OPTIONAL — avahi-daemon's runtime directory. Host networking shares + # ports, not Unix sockets, so this is the only way in. + # + # With it, FTW asks the host's mDNS responder to resolve `.local` device + # names (and `getent hosts zap.local` works inside the container, via + # libnss-mdns). Without it FTW queries the LAN itself, which needs no + # host software and is why this stays commented out by default. + # + # Uncomment only if the host runs avahi-daemon — the Raspberry Pi image + # does. Mount the DIRECTORY, not the socket inside it: if the path is + # missing Docker creates it, and an empty directory there is harmless, + # whereas a directory created where the *socket* belongs stops + # avahi-daemon from ever starting. Restarting avahi after the container + # detaches the mount, so restart FTW too if you do. + # - /run/avahi-daemon:/run/avahi-daemon:ro ftw-optimizer: image: ghcr.io/srcfl/ftw-optimizer:${FTW_OPTIMIZER_IMAGE_TAG:-latest} diff --git a/docs/operations.md b/docs/operations.md index 32221328..e6963223 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -180,30 +180,57 @@ Verify the broker address from the same network namespace as core, then inspect broker and driver logs. Device credentials and topic mappings belong to the driver configuration. -### `.local` names inside the container +### A device `.local` name does not resolve -The image ships `libnss-mdns`, so ordinary glibc tools inside the container — -`getent hosts zap.local` and `wget` — resolve `.local` the way they do on -the host. `apt` wires `mdns4_minimal [NOTFOUND=return]` into -`/etc/nsswitch.conf` when the package is installed; nothing else is needed at -build time. +FTW resolves `.local` device names itself; the OS resolver is not involved, +because a `CGO_ENABLED=0` Go binary never consults NSS. There are two places an +answer can come from, and the log line for a successful lookup says which: -At run time that path talks to `avahi-daemon` over a Unix socket, and a socket -is not shared by host networking the way a port is. Mount it explicitly: +``` +resolved host over mDNS host=zap.local addr=192.168.1.42 via=avahi +``` + +**`via=avahi`** — the host's `avahi-daemon` answered over its socket. This is +preferred when available: avahi already holds a record cache, and it is the +same daemon that `getent hosts zap.local` inside the container goes through, so +FTW and your shell cannot disagree about an address. + +**`via=multicast`** — FTW queried the LAN directly. This is what happens when +the avahi socket is not mounted, which is the default, and it needs no host +software at all. + +Failures log `mDNS resolution failed`, and the message names both backends when +both were tried. + +Either way multicast has to reach the LAN, which the Linux Compose topology +provides through `network_mode: host`. Under `docker-compose.macos.yml` the +container is bridged and multicast does not reach the LAN, so configure devices +by IP there. + +#### Letting FTW use avahi + +Host networking shares ports, not Unix sockets, so avahi has to be bind-mounted +in. `docker-compose.yml` carries the line commented out: ```yaml volumes: - - /run/avahi-daemon/socket:/run/avahi-daemon/socket:ro + - /run/avahi-daemon:/run/avahi-daemon:ro ``` -Only add this on a host that actually runs `avahi-daemon` — the Raspberry Pi -image does. Without the daemon Docker creates a *directory* at that path, which -resolves nothing and is harmless but confusing; `ls -l` there is the quickest -way to tell the two apart. +Mount the *directory*, not the socket file inside it. If the host path is +missing Docker creates it, and an empty directory is harmless — whereas a +directory created where the socket belongs stops `avahi-daemon` from ever +starting. Restarting avahi detaches the mount, so restart FTW after you do. + +This is an optimisation, not a requirement: device connectivity is unchanged +without it. What it does add is `libnss-mdns` working for ordinary tools in the +image — `getent hosts zap.local`, `curl`, `wget` — which is the quickest way to +check a name from inside the container. -This makes the container's own tooling agree with the host. Whether the FTW -process itself resolves a device's `.local` name is a separate question, -answered by `internal/mdnsresolve`. +Under the Home Assistant add-on none of this applies: Supervisor mounts only a +fixed set of named paths, so the socket cannot be provided and FTW always +queries the LAN directly. The add-on runs with `host_network: true`, which is +what makes that work. ### Configuration rejected diff --git a/docs/sourceful-zap.md b/docs/sourceful-zap.md index 75f98d60..30121f2c 100644 --- a/docs/sourceful-zap.md +++ b/docs/sourceful-zap.md @@ -76,6 +76,15 @@ go test ./internal/drivers -run 'Zap|zap' - not found: confirm Zap is on Wi-Fi and reachable at `http://zap.local/api/system` from the FTW host; +- `.local` name does not resolve: FTW resolves `.local` itself — via the host's + `avahi-daemon` where its socket is mounted, otherwise by querying the LAN — + rather than through the OS resolver, so it needs to be on the same L2 segment + as the device. That is the case with the Linux Compose topology + (`network_mode: host`); under `docker-compose.macos.yml` the container is + bridged and multicast does not reach the LAN, so configure the device by IP + there. The log line naming the failure is `mDNS resolution failed`, and a + successful lookup logs `via=avahi` or `via=multicast`; see + [docs/operations.md](operations.md); - no meter: inspect Zap's `/api/devices` and pin `meter_serial` when needed; - duplicate PV/battery: disable the overlapping Zap DER; - visible battery is not controlled: expected for the telemetry-only driver. diff --git a/go/internal/drivers/lua.go b/go/internal/drivers/lua.go index 25286fef..2a8ec2cd 100644 --- a/go/internal/drivers/lua.go +++ b/go/internal/drivers/lua.go @@ -69,6 +69,8 @@ import ( "time" lua "github.com/yuin/gopher-lua" + + "github.com/srcfl/ftw/go/internal/mdnsresolve" ) // LuaDriver wraps a running Lua VM bound to a HostEnv. @@ -1156,8 +1158,16 @@ func registerHost(L *lua.LState, env *HostEnv) { return false, fmt.Sprintf("host %q (port %s) not in allowed_hosts", host, port) } + // Drivers routinely address a device by its ".local" name, which the + // stdlib resolver cannot answer. Clone the default transport so proxying, + // HTTP/2 and connection pooling are all unchanged — only the dial step + // differs, and only for ".local" hosts. + transport := net_http.DefaultTransport.(*net_http.Transport).Clone() + transport.DialContext = mdnsresolve.DialContext + httpClient := &net_http.Client{ - Timeout: 15 * time.Second, + Timeout: 15 * time.Second, + Transport: transport, CheckRedirect: func(req *net_http.Request, via []*net_http.Request) error { if len(via) >= 10 { return fmt.Errorf("stopped after 10 redirects") @@ -1186,7 +1196,10 @@ func registerHost(L *lua.LState, env *HostEnv) { // CA. Drivers WITHOUT a pin keep Go's default transport untouched, so // nothing about existing HTTP drivers changes. if pin := tlsPin; pin != "" { - tr := net_http.DefaultTransport.(*net_http.Transport).Clone() + // Clone the transport built above so the pinned client keeps the same + // mDNS-aware dialer — a pinned device is usually a local appliance + // addressed by its ".local" name, which is exactly the case that needs it. + tr := transport.Clone() tr.TLSClientConfig = &tls.Config{ // We replace chain/hostname verification with our own exact // fingerprint check below, so the stdlib check must be off. diff --git a/go/internal/drivers/tcp_cap.go b/go/internal/drivers/tcp_cap.go index 4153f3cc..cba42622 100644 --- a/go/internal/drivers/tcp_cap.go +++ b/go/internal/drivers/tcp_cap.go @@ -6,6 +6,8 @@ import ( "strings" "sync" "time" + + "github.com/srcfl/ftw/go/internal/mdnsresolve" ) // TCPCap is the host's raw TCP socket capability. One driver = one upstream @@ -93,7 +95,7 @@ func (n *netTCP) Open(addr string) error { return fmt.Errorf("tcp: %s", reason) } - conn, err := net.DialTimeout("tcp", addr, 10*time.Second) + conn, err := mdnsresolve.DialTimeout("tcp", addr, 10*time.Second) if err != nil { return fmt.Errorf("tcp dial: %w", err) } diff --git a/go/internal/drivers/ws_cap.go b/go/internal/drivers/ws_cap.go index 979d5685..bd23736d 100644 --- a/go/internal/drivers/ws_cap.go +++ b/go/internal/drivers/ws_cap.go @@ -9,6 +9,8 @@ import ( "time" "github.com/gorilla/websocket" + + "github.com/srcfl/ftw/go/internal/mdnsresolve" ) // gorillaWS is the production WSCap implementation. One per driver. @@ -73,6 +75,8 @@ func (g *gorillaWS) Open(url string, headers map[string]string) error { } dialer := *websocket.DefaultDialer dialer.HandshakeTimeout = 15 * time.Second + // ".local" hosts need mDNS; everything else falls through to a plain dial. + dialer.NetDialContext = mdnsresolve.DialContext if len(subprotocols) > 0 { dialer.Subprotocols = subprotocols } diff --git a/go/internal/ha/bridge.go b/go/internal/ha/bridge.go index c39e6f57..dd05fd77 100644 --- a/go/internal/ha/bridge.go +++ b/go/internal/ha/bridge.go @@ -12,6 +12,8 @@ import ( "encoding/json" "fmt" "log/slog" + "net" + "net/url" "sort" "strconv" "strings" @@ -22,6 +24,7 @@ import ( "github.com/srcfl/ftw/go/internal/config" "github.com/srcfl/ftw/go/internal/control" + "github.com/srcfl/ftw/go/internal/mdnsresolve" "github.com/srcfl/ftw/go/internal/telemetry" ) @@ -249,6 +252,13 @@ func (b *Bridge) connectAndStart(cfg *config.HomeAssistant, driverNames []string opts := paho.NewClientOptions(). AddBroker(fmt.Sprintf("tcp://%s:%d", cfg.Broker, cfg.Port)). + // A Home Assistant broker is very often reached as homeassistant.local, + // which the stdlib resolver cannot answer. See internal/mqtt for why a + // TCP-only replacement is complete here. + SetCustomOpenConnectionFn(func(uri *url.URL, o paho.ClientOptions) (net.Conn, error) { + d := mdnsresolve.Dialer{Dialer: net.Dialer{Timeout: o.ConnectTimeout}} + return d.Dial("tcp", uri.Host) + }). SetClientID("forty-two-watts-ha"). SetAutoReconnect(true). SetConnectRetry(true). diff --git a/go/internal/mdnsresolve/avahi.go b/go/internal/mdnsresolve/avahi.go new file mode 100644 index 00000000..01b281d6 --- /dev/null +++ b/go/internal/mdnsresolve/avahi.go @@ -0,0 +1,127 @@ +package mdnsresolve + +import ( + "bufio" + "context" + "fmt" + "net" + "net/netip" + "os" + "strings" +) + +// avahiSocket is avahi-daemon's simple-protocol socket. This is not a path we +// invented: libnss_mdns4_minimal.so.2 has the same string compiled into it, so +// asking here is exactly what `getent hosts foo.local` does one layer down. +// +// A var so tests can point at a socket they control. +var avahiSocket = "/run/avahi-daemon/socket" + +// avahiDial is swappable in tests. +var avahiDial = func(ctx context.Context, path string) (net.Conn, error) { + var d net.Dialer + return d.DialContext(ctx, "unix", path) +} + +// avahiTTL is how long an avahi answer is cached here. The protocol carries no +// TTL — avahi keeps its own record cache and re-queries the LAN when its copy +// ages out, so this number only decides how often we ask it, not how fresh the +// answer is. It matches the floor used for a raw multicast answer. +const avahiTTL = minTTL + +// avahiAvailable reports whether the socket is present and is really a socket. +// +// The type check earns its place: the socket has to be bind-mounted into the +// container, and when the host path does not exist Docker helpfully creates a +// *directory* at the mount point instead. That looks present to a bare +// os.Stat, connects to nothing, and is the single most likely misconfiguration +// on this path. +// A var so tests can assert both branches without needing a real socket on +// whatever platform `go test` is running on. +var avahiAvailable = func() bool { + fi, err := os.Stat(avahiSocket) + return err == nil && fi.Mode()&os.ModeSocket != 0 +} + +type avahiResult struct { + addr netip.Addr + err error +} + +// avahiLookup asks avahi-daemon to resolve name, preferring IPv4. +// +// The two address families are asked concurrently on separate connections +// because a name that exists in one family and not the other costs a full +// avahi resolve timeout to answer negatively — serialising them would put that +// wait in front of every dial to a v4-only device. IPv4 is still the answer +// preferred when both arrive. +func avahiLookup(ctx context.Context, name string) ([]netip.Addr, error) { + v4 := make(chan avahiResult, 1) + v6 := make(chan avahiResult, 1) + go func() { v6 <- avahiResolve(ctx, "RESOLVE-HOSTNAME-IPV6", name) }() + go func() { v4 <- avahiResolve(ctx, "RESOLVE-HOSTNAME-IPV4", name) }() + + r4 := <-v4 + if r4.err == nil { + return []netip.Addr{r4.addr}, nil + } + if r6 := <-v6; r6.err == nil { + return []netip.Addr{r6.addr}, nil + } + return nil, fmt.Errorf("avahi: %s: %w", name, r4.err) +} + +// avahiResolve runs one request/response exchange over the socket. +// +// The wire format is a single line each way. A success is +// +// +
+// +// and a failure is "- ". There is no framing, no length +// prefix and nothing to decode, which is the point of using it: no DNS wire +// format is parsed anywhere on this path. +func avahiResolve(ctx context.Context, command, name string) avahiResult { + conn, err := avahiDial(ctx, avahiSocket) + if err != nil { + return avahiResult{err: fmt.Errorf("connect %s: %w", avahiSocket, err)} + } + defer conn.Close() + + if deadline, ok := ctx.Deadline(); ok { + _ = conn.SetDeadline(deadline) + } + + if _, err := fmt.Fprintf(conn, "%s %s\n", command, name); err != nil { + return avahiResult{err: fmt.Errorf("write request: %w", err)} + } + + scanner := bufio.NewScanner(conn) + if !scanner.Scan() { + if err := scanner.Err(); err != nil { + return avahiResult{err: fmt.Errorf("read reply: %w", err)} + } + return avahiResult{err: fmt.Errorf("no reply")} + } + return parseAvahiReply(scanner.Text()) +} + +func parseAvahiReply(line string) avahiResult { + fields := strings.Fields(line) + if len(fields) == 0 { + return avahiResult{err: fmt.Errorf("empty reply")} + } + if fields[0] != "+" { + // "- 15 Timeout reached" and friends. Hand the daemon's own wording + // back rather than inventing one, so the log says what avahi said. + return avahiResult{err: fmt.Errorf("%s", strings.TrimSpace(strings.TrimPrefix(line, "-")))} + } + // +
+ if len(fields) < 5 { + return avahiResult{err: fmt.Errorf("short reply %q", line)} + } + addr, err := netip.ParseAddr(fields[4]) + if err != nil { + return avahiResult{err: fmt.Errorf("unparsable address %q", fields[4])} + } + return avahiResult{addr: addr.Unmap()} +} diff --git a/go/internal/mdnsresolve/avahi_test.go b/go/internal/mdnsresolve/avahi_test.go new file mode 100644 index 00000000..236b95c7 --- /dev/null +++ b/go/internal/mdnsresolve/avahi_test.go @@ -0,0 +1,214 @@ +package mdnsresolve + +import ( + "bufio" + "context" + "errors" + "io" + "net" + "net/netip" + "strings" + "testing" + "time" + + "golang.org/x/net/dns/dnsmessage" +) + +// fakeAvahi stands in for avahi-daemon. reply is handed the command and the +// name and returns the line the daemon would write back; returning "" makes it +// hang up without answering. +// +// net.Pipe rather than a real unix socket: the protocol is what is under test, +// and the test then runs the same way on every platform the suite runs on. +func fakeAvahi(t *testing.T, reply func(command, name string) string) { + t.Helper() + origAvail, origDial := avahiAvailable, avahiDial + avahiAvailable = func() bool { return true } + avahiDial = func(ctx context.Context, path string) (net.Conn, error) { + client, server := net.Pipe() + go func() { + defer server.Close() + sc := bufio.NewScanner(server) + if !sc.Scan() { + return + } + fields := strings.Fields(sc.Text()) + if len(fields) < 2 { + return + } + if out := reply(fields[0], fields[1]); out != "" { + _, _ = io.WriteString(server, out+"\n") + } + }() + return client, nil + } + t.Cleanup(func() { + avahiAvailable, avahiDial = origAvail, origDial + Flush() + }) +} + +func TestParseAvahiReply(t *testing.T) { + cases := []struct { + name string + line string + want string // empty means the reply must be rejected + }{ + // The exact shape avahi-daemon returns, confirmed against the daemon. + {"ipv4", "+ 2 0 zap.local 192.168.1.42", "192.168.1.42"}, + {"ipv6", "+ 2 1 zap.local fe80::1", "fe80::1"}, + // A v4-mapped answer must dial as plain IPv4. + {"v4 mapped", "+ 2 1 zap.local ::ffff:192.168.1.42", "192.168.1.42"}, + {"failure", "- 15 Timeout reached", ""}, + {"empty", "", ""}, + {"truncated", "+ 2 0 zap.local", ""}, + {"unparsable address", "+ 2 0 zap.local not-an-address", ""}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + got := parseAvahiReply(c.line) + if c.want == "" { + if got.err == nil { + t.Fatalf("accepted %q, got %v", c.line, got.addr) + } + return + } + if got.err != nil { + t.Fatalf("rejected %q: %v", c.line, got.err) + } + if got.addr.String() != c.want { + t.Fatalf("addr = %v, want %v", got.addr, c.want) + } + }) + } +} + +// A failure line must carry avahi's own wording, so the log says what the +// daemon said rather than something this package made up. +func TestParseAvahiReplyKeepsDaemonWording(t *testing.T) { + got := parseAvahiReply("- 15 Timeout reached") + if got.err == nil { + t.Fatal("expected an error") + } + if !strings.Contains(got.err.Error(), "Timeout reached") { + t.Fatalf("error %q dropped the daemon's message", got.err) + } +} + +func TestLookupPrefersAvahi(t *testing.T) { + Flush() + fakeAvahi(t, func(command, name string) string { + if command == "RESOLVE-HOSTNAME-IPV4" && name == "inverter.local" { + return "+ 2 0 inverter.local 192.168.1.42" + } + return "- 15 Timeout reached" + }) + + orig := listenPacket + listenPacket = func() (*net.UDPConn, error) { + t.Error("queried the LAN directly even though avahi answered") + return nil, errors.New("should not be called") + } + t.Cleanup(func() { listenPacket = orig }) + + addrs, err := Lookup(context.Background(), "inverter.local") + if err != nil { + t.Fatalf("Lookup: %v", err) + } + if len(addrs) != 1 || addrs[0] != netip.MustParseAddr("192.168.1.42") { + t.Fatalf("addrs = %v, want [192.168.1.42]", addrs) + } +} + +// IPv4 wins when both families answer, so the address a device is dialled on +// does not depend on which goroutine happened to finish first. +func TestAvahiLookupPrefersIPv4(t *testing.T) { + fakeAvahi(t, func(command, name string) string { + if command == "RESOLVE-HOSTNAME-IPV6" { + return "+ 2 1 dual.local fe80::1" + } + return "+ 2 0 dual.local 192.168.1.7" + }) + + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + addrs, err := avahiLookup(ctx, "dual.local") + if err != nil { + t.Fatalf("avahiLookup: %v", err) + } + if len(addrs) != 1 || addrs[0] != netip.MustParseAddr("192.168.1.7") { + t.Fatalf("addrs = %v, want [192.168.1.7]", addrs) + } +} + +// A device that only advertises IPv6 still resolves. +func TestAvahiLookupFallsBackToIPv6(t *testing.T) { + fakeAvahi(t, func(command, name string) string { + if command == "RESOLVE-HOSTNAME-IPV6" { + return "+ 2 1 v6only.local fe80::2" + } + return "- 15 Timeout reached" + }) + + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + addrs, err := avahiLookup(ctx, "v6only.local") + if err != nil { + t.Fatalf("avahiLookup: %v", err) + } + if len(addrs) != 1 || addrs[0] != netip.MustParseAddr("fe80::2") { + t.Fatalf("addrs = %v, want [fe80::2]", addrs) + } +} + +// A socket that is present but unhelpful — daemon starting, wedged, or simply +// without the record — must not be a dead end. +func TestLookupFallsBackToMulticastWhenAvahiFails(t *testing.T) { + Flush() + fakeAvahi(t, func(command, name string) string { return "- 15 Timeout reached" }) + startResponder(t, []dnsmessage.Resource{aResource(t, "inverter.local.", [4]byte{10, 0, 0, 5}, 60)}) + + addrs, err := Lookup(context.Background(), "inverter.local") + if err != nil { + t.Fatalf("Lookup: %v", err) + } + if len(addrs) != 1 || addrs[0] != netip.MustParseAddr("10.0.0.5") { + t.Fatalf("addrs = %v, want [10.0.0.5]", addrs) + } +} + +// When neither backend answers, the error has to say that avahi was asked too +// — "there is no avahi" and "avahi said no" call for different fixes. +func TestLookupErrorNamesBothBackends(t *testing.T) { + Flush() + fakeAvahi(t, func(command, name string) string { return "- 15 Timeout reached" }) + startResponder(t, nil) // never answers + + ctx, cancel := context.WithTimeout(context.Background(), 150*time.Millisecond) + defer cancel() + _, err := Lookup(ctx, "missing.local") + if err == nil { + t.Fatal("expected a failure") + } + if !strings.Contains(err.Error(), "avahi") { + t.Fatalf("error %q does not mention avahi", err) + } +} + +// A socket that does not exist must not be dialled at all: the probe is what +// keeps a stray connect attempt out of every lookup on a host without avahi. +func TestAvahiSkippedWhenSocketAbsent(t *testing.T) { + Flush() + origAvail, origDial := avahiAvailable, avahiDial + avahiAvailable = func() bool { return false } + avahiDial = func(ctx context.Context, path string) (net.Conn, error) { + t.Error("dialled the avahi socket although it is not available") + return nil, errors.New("should not be called") + } + t.Cleanup(func() { avahiAvailable, avahiDial = origAvail, origDial }) + + startResponder(t, []dnsmessage.Resource{aResource(t, "inverter.local.", [4]byte{10, 0, 0, 6}, 60)}) + if _, err := Lookup(context.Background(), "inverter.local"); err != nil { + t.Fatalf("Lookup: %v", err) + } +} diff --git a/go/internal/mdnsresolve/mdnsresolve.go b/go/internal/mdnsresolve/mdnsresolve.go new file mode 100644 index 00000000..07d8d907 --- /dev/null +++ b/go/internal/mdnsresolve/mdnsresolve.go @@ -0,0 +1,231 @@ +// Package mdnsresolve resolves RFC 6762 ".local" host names for outbound +// connections, and provides a dialer that uses it. +// +// Go will not do this itself. net/conf.go routes a ".local" lookup to libc +// only when cgo is available, and every FTW build sets CGO_ENABLED=0, so the +// pure Go resolver is always selected: it reads /etc/resolv.conf and sends a +// *unicast* query to the site router, which has no idea what "inverter.local" +// is. That holds on every base image and every libc, glibc included — the +// image shipping libnss-mdns changes what `getent` and `curl` can resolve +// inside the container, not what this process can. +// +// # Where the answer comes from +// +// First choice is to ask the machine's own mDNS responder, avahi-daemon, over +// its simple-protocol socket. One line out, one line back, no DNS wire format +// decoded here — and it is the same daemon over the same socket that +// libnss_mdns4_minimal.so.2 uses, so a name resolves to the same address +// whether FTW dials it or an operator runs `getent hosts zap.local` in the +// container. avahi also holds a record cache, so a repeat lookup usually costs +// no LAN traffic at all. +// +// Second choice, in multicast.go, is to query the LAN directly. It exists +// because the socket cannot always be reached: it has to be bind-mounted, and +// under the Home Assistant Supervisor an add-on cannot mount arbitrary host +// paths at all — only a fixed set of named ones. FTW ships as an add-on, so a +// resolver that required that socket would simply not work there. The fallback +// also covers any Docker host that does not run avahi. +// +// Only ".local" names take either path. Literal IPs and ordinary DNS names are +// handed straight to the standard dialer. +package mdnsresolve + +import ( + "context" + "fmt" + "log/slog" + "net" + "net/netip" + "strings" + "sync" + "time" +) + +// now is swappable so cache-expiry tests do not have to sleep. +var now = time.Now + +const ( + // queryTimeout bounds each backend separately, so falling back costs at + // most two of these rather than an unbounded wait. It is long enough for a + // sleepy device and short enough that a driver dial does not stall a + // control tick. + queryTimeout = 900 * time.Millisecond + + // A responder's TTL is advisory here. The floor stops a device that + // advertises a very short TTL from turning every Modbus reconnect into a + // multicast storm; the ceiling keeps a DHCP move from taking effect + // arbitrarily late, which is the whole point of binding by name. + minTTL = 30 * time.Second + maxTTL = 120 * time.Second + + // negativeTTL is deliberately short: a device that was off when we first + // looked should become reachable soon after it boots. + negativeTTL = 5 * time.Second +) + +type cacheEntry struct { + addrs []netip.Addr // empty means a cached negative answer + expires time.Time +} + +var ( + cacheMu sync.Mutex + cache = map[string]cacheEntry{} +) + +// IsLocal reports whether host is a ".local" name that mDNS should resolve. +// A literal IP is never one, so a configured "192.168.1.5" keeps the plain +// dial path and never touches the network for resolution. +func IsLocal(host string) bool { + if host == "" || net.ParseIP(host) != nil { + return false + } + return strings.HasSuffix(strings.ToLower(strings.TrimSuffix(host, ".")), ".local") +} + +func canonical(name string) string { + return strings.ToLower(strings.TrimSuffix(name, ".")) +} + +func cacheLookup(key string) ([]netip.Addr, bool) { + cacheMu.Lock() + defer cacheMu.Unlock() + entry, ok := cache[key] + if !ok || now().After(entry.expires) { + return nil, false + } + return entry.addrs, true +} + +func cacheStore(key string, addrs []netip.Addr, ttl time.Duration) { + cacheMu.Lock() + defer cacheMu.Unlock() + cache[key] = cacheEntry{addrs: addrs, expires: now().Add(ttl)} +} + +// Flush drops every cached answer. Tests use it; nothing in production does. +func Flush() { + cacheMu.Lock() + defer cacheMu.Unlock() + cache = map[string]cacheEntry{} +} + +// Lookup resolves a ".local" name to its advertised addresses, asking +// avahi-daemon first and the LAN directly if that is not answerable. +func Lookup(ctx context.Context, name string) ([]netip.Addr, error) { + key := canonical(name) + if addrs, ok := cacheLookup(key); ok { + if len(addrs) == 0 { + return nil, fmt.Errorf("no mDNS responder for %s (cached)", name) + } + return addrs, nil + } + + addrs, ttl, via, err := resolve(ctx, key) + if err != nil || len(addrs) == 0 { + cacheStore(key, nil, negativeTTL) + if err == nil { + err = fmt.Errorf("no mDNS responder for %s", name) + } + return nil, err + } + + cacheStore(key, addrs, ttl) + // Logged on a cache miss only, so this is at most one line per TTL per + // device rather than one per reconnect. "via" is here because the two + // backends fail for completely different reasons, and an operator reading + // a support report needs to know which one was in play. + slog.Info("resolved host over mDNS", "host", key, "addr", addrs[0].String(), "ttl", ttl, "via", via) + return addrs, nil +} + +// resolve tries avahi, then a direct query. An avahi socket that is present +// but unhelpful still falls through: a daemon that is starting up, wedged or +// misconfigured should degrade to a slower answer, not to no answer. +func resolve(ctx context.Context, key string) ([]netip.Addr, time.Duration, string, error) { + var avahiErr error + if avahiAvailable() { + actx, cancel := context.WithTimeout(ctx, queryTimeout) + addrs, err := avahiLookup(actx, key) + cancel() + if err == nil && len(addrs) > 0 { + return addrs, avahiTTL, "avahi", nil + } + avahiErr = err + } + + addrs, ttl, err := queryAddrs(ctx, key) + if err != nil && avahiErr != nil { + // Both were tried; report both, because "avahi said no and the LAN + // said nothing" and "there is no avahi and the LAN said nothing" call + // for different fixes. + return nil, 0, "", fmt.Errorf("%w (avahi: %v)", err, avahiErr) + } + if err != nil { + return nil, 0, "", err + } + return addrs, ttl, "multicast", nil +} + +// Dialer dials TCP addresses, resolving ".local" host names over mDNS first. +// The embedded net.Dialer carries timeout and keep-alive; anything that is not +// a ".local" name is handed straight to it. +// +// Resolution happens per dial, not once at startup. That is what makes binding +// a device by name survive a DHCP lease change: callers that rebuild their +// connection from the original address string pick up the new IP on reconnect. +type Dialer struct { + net.Dialer +} + +// DialContext resolves address if it names a ".local" host, then dials it. +func (d *Dialer) DialContext(ctx context.Context, network, address string) (net.Conn, error) { + host, port, err := net.SplitHostPort(address) + if err != nil || !IsLocal(host) { + return d.Dialer.DialContext(ctx, network, address) + } + + addrs, err := Lookup(ctx, host) + if err != nil { + // Name the mechanism. Without this the operator sees a bare dial + // failure and has no way to tell that resolution was the reason. + slog.Warn("mDNS resolution failed; check the device is on this LAN and the container uses host networking", + "host", host, "err", err) + return nil, fmt.Errorf("resolve %s over mDNS: %w", host, err) + } + + var firstErr error + for _, a := range addrs { + conn, err := d.Dialer.DialContext(ctx, network, net.JoinHostPort(a.String(), port)) + if err == nil { + return conn, nil + } + if firstErr == nil { + firstErr = err + } + } + return nil, fmt.Errorf("dial %s over mDNS: %w", host, firstErr) +} + +// Dial is the context-free form, for callers that have no context to pass. +func (d *Dialer) Dial(network, address string) (net.Conn, error) { + ctx := context.Background() + if d.Timeout > 0 { + var cancel context.CancelFunc + ctx, cancel = context.WithTimeout(ctx, d.Timeout) + defer cancel() + } + return d.DialContext(ctx, network, address) +} + +// DialContext dials with default settings. +func DialContext(ctx context.Context, network, address string) (net.Conn, error) { + var d Dialer + return d.DialContext(ctx, network, address) +} + +// DialTimeout mirrors net.DialTimeout with mDNS resolution added. +func DialTimeout(network, address string, timeout time.Duration) (net.Conn, error) { + d := Dialer{Dialer: net.Dialer{Timeout: timeout}} + return d.Dial(network, address) +} diff --git a/go/internal/mdnsresolve/mdnsresolve_test.go b/go/internal/mdnsresolve/mdnsresolve_test.go new file mode 100644 index 00000000..65a6eaf3 --- /dev/null +++ b/go/internal/mdnsresolve/mdnsresolve_test.go @@ -0,0 +1,290 @@ +package mdnsresolve + +import ( + "context" + "errors" + "net" + "net/netip" + "strings" + "testing" + "time" + + "golang.org/x/net/dns/dnsmessage" +) + +func TestIsLocal(t *testing.T) { + cases := []struct { + host string + want bool + }{ + {"inverter.local", true}, + {"INVERTER.LOCAL", true}, + {"inverter.local.", true}, + {"zap.local", true}, + // A literal address must never trigger a lookup. + {"192.168.1.5", false}, + {"::1", false}, + {"example.com", false}, + {"localhost", false}, + {"local", false}, + {"notlocal", false}, + {"", false}, + } + for _, c := range cases { + if got := IsLocal(c.host); got != c.want { + t.Errorf("IsLocal(%q) = %v, want %v", c.host, got, c.want) + } + } +} + +func mustDNSName(t *testing.T, s string) dnsmessage.Name { + t.Helper() + n, err := dnsmessage.NewName(s) + if err != nil { + t.Fatalf("NewName(%q): %v", s, err) + } + return n +} + +func aResource(t *testing.T, name string, ip [4]byte, ttl uint32) dnsmessage.Resource { + t.Helper() + return dnsmessage.Resource{ + Header: dnsmessage.ResourceHeader{ + Name: mustDNSName(t, name), Type: dnsmessage.TypeA, + Class: dnsmessage.ClassINET, TTL: ttl, + }, + Body: &dnsmessage.AResource{A: ip}, + } +} + +func packAnswer(t *testing.T, qname string, answers []dnsmessage.Resource) []byte { + t.Helper() + msg := dnsmessage.Message{ + Header: dnsmessage.Header{Response: true, Authoritative: true}, + Questions: []dnsmessage.Question{{Name: mustDNSName(t, qname), Type: dnsmessage.TypeA, Class: dnsmessage.ClassINET}}, + Answers: answers, + } + packet, err := msg.Pack() + if err != nil { + t.Fatalf("pack: %v", err) + } + return packet +} + +func TestParseAddrAnswer(t *testing.T) { + qname := "inverter.local." + packet := packAnswer(t, qname, []dnsmessage.Resource{aResource(t, qname, [4]byte{192, 168, 1, 42}, 60)}) + + addrs, ttl, ok := parseAddrAnswer(packet, qname) + if !ok { + t.Fatal("parseAddrAnswer did not accept a valid answer") + } + if len(addrs) != 1 || addrs[0].String() != "192.168.1.42" { + t.Fatalf("addrs = %v, want [192.168.1.42]", addrs) + } + if ttl != 60*time.Second { + t.Fatalf("ttl = %v, want 60s", ttl) + } + + // An answer for a different name must be ignored. + if _, _, ok := parseAddrAnswer(packet, "other.local."); ok { + t.Fatal("accepted an answer for a different name") + } + // Garbage must not panic or resolve. + if _, _, ok := parseAddrAnswer([]byte{1, 2, 3}, qname); ok { + t.Fatal("accepted a malformed packet") + } +} + +func TestParseAddrAnswerClampsTTL(t *testing.T) { + qname := "inverter.local." + for _, c := range []struct { + name string + ttl uint32 + want time.Duration + }{ + // A device advertising a 1 s TTL must not make every Modbus reconnect + // re-query the LAN. + {"below floor", 1, minTTL}, + // A very long TTL must not outlive a DHCP move. + {"above ceiling", 86400, maxTTL}, + {"inside range", 90, 90 * time.Second}, + } { + t.Run(c.name, func(t *testing.T) { + packet := packAnswer(t, qname, []dnsmessage.Resource{aResource(t, qname, [4]byte{10, 0, 0, 1}, c.ttl)}) + _, ttl, ok := parseAddrAnswer(packet, qname) + if !ok { + t.Fatal("answer rejected") + } + if ttl != c.want { + t.Fatalf("ttl = %v, want %v", ttl, c.want) + } + }) + } +} + +// disableAvahi forces the fallback path. Without it these tests would behave +// differently on a developer machine that happens to run avahi-daemon. +func disableAvahi(t *testing.T) { + t.Helper() + orig := avahiAvailable + avahiAvailable = func() bool { return false } + t.Cleanup(func() { avahiAvailable = orig }) +} + +// startResponder points the package at a loopback UDP socket that answers one +// query, so the real send/parse path is exercised without touching the LAN. +func startResponder(t *testing.T, answers []dnsmessage.Resource) { + t.Helper() + rc, err := net.ListenUDP("udp4", &net.UDPAddr{IP: net.IPv4(127, 0, 0, 1), Port: 0}) + if err != nil { + t.Fatalf("listen responder: %v", err) + } + + origAddr, origListen := mdnsAddr, listenPacket + mdnsAddr = rc.LocalAddr().(*net.UDPAddr) + listenPacket = func() (*net.UDPConn, error) { + return net.ListenUDP("udp4", &net.UDPAddr{IP: net.IPv4(127, 0, 0, 1)}) + } + + done := make(chan struct{}) + go func() { + defer close(done) + buf := make([]byte, 1500) + _ = rc.SetReadDeadline(time.Now().Add(2 * time.Second)) + n, from, err := rc.ReadFromUDP(buf) + if err != nil { + return + } + if answers == nil { + return // silent responder: exercises the negative path + } + var p dnsmessage.Parser + hdr, err := p.Start(buf[:n]) + if err != nil { + return + } + q, err := p.Question() + if err != nil { + return + } + resp := dnsmessage.Message{ + Header: dnsmessage.Header{ID: hdr.ID, Response: true, Authoritative: true}, + Questions: []dnsmessage.Question{q}, + Answers: answers, + } + packed, err := resp.Pack() + if err != nil { + return + } + _, _ = rc.WriteToUDP(packed, from) + }() + + t.Cleanup(func() { + _ = rc.Close() + <-done + mdnsAddr, listenPacket = origAddr, origListen + Flush() + }) +} + +func TestLookupResolvesLocalName(t *testing.T) { + Flush() + disableAvahi(t) + startResponder(t, []dnsmessage.Resource{aResource(t, "inverter.local.", [4]byte{192, 168, 1, 42}, 60)}) + + addrs, err := Lookup(context.Background(), "inverter.local") + if err != nil { + t.Fatalf("Lookup: %v", err) + } + if len(addrs) != 1 || addrs[0] != netip.MustParseAddr("192.168.1.42") { + t.Fatalf("addrs = %v, want [192.168.1.42]", addrs) + } + + // The answer must now be cached: a second call cannot need the responder, + // which has already stopped. + again, err := Lookup(context.Background(), "INVERTER.local") + if err != nil { + t.Fatalf("cached Lookup: %v", err) + } + if len(again) != 1 || again[0] != addrs[0] { + t.Fatalf("cached addrs = %v, want %v", again, addrs) + } +} + +func TestLookupCachesNegativeAnswer(t *testing.T) { + Flush() + disableAvahi(t) + startResponder(t, nil) // never answers + + ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) + defer cancel() + if _, err := Lookup(ctx, "missing.local"); err == nil { + t.Fatal("expected a lookup failure when nothing answers") + } + + addrs, ok := cacheLookup("missing.local") + if !ok { + t.Fatal("a failed lookup should be negatively cached") + } + if len(addrs) != 0 { + t.Fatalf("negative cache holds %v, want no addresses", addrs) + } +} + +func TestCacheExpires(t *testing.T) { + Flush() + base := time.Now() + orig := now + now = func() time.Time { return base } + t.Cleanup(func() { now = orig; Flush() }) + + cacheStore("inverter.local", []netip.Addr{netip.MustParseAddr("192.168.1.9")}, 30*time.Second) + if _, ok := cacheLookup("inverter.local"); !ok { + t.Fatal("entry should be live immediately after store") + } + + now = func() time.Time { return base.Add(31 * time.Second) } + if _, ok := cacheLookup("inverter.local"); ok { + t.Fatal("entry should have expired") + } +} + +func TestDialerSkipsResolutionForPlainHosts(t *testing.T) { + origListen, origAvail := listenPacket, avahiAvailable + listenPacket = func() (*net.UDPConn, error) { + t.Error("issued an mDNS query for a host that is not a .local name") + return nil, errors.New("should not be called") + } + avahiAvailable = func() bool { + t.Error("consulted avahi for a host that is not a .local name") + return false + } + t.Cleanup(func() { listenPacket, avahiAvailable = origListen, origAvail }) + + d := Dialer{Dialer: net.Dialer{Timeout: 500 * time.Millisecond}} + // Nothing listens on port 1; the point is that the failure comes from the + // dial, not from resolution. + if _, err := d.Dial("tcp", "127.0.0.1:1"); err == nil { + t.Fatal("expected the dial to fail") + } else if strings.Contains(err.Error(), "mDNS") { + t.Fatalf("plain IP dial went through mDNS: %v", err) + } +} + +func TestDialerReportsResolutionFailure(t *testing.T) { + Flush() + disableAvahi(t) + startResponder(t, nil) // never answers + + d := Dialer{Dialer: net.Dialer{Timeout: 100 * time.Millisecond}} + _, err := d.Dial("tcp", "missing.local:502") + if err == nil { + t.Fatal("expected a failure") + } + // The error must name the mechanism — an operator reading the log has to be + // able to tell resolution apart from an unreachable device. + if !strings.Contains(err.Error(), "mDNS") { + t.Fatalf("error %q does not mention mDNS", err) + } +} diff --git a/go/internal/mdnsresolve/multicast.go b/go/internal/mdnsresolve/multicast.go new file mode 100644 index 00000000..90a9d028 --- /dev/null +++ b/go/internal/mdnsresolve/multicast.go @@ -0,0 +1,161 @@ +package mdnsresolve + +import ( + "context" + "fmt" + "net" + "net/netip" + "strings" + "time" + + "golang.org/x/net/dns/dnsmessage" +) + +// This file is the fallback described in the package comment: a direct RFC +// 6762 query, used only where avahi-daemon's socket cannot be reached. It is +// deliberately the second choice — see avahi.go for the first. + +// mdnsAddr is the RFC 6762 IPv4 multicast group. A var, not a const, so tests +// can aim a query at a loopback responder. +var mdnsAddr = &net.UDPAddr{IP: net.IPv4(224, 0, 0, 251), Port: 5353} + +// listenPacket opens the ephemeral socket a query is sent from. Replaced in +// tests. It deliberately does NOT bind port 5353: where avahi-daemon runs it +// already owns that port, and the QU bit below asks responders to reply +// directly to this socket instead. +var listenPacket = func() (*net.UDPConn, error) { + return net.ListenUDP("udp4", &net.UDPAddr{IP: net.IPv4zero}) +} + +// classQU is IN with the RFC 6762 unicast-response bit set. +const classQU = dnsmessage.Class(0x8001) + +func queryAddrs(ctx context.Context, name string) ([]netip.Addr, time.Duration, error) { + qname, err := dnsmessage.NewName(name + ".") + if err != nil { + return nil, 0, fmt.Errorf("mdns: bad name %q: %w", name, err) + } + // One packet, two questions. RFC 6762 §5.2 allows it and it saves a round + // trip on dual-stack devices. + msg := dnsmessage.Message{Questions: []dnsmessage.Question{ + {Name: qname, Type: dnsmessage.TypeA, Class: classQU}, + {Name: qname, Type: dnsmessage.TypeAAAA, Class: classQU}, + }} + packed, err := msg.Pack() + if err != nil { + return nil, 0, fmt.Errorf("mdns: pack query: %w", err) + } + + var ( + addrs []netip.Addr + ttl time.Duration + ) + err = exchange(ctx, packed, func(packet []byte) bool { + got, gotTTL, ok := parseAddrAnswer(packet, name+".") + if !ok { + return false + } + addrs, ttl = got, gotTTL + return true + }) + if err != nil { + return nil, 0, err + } + return addrs, ttl, nil +} + +// exchange sends one multicast query and feeds every reply to handle until it +// accepts one or the deadline passes. +func exchange(ctx context.Context, packed []byte, handle func([]byte) bool) error { + conn, err := listenPacket() + if err != nil { + return fmt.Errorf("mdns: open socket: %w", err) + } + defer conn.Close() + + deadline := now().Add(queryTimeout) + if d, ok := ctx.Deadline(); ok && d.Before(deadline) { + deadline = d + } + if err := conn.SetDeadline(deadline); err != nil { + return fmt.Errorf("mdns: set deadline: %w", err) + } + if _, err := conn.WriteToUDP(packed, mdnsAddr); err != nil { + return fmt.Errorf("mdns: send query: %w", err) + } + + buf := make([]byte, 1500) + for { + n, _, err := conn.ReadFromUDP(buf) + if err != nil { + return fmt.Errorf("mdns: no usable answer: %w", err) + } + if handle(buf[:n]) { + return nil + } + } +} + +func parseAddrAnswer(packet []byte, qname string) ([]netip.Addr, time.Duration, bool) { + var p dnsmessage.Parser + if _, err := p.Start(packet); err != nil { + return nil, 0, false + } + if err := p.SkipAllQuestions(); err != nil { + return nil, 0, false + } + var addrs []netip.Addr + ttl := maxTTL + // Labelled so a parse error inside the type switch abandons the whole + // packet: once the parser desynchronises, every later record is suspect. +parse: + for { + h, err := p.AnswerHeader() + if err != nil { + break parse + } + if !strings.EqualFold(h.Name.String(), qname) { + if err := p.SkipAnswer(); err != nil { + break parse + } + continue + } + switch h.Type { + case dnsmessage.TypeA: + r, err := p.AResource() + if err != nil { + break parse + } + addrs = append(addrs, netip.AddrFrom4(r.A)) + case dnsmessage.TypeAAAA: + r, err := p.AAAAResource() + if err != nil { + break parse + } + // Unmap so a v4-mapped AAAA dials as plain IPv4. + addrs = append(addrs, netip.AddrFrom16(r.AAAA).Unmap()) + default: + if err := p.SkipAnswer(); err != nil { + break parse + } + continue + } + if d := time.Duration(h.TTL) * time.Second; d < ttl { + ttl = d + } + } + return finishAnswer(addrs, ttl) +} + +func finishAnswer(addrs []netip.Addr, ttl time.Duration) ([]netip.Addr, time.Duration, bool) { + if len(addrs) == 0 { + return nil, 0, false + } + switch { + case ttl < minTTL: + ttl = minTTL + case ttl > maxTTL: + ttl = maxTTL + } + return addrs, ttl, true +} diff --git a/go/internal/modbus/tcp_client.go b/go/internal/modbus/tcp_client.go index 78b7e7ba..b5b790ab 100644 --- a/go/internal/modbus/tcp_client.go +++ b/go/internal/modbus/tcp_client.go @@ -7,6 +7,8 @@ import ( "io" "net" "time" + + "github.com/srcfl/ftw/go/internal/mdnsresolve" ) const ( @@ -39,10 +41,13 @@ func newTCPClient(addr string, timeout, keepAlive time.Duration) *tcpClient { } func (c *tcpClient) Open() error { - dialer := net.Dialer{ + // Resolution happens here, on every Open, so a device configured by its + // ".local" name is found again after a DHCP lease moves it — the reconnect + // path rebuilds the client from c.addr and picks up the new address. + dialer := mdnsresolve.Dialer{Dialer: net.Dialer{ Timeout: modbusDialTimeout, KeepAlive: c.keepAlive, - } + }} conn, err := dialer.Dial("tcp", c.addr) if err != nil { return err diff --git a/go/internal/mqtt/client.go b/go/internal/mqtt/client.go index fb1ec167..a711dd96 100644 --- a/go/internal/mqtt/client.go +++ b/go/internal/mqtt/client.go @@ -4,12 +4,15 @@ package mqtt import ( "fmt" "log/slog" + "net" + "net/url" "sync" "time" paho "github.com/eclipse/paho.mqtt.golang" "github.com/srcfl/ftw/go/internal/drivers" + "github.com/srcfl/ftw/go/internal/mdnsresolve" ) // Capability wraps a paho client to match drivers.MQTTCap. @@ -51,6 +54,14 @@ func Dial(host string, port int, username, password, clientID string) (*Capabili } opts := paho.NewClientOptions(). AddBroker(fmt.Sprintf("tcp://%s:%d", host, port)). + // paho's built-in dialer goes through the stdlib resolver, which never + // answers a ".local" name. Every broker URL built here is tcp://, so a + // TCP-only replacement is complete; non-".local" hosts fall through to + // a plain dial inside mdnsresolve. + SetCustomOpenConnectionFn(func(uri *url.URL, o paho.ClientOptions) (net.Conn, error) { + d := mdnsresolve.Dialer{Dialer: net.Dialer{Timeout: o.ConnectTimeout}} + return d.Dial("tcp", uri.Host) + }). SetClientID(clientID). SetAutoReconnect(true). SetConnectRetry(true). From 61e1a639d122dba40011e974169a8dcff2792c75 Mon Sep 17 00:00:00 2001 From: Fredrik Ahlgren Date: Tue, 4 Aug 2026 12:09:08 +0200 Subject: [PATCH 2/7] fix(net): harden local-name resolution --- .changeset/resolve-local-device-names.md | 5 + docs/operations.md | 15 +- go/internal/drivers/registry_restart_test.go | 31 +++ go/internal/drivers/tcp_cap_test.go | 22 +- go/internal/mdnsresolve/avahi.go | 73 +++++- go/internal/mdnsresolve/avahi_test.go | 27 +- go/internal/mdnsresolve/mdnsresolve.go | 3 + go/internal/mdnsresolve/mdnsresolve_test.go | 175 ++++++++++++- go/internal/mdnsresolve/multicast.go | 258 ++++++++++++++++--- scripts/test-container-boundaries.sh | 15 ++ 10 files changed, 565 insertions(+), 59 deletions(-) diff --git a/.changeset/resolve-local-device-names.md b/.changeset/resolve-local-device-names.md index 3f496148..b81a2a9a 100644 --- a/.changeset/resolve-local-device-names.md +++ b/.changeset/resolve-local-device-names.md @@ -35,3 +35,8 @@ Only `.local` names take this path; literal IPs and ordinary DNS names dial exactly as before. Multicast still has to reach the LAN, which the Linux Compose topology has via `network_mode: host`. Under `docker-compose.macos.yml` the container is bridged, so configure devices by IP there. + +Direct queries use each active multicast interface. IPv4 and IPv6 are +supported; link-local IPv6 addresses carry their interface zone and unscoped +answers are rejected. mDNS is unauthenticated, so reserve control-device names +on the LAN and use TLS certificate pins where available. diff --git a/docs/operations.md b/docs/operations.md index e6963223..b239ae86 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -205,7 +205,15 @@ both were tried. Either way multicast has to reach the LAN, which the Linux Compose topology provides through `network_mode: host`. Under `docker-compose.macos.yml` the container is bridged and multicast does not reach the LAN, so configure devices -by IP there. +by IP there. The direct path sends on every active, non-loopback multicast +interface. It supports IPv4 and IPv6; a link-local IPv6 answer is used only +with its interface zone, and an unscoped link-local answer is discarded. + +mDNS has no built-in authentication. Treat a `.local` name as a LAN trust +boundary, reserve names used by control drivers, and use TLS certificate pins +where the driver supports them. Network allowlists still check the configured +host name and port before resolution; they do not prove that an mDNS responder +is the intended device. #### Letting FTW use avahi @@ -223,9 +231,8 @@ directory created where the socket belongs stops `avahi-daemon` from ever starting. Restarting avahi detaches the mount, so restart FTW after you do. This is an optimisation, not a requirement: device connectivity is unchanged -without it. What it does add is `libnss-mdns` working for ordinary tools in the -image — `getent hosts zap.local`, `curl`, `wget` — which is the quickest way to -check a name from inside the container. +without it. It lets `getent hosts zap.local`, `curl` and `wget` check the name +from inside the container. Under the Home Assistant add-on none of this applies: Supervisor mounts only a fixed set of named paths, so the socket cannot be provided and FTW always diff --git a/go/internal/drivers/registry_restart_test.go b/go/internal/drivers/registry_restart_test.go index 2031b1e1..b7dcdde3 100644 --- a/go/internal/drivers/registry_restart_test.go +++ b/go/internal/drivers/registry_restart_test.go @@ -368,6 +368,37 @@ func TestReloadRestartsDriversWhenTroubleshootingModeChanges(t *testing.T) { } } +func TestReloadRestartsDriverWhenTransportHostChangesToLocalName(t *testing.T) { + var dials atomic.Int32 + r := NewRegistry(telemetry.NewStore()) + r.ModbusFactory = func(name string, cfg *config.ModbusConfig) (ModbusCap, error) { + dials.Add(1) + return &mockModbus{}, nil + } + path := writeTestDriver(t, registryRestartTestDriver) + cfg := config.Driver{ + Name: "d1", + Lua: path, + Capabilities: config.Capabilities{ + Modbus: &config.ModbusConfig{Host: "192.168.1.20", Port: 502, UnitID: 1}, + }, + } + if err := r.Add(context.Background(), cfg); err != nil { + t.Fatal(err) + } + defer r.ShutdownAll() + if got := dials.Load(); got != 1 { + t.Fatalf("initial Modbus factory calls = %d, want 1", got) + } + + updated := cfg + updated.Capabilities.Modbus = &config.ModbusConfig{Host: "inverter.local", Port: 502, UnitID: 1} + r.Reload(context.Background(), []config.Driver{updated}, false) + if got := dials.Load(); got != 2 { + t.Fatalf("Modbus factory calls after .local reload = %d, want 2", got) + } +} + // runLoop should bump TickCount on every poll-return-without-error so // a Lua driver that is alive but hasn't emitted yet (e.g. between // MQTT subscribe and the first inbound message) is visibly running diff --git a/go/internal/drivers/tcp_cap_test.go b/go/internal/drivers/tcp_cap_test.go index ff5794a4..01e3627f 100644 --- a/go/internal/drivers/tcp_cap_test.go +++ b/go/internal/drivers/tcp_cap_test.go @@ -97,6 +97,18 @@ func TestTCPCap_AllowedHosts(t *testing.T) { } } +func TestTCPAllowlistMatchesLocalNameBeforeResolution(t *testing.T) { + if ok, reason := tcpHostAllowed("inverter.local:502", []string{"inverter.local:502"}); !ok || reason != "" { + t.Fatalf("matching .local allowlist = (%v, %q), want (true, empty)", ok, reason) + } + if ok, reason := tcpHostAllowed("inverter.local:502", []string{"other.local:502"}); ok || !strings.Contains(reason, "not in allowed_hosts") { + t.Fatalf("different .local allowlist = (%v, %q), want a pre-resolution rejection", ok, reason) + } + if ok, reason := tcpHostAllowed("inverter.local:1883", []string{"inverter.local:502"}); ok || !strings.Contains(reason, "not in allowed_hosts") { + t.Fatalf("wrong port for .local allowlist = (%v, %q), want rejection", ok, reason) + } +} + // TestTCPCap_StalePumpDoesNotClobberLiveState exercises the readPump→Close // race. We open against listener A, close, open against listener B, then // force A's accepted connection to drop. The stale pump for A wakes from @@ -207,11 +219,11 @@ func (f *fakeTCPCap) Close() error { f.closed = true; return nil } // Synthetic DSMR 5.0 telegram body. Values chosen so we can pin every emit: // -// import 1.234 kW, export 0.500 kW → meter.w = +734 W -// per-phase voltages 230.1 / 230.2 / 230.3 V -// per-phase currents 5 / 3 / 7 A -// import T1 100.000 kWh + T2 200.000 kWh → import_wh = 300_000 -// export T1 10.000 kWh + T2 20.000 kWh → export_wh = 30_000 +// import 1.234 kW, export 0.500 kW → meter.w = +734 W +// per-phase voltages 230.1 / 230.2 / 230.3 V +// per-phase currents 5 / 3 / 7 A +// import T1 100.000 kWh + T2 200.000 kWh → import_wh = 300_000 +// export T1 10.000 kWh + T2 20.000 kWh → export_wh = 30_000 // // CRC is computed at runtime via dsmrCRC16; tests build the full telegram // with dsmrWrap() so they exercise the same CRC path the live meter does. diff --git a/go/internal/mdnsresolve/avahi.go b/go/internal/mdnsresolve/avahi.go index 01b281d6..de7b1d3f 100644 --- a/go/internal/mdnsresolve/avahi.go +++ b/go/internal/mdnsresolve/avahi.go @@ -7,6 +7,7 @@ import ( "net" "net/netip" "os" + "strconv" "strings" ) @@ -43,6 +44,10 @@ var avahiAvailable = func() bool { return err == nil && fi.Mode()&os.ModeSocket != 0 } +// avahiInterfaceByIndex is a var so tests can provide a stable interface name +// for link-local IPv6 answers without depending on the host's interfaces. +var avahiInterfaceByIndex = net.InterfaceByIndex + type avahiResult struct { addr netip.Addr err error @@ -58,14 +63,33 @@ type avahiResult struct { func avahiLookup(ctx context.Context, name string) ([]netip.Addr, error) { v4 := make(chan avahiResult, 1) v6 := make(chan avahiResult, 1) - go func() { v6 <- avahiResolve(ctx, "RESOLVE-HOSTNAME-IPV6", name) }() - go func() { v4 <- avahiResolve(ctx, "RESOLVE-HOSTNAME-IPV4", name) }() + lookupCtx, cancel := context.WithCancel(ctx) + defer cancel() + go func() { v6 <- avahiResolve(lookupCtx, "RESOLVE-HOSTNAME-IPV6", name) }() + go func() { v4 <- avahiResolve(lookupCtx, "RESOLVE-HOSTNAME-IPV4", name) }() - r4 := <-v4 + var r4, r6 avahiResult + got4, got6 := false, false + for !got4 || !got6 { + select { + case r4 = <-v4: + got4 = true + if r4.err == nil { + // IPv4 is preferred, but wait for the cancelled IPv6 + // exchange before returning so no goroutine can outlive + // this lookup or read test hooks during cleanup. + cancel() + } + case r6 = <-v6: + got6 = true + // Do not cancel IPv4 here. IPv4 is the preferred answer, so + // let an in-flight IPv4 request finish before accepting IPv6. + } + } if r4.err == nil { return []netip.Addr{r4.addr}, nil } - if r6 := <-v6; r6.err == nil { + if r6.err == nil { return []netip.Addr{r6.addr}, nil } return nil, fmt.Errorf("avahi: %s: %w", name, r4.err) @@ -73,7 +97,7 @@ func avahiLookup(ctx context.Context, name string) ([]netip.Addr, error) { // avahiResolve runs one request/response exchange over the socket. // -// The wire format is a single line each way. A success is +// The wire format is a single line each way. A success is: // // +
// @@ -85,7 +109,16 @@ func avahiResolve(ctx context.Context, command, name string) avahiResult { if err != nil { return avahiResult{err: fmt.Errorf("connect %s: %w", avahiSocket, err)} } + done := make(chan struct{}) + defer close(done) defer conn.Close() + go func() { + select { + case <-ctx.Done(): + _ = conn.Close() + case <-done: + } + }() if deadline, ok := ctx.Deadline(); ok { _ = conn.SetDeadline(deadline) @@ -102,10 +135,10 @@ func avahiResolve(ctx context.Context, command, name string) avahiResult { } return avahiResult{err: fmt.Errorf("no reply")} } - return parseAvahiReply(scanner.Text()) + return parseAvahiReply(scanner.Text(), name) } -func parseAvahiReply(line string) avahiResult { +func parseAvahiReply(line, expectedName string) avahiResult { fields := strings.Fields(line) if len(fields) == 0 { return avahiResult{err: fmt.Errorf("empty reply")} @@ -119,9 +152,35 @@ func parseAvahiReply(line string) avahiResult { if len(fields) < 5 { return avahiResult{err: fmt.Errorf("short reply %q", line)} } + if expectedName != "" && canonical(fields[3]) != canonical(expectedName) { + return avahiResult{err: fmt.Errorf("reply name %q does not match %q", fields[3], expectedName)} + } + protocol, err := strconv.Atoi(fields[2]) + if err != nil { + return avahiResult{err: fmt.Errorf("unparsable protocol %q", fields[2])} + } addr, err := netip.ParseAddr(fields[4]) if err != nil { return avahiResult{err: fmt.Errorf("unparsable address %q", fields[4])} } + if (addr.Is4() && protocol != 0) || (addr.Is6() && protocol != 1) { + return avahiResult{err: fmt.Errorf("protocol %d does not match address %s", protocol, addr)} + } + if addr.Is6() && addr.IsLinkLocalUnicast() { + interfaceIndex, err := strconv.Atoi(fields[1]) + if err != nil || interfaceIndex <= 0 { + return avahiResult{err: fmt.Errorf("link-local address %s has no interface", addr)} + } + iface, err := avahiInterfaceByIndex(interfaceIndex) + if err != nil || iface == nil || iface.Name == "" || + iface.Index != interfaceIndex || iface.Flags&net.FlagUp == 0 || + iface.Flags&net.FlagMulticast == 0 || iface.Flags&net.FlagLoopback != 0 { + if err == nil { + err = fmt.Errorf("interface %d is unavailable or not multicast-capable", interfaceIndex) + } + return avahiResult{err: fmt.Errorf("link-local address %s: %w", addr, err)} + } + addr = addr.WithZone(iface.Name) + } return avahiResult{addr: addr.Unmap()} } diff --git a/go/internal/mdnsresolve/avahi_test.go b/go/internal/mdnsresolve/avahi_test.go index 236b95c7..05d01f1f 100644 --- a/go/internal/mdnsresolve/avahi_test.go +++ b/go/internal/mdnsresolve/avahi_test.go @@ -23,7 +23,11 @@ import ( func fakeAvahi(t *testing.T, reply func(command, name string) string) { t.Helper() origAvail, origDial := avahiAvailable, avahiDial + origInterfaceByIndex := avahiInterfaceByIndex avahiAvailable = func() bool { return true } + avahiInterfaceByIndex = func(index int) (*net.Interface, error) { + return &net.Interface{Index: index, Name: "test0", Flags: net.FlagUp | net.FlagMulticast}, nil + } avahiDial = func(ctx context.Context, path string) (net.Conn, error) { client, server := net.Pipe() go func() { @@ -44,6 +48,7 @@ func fakeAvahi(t *testing.T, reply func(command, name string) string) { } t.Cleanup(func() { avahiAvailable, avahiDial = origAvail, origDial + avahiInterfaceByIndex = origInterfaceByIndex Flush() }) } @@ -56,17 +61,23 @@ func TestParseAvahiReply(t *testing.T) { }{ // The exact shape avahi-daemon returns, confirmed against the daemon. {"ipv4", "+ 2 0 zap.local 192.168.1.42", "192.168.1.42"}, - {"ipv6", "+ 2 1 zap.local fe80::1", "fe80::1"}, + {"ipv6", "+ 2 1 zap.local fe80::1", "fe80::1%test0"}, // A v4-mapped answer must dial as plain IPv4. {"v4 mapped", "+ 2 1 zap.local ::ffff:192.168.1.42", "192.168.1.42"}, + {"wrong name", "+ 2 0 other.local 192.168.1.42", ""}, {"failure", "- 15 Timeout reached", ""}, {"empty", "", ""}, {"truncated", "+ 2 0 zap.local", ""}, {"unparsable address", "+ 2 0 zap.local not-an-address", ""}, } + origInterfaceByIndex := avahiInterfaceByIndex + avahiInterfaceByIndex = func(index int) (*net.Interface, error) { + return &net.Interface{Index: index, Name: "test0", Flags: net.FlagUp | net.FlagMulticast}, nil + } + t.Cleanup(func() { avahiInterfaceByIndex = origInterfaceByIndex }) for _, c := range cases { t.Run(c.name, func(t *testing.T) { - got := parseAvahiReply(c.line) + got := parseAvahiReply(c.line, "zap.local") if c.want == "" { if got.err == nil { t.Fatalf("accepted %q, got %v", c.line, got.addr) @@ -86,7 +97,7 @@ func TestParseAvahiReply(t *testing.T) { // A failure line must carry avahi's own wording, so the log says what the // daemon said rather than something this package made up. func TestParseAvahiReplyKeepsDaemonWording(t *testing.T) { - got := parseAvahiReply("- 15 Timeout reached") + got := parseAvahiReply("- 15 Timeout reached", "zap.local") if got.err == nil { t.Fatal("expected an error") } @@ -104,12 +115,12 @@ func TestLookupPrefersAvahi(t *testing.T) { return "- 15 Timeout reached" }) - orig := listenPacket - listenPacket = func() (*net.UDPConn, error) { + orig := listenMulticastPacket + listenMulticastPacket = func(network string, iface *net.Interface, group *net.UDPAddr) (*net.UDPConn, error) { t.Error("queried the LAN directly even though avahi answered") return nil, errors.New("should not be called") } - t.Cleanup(func() { listenPacket = orig }) + t.Cleanup(func() { listenMulticastPacket = orig }) addrs, err := Lookup(context.Background(), "inverter.local") if err != nil { @@ -156,8 +167,8 @@ func TestAvahiLookupFallsBackToIPv6(t *testing.T) { if err != nil { t.Fatalf("avahiLookup: %v", err) } - if len(addrs) != 1 || addrs[0] != netip.MustParseAddr("fe80::2") { - t.Fatalf("addrs = %v, want [fe80::2]", addrs) + if len(addrs) != 1 || addrs[0].String() != "fe80::2%test0" { + t.Fatalf("addrs = %v, want [fe80::2%%test0]", addrs) } } diff --git a/go/internal/mdnsresolve/mdnsresolve.go b/go/internal/mdnsresolve/mdnsresolve.go index 07d8d907..95b2b5ed 100644 --- a/go/internal/mdnsresolve/mdnsresolve.go +++ b/go/internal/mdnsresolve/mdnsresolve.go @@ -25,6 +25,9 @@ // paths at all — only a fixed set of named ones. FTW ships as an add-on, so a // resolver that required that socket would simply not work there. The fallback // also covers any Docker host that does not run avahi. +// Direct queries bind an ephemeral socket to every active, non-loopback +// multicast interface. IPv4 and IPv6 are queried separately; IPv6 link-local +// answers carry the interface zone and are rejected when no zone is known. // // Only ".local" names take either path. Literal IPs and ordinary DNS names are // handed straight to the standard dialer. diff --git a/go/internal/mdnsresolve/mdnsresolve_test.go b/go/internal/mdnsresolve/mdnsresolve_test.go index 65a6eaf3..6cd139d0 100644 --- a/go/internal/mdnsresolve/mdnsresolve_test.go +++ b/go/internal/mdnsresolve/mdnsresolve_test.go @@ -57,6 +57,17 @@ func aResource(t *testing.T, name string, ip [4]byte, ttl uint32) dnsmessage.Res } } +func aaaaResource(t *testing.T, name string, ip [16]byte, ttl uint32) dnsmessage.Resource { + t.Helper() + return dnsmessage.Resource{ + Header: dnsmessage.ResourceHeader{ + Name: mustDNSName(t, name), Type: dnsmessage.TypeAAAA, + Class: dnsmessage.ClassINET, TTL: ttl, + }, + Body: &dnsmessage.AAAAResource{AAAA: ip}, + } +} + func packAnswer(t *testing.T, qname string, answers []dnsmessage.Resource) []byte { t.Helper() msg := dnsmessage.Message{ @@ -123,6 +134,112 @@ func TestParseAddrAnswerClampsTTL(t *testing.T) { } } +func TestParseAddrAnswerRequiresInterfaceZoneForLinkLocalIPv6(t *testing.T) { + qname := "inverter.local." + packet := packAnswer(t, qname, []dnsmessage.Resource{ + aaaaResource(t, qname, [16]byte{0xfe, 0x80, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1}, 60), + }) + if _, _, ok := parseAddrAnswer(packet, qname); ok { + t.Fatal("accepted link-local IPv6 answer without an interface zone") + } + addrs, _, ok := parseAddrAnswer(packet, qname, "test0") + if !ok || len(addrs) != 1 || addrs[0].String() != "fe80::1%test0" { + t.Fatalf("zoned answer = %v, ok=%v; want [fe80::1%%test0]", addrs, ok) + } +} + +func TestQueryIPv6UsesSelectedInterfaceForLinkLocalAnswer(t *testing.T) { + qname := "inverter.local" + qnameWire := mustDNSName(t, qname+".") + queryMessage := dnsmessage.Message{Questions: []dnsmessage.Question{ + {Name: qnameWire, Type: dnsmessage.TypeAAAA, Class: classQU}, + }} + query, err := queryMessage.Pack() + if err != nil { + t.Fatal(err) + } + + responder, err := net.ListenUDP("udp6", &net.UDPAddr{IP: net.ParseIP("::1"), Port: 0}) + if err != nil { + t.Skipf("IPv6 loopback unavailable: %v", err) + } + defer responder.Close() + + origAddr, origInterfaces, origListen := mdnsAddr6, multicastInterfaces, listenMulticastPacket + mdnsAddr6 = responder.LocalAddr().(*net.UDPAddr) + multicastInterfaces = func() ([]net.Interface, error) { + return []net.Interface{{Index: 7, Name: "test0", Flags: net.FlagUp | net.FlagMulticast}}, nil + } + listenMulticastPacket = func(network string, iface *net.Interface, group *net.UDPAddr) (*net.UDPConn, error) { + if network != "udp6" || iface.Name != "test0" { + t.Fatalf("query used network=%q iface=%v", network, iface) + } + return net.ListenUDP("udp6", &net.UDPAddr{IP: net.ParseIP("::1")}) + } + t.Cleanup(func() { + mdnsAddr6, multicastInterfaces, listenMulticastPacket = origAddr, origInterfaces, origListen + }) + + done := make(chan struct{}) + go func() { + defer close(done) + buf := make([]byte, 1500) + _ = responder.SetReadDeadline(time.Now().Add(2 * time.Second)) + n, from, err := responder.ReadFromUDP(buf) + if err != nil { + return + } + var parser dnsmessage.Parser + header, err := parser.Start(buf[:n]) + if err != nil { + return + } + question, err := parser.Question() + if err != nil { + return + } + responseMessage := dnsmessage.Message{ + Header: dnsmessage.Header{ID: header.ID, Response: true, Authoritative: true}, + Questions: []dnsmessage.Question{question}, + Answers: []dnsmessage.Resource{aaaaResource(t, qname+".", [16]byte{0xfe, 0x80, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1}, 60)}, + } + response, err := responseMessage.Pack() + if err == nil { + _, _ = responder.WriteToUDP(response, from) + } + }() + + addrs, _, err := queryIPv6(context.Background(), query, qname) + if err != nil { + t.Fatalf("queryIPv6: %v", err) + } + <-done + if len(addrs) != 1 || addrs[0].String() != "fe80::1%test0" { + t.Fatalf("addrs = %v, want [fe80::1%%test0]", addrs) + } +} + +func TestEligibleMulticastInterfacesRejectsUnsafeChoices(t *testing.T) { + orig := multicastInterfaces + multicastInterfaces = func() ([]net.Interface, error) { + return []net.Interface{ + {Index: 1, Name: "lo", Flags: net.FlagUp | net.FlagMulticast | net.FlagLoopback}, + {Index: 2, Name: "down0", Flags: net.FlagMulticast}, + {Index: 3, Name: "unicast0", Flags: net.FlagUp}, + {Index: 4, Name: "lan0", Flags: net.FlagUp | net.FlagMulticast}, + }, nil + } + t.Cleanup(func() { multicastInterfaces = orig }) + + got, err := eligibleMulticastInterfaces() + if err != nil { + t.Fatal(err) + } + if len(got) != 1 || got[0].Name != "lan0" { + t.Fatalf("eligible interfaces = %v, want [lan0]", got) + } +} + // disableAvahi forces the fallback path. Without it these tests would behave // differently on a developer machine that happens to run avahi-daemon. func disableAvahi(t *testing.T) { @@ -141,9 +258,15 @@ func startResponder(t *testing.T, answers []dnsmessage.Resource) { t.Fatalf("listen responder: %v", err) } - origAddr, origListen := mdnsAddr, listenPacket + origAddr, origMulticast, origInterfaces := mdnsAddr, listenMulticastPacket, multicastInterfaces mdnsAddr = rc.LocalAddr().(*net.UDPAddr) - listenPacket = func() (*net.UDPConn, error) { + multicastInterfaces = func() ([]net.Interface, error) { + return []net.Interface{{Index: 1, Name: "test0", Flags: net.FlagUp | net.FlagMulticast}}, nil + } + listenMulticastPacket = func(network string, iface *net.Interface, group *net.UDPAddr) (*net.UDPConn, error) { + if network != "udp4" { + return nil, errors.New("IPv6 disabled in IPv4 responder test") + } return net.ListenUDP("udp4", &net.UDPAddr{IP: net.IPv4(127, 0, 0, 1)}) } @@ -183,7 +306,8 @@ func startResponder(t *testing.T, answers []dnsmessage.Resource) { t.Cleanup(func() { _ = rc.Close() <-done - mdnsAddr, listenPacket = origAddr, origListen + mdnsAddr = origAddr + listenMulticastPacket, multicastInterfaces = origMulticast, origInterfaces Flush() }) } @@ -251,8 +375,8 @@ func TestCacheExpires(t *testing.T) { } func TestDialerSkipsResolutionForPlainHosts(t *testing.T) { - origListen, origAvail := listenPacket, avahiAvailable - listenPacket = func() (*net.UDPConn, error) { + origAvail, origMulticast := avahiAvailable, listenMulticastPacket + listenMulticastPacket = func(network string, iface *net.Interface, group *net.UDPAddr) (*net.UDPConn, error) { t.Error("issued an mDNS query for a host that is not a .local name") return nil, errors.New("should not be called") } @@ -260,7 +384,7 @@ func TestDialerSkipsResolutionForPlainHosts(t *testing.T) { t.Error("consulted avahi for a host that is not a .local name") return false } - t.Cleanup(func() { listenPacket, avahiAvailable = origListen, origAvail }) + t.Cleanup(func() { avahiAvailable, listenMulticastPacket = origAvail, origMulticast }) d := Dialer{Dialer: net.Dialer{Timeout: 500 * time.Millisecond}} // Nothing listens on port 1; the point is that the failure comes from the @@ -272,6 +396,45 @@ func TestDialerSkipsResolutionForPlainHosts(t *testing.T) { } } +func TestDialerResolvesLocalNameBeforeConnecting(t *testing.T) { + listener, err := net.Listen("tcp4", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + defer listener.Close() + _, port, err := net.SplitHostPort(listener.Addr().String()) + if err != nil { + t.Fatal(err) + } + + accepted := make(chan struct{}) + go func() { + conn, err := listener.Accept() + if err == nil { + close(accepted) + _ = conn.Close() + } + }() + fakeAvahi(t, func(command, name string) string { + if command == "RESOLVE-HOSTNAME-IPV4" && name == "inverter.local" { + return "+ 2 0 inverter.local 127.0.0.1" + } + return "- 15 Timeout reached" + }) + + d := Dialer{Dialer: net.Dialer{Timeout: time.Second}} + conn, err := d.Dial("tcp", "inverter.local:"+port) + if err != nil { + t.Fatalf("Dial: %v", err) + } + _ = conn.Close() + select { + case <-accepted: + case <-time.After(time.Second): + t.Fatal("resolved TCP endpoint was not reached") + } +} + func TestDialerReportsResolutionFailure(t *testing.T) { Flush() disableAvahi(t) diff --git a/go/internal/mdnsresolve/multicast.go b/go/internal/mdnsresolve/multicast.go index 90a9d028..c151fe6b 100644 --- a/go/internal/mdnsresolve/multicast.go +++ b/go/internal/mdnsresolve/multicast.go @@ -19,12 +19,23 @@ import ( // can aim a query at a loopback responder. var mdnsAddr = &net.UDPAddr{IP: net.IPv4(224, 0, 0, 251), Port: 5353} -// listenPacket opens the ephemeral socket a query is sent from. Replaced in -// tests. It deliberately does NOT bind port 5353: where avahi-daemon runs it -// already owns that port, and the QU bit below asks responders to reply -// directly to this socket instead. -var listenPacket = func() (*net.UDPConn, error) { - return net.ListenUDP("udp4", &net.UDPAddr{IP: net.IPv4zero}) +// mdnsAddr6 is the RFC 6762 IPv6 multicast group. The interface zone is added +// to a copy for each query because ff02::fb is link-local by definition. +var mdnsAddr6 = &net.UDPAddr{IP: net.ParseIP("ff02::fb"), Port: 5353} + +// multicastInterfaces and listenMulticastPacket are vars so tests can select +// a stable interface and responder without touching the host LAN. +var multicastInterfaces = net.Interfaces + +// listenMulticastPacket opens an ephemeral socket on one selected interface. +// It deliberately does not bind port 5353: avahi-daemon may already own it, +// and the QU bit asks responders to reply directly to this socket instead. +var listenMulticastPacket = func(network string, iface *net.Interface, group *net.UDPAddr) (*net.UDPConn, error) { + return net.ListenMulticastUDP(network, iface, &net.UDPAddr{ + IP: group.IP, + Port: 0, + Zone: group.Zone, + }) } // classQU is IN with the RFC 6762 unicast-response bit set. @@ -46,57 +57,205 @@ func queryAddrs(ctx context.Context, name string) ([]netip.Addr, time.Duration, return nil, 0, fmt.Errorf("mdns: pack query: %w", err) } - var ( + lookupCtx, cancel := context.WithCancel(ctx) + defer cancel() + type result struct { + family int + addrs []netip.Addr + ttl time.Duration + err error + } + results := make(chan result, 2) + go func() { + addrs, ttl, err := queryIPv4(lookupCtx, packed, name) + results <- result{family: 4, addrs: addrs, ttl: ttl, err: err} + }() + go func() { + addrs, ttl, err := queryIPv6(lookupCtx, packed, name) + results <- result{family: 6, addrs: addrs, ttl: ttl, err: err} + }() + + var v4, v6 result + for i := 0; i < 2; i++ { + got := <-results + if got.family == 4 { + v4 = got + } else { + v6 = got + } + if got.family == 4 && got.err == nil && len(got.addrs) > 0 { + // One family is enough to dial the device. Cancel the other + // family, then still collect its result before returning so no + // query goroutine outlives this lookup. Do not cancel on IPv6 + // first: IPv4 remains the preferred address family. + cancel() + } + } + + addrs := appendUnique(nil, v4.addrs...) + addrs = appendUnique(addrs, v6.addrs...) + if len(addrs) == 0 { + return nil, 0, fmt.Errorf("mdns: no usable answer (IPv4: %v; IPv6: %v)", v4.err, v6.err) + } + ttl := minAnswerTTL(v4.ttl, v6.ttl) + return addrs, ttl, nil +} + +func queryIPv4(ctx context.Context, packed []byte, name string) ([]netip.Addr, time.Duration, error) { + ifaces, err := eligibleMulticastInterfaces() + if err != nil { + return nil, 0, fmt.Errorf("mdns: list IPv4 multicast interfaces: %w", err) + } + return queryInterfaces(ctx, packed, name, ifaces, "udp4", mdnsAddr, "") +} + +func queryIPv6(ctx context.Context, packed []byte, name string) ([]netip.Addr, time.Duration, error) { + ifaces, err := eligibleMulticastInterfaces() + if err != nil { + return nil, 0, fmt.Errorf("mdns: list IPv6 multicast interfaces: %w", err) + } + return queryInterfaces(ctx, packed, name, ifaces, "udp6", mdnsAddr6, "interface") +} + +func eligibleMulticastInterfaces() ([]net.Interface, error) { + ifaces, err := multicastInterfaces() + if err != nil { + return nil, err + } + eligible := make([]net.Interface, 0, len(ifaces)) + for _, iface := range ifaces { + if iface.Index <= 0 || iface.Flags&net.FlagUp == 0 || + iface.Flags&net.FlagMulticast == 0 || iface.Flags&net.FlagLoopback != 0 { + continue + } + eligible = append(eligible, iface) + } + if len(eligible) == 0 { + return nil, fmt.Errorf("no active non-loopback multicast interface") + } + return eligible, nil +} + +func queryInterfaces(ctx context.Context, packed []byte, name string, ifaces []net.Interface, network string, group *net.UDPAddr, zoneMode string) ([]netip.Addr, time.Duration, error) { + if len(ifaces) == 0 { + return nil, 0, fmt.Errorf("no multicast interface") + } + type result struct { addrs []netip.Addr ttl time.Duration + err error + } + queryCtx, cancel := context.WithCancel(ctx) + defer cancel() + results := make(chan result, len(ifaces)) + for _, iface := range ifaces { + iface := iface + go func() { + target := *group + if zoneMode == "interface" && target.IP.IsLinkLocalMulticast() { + target.Zone = iface.Name + } + _, _, err := exchange(queryCtx, packed, &target, func() (*net.UDPConn, error) { + return listenMulticastPacket(network, &iface, &target) + }, func(packet []byte, source *net.UDPAddr) bool { + zone := "" + if zoneMode == "interface" { + zone = iface.Name + if source != nil && source.Zone != "" { + zone = source.Zone + } + } + got, gotTTL, ok := parseAddrAnswer(packet, name+".", zone) + if !ok { + return false + } + results <- result{addrs: got, ttl: gotTTL} + return true + }) + if err != nil { + results <- result{err: err} + } + }() + } + + var ( + addrs []netip.Addr + ttl time.Duration + firstErr error ) - err = exchange(ctx, packed, func(packet []byte) bool { - got, gotTTL, ok := parseAddrAnswer(packet, name+".") - if !ok { - return false + for i := 0; i < len(ifaces); i++ { + got := <-results + if got.err != nil { + if firstErr == nil { + firstErr = got.err + } + continue + } + addrs = appendUnique(addrs, got.addrs...) + ttl = minAnswerTTL(ttl, got.ttl) + if len(got.addrs) > 0 { + cancel() } - addrs, ttl = got, gotTTL - return true - }) - if err != nil { - return nil, 0, err + } + if len(addrs) == 0 { + if firstErr == nil { + firstErr = fmt.Errorf("no usable answer") + } + return nil, 0, firstErr } return addrs, ttl, nil } // exchange sends one multicast query and feeds every reply to handle until it -// accepts one or the deadline passes. -func exchange(ctx context.Context, packed []byte, handle func([]byte) bool) error { - conn, err := listenPacket() +// accepts one or the deadline passes. Closing the socket on context cancel is +// important: a cancelled family/interface query must not linger until the +// original read deadline. +func exchange(ctx context.Context, packed []byte, target *net.UDPAddr, open func() (*net.UDPConn, error), handle func([]byte, *net.UDPAddr) bool) ([]netip.Addr, time.Duration, error) { + conn, err := open() if err != nil { - return fmt.Errorf("mdns: open socket: %w", err) + return nil, 0, fmt.Errorf("mdns: open socket: %w", err) } + done := make(chan struct{}) + defer close(done) defer conn.Close() + go func() { + select { + case <-ctx.Done(): + _ = conn.Close() + case <-done: + } + }() deadline := now().Add(queryTimeout) if d, ok := ctx.Deadline(); ok && d.Before(deadline) { deadline = d } if err := conn.SetDeadline(deadline); err != nil { - return fmt.Errorf("mdns: set deadline: %w", err) + return nil, 0, fmt.Errorf("mdns: set deadline: %w", err) } - if _, err := conn.WriteToUDP(packed, mdnsAddr); err != nil { - return fmt.Errorf("mdns: send query: %w", err) + if _, err := conn.WriteToUDP(packed, target); err != nil { + return nil, 0, fmt.Errorf("mdns: send query: %w", err) } buf := make([]byte, 1500) for { - n, _, err := conn.ReadFromUDP(buf) + n, source, err := conn.ReadFromUDP(buf) if err != nil { - return fmt.Errorf("mdns: no usable answer: %w", err) + return nil, 0, fmt.Errorf("mdns: no usable answer: %w", err) } - if handle(buf[:n]) { - return nil + if handle(buf[:n], source) { + // The callback stores the parsed answer in its closure. The + // caller only needs the success signal here. + return nil, 0, nil } } } -func parseAddrAnswer(packet []byte, qname string) ([]netip.Addr, time.Duration, bool) { +func parseAddrAnswer(packet []byte, qname string, zones ...string) ([]netip.Addr, time.Duration, bool) { + zone := "" + if len(zones) > 0 { + zone = zones[0] + } var p dnsmessage.Parser if _, err := p.Start(packet); err != nil { return nil, 0, false @@ -133,7 +292,16 @@ parse: break parse } // Unmap so a v4-mapped AAAA dials as plain IPv4. - addrs = append(addrs, netip.AddrFrom16(r.AAAA).Unmap()) + addr := netip.AddrFrom16(r.AAAA).Unmap() + if addr.Is6() && addr.IsLinkLocalUnicast() { + // A link-local address without a zone is not a safe dial + // target: the kernel cannot know which interface to use. + if zone == "" { + continue + } + addr = addr.WithZone(zone) + } + addrs = append(addrs, addr) default: if err := p.SkipAnswer(); err != nil { break parse @@ -159,3 +327,35 @@ func finishAnswer(addrs []netip.Addr, ttl time.Duration) ([]netip.Addr, time.Dur } return addrs, ttl, true } + +func appendUnique(dst []netip.Addr, src ...netip.Addr) []netip.Addr { + for _, candidate := range src { + seen := false + for _, existing := range dst { + if existing == candidate { + seen = true + break + } + } + if !seen { + dst = append(dst, candidate) + } + } + return dst +} + +func minAnswerTTL(values ...time.Duration) time.Duration { + var min time.Duration + for _, value := range values { + if value <= 0 { + continue + } + if min == 0 || value < min { + min = value + } + } + if min == 0 { + return minTTL + } + return min +} diff --git a/scripts/test-container-boundaries.sh b/scripts/test-container-boundaries.sh index 1b4c40d5..bc3bae61 100755 --- a/scripts/test-container-boundaries.sh +++ b/scripts/test-container-boundaries.sh @@ -38,4 +38,19 @@ fi grep -q '^ ftw-optimizer:' docker-compose.yml grep -q 'FTW_OPTIMIZER_SOCKET: /run/ftw-optimizer/optimizer.sock' docker-compose.yml +# mDNS container contract: the static core resolves names itself, direct +# multicast needs Linux host networking, and the optional Avahi bind is the +# whole runtime directory. A socket-file bind is unsafe because Docker creates +# a directory at a missing file mount point. +grep -Eq '^ network_mode: host$' docker-compose.yml +grep -q 'libnss-mdns' Dockerfile +grep -q -- '# - /run/avahi-daemon:/run/avahi-daemon:ro' docker-compose.yml +if grep -Eq '^[[:space:]]+-[[:space:]]*/run/avahi-daemon/socket' docker-compose.yml; then + echo "avahi must be mounted as a runtime directory, not a socket file" >&2 + exit 1 +fi +if command -v docker >/dev/null 2>&1 && docker compose version >/dev/null 2>&1; then + docker compose -f docker-compose.yml config --quiet +fi + echo "container module boundaries verified" From 32651ccbfedd60db67380377e4a1a2d27bb41c46 Mon Sep 17 00:00:00 2001 From: Fredrik Ahlgren Date: Tue, 4 Aug 2026 13:06:03 +0200 Subject: [PATCH 3/7] fix(net): validate mDNS answer provenance --- .changeset/resolve-local-device-names.md | 7 +- go/internal/mdnsresolve/avahi.go | 54 +++++--- go/internal/mdnsresolve/avahi_test.go | 30 +++-- go/internal/mdnsresolve/mdnsresolve_test.go | 137 +++++++++++++++++--- go/internal/mdnsresolve/multicast.go | 98 ++++++++++---- 5 files changed, 256 insertions(+), 70 deletions(-) diff --git a/.changeset/resolve-local-device-names.md b/.changeset/resolve-local-device-names.md index b81a2a9a..ea90cbb8 100644 --- a/.changeset/resolve-local-device-names.md +++ b/.changeset/resolve-local-device-names.md @@ -38,5 +38,8 @@ the container is bridged, so configure devices by IP there. Direct queries use each active multicast interface. IPv4 and IPv6 are supported; link-local IPv6 addresses carry their interface zone and unscoped -answers are rejected. mDNS is unauthenticated, so reserve control-device names -on the LAN and use TLS certificate pins where available. +answers are rejected. The resolver also rejects non-response DNS packets, +wrong answer classes or families, invalid sources, and Avahi replies whose +interface, name, address family or address does not match the request. mDNS is +unauthenticated, so reserve control-device names on the LAN and use TLS +certificate pins where available. diff --git a/go/internal/mdnsresolve/avahi.go b/go/internal/mdnsresolve/avahi.go index de7b1d3f..c3c7f32f 100644 --- a/go/internal/mdnsresolve/avahi.go +++ b/go/internal/mdnsresolve/avahi.go @@ -53,6 +53,11 @@ type avahiResult struct { err error } +const ( + avahiProtocolIPv4 = 0 + avahiProtocolIPv6 = 1 +) + // avahiLookup asks avahi-daemon to resolve name, preferring IPv4. // // The two address families are asked concurrently on separate connections @@ -99,12 +104,16 @@ func avahiLookup(ctx context.Context, name string) ([]netip.Addr, error) { // // The wire format is a single line each way. A success is: // -// +
+// "+
" // // and a failure is "- ". There is no framing, no length // prefix and nothing to decode, which is the point of using it: no DNS wire // format is parsed anywhere on this path. func avahiResolve(ctx context.Context, command, name string) avahiResult { + expectedProtocol, ok := avahiProtocolForCommand(command) + if !ok { + return avahiResult{err: fmt.Errorf("unsupported resolve command %q", command)} + } conn, err := avahiDial(ctx, avahiSocket) if err != nil { return avahiResult{err: fmt.Errorf("connect %s: %w", avahiSocket, err)} @@ -135,10 +144,21 @@ func avahiResolve(ctx context.Context, command, name string) avahiResult { } return avahiResult{err: fmt.Errorf("no reply")} } - return parseAvahiReply(scanner.Text(), name) + return parseAvahiReply(scanner.Text(), name, expectedProtocol) } -func parseAvahiReply(line, expectedName string) avahiResult { +func avahiProtocolForCommand(command string) (int, bool) { + switch command { + case "RESOLVE-HOSTNAME-IPV4": + return avahiProtocolIPv4, true + case "RESOLVE-HOSTNAME-IPV6": + return avahiProtocolIPv6, true + default: + return 0, false + } +} + +func parseAvahiReply(line, expectedName string, expectedProtocol int) avahiResult { fields := strings.Fields(line) if len(fields) == 0 { return avahiResult{err: fmt.Errorf("empty reply")} @@ -155,10 +175,17 @@ func parseAvahiReply(line, expectedName string) avahiResult { if expectedName != "" && canonical(fields[3]) != canonical(expectedName) { return avahiResult{err: fmt.Errorf("reply name %q does not match %q", fields[3], expectedName)} } + interfaceIndex, err := strconv.Atoi(fields[1]) + if err != nil || interfaceIndex <= 0 { + return avahiResult{err: fmt.Errorf("unparsable interface %q", fields[1])} + } protocol, err := strconv.Atoi(fields[2]) if err != nil { return avahiResult{err: fmt.Errorf("unparsable protocol %q", fields[2])} } + if protocol != expectedProtocol { + return avahiResult{err: fmt.Errorf("protocol %d does not match requested protocol %d", protocol, expectedProtocol)} + } addr, err := netip.ParseAddr(fields[4]) if err != nil { return avahiResult{err: fmt.Errorf("unparsable address %q", fields[4])} @@ -166,20 +193,15 @@ func parseAvahiReply(line, expectedName string) avahiResult { if (addr.Is4() && protocol != 0) || (addr.Is6() && protocol != 1) { return avahiResult{err: fmt.Errorf("protocol %d does not match address %s", protocol, addr)} } - if addr.Is6() && addr.IsLinkLocalUnicast() { - interfaceIndex, err := strconv.Atoi(fields[1]) - if err != nil || interfaceIndex <= 0 { - return avahiResult{err: fmt.Errorf("link-local address %s has no interface", addr)} - } - iface, err := avahiInterfaceByIndex(interfaceIndex) - if err != nil || iface == nil || iface.Name == "" || - iface.Index != interfaceIndex || iface.Flags&net.FlagUp == 0 || - iface.Flags&net.FlagMulticast == 0 || iface.Flags&net.FlagLoopback != 0 { - if err == nil { - err = fmt.Errorf("interface %d is unavailable or not multicast-capable", interfaceIndex) - } - return avahiResult{err: fmt.Errorf("link-local address %s: %w", addr, err)} + iface, err := avahiInterfaceByIndex(interfaceIndex) + if err != nil || iface == nil || iface.Name == "" || iface.Index != interfaceIndex || + iface.Flags&net.FlagUp == 0 || iface.Flags&net.FlagMulticast == 0 || iface.Flags&net.FlagLoopback != 0 { + if err == nil { + err = fmt.Errorf("interface %d is unavailable or not multicast-capable", interfaceIndex) } + return avahiResult{err: fmt.Errorf("interface %d: %w", interfaceIndex, err)} + } + if addr.Is6() && addr.IsLinkLocalUnicast() { addr = addr.WithZone(iface.Name) } return avahiResult{addr: addr.Unmap()} diff --git a/go/internal/mdnsresolve/avahi_test.go b/go/internal/mdnsresolve/avahi_test.go index 05d01f1f..d788c3f9 100644 --- a/go/internal/mdnsresolve/avahi_test.go +++ b/go/internal/mdnsresolve/avahi_test.go @@ -55,20 +55,24 @@ func fakeAvahi(t *testing.T, reply func(command, name string) string) { func TestParseAvahiReply(t *testing.T) { cases := []struct { - name string - line string - want string // empty means the reply must be rejected + name string + line string + protocol int + want string // empty means the reply must be rejected }{ // The exact shape avahi-daemon returns, confirmed against the daemon. - {"ipv4", "+ 2 0 zap.local 192.168.1.42", "192.168.1.42"}, - {"ipv6", "+ 2 1 zap.local fe80::1", "fe80::1%test0"}, + {"ipv4", "+ 2 0 zap.local 192.168.1.42", 0, "192.168.1.42"}, + {"ipv6", "+ 2 1 zap.local fe80::1", 1, "fe80::1%test0"}, // A v4-mapped answer must dial as plain IPv4. - {"v4 mapped", "+ 2 1 zap.local ::ffff:192.168.1.42", "192.168.1.42"}, - {"wrong name", "+ 2 0 other.local 192.168.1.42", ""}, - {"failure", "- 15 Timeout reached", ""}, - {"empty", "", ""}, - {"truncated", "+ 2 0 zap.local", ""}, - {"unparsable address", "+ 2 0 zap.local not-an-address", ""}, + {"v4 mapped", "+ 2 1 zap.local ::ffff:192.168.1.42", 1, "192.168.1.42"}, + {"wrong name", "+ 2 0 other.local 192.168.1.42", 0, ""}, + {"nonnumeric interface", "+ eth0 0 zap.local 192.168.1.42", 0, ""}, + {"ipv6 answer to ipv4 command", "+ 2 1 zap.local fe80::1", 0, ""}, + {"ipv4 answer to ipv6 command", "+ 2 0 zap.local 192.168.1.42", 1, ""}, + {"failure", "- 15 Timeout reached", 0, ""}, + {"empty", "", 0, ""}, + {"truncated", "+ 2 0 zap.local", 0, ""}, + {"unparsable address", "+ 2 0 zap.local not-an-address", 0, ""}, } origInterfaceByIndex := avahiInterfaceByIndex avahiInterfaceByIndex = func(index int) (*net.Interface, error) { @@ -77,7 +81,7 @@ func TestParseAvahiReply(t *testing.T) { t.Cleanup(func() { avahiInterfaceByIndex = origInterfaceByIndex }) for _, c := range cases { t.Run(c.name, func(t *testing.T) { - got := parseAvahiReply(c.line, "zap.local") + got := parseAvahiReply(c.line, "zap.local", c.protocol) if c.want == "" { if got.err == nil { t.Fatalf("accepted %q, got %v", c.line, got.addr) @@ -97,7 +101,7 @@ func TestParseAvahiReply(t *testing.T) { // A failure line must carry avahi's own wording, so the log says what the // daemon said rather than something this package made up. func TestParseAvahiReplyKeepsDaemonWording(t *testing.T) { - got := parseAvahiReply("- 15 Timeout reached", "zap.local") + got := parseAvahiReply("- 15 Timeout reached", "zap.local", avahiProtocolIPv4) if got.err == nil { t.Fatal("expected an error") } diff --git a/go/internal/mdnsresolve/mdnsresolve_test.go b/go/internal/mdnsresolve/mdnsresolve_test.go index 6cd139d0..f548c8a6 100644 --- a/go/internal/mdnsresolve/mdnsresolve_test.go +++ b/go/internal/mdnsresolve/mdnsresolve_test.go @@ -70,11 +70,13 @@ func aaaaResource(t *testing.T, name string, ip [16]byte, ttl uint32) dnsmessage func packAnswer(t *testing.T, qname string, answers []dnsmessage.Resource) []byte { t.Helper() - msg := dnsmessage.Message{ - Header: dnsmessage.Header{Response: true, Authoritative: true}, - Questions: []dnsmessage.Question{{Name: mustDNSName(t, qname), Type: dnsmessage.TypeA, Class: dnsmessage.ClassINET}}, - Answers: answers, - } + return packDNSMessage(t, dnsmessage.Header{Response: true, Authoritative: true}, + []dnsmessage.Question{{Name: mustDNSName(t, qname), Type: dnsmessage.TypeA, Class: dnsmessage.ClassINET}}, answers) +} + +func packDNSMessage(t *testing.T, header dnsmessage.Header, questions []dnsmessage.Question, answers []dnsmessage.Resource) []byte { + t.Helper() + msg := dnsmessage.Message{Header: header, Questions: questions, Answers: answers} packet, err := msg.Pack() if err != nil { t.Fatalf("pack: %v", err) @@ -82,11 +84,44 @@ func packAnswer(t *testing.T, qname string, answers []dnsmessage.Resource) []byt return packet } +func parseTestAddrAnswer(t *testing.T, packet []byte, qname, network string) ([]netip.Addr, time.Duration, bool) { + t.Helper() + var sourceIP string + if network == "udp6" { + sourceIP = "2001:db8::1" + } else { + sourceIP = "192.0.2.1" + } + return parseAddrAnswer(packet, qname, &net.UDPAddr{ + IP: net.ParseIP(sourceIP), + Port: 5353, + }, network, &net.Interface{ + Index: 1, + Name: "test0", + Flags: net.FlagUp | net.FlagMulticast, + }) +} + +func TestParseAddrAnswerRequiresResponseBitButNotQuestion(t *testing.T) { + qname := "inverter.local." + answers := []dnsmessage.Resource{aResource(t, qname, [4]byte{192, 168, 1, 42}, 60)} + + withoutQuestion := packDNSMessage(t, dnsmessage.Header{Response: true, Authoritative: true}, nil, answers) + if _, _, ok := parseTestAddrAnswer(t, withoutQuestion, qname, "udp4"); !ok { + t.Fatal("rejected a valid mDNS response without an echoed question") + } + + query := packDNSMessage(t, dnsmessage.Header{Response: false}, nil, answers) + if _, _, ok := parseTestAddrAnswer(t, query, qname, "udp4"); ok { + t.Fatal("accepted a DNS query with answer records as an mDNS response") + } +} + func TestParseAddrAnswer(t *testing.T) { qname := "inverter.local." packet := packAnswer(t, qname, []dnsmessage.Resource{aResource(t, qname, [4]byte{192, 168, 1, 42}, 60)}) - addrs, ttl, ok := parseAddrAnswer(packet, qname) + addrs, ttl, ok := parseTestAddrAnswer(t, packet, qname, "udp4") if !ok { t.Fatal("parseAddrAnswer did not accept a valid answer") } @@ -98,15 +133,70 @@ func TestParseAddrAnswer(t *testing.T) { } // An answer for a different name must be ignored. - if _, _, ok := parseAddrAnswer(packet, "other.local."); ok { + if _, _, ok := parseTestAddrAnswer(t, packet, "other.local.", "udp4"); ok { t.Fatal("accepted an answer for a different name") } // Garbage must not panic or resolve. - if _, _, ok := parseAddrAnswer([]byte{1, 2, 3}, qname); ok { + if _, _, ok := parseTestAddrAnswer(t, []byte{1, 2, 3}, qname, "udp4"); ok { t.Fatal("accepted a malformed packet") } } +func TestParseAddrAnswerValidatesSourceFamilyAndClass(t *testing.T) { + qname := "inverter.local." + answer := aResource(t, qname, [4]byte{192, 168, 1, 42}, 60) + packet := packAnswer(t, qname, []dnsmessage.Resource{answer}) + iface := &net.Interface{Index: 1, Name: "test0", Flags: net.FlagUp | net.FlagMulticast} + + for _, tc := range []struct { + name string + source *net.UDPAddr + network string + }{ + {"nil source", nil, "udp4"}, + {"wrong source port", &net.UDPAddr{IP: net.ParseIP("192.0.2.1"), Port: 5354}, "udp4"}, + {"wrong source family", &net.UDPAddr{IP: net.ParseIP("2001:db8::1"), Port: 5353}, "udp4"}, + {"unspecified source", &net.UDPAddr{IP: net.ParseIP("0.0.0.0"), Port: 5353}, "udp4"}, + {"multicast source", &net.UDPAddr{IP: net.ParseIP("224.0.0.251"), Port: 5353}, "udp4"}, + {"wrong requested family", &net.UDPAddr{IP: net.ParseIP("192.0.2.1"), Port: 5353}, "udp6"}, + } { + t.Run(tc.name, func(t *testing.T) { + if _, _, ok := parseAddrAnswer(packet, qname, tc.source, tc.network, iface); ok { + t.Fatalf("accepted source=%v network=%s", tc.source, tc.network) + } + }) + } + + wrongClass := answer + wrongClass.Header.Class = dnsmessage.ClassCHAOS + if _, _, ok := parseTestAddrAnswer(t, packAnswer(t, qname, []dnsmessage.Resource{wrongClass}), qname, "udp4"); ok { + t.Fatal("accepted an A answer from the wrong DNS class") + } + + cacheFlush := answer + cacheFlush.Header.Class |= classCacheFlush + if _, _, ok := parseTestAddrAnswer(t, packAnswer(t, qname, []dnsmessage.Resource{cacheFlush}), qname, "udp4"); !ok { + t.Fatal("rejected an IN answer carrying the mDNS cache-flush bit") + } +} + +func TestParseAddrAnswerRejectsAnswerFromWrongFamily(t *testing.T) { + qname := "inverter.local." + iface := &net.Interface{Index: 1, Name: "test0", Flags: net.FlagUp | net.FlagMulticast} + + ipv6 := packDNSMessage(t, dnsmessage.Header{Response: true}, nil, []dnsmessage.Resource{ + aaaaResource(t, qname, [16]byte{0x20, 1, 0xdb, 8, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1}, 60), + }) + if _, _, ok := parseAddrAnswer(ipv6, qname, &net.UDPAddr{IP: net.ParseIP("192.0.2.1"), Port: 5353}, "udp4", iface); ok { + t.Fatal("accepted an IPv6 answer on the IPv4 query path") + } + + ipv4 := packAnswer(t, qname, []dnsmessage.Resource{aResource(t, qname, [4]byte{192, 168, 1, 42}, 60)}) + if _, _, ok := parseAddrAnswer(ipv4, qname, &net.UDPAddr{IP: net.ParseIP("2001:db8::1"), Port: 5353}, "udp6", iface); ok { + t.Fatal("accepted an IPv4 answer on the IPv6 query path") + } +} + func TestParseAddrAnswerClampsTTL(t *testing.T) { qname := "inverter.local." for _, c := range []struct { @@ -123,7 +213,7 @@ func TestParseAddrAnswerClampsTTL(t *testing.T) { } { t.Run(c.name, func(t *testing.T) { packet := packAnswer(t, qname, []dnsmessage.Resource{aResource(t, qname, [4]byte{10, 0, 0, 1}, c.ttl)}) - _, ttl, ok := parseAddrAnswer(packet, qname) + _, ttl, ok := parseTestAddrAnswer(t, packet, qname, "udp4") if !ok { t.Fatal("answer rejected") } @@ -134,15 +224,19 @@ func TestParseAddrAnswerClampsTTL(t *testing.T) { } } -func TestParseAddrAnswerRequiresInterfaceZoneForLinkLocalIPv6(t *testing.T) { +func TestParseAddrAnswerUsesSelectedInterfaceForLinkLocalIPv6(t *testing.T) { qname := "inverter.local." packet := packAnswer(t, qname, []dnsmessage.Resource{ aaaaResource(t, qname, [16]byte{0xfe, 0x80, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1}, 60), }) - if _, _, ok := parseAddrAnswer(packet, qname); ok { - t.Fatal("accepted link-local IPv6 answer without an interface zone") - } - addrs, _, ok := parseAddrAnswer(packet, qname, "test0") + source := &net.UDPAddr{IP: net.ParseIP("2001:db8::1"), Port: 5353} + if _, _, ok := parseAddrAnswer(packet, qname, source, "udp6", &net.Interface{ + Index: 1, + Flags: net.FlagUp | net.FlagMulticast, + }); ok { + t.Fatal("accepted link-local IPv6 answer without a valid selected interface") + } + addrs, _, ok := parseTestAddrAnswer(t, packet, qname, "udp6") if !ok || len(addrs) != 1 || addrs[0].String() != "fe80::1%test0" { t.Fatalf("zoned answer = %v, ok=%v; want [fe80::1%%test0]", addrs, ok) } @@ -164,6 +258,11 @@ func TestQueryIPv6UsesSelectedInterfaceForLinkLocalAnswer(t *testing.T) { t.Skipf("IPv6 loopback unavailable: %v", err) } defer responder.Close() + responseConn, err := net.ListenUDP("udp6", &net.UDPAddr{IP: net.ParseIP("::1"), Port: mdnsPort}) + if err != nil { + t.Skipf("IPv6 mDNS response port unavailable: %v", err) + } + defer responseConn.Close() origAddr, origInterfaces, origListen := mdnsAddr6, multicastInterfaces, listenMulticastPacket mdnsAddr6 = responder.LocalAddr().(*net.UDPAddr) @@ -205,7 +304,7 @@ func TestQueryIPv6UsesSelectedInterfaceForLinkLocalAnswer(t *testing.T) { } response, err := responseMessage.Pack() if err == nil { - _, _ = responder.WriteToUDP(response, from) + _, _ = responseConn.WriteToUDP(response, from) } }() @@ -257,6 +356,11 @@ func startResponder(t *testing.T, answers []dnsmessage.Resource) { if err != nil { t.Fatalf("listen responder: %v", err) } + responseConn, err := net.ListenUDP("udp4", &net.UDPAddr{IP: net.IPv4(127, 0, 0, 1), Port: mdnsPort}) + if err != nil { + _ = rc.Close() + t.Skipf("mDNS response port unavailable: %v", err) + } origAddr, origMulticast, origInterfaces := mdnsAddr, listenMulticastPacket, multicastInterfaces mdnsAddr = rc.LocalAddr().(*net.UDPAddr) @@ -300,12 +404,13 @@ func startResponder(t *testing.T, answers []dnsmessage.Resource) { if err != nil { return } - _, _ = rc.WriteToUDP(packed, from) + _, _ = responseConn.WriteToUDP(packed, from) }() t.Cleanup(func() { _ = rc.Close() <-done + _ = responseConn.Close() mdnsAddr = origAddr listenMulticastPacket, multicastInterfaces = origMulticast, origInterfaces Flush() diff --git a/go/internal/mdnsresolve/multicast.go b/go/internal/mdnsresolve/multicast.go index c151fe6b..408c13c3 100644 --- a/go/internal/mdnsresolve/multicast.go +++ b/go/internal/mdnsresolve/multicast.go @@ -15,13 +15,15 @@ import ( // 6762 query, used only where avahi-daemon's socket cannot be reached. It is // deliberately the second choice — see avahi.go for the first. +const mdnsPort = 5353 + // mdnsAddr is the RFC 6762 IPv4 multicast group. A var, not a const, so tests // can aim a query at a loopback responder. -var mdnsAddr = &net.UDPAddr{IP: net.IPv4(224, 0, 0, 251), Port: 5353} +var mdnsAddr = &net.UDPAddr{IP: net.IPv4(224, 0, 0, 251), Port: mdnsPort} // mdnsAddr6 is the RFC 6762 IPv6 multicast group. The interface zone is added // to a copy for each query because ff02::fb is link-local by definition. -var mdnsAddr6 = &net.UDPAddr{IP: net.ParseIP("ff02::fb"), Port: 5353} +var mdnsAddr6 = &net.UDPAddr{IP: net.ParseIP("ff02::fb"), Port: mdnsPort} // multicastInterfaces and listenMulticastPacket are vars so tests can select // a stable interface and responder without touching the host LAN. @@ -41,6 +43,10 @@ var listenMulticastPacket = func(network string, iface *net.Interface, group *ne // classQU is IN with the RFC 6762 unicast-response bit set. const classQU = dnsmessage.Class(0x8001) +// mDNS answer classes may carry the cache-flush bit in the high bit. It is a +// record flag, not a different DNS class. +const classCacheFlush = dnsmessage.Class(0x8000) + func queryAddrs(ctx context.Context, name string) ([]netip.Addr, time.Duration, error) { qname, err := dnsmessage.NewName(name + ".") if err != nil { @@ -124,8 +130,7 @@ func eligibleMulticastInterfaces() ([]net.Interface, error) { } eligible := make([]net.Interface, 0, len(ifaces)) for _, iface := range ifaces { - if iface.Index <= 0 || iface.Flags&net.FlagUp == 0 || - iface.Flags&net.FlagMulticast == 0 || iface.Flags&net.FlagLoopback != 0 { + if !validMulticastInterface(&iface) { continue } eligible = append(eligible, iface) @@ -136,6 +141,12 @@ func eligibleMulticastInterfaces() ([]net.Interface, error) { return eligible, nil } +func validMulticastInterface(iface *net.Interface) bool { + return iface != nil && iface.Index > 0 && iface.Name != "" && + iface.Flags&net.FlagUp != 0 && iface.Flags&net.FlagMulticast != 0 && + iface.Flags&net.FlagLoopback == 0 +} + func queryInterfaces(ctx context.Context, packed []byte, name string, ifaces []net.Interface, network string, group *net.UDPAddr, zoneMode string) ([]netip.Addr, time.Duration, error) { if len(ifaces) == 0 { return nil, 0, fmt.Errorf("no multicast interface") @@ -158,14 +169,7 @@ func queryInterfaces(ctx context.Context, packed []byte, name string, ifaces []n _, _, err := exchange(queryCtx, packed, &target, func() (*net.UDPConn, error) { return listenMulticastPacket(network, &iface, &target) }, func(packet []byte, source *net.UDPAddr) bool { - zone := "" - if zoneMode == "interface" { - zone = iface.Name - if source != nil && source.Zone != "" { - zone = source.Zone - } - } - got, gotTTL, ok := parseAddrAnswer(packet, name+".", zone) + got, gotTTL, ok := parseAddrAnswer(packet, name+".", source, network, &iface) if !ok { return false } @@ -251,13 +255,13 @@ func exchange(ctx context.Context, packed []byte, target *net.UDPAddr, open func } } -func parseAddrAnswer(packet []byte, qname string, zones ...string) ([]netip.Addr, time.Duration, bool) { - zone := "" - if len(zones) > 0 { - zone = zones[0] +func parseAddrAnswer(packet []byte, qname string, source *net.UDPAddr, network string, iface *net.Interface) ([]netip.Addr, time.Duration, bool) { + if !validMulticastInterface(iface) || !validMDNSSource(source, network, iface) { + return nil, 0, false } var p dnsmessage.Parser - if _, err := p.Start(packet); err != nil { + header, err := p.Start(packet) + if err != nil || !header.Response || header.RCode != dnsmessage.RCodeSuccess { return nil, 0, false } if err := p.SkipAllQuestions(); err != nil { @@ -271,11 +275,16 @@ parse: for { h, err := p.AnswerHeader() if err != nil { - break parse + if err == dnsmessage.ErrSectionDone { + break parse + } + return nil, 0, false } - if !strings.EqualFold(h.Name.String(), qname) { + if !strings.EqualFold(h.Name.String(), qname) || + h.Class&^classCacheFlush != dnsmessage.ClassINET || + !answerTypeMatchesNetwork(h.Type, network) { if err := p.SkipAnswer(); err != nil { - break parse + return nil, 0, false } continue } @@ -296,15 +305,15 @@ parse: if addr.Is6() && addr.IsLinkLocalUnicast() { // A link-local address without a zone is not a safe dial // target: the kernel cannot know which interface to use. - if zone == "" { + if iface.Name == "" { continue } - addr = addr.WithZone(zone) + addr = addr.WithZone(iface.Name) } addrs = append(addrs, addr) default: if err := p.SkipAnswer(); err != nil { - break parse + return nil, 0, false } continue } @@ -315,6 +324,49 @@ parse: return finishAnswer(addrs, ttl) } +func answerTypeMatchesNetwork(typ dnsmessage.Type, network string) bool { + switch network { + case "udp4": + return typ == dnsmessage.TypeA + case "udp6": + return typ == dnsmessage.TypeAAAA + default: + return false + } +} + +func validMDNSSource(source *net.UDPAddr, network string, iface *net.Interface) bool { + // RFC 6762 requires every mDNS response to use the well-known source + // port, including a response sent directly to a QU query's ephemeral port. + if source == nil || source.Port != mdnsPort || source.IP == nil || !validMulticastInterface(iface) { + return false + } + ip, ok := netip.AddrFromSlice(source.IP) + if !ok { + return false + } + ip = ip.Unmap() + if ip.IsUnspecified() || ip.IsMulticast() { + return false + } + switch network { + case "udp4": + if !ip.Is4() { + return false + } + case "udp6": + if !ip.Is6() { + return false + } + if source.Zone != "" && source.Zone != iface.Name { + return false + } + default: + return false + } + return true +} + func finishAnswer(addrs []netip.Addr, ttl time.Duration) ([]netip.Addr, time.Duration, bool) { if len(addrs) == 0 { return nil, 0, false From 0f5635b2032daec33d6e2e2562a63c89e34a6548 Mon Sep 17 00:00:00 2001 From: Fredrik Ahlgren Date: Tue, 4 Aug 2026 13:35:34 +0200 Subject: [PATCH 4/7] fix(net): gate unverified local endpoints --- .changeset/resolve-local-device-names.md | 8 +- config.example.yaml | 5 + docs/operations.md | 19 +++ go/cmd/ftw/main.go | 4 +- go/internal/config/config.go | 29 +++- go/internal/config/config_test.go | 44 ++++++ go/internal/drivers/host.go | 10 ++ go/internal/drivers/lua.go | 22 ++- go/internal/drivers/lua_http_test.go | 68 +++++++++ go/internal/drivers/proxy_policy.go | 27 ++++ go/internal/drivers/registry.go | 24 ++-- go/internal/drivers/registry_restart_test.go | 28 ++++ go/internal/drivers/tcp_cap.go | 24 ++-- go/internal/drivers/ws_cap.go | 34 +++-- go/internal/drivers/ws_proxy_test.go | 144 +++++++++++++++++++ go/internal/ha/bridge.go | 5 +- go/internal/mdnsresolve/mdnsresolve.go | 34 ++++- go/internal/mdnsresolve/mdnsresolve_test.go | 51 ++++++- go/internal/modbus/client.go | 34 +++-- go/internal/modbus/tcp_client.go | 28 ++-- go/internal/mqtt/client.go | 11 +- 21 files changed, 579 insertions(+), 74 deletions(-) create mode 100644 go/internal/drivers/proxy_policy.go create mode 100644 go/internal/drivers/ws_proxy_test.go diff --git a/.changeset/resolve-local-device-names.md b/.changeset/resolve-local-device-names.md index ea90cbb8..dd4d0985 100644 --- a/.changeset/resolve-local-device-names.md +++ b/.changeset/resolve-local-device-names.md @@ -41,5 +41,9 @@ supported; link-local IPv6 addresses carry their interface zone and unscoped answers are rejected. The resolver also rejects non-response DNS packets, wrong answer classes or families, invalid sources, and Avahi replies whose interface, name, address family or address does not match the request. mDNS is -unauthenticated, so reserve control-device names on the LAN and use TLS -certificate pins where available. +unauthenticated, so `.local` transport dials are denied by default. Set +`capabilities.allow_unverified_local: true` for a driver, or +`homeassistant.allow_unverified_local: true` for the Home Assistant bridge, +when the operator accepts that trust boundary. Host allowlists do not prove +server identity, and a TLS pin does not bypass the gate yet. Literal IP and +ordinary DNS endpoints are unchanged. diff --git a/config.example.yaml b/config.example.yaml index ff047809..435e90c6 100644 --- a/config.example.yaml +++ b/config.example.yaml @@ -73,6 +73,8 @@ drivers: # - name: ambibox # lua: drivers/ambibox_v2x.lua # capabilities: + # # mDNS does not prove the server identity; opt in per driver only + # allow_unverified_local: true # mqtt: # host: sid-os.local # port: 1883 @@ -90,6 +92,8 @@ drivers: # is_site_meter: true # battery_telemetry_only: true # capabilities: + # # Required for an unauthenticated raw .local endpoint. + # allow_unverified_local: true # http: # allowed_hosts: ["zap.local"] # .local is resolved by FTW over mDNS # config: @@ -136,6 +140,7 @@ api: homeassistant: enabled: false broker: 192.168.1.1 + # allow_unverified_local: true # required for an unauthenticated broker.local port: 1883 username: homeems password: homeems diff --git a/docs/operations.md b/docs/operations.md index b239ae86..3e8dcb01 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -215,6 +215,25 @@ where the driver supports them. Network allowlists still check the configured host name and port before resolution; they do not prove that an mDNS responder is the intended device. +FTW therefore rejects `.local` transport dials by default, before it queries +Avahi or the LAN. To use an unauthenticated `.local` endpoint, opt in on that +driver: + +```yaml +capabilities: + allow_unverified_local: true + modbus: + host: inverter.local + port: 502 +``` + +For the Home Assistant bridge, set `homeassistant.allow_unverified_local: true` +instead. This is required for every mDNS transport, including HTTP, WebSocket, +MQTT, Modbus and raw TCP. A TLS pin does not bypass this gate yet. Literal IP +addresses and ordinary DNS names keep their existing behavior. The opt-in is +per driver so a name allowlist never becomes a server identity for another +driver. + #### Letting FTW use avahi Host networking shares ports, not Unix sockets, so avahi has to be bind-mounted diff --git a/go/cmd/ftw/main.go b/go/cmd/ftw/main.go index 1b75564c..506d1fbe 100644 --- a/go/cmd/ftw/main.go +++ b/go/cmd/ftw/main.go @@ -521,10 +521,10 @@ func main() { } } reg.MQTTFactory = func(name string, c *config.MQTTConfig) (drivers.MQTTCap, error) { - return mqttcli.Dial(c.Host, c.Port, c.Username, c.Password, "ftw-"+name) + return mqttcli.DialWithOptions(c.Host, c.Port, c.Username, c.Password, "ftw-"+name, c.AllowUnverifiedLocal) } reg.ModbusFactory = func(name string, c *config.ModbusConfig) (drivers.ModbusCap, error) { - return modbuscli.Dial(c.Host, c.Port, c.UnitID) + return modbuscli.DialWithOptions(c.Host, c.Port, c.UnitID, c.AllowUnverifiedLocal) } reg.SerialFactory = func(name string, c *config.SerialConfig) (drivers.SerialCap, error) { return drivers.OpenSerial(c) diff --git a/go/internal/config/config.go b/go/internal/config/config.go index 21f5ed0d..a17e6016 100644 --- a/go/internal/config/config.go +++ b/go/internal/config/config.go @@ -797,13 +797,17 @@ type DriverControlOptIn struct { // Capabilities explicitly scope what host resources a driver can access. type Capabilities struct { - MQTT *MQTTConfig `yaml:"mqtt,omitempty" json:"mqtt,omitempty"` - Modbus *ModbusConfig `yaml:"modbus,omitempty" json:"modbus,omitempty"` - Serial *SerialConfig `yaml:"serial,omitempty" json:"serial,omitempty"` - HTTP *HTTPCapability `yaml:"http,omitempty" json:"http,omitempty"` - WebSocket *WSCapability `yaml:"websocket,omitempty" json:"websocket,omitempty"` - TCP *TCPCapability `yaml:"tcp,omitempty" json:"tcp,omitempty"` - Standalone bool `yaml:"standalone,omitempty" json:"standalone,omitempty"` + // AllowUnverifiedLocal permits a driver to use an mDNS-resolved .local + // endpoint when the transport has no cryptographic server identity. The + // name allowlist is not identity, so the safe default is false. + AllowUnverifiedLocal bool `yaml:"allow_unverified_local,omitempty" json:"allow_unverified_local,omitempty"` + MQTT *MQTTConfig `yaml:"mqtt,omitempty" json:"mqtt,omitempty"` + Modbus *ModbusConfig `yaml:"modbus,omitempty" json:"modbus,omitempty"` + Serial *SerialConfig `yaml:"serial,omitempty" json:"serial,omitempty"` + HTTP *HTTPCapability `yaml:"http,omitempty" json:"http,omitempty"` + WebSocket *WSCapability `yaml:"websocket,omitempty" json:"websocket,omitempty"` + TCP *TCPCapability `yaml:"tcp,omitempty" json:"tcp,omitempty"` + Standalone bool `yaml:"standalone,omitempty" json:"standalone,omitempty"` } // MQTTConfig grants access to one MQTT broker. @@ -812,6 +816,10 @@ type MQTTConfig struct { Port int `yaml:"port,omitempty" json:"port,omitempty"` // default 1883 Username string `yaml:"username,omitempty" json:"username,omitempty"` Password string `yaml:"password,omitempty" json:"password,omitempty"` + // AllowUnverifiedLocal is copied from capabilities.allow_unverified_local + // by the core before this config reaches the transport factory. It is + // runtime-only and never comes from this nested YAML block. + AllowUnverifiedLocal bool `yaml:"-" json:"-"` } // ModbusConfig grants access to one Modbus TCP endpoint. @@ -819,6 +827,10 @@ type ModbusConfig struct { Host string `yaml:"host" json:"host"` Port int `yaml:"port,omitempty" json:"port,omitempty"` // default 502 UnitID int `yaml:"unit_id,omitempty" json:"unit_id,omitempty"` // default 1 + // AllowUnverifiedLocal is copied from capabilities.allow_unverified_local + // by the core before this config reaches the transport factory. It is + // runtime-only and never comes from this nested YAML block. + AllowUnverifiedLocal bool `yaml:"-" json:"-"` } // SerialConfig grants read-only access to one local serial device. @@ -898,6 +910,9 @@ type HomeAssistant struct { Username string `yaml:"username,omitempty" json:"username,omitempty"` Password string `yaml:"password,omitempty" json:"password,omitempty"` PublishIntervalS int `yaml:"publish_interval_s,omitempty" json:"publish_interval_s,omitempty"` + // AllowUnverifiedLocal permits the bridge to use an mDNS-resolved broker + // without a verified server identity. The default is fail-closed. + AllowUnverifiedLocal bool `yaml:"allow_unverified_local,omitempty" json:"allow_unverified_local,omitempty"` } // StateConf is the persistent state DB config. diff --git a/go/internal/config/config_test.go b/go/internal/config/config_test.go index eea190bd..2e1a8e7f 100644 --- a/go/internal/config/config_test.go +++ b/go/internal/config/config_test.go @@ -119,6 +119,50 @@ func TestLoadMinimalYAML(t *testing.T) { } } +func TestAllowUnverifiedLocalDefaultsDenyAndParsesExplicitOptIn(t *testing.T) { + cfg, err := Parse([]byte(minimalYAML), "/tmp") + if err != nil { + t.Fatal(err) + } + if cfg.Drivers[0].Capabilities.AllowUnverifiedLocal { + t.Fatal("allow_unverified_local must default to false") + } + + withOptIn := strings.Replace(minimalYAML, + "capabilities:\n mqtt:", + "capabilities:\n allow_unverified_local: true\n mqtt:", 1) + optedIn, err := Parse([]byte(withOptIn), "/tmp") + if err != nil { + t.Fatal(err) + } + if !optedIn.Drivers[0].Capabilities.AllowUnverifiedLocal { + t.Fatal("explicit allow_unverified_local=true was not retained") + } +} + +func TestHomeAssistantAllowUnverifiedLocalDefaultsDeny(t *testing.T) { + base := minimalYAML + ` +homeassistant: + enabled: true + broker: broker.local +` + cfg, err := Parse([]byte(base), "/tmp") + if err != nil { + t.Fatal(err) + } + if cfg.HomeAssistant.AllowUnverifiedLocal { + t.Fatal("homeassistant allow_unverified_local must default to false") + } + + optedIn, err := Parse([]byte(base+" allow_unverified_local: true\n"), "/tmp") + if err != nil { + t.Fatal(err) + } + if !optedIn.HomeAssistant.AllowUnverifiedLocal { + t.Fatal("homeassistant explicit local opt-in was not retained") + } +} + func TestParseIgnoresRetiredRemoteAccessKeys(t *testing.T) { legacy := minimalYAML + ` remote_access: diff --git a/go/internal/drivers/host.go b/go/internal/drivers/host.go index ce304ce9..3daebb79 100644 --- a/go/internal/drivers/host.go +++ b/go/internal/drivers/host.go @@ -86,6 +86,10 @@ type HostEnv struct { // backward compat with existing drivers that didn't declare a list. // Populated from driver config `capabilities.http.allowed_hosts`. HTTPAllowedHosts []string + // AllowUnverifiedLocal permits all this driver's mDNS-resolved .local + // transports. The name allowlist is not server identity; core keeps this + // false unless the operator explicitly opts in. + AllowUnverifiedLocal bool // HTTPTLSPinSHA256, when non-empty, pins the HTTPS leaf certificate to // this SHA-256 fingerprint (hex; colons/whitespace ignored, case- // insensitive — same value as `openssl x509 -fingerprint -sha256`). @@ -492,6 +496,12 @@ func (h *HostEnv) WithHTTPAllowedHosts(hosts []string) *HostEnv { return h } +// WithAllowUnverifiedLocal permits this driver's raw .local transports. +func (h *HostEnv) WithAllowUnverifiedLocal() *HostEnv { + h.AllowUnverifiedLocal = true + return h +} + // WithHTTPTLSPin pins the HTTPS leaf certificate this driver's http_* // calls will accept, by SHA-256 fingerprint. Empty string = no pin // (standard system-root verification). See HostEnv.HTTPTLSPinSHA256. diff --git a/go/internal/drivers/lua.go b/go/internal/drivers/lua.go index 2a8ec2cd..f67fdd3c 100644 --- a/go/internal/drivers/lua.go +++ b/go/internal/drivers/lua.go @@ -566,6 +566,17 @@ func luaReturnError(name string, ret lua.LValue) error { // ---- host.* API exposed to Lua ---- +func newLuaHTTPTransport(allowUnverifiedLocal bool, proxy func(*net_http.Request) (*net_url.URL, error)) *net_http.Transport { + transport := net_http.DefaultTransport.(*net_http.Transport).Clone() + if proxy == nil { + proxy = transport.Proxy + } + transport.Proxy = guardMDNSProxy(proxy, allowUnverifiedLocal) + mdnsDialer := mdnsresolve.Dialer{AllowUnverifiedLocal: allowUnverifiedLocal} + transport.DialContext = mdnsDialer.DialContext + return transport +} + func registerHost(L *lua.LState, env *HostEnv) { host := L.NewTable() @@ -1160,10 +1171,9 @@ func registerHost(L *lua.LState, env *HostEnv) { // Drivers routinely address a device by its ".local" name, which the // stdlib resolver cannot answer. Clone the default transport so proxying, - // HTTP/2 and connection pooling are all unchanged — only the dial step - // differs, and only for ".local" hosts. - transport := net_http.DefaultTransport.(*net_http.Transport).Clone() - transport.DialContext = mdnsresolve.DialContext + // HTTP/2 and connection pooling are all unchanged. Guard proxy selection + // as well as the dial step: net/http chooses a proxy before DialContext. + transport := newLuaHTTPTransport(env.AllowUnverifiedLocal, nil) httpClient := &net_http.Client{ Timeout: 15 * time.Second, @@ -1193,8 +1203,8 @@ func registerHost(L *lua.LState, env *HostEnv) { // endpoint with a self-signed cert (a NIBE heat pump's local REST API) // without the SSRF-grade hole of blanket InsecureSkipVerify: a swapped // cert (MITM) is rejected at the handshake even if it chains to a real - // CA. Drivers WITHOUT a pin keep Go's default transport untouched, so - // nothing about existing HTTP drivers changes. + // CA. Drivers WITHOUT a pin keep standard system-root certificate + // verification; the shared mDNS and proxy policy still applies. if pin := tlsPin; pin != "" { // Clone the transport built above so the pinned client keeps the same // mDNS-aware dialer — a pinned device is usually a local appliance diff --git a/go/internal/drivers/lua_http_test.go b/go/internal/drivers/lua_http_test.go index 42978fb7..1aab144e 100644 --- a/go/internal/drivers/lua_http_test.go +++ b/go/internal/drivers/lua_http_test.go @@ -4,14 +4,17 @@ import ( "context" "crypto/sha256" "encoding/hex" + "errors" "net/http" "net/http/httptest" + "net/url" "os" "path/filepath" "strings" "sync/atomic" "testing" + "github.com/srcfl/ftw/go/internal/mdnsresolve" "github.com/srcfl/ftw/go/internal/telemetry" ) @@ -197,6 +200,71 @@ func TestHTTPTestRigSelfCheck(t *testing.T) { } } +func TestLuaHTTPProxyChecksLocalDestinationBeforeProxy(t *testing.T) { + var proxyHits atomic.Int32 + proxy := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + proxyHits.Add(1) + if r.URL.Host == "" { + t.Errorf("proxy request lost absolute destination: %+v", r.URL) + } + _, _ = w.Write([]byte("proxy-ok")) + })) + defer proxy.Close() + proxyURL, err := url.Parse(proxy.URL) + if err != nil { + t.Fatal(err) + } + + newRequest := func(method, target string) *http.Request { + req, err := http.NewRequest(method, target, strings.NewReader(`{"command":"start"}`)) + if err != nil { + t.Fatal(err) + } + req.Header.Set("Authorization", "Basic dTpw") + return req + } + + t.Run("default deny stops every HTTP method before proxy", func(t *testing.T) { + proxyHits.Store(0) + client := &http.Client{Transport: newLuaHTTPTransport(false, http.ProxyURL(proxyURL))} + for _, method := range []string{http.MethodGet, http.MethodPost, http.MethodPatch} { + _, err := client.Do(newRequest(method, "http://inverter.local/api")) + if !errors.Is(err, mdnsresolve.ErrUnverifiedLocal) { + t.Errorf("%s error = %v, want ErrUnverifiedLocal", method, err) + } + } + if got := proxyHits.Load(); got != 0 { + t.Fatalf("denied .local requests reached proxy %d times, want 0", got) + } + }) + + t.Run("explicit opt-in uses the configured proxy", func(t *testing.T) { + proxyHits.Store(0) + client := &http.Client{Transport: newLuaHTTPTransport(true, http.ProxyURL(proxyURL))} + resp, err := client.Do(newRequest(http.MethodPost, "http://inverter.local/api")) + if err != nil { + t.Fatalf("opt-in request failed: %v", err) + } + _ = resp.Body.Close() + if got := proxyHits.Load(); got != 1 { + t.Fatalf("opt-in request reached proxy %d times, want 1", got) + } + }) + + t.Run("ordinary host still uses the proxy without opt-in", func(t *testing.T) { + proxyHits.Store(0) + client := &http.Client{Transport: newLuaHTTPTransport(false, http.ProxyURL(proxyURL))} + resp, err := client.Do(newRequest(http.MethodGet, "http://ordinary.example/api")) + if err != nil { + t.Fatalf("ordinary host request failed: %v", err) + } + _ = resp.Body.Close() + if got := proxyHits.Load(); got != 1 { + t.Fatalf("ordinary host reached proxy %d times, want 1", got) + } + }) +} + // TLS pinning lets a driver reach a self-signed HTTPS endpoint (e.g. a // NIBE heat pump's local REST API) by accepting exactly one leaf cert. // httptest.NewTLSServer mints a self-signed cert NOT in the system root diff --git a/go/internal/drivers/proxy_policy.go b/go/internal/drivers/proxy_policy.go new file mode 100644 index 00000000..647b257f --- /dev/null +++ b/go/internal/drivers/proxy_policy.go @@ -0,0 +1,27 @@ +package drivers + +import ( + net_http "net/http" + net_url "net/url" + + "github.com/srcfl/ftw/go/internal/mdnsresolve" +) + +// guardMDNSProxy checks the original request destination before delegating to +// proxy selection. HTTP transports and WebSocket dialers call their proxy +// function before NetDialContext, so a check in the dialer alone is too late: +// a denied .local request could already have reached a proxy with credentials +// or a command payload. +func guardMDNSProxy(proxy func(*net_http.Request) (*net_url.URL, error), allowUnverifiedLocal bool) func(*net_http.Request) (*net_url.URL, error) { + return func(req *net_http.Request) (*net_url.URL, error) { + if req != nil && req.URL != nil { + if err := mdnsresolve.CheckLocalDestination(req.URL.Hostname(), allowUnverifiedLocal); err != nil { + return nil, err + } + } + if proxy == nil { + return nil, nil + } + return proxy(req) + } +} diff --git a/go/internal/drivers/registry.go b/go/internal/drivers/registry.go index 29f99c31..19f0ced5 100644 --- a/go/internal/drivers/registry.go +++ b/go/internal/drivers/registry.go @@ -187,29 +187,33 @@ func (r *Registry) Add(ctx context.Context, cfg config.Driver) error { return r.SecretPersister(driverName, key, value) } if mq := cfg.EffectiveMQTT(); mq != nil && r.MQTTFactory != nil { - cap, err := r.MQTTFactory(cfg.Name, mq) + dialCfg := *mq + dialCfg.AllowUnverifiedLocal = cfg.Capabilities.AllowUnverifiedLocal + cap, err := r.MQTTFactory(cfg.Name, &dialCfg) if err != nil { return fmt.Errorf("mqtt capability: %w", err) } env.WithMQTT(cap) - env.SetEndpoint(fmt.Sprintf("mqtt://%s:%d", mq.Host, mq.Port)) + env.SetEndpoint(fmt.Sprintf("mqtt://%s:%d", dialCfg.Host, dialCfg.Port)) // Best-effort MAC resolution. Cross-VLAN devices return ""; that's // fine — device_id falls back to the endpoint. if r.ARPLookup != nil { - if mac, ok := r.ARPLookup(mq.Host); ok { + if mac, ok := r.ARPLookup(dialCfg.Host); ok { env.SetMAC(mac) } } } if mb := cfg.EffectiveModbus(); mb != nil && r.ModbusFactory != nil { - cap, err := r.ModbusFactory(cfg.Name, mb) + dialCfg := *mb + dialCfg.AllowUnverifiedLocal = cfg.Capabilities.AllowUnverifiedLocal + cap, err := r.ModbusFactory(cfg.Name, &dialCfg) if err != nil { return fmt.Errorf("modbus capability: %w", err) } env.WithModbus(cap) - env.SetEndpoint(fmt.Sprintf("modbus://%s:%d", mb.Host, mb.Port)) + env.SetEndpoint(fmt.Sprintf("modbus://%s:%d", dialCfg.Host, dialCfg.Port)) if r.ARPLookup != nil { - if mac, ok := r.ARPLookup(mb.Host); ok { + if mac, ok := r.ARPLookup(dialCfg.Host); ok { env.SetMAC(mac) } } @@ -235,8 +239,11 @@ func (r *Registry) Add(ctx context.Context, cfg config.Driver) error { env.WithHTTPAllowWrite() } } + if cfg.Capabilities.AllowUnverifiedLocal { + env.WithAllowUnverifiedLocal() + } if cfg.Capabilities.WebSocket != nil { - env.WithWS(NewGorillaWS(cfg.Name)) + env.WithWS(NewGorillaWS(cfg.Name, cfg.Capabilities.AllowUnverifiedLocal)) if hosts := cfg.Capabilities.WebSocket.AllowedHosts; len(hosts) > 0 { env.WithWSAllowedHosts(hosts) } @@ -250,7 +257,7 @@ func (r *Registry) Add(ctx context.Context, cfg config.Driver) error { // the operator can still loosen this by listing bare hosts in // capabilities.tcp.allowed_hosts when they want any-port access. hosts := tcpAllowedHostsFor(cfg) - env.WithTCP(NewNetTCP(cfg.Name, hosts)) + env.WithTCP(NewNetTCP(cfg.Name, hosts, cfg.Capabilities.AllowUnverifiedLocal)) if len(hosts) > 0 { env.WithTCPAllowedHosts(hosts) } @@ -884,6 +891,7 @@ func sameDriverConfig(a, b config.Driver) bool { a.BatteryTelemetryOnly != b.BatteryTelemetryOnly || a.ObserveOnly != b.ObserveOnly || a.Disabled != b.Disabled || + a.Capabilities.AllowUnverifiedLocal != b.Capabilities.AllowUnverifiedLocal || !reflect.DeepEqual(a.Control, b.Control) { return false } diff --git a/go/internal/drivers/registry_restart_test.go b/go/internal/drivers/registry_restart_test.go index b7dcdde3..35fd2765 100644 --- a/go/internal/drivers/registry_restart_test.go +++ b/go/internal/drivers/registry_restart_test.go @@ -399,6 +399,34 @@ func TestReloadRestartsDriverWhenTransportHostChangesToLocalName(t *testing.T) { } } +func TestReloadRestartsDriverWhenLocalTrustPolicyChanges(t *testing.T) { + var seen []bool + r := NewRegistry(telemetry.NewStore()) + r.ModbusFactory = func(name string, cfg *config.ModbusConfig) (ModbusCap, error) { + seen = append(seen, cfg.AllowUnverifiedLocal) + return &mockModbus{}, nil + } + path := writeTestDriver(t, registryRestartTestDriver) + cfg := config.Driver{ + Name: "d1", + Lua: path, + Capabilities: config.Capabilities{ + Modbus: &config.ModbusConfig{Host: "192.168.1.20", Port: 502, UnitID: 1}, + }, + } + if err := r.Add(context.Background(), cfg); err != nil { + t.Fatal(err) + } + defer r.ShutdownAll() + + updated := cfg + updated.Capabilities.AllowUnverifiedLocal = true + r.Reload(context.Background(), []config.Driver{updated}, false) + if len(seen) != 2 || seen[0] || !seen[1] { + t.Fatalf("Modbus factory trust flags after reload = %v, want [false true]", seen) + } +} + // runLoop should bump TickCount on every poll-return-without-error so // a Lua driver that is alive but hasn't emitted yet (e.g. between // MQTT subscribe and the first inbound message) is visibly running diff --git a/go/internal/drivers/tcp_cap.go b/go/internal/drivers/tcp_cap.go index cba42622..9f7f17e8 100644 --- a/go/internal/drivers/tcp_cap.go +++ b/go/internal/drivers/tcp_cap.go @@ -35,21 +35,23 @@ type TCPCap interface { // background read pump that appends bytes to an in-memory buffer, and a // non-blocking PopBytes drain. Mirrors gorillaWS's concurrency model. type netTCP struct { - driverName string - allowed []string // empty = any host - - mu sync.Mutex - conn net.Conn - open bool - buf []byte - stop chan struct{} + driverName string + allowed []string // empty = any host + allowUnverifiedLocal bool + + mu sync.Mutex + conn net.Conn + open bool + buf []byte + stop chan struct{} } // NewNetTCP returns a TCPCap bound to a driver name. The connection is not // opened until the driver calls host.tcp_open. -func NewNetTCP(driverName string, allowedHosts []string) TCPCap { +func NewNetTCP(driverName string, allowedHosts []string, allowUnverifiedLocal ...bool) TCPCap { cp := append([]string(nil), allowedHosts...) - return &netTCP{driverName: driverName, allowed: cp} + allow := len(allowUnverifiedLocal) > 0 && allowUnverifiedLocal[0] + return &netTCP{driverName: driverName, allowed: cp, allowUnverifiedLocal: allow} } // tcpHostAllowed checks `host:port` style addresses against the allowlist. @@ -95,7 +97,7 @@ func (n *netTCP) Open(addr string) error { return fmt.Errorf("tcp: %s", reason) } - conn, err := mdnsresolve.DialTimeout("tcp", addr, 10*time.Second) + conn, err := mdnsresolve.DialTimeoutWithOptions("tcp", addr, 10*time.Second, n.allowUnverifiedLocal) if err != nil { return fmt.Errorf("tcp dial: %w", err) } diff --git a/go/internal/drivers/ws_cap.go b/go/internal/drivers/ws_cap.go index bd23736d..18eb00b5 100644 --- a/go/internal/drivers/ws_cap.go +++ b/go/internal/drivers/ws_cap.go @@ -25,12 +25,13 @@ import ( // net.Conn close races a graceful WS close message — either path lands // the goroutine on a sane stop). type gorillaWS struct { - driverName string + driverName string + allowUnverifiedLocal bool - mu sync.Mutex // protects conn, open, queue - conn *websocket.Conn - open bool - queue []string + mu sync.Mutex // protects conn, open, queue + conn *websocket.Conn + open bool + queue []string writeMu sync.Mutex // serializes WriteMessage calls stop chan struct{} } @@ -39,8 +40,21 @@ type gorillaWS struct { // until Open is called by the driver — drivers commonly need to do an // HTTP call (e.g. resolve a Tibber homeId) before they know what to // subscribe to, so connecting eagerly from the registry would be wrong. -func NewGorillaWS(driverName string) WSCap { - return &gorillaWS{driverName: driverName} +func NewGorillaWS(driverName string, allowUnverifiedLocal ...bool) WSCap { + allow := len(allowUnverifiedLocal) > 0 && allowUnverifiedLocal[0] + return &gorillaWS{driverName: driverName, allowUnverifiedLocal: allow} +} + +func newGorillaWSDialer(allowUnverifiedLocal bool, proxy func(*net_http.Request) (*net_url.URL, error)) websocket.Dialer { + dialer := *websocket.DefaultDialer + dialer.HandshakeTimeout = 15 * time.Second + if proxy == nil { + proxy = dialer.Proxy + } + dialer.Proxy = guardMDNSProxy(proxy, allowUnverifiedLocal) + mdnsDialer := mdnsresolve.Dialer{AllowUnverifiedLocal: allowUnverifiedLocal} + dialer.NetDialContext = mdnsDialer.DialContext + return dialer } // Open establishes the WebSocket connection. headers go into the HTTP @@ -73,10 +87,10 @@ func (g *gorillaWS) Open(url string, headers map[string]string) error { } hdr.Set(k, v) } - dialer := *websocket.DefaultDialer - dialer.HandshakeTimeout = 15 * time.Second // ".local" hosts need mDNS; everything else falls through to a plain dial. - dialer.NetDialContext = mdnsresolve.DialContext + // Guard the proxy callback too: gorilla selects a proxy before it invokes + // NetDialContext. + dialer := newGorillaWSDialer(g.allowUnverifiedLocal, nil) if len(subprotocols) > 0 { dialer.Subprotocols = subprotocols } diff --git a/go/internal/drivers/ws_proxy_test.go b/go/internal/drivers/ws_proxy_test.go new file mode 100644 index 00000000..dda8294a --- /dev/null +++ b/go/internal/drivers/ws_proxy_test.go @@ -0,0 +1,144 @@ +package drivers + +import ( + "bufio" + "crypto/sha1" + "encoding/base64" + "errors" + "fmt" + "net" + "net/http" + "net/url" + "sync/atomic" + "testing" + "time" + + "github.com/srcfl/ftw/go/internal/mdnsresolve" +) + +const websocketGUID = "258EAFA5-E914-47DA-95CA-C5AB0DC85B11" + +type wsTestProxy struct { + listener net.Listener + hits atomic.Int32 + hosts chan string +} + +func newWSTestProxy(t *testing.T) *wsTestProxy { + t.Helper() + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + p := &wsTestProxy{listener: listener, hosts: make(chan string, 8)} + t.Cleanup(func() { _ = listener.Close() }) + go p.serve(t) + return p +} + +func (p *wsTestProxy) serve(t *testing.T) { + t.Helper() + for { + conn, err := p.listener.Accept() + if err != nil { + return + } + go p.handle(conn) + } +} + +func (p *wsTestProxy) handle(conn net.Conn) { + defer conn.Close() + reader := bufio.NewReader(conn) + connectReq, err := http.ReadRequest(reader) + if err != nil { + return + } + p.hits.Add(1) + p.hosts <- connectReq.Host + if connectReq.Method != http.MethodConnect { + return + } + _, _ = fmt.Fprint(conn, "HTTP/1.1 200 Connection Established\r\n\r\n") + + upgradeReq, err := http.ReadRequest(reader) + if err != nil { + return + } + key := upgradeReq.Header.Get("Sec-WebSocket-Key") + sum := sha1.Sum([]byte(key + websocketGUID)) + accept := base64.StdEncoding.EncodeToString(sum[:]) + _, _ = fmt.Fprintf(conn, + "HTTP/1.1 101 Switching Protocols\r\nUpgrade: websocket\r\nConnection: Upgrade\r\nSec-WebSocket-Accept: %s\r\n\r\n", + accept, + ) + // The client closes the connection after the handshake test. Keep the + // tunnel open until then so Gorilla can finish Dial successfully. + _, _ = reader.ReadByte() +} + +func (p *wsTestProxy) proxyURL(t *testing.T) *url.URL { + t.Helper() + u, err := url.Parse("http://" + p.listener.Addr().String()) + if err != nil { + t.Fatal(err) + } + return u +} + +func (p *wsTestProxy) nextHost(t *testing.T) string { + t.Helper() + select { + case host := <-p.hosts: + return host + case <-time.After(time.Second): + t.Fatal("proxy did not receive CONNECT") + return "" + } +} + +func TestGorillaWSProxyChecksLocalDestinationBeforeProxy(t *testing.T) { + proxy := newWSTestProxy(t) + proxyURL := proxy.proxyURL(t) + + t.Run("default deny stops local CONNECT", func(t *testing.T) { + dialer := newGorillaWSDialer(false, http.ProxyURL(proxyURL)) + _, _, err := dialer.Dial("ws://inverter.local/v1", nil) + if !errors.Is(err, mdnsresolve.ErrUnverifiedLocal) { + t.Fatalf("local WebSocket error = %v, want ErrUnverifiedLocal", err) + } + if got := proxy.hits.Load(); got != 0 { + t.Fatalf("denied local WebSocket reached proxy %d times, want 0", got) + } + }) + + t.Run("explicit opt-in uses the configured proxy", func(t *testing.T) { + dialer := newGorillaWSDialer(true, http.ProxyURL(proxyURL)) + conn, _, err := dialer.Dial("ws://inverter.local/v1", nil) + if err != nil { + t.Fatalf("opt-in WebSocket failed: %v", err) + } + _ = conn.Close() + if got := proxy.hits.Load(); got != 1 { + t.Fatalf("opt-in local WebSocket reached proxy %d times, want 1", got) + } + if host := proxy.nextHost(t); host != "inverter.local:80" { + t.Fatalf("proxy CONNECT host = %q, want inverter.local:80", host) + } + }) + + t.Run("ordinary host still uses the proxy", func(t *testing.T) { + dialer := newGorillaWSDialer(false, http.ProxyURL(proxyURL)) + conn, _, err := dialer.Dial("ws://ordinary.example/v1", nil) + if err != nil { + t.Fatalf("ordinary WebSocket failed: %v", err) + } + _ = conn.Close() + if got := proxy.hits.Load(); got != 2 { + t.Fatalf("ordinary WebSocket total proxy hits = %d, want 2", got) + } + if host := proxy.nextHost(t); host != "ordinary.example:80" { + t.Fatalf("proxy CONNECT host = %q, want ordinary.example:80", host) + } + }) +} diff --git a/go/internal/ha/bridge.go b/go/internal/ha/bridge.go index dd05fd77..e801e83e 100644 --- a/go/internal/ha/bridge.go +++ b/go/internal/ha/bridge.go @@ -256,7 +256,10 @@ func (b *Bridge) connectAndStart(cfg *config.HomeAssistant, driverNames []string // which the stdlib resolver cannot answer. See internal/mqtt for why a // TCP-only replacement is complete here. SetCustomOpenConnectionFn(func(uri *url.URL, o paho.ClientOptions) (net.Conn, error) { - d := mdnsresolve.Dialer{Dialer: net.Dialer{Timeout: o.ConnectTimeout}} + d := mdnsresolve.Dialer{ + Dialer: net.Dialer{Timeout: o.ConnectTimeout}, + AllowUnverifiedLocal: cfg.AllowUnverifiedLocal, + } return d.Dial("tcp", uri.Host) }). SetClientID("forty-two-watts-ha"). diff --git a/go/internal/mdnsresolve/mdnsresolve.go b/go/internal/mdnsresolve/mdnsresolve.go index 95b2b5ed..6bbd0102 100644 --- a/go/internal/mdnsresolve/mdnsresolve.go +++ b/go/internal/mdnsresolve/mdnsresolve.go @@ -35,6 +35,7 @@ package mdnsresolve import ( "context" + "errors" "fmt" "log/slog" "net" @@ -44,6 +45,10 @@ import ( "time" ) +// ErrUnverifiedLocal is returned before any mDNS query when a caller has not +// explicitly accepted that a .local name is not a server identity. +var ErrUnverifiedLocal = errors.New("mDNS .local resolution requires allow_unverified_local=true") + // now is swappable so cache-expiry tests do not have to sleep. var now = time.Now @@ -86,6 +91,17 @@ func IsLocal(host string) bool { return strings.HasSuffix(strings.ToLower(strings.TrimSuffix(host, ".")), ".local") } +// CheckLocalDestination enforces the core trust decision before a transport +// chooses a direct socket or an HTTP/WebSocket proxy. Proxy selection happens +// before a net.Dialer callback, so keeping this check separate from Dialer is +// what prevents an untrusted .local request from reaching a proxy first. +func CheckLocalDestination(host string, allowUnverifiedLocal bool) error { + if IsLocal(host) && !allowUnverifiedLocal { + return fmt.Errorf("mDNS resolution for %s denied: %w", host, ErrUnverifiedLocal) + } + return nil +} + func canonical(name string) string { return strings.ToLower(strings.TrimSuffix(name, ".")) } @@ -179,6 +195,10 @@ func resolve(ctx context.Context, key string) ([]netip.Addr, time.Duration, stri // connection from the original address string pick up the new IP on reconnect. type Dialer struct { net.Dialer + // AllowUnverifiedLocal is a per-driver/core policy. Name allowlists do not + // prove which LAN host answered an mDNS query, so this stays false unless + // the operator explicitly accepts that trust model. + AllowUnverifiedLocal bool } // DialContext resolves address if it names a ".local" host, then dials it. @@ -187,6 +207,9 @@ func (d *Dialer) DialContext(ctx context.Context, network, address string) (net. if err != nil || !IsLocal(host) { return d.Dialer.DialContext(ctx, network, address) } + if err := CheckLocalDestination(host, d.AllowUnverifiedLocal); err != nil { + return nil, err + } addrs, err := Lookup(ctx, host) if err != nil { @@ -229,6 +252,15 @@ func DialContext(ctx context.Context, network, address string) (net.Conn, error) // DialTimeout mirrors net.DialTimeout with mDNS resolution added. func DialTimeout(network, address string, timeout time.Duration) (net.Conn, error) { - d := Dialer{Dialer: net.Dialer{Timeout: timeout}} + return DialTimeoutWithOptions(network, address, timeout, false) +} + +// DialTimeoutWithOptions mirrors DialTimeout and carries the core's explicit +// mDNS trust decision to the TCP capability. +func DialTimeoutWithOptions(network, address string, timeout time.Duration, allowUnverifiedLocal bool) (net.Conn, error) { + d := Dialer{ + Dialer: net.Dialer{Timeout: timeout}, + AllowUnverifiedLocal: allowUnverifiedLocal, + } return d.Dial(network, address) } diff --git a/go/internal/mdnsresolve/mdnsresolve_test.go b/go/internal/mdnsresolve/mdnsresolve_test.go index f548c8a6..9f3b6bc6 100644 --- a/go/internal/mdnsresolve/mdnsresolve_test.go +++ b/go/internal/mdnsresolve/mdnsresolve_test.go @@ -501,6 +501,52 @@ func TestDialerSkipsResolutionForPlainHosts(t *testing.T) { } } +func TestDialerDeniesUnverifiedLocalByDefault(t *testing.T) { + _, err := (&Dialer{}).Dial("tcp", "inverter.local:502") + if !errors.Is(err, ErrUnverifiedLocal) { + t.Fatalf("Dial without local opt-in error = %v, want ErrUnverifiedLocal", err) + } +} + +func TestDialerLeavesOrdinaryDNSAndIPUnchanged(t *testing.T) { + listener, err := net.Listen("tcp4", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + defer listener.Close() + _, port, err := net.SplitHostPort(listener.Addr().String()) + if err != nil { + t.Fatal(err) + } + + accepted := make(chan struct{}, 2) + go func() { + for range 2 { + conn, err := listener.Accept() + if err != nil { + return + } + accepted <- struct{}{} + _ = conn.Close() + } + }() + + for _, address := range []string{"127.0.0.1:" + port, "localhost:" + port} { + conn, err := (&Dialer{}).Dial("tcp4", address) + if err != nil { + t.Fatalf("Dial(%q): %v", address, err) + } + _ = conn.Close() + } + for range 2 { + select { + case <-accepted: + case <-time.After(time.Second): + t.Fatal("ordinary DNS/IP dial was not accepted") + } + } +} + func TestDialerResolvesLocalNameBeforeConnecting(t *testing.T) { listener, err := net.Listen("tcp4", "127.0.0.1:0") if err != nil { @@ -527,7 +573,10 @@ func TestDialerResolvesLocalNameBeforeConnecting(t *testing.T) { return "- 15 Timeout reached" }) - d := Dialer{Dialer: net.Dialer{Timeout: time.Second}} + d := Dialer{ + Dialer: net.Dialer{Timeout: time.Second}, + AllowUnverifiedLocal: true, + } conn, err := d.Dial("tcp", "inverter.local:"+port) if err != nil { t.Fatalf("Dial: %v", err) diff --git a/go/internal/modbus/client.go b/go/internal/modbus/client.go index ef461017..7b06a716 100644 --- a/go/internal/modbus/client.go +++ b/go/internal/modbus/client.go @@ -29,12 +29,13 @@ const ( // single-session dongle can release its old socket without blocking the // driver's poll and command loop. type Capability struct { - mu sync.Mutex - client *tcpClient - url string - addr string - unitID int - requestTimeout time.Duration + mu sync.Mutex + client *tcpClient + url string + addr string + unitID int + allowUnverifiedLocal bool + requestTimeout time.Duration consecutiveTransportFailures int nextReconnectAt time.Time @@ -43,18 +44,25 @@ type Capability struct { // Dial opens a Modbus TCP connection. func Dial(host string, port, unitID int) (*Capability, error) { + return DialWithOptions(host, port, unitID, false) +} + +// DialWithOptions opens a Modbus TCP connection with the core's explicit +// policy for unauthenticated mDNS names. +func DialWithOptions(host string, port, unitID int, allowUnverifiedLocal bool) (*Capability, error) { if err := validateEndpoint(host, port, unitID); err != nil { return nil, err } addr := net.JoinHostPort(host, strconv.Itoa(port)) url := "tcp://" + addr - cli := newTCPClient(addr, modbusRequestTimeout, modbusTCPKeepAlive) + cli := newTCPClientWithOptions(addr, modbusRequestTimeout, modbusTCPKeepAlive, allowUnverifiedLocal) capability := &Capability{ - url: url, - addr: addr, - unitID: unitID, - requestTimeout: modbusRequestTimeout, - now: time.Now, + url: url, + addr: addr, + unitID: unitID, + allowUnverifiedLocal: allowUnverifiedLocal, + requestTimeout: modbusRequestTimeout, + now: time.Now, } if err := cli.Open(); err != nil { if !isRetryableDialError(err) { @@ -330,7 +338,7 @@ func (c *Capability) reconnect() error { if timeout <= 0 { timeout = modbusRequestTimeout } - cli := newTCPClient(c.addr, timeout, modbusTCPKeepAlive) + cli := newTCPClientWithOptions(c.addr, timeout, modbusTCPKeepAlive, c.allowUnverifiedLocal) if err := cli.Open(); err != nil { c.noteTransportFailure() return err diff --git a/go/internal/modbus/tcp_client.go b/go/internal/modbus/tcp_client.go index b5b790ab..a95b4f38 100644 --- a/go/internal/modbus/tcp_client.go +++ b/go/internal/modbus/tcp_client.go @@ -23,20 +23,26 @@ const ( ) type tcpClient struct { - addr string - timeout time.Duration - keepAlive time.Duration - unitID uint8 - txID uint16 - conn net.Conn + addr string + timeout time.Duration + keepAlive time.Duration + allowUnverifiedLocal bool + unitID uint8 + txID uint16 + conn net.Conn } func newTCPClient(addr string, timeout, keepAlive time.Duration) *tcpClient { + return newTCPClientWithOptions(addr, timeout, keepAlive, false) +} + +func newTCPClientWithOptions(addr string, timeout, keepAlive time.Duration, allowUnverifiedLocal bool) *tcpClient { return &tcpClient{ - addr: addr, - timeout: timeout, - keepAlive: keepAlive, - unitID: 1, + addr: addr, + timeout: timeout, + keepAlive: keepAlive, + allowUnverifiedLocal: allowUnverifiedLocal, + unitID: 1, } } @@ -47,7 +53,7 @@ func (c *tcpClient) Open() error { dialer := mdnsresolve.Dialer{Dialer: net.Dialer{ Timeout: modbusDialTimeout, KeepAlive: c.keepAlive, - }} + }, AllowUnverifiedLocal: c.allowUnverifiedLocal} conn, err := dialer.Dial("tcp", c.addr) if err != nil { return err diff --git a/go/internal/mqtt/client.go b/go/internal/mqtt/client.go index a711dd96..226834b5 100644 --- a/go/internal/mqtt/client.go +++ b/go/internal/mqtt/client.go @@ -49,6 +49,12 @@ type Capability struct { // the replay is what restores the subscription set the broker just // dropped. func Dial(host string, port int, username, password, clientID string) (*Capability, error) { + return DialWithOptions(host, port, username, password, clientID, false) +} + +// DialWithOptions connects to an MQTT broker with the core's explicit policy +// for unauthenticated mDNS names. +func DialWithOptions(host string, port int, username, password, clientID string, allowUnverifiedLocal bool) (*Capability, error) { cap := &Capability{ subs: make(map[string]struct{}), } @@ -59,7 +65,10 @@ func Dial(host string, port int, username, password, clientID string) (*Capabili // TCP-only replacement is complete; non-".local" hosts fall through to // a plain dial inside mdnsresolve. SetCustomOpenConnectionFn(func(uri *url.URL, o paho.ClientOptions) (net.Conn, error) { - d := mdnsresolve.Dialer{Dialer: net.Dialer{Timeout: o.ConnectTimeout}} + d := mdnsresolve.Dialer{ + Dialer: net.Dialer{Timeout: o.ConnectTimeout}, + AllowUnverifiedLocal: allowUnverifiedLocal, + } return d.Dial("tcp", uri.Host) }). SetClientID(clientID). From cf9cd8d08cbb8c7db212dcbca684a30f92853716 Mon Sep 17 00:00:00 2001 From: Claude Opus 5 Date: Wed, 5 Aug 2026 10:26:35 +0200 Subject: [PATCH 5/7] fix(net): keep the system resolver as a fallback for .local MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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> --- go/internal/mdnsresolve/mdnsresolve.go | 26 +++++++- go/internal/mdnsresolve/mdnsresolve_test.go | 67 +++++++++++++++++++++ go/internal/mdnsresolve/multicast.go | 10 +-- 3 files changed, 97 insertions(+), 6 deletions(-) diff --git a/go/internal/mdnsresolve/mdnsresolve.go b/go/internal/mdnsresolve/mdnsresolve.go index 6bbd0102..e33b9a6f 100644 --- a/go/internal/mdnsresolve/mdnsresolve.go +++ b/go/internal/mdnsresolve/mdnsresolve.go @@ -215,9 +215,9 @@ func (d *Dialer) DialContext(ctx context.Context, network, address string) (net. if err != nil { // Name the mechanism. Without this the operator sees a bare dial // failure and has no way to tell that resolution was the reason. - slog.Warn("mDNS resolution failed; check the device is on this LAN and the container uses host networking", + slog.Warn("mDNS resolution failed, falling back to the system resolver", "host", host, "err", err) - return nil, fmt.Errorf("resolve %s over mDNS: %w", host, err) + return d.systemFallback(ctx, network, address, err) } var firstErr error @@ -233,6 +233,28 @@ func (d *Dialer) DialContext(ctx context.Context, network, address string) (net. return nil, fmt.Errorf("dial %s over mDNS: %w", host, firstErr) } +// systemFallback hands a ".local" name that mDNS could not resolve to the +// stdlib resolver. +// +// It exists because one supported platform already resolves these names +// without us. Under Home Assistant the app has no avahi socket to ask, but +// Supervisor points every container at its own CoreDNS, which carries an mdns +// plugin backed by systemd-resolved — so `zap.local` resolves there today over +// ordinary unicast DNS, verified on a pilot install running v1.10.0-beta.1: +// +// lookup ftw-no-such-device-xyz.local on 172.30.32.3:53: no such host +// +// Returning the mDNS error instead of trying that resolver would take a +// working install and break it. Both errors are reported when both paths fail, +// because "no such host" alone sends an operator looking in the wrong place. +func (d *Dialer) systemFallback(ctx context.Context, network, address string, mdnsErr error) (net.Conn, error) { + conn, err := d.Dialer.DialContext(ctx, network, address) + if err == nil { + return conn, nil + } + return nil, fmt.Errorf("%w (mDNS: %v)", err, mdnsErr) +} + // Dial is the context-free form, for callers that have no context to pass. func (d *Dialer) Dial(network, address string) (net.Conn, error) { ctx := context.Background() diff --git a/go/internal/mdnsresolve/mdnsresolve_test.go b/go/internal/mdnsresolve/mdnsresolve_test.go index 9f3b6bc6..c0739740 100644 --- a/go/internal/mdnsresolve/mdnsresolve_test.go +++ b/go/internal/mdnsresolve/mdnsresolve_test.go @@ -142,6 +142,28 @@ func TestParseAddrAnswer(t *testing.T) { } } +// A truncated record after a good one must void the whole packet. Keeping the +// address read before the damage would let a malformed UDP response populate +// the cache, and the parser has already lost sync at that point. +func TestParseAddrAnswerRejectsPacketTruncatedAfterAGoodAnswer(t *testing.T) { + qname := "inverter.local." + packet := packAnswer(t, qname, []dnsmessage.Resource{ + aResource(t, qname, [4]byte{192, 168, 1, 42}, 60), + aResource(t, qname, [4]byte{192, 168, 1, 43}, 60), + }) + + // Sanity check: intact, both answers are accepted. + if addrs, _, ok := parseTestAddrAnswer(t, packet, qname, "udp4"); !ok || len(addrs) != 2 { + t.Fatalf("intact packet: addrs = %v, ok = %v, want two addresses", addrs, ok) + } + + // Cut into the final record's rdata: its header still parses, its body + // does not. + if _, _, ok := parseTestAddrAnswer(t, packet[:len(packet)-2], qname, "udp4"); ok { + t.Fatal("accepted a packet whose last matching record is truncated") + } +} + func TestParseAddrAnswerValidatesSourceFamilyAndClass(t *testing.T) { qname := "inverter.local." answer := aResource(t, qname, [4]byte{192, 168, 1, 42}, 60) @@ -605,3 +627,48 @@ func TestDialerReportsResolutionFailure(t *testing.T) { t.Fatalf("error %q does not mention mDNS", err) } } + +// A ".local" name we cannot resolve must still be offered to the system +// resolver. Home Assistant is the case that makes this mandatory: the app has +// no avahi socket, but Supervisor's CoreDNS answers ".local" over unicast DNS, +// so failing here would break an install that works today. +func TestDialerFallsBackToSystemResolver(t *testing.T) { + Flush() + disableAvahi(t) + // No usable interface fails the multicast path immediately, without + // binding a socket — startResponder needs port 5353, which some hosts + // refuse outright. + origInterfaces := multicastInterfaces + multicastInterfaces = func() ([]net.Interface, error) { return nil, nil } + t.Cleanup(func() { multicastInterfaces = origInterfaces }) + + consulted := make(chan struct{}, 1) + d := Dialer{AllowUnverifiedLocal: true, Dialer: net.Dialer{ + Timeout: 500 * time.Millisecond, + Resolver: &net.Resolver{ + PreferGo: true, + Dial: func(context.Context, string, string) (net.Conn, error) { + select { + case consulted <- struct{}{}: + default: + } + return nil, errors.New("no nameserver in this test") + }, + }, + }} + + _, err := d.Dial("tcp", "missing.local:502") + if err == nil { + t.Fatal("expected a failure") + } + select { + case <-consulted: + default: + t.Fatal("system resolver was never consulted after mDNS failed") + } + // Both failures have to survive into the message. "no such host" on its own + // sends an operator looking for a DNS problem that isn't there. + if !strings.Contains(err.Error(), "mDNS:") { + t.Fatalf("error %q drops the mDNS cause", err) + } +} diff --git a/go/internal/mdnsresolve/multicast.go b/go/internal/mdnsresolve/multicast.go index 408c13c3..9529c8a5 100644 --- a/go/internal/mdnsresolve/multicast.go +++ b/go/internal/mdnsresolve/multicast.go @@ -269,8 +269,10 @@ func parseAddrAnswer(packet []byte, qname string, source *net.UDPAddr, network s } var addrs []netip.Addr ttl := maxTTL - // Labelled so a parse error inside the type switch abandons the whole - // packet: once the parser desynchronises, every later record is suspect. + // A malformed record abandons the whole packet rather than keeping the + // addresses read before it: once the parser desynchronises, nothing after + // that point is trustworthy, and a truncated answer must not be allowed to + // populate the cache. The label is only the normal end-of-section exit. parse: for { h, err := p.AnswerHeader() @@ -292,13 +294,13 @@ parse: case dnsmessage.TypeA: r, err := p.AResource() if err != nil { - break parse + return nil, 0, false } addrs = append(addrs, netip.AddrFrom4(r.A)) case dnsmessage.TypeAAAA: r, err := p.AAAAResource() if err != nil { - break parse + return nil, 0, false } // Unmap so a v4-mapped AAAA dials as plain IPv4. addr := netip.AddrFrom16(r.AAAA).Unmap() From 97bb98238aeb4c851f30b7ceab8e37898b68e223 Mon Sep 17 00:00:00 2001 From: Claude Opus 5 Date: Wed, 5 Aug 2026 11:59:52 +0200 Subject: [PATCH 6/7] fix(net): gate the resolver, not the connection, for .local names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit allow_unverified_local refused the dial outright. That broke two things that matter more than the risk it was guarding. #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> --- config.example.yaml | 9 ++- docs/operations.md | 32 ++++++--- go/internal/drivers/lua.go | 2 +- go/internal/drivers/lua_http_test.go | 34 ++++----- go/internal/drivers/proxy_policy.go | 27 -------- go/internal/drivers/ws_cap.go | 2 +- go/internal/drivers/ws_proxy_test.go | 33 +++++---- go/internal/mdnsresolve/mdnsresolve.go | 26 +++++-- go/internal/mdnsresolve/mdnsresolve_test.go | 77 ++++++++++++++++++++- 9 files changed, 159 insertions(+), 83 deletions(-) delete mode 100644 go/internal/drivers/proxy_policy.go diff --git a/config.example.yaml b/config.example.yaml index 435e90c6..7fc0c4e8 100644 --- a/config.example.yaml +++ b/config.example.yaml @@ -73,7 +73,8 @@ drivers: # - name: ambibox # lua: drivers/ambibox_v2x.lua # capabilities: - # # mDNS does not prove the server identity; opt in per driver only + # # Lets FTW resolve the name itself over mDNS. Without it the name is + # # left to the system resolver, which answers on some platforms only. # allow_unverified_local: true # mqtt: # host: sid-os.local @@ -92,7 +93,9 @@ drivers: # is_site_meter: true # battery_telemetry_only: true # capabilities: - # # Required for an unauthenticated raw .local endpoint. + # # Lets FTW resolve zap.local itself. Without it the name is left to + # # the system resolver — which answers under Home Assistant, but not on + # # a plain Compose or Pi install. # allow_unverified_local: true # http: # allowed_hosts: ["zap.local"] # .local is resolved by FTW over mDNS @@ -140,7 +143,7 @@ api: homeassistant: enabled: false broker: 192.168.1.1 - # allow_unverified_local: true # required for an unauthenticated broker.local + # allow_unverified_local: true # let FTW resolve broker.local over mDNS itself port: 1883 username: homeems password: homeems diff --git a/docs/operations.md b/docs/operations.md index 3e8dcb01..194ad3c2 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -215,9 +215,17 @@ where the driver supports them. Network allowlists still check the configured host name and port before resolution; they do not prove that an mDNS responder is the intended device. -FTW therefore rejects `.local` transport dials by default, before it queries -Avahi or the LAN. To use an unauthenticated `.local` endpoint, opt in on that -driver: +That risk is not unique to names, which is why the opt-in below gates the +resolver rather than the connection. A raw IP address is no more an identity on +a LAN than a name is — it can be claimed by ARP, and DHCP can hand it to a +different device with no attacker involved at all. The durable check is the +identity a device reports once connected: make and serial, or its MAC. + +So by default FTW does not use *its own* mDNS answer for a `.local` name: the +name goes to the system resolver, exactly as it did before this package +existed, and on platforms that answer `.local` themselves — Home Assistant does, +through Supervisor's DNS service — it simply works. Opt in per driver to let FTW +resolve the name itself, over Avahi or the LAN: ```yaml capabilities: @@ -228,11 +236,19 @@ capabilities: ``` For the Home Assistant bridge, set `homeassistant.allow_unverified_local: true` -instead. This is required for every mDNS transport, including HTTP, WebSocket, -MQTT, Modbus and raw TCP. A TLS pin does not bypass this gate yet. Literal IP -addresses and ordinary DNS names keep their existing behavior. The opt-in is -per driver so a name allowlist never becomes a server identity for another -driver. +instead. It applies to every transport — HTTP, WebSocket, MQTT, Modbus and raw +TCP — and is per driver, so a name allowlist never becomes a server identity for +another driver. Literal IP addresses and ordinary DNS names are untouched. + +Without it, a `.local` dial is not refused; it is handed to the system resolver, +and the log records why FTW's own answer was not used. That matters on a host +where nothing else resolves `.local` — a plain Compose or Raspberry Pi install, +whose `resolv.conf` points at the router — because there the name will simply +not resolve until you opt in. + +Note the limit of the gate: where an HTTP proxy is configured, FTW never +resolves the destination at all, so the flag has nothing to say about that path. +A TLS pin does not bypass the gate yet. #### Letting FTW use avahi diff --git a/go/internal/drivers/lua.go b/go/internal/drivers/lua.go index 679e639f..4e7045bf 100644 --- a/go/internal/drivers/lua.go +++ b/go/internal/drivers/lua.go @@ -616,7 +616,7 @@ func newLuaHTTPTransport(allowUnverifiedLocal bool, proxy func(*net_http.Request if proxy == nil { proxy = transport.Proxy } - transport.Proxy = guardMDNSProxy(proxy, allowUnverifiedLocal) + transport.Proxy = proxy mdnsDialer := mdnsresolve.Dialer{AllowUnverifiedLocal: allowUnverifiedLocal} transport.DialContext = mdnsDialer.DialContext return transport diff --git a/go/internal/drivers/lua_http_test.go b/go/internal/drivers/lua_http_test.go index 1aab144e..31f08fd5 100644 --- a/go/internal/drivers/lua_http_test.go +++ b/go/internal/drivers/lua_http_test.go @@ -4,7 +4,6 @@ import ( "context" "crypto/sha256" "encoding/hex" - "errors" "net/http" "net/http/httptest" "net/url" @@ -14,7 +13,6 @@ import ( "sync/atomic" "testing" - "github.com/srcfl/ftw/go/internal/mdnsresolve" "github.com/srcfl/ftw/go/internal/telemetry" ) @@ -224,30 +222,24 @@ func TestLuaHTTPProxyChecksLocalDestinationBeforeProxy(t *testing.T) { return req } - t.Run("default deny stops every HTTP method before proxy", func(t *testing.T) { + t.Run("no opt-in still goes through the proxy", func(t *testing.T) { + // allow_unverified_local gates *our own* mDNS answer. With a proxy + // configured FTW never resolves the name at all — the proxy does — so + // the flag has nothing to say here and a .local host must behave like + // any other. Refusing would break Home Assistant, where the platform + // resolves .local and always has. proxyHits.Store(0) client := &http.Client{Transport: newLuaHTTPTransport(false, http.ProxyURL(proxyURL))} for _, method := range []string{http.MethodGet, http.MethodPost, http.MethodPatch} { - _, err := client.Do(newRequest(method, "http://inverter.local/api")) - if !errors.Is(err, mdnsresolve.ErrUnverifiedLocal) { - t.Errorf("%s error = %v, want ErrUnverifiedLocal", method, err) + resp, err := client.Do(newRequest(method, "http://inverter.local/api")) + if err != nil { + t.Errorf("%s failed: %v", method, err) + continue } + _ = resp.Body.Close() } - if got := proxyHits.Load(); got != 0 { - t.Fatalf("denied .local requests reached proxy %d times, want 0", got) - } - }) - - t.Run("explicit opt-in uses the configured proxy", func(t *testing.T) { - proxyHits.Store(0) - client := &http.Client{Transport: newLuaHTTPTransport(true, http.ProxyURL(proxyURL))} - resp, err := client.Do(newRequest(http.MethodPost, "http://inverter.local/api")) - if err != nil { - t.Fatalf("opt-in request failed: %v", err) - } - _ = resp.Body.Close() - if got := proxyHits.Load(); got != 1 { - t.Fatalf("opt-in request reached proxy %d times, want 1", got) + if got := proxyHits.Load(); got != 3 { + t.Fatalf(".local requests reached proxy %d times, want 3", got) } }) diff --git a/go/internal/drivers/proxy_policy.go b/go/internal/drivers/proxy_policy.go deleted file mode 100644 index 647b257f..00000000 --- a/go/internal/drivers/proxy_policy.go +++ /dev/null @@ -1,27 +0,0 @@ -package drivers - -import ( - net_http "net/http" - net_url "net/url" - - "github.com/srcfl/ftw/go/internal/mdnsresolve" -) - -// guardMDNSProxy checks the original request destination before delegating to -// proxy selection. HTTP transports and WebSocket dialers call their proxy -// function before NetDialContext, so a check in the dialer alone is too late: -// a denied .local request could already have reached a proxy with credentials -// or a command payload. -func guardMDNSProxy(proxy func(*net_http.Request) (*net_url.URL, error), allowUnverifiedLocal bool) func(*net_http.Request) (*net_url.URL, error) { - return func(req *net_http.Request) (*net_url.URL, error) { - if req != nil && req.URL != nil { - if err := mdnsresolve.CheckLocalDestination(req.URL.Hostname(), allowUnverifiedLocal); err != nil { - return nil, err - } - } - if proxy == nil { - return nil, nil - } - return proxy(req) - } -} diff --git a/go/internal/drivers/ws_cap.go b/go/internal/drivers/ws_cap.go index 18eb00b5..9823481e 100644 --- a/go/internal/drivers/ws_cap.go +++ b/go/internal/drivers/ws_cap.go @@ -51,7 +51,7 @@ func newGorillaWSDialer(allowUnverifiedLocal bool, proxy func(*net_http.Request) if proxy == nil { proxy = dialer.Proxy } - dialer.Proxy = guardMDNSProxy(proxy, allowUnverifiedLocal) + dialer.Proxy = proxy mdnsDialer := mdnsresolve.Dialer{AllowUnverifiedLocal: allowUnverifiedLocal} dialer.NetDialContext = mdnsDialer.DialContext return dialer diff --git a/go/internal/drivers/ws_proxy_test.go b/go/internal/drivers/ws_proxy_test.go index dda8294a..2538a215 100644 --- a/go/internal/drivers/ws_proxy_test.go +++ b/go/internal/drivers/ws_proxy_test.go @@ -4,7 +4,6 @@ import ( "bufio" "crypto/sha1" "encoding/base64" - "errors" "fmt" "net" "net/http" @@ -13,7 +12,6 @@ import ( "testing" "time" - "github.com/srcfl/ftw/go/internal/mdnsresolve" ) const websocketGUID = "258EAFA5-E914-47DA-95CA-C5AB0DC85B11" @@ -97,18 +95,27 @@ func (p *wsTestProxy) nextHost(t *testing.T) string { } } -func TestGorillaWSProxyChecksLocalDestinationBeforeProxy(t *testing.T) { +// allow_unverified_local gates FTW's *own* mDNS answer. With a proxy +// configured FTW never resolves the destination — the proxy does — so a +// ".local" WebSocket takes the same path as any other host, with or without +// the opt-in. Denying it would break Home Assistant, where the platform +// resolves ".local" and always has. +func TestGorillaWSLocalDestinationTakesTheProxyLikeAnyHost(t *testing.T) { proxy := newWSTestProxy(t) proxyURL := proxy.proxyURL(t) - t.Run("default deny stops local CONNECT", func(t *testing.T) { + t.Run("no opt-in still reaches the proxy", func(t *testing.T) { dialer := newGorillaWSDialer(false, http.ProxyURL(proxyURL)) - _, _, err := dialer.Dial("ws://inverter.local/v1", nil) - if !errors.Is(err, mdnsresolve.ErrUnverifiedLocal) { - t.Fatalf("local WebSocket error = %v, want ErrUnverifiedLocal", err) + conn, _, err := dialer.Dial("ws://inverter.local/v1", nil) + if err != nil { + t.Fatalf("local WebSocket without opt-in failed: %v", err) + } + _ = conn.Close() + if got := proxy.hits.Load(); got != 1 { + t.Fatalf("local WebSocket reached proxy %d times, want 1", got) } - if got := proxy.hits.Load(); got != 0 { - t.Fatalf("denied local WebSocket reached proxy %d times, want 0", got) + if host := proxy.nextHost(t); host != "inverter.local:80" { + t.Fatalf("proxy CONNECT host = %q, want inverter.local:80", host) } }) @@ -119,8 +126,8 @@ func TestGorillaWSProxyChecksLocalDestinationBeforeProxy(t *testing.T) { t.Fatalf("opt-in WebSocket failed: %v", err) } _ = conn.Close() - if got := proxy.hits.Load(); got != 1 { - t.Fatalf("opt-in local WebSocket reached proxy %d times, want 1", got) + if got := proxy.hits.Load(); got != 2 { + t.Fatalf("opt-in local WebSocket reached proxy %d times, want 2", got) } if host := proxy.nextHost(t); host != "inverter.local:80" { t.Fatalf("proxy CONNECT host = %q, want inverter.local:80", host) @@ -134,8 +141,8 @@ func TestGorillaWSProxyChecksLocalDestinationBeforeProxy(t *testing.T) { t.Fatalf("ordinary WebSocket failed: %v", err) } _ = conn.Close() - if got := proxy.hits.Load(); got != 2 { - t.Fatalf("ordinary WebSocket total proxy hits = %d, want 2", got) + if got := proxy.hits.Load(); got != 3 { + t.Fatalf("ordinary WebSocket total proxy hits = %d, want 3", got) } if host := proxy.nextHost(t); host != "ordinary.example:80" { t.Fatalf("proxy CONNECT host = %q, want ordinary.example:80", host) diff --git a/go/internal/mdnsresolve/mdnsresolve.go b/go/internal/mdnsresolve/mdnsresolve.go index e33b9a6f..7c339b1a 100644 --- a/go/internal/mdnsresolve/mdnsresolve.go +++ b/go/internal/mdnsresolve/mdnsresolve.go @@ -91,10 +91,16 @@ func IsLocal(host string) bool { return strings.HasSuffix(strings.ToLower(strings.TrimSuffix(host, ".")), ".local") } -// CheckLocalDestination enforces the core trust decision before a transport -// chooses a direct socket or an HTTP/WebSocket proxy. Proxy selection happens -// before a net.Dialer callback, so keeping this check separate from Dialer is -// what prevents an untrusted .local request from reaching a proxy first. +// CheckLocalDestination reports whether this caller may use FTW's own mDNS +// answer for host. It gates the resolution path, not the connection: a caller +// that has not opted in still reaches the name through the system resolver, +// exactly as it did before this package existed. +// +// The distinction matters because the two are not the same risk. An mDNS +// answer FTW obtained itself is a new, unverified input. A name the operating +// system resolves is the status quo on every platform FTW ships to — and under +// Home Assistant it is the only path there is, since Supervisor answers .local +// through its own DNS service. func CheckLocalDestination(host string, allowUnverifiedLocal bool) error { if IsLocal(host) && !allowUnverifiedLocal { return fmt.Errorf("mDNS resolution for %s denied: %w", host, ErrUnverifiedLocal) @@ -208,7 +214,13 @@ func (d *Dialer) DialContext(ctx context.Context, network, address string) (net. return d.Dialer.DialContext(ctx, network, address) } if err := CheckLocalDestination(host, d.AllowUnverifiedLocal); err != nil { - return nil, err + // Not a refusal to connect — a refusal to trust *our* mDNS answer. + // The name still goes to the system resolver, which is what every + // shipped version of FTW does with it today, and what makes it work + // under Home Assistant. Denying it outright would break installs that + // work now, and would not buy the security it appears to: a raw IP is + // no more an identity on a LAN than a name is. + return d.systemFallback(ctx, network, address, err) } addrs, err := Lookup(ctx, host) @@ -252,7 +264,9 @@ func (d *Dialer) systemFallback(ctx context.Context, network, address string, md if err == nil { return conn, nil } - return nil, fmt.Errorf("%w (mDNS: %v)", err, mdnsErr) + // Both causes are wrapped, not just formatted, so a caller can test for + // either with errors.Is — ErrUnverifiedLocal in particular. + return nil, fmt.Errorf("%w (mDNS: %w)", err, mdnsErr) } // Dial is the context-free form, for callers that have no context to pass. diff --git a/go/internal/mdnsresolve/mdnsresolve_test.go b/go/internal/mdnsresolve/mdnsresolve_test.go index c0739740..2630373e 100644 --- a/go/internal/mdnsresolve/mdnsresolve_test.go +++ b/go/internal/mdnsresolve/mdnsresolve_test.go @@ -523,11 +523,82 @@ func TestDialerSkipsResolutionForPlainHosts(t *testing.T) { } } -func TestDialerDeniesUnverifiedLocalByDefault(t *testing.T) { - _, err := (&Dialer{}).Dial("tcp", "inverter.local:502") +// Without the opt-in, FTW must not use its *own* mDNS answer — but it must +// still let the system resolver try the name. Refusing outright would break +// Home Assistant, where Supervisor's DNS answers .local and always has. +func TestDialerWithoutOptInIgnoresOurAnswerButStillResolves(t *testing.T) { + Flush() + listener, err := net.Listen("tcp4", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + defer listener.Close() + _, port, _ := net.SplitHostPort(listener.Addr().String()) + + // An avahi that would happily point at the live listener. A dialer without + // the opt-in must never reach it. + fakeAvahi(t, func(command, name string) string { + if command == "RESOLVE-HOSTNAME-IPV4" && name == "inverter.local" { + return "+ 2 0 inverter.local 127.0.0.1" + } + return "- 15 Timeout reached" + }) + + consulted := make(chan struct{}, 1) + d := Dialer{Dialer: net.Dialer{ + Timeout: 500 * time.Millisecond, + Resolver: &net.Resolver{ + PreferGo: true, + Dial: func(context.Context, string, string) (net.Conn, error) { + select { + case consulted <- struct{}{}: + default: + } + return nil, errors.New("no nameserver in this test") + }, + }, + }} + + conn, err := d.Dial("tcp", "inverter.local:"+port) + if err == nil { + conn.Close() + t.Fatal("dialed our own mDNS answer without allow_unverified_local") + } + select { + case <-consulted: + default: + t.Fatal("the system resolver was never given the name") + } + // The reason we did not use our own answer has to survive into the error, + // or an operator cannot tell this from a device that is simply gone. if !errors.Is(err, ErrUnverifiedLocal) { - t.Fatalf("Dial without local opt-in error = %v, want ErrUnverifiedLocal", err) + t.Fatalf("error %v does not carry ErrUnverifiedLocal", err) + } +} + +// With the opt-in, our own answer is used. +func TestDialerWithOptInUsesOurAnswer(t *testing.T) { + Flush() + listener, err := net.Listen("tcp4", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + defer listener.Close() + _, port, _ := net.SplitHostPort(listener.Addr().String()) + + fakeAvahi(t, func(command, name string) string { + if command == "RESOLVE-HOSTNAME-IPV4" && name == "inverter.local" { + return "+ 2 0 inverter.local 127.0.0.1" + } + return "- 15 Timeout reached" + }) + + d := Dialer{AllowUnverifiedLocal: true, Dialer: net.Dialer{Timeout: time.Second}} + conn, err := d.Dial("tcp", "inverter.local:"+port) + if err != nil { + t.Fatalf("dial with opt-in: %v", err) } + conn.Close() } func TestDialerLeavesOrdinaryDNSAndIPUnchanged(t *testing.T) { From 4d746dddaa3b9f13867d1480d9709487fdf69ae1 Mon Sep 17 00:00:00 2001 From: Claude Opus 5 Date: Wed, 5 Aug 2026 12:32:42 +0200 Subject: [PATCH 7/7] fix(scan): report the name a device answers for itself 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