diff --git a/.changeset/tighten-local-authority.md b/.changeset/tighten-local-authority.md new file mode 100644 index 00000000..86c277cf --- /dev/null +++ b/.changeset/tighten-local-authority.md @@ -0,0 +1,5 @@ +--- +"ftw": patch +--- + +A hostname with no dot is no longer treated as local. If the driver catalog cannot be read, or a configured driver is missing from it, config secrets stay hidden. Driver test and fingerprint refuse loopback, localhost, and link-local targets on MQTT, Modbus, HTTP, WebSocket and TCP. diff --git a/docs/operations.md b/docs/operations.md index d5272d62..193fd1db 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -84,7 +84,7 @@ changing them. When unsure, inspect the restart classification in FTW accepts state-changing requests without credentials only when they are addressed through a local name or address: loopback, private/link-local IP, -an unqualified hostname, `.local`, `.localhost`, or `.home.arpa`. The setup +`.local`, `.localhost`, or `.home.arpa`. The setup wizard follows the same rule, and the actual client address must also be local. Browser writes must also be same-origin; FTW checks `Origin`, `Host`, and `Sec-Fetch-Site` and does not advertise CORS. diff --git a/go/internal/api/api.go b/go/internal/api/api.go index 80cd7f1d..ce301167 100644 --- a/go/internal/api/api.go +++ b/go/internal/api/api.go @@ -1325,8 +1325,9 @@ func (s *Server) handleGetConfig(w http.ResponseWriter, r *http.Request) { // driverSecretKeys returns a map[lua-path]→[]secret-key built from the // drivers/ catalog. Used by handleGetConfig + handlePostConfig to scope // which `Driver.Config[*]` keys participate in the mask/restore cycle. -// On catalog read errors returns nil — handlers then skip the secrets -// pass entirely (fail-open: they still mask the structured fields). +// On catalog read errors returns nil. maskDriverConfigSecrets then +// blanks every string in Driver.Config (fail-closed) instead of +// leaving undeclared tokens in the GET body. func (s *Server) driverSecretKeys() map[string][]string { dir := s.deps.DriverDir if dir == "" { @@ -1338,17 +1339,18 @@ func (s *Server) driverSecretKeys() map[string][]string { } out := make(map[string][]string, len(entries)) for _, e := range entries { - if len(e.ConfigSecrets) == 0 { - continue + keys := e.ConfigSecrets + if keys == nil { + keys = []string{} } path := filepath.ToSlash(e.Path) - out[path] = e.ConfigSecrets + out[path] = keys base := filepath.ToSlash(filepath.Base(dir)) if rel, ok := strings.CutPrefix(path, base+"/"); ok { // Config round-trips paths resolved via -drivers as // "drivers/" regardless of the actual directory name. // Keep catalog secret matching on that portable alias too. - out[filepath.ToSlash(filepath.Join("drivers", rel))] = e.ConfigSecrets + out[filepath.ToSlash(filepath.Join("drivers", rel))] = keys } } return out @@ -1360,11 +1362,19 @@ func (s *Server) driverSecretKeys() map[string][]string { // MaskSecrets for fields the config package can't know about (the // catalog isn't a config-package dependency on purpose). func maskDriverConfigSecrets(cfg *config.Config, secretsByLua map[string][]string) { - if cfg == nil || len(secretsByLua) == 0 { + if cfg == nil { + return + } + if secretsByLua == nil { + maskAllDriverConfigStrings(cfg) return } for i := range cfg.Drivers { - keys := secretsByLua[cfg.Drivers[i].Lua] + keys, known := secretsByLua[cfg.Drivers[i].Lua] + if !known { + maskOneDriverConfigStrings(&cfg.Drivers[i]) + continue + } if len(keys) == 0 || cfg.Drivers[i].Config == nil { continue } @@ -1386,17 +1396,85 @@ func maskDriverConfigSecrets(cfg *config.Config, secretsByLua map[string][]strin } } +func maskAllDriverConfigStrings(cfg *config.Config) { + for i := range cfg.Drivers { + maskOneDriverConfigStrings(&cfg.Drivers[i]) + } +} + +func maskOneDriverConfigStrings(d *config.Driver) { + if d == nil || d.Config == nil { + return + } + cp := make(map[string]any, len(d.Config)) + for k, v := range d.Config { + if s, ok := v.(string); ok && s != "" { + cp[k] = maskedPlaceholder + } else { + cp[k] = v + } + } + d.Config = cp +} + +func restoreAllBlankDriverConfigStrings(incoming, existing *config.Config) { + if incoming == nil || existing == nil { + return + } + for i := range incoming.Drivers { + restoreOneDriverBlankStrings(&incoming.Drivers[i], existing) + } +} + +func restoreOneDriverBlankStrings(incoming *config.Driver, existing *config.Config) { + if incoming == nil || existing == nil { + return + } + var ed *config.Driver + for j := range existing.Drivers { + if existing.Drivers[j].Name == incoming.Name { + ed = &existing.Drivers[j] + break + } + } + if ed == nil || ed.Config == nil { + return + } + if incoming.Config == nil { + incoming.Config = map[string]any{} + } + for k, existingV := range ed.Config { + existingS, ok := existingV.(string) + if !ok || existingS == "" { + continue + } + incomingV, hasI := incoming.Config[k] + incomingS, _ := incomingV.(string) + if !hasI || incomingS == "" || incomingS == maskedPlaceholder { + incoming.Config[k] = existingS + } + } +} + // restoreDriverConfigSecrets is the symmetric POST-side step: for each // driver, any catalog-declared secret value the UI sent back as the // masked placeholder OR as an empty string (with a non-empty existing // value) gets restored from `existing`. Without this, blanking the // password input in the Settings tab would clobber the saved token. func restoreDriverConfigSecrets(incoming, existing *config.Config, secretsByLua map[string][]string) { - if incoming == nil || existing == nil || len(secretsByLua) == 0 { + if incoming == nil || existing == nil { + return + } + if secretsByLua == nil { + restoreAllBlankDriverConfigStrings(incoming, existing) return } for i := range incoming.Drivers { - keys := secretsByLua[incoming.Drivers[i].Lua] + keys, known := secretsByLua[incoming.Drivers[i].Lua] + if !known { + restoreOneDriverBlankStrings(&incoming.Drivers[i], existing) + continue + } if len(keys) == 0 { continue } diff --git a/go/internal/api/api_driver_secrets_test.go b/go/internal/api/api_driver_secrets_test.go index 52e0eebc..9db06476 100644 --- a/go/internal/api/api_driver_secrets_test.go +++ b/go/internal/api/api_driver_secrets_test.go @@ -62,3 +62,115 @@ DRIVER = { t.Fatalf("restored api_token = %q, want original secret", got) } } + +func TestMaskDriverConfigSecretsFailsClosedWhenCatalogMissing(t *testing.T) { + cfg := &config.Config{Drivers: []config.Driver{{ + Name: "sonnen", + Lua: "drivers/sonnen.lua", + Config: map[string]any{ + "api_token": "secret-token", + "host": "192.168.1.10", + }, + }}} + maskDriverConfigSecrets(cfg, nil) + if got := cfg.Drivers[0].Config["api_token"]; got != maskedPlaceholder { + t.Fatalf("api_token = %q, want masked when catalog is unreadable", got) + } + if got := cfg.Drivers[0].Config["host"]; got != maskedPlaceholder { + t.Fatalf("host = %q, want masked when catalog is unreadable", got) + } + + incoming := &config.Config{Drivers: []config.Driver{{ + Name: "sonnen", + Config: map[string]any{ + "api_token": maskedPlaceholder, + "host": maskedPlaceholder, + }, + }}} + existing := &config.Config{Drivers: []config.Driver{{ + Name: "sonnen", + Config: map[string]any{ + "api_token": "secret-token", + "host": "192.168.1.10", + }, + }}} + restoreDriverConfigSecrets(incoming, existing, nil) + if got := incoming.Drivers[0].Config["api_token"]; got != "secret-token" { + t.Fatalf("restored api_token = %q", got) + } + if got := incoming.Drivers[0].Config["host"]; got != "192.168.1.10" { + t.Fatalf("restored host = %q", got) + } +} + +func TestMaskDriverConfigSecretsFailsClosedWhenCatalogSkipsDriver(t *testing.T) { + cfg := &config.Config{Drivers: []config.Driver{{ + Name: "sonnen", + Lua: "drivers/sonnen.lua", + Config: map[string]any{ + "api_token": "secret-token", + }, + }}} + // Another driver loaded; this lua file was skipped. + maskDriverConfigSecrets(cfg, map[string][]string{"drivers/other.lua": {"api_token"}}) + if got := cfg.Drivers[0].Config["api_token"]; got != maskedPlaceholder { + t.Fatalf("api_token = %q, want masked when the catalog skipped this driver", got) + } + + emptyCatalog := &config.Config{Drivers: []config.Driver{{ + Name: "sonnen", + Lua: "drivers/sonnen.lua", + Config: map[string]any{"api_token": "secret-token"}, + }}} + maskDriverConfigSecrets(emptyCatalog, map[string][]string{}) + if got := emptyCatalog.Drivers[0].Config["api_token"]; got != maskedPlaceholder { + t.Fatalf("api_token = %q, want masked when the catalog loaded nothing", got) + } +} + +func TestRejectUnsafeProbeHost(t *testing.T) { + if err := rejectUnsafeProbeHost("127.0.0.1"); err == nil { + t.Fatal("loopback should be refused") + } + if err := rejectUnsafeProbeHost("::1"); err == nil { + t.Fatal("IPv6 loopback should be refused") + } + if err := rejectUnsafeProbeHost("[::1]"); err == nil { + t.Fatal("bracketed IPv6 loopback should be refused") + } + if err := rejectUnsafeProbeHost("localhost"); err == nil { + t.Fatal("localhost should be refused") + } + if err := rejectUnsafeProbeHost("169.254.1.1"); err == nil { + t.Fatal("link-local should be refused") + } + if err := rejectUnsafeProbeHost("fe80::1"); err == nil { + t.Fatal("IPv6 link-local should be refused") + } + if err := rejectUnsafeProbeHost("0.0.0.0"); err == nil { + t.Fatal("unspecified should be refused") + } + if err := rejectUnsafeProbeHost("192.168.1.10"); err != nil { + t.Fatalf("private IP refused: %v", err) + } + if err := rejectUnsafeProbeHost("zap.local"); err != nil { + t.Fatalf("hostname refused: %v", err) + } +} + +func TestRejectUnsafeProbeTargetsCoversHTTP(t *testing.T) { + cfg := config.Driver{ + Config: map[string]any{"host": "127.0.0.1"}, + Capabilities: config.Capabilities{ + HTTP: &config.HTTPCapability{AllowedHosts: []string{"127.0.0.1"}}, + }, + } + if err := rejectUnsafeProbeTargets(cfg); err == nil { + t.Fatal("HTTP loopback probe should be refused") + } + cfg.Config["host"] = "192.168.1.10" + cfg.Capabilities.HTTP.AllowedHosts = []string{"inverter.local"} + if err := rejectUnsafeProbeTargets(cfg); err != nil { + t.Fatalf("LAN HTTP probe refused: %v", err) + } +} diff --git a/go/internal/api/api_drivers_debug.go b/go/internal/api/api_drivers_debug.go index a4a1b7ca..61140382 100644 --- a/go/internal/api/api_drivers_debug.go +++ b/go/internal/api/api_drivers_debug.go @@ -11,7 +11,9 @@ import ( "context" "encoding/json" "fmt" + "net" "net/http" + "net/url" "os" "path/filepath" "runtime" @@ -234,6 +236,10 @@ func (s *Server) handleDriverTest(w http.ResponseWriter, r *http.Request) { resolved.ResolveDriverPaths(baseDir) cfg = resolved.Drivers[0] + if err := rejectUnsafeProbeTargets(cfg); err != nil { + writeJSON(w, 400, map[string]string{"error": err.Error()}) + return + } if mq := cfg.EffectiveMQTT(); mq != nil { if mq.Port == 0 { mq.Port = 1883 @@ -314,6 +320,122 @@ func (s *Server) handleDriverTest(w http.ResponseWriter, r *http.Request) { } } +// rejectUnsafeProbeTargets checks every host a driver test might dial: +// MQTT, Modbus, config.host / config.url, and HTTP/WS/TCP allowlists. +func rejectUnsafeProbeTargets(cfg config.Driver) error { + if mq := cfg.EffectiveMQTT(); mq != nil { + if err := rejectUnsafeProbeHost(mq.Host); err != nil { + return fmt.Errorf("mqtt host: %w", err) + } + } + if mb := cfg.EffectiveModbus(); mb != nil { + if err := rejectUnsafeProbeHost(mb.Host); err != nil { + return fmt.Errorf("modbus host: %w", err) + } + } + if cfg.Config != nil { + if h, ok := cfg.Config["host"].(string); ok && strings.TrimSpace(h) != "" { + if err := rejectUnsafeProbeHost(h); err != nil { + return fmt.Errorf("config.host: %w", err) + } + } + if u, ok := cfg.Config["url"].(string); ok && strings.TrimSpace(u) != "" { + if host := hostFromProbeURL(u); host != "" { + if err := rejectUnsafeProbeHost(host); err != nil { + return fmt.Errorf("config.url: %w", err) + } + } + } + } + if httpCap := cfg.Capabilities.HTTP; httpCap != nil { + for _, h := range httpCap.AllowedHosts { + if strings.TrimSpace(h) == "" { + continue + } + if err := rejectUnsafeProbeHost(hostFromAllowlistEntry(h)); err != nil { + return fmt.Errorf("http allowlist: %w", err) + } + } + } + if ws := cfg.Capabilities.WebSocket; ws != nil { + for _, h := range ws.AllowedHosts { + if strings.TrimSpace(h) == "" { + continue + } + if err := rejectUnsafeProbeHost(hostFromAllowlistEntry(h)); err != nil { + return fmt.Errorf("websocket allowlist: %w", err) + } + } + } + if tcp := cfg.Capabilities.TCP; tcp != nil { + for _, h := range tcp.AllowedHosts { + if strings.TrimSpace(h) == "" { + continue + } + if err := rejectUnsafeProbeHost(hostFromAllowlistEntry(h)); err != nil { + return fmt.Errorf("tcp allowlist: %w", err) + } + } + } + return nil +} + +func hostFromProbeURL(raw string) string { + u, err := url.Parse(raw) + if err != nil || u.Host == "" { + return strings.TrimSpace(raw) + } + return u.Hostname() +} + +func hostFromAllowlistEntry(entry string) string { + entry = strings.TrimSpace(entry) + if host, _, err := net.SplitHostPort(entry); err == nil { + return host + } + return entry +} + +// rejectUnsafeProbeHost stops a driver test or fingerprint from dialing +// the box itself or link-local/metadata addresses. Hostnames such as +// zap.local still probe unless they resolve to a forbidden address. +func rejectUnsafeProbeHost(host string) error { + host = strings.TrimSpace(host) + if i := strings.Index(host, "%"); i >= 0 { + host = host[:i] + } + host = strings.TrimSuffix(strings.TrimPrefix(host, "["), "]") + if host == "" { + return fmt.Errorf("missing host") + } + lower := strings.ToLower(host) + if lower == "localhost" || strings.HasSuffix(lower, ".localhost") { + return fmt.Errorf("loopback, link-local and unspecified addresses are not permitted") + } + if ip := net.ParseIP(host); ip != nil { + return rejectUnsafeProbeIP(ip) + } + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + addrs, err := net.DefaultResolver.LookupIPAddr(ctx, host) + if err != nil { + return nil + } + for _, a := range addrs { + if err := rejectUnsafeProbeIP(a.IP); err != nil { + return err + } + } + return nil +} + +func rejectUnsafeProbeIP(ip net.IP) error { + if ip.IsLoopback() || ip.IsUnspecified() || ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast() || ip.IsMulticast() { + return fmt.Errorf("loopback, link-local and unspecified addresses are not permitted") + } + return nil +} + func collectDriverProbe(displayName, runtimeName string, tel *telemetry.Store, reg *drivers.Registry, started time.Time) driverProbeResp { resp := driverProbeResp{Name: displayName, ElapsedMs: time.Since(started).Milliseconds()} if h := tel.DriverHealth(runtimeName); h != nil { diff --git a/go/internal/api/api_drivers_fingerprint.go b/go/internal/api/api_drivers_fingerprint.go index 84e28b9f..4bdcb6b8 100644 --- a/go/internal/api/api_drivers_fingerprint.go +++ b/go/internal/api/api_drivers_fingerprint.go @@ -218,8 +218,14 @@ func normalizeFingerprintHost(raw string) (string, error) { host = strings.TrimSuffix(strings.TrimPrefix(host, "["), "]") } if ip := net.ParseIP(host); ip != nil { + if err := rejectUnsafeProbeHost(host); err != nil { + return "", err + } return host, nil } + if err := rejectUnsafeProbeHost(host); err != nil { + return "", err + } if len(host) > 253 || strings.ContainsAny(host, "/\\@?#:") { return "", fmt.Errorf("invalid host") } diff --git a/go/internal/api/security.go b/go/internal/api/security.go index 08162f8c..0a08d2e5 100644 --- a/go/internal/api/security.go +++ b/go/internal/api/security.go @@ -390,11 +390,13 @@ func isLocalAuthority(a authority) bool { if ip := net.ParseIP(host); ip != nil { return ip.IsLoopback() || ip.IsPrivate() || ip.IsLinkLocalUnicast() } + // A name with no dot is not local. `http://intranet` rebound onto + // this box would otherwise skip the remote token. Keep `.local` — + // that is the documented LAN name (ftw.local). return host == "localhost" || strings.HasSuffix(host, ".localhost") || strings.HasSuffix(host, ".local") || - strings.HasSuffix(host, ".home.arpa") || - !strings.Contains(host, ".") + strings.HasSuffix(host, ".home.arpa") } func isLocalClient(remoteAddr string) bool { diff --git a/go/internal/api/security_test.go b/go/internal/api/security_test.go index ffccfe2a..70d8b0bd 100644 --- a/go/internal/api/security_test.go +++ b/go/internal/api/security_test.go @@ -349,6 +349,18 @@ func TestAuthenticateRequiresBearerTokenForRemoteHost(t *testing.T) { } } +func TestAuthenticateNoDotHostIsNotLocal(t *testing.T) { + req := mutationRequest(sensitiveMutations[4], "http://intranet:8080") + req.RemoteAddr = "192.168.1.10:43210" + req.Header.Set("Origin", "http://intranet:8080") + req.Header.Set("Sec-Fetch-Site", "same-origin") + rr := httptest.NewRecorder() + Authenticate(statusHandler(http.StatusNoContent), MutationPolicy{RequireTokenForRemote: true}).ServeHTTP(rr, req) + if rr.Code != http.StatusForbidden { + t.Fatalf("no-dot host status = %d, want 403 (body=%s)", rr.Code, rr.Body.String()) + } +} + func TestAuthenticateRemoteClientCannotSpoofLocalHost(t *testing.T) { req := mutationRequest(sensitiveMutations[4], "http://192.168.1.42:8080") req.RemoteAddr = "203.0.113.10:43210"