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

Commit42dd544

Browse files
authored
fix: use unique cookies for workspace proxies (#19930)
There is currently an issue with subdomain workspace apps on workspaceproxies, where if you have a workspace proxy wildcard nested beneath theprimary wildcard, cookies from the primary may be sent to the serverbefore cookies from the proxy specifically.Currently:1. Use a subdomain app via the primary proxy `*.coder.corp.com` a. Client sends no cookies a. Server does token smuggling flowa. Server sets a cookie `coder_subdomain_app_session_token` on`*.coder.corp.com` a. Server redirects client to reload the page a. Request should succeed as usual1. Wait until the primary proxy's session token cookie has expired inthe database (or make it invalid yourself)1. Use a subdomain app via a separate proxy `*.sydney.coder.corp.com`a. Client sends `coder_subdomain_app_session_token` cookie from`*.coder.corp.com` a. Server validates supplied cookie, it fails because it's expired a. Server does token smuggling flowa. Server sets a cookie `coder_subdomain_app_session_token` on`*.sydney.coder.corp.com` a. Server redirects client to reload page a. Client sends BOTH cookies.a. The server will only process the first cookie it receives, so if theexpired cookie for the primary proxy is sent first the request will endup in a permanent loop on step b.The fix is to append `_{hash(wildcard_access_url)}` to the subdomaincookies as we cannot control browser behavior further. This avoids theconflict as each proxy will only read it's specific cookie.
1 parent7baaa16 commit42dd544

File tree

11 files changed

+161
-75
lines changed

11 files changed

+161
-75
lines changed

‎coderd/coderd.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -785,7 +785,7 @@ func New(options *Options) *API {
785785
options.WorkspaceAppsStatsCollectorOptions.Reporter=api.statsReporter
786786
}
787787

788-
api.workspaceAppServer=&workspaceapps.Server{
788+
api.workspaceAppServer=workspaceapps.NewServer(workspaceapps.ServerOptions{
789789
Logger:workspaceAppsLogger,
790790

791791
DashboardURL:api.AccessURL,
@@ -799,9 +799,9 @@ func New(options *Options) *API {
799799
StatsCollector:workspaceapps.NewStatsCollector(options.WorkspaceAppsStatsCollectorOptions),
800800

801801
DisablePathApps:options.DeploymentValues.DisablePathApps.Value(),
802-
Cookies:options.DeploymentValues.HTTPCookies,
802+
CookiesConfig:options.DeploymentValues.HTTPCookies,
803803
APIKeyEncryptionKeycache:options.AppEncryptionKeyCache,
804-
}
804+
})
805805

806806
apiKeyMiddleware:=httpmw.ExtractAPIKeyMW(httpmw.ExtractAPIKeyConfig{
807807
DB:options.Database,

‎coderd/httpapi/cookie.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,11 @@ func StripCoderCookies(header string) string {
2424
name==codersdk.OAuth2StateCookie||
2525
name==codersdk.OAuth2RedirectCookie||
2626
name==codersdk.PathAppSessionTokenCookie||
27-
name==codersdk.SubdomainAppSessionTokenCookie||
27+
// This uses a prefix check because the subdomain cookie is unique
28+
// per workspace proxy and is based on a hash of the workspace proxy
29+
// subdomain hostname. See the workspaceapps package for more
30+
// details.
31+
strings.HasPrefix(name,codersdk.SubdomainAppSessionTokenCookie)||
2832
name==codersdk.SignedAppTokenCookie {
2933
continue
3034
}

‎coderd/httpapi/cookie_test.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,15 @@ func TestStripCoderCookies(t *testing.T) {
2525
}, {
2626
"coder_session_token=ok; oauth_state=wow; oauth_redirect=/",
2727
"",
28+
}, {
29+
"coder_path_app_session_token=ok; wow=test",
30+
"wow=test",
31+
}, {
32+
"coder_subdomain_app_session_token=ok; coder_subdomain_app_session_token_1234567890=ok; wow=test",
33+
"wow=test",
34+
}, {
35+
"coder_signed_app_token=ok; wow=test",
36+
"wow=test",
2837
}} {
2938
t.Run(tc.Input,func(t*testing.T) {
3039
t.Parallel()

‎coderd/workspaceapps/apptest/apptest.go‎

Lines changed: 16 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -264,14 +264,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
264264
require.Equal(t,proxyTestAppBody,string(body))
265265
require.Equal(t,http.StatusOK,resp.StatusCode)
266266

267-
varappTokenCookie*http.Cookie
268-
for_,c:=rangeresp.Cookies() {
269-
ifc.Name==codersdk.SignedAppTokenCookie {
270-
appTokenCookie=c
271-
break
272-
}
273-
}
274-
require.NotNil(t,appTokenCookie,"no signed app token cookie in response")
267+
appTokenCookie:=mustFindCookie(t,resp.Cookies(),codersdk.SignedAppTokenCookie)
275268
require.Equal(t,appTokenCookie.Path,u.Path,"incorrect path on app token cookie")
276269

277270
// Ensure the signed app token cookie is valid.
@@ -310,14 +303,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
310303
require.Equal(t,proxyTestAppBody,string(body))
311304
require.Equal(t,http.StatusOK,resp.StatusCode)
312305

313-
varappTokenCookie*http.Cookie
314-
for_,c:=rangeresp.Cookies() {
315-
ifc.Name==codersdk.SignedAppTokenCookie {
316-
appTokenCookie=c
317-
break
318-
}
319-
}
320-
require.NotNil(t,appTokenCookie,"no signed app token cookie in response")
306+
appTokenCookie:=mustFindCookie(t,resp.Cookies(),codersdk.SignedAppTokenCookie)
321307
require.Equal(t,appTokenCookie.Path,u.Path,"incorrect path on app token cookie")
322308

323309
// Ensure the signed app token cookie is valid.
@@ -426,8 +412,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
426412
require.Equal(t,proxyTestAppBody,string(body))
427413
require.Equal(t,http.StatusOK,resp.StatusCode)
428414

429-
appTokenCookie:=findCookie(resp.Cookies(),codersdk.SignedAppTokenCookie)
430-
require.NotNil(t,appTokenCookie,"no signed app token cookie in response")
415+
appTokenCookie:=mustFindCookie(t,resp.Cookies(),codersdk.SignedAppTokenCookie)
431416
require.Equal(t,appTokenCookie.Path,u.Path,"incorrect path on app token cookie")
432417

433418
object,err:=jose.ParseSigned(appTokenCookie.Value, []jose.SignatureAlgorithm{jwtutils.SigningAlgo})
@@ -467,7 +452,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
467452
assertWorkspaceLastUsedAtUpdated(ctx,t,appDetails)
468453

469454
// Since the old token is invalid, the signed app token cookie should have a new value.
470-
newTokenCookie:=findCookie(resp.Cookies(),codersdk.SignedAppTokenCookie)
455+
newTokenCookie:=mustFindCookie(t,resp.Cookies(),codersdk.SignedAppTokenCookie)
471456
require.NotEqual(t,appTokenCookie.Value,newTokenCookie.Value)
472457
})
473458
})
@@ -978,15 +963,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
978963
resp.Body.Close()
979964
require.Equal(t,http.StatusSeeOther,resp.StatusCode)
980965

981-
cookies:=resp.Cookies()
982-
varcookie*http.Cookie
983-
for_,co:=rangecookies {
984-
ifco.Name==c.sessionTokenCookieName {
985-
cookie=co
986-
break
987-
}
988-
}
989-
require.NotNil(t,cookie,"no app session token cookie was set")
966+
cookie:=mustFindCookie(t,resp.Cookies(),c.sessionTokenCookieName)
990967
apiKey:=cookie.Value
991968

992969
// Fetch the API key from the API.
@@ -1102,14 +1079,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
11021079

11031080
// Parse the returned signed token to verify that it contains the
11041081
// prefix.
1105-
varappTokenCookie*http.Cookie
1106-
for_,c:=rangeresp.Cookies() {
1107-
ifc.Name==codersdk.SignedAppTokenCookie {
1108-
appTokenCookie=c
1109-
break
1110-
}
1111-
}
1112-
require.NotNil(t,appTokenCookie,"no signed app token cookie in response")
1082+
appTokenCookie:=mustFindCookie(t,resp.Cookies(),codersdk.SignedAppTokenCookie)
11131083

11141084
// Parse the JWT without verifying it (since we can't access the key
11151085
// from this test).
@@ -1334,14 +1304,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
13341304
require.Equal(t,proxyTestAppBody,string(body))
13351305
require.Equal(t,http.StatusOK,resp.StatusCode)
13361306

1337-
varappTokenCookie*http.Cookie
1338-
for_,c:=rangeresp.Cookies() {
1339-
ifc.Name==codersdk.SignedAppTokenCookie {
1340-
appTokenCookie=c
1341-
break
1342-
}
1343-
}
1344-
require.NotNil(t,appTokenCookie,"no signed token cookie in response")
1307+
appTokenCookie:=mustFindCookie(t,resp.Cookies(),codersdk.SignedAppTokenCookie)
13451308
require.Equal(t,appTokenCookie.Path,"/","incorrect path on signed token cookie")
13461309

13471310
// Ensure the signed app token cookie is valid.
@@ -1589,8 +1552,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
15891552
require.Equal(t,proxyTestAppBody,string(body))
15901553
require.Equal(t,http.StatusOK,resp.StatusCode)
15911554

1592-
appTokenCookie:=findCookie(resp.Cookies(),codersdk.SignedAppTokenCookie)
1593-
require.NotNil(t,appTokenCookie,"no signed token cookie in response")
1555+
appTokenCookie:=mustFindCookie(t,resp.Cookies(),codersdk.SignedAppTokenCookie)
15941556
require.Equal(t,appTokenCookie.Path,"/","incorrect path on signed token cookie")
15951557

15961558
object,err:=jose.ParseSigned(appTokenCookie.Value, []jose.SignatureAlgorithm{jwtutils.SigningAlgo})
@@ -1614,7 +1576,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
16141576
[]*http.Cookie{
16151577
appTokenCookie,
16161578
{
1617-
Name:codersdk.SubdomainAppSessionTokenCookie,
1579+
Name:codersdk.SessionTokenCookie,
16181580
Value:apiKey,
16191581
},
16201582
},
@@ -1631,7 +1593,7 @@ func Run(t *testing.T, appHostIsPrimary bool, factory DeploymentFactory) {
16311593
assertWorkspaceLastUsedAtUpdated(ctx,t,appDetails)
16321594

16331595
// Since the old token is invalid, the signed app token cookie should have a new value.
1634-
newTokenCookie:=findCookie(resp.Cookies(),codersdk.SignedAppTokenCookie)
1596+
newTokenCookie:=mustFindCookie(t,resp.Cookies(),codersdk.SignedAppTokenCookie)
16351597
require.NotEqual(t,appTokenCookie.Value,newTokenCookie.Value)
16361598
})
16371599
})
@@ -2542,11 +2504,15 @@ func generateBadJWT(t *testing.T, claims interface{}) string {
25422504
returncompact
25432505
}
25442506

