Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit82c14e0

Browse files
authored
feat: add csp headers for embedded apps (#18374)
I modified the proxy host cache we already had and were using forwebsocket csp headers to also include the wildcard app host, then usedthose for frame-src policies.I did not add frame-ancestors, since if I understand correctly, thosewould go on the app, and this middleware does not come into play there.Maybe we will want to add it on workspace apps like we do with cors, ifwe find apps are setting it to `none` or something.Closescoder/internal#684
1 parentaee96c9 commit82c14e0

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
ifapi.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.Host,api.AppHostname),
1554+
},
15411555
}
15421556
iff:=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+
returnproxies
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 supportedexternal websockethosts.
51-
// This is to support the terminal connecting to a workspace proxy.
52-
//The origin of the terminal request does not match the urlof 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 supportedproxyhosts
52+
//(including the primary).This is to support the terminal connecting to a
53+
//workspace proxy and for embedding apps in an iframe. The originof 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 currentavailable 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-
funcCSPHeaders(experiments codersdk.Experiments,telemetrybool,websocketHostsfunc() []string,staticAdditionsmap[CSPFetchDirective][]string)func(next http.Handler) http.Handler {
62+
funcCSPHeaders(experiments codersdk.Experiments,telemetrybool,proxyHostsfunc() []*proxyhealth.ProxyHost,staticAdditionsmap[CSPFetchDirective][]string)func(next http.Handler) http.Handler {
6263
returnfunc(next http.Handler) http.Handler {
6364
returnhttp.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-
ifexperiments.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
iftelemetry {
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 terminalrequires a websocket connection to theworkspaceproxy.
130-
// Make sure we allowthis connection to healthy proxies.
131-
extraConnect:=websocketHosts()
121+
// The terminaland iframed apps can useworkspaceproxies (which includes
122+
//the primary).Make sure we allowconnections to healthy proxies.
123+
extraConnect:=proxyHosts()
132124
iflen(extraConnect)>0 {
133125
for_,extraHost:=rangeextraConnect {
134-
ifextraHost=="*" {
126+
// Allow embedding the app host.
127+
ifexperiments.Enabled(codersdk.ExperimentAITasks) {
128+
cspSrcs.Append(CSPDirectiveFrameSrc,extraHost.AppHost)
129+
}
130+
ifextraHost.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+
ifextraHost.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-
funcTestCSPConnect(t*testing.T) {
16+
funcTestCSP(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-
returnexpected
53+
httpmw.CSPHeaders(codersdk.Experiments{
54+
codersdk.ExperimentAITasks,
55+
},false,func() []*proxyhealth.ProxyHost {
56+
returnproxyHosts
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:=rangeexpected {
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:=rangeexpectedMedia {
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+
typeProxyHoststruct {
4+
// Host is the root host of the proxy.
5+
Hoststring
6+
// AppHost is the wildcard host where apps are hosted.
7+
AppHoststring
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
returnmatches[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+
funcConvertAppHostForCSP(host,wildcardstring)string {
297+
ifwildcard=="" {
298+
returnhost
299+
}
300+
parts:=strings.Split(wildcard,".")
301+
fori,part:=rangeparts {
302+
ifstrings.Contains(part,"*") {
303+
// The wildcard can only be in the first section.
304+
ifi!=0 {
305+
returnhost
306+
}
307+
parts[i]="*"
308+
}
309+
}
310+
returnstrings.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+
funcTestConvertAppURLForCSP(t*testing.T) {
415+
t.Parallel()
416+
417+
testCases:= []struct {
418+
namestring
419+
hoststring
420+
wildcardstring
421+
expectedstring
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:=rangetestCases {
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(statusesmap[uuid.UUID]ProxyStatus) {
147-
varproxyHosts []string
149+
varproxyHosts []*agplproxyhealth.ProxyHost
148150
for_,s:=rangestatuses {
149-
ifs.ProxyHost!="" {
151+
ifs.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-
ProxyHoststring
195+
// ProxyHost is thebasehost:portand app hostof the proxy. This is included
196+
//in the statusto make sure the proxy url is a valid URL. It also makes it
197+
//easier toescalate errors if the url.Parse errors (should never happen).
198+
ProxyHost*agplproxyhealth.ProxyHost
197199
StatusStatus
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
ifptr==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
switchstatus.Status {

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp