fix(csrf): block unsafe cookie-auth requests with no origin signals [R8S-1099] (#3153)

api/http/csrf/csrf.go

@@ -5,6 +5,8 @@ import (
 	"net/http"
 	"os"
 
+	portainer "github.com/portainer/portainer/api"
+
 	"github.com/rs/zerolog/log"
 )
 
@@ -15,6 +17,11 @@ func WithProtect(handler http.Handler, trustedOrigins []string) (http.Handler, e
 		isDockerDesktopExtension = true
 	}
 
+	allowNoOrigin := false
+	if val, ok := os.LookupEnv(portainer.CSRFAllowNoOriginEnvVar); ok && val == "1" {
+		allowNoOrigin = true
+	}
+
 	cop := http.NewCrossOriginProtection()
 	for _, origin := range trustedOrigins {
 		if err := cop.AddTrustedOrigin(origin); err != nil {
@@ -43,6 +50,45 @@ func WithProtect(handler http.Handler, trustedOrigins []string) (http.Handler, e
 			return
 		}
 
+		if !allowNoOrigin && isUnsafeCookieRequestWithoutOrigin(r) {
+			log.Error().
+				Str("request_url", r.URL.String()).
+				Str("host", r.Host).
+				Str("method", r.Method).
+				Msg("CSRF check failed: unsafe cookie-authenticated request without Origin or Sec-Fetch-Site headers, set " + portainer.CSRFAllowNoOriginEnvVar + "=1 to allow such requests")
+
+			http.Error(w, http.StatusText(http.StatusForbidden), http.StatusForbidden)
+
+			return
+		}
+
 		protected.ServeHTTP(w, r)
 	}), nil
 }
+
+// isUnsafeCookieRequestWithoutOrigin reports whether the request uses an unsafe
+// method, relies on the session cookie rather than an API key or bearer token,
+// and carries no Origin or Sec-Fetch-Site header. CrossOriginProtection assumes
+// such requests come from non-browser clients and allows them, but legacy
+// browsers and header-stripping proxies can produce them too, so they are
+// blocked instead (fail closed).
+func isUnsafeCookieRequestWithoutOrigin(r *http.Request) bool {
+	switch r.Method {
+	case http.MethodGet, http.MethodHead, http.MethodOptions:
+		return false
+	}
+
+	if r.Header.Get("Origin") != "" || r.Header.Get("Sec-Fetch-Site") != "" {
+		return false
+	}
+
+	// Token-authenticated clients set these headers explicitly, which a browser
+	// never does on its own, so they are not exposed to CSRF.
+	if r.Header.Get(portainer.APIKeyHeader) != "" || r.Header.Get("Authorization") != "" {
+		return false
+	}
+
+	_, err := r.Cookie(portainer.AuthCookieKey)
+
+	return err == nil
+}

api/http/csrf/csrf_test.go

@@ -37,7 +37,7 @@ func TestWithProtect_safeMethodsAlwaysAllowed(t *testing.T) {
 	}
 }
 