2545-
funcfindCookie(cookies []*http.Cookie,namestring)*http.Cookie {
2507+
funcmustFindCookie(t*testing.T,cookies []*http.Cookie,prefixstring)*http.Cookie {
2508+
t.Helper()
25462509
for_,cookie:=rangecookies {
2547-
ifcookie.Name==name {
2510+
t.Logf("testing cookie against prefix %q: %q",prefix,cookie.Name)
2511+
ifstrings.HasPrefix(cookie.Name,prefix) {
2512+
t.Logf("cookie %q found",cookie.Name)
25482513
returncookie
25492514
}
25502515
}
2516+
t.Fatalf("cookie with prefix %q not found",prefix)
25512517
returnnil
25522518
}

‎coderd/workspaceapps/cookies.go‎

Lines changed: 50 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,62 @@
11
package workspaceapps
22

33
import (
4+
"crypto/sha256"
5+
"encoding/hex"
46
"net/http"
57

68
"github.com/coder/coder/v2/coderd/httpmw"
79
"github.com/coder/coder/v2/codersdk"
810
)
911

10-
// AppConnectSessionTokenCookieName returns the cookie name for the session
12+
typeAppCookiesstruct {
13+
PathAppSessionTokenstring
14+
SubdomainAppSessionTokenstring
15+
SignedAppTokenstring
16+
}
17+
18+
// NewAppCookies returns the cookie names for the app session token for the
19+
// given hostname. The subdomain cookie is unique per workspace proxy and is
20+
// based on a hash of the workspace proxy subdomain hostname. See
21+
// SubdomainAppSessionTokenCookie for more details.
22+
funcNewAppCookies(hostnamestring)AppCookies {
23+
returnAppCookies{
24+
PathAppSessionToken:codersdk.PathAppSessionTokenCookie,
25+
SubdomainAppSessionToken:SubdomainAppSessionTokenCookie(hostname),
26+
SignedAppToken:codersdk.SignedAppTokenCookie,
27+
}
28+
}
29+
30+
// CookieNameForAccessMethod returns the cookie name for the long-lived session
1131
// token for the given access method.
12-
funcAppConnectSessionTokenCookieName(accessMethodAccessMethod)string {
32+
func(cAppCookies)CookieNameForAccessMethod(accessMethodAccessMethod)string {
1333
ifaccessMethod==AccessMethodSubdomain {
14-
returncodersdk.SubdomainAppSessionTokenCookie
34+
returnc.SubdomainAppSessionToken
1535
}
16-
returncodersdk.PathAppSessionTokenCookie
36+
// Path-based and terminal apps are on the same domain:
37+
returnc.PathAppSessionToken
38+
}
39+
40+
// SubdomainAppSessionTokenCookie returns the cookie name for the subdomain app
41+
// session token. This is unique per workspace proxy and is based on a hash of
42+
// the workspace proxy subdomain hostname.
43+
//
44+
// The reason the cookie needs to be unique per workspace proxy is to avoid
45+
// cookies from one proxy (e.g. the primary) being sent on requests to a
46+
// different proxy underneath the wildcard.
47+
//
48+
// E.g. `*.dev.coder.com` and `*.sydney.dev.coder.com`
49+
//
50+
// If you have an expired cookie on the primary proxy (valid for
51+
// `*.dev.coder.com`), your browser will send it on all requests to the Sydney
52+
// proxy as it's underneath the wildcard.
53+
//
54+
// By using a unique cookie name per workspace proxy, we can avoid this issue.
55+
funcSubdomainAppSessionTokenCookie(hostnamestring)string {
56+
hash:=sha256.Sum256([]byte(hostname))
57+
// 16 bytes of uniqueness is probably enough.
58+
str:=hex.EncodeToString(hash[:16])
59+
returncodersdk.SubdomainAppSessionTokenCookie+"_"+str
1760
}
1861

1962
// AppConnectSessionTokenFromRequest returns the session token from the request
@@ -27,22 +70,22 @@ func AppConnectSessionTokenCookieName(accessMethod AccessMethod) string {
2770
// We use different cookie names for:
2871
// - path apps on primary access URL: coder_session_token
2972
// - path apps on proxies: coder_path_app_session_token
30-
// - subdomain apps:coder_subdomain_app_session_token
73+
// - subdomain apps:coder_subdomain_app_session_token_{unique_hash}
3174
//
3275
// First we try the default function to get a token from request, which supports
3376
// query parameters, the Coder-Session-Token header and the coder_session_token
3477
// cookie.
3578
//
3679
// Then we try the specific cookie name for the access method.
37-
funcAppConnectSessionTokenFromRequest(r*http.Request,accessMethodAccessMethod)string {
80+
func(cAppCookies)TokenFromRequest(r*http.Request,accessMethodAccessMethod)string {
3881
// Try the default function first.
3982
token:=httpmw.APITokenFromRequest(r)
4083
iftoken!="" {
4184
returntoken
4285
}
4386

4487
// Then try the specific cookie name for the access method.
45-
cookie,err:=r.Cookie(AppConnectSessionTokenCookieName(accessMethod))
88+
cookie,err:=r.Cookie(c.CookieNameForAccessMethod(accessMethod))
4689
iferr==nil&&cookie.Value!="" {
4790
returncookie.Value
4891
}
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
package workspaceapps_test
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/require"
7+
8+
"github.com/coder/coder/v2/coderd/workspaceapps"
9+
"github.com/coder/coder/v2/codersdk"
10+
)
11+
12+
funcTestAppCookies(t*testing.T) {
13+
t.Parallel()
14+
15+
const (
16+
domain="example.com"
17+
hash="a379a6f6eeafb9a55e378c118034e275"
18+
expectedSubdomainCookie=codersdk.SubdomainAppSessionTokenCookie+"_"+hash
19+
)
20+
21+
cookies:=workspaceapps.NewAppCookies(domain)
22+
require.Equal(t,codersdk.PathAppSessionTokenCookie,cookies.PathAppSessionToken)
23+
require.Equal(t,expectedSubdomainCookie,cookies.SubdomainAppSessionToken)
24+
require.Equal(t,codersdk.SignedAppTokenCookie,cookies.SignedAppToken)
25+
26+
require.Equal(t,cookies.PathAppSessionToken,cookies.CookieNameForAccessMethod(workspaceapps.AccessMethodPath))
27+
require.Equal(t,cookies.PathAppSessionToken,cookies.CookieNameForAccessMethod(workspaceapps.AccessMethodTerminal))
28+
require.Equal(t,cookies.SubdomainAppSessionToken,cookies.CookieNameForAccessMethod(workspaceapps.AccessMethodSubdomain))
29+
30+
// A new cookies object with a different domain should have a different
31+
// subdomain cookie.
32+
newCookies:=workspaceapps.NewAppCookies("different.com")
33+
require.NotEqual(t,cookies.SubdomainAppSessionToken,newCookies.SubdomainAppSessionToken)
34+
}

‎coderd/workspaceapps/db_test.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1236,6 +1236,15 @@ func workspaceappsResolveRequest(t testing.TB, connLogger connectionlog.Connecti
12361236
ifopts.SignedTokenProvider!=nil&&connLogger!=nil {
12371237
opts.SignedTokenProvider=signedTokenProviderWithConnLogger(t,opts.SignedTokenProvider,connLogger,time.Hour)
12381238
}
1239+
ifopts.Cookies.PathAppSessionToken=="" {
1240+
opts.Cookies.PathAppSessionToken=codersdk.PathAppSessionTokenCookie
1241+
}
1242+
ifopts.Cookies.SubdomainAppSessionToken=="" {
1243+
opts.Cookies.SubdomainAppSessionToken=codersdk.SubdomainAppSessionTokenCookie+"_test"
1244+
}
1245+
ifopts.Cookies.SignedAppToken=="" {
1246+
opts.Cookies.SignedAppToken=codersdk.SignedAppTokenCookie
1247+
}
12391248

12401249
tracing.StatusWriterMiddleware(http.HandlerFunc(func(w http.ResponseWriter,r*http.Request) {
12411250
httpmw.AttachRequestID(http.HandlerFunc(func(w http.ResponseWriter,r*http.Request) {

‎coderd/workspaceapps/provider.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ const (
2222
typeResolveRequestOptionsstruct {
2323
Logger slog.Logger
2424
SignedTokenProviderSignedTokenProvider
25+
CookiesAppCookies
2526
CookieCfg codersdk.HTTPCookieConfig
2627

2728
DashboardURL*url.URL
@@ -58,7 +59,7 @@ func ResolveRequest(rw http.ResponseWriter, r *http.Request, opts ResolveRequest
5859
AppRequest:appReq,
5960
PathAppBaseURL:opts.PathAppBaseURL.String(),
6061
AppHostname:opts.AppHostname,
61-
SessionToken:AppConnectSessionTokenFromRequest(r,appReq.AccessMethod),
62+
SessionToken:opts.Cookies.TokenFromRequest(r,appReq.AccessMethod),
6263
AppPath:opts.AppPath,
6364
AppQuery:opts.AppQuery,
6465
}

0 commit comments

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