From c2efd4da4df993cc43300484969a5e2bb0c88ae7 Mon Sep 17 00:00:00 2001 From: Josh Hunt Date: Wed, 28 Jan 2026 12:12:27 +0000 Subject: [PATCH] FS: CSP Middleware (#116819) * Extract CSP header logic into a seperate middleware * Add CSP tests * Improve logging in csp middleware * Set default CSP in docker compose --- devenv/frontend-service/docker-compose.yaml | 3 +- pkg/services/frontend/csp_middleware.go | 51 ++++++ pkg/services/frontend/frontend_service.go | 2 + .../frontend/frontend_service_test.go | 149 ++++++++++++++++++ pkg/services/frontend/index.go | 19 +-- 5 files changed, 206 insertions(+), 18 deletions(-) create mode 100644 pkg/services/frontend/csp_middleware.go diff --git a/devenv/frontend-service/docker-compose.yaml b/devenv/frontend-service/docker-compose.yaml index 606067f9629..45f288f1293 100644 --- a/devenv/frontend-service/docker-compose.yaml +++ b/devenv/frontend-service/docker-compose.yaml @@ -69,7 +69,8 @@ services: GF_DATABASE_ENSURE_DEFAULT_ORG_AND_USER: false GF_DEFAULT_APP_MODE: development GF_DEFAULT_TARGET: frontend-server - GF_SECURITY_CONTENT_SECURITY_POLICY: false + GF_SECURITY_CONTENT_SECURITY_POLICY: true + GF_SECURITY_CONTENT_SECURITY_POLICY_TEMPLATE: "object-src 'none'; base-uri 'self'; form-action 'self'; frame-ancestors 'none'" GF_FEATURE_TOGGLES_ENABLE: enableNativeHTTPHistogram GF_SERVER_CDN_URL: http://localhost:3010 GF_SERVER_ROUTER_LOGGING: true diff --git a/pkg/services/frontend/csp_middleware.go b/pkg/services/frontend/csp_middleware.go new file mode 100644 index 00000000000..a5aeafee303 --- /dev/null +++ b/pkg/services/frontend/csp_middleware.go @@ -0,0 +1,51 @@ +package frontend + +import ( + "net/http" + + "github.com/grafana/grafana/pkg/middleware" + "github.com/grafana/grafana/pkg/services/contexthandler" + "github.com/grafana/grafana/pkg/setting" + "github.com/grafana/grafana/pkg/web" +) + +func CSPMiddleware(cfg *setting.Cfg) web.Middleware { + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + reqCtx := contexthandler.FromContext(r.Context()) + logger := reqCtx.Logger + + logger.Debug("Applying CSP middleware", "enabled", cfg.CSPEnabled, "report_only_enabled", cfg.CSPReportOnlyEnabled) + + // Bail early if CSP is not enabled + if !cfg.CSPEnabled && !cfg.CSPReportOnlyEnabled { + next.ServeHTTP(w, r) + return + } + + nonce, err := middleware.GenerateNonce() + if err != nil { + logger.Error("Failed to generate CSP nonce", "err", err) + http.Error(w, "Internal Server Error", http.StatusInternalServerError) + return + } + + // Stored on the context so the index handler can put it in the HTML + reqCtx.RequestNonce = nonce + + if cfg.CSPEnabled && cfg.CSPTemplate != "" { + logger.Debug("Setting Content-Security-Policy header") + policy := middleware.ReplacePolicyVariables(cfg.CSPTemplate, cfg.AppURL, nonce) + w.Header().Set("Content-Security-Policy", policy) + } + + if cfg.CSPReportOnlyEnabled && cfg.CSPReportOnlyTemplate != "" { + logger.Debug("Setting Content-Security-Policy-Report-Only header") + policy := middleware.ReplacePolicyVariables(cfg.CSPReportOnlyTemplate, cfg.AppURL, nonce) + w.Header().Set("Content-Security-Policy-Report-Only", policy) + } + + next.ServeHTTP(w, r) + }) + } +} diff --git a/pkg/services/frontend/frontend_service.go b/pkg/services/frontend/frontend_service.go index 74776a1169e..f634fe8927b 100644 --- a/pkg/services/frontend/frontend_service.go +++ b/pkg/services/frontend/frontend_service.go @@ -140,6 +140,8 @@ func (s *frontendService) addMiddlewares(m *web.Mux) { m.UseMiddleware(s.contextMiddleware()) m.UseMiddleware(loggermiddleware.Middleware()) + m.UseMiddleware(CSPMiddleware(s.cfg)) + m.UseMiddleware(middleware.Recovery(s.cfg, s.license)) } diff --git a/pkg/services/frontend/frontend_service_test.go b/pkg/services/frontend/frontend_service_test.go index 434f1b37e4a..e4b18a2e2ac 100644 --- a/pkg/services/frontend/frontend_service_test.go +++ b/pkg/services/frontend/frontend_service_test.go @@ -345,3 +345,152 @@ func TestFrontendService_IndexHooks(t *testing.T) { assert.Contains(t, body, "window.grafanaBootData") }) } + +func TestFrontendService_CSP(t *testing.T) { + publicDir := setupTestWebAssets(t) + + t.Run("should set CSP headers when enabled", func(t *testing.T) { + cfg := &setting.Cfg{ + HTTPPort: "3000", + StaticRootPath: publicDir, + BuildVersion: "10.3.0", + AppURL: "https://grafana.example.com/grafana", + CSPEnabled: true, + CSPTemplate: "script-src 'self' $NONCE; style-src 'self' $ROOT_PATH", + } + service := createTestService(t, cfg) + + mux := web.New() + service.addMiddlewares(mux) + service.registerRoutes(mux) + + req := httptest.NewRequest("GET", "/", nil) + recorder := httptest.NewRecorder() + mux.ServeHTTP(recorder, req) + + assert.Equal(t, 200, recorder.Code) + + // Verify CSP header is set + cspHeader := recorder.Header().Get("Content-Security-Policy") + assert.NotEmpty(t, cspHeader, "CSP header should be set") + assert.Contains(t, cspHeader, "script-src 'self' 'nonce-", "CSP should contain nonce") + assert.Contains(t, cspHeader, "style-src 'self' grafana.example.com/grafana", "CSP should contain root path") + }) + + t.Run("should set CSP-Report-Only header when enabled", func(t *testing.T) { + cfg := &setting.Cfg{ + HTTPPort: "3000", + StaticRootPath: publicDir, + BuildVersion: "10.3.0", + AppURL: "https://grafana.example.com", + CSPReportOnlyEnabled: true, + CSPReportOnlyTemplate: "default-src 'self' $NONCE", + } + service := createTestService(t, cfg) + + mux := web.New() + service.addMiddlewares(mux) + service.registerRoutes(mux) + + req := httptest.NewRequest("GET", "/", nil) + recorder := httptest.NewRecorder() + mux.ServeHTTP(recorder, req) + + assert.Equal(t, 200, recorder.Code) + + // Verify CSP-Report-Only header is set + cspReportOnlyHeader := recorder.Header().Get("Content-Security-Policy-Report-Only") + assert.NotEmpty(t, cspReportOnlyHeader, "CSP-Report-Only header should be set") + assert.Contains(t, cspReportOnlyHeader, "default-src 'self' 'nonce-", "CSP-Report-Only should contain nonce") + }) + + t.Run("should set both CSP headers when both enabled", func(t *testing.T) { + cfg := &setting.Cfg{ + HTTPPort: "3000", + StaticRootPath: publicDir, + BuildVersion: "10.3.0", + AppURL: "https://grafana.example.com", + CSPEnabled: true, + CSPTemplate: "script-src 'self' $NONCE", + CSPReportOnlyEnabled: true, + CSPReportOnlyTemplate: "default-src 'self'", + } + service := createTestService(t, cfg) + + mux := web.New() + service.addMiddlewares(mux) + service.registerRoutes(mux) + + req := httptest.NewRequest("GET", "/", nil) + recorder := httptest.NewRecorder() + mux.ServeHTTP(recorder, req) + + assert.Equal(t, 200, recorder.Code) + + // Verify both headers are set + cspHeader := recorder.Header().Get("Content-Security-Policy") + cspReportOnlyHeader := recorder.Header().Get("Content-Security-Policy-Report-Only") + assert.NotEmpty(t, cspHeader, "CSP header should be set") + assert.NotEmpty(t, cspReportOnlyHeader, "CSP-Report-Only header should be set") + }) + + t.Run("should not set CSP headers when disabled", func(t *testing.T) { + cfg := &setting.Cfg{ + HTTPPort: "3000", + StaticRootPath: publicDir, + BuildVersion: "10.3.0", + AppURL: "https://grafana.example.com", + CSPEnabled: false, + } + service := createTestService(t, cfg) + + mux := web.New() + service.addMiddlewares(mux) + service.registerRoutes(mux) + + req := httptest.NewRequest("GET", "/", nil) + recorder := httptest.NewRecorder() + mux.ServeHTTP(recorder, req) + + assert.Equal(t, 200, recorder.Code) + + // Verify CSP headers are not set + cspHeader := recorder.Header().Get("Content-Security-Policy") + cspReportOnlyHeader := recorder.Header().Get("Content-Security-Policy-Report-Only") + assert.Empty(t, cspHeader, "CSP header should not be set when disabled") + assert.Empty(t, cspReportOnlyHeader, "CSP-Report-Only header should not be set when disabled") + }) + + t.Run("should store nonce in request context", func(t *testing.T) { + cfg := &setting.Cfg{ + HTTPPort: "3000", + StaticRootPath: publicDir, + BuildVersion: "10.3.0", + AppURL: "https://grafana.example.com", + CSPEnabled: true, + CSPTemplate: "script-src 'self' $NONCE", + } + service := createTestService(t, cfg) + + mux := web.New() + service.addMiddlewares(mux) + + var capturedNonce string + mux.Get("/test-nonce", func(w http.ResponseWriter, r *http.Request) { + ctx := contexthandler.FromContext(r.Context()) + capturedNonce = ctx.RequestNonce + w.WriteHeader(200) + }) + + req := httptest.NewRequest("GET", "/test-nonce", nil) + recorder := httptest.NewRecorder() + mux.ServeHTTP(recorder, req) + + assert.Equal(t, 200, recorder.Code) + assert.NotEmpty(t, capturedNonce, "Nonce should be stored in request context") + + // Verify the nonce in context matches the one in the CSP header + cspHeader := recorder.Header().Get("Content-Security-Policy") + assert.Contains(t, cspHeader, "'nonce-"+capturedNonce+"'", "Nonce in header should match context nonce") + }) +} diff --git a/pkg/services/frontend/index.go b/pkg/services/frontend/index.go index cefc422d1c0..ec10818ce22 100644 --- a/pkg/services/frontend/index.go +++ b/pkg/services/frontend/index.go @@ -11,7 +11,6 @@ import ( "github.com/grafana/grafana-app-sdk/logging" "github.com/grafana/grafana/pkg/api/dtos" - "github.com/grafana/grafana/pkg/middleware" "github.com/grafana/grafana/pkg/services/contexthandler" contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" "github.com/grafana/grafana/pkg/services/hooks" @@ -134,18 +133,12 @@ func (p *IndexProvider) HandleRequest(writer http.ResponseWriter, request *http. return } - nonce, err := middleware.GenerateNonce() - if err != nil { - p.log.Error("error creating nonce", "err", err) - writer.WriteHeader(500) - return - } - reqCtx := contexthandler.FromContext(ctx) // TODO -- restructure so the static stuff is under one variable and the rest is dynamic data := p.data // copy everything - data.Nonce = nonce + // Use nonce generated by CSP middleware + data.Nonce = reqCtx.RequestNonce data.PublicDashboardAccessToken = reqCtx.PublicDashboardAccessToken // TODO -- reevaluate with mt authnz @@ -172,14 +165,6 @@ func (p *IndexProvider) HandleRequest(writer http.ResponseWriter, request *http. }) } - if data.CSPEnabled { - data.CSPContent = middleware.ReplacePolicyVariables(p.data.CSPContent, p.data.AppSubUrl, data.Nonce) - writer.Header().Set("Content-Security-Policy", data.CSPContent) - - policy := middleware.ReplacePolicyVariables(p.data.CSPReportOnlyContent, p.data.AppSubUrl, data.Nonce) - writer.Header().Set("Content-Security-Policy-Report-Only", policy) - } - p.runIndexDataHooks(reqCtx, &data) writer.Header().Set("Content-Type", "text/html; charset=UTF-8")