Skip to content

Commit ac581e9

Browse files
code-ashercode-asher
committed
feat: add csp headers for embedded apps
1 parent 068f9a0 commit ac581e9

File tree

8 files changed

+180
-57
lines changed

8 files changed

+180
-57
lines changed

‎coderd/coderd.go

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ import (
7676
"github.com/coder/coder/v2/coderd/portsharing"
7777
"github.com/coder/coder/v2/coderd/prometheusmetrics"
7878
"github.com/coder/coder/v2/coderd/provisionerdserver"
79+
"github.com/coder/coder/v2/coderd/proxyhealth"
7980
"github.com/coder/coder/v2/coderd/rbac"
8081
"github.com/coder/coder/v2/coderd/rbac/policy"
8182
"github.com/coder/coder/v2/coderd/rbac/rolestore"
@@ -85,6 +86,7 @@ import (
8586
"github.com/coder/coder/v2/coderd/updatecheck"
8687
"github.com/coder/coder/v2/coderd/util/slice"
8788
"github.com/coder/coder/v2/coderd/workspaceapps"
89+
"github.com/coder/coder/v2/coderd/workspaceapps/appurl"
8890
"github.com/coder/coder/v2/coderd/workspacestats"
8991
"github.com/coder/coder/v2/codersdk"
9092
"github.com/coder/coder/v2/codersdk/healthsdk"
@@ -1534,16 +1536,27 @@ func New(options *Options) *API {
15341536
// browsers, so these don't make sense on api routes.
15351537
cspMW := httpmw.CSPHeaders(
15361538
api.Experiments,
1537-
options.Telemetry.Enabled(), func() []string {
1539+
options.Telemetry.Enabled(), func() []*proxyhealth.ProxyHost {
15381540
if api.DeploymentValues.Dangerous.AllowAllCors {
1539-
// In this mode, allow all external requests
1540-
return []string{"*"}
1541+
// In this mode, allow all external requests.
1542+
return []*proxyhealth.ProxyHost{
1543+
{
1544+
Host: "*",
1545+
AppHost: "*",
1546+
},
1547+
}
1548+
}
1549+
// Always add the primary, since the app host may be on a sub-domain.
1550+
proxies := []*proxyhealth.ProxyHost{
1551+
{
1552+
Host: api.AccessURL.Host,
1553+
AppHost: appurl.ConvertAppHostForCSP(api.AccessURL.String(), api.AppHostname),
1554+
},
15411555
}
15421556
if f := api.WorkspaceProxyHostsFn.Load(); f != nil {
1543-
return (*f)()
1557+
proxies = append(proxies, (*f)()...)
15441558
}
1545-
// By default we do not add extra websocket connections to the CSP
1546-
return []string{}
1559+
return proxies
15471560
}, additionalCSPHeaders)
15481561

15491562
// Static file handler must be wrapped with HSTS handler if the
@@ -1582,7 +1595,7 @@ type API struct {
15821595
AppearanceFetcher atomic.Pointer[appearance.Fetcher]
15831596
// WorkspaceProxyHostsFn returns the hosts of healthy workspace proxies
15841597
// for header reasons.
1585-
WorkspaceProxyHostsFn atomic.Pointer[func() []string]
1598+
WorkspaceProxyHostsFn atomic.Pointer[func() []*proxyhealth.ProxyHost]
15861599
// TemplateScheduleStore is a pointer to an atomic pointer because this is
15871600
// passed to another struct, and we want them all to be the same reference.
15881601
TemplateScheduleStore *atomic.Pointer[schedule.TemplateScheduleStore]

‎coderd/httpmw/csp.go

Lines changed: 20 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"net/http"
66
"strings"
77

8+
"github.com/coder/coder/v2/coderd/proxyhealth"
89
"github.com/coder/coder/v2/codersdk"
910
)
1011

@@ -47,18 +48,18 @@ const (
4748
// for coderd.
4849
//
4950
// Arguments:
50-
// - websocketHosts: a function that returns a list of supported external websocket hosts.
51-
// This is to support the terminal connecting to a workspace proxy.
52-
// The origin of the terminal request does not match the url of the proxy,
53-
// so the CSP list of allowed hosts must be dynamic and match the current
54-
// available proxy urls.
51+
// - proxyHosts: a function that returns a list of supported proxy hosts
52+
// (including the primary). This is to support the terminal connecting to a
53+
// workspace proxy and for embedding apps in an iframe. The origin of the
54+
// requests do not match the url of the proxy, so the CSP list of allowed
55+
// hosts must be dynamic and match the current available proxy urls.
5556
// - staticAdditions: a map of CSP directives to append to the default CSP headers.
5657
// Used to allow specific static additions to the CSP headers. Allows some niche
5758
// use cases, such as embedding Coder in an iframe.
5859
// Example: https://github.com/coder/coder/issues/15118
5960
//
6061
//nolint:revive
61-
func CSPHeaders(experiments codersdk.Experiments, telemetry bool, websocketHosts func() []string, staticAdditions map[CSPFetchDirective][]string) func(next http.Handler) http.Handler {
62+
func CSPHeaders(experiments codersdk.Experiments, telemetry bool, proxyHosts func() []*proxyhealth.ProxyHost, staticAdditions map[CSPFetchDirective][]string) func(next http.Handler) http.Handler {
6263
return func(next http.Handler) http.Handler {
6364
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
6465
// Content-Security-Policy disables loading certain content types and can prevent XSS injections.
@@ -97,15 +98,6 @@ func CSPHeaders(experiments codersdk.Experiments, telemetry bool, websocketHosts
9798
// "require-trusted-types-for" : []string{"'script'"},
9899
}
99100

100-
if experiments.Enabled(codersdk.ExperimentAITasks) {
101-
// AI tasks use iframe embeds of local apps.
102-
// TODO: Handle region domains too, not just path based apps
103-
cspSrcs.Append(CSPFrameAncestors, `'self'`)
104-
cspSrcs.Append(CSPFrameSource, `'self'`)
105-
} else {
106-
cspSrcs.Append(CSPFrameAncestors, `'none'`)
107-
}
108-
109101
if telemetry {
110102
// If telemetry is enabled, we report to coder.com.
111103
cspSrcs.Append(CSPDirectiveConnectSrc, "https://coder.com")
@@ -126,19 +118,26 @@ func CSPHeaders(experiments codersdk.Experiments, telemetry bool, websocketHosts
126118
cspSrcs.Append(CSPDirectiveConnectSrc, fmt.Sprintf("wss://%[1]s ws://%[1]s", host))
127119
}
128120

129-
// The terminal requires a websocket connection to the workspace proxy.
130-
// Make sure we allow this connection to healthy proxies.
131-
extraConnect := websocketHosts()
121+
// The terminal and iframed apps can use workspace proxies (which includes
122+
// the primary). Make sure we allow connections to healthy proxies.
123+
extraConnect := proxyHosts()
132124
if len(extraConnect) > 0 {
133125
for _, extraHost := range extraConnect {
134-
if extraHost == "*" {
126+
// Allow embedding the app host.
127+
if experiments.Enabled(codersdk.ExperimentAITasks) {
128+
cspSrcs.Append(CSPDirectiveFrameSrc, extraHost.AppHost)
129+
}
130+
if extraHost.Host == "*" {
135131
// '*' means all
136132
cspSrcs.Append(CSPDirectiveConnectSrc, "*")
137133
continue
138134
}
139-
cspSrcs.Append(CSPDirectiveConnectSrc, fmt.Sprintf("wss://%[1]s ws://%[1]s", extraHost))
135+
// Avoid double-adding r.Host.
136+
if extraHost.Host != r.Host {
137+
cspSrcs.Append(CSPDirectiveConnectSrc, fmt.Sprintf("wss://%[1]s ws://%[1]s", extraHost.Host))
138+
}
140139
// We also require this to make http/https requests to the workspace proxy for latency checking.
141-
cspSrcs.Append(CSPDirectiveConnectSrc, fmt.Sprintf("https://%[1]s http://%[1]s", extraHost))
140+
cspSrcs.Append(CSPDirectiveConnectSrc, fmt.Sprintf("https://%[1]s http://%[1]s", extraHost.Host))
142141
}
143142
}
144143

‎coderd/httpmw/csp_test.go

Lines changed: 37 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,59 @@
11
package httpmw_test
22

33
import (
4-
"fmt"
54
"net/http"
65
"net/http/httptest"
6+
"strings"
77
"testing"
88

99
"github.com/stretchr/testify/require"
1010

1111
"github.com/coder/coder/v2/coderd/httpmw"
12+
"github.com/coder/coder/v2/coderd/proxyhealth"
1213
"github.com/coder/coder/v2/codersdk"
1314
)
1415

15-
func TestCSPConnect(t *testing.T) {
16+
func TestCSP(t *testing.T) {
1617
t.Parallel()
1718

18-
expected := []string{"example.com", "coder.com"}
19+
proxyHosts := []*proxyhealth.ProxyHost{
20+
{
21+
Host: "test.com",
22+
AppHost: "*.test.com",
23+
},
24+
{
25+
Host: "coder.com",
26+
AppHost: "*.coder.com",
27+
},
28+
{
29+
// Host is not added because it duplicates the host header.
30+
Host: "example.com",
31+
AppHost: "*.coder2.com",
32+
},
33+
}
1934
expectedMedia := []string{"media.com", "media2.com"}
2035

36+
expected := []string{
37+
"frame-src 'self' *.test.com *.coder.com *.coder2.com",
38+
"media-src 'self' media.com media2.com",
39+
strings.Join([]string{
40+
"connect-src", "'self'",
41+
// Added from host header.
42+
"wss://example.com", "ws://example.com",
43+
// Added via proxy hosts.
44+
"wss://test.com", "ws://test.com", "https://test.com", "http://test.com",
45+
"wss://coder.com", "ws://coder.com", "https://coder.com", "http://coder.com",
46+
}, " "),
47+
}
48+
49+
// When the host is empty, it uses example.com.
2150
r := httptest.NewRequest(http.MethodGet, "/", nil)
2251
rw := httptest.NewRecorder()
2352

24-
httpmw.CSPHeaders(codersdk.Experiments{}, false, func() []string {
25-
return expected
53+
httpmw.CSPHeaders(codersdk.Experiments{
54+
codersdk.ExperimentAITasks,
55+
}, false, func() []*proxyhealth.ProxyHost {
56+
return proxyHosts
2657
}, map[httpmw.CSPFetchDirective][]string{
2758
httpmw.CSPDirectiveMediaSrc: expectedMedia,
2859
})(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
@@ -31,10 +62,6 @@ func TestCSPConnect(t *testing.T) {
3162

3263
require.NotEmpty(t, rw.Header().Get("Content-Security-Policy"), "Content-Security-Policy header should not be empty")
3364
for _, e := range expected {
34-
require.Containsf(t, rw.Header().Get("Content-Security-Policy"), fmt.Sprintf("ws://%s", e), "Content-Security-Policy header should contain ws://%s", e)
35-
require.Containsf(t, rw.Header().Get("Content-Security-Policy"), fmt.Sprintf("wss://%s", e), "Content-Security-Policy header should contain wss://%s", e)
36-
}
37-
for _, e := range expectedMedia {
38-
require.Containsf(t, rw.Header().Get("Content-Security-Policy"), e, "Content-Security-Policy header should contain %s", e)
65+
require.Contains(t, rw.Header().Get("Content-Security-Policy"), e)
3966
}
4067
}

‎coderd/proxyhealth/proxyhealth.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
package proxyhealth
2+
3+
type ProxyHost struct {
4+
// Host is the root host of the proxy.
5+
Host string
6+
// AppHost is the wildcard host where apps are hosted.
7+
AppHost string
8+
}

‎coderd/workspaceapps/appurl/appurl.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -289,3 +289,23 @@ func ExecuteHostnamePattern(pattern *regexp.Regexp, hostname string) (string, bo
289289

290290
return matches[1], true
291291
}
292+
293+
// ConvertAppHostForCSP converts the wildcard host to a format accepted by CSP.
294+
// For example *--apps.coder.com must become *.coder.com. If there is no
295+
// wildcard host, or it cannot be converted, return the base host.
296+
func ConvertAppHostForCSP(host, wildcard string) string {
297+
if wildcard == "" {
298+
return host
299+
}
300+
parts := strings.Split(wildcard, ".")
301+
for i, part := range parts {
302+
if strings.Contains(part, "*") {
303+
// The wildcard can only be in the first section.
304+
if i != 0 {
305+
return host
306+
}
307+
parts[i] = "*"
308+
}
309+
}
310+
return strings.Join(parts, ".")
311+
}

‎coderd/workspaceapps/appurl/appurl_test.go

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -410,3 +410,59 @@ func TestCompileHostnamePattern(t *testing.T) {
410410
})
411411
}
412412
}
413+
414+
func TestConvertAppURLForCSP(t *testing.T) {
415+
t.Parallel()
416+
417+
testCases := []struct {
418+
name string
419+
host string
420+
wildcard string
421+
expected string
422+
}{
423+
{
424+
name: "Empty",
425+
host: "example.com",
426+
wildcard: "",
427+
expected: "example.com",
428+
},
429+
{
430+
name: "NoAsterisk",
431+
host: "example.com",
432+
wildcard: "coder.com",
433+
expected: "coder.com",
434+
},
435+
{
436+
name: "Asterisk",
437+
host: "example.com",
438+
wildcard: "*.coder.com",
439+
expected: "*.coder.com",
440+
},
441+
{
442+
name: "FirstPrefix",
443+
host: "example.com",
444+
wildcard: "*--apps.coder.com",
445+
expected: "*.coder.com",
446+
},
447+
{
448+
name: "FirstSuffix",
449+
host: "example.com",
450+
wildcard: "apps--*.coder.com",
451+
expected: "*.coder.com",
452+
},
453+
{
454+
name: "Middle",
455+
host: "example.com",
456+
wildcard: "apps.*.com",
457+
expected: "example.com",
458+
},
459+
}
460+
461+
for _, c := range testCases {
462+
c := c
463+
t.Run(c.name, func(t *testing.T) {
464+
t.Parallel()
465+
require.Equal(t, c.expected, appurl.ConvertAppHostForCSP(c.host, c.wildcard))
466+
})
467+
}
468+
}

‎enterprise/coderd/proxyhealth/proxyhealth.go

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ import (
2121
"github.com/coder/coder/v2/coderd/database"
2222
"github.com/coder/coder/v2/coderd/database/dbauthz"
2323
"github.com/coder/coder/v2/coderd/prometheusmetrics"
24+
agplproxyhealth "github.com/coder/coder/v2/coderd/proxyhealth"
25+
"github.com/coder/coder/v2/coderd/workspaceapps/appurl"
2426
"github.com/coder/coder/v2/codersdk"
2527
)
2628

@@ -63,7 +65,7 @@ type ProxyHealth struct {
6365

6466
// Cached values for quick access to the health of proxies.
6567
cache *atomic.Pointer[map[uuid.UUID]ProxyStatus]
66-
proxyHosts *atomic.Pointer[[]string]
68+
proxyHosts *atomic.Pointer[[]*agplproxyhealth.ProxyHost]
6769

6870
// PromMetrics
6971
healthCheckDuration prometheus.Histogram
@@ -116,7 +118,7 @@ func New(opts *Options) (*ProxyHealth, error) {
116118
logger: opts.Logger,
117119
client: client,
118120
cache: &atomic.Pointer[map[uuid.UUID]ProxyStatus]{},
119-
proxyHosts: &atomic.Pointer[[]string]{},
121+
proxyHosts: &atomic.Pointer[[]*agplproxyhealth.ProxyHost]{},
120122
healthCheckDuration: healthCheckDuration,
121123
healthCheckResults: healthCheckResults,
122124
}, nil
@@ -144,9 +146,9 @@ func (p *ProxyHealth) Run(ctx context.Context) {
144146
}
145147

146148
func (p *ProxyHealth) storeProxyHealth(statuses map[uuid.UUID]ProxyStatus) {
147-
var proxyHosts []string
149+
var proxyHosts []*agplproxyhealth.ProxyHost
148150
for _, s := range statuses {
149-
if s.ProxyHost != "" {
151+
if s.ProxyHost != nil {
150152
proxyHosts = append(proxyHosts, s.ProxyHost)
151153
}
152154
}
@@ -190,23 +192,22 @@ type ProxyStatus struct {
190192
// then the proxy in hand. AKA if the proxy was updated, and the status was for
191193
// an older proxy.
192194
Proxy database.WorkspaceProxy
193-
// ProxyHost is the host:port of the proxy url. This is included in the status
194-
// to make sure the proxy url is a valid URL. It also makes it easier to
195-
// escalate errors if the url.Parse errors (should never happen).
196-
ProxyHost string
195+
// ProxyHost is the base host:port and app host of the proxy. This is included
196+
// in the status to make sure the proxy url is a valid URL. It also makes it
197+
// easier to escalate errors if the url.Parse errors (should never happen).
198+
ProxyHost *agplproxyhealth.ProxyHost
197199
Status Status
198200
Report codersdk.ProxyHealthReport
199201
CheckedAt time.Time
200202
}
201203

202-
// ProxyHosts returns the host:port of all healthy proxies.
203-
// This can be computed from HealthStatus, but is cached to avoid the
204-
// caller needing to loop over all proxies to compute this on all
205-
// static web requests.
206-
func (p *ProxyHealth) ProxyHosts() []string {
204+
// ProxyHosts returns the host:port and wildcard host of all healthy proxies.
205+
// This can be computed from HealthStatus, but is cached to avoid the caller
206+
// needing to loop over all proxies to compute this on all static web requests.
207+
func (p *ProxyHealth) ProxyHosts() []*agplproxyhealth.ProxyHost {
207208
ptr := p.proxyHosts.Load()
208209
if ptr == nil {
209-
return []string{}
210+
return []*agplproxyhealth.ProxyHost{}
210211
}
211212
return *ptr
212213
}
@@ -350,7 +351,10 @@ func (p *ProxyHealth) runOnce(ctx context.Context, now time.Time) (map[uuid.UUID
350351
status.Report.Errors = append(status.Report.Errors, fmt.Sprintf("failed to parse proxy url: %s", err.Error()))
351352
status.Status = Unhealthy
352353
}
353-
status.ProxyHost = u.Host
354+
status.ProxyHost = &agplproxyhealth.ProxyHost{
355+
Host: u.Host,
356+
AppHost: appurl.ConvertAppHostForCSP(u.Host, proxy.WildcardHostname),
357+
}
354358

355359
// Set the prometheus metric correctly.
356360
switch status.Status {

0 commit comments

Comments
 (0)