mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-09-10 02:25:56 +00:00
Require CSRF token regardless of Authorization header
CheckCSRF was skipping the CSRF check whenever the request carried Authorization: Bearer, Authorization: Basic, or X-API-Token, without validating the credential. An attacker on a cross-origin page could fetch() any state-changing endpoint with credentials: 'include' plus an arbitrary Authorization header — the browser would auto-attach the victim's pulse_session cookie, the server would skip CSRF, and the request would execute as the logged-in user. Full CSRF bypass for every session-authenticated user. CSRF protection exists because the browser auto-attaches the session cookie. That cookie is the only auto-attached credential we issue, so it is the only correct signal for whether CSRF applies. Header-based auth is set explicitly per request and is not CSRF-vulnerable, but its presence does not make a session-cookie-bearing request safe. Skip CSRF only when no session cookie is present; otherwise require the token regardless of any Authorization or X-API-Token header. Tests: the three "header bypasses CSRF" cases in security_test.go were passing only because they sent no session cookie (so the no-cookie path returned true). Renamed those to make the no-cookie precondition explicit. Added TestCheckCSRF_HeaderDoesNotBypassWhenSessionCookiePresent in security_regression_test.go covering X-API-Token, Authorization: Basic, Authorization: Bearer, and mixed-case Bearer with a session cookie present — each must require a valid CSRF token.
This commit is contained in:
+12
-21
@@ -59,30 +59,21 @@ func CheckCSRF(w http.ResponseWriter, r *http.Request) bool {
|
||||
return true
|
||||
}
|
||||
|
||||
// Skip CSRF for API token auth (API clients don't have sessions)
|
||||
if r.Header.Get("X-API-Token") != "" {
|
||||
log.Debug().Str("path", r.URL.Path).Msg("CSRF check skipped: API token auth")
|
||||
return true
|
||||
}
|
||||
// CSRF is a defence against browser-auto-attached credentials. The only
|
||||
// auto-attached credential we issue is the session cookie, so the session
|
||||
// cookie is the ONLY signal for whether CSRF applies. Explicit per-request
|
||||
// credentials (X-API-Token, Authorization: Bearer, Authorization: Basic)
|
||||
// are NOT a skip-signal: an attacker on a cross-origin page can attach
|
||||
// those headers after a CORS preflight while the browser still
|
||||
// auto-attaches the victim's session cookie, fully bypassing CSRF. The
|
||||
// previous behaviour — early-returning on any Authorization header — is a
|
||||
// full CSRF bypass for session-authenticated users and must not return.
|
||||
|
||||
// Skip CSRF only for explicit non-session auth schemes.
|
||||
if authHeader := strings.TrimSpace(r.Header.Get("Authorization")); authHeader != "" {
|
||||
lower := strings.ToLower(authHeader)
|
||||
if strings.HasPrefix(lower, "basic ") {
|
||||
log.Debug().Str("path", r.URL.Path).Msg("CSRF check skipped: Basic auth header present")
|
||||
return true
|
||||
}
|
||||
if strings.HasPrefix(lower, "bearer ") {
|
||||
log.Debug().Str("path", r.URL.Path).Msg("CSRF check skipped: Bearer auth header present")
|
||||
return true
|
||||
}
|
||||
}
|
||||
|
||||
// Get session from cookie
|
||||
// Get session from cookie. No cookie => purely header-authenticated (or
|
||||
// unauthenticated) request that downstream auth will accept or reject on
|
||||
// its own merits; CSRF does not apply.
|
||||
cookie, err := readSessionCookie(r)
|
||||
if err != nil {
|
||||
// No session cookie means no CSRF check needed
|
||||
// (either no auth configured or using basic auth which doesn't use sessions)
|
||||
log.Debug().Str("path", r.URL.Path).Msg("CSRF check skipped: no session cookie")
|
||||
return true
|
||||
}
|
||||
|
||||
@@ -4603,3 +4603,68 @@ func TestSSHConfigRejectsSetupTokenQueryParam(t *testing.T) {
|
||||
t.Fatalf("expected 401 when setup token is only provided in query string, got %d", rec.Code)
|
||||
}
|
||||
}
|
||||
|
||||
// TestCheckCSRF_HeaderDoesNotBypassWhenSessionCookiePresent regression-tests the
|
||||
// fix for a CSRF bypass: CheckCSRF was skipping the entire CSRF check whenever
|
||||
// the request carried Authorization: Basic, Authorization: Bearer, or
|
||||
// X-API-Token, without validating the credential. A cross-origin attacker
|
||||
// could fetch() with credentials: 'include' and an arbitrary Authorization
|
||||
// header — the browser would still auto-attach the victim's session cookie
|
||||
// and the server would skip CSRF, fully bypassing protection. The contract is
|
||||
// now: a session cookie is the only signal for whether CSRF applies. If a
|
||||
// session cookie is present, CSRF must be valid regardless of any other
|
||||
// auth-style header.
|
||||
func TestCheckCSRF_HeaderDoesNotBypassWhenSessionCookiePresent(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
setHeader func(*http.Request)
|
||||
description string
|
||||
}{
|
||||
{
|
||||
name: "x_api_token",
|
||||
setHeader: func(r *http.Request) {
|
||||
r.Header.Set("X-API-Token", "some-api-token")
|
||||
},
|
||||
description: "X-API-Token must not bypass CSRF when a session cookie is present",
|
||||
},
|
||||
{
|
||||
name: "authorization_basic",
|
||||
setHeader: func(r *http.Request) {
|
||||
r.Header.Set("Authorization", "Basic dXNlcjpwYXNz")
|
||||
},
|
||||
description: "Authorization: Basic must not bypass CSRF when a session cookie is present",
|
||||
},
|
||||
{
|
||||
name: "authorization_bearer",
|
||||
setHeader: func(r *http.Request) {
|
||||
r.Header.Set("Authorization", "Bearer some-token")
|
||||
},
|
||||
description: "Authorization: Bearer must not bypass CSRF when a session cookie is present",
|
||||
},
|
||||
{
|
||||
name: "authorization_bearer_mixed_case",
|
||||
setHeader: func(r *http.Request) {
|
||||
// Mixed-case scheme to ensure the old lower-case prefix check
|
||||
// is not reintroduced as a guarded skip.
|
||||
r.Header.Set("Authorization", "BeArEr some-token")
|
||||
},
|
||||
description: "Mixed-case Bearer must not bypass CSRF when a session cookie is present",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
w := httptest.NewRecorder()
|
||||
req := httptest.NewRequest("POST", "/api/test", nil)
|
||||
tc.setHeader(req)
|
||||
req.AddCookie(&http.Cookie{
|
||||
Name: "pulse_session",
|
||||
Value: "test-session-id-1234567890",
|
||||
})
|
||||
|
||||
if CheckCSRF(w, req) {
|
||||
t.Fatal(tc.description)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1369,39 +1369,41 @@ func TestCheckCSRF_SafeMethods(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestCheckCSRF_APITokenAuth(t *testing.T) {
|
||||
func TestCheckCSRF_APITokenAuth_NoSessionCookie(t *testing.T) {
|
||||
w := httptest.NewRecorder()
|
||||
req := httptest.NewRequest("POST", "/api/test", nil)
|
||||
req.Header.Set("X-API-Token", "some-api-token")
|
||||
|
||||
// API token auth bypasses CSRF check
|
||||
// No session cookie => no CSRF check needed (the API client is purely
|
||||
// header-authenticated). Regression coverage for the header+cookie case
|
||||
// lives in security_regression_test.go.
|
||||
result := CheckCSRF(w, req)
|
||||
if !result {
|
||||
t.Error("CheckCSRF should return true when X-API-Token is present")
|
||||
t.Error("CheckCSRF should return true when X-API-Token is present and no session cookie")
|
||||
}
|
||||
}
|
||||
|
||||
func TestCheckCSRF_BasicAuth(t *testing.T) {
|
||||
func TestCheckCSRF_BasicAuth_NoSessionCookie(t *testing.T) {
|
||||
w := httptest.NewRecorder()
|
||||
req := httptest.NewRequest("POST", "/api/test", nil)
|
||||
req.Header.Set("Authorization", "Basic dXNlcjpwYXNz")
|
||||
|
||||
// Basic auth bypasses CSRF check
|
||||
// No session cookie => no CSRF check needed.
|
||||
result := CheckCSRF(w, req)
|
||||
if !result {
|
||||
t.Error("CheckCSRF should return true when Basic Authorization header is present")
|
||||
t.Error("CheckCSRF should return true when Basic Authorization header is present and no session cookie")
|
||||
}
|
||||
}
|
||||
|
||||
func TestCheckCSRF_BearerAuth(t *testing.T) {
|
||||
func TestCheckCSRF_BearerAuth_NoSessionCookie(t *testing.T) {
|
||||
w := httptest.NewRecorder()
|
||||
req := httptest.NewRequest("POST", "/api/test", nil)
|
||||
req.Header.Set("Authorization", "Bearer some-token")
|
||||
|
||||
// Bearer auth bypasses CSRF check for token-based API clients.
|
||||
// No session cookie => no CSRF check needed.
|
||||
result := CheckCSRF(w, req)
|
||||
if !result {
|
||||
t.Error("CheckCSRF should return true when Bearer Authorization header is present")
|
||||
t.Error("CheckCSRF should return true when Bearer Authorization header is present and no session cookie")
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user