-func TestWithProtect_allowsPostWithNoOriginHeaders(t *testing.T) {
+func TestWithProtect_allowsPostWithNoOriginHeadersAndNoAuthCookie(t *testing.T) {
 	t.Parallel()
 
 	handler, err := WithProtect(okHandler, nil)
@@ -50,6 +50,80 @@ func TestWithProtect_allowsPostWithNoOriginHeaders(t *testing.T) {
 	require.Equal(t, http.StatusOK, rr.Code)
 }
 
+func TestWithProtect_blocksUnsafeCookieRequestWithNoOriginHeaders(t *testing.T) {
+	t.Parallel()
+
+	handler, err := WithProtect(okHandler, nil)
+	require.NoError(t, err)
+
+	for _, method := range []string{http.MethodPost, http.MethodPut, http.MethodPatch, http.MethodDelete} {
+		req := httptest.NewRequest(method, "/", nil)
+		req.AddCookie(&http.Cookie{Name: portainer.AuthCookieKey, Value: "some-token"})
+
+		rr := httptest.NewRecorder()
+		handler.ServeHTTP(rr, req)
+		require.Equal(t, http.StatusForbidden, rr.Code, "method %s with auth cookie and no origin headers should be blocked", method)
+	}
+}
+
+func TestWithProtect_allowsPostWithNonAuthCookieAndNoOriginHeaders(t *testing.T) {
+	t.Parallel()
+
+	handler, err := WithProtect(okHandler, nil)
+	require.NoError(t, err)
+
+	req := httptest.NewRequest(http.MethodPost, "/", nil)
+	req.AddCookie(&http.Cookie{Name: "some-other-cookie", Value: "some-value"})
+
+	rr := httptest.NewRecorder()
+	handler.ServeHTTP(rr, req)
+	require.Equal(t, http.StatusOK, rr.Code)
+}
+
+func TestWithProtect_allowsAPIKeyRequestWithCookieAndNoOriginHeaders(t *testing.T) {
+	t.Parallel()
+
+	handler, err := WithProtect(okHandler, nil)
+	require.NoError(t, err)
+
+	req := httptest.NewRequest(http.MethodPost, "/", nil)
+	req.AddCookie(&http.Cookie{Name: portainer.AuthCookieKey, Value: "some-token"})
+	req.Header.Set(portainer.APIKeyHeader, "some-api-key")
+
+	rr := httptest.NewRecorder()
+	handler.ServeHTTP(rr, req)
+	require.Equal(t, http.StatusOK, rr.Code)
+}
+
+func TestWithProtect_allowsBearerRequestWithCookieAndNoOriginHeaders(t *testing.T) {
+	t.Parallel()
+
+	handler, err := WithProtect(okHandler, nil)
+	require.NoError(t, err)
+
+	req := httptest.NewRequest(http.MethodPost, "/", nil)
+	req.AddCookie(&http.Cookie{Name: portainer.AuthCookieKey, Value: "some-token"})
+	req.Header.Set("Authorization", "Bearer some-token")
+
+	rr := httptest.NewRecorder()
+	handler.ServeHTTP(rr, req)
+	require.Equal(t, http.StatusOK, rr.Code)
+}
+
+func TestWithProtect_allowNoOriginEnvVarRestoresFailOpen(t *testing.T) {
+	t.Setenv(portainer.CSRFAllowNoOriginEnvVar, "1")
+
+	handler, err := WithProtect(okHandler, nil)
+	require.NoError(t, err)
+
+	req := httptest.NewRequest(http.MethodPost, "/", nil)
+	req.AddCookie(&http.Cookie{Name: portainer.AuthCookieKey, Value: "some-token"})
+
+	rr := httptest.NewRecorder()
+	handler.ServeHTTP(rr, req)
+	require.Equal(t, http.StatusOK, rr.Code)
+}
+
 func TestWithProtect_allowsPostWithSameOriginSecFetchSite(t *testing.T) {
 	t.Parallel()
 

api/http/security/bouncer.go

@@ -20,7 +20,7 @@ import (
 	"github.com/rs/zerolog/log"
 )
 
-const apiKeyHeader = "X-API-KEY"
+const apiKeyHeader = portainer.APIKeyHeader
 const jwtTokenHeader = "Authorization"
 
 type (

api/portainer.go

@@ -2120,6 +2120,8 @@ const (
 	AuthCookieKey = "portainer_api_key"
 	// PortainerCacheHeader is used to enabled FE caching for Kubernetes resources
 	PortainerCacheHeader = "X-Portainer-Cache"
+	// APIKeyHeader is the name of the header used for API key authentication
+	APIKeyHeader = "X-API-KEY"
 	// KubectlShellImageEnvVar is the environment variable used to override the default kubectl shell image
 	KubectlShellImageEnvVar = "KUBECTL_SHELL_IMAGE"
 	// PullLimitCheckDisabledEnvVar is the environment variable used to disable the pull limit check
@@ -2141,6 +2143,8 @@ const (
 	NoSetupTokenEnvVar = "PORTAINER_NO_SETUP_TOKEN"
 	// SetupTokenEnvVar is the environment variable used to provide a custom setup token for admin initialization and restore on an uninitialized instance
 	SetupTokenEnvVar = "PORTAINER_SETUP_TOKEN"
+	// CSRFAllowNoOriginEnvVar is the environment variable used to allow unsafe cookie-authenticated requests that carry no Origin or Sec-Fetch-Site header, reverting the CSRF protection to fail open for such requests
+	CSRFAllowNoOriginEnvVar = "CSRF_ALLOW_NO_ORIGIN"
 )
 
 // List of supported features