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

Commit0374af2

Browse files
authored
fix(security)!: path-based app sharing changes (#5772)
This commit disables path-based app sharing by default. It is possiblefor a workspace app on a path (not a subdomain) to make API requests tothe Coder API. When accessing your own workspace, this is not much of aproblem. When accessing a shared workspace app, the workspace ownercould include malicious javascript in the page that makes requests tothe Coder API on behalf of the visitor.This vulnerability does not affect subdomain apps.- Disables path-based app sharing by default. Previous behavior can be restored using the `--dangerous-allow-path-app-sharing` flag which is not recommended.- Disables users with the site "owner" role from accessing path-based apps from workspaces they do not own. Previous behavior can be restored using the `--dangerous-allow-path-app-site-owner-access` flag which is not recommended.- Adds a flag `--disable-path-apps` which can be used by security-conscious admins to disable all path-based apps across the entire deployment. This check is enforced at app-access time, not at template-ingest time.
1 parentb42e2ae commit0374af2

File tree

10 files changed

+506
-78
lines changed

10 files changed

+506
-78
lines changed

‎cli/deployment/config.go‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -500,6 +500,26 @@ func newConfig() *codersdk.DeploymentConfig {
500500
Default:"",
501501
},
502502
},
503+
Dangerous:&codersdk.DangerousConfig{
504+
AllowPathAppSharing:&codersdk.DeploymentConfigField[bool]{
505+
Name:"DANGEROUS: Allow Path App Sharing",
506+
Usage:"Allow workspace apps that are not served from subdomains to be shared. Path-based app sharing is DISABLED by default for security purposes. Path-based apps can make requests to the Coder API and pose a security risk when the workspace serves malicious JavaScript. Path-based apps can be disabled entirely with --disable-path-apps for further security.",
507+
Flag:"dangerous-allow-path-app-sharing",
508+
Default:false,
509+
},
510+
AllowPathAppSiteOwnerAccess:&codersdk.DeploymentConfigField[bool]{
511+
Name:"DANGEROUS: Allow Site Owners to Access Path Apps",
512+
Usage:"Allow site-owners to access workspace apps from workspaces they do not own. Owners cannot access path-based apps they do not own by default. Path-based apps can make requests to the Coder API and pose a security risk when the workspace serves malicious JavaScript. Path-based apps can be disabled entirely with --disable-path-apps for further security.",
513+
Flag:"dangerous-allow-path-app-site-owner-access",
514+
Default:false,
515+
},
516+
},
517+
DisablePathApps:&codersdk.DeploymentConfigField[bool]{
518+
Name:"Disable Path Apps",
519+
Usage:"Disable workspace apps that are not served from subdomains. Path-based apps can make requests to the Coder API and pose a security risk when the workspace serves malicious JavaScript. This is recommended for security purposes if a --wildcard-access-url is configured.",
520+
Flag:"disable-path-apps",
521+
Default:false,
522+
},
503523
}
504524
}
505525

