From 60f3ca4a3a0c44f7170441afa5c8c0211fdfc226 Mon Sep 17 00:00:00 2001 From: Paul Payne Date: Tue, 14 Jul 2026 12:38:17 +0000 Subject: [PATCH] Security hardening: input validation, secrets protection, NATS auth MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Config injection prevention: - Add FQDN validation for domain names (RFC 1123) in Register/Update — rejects newlines, spaces, shell metacharacters that could inject into HAProxy/dnsmasq configs - Add backend address validation (valid host:port format, valid IP or hostname, port 1-65535). DNS-only backends allow bare IPs. - Add header key/value validation — keys must be HTTP token chars only, values must not contain newlines or NULs - Add WireGuard peer name validation (alphanumeric + hyphens + underscores) - Add defense-in-depth domain validation in certbot Provision() Secrets protection: - Remove ?raw=true bypass on GET /api/v1/secrets — secrets are now always redacted in API responses regardless of query parameters - Update test to verify redaction cannot be bypassed NATS authentication: - Generate random auth token on first startup, store in secrets.yaml - Pass token to embedded NATS server via Authorization option - Internal client connects with the same token - External NATS clients (Wild Cloud) must now authenticate Security headers: - Add X-Content-Type-Options: nosniff - Add X-Frame-Options: DENY - Add Cache-Control: no-store --- internal/api/v1/handlers.go | 6 +- internal/api/v1/handlers_config_test.go | 7 +- internal/api/v1/middleware.go | 5 ++ internal/certbot/manager.go | 7 ++ internal/domains/manager.go | 99 +++++++++++++++++++++++++ internal/natsbus/server.go | 19 +++-- internal/wireguard/manager.go | 7 ++ main.go | 15 +++- 8 files changed, 149 insertions(+), 16 deletions(-) diff --git a/internal/api/v1/handlers.go b/internal/api/v1/handlers.go index a2de414..e60080e 100644 --- a/internal/api/v1/handlers.go +++ b/internal/api/v1/handlers.go @@ -406,11 +406,7 @@ func (api *API) GetGlobalSecrets(w http.ResponseWriter, r *http.Request) { return } - showRaw := r.URL.Query().Get("raw") == "true" - if !showRaw { - redactSecrets(secretsMap) - } - + redactSecrets(secretsMap) respondJSON(w, http.StatusOK, secretsMap) } diff --git a/internal/api/v1/handlers_config_test.go b/internal/api/v1/handlers_config_test.go index c9209eb..ed2a276 100644 --- a/internal/api/v1/handlers_config_test.go +++ b/internal/api/v1/handlers_config_test.go @@ -138,7 +138,7 @@ func TestGetGlobalSecrets_RedactsLeafValues(t *testing.T) { } } -func TestGetGlobalSecrets_RawShowsValues(t *testing.T) { +func TestGetGlobalSecrets_AlwaysRedacted(t *testing.T) { api, tmpDir := setupTestAPI(t) secrets := map[string]any{ @@ -149,6 +149,7 @@ func TestGetGlobalSecrets_RawShowsValues(t *testing.T) { data, _ := yaml.Marshal(secrets) os.WriteFile(filepath.Join(tmpDir, "secrets.yaml"), data, 0600) + // Even with ?raw=true, secrets must be redacted (bypass removed for security) req := httptest.NewRequest("GET", "/api/v1/secrets?raw=true", nil) w := httptest.NewRecorder() @@ -158,8 +159,8 @@ func TestGetGlobalSecrets_RawShowsValues(t *testing.T) { json.Unmarshal(w.Body.Bytes(), &resp) cf := resp["cloudflare"].(map[string]any) - if cf["apiToken"] != "my-token" { - t.Errorf("expected raw apiToken, got %v", cf["apiToken"]) + if cf["apiToken"] == "my-token" { + t.Error("expected apiToken to be redacted, but got raw value") } } diff --git a/internal/api/v1/middleware.go b/internal/api/v1/middleware.go index cdd2e81..562a0e1 100644 --- a/internal/api/v1/middleware.go +++ b/internal/api/v1/middleware.go @@ -49,6 +49,11 @@ func RequestLoggingMiddleware(next http.Handler) http.Handler { return } + // Security response headers + w.Header().Set("X-Content-Type-Options", "nosniff") + w.Header().Set("X-Frame-Options", "DENY") + w.Header().Set("Cache-Control", "no-store") + start := time.Now() sw := &statusResponseWriter{ResponseWriter: w, status: http.StatusOK} next.ServeHTTP(sw, r) diff --git a/internal/certbot/manager.go b/internal/certbot/manager.go index cba3928..8af352c 100644 --- a/internal/certbot/manager.go +++ b/internal/certbot/manager.go @@ -61,6 +61,13 @@ func (m *Manager) Provision(domain, email string) error { if email == "" { return fmt.Errorf("email is required") } + // Defense-in-depth: reject domains with shell metacharacters. + // Domain validation at the API boundary should already prevent this. + for _, c := range domain { + if !((c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c >= '0' && c <= '9') || c == '-' || c == '.' || c == '*') { + return fmt.Errorf("invalid domain for cert provisioning: %q", domain) + } + } if _, err := os.Stat(m.credsPath); err != nil { return fmt.Errorf("cloudflare credentials not found at %s; configure the Cloudflare API token first", m.credsPath) } diff --git a/internal/domains/manager.go b/internal/domains/manager.go index 1534c48..81772bf 100644 --- a/internal/domains/manager.go +++ b/internal/domains/manager.go @@ -9,8 +9,10 @@ package domains import ( "fmt" "log/slog" + "net" "os" "path/filepath" + "strconv" "strings" "sync" @@ -161,6 +163,86 @@ func (m *Manager) domainPath(domain string) string { return filepath.Join(m.domainsDir(), domainToFilename(domain)) } +// isValidDomain checks that a string is a valid FQDN (RFC 1123) or wildcard domain. +// Prevents config injection via newlines, spaces, or shell metacharacters. +func isValidDomain(s string) bool { + if len(s) == 0 || len(s) > 253 { + return false + } + for _, label := range strings.Split(s, ".") { + if len(label) == 0 || len(label) > 63 { + return false + } + if label[0] == '-' || label[len(label)-1] == '-' { + return false + } + for _, c := range label { + if !((c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c >= '0' && c <= '9') || c == '-' || c == '*') { + return false + } + } + } + return true +} + +// isValidBackendAddress checks that an address is a valid host:port. +func isValidBackendAddress(addr string) bool { + host, portStr, err := net.SplitHostPort(addr) + if err != nil { + return false + } + port, err := strconv.Atoi(portStr) + if err != nil || port < 1 || port > 65535 { + return false + } + if net.ParseIP(host) != nil { + return true + } + return isValidDomain(host) +} + +// isValidHeaderName checks that a header name contains only HTTP token characters. +func isValidHeaderName(s string) bool { + if len(s) == 0 { + return false + } + for _, c := range s { + if c <= ' ' || c == ':' || c == '"' || c >= 0x7f { + return false + } + } + return true +} + +// isValidHeaderValue checks that a header value contains no newlines or NULs. +func isValidHeaderValue(s string) bool { + return !strings.ContainsAny(s, "\r\n\x00") +} + +// validateHeaders checks all header keys and values in a HeaderConfig. +func validateHeaders(h *HeaderConfig) error { + if h == nil { + return nil + } + for k, v := range h.Request { + if !isValidHeaderName(k) { + return fmt.Errorf("invalid request header name: %q", k) + } + if !isValidHeaderValue(v) { + return fmt.Errorf("invalid request header value for %q", k) + } + } + for k, v := range h.Response { + if !isValidHeaderName(k) { + return fmt.Errorf("invalid response header name: %q", k) + } + if !isValidHeaderValue(v) { + return fmt.Errorf("invalid response header value for %q", k) + } + } + return nil +} + // Register creates or updates a domain registration. func (m *Manager) Register(dom Domain) error { m.mu.Lock() @@ -169,6 +251,9 @@ func (m *Manager) Register(dom Domain) error { if dom.DomainName == "" { return fmt.Errorf("domain is required") } + if !isValidDomain(dom.DomainName) { + return fmt.Errorf("invalid domain name: %q", dom.DomainName) + } hasBackend := dom.Backend.Address != "" hasRoutes := len(dom.Routes) > 0 @@ -185,14 +270,28 @@ func (m *Manager) Register(dom Domain) error { if r.Backend.Address == "" { return fmt.Errorf("route %d: backend address is required", i) } + if !isValidBackendAddress(r.Backend.Address) { + return fmt.Errorf("route %d: invalid backend address: %q", i, r.Backend.Address) + } if r.Backend.Type == "" { return fmt.Errorf("route %d: backend type is required", i) } + if err := validateHeaders(r.Headers); err != nil { + return fmt.Errorf("route %d: %w", i, err) + } } } else { if dom.Backend.Type == "" { return fmt.Errorf("backend type is required") } + // dns-only backends may omit port (just an IP for resolution) + if dom.Backend.Type == BackendDNSOnly { + if net.ParseIP(dom.Backend.Address) == nil && !isValidBackendAddress(dom.Backend.Address) { + return fmt.Errorf("invalid backend address: %q", dom.Backend.Address) + } + } else if !isValidBackendAddress(dom.Backend.Address) { + return fmt.Errorf("invalid backend address: %q", dom.Backend.Address) + } } // Default TLS mode based on backend type diff --git a/internal/natsbus/server.go b/internal/natsbus/server.go index d065e12..208a6c6 100644 --- a/internal/natsbus/server.go +++ b/internal/natsbus/server.go @@ -37,8 +37,9 @@ type Server struct { // Config for the embedded NATS server. type Config struct { - Port int // listen port (default 4222) - DataDir string // JetStream storage directory + Port int // listen port (default 4222) + DataDir string // JetStream storage directory + AuthToken string // require this token for client connections (empty = no auth) } // Start launches the embedded NATS server with JetStream and creates @@ -52,9 +53,11 @@ func Start(cfg Config) (*Server, error) { Port: cfg.Port, JetStream: true, StoreDir: cfg.DataDir, - // Quiet logging — NATS logs at its own level - NoLog: true, - NoSigs: true, + NoLog: true, + NoSigs: true, + } + if cfg.AuthToken != "" { + opts.Authorization = cfg.AuthToken } ns, err := natsserver.NewServer(opts) @@ -87,7 +90,11 @@ func Start(cfg Config) (*Server, error) { slog.Info("NATS JetStream server started", "port", cfg.Port, "dataDir", cfg.DataDir) // Connect as internal client - nc, err := nats.Connect(ns.ClientURL()) + connectOpts := []nats.Option{} + if cfg.AuthToken != "" { + connectOpts = append(connectOpts, nats.Token(cfg.AuthToken)) + } + nc, err := nats.Connect(ns.ClientURL(), connectOpts...) if err != nil { ns.Shutdown() return nil, fmt.Errorf("connecting to embedded NATS: %w", err) diff --git a/internal/wireguard/manager.go b/internal/wireguard/manager.go index 49b2722..829ff29 100644 --- a/internal/wireguard/manager.go +++ b/internal/wireguard/manager.go @@ -229,6 +229,13 @@ func (m *Manager) readPeerFile(path string) (*Peer, error) { // AddPeer generates a new peer keypair, assigns the next available IP in the VPN // subnet, saves the peer, and returns it. func (m *Manager) AddPeer(name string) (*Peer, error) { + // Validate peer name — prevents config injection via wg0.conf comments + for _, c := range name { + if !((c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c >= '0' && c <= '9') || c == '-' || c == '_' || c == ' ') { + return nil, fmt.Errorf("invalid peer name: only letters, digits, hyphens, underscores, and spaces allowed") + } + } + cfg, err := m.GetConfig() if err != nil { return nil, err diff --git a/main.go b/main.go index 358ce89..765a52a 100644 --- a/main.go +++ b/main.go @@ -19,6 +19,7 @@ import ( "github.com/wild-cloud/wild-central/internal/frontend" "github.com/wild-cloud/wild-central/internal/logging" "github.com/wild-cloud/wild-central/internal/natsbus" + "github.com/wild-cloud/wild-central/internal/secrets" ) var startTime time.Time @@ -99,9 +100,19 @@ func main() { fmt.Sscanf(v, "%d", &natsPort) } + // Load or generate NATS auth token from secrets + secretsMgr := secrets.NewManager(filepath.Join(dataDir, "secrets.yaml")) + natsToken, _ := secretsMgr.GetSecret("nats.authToken") + if natsToken == "" { + natsToken, _ = secrets.GenerateSecret(32) + _ = secretsMgr.SetSecret("nats.authToken", natsToken) + slog.Info("generated NATS auth token", "component", "startup") + } + natsSrv, err := natsbus.Start(natsbus.Config{ - Port: natsPort, - DataDir: natsDataDir, + Port: natsPort, + DataDir: natsDataDir, + AuthToken: natsToken, }) if err != nil { slog.Error("failed to start NATS server", "error", err)