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

Commit496a096

Browse files
committed
chore: check proper rbac perms on open file in cache
1 parent5944b1c commit496a096

File tree

6 files changed

+62
-14
lines changed

6 files changed

+62
-14
lines changed

‎coderd/coderd.go

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -572,10 +572,13 @@ func New(options *Options) *API {
572572
TemplateScheduleStore:options.TemplateScheduleStore,
573573
UserQuietHoursScheduleStore:options.UserQuietHoursScheduleStore,
574574
AccessControlStore:options.AccessControlStore,
575-
FileCache:files.NewFromStore(options.Database,options.PrometheusRegistry),
576-
Experiments:experiments,
577-
WebpushDispatcher:options.WebPushDispatcher,
578-
healthCheckGroup:&singleflight.Group[string,*healthsdk.HealthcheckReport]{},
575+
FileCache:files.NewFromStore(options.Database,options.PrometheusRegistry,func(ctx context.Context,action policy.Action,object rbac.Object)error {
576+
subject:=httpmw.UserAuthorizationCtx(ctx)
577+
returnoptions.Authorizer.Authorize(ctx,subject,action,object)
578+
}),
579+
Experiments:experiments,
580+
WebpushDispatcher:options.WebPushDispatcher,
581+
healthCheckGroup:&singleflight.Group[string,*healthsdk.HealthcheckReport]{},
579582
Acquirer:provisionerdserver.NewAcquirer(
580583
ctx,
581584
options.Logger.Named("acquirer"),

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -432,6 +432,24 @@ var (
432432
}),
433433
Scope:rbac.ScopeAll,
434434
}.WithCachedASTValue()
435+
436+
subjectFileReader= rbac.Subject{
437+
Type:rbac.SubjectTypeFileReader,
438+
FriendlyName:"Can Read All Files",
439+
ID:uuid.Nil.String(),
440+
Roles:rbac.Roles([]rbac.Role{
441+
{
442+
Identifier: rbac.RoleIdentifier{Name:"file-reader"},
443+
DisplayName:"FileReader",
444+
Site:rbac.Permissions(map[string][]policy.Action{
445+
rbac.ResourceFile.Type: {policy.ActionRead},
446+
}),
447+
Org:map[string][]rbac.Permission{},
448+
User: []rbac.Permission{},
449+
},
450+
}),
451+
Scope:rbac.ScopeAll,
452+
}.WithCachedASTValue()
435453
)
436454

437455
// AsProvisionerd returns a context with an actor that has permissions required
@@ -498,6 +516,10 @@ func AsPrebuildsOrchestrator(ctx context.Context) context.Context {
498516
returnAs(ctx,subjectPrebuildsOrchestrator)
499517
}
500518

519+
funcAsFileReader(ctx context.Context) context.Context {
520+
returnAs(ctx,subjectFileReader)
521+
}
522+
501523
varAsRemoveActor= rbac.Subject{
502524
ID:"remove-actor",
503525
}

‎coderd/files/cache.go

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,33 +13,42 @@ import (
1313

1414
archivefs"github.com/coder/coder/v2/archive/fs"
1515
"github.com/coder/coder/v2/coderd/database"
16+
"github.com/coder/coder/v2/coderd/database/dbauthz"
17+
"github.com/coder/coder/v2/coderd/rbac"
18+
"github.com/coder/coder/v2/coderd/rbac/policy"
1619
"github.com/coder/coder/v2/coderd/util/lazy"
1720
)
1821