‎cli/testdata/coder_server_--help.golden‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,28 @@ Flags:
2929
with systemd.
3030
Consumes $CODER_CACHE_DIRECTORY (default
3131
"/tmp/coder-cli-test-cache")
32+
--dangerous-allow-path-app-sharing Allow workspace apps that are not served
33+
from subdomains to be shared. Path-based
34+
app sharing is DISABLED by default for
35+
security purposes. Path-based apps can
36+
make requests to the Coder API and pose a
37+
security risk when the workspace serves
38+
malicious JavaScript. Path-based apps can
39+
be disabled entirely with
40+
--disable-path-apps for further security.
41+
Consumes
42+
$CODER_DANGEROUS_ALLOW_PATH_APP_SHARING
43+
--dangerous-allow-path-app-site-owner-access Allow site-owners to access workspace
44+
apps from workspaces they do not own.
45+
Owners cannot access path-based apps they
46+
do not own by default. Path-based apps
47+
can make requests to the Coder API and
48+
pose a security risk when the workspace
49+
serves malicious JavaScript. Path-based
50+
apps can be disabled entirely with
51+
--disable-path-apps for further security.
52+
Consumes
53+
$CODER_DANGEROUS_ALLOW_PATH_APP_SITE_OWNER_ACCESS
3254
--dangerous-disable-rate-limits Disables all rate limits. This is not
3355
recommended in production.
3456
Consumes $CODER_RATE_LIMIT_DISABLE_ALL
@@ -61,6 +83,14 @@ Flags:
6183
Consumes
6284
$CODER_DERP_SERVER_STUN_ADDRESSES
6385
(default [stun.l.google.com:19302])
86+
--disable-path-apps Disable workspace apps that are not
87+
served from subdomains. Path-based apps
88+
can make requests to the Coder API and
89+
pose a security risk when the workspace
90+
serves malicious JavaScript. This is
91+
recommended for security purposes if a
92+
--wildcard-access-url is configured.
93+
Consumes $CODER_DISABLE_PATH_APPS
6494
--experiments strings Enable one or more experiments. These are
6595
not ready for production. Separate
6696
multiple experiments with commas, or

‎coderd/apidoc/docs.go‎

Lines changed: 17 additions & 0 deletions
Some generated files are not rendered by default. Learn more aboutcustomizing how changed files appear on GitHub.

‎coderd/apidoc/swagger.json‎

Lines changed: 17 additions & 0 deletions
Some generated files are not rendered by default. Learn more aboutcustomizing how changed files appear on GitHub.

‎coderd/workspaceapps.go‎

Lines changed: 72 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,13 @@ var nonCanonicalHeaders = map[string]string{
6565
"Sec-Websocket-Version":"Sec-WebSocket-Version",
6666
}
6767

68+
typeworkspaceAppAccessMethodstring
69+
70+
const (
71+
workspaceAppAccessMethodPathworkspaceAppAccessMethod="path"
72+
workspaceAppAccessMethodSubdomainworkspaceAppAccessMethod="subdomain"
73+
)
74+
6875
// @Summary Get applications host
6976
// @ID get-applications-host
7077
// @Security CoderSessionToken
@@ -89,6 +96,17 @@ func (api *API) workspaceAppsProxyPath(rw http.ResponseWriter, r *http.Request)
8996
workspace:=httpmw.WorkspaceParam(r)
9097
agent:=httpmw.WorkspaceAgentParam(r)
9198

99+
ifapi.DeploymentConfig.DisablePathApps.Value {
100+
site.RenderStaticErrorPage(rw,r, site.ErrorPageData{
101+
Status:http.StatusUnauthorized,
102+
Title:"Unauthorized",
103+
Description:"Path-based applications are disabled on this Coder deployment by the administrator.",
104+
RetryEnabled:false,
105+
DashboardURL:api.AccessURL.String(),
106+
})
107+
return
108+
}
109+
92110
// We do not support port proxying on paths, so lookup the app by slug.
93111
appSlug:=chi.URLParam(r,"workspaceapp")
94112
app,ok:=api.lookupWorkspaceApp(rw,r,agent.ID,appSlug)
@@ -100,7 +118,7 @@ func (api *API) workspaceAppsProxyPath(rw http.ResponseWriter, r *http.Request)
100118
ifapp.SharingLevel!="" {
101119
appSharingLevel=app.SharingLevel
102120
}
103-
authed,ok:=api.fetchWorkspaceApplicationAuth(rw,r,workspace,appSharingLevel)
121+
authed,ok:=api.fetchWorkspaceApplicationAuth(rw,r,workspaceAppAccessMethodPath,workspace,appSharingLevel)
104122
if!ok {
105123
return
106124
}
@@ -127,11 +145,12 @@ func (api *API) workspaceAppsProxyPath(rw http.ResponseWriter, r *http.Request)
127145
}
128146

