diff --git a/apps/backend/internal/backendapp/middleware.go b/apps/backend/internal/backendapp/middleware.go index 2f5d860a34..1f8702c7f1 100644 --- a/apps/backend/internal/backendapp/middleware.go +++ b/apps/backend/internal/backendapp/middleware.go @@ -1,19 +1,18 @@ package backendapp import ( - "net" "net/http" - "net/url" - "strings" "github.com/gin-gonic/gin" + + "github.com/kandev/kandev/internal/common/httpmw" ) // corsMiddleware returns a CORS middleware for HTTP and WebSocket connections. func corsMiddleware() gin.HandlerFunc { return func(c *gin.Context) { if origin := c.Request.Header.Get("Origin"); origin != "" { - if !isAllowedCORSOrigin(origin, c.Request.Host) { + if !httpmw.AllowedOrigin(origin, c.Request.Host) { if c.Request.Method == http.MethodOptions { c.AbortWithStatus(http.StatusForbidden) return @@ -40,38 +39,3 @@ func corsMiddleware() gin.HandlerFunc { c.Next() } } - -func isAllowedCORSOrigin(origin string, requestHost string) bool { - parsed, err := url.Parse(origin) - if err != nil || parsed.Scheme == "" || parsed.Host == "" { - return false - } - if parsed.Scheme != "http" && parsed.Scheme != "https" { - return false - } - - originHost := normalizeCORSHost(parsed.Hostname()) - host := normalizeCORSHost(requestHost) - if originHost == "" || host == "" { - return false - } - - return originHost == host || (isLoopbackCORSHost(originHost) && isLoopbackCORSHost(host)) -} - -func normalizeCORSHost(host string) string { - if parsedHost, _, err := net.SplitHostPort(host); err == nil { - host = parsedHost - } - - return strings.ToLower(strings.Trim(host, "[]")) -} - -func isLoopbackCORSHost(host string) bool { - if host == "localhost" { - return true - } - - ip := net.ParseIP(host) - return ip != nil && ip.IsLoopback() -} diff --git a/apps/backend/internal/common/httpmw/origin.go b/apps/backend/internal/common/httpmw/origin.go new file mode 100644 index 0000000000..663e0a8908 --- /dev/null +++ b/apps/backend/internal/common/httpmw/origin.go @@ -0,0 +1,70 @@ +package httpmw + +import ( + "net" + "net/url" + "strings" +) + +// AllowedOrigin reports whether a browser Origin header value is trusted to +// reach this backend, given the Host of the incoming request. It is the single +// origin trust policy shared by the HTTP CORS middleware and the WebSocket +// upgraders so the two cannot silently diverge. +// +// Policy: +// - Origin hostname equals the request hostname: allow. Ports are +// deliberately not compared — the dev SPA connects cross-port over the +// same hostname (apps/web/lib/config.ts), and content served from the +// backend's own hostname implies a process on that host, which could +// open a direct connection without an Origin header anyway. +// - Origin and request host are both loopback: allow (dev servers and the +// desktop shell talk to the backend across different loopback ports). +// - Anything else: deny. +// +// Hostnames are compared exactly after parsing — a prefix check against +// "http://localhost" would also accept http://localhost.attacker.tld. +// +// The empty-Origin case is intentionally not handled here; callers decide +// (CORS answers with a wildcard, the WebSocket upgrader allows non-browser +// clients such as the CLI or curl). +func AllowedOrigin(origin, requestHost string) bool { + parsed, err := url.Parse(origin) + if err != nil || !isWellFormedOrigin(parsed) { + return false + } + if parsed.Scheme != "http" && parsed.Scheme != "https" { + return false + } + + originHost := normalizeHost(parsed.Hostname()) + host := normalizeHost(requestHost) + if originHost == "" || host == "" { + return false + } + + return originHost == host || (isLoopbackHost(originHost) && isLoopbackHost(host)) +} + +// isWellFormedOrigin rejects URL components that never appear in a +// browser-sent Origin header (userinfo, path, query, fragment, opaque data); +// url.Parse accepts them, but the strict policy treats them as malformed. +func isWellFormedOrigin(u *url.URL) bool { + return u.Scheme != "" && u.Host != "" && + u.User == nil && u.Opaque == "" && u.Path == "" && + u.RawQuery == "" && !u.ForceQuery && u.Fragment == "" +} + +func normalizeHost(host string) string { + if parsedHost, _, err := net.SplitHostPort(host); err == nil { + host = parsedHost + } + return strings.ToLower(strings.Trim(host, "[]")) +} + +func isLoopbackHost(host string) bool { + if host == "localhost" { + return true + } + ip := net.ParseIP(host) + return ip != nil && ip.IsLoopback() +} diff --git a/apps/backend/internal/common/httpmw/origin_test.go b/apps/backend/internal/common/httpmw/origin_test.go new file mode 100644 index 0000000000..33db438fde --- /dev/null +++ b/apps/backend/internal/common/httpmw/origin_test.go @@ -0,0 +1,66 @@ +package httpmw + +import "testing" + +func TestAllowedOrigin(t *testing.T) { + tests := []struct { + name string + origin string + host string + want bool + }{ + // Loopback origin ↔ loopback host (dev servers, desktop shell) + {"localhost", "http://localhost", "localhost", true}, + {"localhost different ports", "http://localhost:3000", "localhost:8080", true}, + {"https localhost", "https://localhost", "localhost", true}, + {"127.0.0.1", "http://127.0.0.1", "127.0.0.1", true}, + {"127.0.0.1 different ports", "http://127.0.0.1:3000", "127.0.0.1:8080", true}, + {"localhost origin to 127.0.0.1 host", "http://localhost:3000", "127.0.0.1:8080", true}, + {"ipv6 loopback origin to ipv4 loopback host", "http://[::1]:3000", "127.0.0.1:8080", true}, + {"ipv6 loopback host", "http://localhost:3000", "[::1]:8080", true}, + + // Same hostname (ports ignored — see AllowedOrigin doc) + {"same origin", "https://example.com", "example.com", true}, + {"same origin with port", "https://example.com:443", "example.com:8080", true}, + {"same origin case insensitive", "https://Example.COM", "example.com", true}, + {"uppercase scheme", "HTTPS://example.com", "example.com", true}, + + // Cross-origin — reject + {"cross origin", "https://evil.com", "example.com", false}, + {"cross origin similar", "https://notexample.com", "example.com", false}, + {"ipv6 loopback origin to public host", "http://[::1]:3000", "example.com:8080", false}, + {"loopback origin to public host", "http://localhost:3000", "example.com", false}, + + // Loopback prefix-match bypass attempts — hostname must match exactly + {"localhost subdomain bypass", "http://localhost.attacker.tld", "localhost:8080", false}, + {"127.0.0.1 subdomain bypass", "http://127.0.0.1.attacker.tld:80", "127.0.0.1:8080", false}, + + // Malformed / non-http origins + {"empty origin", "", "example.com", false}, + {"malformed origin", "not-a-url", "example.com", false}, + {"null origin", "null", "localhost:8080", false}, + {"file scheme", "file:///etc/passwd", "localhost:8080", false}, + {"ws scheme", "ws://example.com", "example.com", false}, + + // Components that never appear in a browser Origin header — reject + {"userinfo", "https://user@example.com", "example.com", false}, + {"userinfo with password", "https://user:pass@example.com", "example.com", false}, + {"path", "https://example.com/path", "example.com", false}, + {"trailing slash", "https://example.com/", "example.com", false}, + {"query", "https://example.com?x=1", "example.com", false}, + {"empty query", "https://example.com?", "example.com", false}, + {"fragment", "https://example.com#frag", "example.com", false}, + {"opaque", "https:example.com", "example.com", false}, + + // Empty host — no match possible + {"empty request host", "https://example.com", "", false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := AllowedOrigin(tt.origin, tt.host); got != tt.want { + t.Errorf("AllowedOrigin(%q, %q) = %v, want %v", tt.origin, tt.host, got, tt.want) + } + }) + } +} diff --git a/apps/backend/internal/gateway/websocket/handler.go b/apps/backend/internal/gateway/websocket/handler.go index 37147882e0..11643835fb 100644 --- a/apps/backend/internal/gateway/websocket/handler.go +++ b/apps/backend/internal/gateway/websocket/handler.go @@ -2,7 +2,6 @@ package websocket import ( "context" - "net/http" "github.com/gin-gonic/gin" "github.com/google/uuid" @@ -16,10 +15,7 @@ import ( var upgrader = gorillaws.Upgrader{ ReadBufferSize: 1024, WriteBufferSize: 1024, - CheckOrigin: func(r *http.Request) bool { - // TODO: In production, validate origin - return true - }, + CheckOrigin: checkWebSocketOrigin, } // Handler handles WebSocket connections diff --git a/apps/backend/internal/gateway/websocket/handler_origin_test.go b/apps/backend/internal/gateway/websocket/handler_origin_test.go new file mode 100644 index 0000000000..c5b63e28c2 --- /dev/null +++ b/apps/backend/internal/gateway/websocket/handler_origin_test.go @@ -0,0 +1,117 @@ +package websocket + +import ( + "context" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "github.com/gin-gonic/gin" + gorillaws "github.com/gorilla/websocket" +) + +// startWSGateway serves the real /ws route backed by a running hub and returns +// the gateway plus the ws:// URL to dial. +func startWSGateway(t *testing.T) (*Gateway, string) { + t.Helper() + + g := NewGateway(newTestProxyLogger()) + + ctx, cancel := context.WithCancel(context.Background()) + hubDone := make(chan struct{}) + go func() { + defer close(hubDone) + g.Hub.Run(ctx) + }() + + gin.SetMode(gin.TestMode) + router := gin.New() + g.SetupRoutes(router) + srv := httptest.NewServer(router) + + t.Cleanup(func() { + srv.Close() + cancel() + <-hubDone + }) + + return g, "ws" + strings.TrimPrefix(srv.URL, "http") + "/ws" +} + +func dialWS(t *testing.T, wsURL, origin string) (*gorillaws.Conn, *http.Response, error) { + t.Helper() + + header := http.Header{} + if origin != "" { + header.Set("Origin", origin) + } + conn, resp, err := gorillaws.DefaultDialer.Dial(wsURL, header) + if resp != nil && resp.Body != nil { + t.Cleanup(func() { _ = resp.Body.Close() }) + } + return conn, resp, err +} + +// waitForNoClients blocks until every connection has unregistered from the hub +// so goleak sees clean read/write pumps at package teardown. +func waitForNoClients(t *testing.T, g *Gateway) { + t.Helper() + + deadline := time.Now().Add(2 * time.Second) + for g.Hub.GetClientCount() != 0 { + if time.Now().After(deadline) { + t.Fatalf("hub still has %d client(s)", g.Hub.GetClientCount()) + } + time.Sleep(5 * time.Millisecond) + } +} + +// TestHandleConnection_RejectsForeignOrigin is the CSWSH regression test: a +// browser page on an attacker-controlled origin must not be able to complete +// a WebSocket upgrade against the gateway. +func TestHandleConnection_RejectsForeignOrigin(t *testing.T) { + _, wsURL := startWSGateway(t) + + conn, resp, err := dialWS(t, wsURL, "https://attacker.example") + if err == nil { + _ = conn.Close() + t.Fatal("expected upgrade with foreign Origin to fail") + } + if resp == nil { + t.Fatalf("expected HTTP response on rejected upgrade, got err=%v", err) + } + if resp.StatusCode != http.StatusForbidden { + t.Fatalf("rejected upgrade status = %d, want %d", resp.StatusCode, http.StatusForbidden) + } +} + +// TestHandleConnection_AllowsTrustedOrigins verifies legitimate clients still +// connect: same-origin browsers, loopback dev servers on another port, and +// non-browser clients that send no Origin header. +func TestHandleConnection_AllowsTrustedOrigins(t *testing.T) { + g, wsURL := startWSGateway(t) + + sameOrigin := "http" + strings.TrimSuffix(strings.TrimPrefix(wsURL, "ws"), "/ws") + + for name, origin := range map[string]string{ + "no origin": "", + "same origin": sameOrigin, + "loopback cross port": "http://localhost:5173", + "loopback 127 with port": "http://127.0.0.1:12345", + } { + t.Run(name, func(t *testing.T) { + conn, resp, err := dialWS(t, wsURL, origin) + if err != nil { + status := 0 + if resp != nil { + status = resp.StatusCode + } + t.Fatalf("upgrade with origin %q failed (status %d): %v", origin, status, err) + } + _ = conn.Close() + waitForNoClients(t, g) + }) + } +} diff --git a/apps/backend/internal/gateway/websocket/origin.go b/apps/backend/internal/gateway/websocket/origin.go new file mode 100644 index 0000000000..409fd85cfc --- /dev/null +++ b/apps/backend/internal/gateway/websocket/origin.go @@ -0,0 +1,23 @@ +package websocket + +import ( + "net/http" + + "github.com/kandev/kandev/internal/common/httpmw" +) + +// checkWebSocketOrigin validates the Origin header on WebSocket upgrade +// requests to prevent cross-site WebSocket hijacking: without it, any web +// page the user visits could open a socket to the local backend and drive +// session/shell actions on the host. +// +// Requests without an Origin header are allowed (non-browser clients such as +// the CLI or curl); everything else defers to the shared origin trust policy +// in httpmw.AllowedOrigin, the same policy the CORS middleware enforces. +func checkWebSocketOrigin(r *http.Request) bool { + origin := r.Header.Get("Origin") + if origin == "" { + return true + } + return httpmw.AllowedOrigin(origin, r.Host) +} diff --git a/apps/backend/internal/gateway/websocket/origin_test.go b/apps/backend/internal/gateway/websocket/origin_test.go new file mode 100644 index 0000000000..76103f7cfe --- /dev/null +++ b/apps/backend/internal/gateway/websocket/origin_test.go @@ -0,0 +1,40 @@ +package websocket + +import ( + "net/http" + "testing" +) + +// The exhaustive origin policy table lives in internal/common/httpmw; this +// covers the upgrade-specific wrapper semantics. +func TestCheckWebSocketOrigin(t *testing.T) { + tests := []struct { + name string + origin string + host string + want bool + }{ + {"no origin allows non-browser clients", "", "example.com", true}, + {"same hostname", "https://example.com", "example.com:8080", true}, + {"loopback cross port", "http://localhost:3000", "127.0.0.1:8080", true}, + {"foreign origin", "https://evil.com", "example.com", false}, + {"malformed origin", "not-a-url", "example.com", false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + r := &http.Request{ + Header: http.Header{}, + Host: tt.host, + } + if tt.origin != "" { + r.Header.Set("Origin", tt.origin) + } + + if got := checkWebSocketOrigin(r); got != tt.want { + t.Errorf("checkWebSocketOrigin(origin=%q, host=%q) = %v, want %v", + tt.origin, tt.host, got, tt.want) + } + }) + } +} diff --git a/apps/backend/internal/gateway/websocket/terminal_handler.go b/apps/backend/internal/gateway/websocket/terminal_handler.go index c0d61923ac..bf1f7e80a9 100644 --- a/apps/backend/internal/gateway/websocket/terminal_handler.go +++ b/apps/backend/internal/gateway/websocket/terminal_handler.go @@ -6,7 +6,6 @@ import ( "errors" "fmt" "net/http" - "net/url" "strings" "github.com/gin-gonic/gin" @@ -51,48 +50,6 @@ var terminalUpgrader = gorillaws.Upgrader{ CheckOrigin: checkWebSocketOrigin, } -// checkWebSocketOrigin validates the Origin header for WebSocket connections. -// This prevents cross-site WebSocket hijacking attacks. -func checkWebSocketOrigin(r *http.Request) bool { - origin := r.Header.Get("Origin") - if origin == "" { - // No origin header - allow (could be a non-browser client) - return true - } - - // Allow localhost origins for development - if strings.HasPrefix(origin, "http://localhost") || - strings.HasPrefix(origin, "http://127.0.0.1") || - strings.HasPrefix(origin, "https://localhost") || - strings.HasPrefix(origin, "https://127.0.0.1") { - return true - } - - // Check same-origin: Origin should match the Host header - host := r.Host - if host == "" { - host = r.URL.Host - } - - // Parse the origin URL to get its host - originURL, err := url.Parse(origin) - if err != nil { - return false - } - - // Compare hosts (ignoring port for flexibility) - originHost := originURL.Hostname() - requestHost := host - if colonIdx := strings.LastIndex(requestHost, ":"); colonIdx != -1 { - // Strip port from host if present (but be careful with IPv6) - if !strings.Contains(requestHost, "]") || colonIdx > strings.Index(requestHost, "]") { - requestHost = requestHost[:colonIdx] - } - } - - return originHost == requestHost -} - type terminalRoute struct { kind string id string diff --git a/apps/backend/internal/gateway/websocket/terminal_handler_test.go b/apps/backend/internal/gateway/websocket/terminal_handler_test.go index 8bc9756c78..0ae1c92bdd 100644 --- a/apps/backend/internal/gateway/websocket/terminal_handler_test.go +++ b/apps/backend/internal/gateway/websocket/terminal_handler_test.go @@ -19,62 +19,6 @@ import ( "github.com/kandev/kandev/internal/events/bus" ) -func TestCheckWebSocketOrigin(t *testing.T) { - tests := []struct { - name string - origin string - host string - want bool - }{ - // No origin — allow (non-browser client) - {"no origin", "", "example.com", true}, - - // Localhost variants - {"http://localhost", "http://localhost", "localhost", true}, - {"http://localhost:3000", "http://localhost:3000", "localhost:8080", true}, - {"https://localhost", "https://localhost", "localhost", true}, - {"http://127.0.0.1", "http://127.0.0.1", "127.0.0.1", true}, - {"http://127.0.0.1:3000", "http://127.0.0.1:3000", "127.0.0.1:8080", true}, - {"https://127.0.0.1", "https://127.0.0.1", "127.0.0.1", true}, - - // Same-origin (origin host matches request host) - {"same origin", "https://example.com", "example.com", true}, - {"same origin with port", "https://example.com:443", "example.com:8080", true}, - - // Cross-origin — reject - {"cross origin", "https://evil.com", "example.com", false}, - {"cross origin similar", "https://notexample.com", "example.com", false}, - - // Malformed origin - {"malformed origin", "not-a-url", "example.com", false}, - - // IPv6 host - {"ipv6 cross origin", "http://[::1]:3000", "example.com:8080", false}, - - // Empty host — no match possible - {"empty host rejects", "https://example.com", "", false}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - r := &http.Request{ - Header: http.Header{}, - Host: tt.host, - URL: &url.URL{Host: tt.host}, - } - if tt.origin != "" { - r.Header.Set("Origin", tt.origin) - } - - got := checkWebSocketOrigin(r) - if got != tt.want { - t.Errorf("checkWebSocketOrigin(origin=%q, host=%q) = %v, want %v", - tt.origin, tt.host, got, tt.want) - } - }) - } -} - func TestStripTerminalResponses(t *testing.T) { tests := []struct { name string