server: reject cross-origin WebSocket handshakes when CORS is empty.

With an empty server.cors, AllowOrigin accepted every Origin, so a hostile
page could open an authenticated WebSocket using the SameSite=None session
cookie. Now only same-origin, listed origins, or '*' are accepted.

Discovered by Yves Soete of Blacksight LLC.

server/internal/config/server.go

@@ -62,7 +62,7 @@ func (Server) Init(cmd *cobra.Command) error {
 		return err
 	}
 
-	cmd.PersistentFlags().StringSlice("server.cors", []string{}, "list of allowed origins for CORS, if empty CORS is disabled, if '*' is present all origins are allowed")
+	cmd.PersistentFlags().StringSlice("server.cors", []string{}, "list of allowed origins for CORS and WebSocket connections, if empty only same-origin WebSocket connections are accepted, if '*' is present all origins are allowed")
 	if err := viper.BindPFlag("server.cors", cmd.PersistentFlags().Lookup("server.cors")); err != nil {
 		return err
 	}
@@ -193,9 +193,8 @@ func (s *Server) HasCors() bool {
 }
 
 func (s *Server) AllowOrigin(origin string) bool {
-	// if CORS is not configured, or a wildcard is present,
-	// allow WebSocket connections from any origin
-	if len(s.CORS) == 0 || s.CORS[0] == "*" {
+	// only an explicit wildcard allows any origin
+	if len(s.CORS) > 0 && s.CORS[0] == "*" {
 		return true
 	}
 

server/internal/http/manager.go

@@ -3,8 +3,10 @@ package http
 import (
 	"context"
 	"net/http"
+	"net/url"
 	"os"
 	"path"
+	"strings"
 
 	"github.com/prometheus/client_golang/prometheus/promhttp"
 	"github.com/rs/zerolog"
@@ -53,7 +55,13 @@ func New(WebSocketManager types.WebSocketManager, ApiManager types.ApiManager, c
 
 	router.Get("/api/ws", WebSocketManager.Upgrade(func(r *http.Request) bool {
 		origin := r.Header.Get("Origin")
-		return origin == "" || config.AllowOrigin(origin)
+		if origin == "" || config.AllowOrigin(origin) {
+			return true
+		}
+
+		// by default, only same-origin browser connections are accepted
+		u, err := url.Parse(origin)
+		return err == nil && u.Host != "" && strings.EqualFold(u.Host, r.Host)
 	}))
 
 	batch := batchHandler{

server/internal/http/manager_test.go

@@ -109,3 +109,24 @@ func TestWebSocketOriginAllowsInternalDialWithoutOrigin(t *testing.T) {
 		t.Fatal("unconfigured browser Origin was accepted")
 	}
 }
+
+func TestWebSocketOriginDefaultsToSameOrigin(t *testing.T) {
+	webSocketManager := &originCapturingWebSocketManager{}
+	serverConfig := &config.Server{}
+	memberConfig := &config.Member{}
+	sessionManager := session.New(&config.Session{})
+	apiManager := api.New(sessionManager, member.New(sessionManager, memberConfig), nil, nil, memberConfig, serverConfig)
+	New(webSocketManager, apiManager, serverConfig, memberConfig)
+
+	request := httptest.NewRequest(http.MethodGet, "http://neko.example.test/api/ws", nil)
+
+	request.Header.Set("Origin", "http://neko.example.test")
+	if !webSocketManager.checkOrigin(request) {
+		t.Fatal("same Origin was rejected")
+	}
+
+	request.Header.Set("Origin", "https://attacker.example")
+	if webSocketManager.checkOrigin(request) {
+		t.Fatal("cross-site Origin was accepted with empty CORS")
+	}
+}