129147
api.proxyWorkspaceApplication(proxyApplication{
130-
Workspace:workspace,
131-
Agent:agent,
132-
App:&app,
133-
Port:0,
134-
Path:chiPath,
148+
AccessMethod:workspaceAppAccessMethodPath,
149+
Workspace:workspace,
150+
Agent:agent,
151+
App:&app,
152+
Port:0,
153+
Path:chiPath,
135154
},rw,r)
136155
}
137156

@@ -238,11 +257,12 @@ func (api *API) handleSubdomainApplications(middlewares ...func(http.Handler) ht
238257
}
239258

240259
api.proxyWorkspaceApplication(proxyApplication{
241-
Workspace:workspace,
242-
Agent:agent,
243-
App:workspaceAppPtr,
244-
Port:app.Port,
245-
Path:r.URL.Path,
260+
AccessMethod:workspaceAppAccessMethodSubdomain,
261+
Workspace:workspace,
262+
Agent:agent,
263+
App:workspaceAppPtr,
264+
Port:app.Port,
265+
Path:r.URL.Path,
246266
},rw,r)
247267
})).ServeHTTP(rw,r.WithContext(ctx))
248268
})
@@ -411,9 +431,25 @@ func (api *API) lookupWorkspaceApp(rw http.ResponseWriter, r *http.Request, agen
411431
returnapp,true
412432
}
413433

