Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 41 additions & 20 deletions coderd/coderd.go
Original file line number Diff line number Diff line change
Expand Up @@ -1982,29 +1982,50 @@ func New(options *Options) *API {

// 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() []*proxyhealth.ProxyHost {
if api.DeploymentValues.Dangerous.AllowAllCors {
// In this mode, allow all external requests.
return []*proxyhealth.ProxyHost{
{
Host: "*",
AppHost: "*",
},
}
}
// Always add the primary, since the app host may be on a sub-domain.
proxies := []*proxyhealth.ProxyHost{
cspProxyHosts := func() []*proxyhealth.ProxyHost {
if api.DeploymentValues.Dangerous.AllowAllCors {
// In this mode, allow all external requests.
return []*proxyhealth.ProxyHost{
{
Host: api.AccessURL.Host,
AppHost: appurl.ConvertAppHostForCSP(api.AccessURL.Host, api.AppHostname),
Host: "*",
AppHost: "*",
},
}
if f := api.WorkspaceProxyHostsFn.Load(); f != nil {
proxies = append(proxies, (*f)()...)
}
return proxies
}, additionalCSPHeaders)
}
// Always add the primary, since the app host may be on a sub-domain.
proxies := []*proxyhealth.ProxyHost{
{
Host: api.AccessURL.Host,
AppHost: appurl.ConvertAppHostForCSP(api.AccessURL.Host, api.AppHostname),
},
}
if f := api.WorkspaceProxyHostsFn.Load(); f != nil {
proxies = append(proxies, (*f)()...)
}
return proxies
}
cspMW := httpmw.CSPHeaders(options.Telemetry.Enabled(), cspProxyHosts, additionalCSPHeaders)

// Embed routes (e.g. VS Code extension chat) are designed to be
// loaded inside iframes, so they must not include frame-ancestors
// in their CSP. The CSP wildcard '*' only matches network schemes
// (http, https, ws, wss) and cannot cover custom schemes like
// vscode-webview://, so the only way to allow all embedders is
// to omit the directive entirely. If the operator explicitly
// configured frame-ancestors via CODER_ADDITIONAL_CSP_POLICY,
// respect that setting.

embedCSPHeaders := make(map[httpmw.CSPFetchDirective][]string, len(additionalCSPHeaders))
for k, v := range additionalCSPHeaders {
embedCSPHeaders[k] = v
}
if _, ok := additionalCSPHeaders[httpmw.CSPFrameAncestors]; !ok {
embedCSPHeaders[httpmw.CSPFrameAncestors] = []string{}
}
embedCSPMW := httpmw.CSPHeaders(options.Telemetry.Enabled(), cspProxyHosts, embedCSPHeaders)
embedHandler := embedCSPMW(compressHandler(httpmw.HSTS(api.SiteHandler, options.StrictTransportSecurityCfg)))
r.Get("/agents/{agentId}/embed", embedHandler.ServeHTTP)
r.Get("/agents/{agentId}/embed/*", embedHandler.ServeHTTP)

// Static file handler must be wrapped with HSTS handler if the
// StrictTransportSecurityAge is set. We only need to set this header on
Expand Down
16 changes: 16 additions & 0 deletions coderd/httpmw/csp.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,22 @@ func CSPHeaders(telemetry bool, proxyHosts func() []*proxyhealth.ProxyHost, stat
cspSrcs.Append(directive, values...)
}

// Default to 'self' to prevent clickjacking unless
// explicitly overridden via staticAdditions (e.g. for
// embeddable routes).
//
// An explicit empty value means "omit frame-ancestors
// entirely", which is needed for embed routes where
// non-network-scheme parents (e.g. vscode-webview://)
// must be able to frame the page. The CSP wildcard '*'
// only matches network schemes (http, https, ws, wss)
// so it cannot cover custom schemes.
if vals, ok := cspSrcs[CSPFrameAncestors]; !ok {
cspSrcs[CSPFrameAncestors] = []string{"'self'"}
} else if len(vals) == 0 {
delete(cspSrcs, CSPFrameAncestors)
}

var csp strings.Builder
for src, vals := range cspSrcs {
_, _ = fmt.Fprintf(&csp, "%s %s; ", src, strings.Join(vals, " "))
Expand Down
57 changes: 57 additions & 0 deletions coderd/httpmw/csp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,63 @@ import (
"github.com/coder/coder/v2/coderd/proxyhealth"
)

func TestCSPFrameAncestors(t *testing.T) {
t.Parallel()

t.Run("DefaultSelf", func(t *testing.T) {
t.Parallel()

r := httptest.NewRequest(http.MethodGet, "/", nil)
rw := httptest.NewRecorder()

httpmw.CSPHeaders(false, func() []*proxyhealth.ProxyHost {
return nil
}, nil)(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
rw.WriteHeader(http.StatusOK)
})).ServeHTTP(rw, r)

csp := rw.Header().Get("Content-Security-Policy")
require.Contains(t, csp, "frame-ancestors 'self'")
})

t.Run("OverrideViaStaticAdditions", func(t *testing.T) {
t.Parallel()

r := httptest.NewRequest(http.MethodGet, "/", nil)
rw := httptest.NewRecorder()

httpmw.CSPHeaders(false, func() []*proxyhealth.ProxyHost {
return nil
}, map[httpmw.CSPFetchDirective][]string{
httpmw.CSPFrameAncestors: {"https://example.com"},
})(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
rw.WriteHeader(http.StatusOK)
})).ServeHTTP(rw, r)

csp := rw.Header().Get("Content-Security-Policy")
require.Contains(t, csp, "frame-ancestors https://example.com")
require.NotContains(t, csp, "frame-ancestors 'self'")
})

t.Run("OmitWhenEmpty", func(t *testing.T) {
t.Parallel()

r := httptest.NewRequest(http.MethodGet, "/", nil)
rw := httptest.NewRecorder()

httpmw.CSPHeaders(false, func() []*proxyhealth.ProxyHost {
return nil
}, map[httpmw.CSPFetchDirective][]string{
httpmw.CSPFrameAncestors: {},
})(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
rw.WriteHeader(http.StatusOK)
})).ServeHTTP(rw, r)

csp := rw.Header().Get("Content-Security-Policy")
require.NotContains(t, csp, "frame-ancestors")
})
}

func TestCSP(t *testing.T) {
t.Parallel()

Expand Down
Loading