chore: add deployment config option to append custom csp directives (#15596)

Allows adding custom static CSP directives to Coder. Niche use case but
makes this easier then creating a reverse proxy that has to replace the
header. We want to preserve our directives, so having an append option
is preferred to a "replace" option via a reverse proxy.


Closes https://github.com/coder/coder/issues/15118
This commit is contained in:
Steven Masley
2024-11-21 11:53:53 -06:00
committed by GitHub
parent f38f746f5d
commit 5b7fa78676
13 changed files with 140 additions and 39 deletions
+6
View File
@@ -10537,6 +10537,12 @@ const docTemplate = `{
"access_url": {
"$ref": "#/definitions/serpent.URL"
},
"additional_csp_policy": {
"type": "array",
"items": {
"type": "string"
}
},
"address": {
"description": "DEPRECATED: Use HTTPAddress or TLS.Address instead.",
"allOf": [
+6
View File
@@ -9403,6 +9403,12 @@
"access_url": {
"$ref": "#/definitions/serpent.URL"
},
"additional_csp_policy": {
"type": "array",
"items": {
"type": "string"
}
},
"address": {
"description": "DEPRECATED: Use HTTPAddress or TLS.Address instead.",
"allOf": [
+22 -1
View File
@@ -5,6 +5,7 @@ import (
"crypto/tls"
"crypto/x509"
"database/sql"
"errors"
"expvar"
"flag"
"fmt"
@@ -1378,6 +1379,26 @@ func New(options *Options) *API {
r.Get("/swagger/*", swaggerDisabled)
}
additionalCSPHeaders := make(map[httpmw.CSPFetchDirective][]string, len(api.DeploymentValues.AdditionalCSPPolicy))
var cspParseErrors error
for _, v := range api.DeploymentValues.AdditionalCSPPolicy {
// Format is "<directive> <value> <value> ..."
v = strings.TrimSpace(v)
parts := strings.Split(v, " ")
if len(parts) < 2 {
cspParseErrors = errors.Join(cspParseErrors, xerrors.Errorf("invalid CSP header %q, not enough parts to be valid", v))
continue
}
additionalCSPHeaders[httpmw.CSPFetchDirective(strings.ToLower(parts[0]))] = parts[1:]
}
if cspParseErrors != nil {
// Do not fail Coder deployment startup because of this. Just log an error
// and continue
api.Logger.Error(context.Background(),
"parsing additional CSP headers", slog.Error(cspParseErrors))
}
// Add CSP headers to all static assets and pages. CSP headers only affect
// browsers, so these don't make sense on api routes.
cspMW := httpmw.CSPHeaders(options.Telemetry.Enabled(), func() []string {
@@ -1390,7 +1411,7 @@ func New(options *Options) *API {
}
// By default we do not add extra websocket connections to the CSP
return []string{}
})
}, additionalCSPHeaders)
// Static file handler must be wrapped with HSTS handler if the
// StrictTransportSecurityAge is set. We only need to set this header on
+52 -38
View File
@@ -23,29 +23,39 @@ func (s cspDirectives) Append(d CSPFetchDirective, values ...string) {
type CSPFetchDirective string
const (
cspDirectiveDefaultSrc = "default-src"
cspDirectiveConnectSrc = "connect-src"
cspDirectiveChildSrc = "child-src"
cspDirectiveScriptSrc = "script-src"
cspDirectiveFontSrc = "font-src"
cspDirectiveStyleSrc = "style-src"
cspDirectiveObjectSrc = "object-src"
cspDirectiveManifestSrc = "manifest-src"
cspDirectiveFrameSrc = "frame-src"
cspDirectiveImgSrc = "img-src"
cspDirectiveReportURI = "report-uri"
cspDirectiveFormAction = "form-action"
cspDirectiveMediaSrc = "media-src"
cspFrameAncestors = "frame-ancestors"
cspDirectiveWorkerSrc = "worker-src"
CSPDirectiveDefaultSrc CSPFetchDirective = "default-src"
CSPDirectiveConnectSrc CSPFetchDirective = "connect-src"
CSPDirectiveChildSrc CSPFetchDirective = "child-src"
CSPDirectiveScriptSrc CSPFetchDirective = "script-src"
CSPDirectiveFontSrc CSPFetchDirective = "font-src"
CSPDirectiveStyleSrc CSPFetchDirective = "style-src"
CSPDirectiveObjectSrc CSPFetchDirective = "object-src"
CSPDirectiveManifestSrc CSPFetchDirective = "manifest-src"
CSPDirectiveFrameSrc CSPFetchDirective = "frame-src"
CSPDirectiveImgSrc CSPFetchDirective = "img-src"
CSPDirectiveReportURI CSPFetchDirective = "report-uri"
CSPDirectiveFormAction CSPFetchDirective = "form-action"
CSPDirectiveMediaSrc CSPFetchDirective = "media-src"
CSPFrameAncestors CSPFetchDirective = "frame-ancestors"
CSPDirectiveWorkerSrc CSPFetchDirective = "worker-src"
)
// CSPHeaders returns a middleware that sets the Content-Security-Policy header
// for coderd. It takes a function that allows adding supported external websocket
// hosts. This is primarily to support the terminal connecting to a workspace proxy.
// for coderd.
//
// Arguments:
// - websocketHosts: a function that returns a list of supported external websocket hosts.
// This is to support the terminal connecting to a workspace proxy.
// The origin of the terminal request does not match the url of the proxy,
// so the CSP list of allowed hosts must be dynamic and match the current
// available proxy urls.
// - staticAdditions: a map of CSP directives to append to the default CSP headers.
// Used to allow specific static additions to the CSP headers. Allows some niche
// use cases, such as embedding Coder in an iframe.
// Example: https://github.com/coder/coder/issues/15118
//
//nolint:revive
func CSPHeaders(telemetry bool, websocketHosts func() []string) func(next http.Handler) http.Handler {
func CSPHeaders(telemetry bool, websocketHosts func() []string, staticAdditions map[CSPFetchDirective][]string) func(next http.Handler) http.Handler {
return func(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
// Content-Security-Policy disables loading certain content types and can prevent XSS injections.
@@ -55,30 +65,30 @@ func CSPHeaders(telemetry bool, websocketHosts func() []string) func(next http.H
// The list of CSP options: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/default-src
cspSrcs := cspDirectives{
// All omitted fetch csp srcs default to this.
cspDirectiveDefaultSrc: {"'self'"},
cspDirectiveConnectSrc: {"'self'"},
cspDirectiveChildSrc: {"'self'"},
CSPDirectiveDefaultSrc: {"'self'"},
CSPDirectiveConnectSrc: {"'self'"},
CSPDirectiveChildSrc: {"'self'"},
// https://github.com/suren-atoyan/monaco-react/issues/168
cspDirectiveScriptSrc: {"'self'"},
cspDirectiveStyleSrc: {"'self' 'unsafe-inline'"},
CSPDirectiveScriptSrc: {"'self'"},
CSPDirectiveStyleSrc: {"'self' 'unsafe-inline'"},
// data: is used by monaco editor on FE for Syntax Highlight
cspDirectiveFontSrc: {"'self' data:"},
cspDirectiveWorkerSrc: {"'self' blob:"},
CSPDirectiveFontSrc: {"'self' data:"},
CSPDirectiveWorkerSrc: {"'self' blob:"},
// object-src is needed to support code-server
cspDirectiveObjectSrc: {"'self'"},
CSPDirectiveObjectSrc: {"'self'"},
// blob: for loading the pwa manifest for code-server
cspDirectiveManifestSrc: {"'self' blob:"},
cspDirectiveFrameSrc: {"'self'"},
CSPDirectiveManifestSrc: {"'self' blob:"},
CSPDirectiveFrameSrc: {"'self'"},
// data: for loading base64 encoded icons for generic applications.
// https: allows loading images from external sources. This is not ideal
// but is required for the templates page that renders readmes.
// We should find a better solution in the future.
cspDirectiveImgSrc: {"'self' https: data:"},
cspDirectiveFormAction: {"'self'"},
cspDirectiveMediaSrc: {"'self'"},
CSPDirectiveImgSrc: {"'self' https: data:"},
CSPDirectiveFormAction: {"'self'"},
CSPDirectiveMediaSrc: {"'self'"},
// Report all violations back to the server to log
cspDirectiveReportURI: {"/api/v2/csp/reports"},
cspFrameAncestors: {"'none'"},
CSPDirectiveReportURI: {"/api/v2/csp/reports"},
CSPFrameAncestors: {"'none'"},
// Only scripts can manipulate the dom. This prevents someone from
// naming themselves something like '<svg onload="alert(/cross-site-scripting/)" />'.
@@ -87,7 +97,7 @@ func CSPHeaders(telemetry bool, websocketHosts func() []string) func(next http.H
if telemetry {
// If telemetry is enabled, we report to coder.com.
cspSrcs.Append(cspDirectiveConnectSrc, "https://coder.com")
cspSrcs.Append(CSPDirectiveConnectSrc, "https://coder.com")
}
// This extra connect-src addition is required to support old webkit
@@ -102,7 +112,7 @@ func CSPHeaders(telemetry bool, websocketHosts func() []string) func(next http.H
// We can add both ws:// and wss:// as browsers do not let https
// pages to connect to non-tls websocket connections. So this
// supports both http & https webpages.
cspSrcs.Append(cspDirectiveConnectSrc, fmt.Sprintf("wss://%[1]s ws://%[1]s", host))
cspSrcs.Append(CSPDirectiveConnectSrc, fmt.Sprintf("wss://%[1]s ws://%[1]s", host))
}
// The terminal requires a websocket connection to the workspace proxy.
@@ -112,15 +122,19 @@ func CSPHeaders(telemetry bool, websocketHosts func() []string) func(next http.H
for _, extraHost := range extraConnect {
if extraHost == "*" {
// '*' means all
cspSrcs.Append(cspDirectiveConnectSrc, "*")
cspSrcs.Append(CSPDirectiveConnectSrc, "*")
continue
}
cspSrcs.Append(cspDirectiveConnectSrc, fmt.Sprintf("wss://%[1]s ws://%[1]s", extraHost))
cspSrcs.Append(CSPDirectiveConnectSrc, fmt.Sprintf("wss://%[1]s ws://%[1]s", extraHost))
// We also require this to make http/https requests to the workspace proxy for latency checking.
cspSrcs.Append(cspDirectiveConnectSrc, fmt.Sprintf("https://%[1]s http://%[1]s", extraHost))
cspSrcs.Append(CSPDirectiveConnectSrc, fmt.Sprintf("https://%[1]s http://%[1]s", extraHost))
}
}
for directive, values := range staticAdditions {
cspSrcs.Append(directive, values...)
}
var csp strings.Builder
for src, vals := range cspSrcs {
_, _ = fmt.Fprintf(&csp, "%s %s; ", src, strings.Join(vals, " "))
+6
View File
@@ -15,12 +15,15 @@ func TestCSPConnect(t *testing.T) {
t.Parallel()
expected := []string{"example.com", "coder.com"}
expectedMedia := []string{"media.com", "media2.com"}
r := httptest.NewRequest(http.MethodGet, "/", nil)
rw := httptest.NewRecorder()
httpmw.CSPHeaders(false, func() []string {
return expected
}, map[httpmw.CSPFetchDirective][]string{
httpmw.CSPDirectiveMediaSrc: expectedMedia,
})(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
rw.WriteHeader(http.StatusOK)
})).ServeHTTP(rw, r)
@@ -30,4 +33,7 @@ func TestCSPConnect(t *testing.T) {
require.Containsf(t, rw.Header().Get("Content-Security-Policy"), fmt.Sprintf("ws://%s", e), "Content-Security-Policy header should contain ws://%s", e)
require.Containsf(t, rw.Header().Get("Content-Security-Policy"), fmt.Sprintf("wss://%s", e), "Content-Security-Policy header should contain wss://%s", e)
}
for _, e := range expectedMedia {
require.Containsf(t, rw.Header().Get("Content-Security-Policy"), e, "Content-Security-Policy header should contain %s", e)
}
}