-
Notifications
You must be signed in to change notification settings - Fork 0
π Restrict WebSocket Origins to Mitigate CSWSH #15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,120 @@ | ||
| package server | ||
|
|
||
| import ( | ||
| "context" | ||
| "net/http" | ||
| "net/http/httptest" | ||
| "testing" | ||
| "time" | ||
|
|
||
| "github.com/coder/websocket" | ||
| "github.com/adcondev/poster/pkg/connection" | ||
| "github.com/adcondev/ticket-daemon/internal/posprinter" | ||
| ) | ||
|
|
||
| type mockPrinterDiscovery struct{} | ||
|
|
||
| func (m *mockPrinterDiscovery) GetPrinters(_ bool) ([]connection.PrinterDetail, error) { | ||
| return []connection.PrinterDetail{}, nil | ||
| } | ||
|
|
||
| func (m *mockPrinterDiscovery) GetSummary() posprinter.Summary { | ||
| return posprinter.Summary{} | ||
| } | ||
|
|
||
| func TestWebSocketOrigin(t *testing.T) { | ||
| // 1. Test Restricted Origin (Default behavior / Explicit Allow) | ||
| t.Run("Restricted Origin", func(t *testing.T) { | ||
| // Create server with specific allowed origin | ||
| cfg := Config{ | ||
| QueueSize: 10, | ||
| AllowedOrigins: []string{"http://good.com"}, | ||
| } | ||
| discovery := &mockPrinterDiscovery{} | ||
| srv := NewServer(cfg, discovery) | ||
| defer srv.Shutdown() | ||
|
|
||
| ts := httptest.NewServer(http.HandlerFunc(srv.HandleWebSocket)) | ||
| defer ts.Close() | ||
|
|
||
| u := "ws" + ts.URL[4:] | ||
|
|
||
| // Case A: Connection from Allowed Origin | ||
| ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) | ||
| defer cancel() | ||
|
|
||
| opts := &websocket.DialOptions{ | ||
| HTTPHeader: http.Header{ | ||
| "Origin": []string{"http://good.com"}, | ||
| }, | ||
| } | ||
|
|
||
| conn, resp, err := websocket.Dial(ctx, u, opts) | ||
| if resp != nil && resp.Body != nil { | ||
| _ = resp.Body.Close() | ||
| } | ||
| if err != nil { | ||
| t.Fatalf("Connection from good.com failed: %v", err) | ||
| } | ||
| _ = conn.Close(websocket.StatusNormalClosure, "") | ||
|
|
||
| // Case B: Connection from Disallowed Origin | ||
| optsBad := &websocket.DialOptions{ | ||
| HTTPHeader: http.Header{ | ||
| "Origin": []string{"http://evil.com"}, | ||
| }, | ||
| } | ||
|
|
||
| _, respBad, err := websocket.Dial(ctx, u, optsBad) | ||
| if respBad != nil && respBad.Body != nil { | ||
| _ = respBad.Body.Close() | ||
| } | ||
| if err == nil { | ||
| t.Fatalf("Connection from evil.com succeeded (should fail)") | ||
| } | ||
|
Comment on lines
+42
to
+74
|
||
| }) | ||
|
|
||
| // 2. Test Same Origin Enforcement (When AllowedOrigins is empty/nil) | ||
| t.Run("Same Origin Enforcement", func(t *testing.T) { | ||
| cfg := Config{ | ||
| QueueSize: 10, | ||
| AllowedOrigins: nil, // Enforce same origin | ||
| } | ||
| discovery := &mockPrinterDiscovery{} | ||
| srv := NewServer(cfg, discovery) | ||
| defer srv.Shutdown() | ||
|
|
||
| ts := httptest.NewServer(http.HandlerFunc(srv.HandleWebSocket)) | ||
| defer ts.Close() | ||
|
|
||
| u := "ws" + ts.URL[4:] | ||
|
|
||
| // Case A: Connection from Same Origin (Default behavior of Dial) | ||
| ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) | ||
| defer cancel() | ||
|
|
||
| // websocket.Dial sets Origin to the URL's host by default, mimicking a same-origin request | ||
| conn, resp, err := websocket.Dial(ctx, u, nil) | ||
| if resp != nil && resp.Body != nil { | ||
| _ = resp.Body.Close() | ||
| } | ||
| if err != nil { | ||
| t.Fatalf("Connection from same origin failed: %v", err) | ||
| } | ||
| _ = conn.Close(websocket.StatusNormalClosure, "") | ||
|
|
||
| // Case B: Connection from Different Origin | ||
| optsBad := &websocket.DialOptions{ | ||
| HTTPHeader: http.Header{ | ||
| "Origin": []string{"http://external-site.com"}, | ||
| }, | ||
| } | ||
| _, respBad, err := websocket.Dial(ctx, u, optsBad) | ||
| if respBad != nil && respBad.Body != nil { | ||
| _ = respBad.Body.Close() | ||
| } | ||
| if err == nil { | ||
| t.Fatalf("Connection from external-site.com succeeded (should fail)") | ||
| } | ||
| }) | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Parsing AllowedOrigins from the ldflag with strings.Split leaves leading/trailing whitespace intact and can introduce empty entries (e.g., trailing comma). This can make valid origins fail to match unexpectedly and makes configuration fragile. Consider splitting, trimming spaces on each entry, and filtering out empty strings before assigning cfg.AllowedOrigins (optionally validate patterns and log/ignore invalid ones).