22+
typeAuthorizeFilefunc(ctx context.Context,action policy.Action,object rbac.Object)error
23+
1924
// NewFromStore returns a file cache that will fetch files from the provided
2025
// database.
21-
funcNewFromStore(store database.Store,registerer prometheus.Registerer)*Cache {
26+
funcNewFromStore(store database.Store,registerer prometheus.Registerer,authzAuthorizeFile)*Cache {
2227
fetch:=func(ctx context.Context,fileID uuid.UUID) (cacheEntryValue,error) {
23-
file,err:=store.GetFileByID(ctx,fileID)
28+
// Make sure the read does not fail due to authorization issues.
29+
// Authz is checked on the Acquire call, so this is safe.
30+
file,err:=store.GetFileByID(dbauthz.AsFileReader(ctx),fileID)
2431
iferr!=nil {
2532
returncacheEntryValue{},xerrors.Errorf("failed to read file from database: %w",err)
2633
}
2734

2835
content:=bytes.NewBuffer(file.Data)
2936
returncacheEntryValue{
30-
FS:archivefs.FromTarReader(content),
31-
size:int64(content.Len()),
37+
object:rbac.ResourceFile.WithID(file.ID).WithOwner(file.CreatedBy.String()),
38+
FS:archivefs.FromTarReader(content),
39+
size:int64(content.Len()),
3240
},nil
3341
}
3442

35-
returnNew(fetch,registerer)
43+
returnNew(fetch,registerer,authz)
3644
}
3745

38-
funcNew(fetchfetcher,registerer prometheus.Registerer)*Cache {
46+
funcNew(fetchfetcher,registerer prometheus.Registerer,authzAuthorizeFile)*Cache {
3947
return (&Cache{
4048
lock: sync.Mutex{},
4149
data:make(map[uuid.UUID]*cacheEntry),
4250
fetcher:fetch,
51+
authz:authz,
4352
}).registerMetrics(registerer)
4453
}
4554

@@ -101,6 +110,7 @@ type Cache struct {
101110
lock sync.Mutex
102111
datamap[uuid.UUID]*cacheEntry
103112
fetcher
113+
authzAuthorizeFile
104114

105115
// metrics
106116
cacheMetrics
@@ -118,6 +128,7 @@ type cacheMetrics struct {
118128
}
119129

120130
typecacheEntryValuestruct {
131+
object rbac.Object
121132
fs.FS
122133
sizeint64
123134
}
@@ -146,6 +157,13 @@ func (c *Cache) Acquire(ctx context.Context, fileID uuid.UUID) (fs.FS, error) {
146157
c.Release(fileID)
147158
returnnil,err
148159
}
160+
161+
// Always check the caller can actually read the file.
162+
ifc.authz(ctx,policy.ActionRead,it.object)!=nil {
163+
c.Release(fileID)
164+
returnnil,err
165+
}
166+
149167
returnit.FS,err
150168
}
151169

‎coderd/httpmw/apikey.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,14 +47,18 @@ func APIKey(r *http.Request) database.APIKey {
4747

4848
// UserAuthorizationOptional may return the roles and scope used for
4949
// authorization. Depends on the ExtractAPIKey handler.
50-
funcUserAuthorizationOptional(r*http.Request) (rbac.Subject,bool) {
51-
returndbauthz.ActorFromContext(r.Context())
50+
funcUserAuthorizationOptional(ctx context.Context) (rbac.Subject,bool) {
51+
returndbauthz.ActorFromContext(ctx)
5252
}
5353

5454
// UserAuthorization returns the roles and scope used for authorization. Depends
5555
// on the ExtractAPIKey handler.
5656
funcUserAuthorization(r*http.Request) rbac.Subject {
57-
auth,ok:=UserAuthorizationOptional(r)
57+
returnUserAuthorizationCtx(r.Context())
58+
}
59+
60+
funcUserAuthorizationCtx(ctx context.Context) rbac.Subject {
61+
auth,ok:=UserAuthorizationOptional(ctx)
5862
if!ok {
5963
panic("developer error: ExtractAPIKey middleware not provided")
6064
}

‎coderd/httpmw/apikey_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ func TestAPIKey(t *testing.T) {
5858
assert.NoError(t,err,"actor rego ok")
5959
}
6060

61-
auth,ok:=httpmw.UserAuthorizationOptional(r)
61+
auth,ok:=httpmw.UserAuthorizationOptional(r.Context())
6262
assert.True(t,ok,"httpmw auth ok")
6363
ifok {
6464
_,err:=auth.Roles.Expand()

‎coderd/rbac/authz.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ const (
7474
SubjectTypeSystemRestrictedSubjectType="system_restricted"
7575
SubjectTypeNotifierSubjectType="notifier"
7676
SubjectTypeSubAgentAPISubjectType="sub_agent_api"
77+
SubjectTypeFileReaderSubjectType="file_reader"
7778
)
7879

7980
// Subject is a struct that contains all the elements of a subject in an rbac

0 commit comments

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