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

fix(security)!: path-based app sharing changes#5772

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
deansheather merged 5 commits intomainfromdean/path-app-security
Jan 18, 2023
Merged
Show file tree
Hide file tree
Changes fromall 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
20 changes: 20 additions & 0 deletionscli/deployment/config.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -500,6 +500,26 @@ func newConfig() *codersdk.DeploymentConfig {
Default: "",
},
},
Dangerous: &codersdk.DangerousConfig{
AllowPathAppSharing: &codersdk.DeploymentConfigField[bool]{
Name: "DANGEROUS: Allow Path App Sharing",
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.",
Flag: "dangerous-allow-path-app-sharing",
Default: false,
},
AllowPathAppSiteOwnerAccess: &codersdk.DeploymentConfigField[bool]{
Name: "DANGEROUS: Allow Site Owners to Access Path Apps",
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.",
Flag: "dangerous-allow-path-app-site-owner-access",
Default: false,
},
},
DisablePathApps: &codersdk.DeploymentConfigField[bool]{
Name: "Disable Path Apps",
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.",
Flag: "disable-path-apps",
Default: false,
},
}
}

Expand Down
30 changes: 30 additions & 0 deletionscli/testdata/coder_server_--help.golden
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -29,6 +29,28 @@ Flags:
with systemd.
Consumes $CODER_CACHE_DIRECTORY (default
"/tmp/coder-cli-test-cache")
--dangerous-allow-path-app-sharing 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.
Consumes
$CODER_DANGEROUS_ALLOW_PATH_APP_SHARING
--dangerous-allow-path-app-site-owner-access 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.
Consumes
$CODER_DANGEROUS_ALLOW_PATH_APP_SITE_OWNER_ACCESS
--dangerous-disable-rate-limits Disables all rate limits. This is not
recommended in production.
Consumes $CODER_RATE_LIMIT_DISABLE_ALL
Expand DownExpand Up@@ -61,6 +83,14 @@ Flags:
Consumes
$CODER_DERP_SERVER_STUN_ADDRESSES
(default [stun.l.google.com:19302])
--disable-path-apps 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.
Consumes $CODER_DISABLE_PATH_APPS
--experiments strings Enable one or more experiments. These are
not ready for production. Separate
multiple experiments with commas, or
Expand Down
17 changes: 17 additions & 0 deletionscoderd/apidoc/docs.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more abouthow customized files appear on GitHub.

17 changes: 17 additions & 0 deletionscoderd/apidoc/swagger.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more abouthow customized files appear on GitHub.

92 changes: 72 additions & 20 deletionscoderd/workspaceapps.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -65,6 +65,13 @@ var nonCanonicalHeaders = map[string]string{
"Sec-Websocket-Version": "Sec-WebSocket-Version",
}

type workspaceAppAccessMethod string

const (
workspaceAppAccessMethodPath workspaceAppAccessMethod = "path"
workspaceAppAccessMethodSubdomain workspaceAppAccessMethod = "subdomain"
)

// @Summary Get applications host
// @ID get-applications-host
// @Security CoderSessionToken
Expand All@@ -89,6 +96,17 @@ func (api *API) workspaceAppsProxyPath(rw http.ResponseWriter, r *http.Request)
workspace := httpmw.WorkspaceParam(r)
agent := httpmw.WorkspaceAgentParam(r)

if api.DeploymentConfig.DisablePathApps.Value {
site.RenderStaticErrorPage(rw, r, site.ErrorPageData{
Status: http.StatusUnauthorized,
Title: "Unauthorized",
Description: "Path-based applications are disabled on this Coder deployment by the administrator.",
RetryEnabled: false,
DashboardURL: api.AccessURL.String(),
})
return
}

// We do not support port proxying on paths, so lookup the app by slug.
appSlug := chi.URLParam(r, "workspaceapp")
app, ok := api.lookupWorkspaceApp(rw, r, agent.ID, appSlug)
Expand All@@ -100,7 +118,7 @@ func (api *API) workspaceAppsProxyPath(rw http.ResponseWriter, r *http.Request)
if app.SharingLevel != "" {
appSharingLevel = app.SharingLevel
}
authed, ok := api.fetchWorkspaceApplicationAuth(rw, r, workspace, appSharingLevel)
authed, ok := api.fetchWorkspaceApplicationAuth(rw, r,workspaceAppAccessMethodPath,workspace, appSharingLevel)
if !ok {
return
}
Expand All@@ -127,11 +145,12 @@ func (api *API) workspaceAppsProxyPath(rw http.ResponseWriter, r *http.Request)
}

api.proxyWorkspaceApplication(proxyApplication{
Workspace: workspace,
Agent: agent,
App: &app,
Port: 0,
Path: chiPath,
AccessMethod: workspaceAppAccessMethodPath,
Workspace: workspace,
Agent: agent,
App: &app,
Port: 0,
Path: chiPath,
}, rw, r)
}

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

api.proxyWorkspaceApplication(proxyApplication{
Workspace: workspace,
Agent: agent,
App: workspaceAppPtr,
Port: app.Port,
Path: r.URL.Path,
AccessMethod: workspaceAppAccessMethodSubdomain,
Workspace: workspace,
Agent: agent,
App: workspaceAppPtr,
Port: app.Port,
Path: r.URL.Path,
}, rw, r)
})).ServeHTTP(rw, r.WithContext(ctx))
})
Expand DownExpand Up@@ -411,9 +431,25 @@ func (api *API) lookupWorkspaceApp(rw http.ResponseWriter, r *http.Request, agen
return app, true
}