414-
func (api*API)authorizeWorkspaceApp(r*http.Request,sharingLevel database.AppSharingLevel,workspace database.Workspace) (bool,error) {
434+
//nolint:revive
435+
func (api*API)authorizeWorkspaceApp(r*http.Request,accessMethodworkspaceAppAccessMethod,sharingLevel database.AppSharingLevel,workspace database.Workspace) (bool,error) {
415436
ctx:=r.Context()
416437

438+
ifaccessMethod=="" {
439+
accessMethod=workspaceAppAccessMethodPath
440+
}
441+
isPathApp:=accessMethod==workspaceAppAccessMethodPath
442+
443+
// If path-based app sharing is disabled (which is the default), we can
444+
// force the sharing level to be "owner" so that the user can only access
445+
// their own apps.
446+
//
447+
// Site owners are blocked from accessing path-based apps unless the
448+
// Dangerous.AllowPathAppSiteOwnerAccess flag is enabled in the check below.
449+
ifisPathApp&&!api.DeploymentConfig.Dangerous.AllowPathAppSharing.Value {
450+
sharingLevel=database.AppSharingLevelOwner
451+
}
452+
417453
// Short circuit if not authenticated.
418454
roles,ok:=httpmw.UserAuthorizationOptional(r)
419455
if!ok {
@@ -422,6 +458,21 @@ func (api *API) authorizeWorkspaceApp(r *http.Request, sharingLevel database.App
422458
returnsharingLevel==database.AppSharingLevelPublic,nil
423459
}
424460

461+
// Block anyone from accessing workspaces they don't own in path-based apps
462+
// unless the admin disables this security feature. This blocks site-owners
463+
// from accessing any apps from any user's workspaces.
464+
//
465+
// When the Dangerous.AllowPathAppSharing flag is not enabled, the sharing
466+
// level will be forced to "owner", so this check will always be true for
467+
// workspaces owned by different users.
468+
ifisPathApp&&
469+
sharingLevel==database.AppSharingLevelOwner&&
470+
workspace.OwnerID!=roles.ID&&
471+
!api.DeploymentConfig.Dangerous.AllowPathAppSiteOwnerAccess.Value {
472+
473+
returnfalse,nil
474+
}
475+
425476
// Do a standard RBAC check. This accounts for share level "owner" and any
426477
// other RBAC rules that may be in place.
427478
//
@@ -463,8 +514,8 @@ func (api *API) authorizeWorkspaceApp(r *http.Request, sharingLevel database.App
463514
// for a given app share level in the given workspace. The user's authorization
464515
// status is returned. If a server error occurs, a HTML error page is rendered
465516
// and false is returned so the caller can return early.
466-
func (api*API)fetchWorkspaceApplicationAuth(rw http.ResponseWriter,r*http.Request,workspace database.Workspace,appSharingLevel database.AppSharingLevel) (authedbool,okbool) {
467-
ok,err:=api.authorizeWorkspaceApp(r,appSharingLevel,workspace)
517+
func (api*API)fetchWorkspaceApplicationAuth(rw http.ResponseWriter,r*http.Request,accessMethodworkspaceAppAccessMethod,workspace database.Workspace,appSharingLevel database.AppSharingLevel) (authedbool,okbool) {
518+
ok,err:=api.authorizeWorkspaceApp(r,accessMethod,appSharingLevel,workspace)
468519
iferr!=nil {
469520
api.Logger.Error(r.Context(),"authorize workspace app",slog.Error(err))
470521
site.RenderStaticErrorPage(rw,r, site.ErrorPageData{
@@ -484,8 +535,8 @@ func (api *API) fetchWorkspaceApplicationAuth(rw http.ResponseWriter, r *http.Re
484535
// for a given app share level in the given workspace. If the user is not
485536
// authorized or a server error occurs, a discrete HTML error page is rendered
486537
// and false is returned so the caller can return early.
487-
func (api*API)checkWorkspaceApplicationAuth(rw http.ResponseWriter,r*http.Request,workspace database.Workspace,appSharingLevel database.AppSharingLevel)bool {
488-
authed,ok:=api.fetchWorkspaceApplicationAuth(rw,r,workspace,appSharingLevel)
538+
func (api*API)checkWorkspaceApplicationAuth(rw http.ResponseWriter,r*http.Request,accessMethodworkspaceAppAccessMethod,workspace database.Workspace,appSharingLevel database.AppSharingLevel)bool {
539+
authed,ok:=api.fetchWorkspaceApplicationAuth(rw,r,accessMethod,workspace,appSharingLevel)
489540
if!ok {
490541
returnfalse
491542
}
@@ -502,7 +553,7 @@ func (api *API) checkWorkspaceApplicationAuth(rw http.ResponseWriter, r *http.Re
502553
// they will be redirected to the route below. If the user does have a session
503554
// key but insufficient permissions a static error page will be rendered.
504555
func (api*API)verifyWorkspaceApplicationSubdomainAuth(rw http.ResponseWriter,r*http.Request,hoststring,workspace database.Workspace,appSharingLevel database.AppSharingLevel)bool {
505-
authed,ok:=api.fetchWorkspaceApplicationAuth(rw,r,workspace,appSharingLevel)
556+
authed,ok:=api.fetchWorkspaceApplicationAuth(rw,r,workspaceAppAccessMethodSubdomain,workspace,appSharingLevel)
506557
if!ok {
507558
returnfalse
508559
}
@@ -731,8 +782,9 @@ func (api *API) workspaceApplicationAuth(rw http.ResponseWriter, r *http.Request
731782

732783
// proxyApplication are the required fields to proxy a workspace application.
733784
typeproxyApplicationstruct {
734-
Workspace database.Workspace
735-
Agent database.WorkspaceAgent
785+
AccessMethodworkspaceAppAccessMethod
786+
Workspace database.Workspace
787+
Agent database.WorkspaceAgent
736788

737789
// Either App or Port must be set, but not both.
738790
App*database.WorkspaceApp
@@ -752,7 +804,7 @@ func (api *API) proxyWorkspaceApplication(proxyApp proxyApplication, rw http.Res
752804
ifproxyApp.App!=nil&&proxyApp.App.SharingLevel!="" {
753805
sharingLevel=proxyApp.App.SharingLevel
754806
}
755-
if!api.checkWorkspaceApplicationAuth(rw,r,proxyApp.Workspace,sharingLevel) {
807+
if!api.checkWorkspaceApplicationAuth(rw,r,proxyApp.AccessMethod,proxyApp.Workspace,sharingLevel) {
756808
return
757809
}
758810

0 commit comments

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