Merge pull request #59 from hobbyistlabs-coder/sentinel/fix-pico-websocket-auth-timing-attack-6445880057624673878

🛡️ Sentinel: [HIGH] Fix timing attack vulnerability in Pico WebSocket auth

.jules/sentinel.md

@@ -7,3 +7,8 @@
 **Vulnerability:** Go's standard `http.Post`, `http.PostForm`, `http.Get`, and the default `http.Client` do not have timeouts configured by default.
 **Learning:** These defaults can leave the application vulnerable to resource exhaustion or indefinite hangs if the external service (like an OAuth provider) is slow, unresponsive, or experiencing an outage.
 **Prevention:** Always instantiate `http.Client` explicitly with a sensible `Timeout` (e.g., `Timeout: 15 * time.Second`) before making outbound HTTP requests, instead of using the default package-level convenience functions.
+
+## 2025-03-19 - [HIGH] Fix Timing Attack Vulnerability in Pico WebSocket Auth
+**Vulnerability:** The Pico WebSocket authentication handler was using standard string equality (`==`) to compare the provided bearer token against the configured secret token.
+**Learning:** String equality operators in Go return early as soon as a character mismatch is found. This allows an attacker to measure the time it takes for the server to reject the connection and iteratively guess the token character by character (a timing attack).
+**Prevention:** Always use `subtle.ConstantTimeCompare` from the `crypto/subtle` package when comparing secrets, tokens, passwords, or cryptographic signatures to ensure the comparison time depends only on the length of the secret, not the contents.

pkg/channels/pico/pico.go

@@ -2,6 +2,7 @@ package pico
 
 import (
 	"context"
+	"crypto/subtle"
 	"encoding/json"
 	"fmt"
 	"net/http"
@@ -293,14 +294,14 @@ func (c *PicoChannel) authenticate(r *http.Request) bool {
 	// Check Authorization header
 	auth := r.Header.Get("Authorization")
 	if after, ok := strings.CutPrefix(auth, "Bearer "); ok {
-		if after == token {
+		if subtle.ConstantTimeCompare([]byte(after), []byte(token)) == 1 {
 			return true
 		}
 	}
 
 	// Check query parameter only when explicitly allowed
 	if c.config.AllowTokenQuery {
-		if r.URL.Query().Get("token") == token {
+		if subtle.ConstantTimeCompare([]byte(r.URL.Query().Get("token")), []byte(token)) == 1 {
 			return true
 		}
 	}

pkg/channels/pico/pico_test.go

@@ -0,0 +1,107 @@
+package pico
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"jane/pkg/config"
+)
+
+func TestPicoChannel_Authenticate(t *testing.T) {
+	tests := []struct {
+		name            string
+		token           string
+		allowTokenQuery bool
+		setupRequest    func(*http.Request)
+		want            bool
+	}{
+		{
+			name:            "empty token config",
+			token:           "",
+			allowTokenQuery: false,
+			setupRequest: func(r *http.Request) {
+				r.Header.Set("Authorization", "Bearer valid-token")
+			},
+			want: false,
+		},
+		{
+			name:            "valid authorization header",
+			token:           "valid-token",
+			allowTokenQuery: false,
+			setupRequest: func(r *http.Request) {
+				r.Header.Set("Authorization", "Bearer valid-token")
+			},
+			want: true,
+		},
+		{
+			name:            "invalid authorization header",
+			token:           "valid-token",
+			allowTokenQuery: false,
+			setupRequest: func(r *http.Request) {
+				r.Header.Set("Authorization", "Bearer invalid-token")
+			},
+			want: false,
+		},
+		{
+			name:            "missing authorization header prefix",
+			token:           "valid-token",
+			allowTokenQuery: false,
+			setupRequest: func(r *http.Request) {
+				r.Header.Set("Authorization", "valid-token")
+			},
+			want: false,
+		},
+		{
+			name:            "valid query parameter token - allowed",
+			token:           "valid-token",
+			allowTokenQuery: true,
+			setupRequest: func(r *http.Request) {
+				q := r.URL.Query()
+				q.Add("token", "valid-token")
+				r.URL.RawQuery = q.Encode()
+			},
+			want: true,
+		},
+		{
+			name:            "invalid query parameter token - allowed",
+			token:           "valid-token",
+			allowTokenQuery: true,
+			setupRequest: func(r *http.Request) {
+				q := r.URL.Query()
+				q.Add("token", "invalid-token")
+				r.URL.RawQuery = q.Encode()
+			},
+			want: false,
+		},
+		{
+			name:            "valid query parameter token - not allowed",
+			token:           "valid-token",
+			allowTokenQuery: false,
+			setupRequest: func(r *http.Request) {
+				q := r.URL.Query()
+				q.Add("token", "valid-token")
+				r.URL.RawQuery = q.Encode()
+			},
+			want: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			c := &PicoChannel{
+				config: config.PicoConfig{
+					Token:           tt.token,
+					AllowTokenQuery: tt.allowTokenQuery,
+				},
+			}
+
+			req := httptest.NewRequest(http.MethodGet, "/ws", nil)
+			tt.setupRequest(req)
+
+			if got := c.authenticate(req); got != tt.want {
+				t.Errorf("authenticate() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}