func (api *API) authorizeWorkspaceApp(r *http.Request, sharingLevel database.AppSharingLevel, workspace database.Workspace) (bool, error) {
//nolint:revive
func (api *API) authorizeWorkspaceApp(r *http.Request, accessMethod workspaceAppAccessMethod, sharingLevel database.AppSharingLevel, workspace database.Workspace) (bool, error) {
ctx := r.Context()

if accessMethod == "" {
accessMethod = workspaceAppAccessMethodPath
}
isPathApp := accessMethod == workspaceAppAccessMethodPath

// If path-based app sharing is disabled (which is the default), we can
// force the sharing level to be "owner" so that the user can only access
// their own apps.
//
// Site owners are blocked from accessing path-based apps unless the
// Dangerous.AllowPathAppSiteOwnerAccess flag is enabled in the check below.
if isPathApp && !api.DeploymentConfig.Dangerous.AllowPathAppSharing.Value {
sharingLevel = database.AppSharingLevelOwner
}

// Short circuit if not authenticated.
roles, ok := httpmw.UserAuthorizationOptional(r)
if !ok {
Expand All@@ -422,6 +458,21 @@ func (api *API) authorizeWorkspaceApp(r *http.Request, sharingLevel database.App
return sharingLevel == database.AppSharingLevelPublic, nil
}

// Block anyone from accessing workspaces they don't own in path-based apps
// unless the admin disables this security feature. This blocks site-owners
// from accessing any apps from any user's workspaces.
//
// When the Dangerous.AllowPathAppSharing flag is not enabled, the sharing
// level will be forced to "owner", so this check will always be true for
// workspaces owned by different users.
if isPathApp &&
sharingLevel == database.AppSharingLevelOwner &&
workspace.OwnerID != roles.ID &&
!api.DeploymentConfig.Dangerous.AllowPathAppSiteOwnerAccess.Value {

return false, nil
}

// Do a standard RBAC check. This accounts for share level "owner" and any
// other RBAC rules that may be in place.
//
Expand DownExpand Up@@ -463,8 +514,8 @@ func (api *API) authorizeWorkspaceApp(r *http.Request, sharingLevel database.App
// for a given app share level in the given workspace. The user's authorization
// status is returned. If a server error occurs, a HTML error page is rendered
// and false is returned so the caller can return early.
func (api *API) fetchWorkspaceApplicationAuth(rw http.ResponseWriter, r *http.Request, workspace database.Workspace, appSharingLevel database.AppSharingLevel) (authed bool, ok bool) {
ok, err := api.authorizeWorkspaceApp(r, appSharingLevel, workspace)
func (api *API) fetchWorkspaceApplicationAuth(rw http.ResponseWriter, r *http.Request,accessMethod workspaceAppAccessMethod,workspace database.Workspace, appSharingLevel database.AppSharingLevel) (authed bool, ok bool) {
ok, err := api.authorizeWorkspaceApp(r,accessMethod,appSharingLevel, workspace)
if err != nil {
api.Logger.Error(r.Context(), "authorize workspace app", slog.Error(err))
site.RenderStaticErrorPage(rw, r, site.ErrorPageData{
Expand All@@ -484,8 +535,8 @@ func (api *API) fetchWorkspaceApplicationAuth(rw http.ResponseWriter, r *http.Re
// for a given app share level in the given workspace. If the user is not
// authorized or a server error occurs, a discrete HTML error page is rendered
// and false is returned so the caller can return early.
func (api *API) checkWorkspaceApplicationAuth(rw http.ResponseWriter, r *http.Request, workspace database.Workspace, appSharingLevel database.AppSharingLevel) bool {
authed, ok := api.fetchWorkspaceApplicationAuth(rw, r, workspace, appSharingLevel)
func (api *API) checkWorkspaceApplicationAuth(rw http.ResponseWriter, r *http.Request,accessMethod workspaceAppAccessMethod,workspace database.Workspace, appSharingLevel database.AppSharingLevel) bool {
authed, ok := api.fetchWorkspaceApplicationAuth(rw, r,accessMethod,workspace, appSharingLevel)
if !ok {
return false
}
Expand All@@ -502,7 +553,7 @@ func (api *API) checkWorkspaceApplicationAuth(rw http.ResponseWriter, r *http.Re
// they will be redirected to the route below. If the user does have a session
// key but insufficient permissions a static error page will be rendered.
func (api *API) verifyWorkspaceApplicationSubdomainAuth(rw http.ResponseWriter, r *http.Request, host string, workspace database.Workspace, appSharingLevel database.AppSharingLevel) bool {
authed, ok := api.fetchWorkspaceApplicationAuth(rw, r, workspace, appSharingLevel)
authed, ok := api.fetchWorkspaceApplicationAuth(rw, r,workspaceAppAccessMethodSubdomain,workspace, appSharingLevel)
if !ok {
return false
}
Expand DownExpand Up@@ -731,8 +782,9 @@ func (api *API) workspaceApplicationAuth(rw http.ResponseWriter, r *http.Request

// proxyApplication are the required fields to proxy a workspace application.
type proxyApplication struct {
Workspace database.Workspace
Agent database.WorkspaceAgent
AccessMethod workspaceAppAccessMethod
Workspace database.Workspace
Agent database.WorkspaceAgent

// Either App or Port must be set, but not both.
App *database.WorkspaceApp
Expand All@@ -752,7 +804,7 @@ func (api *API) proxyWorkspaceApplication(proxyApp proxyApplication, rw http.Res
if proxyApp.App != nil && proxyApp.App.SharingLevel != "" {
sharingLevel = proxyApp.App.SharingLevel
}
if !api.checkWorkspaceApplicationAuth(rw, r, proxyApp.Workspace, sharingLevel) {
if !api.checkWorkspaceApplicationAuth(rw, r, proxyApp.AccessMethod, proxyApp.Workspace, sharingLevel) {
return
}

Expand Down
Loading
[8]ページ先頭
©2009-2025 Movatter.jp