Skip to content

Commit aaee022

Browse files
Merge pull request from GHSA-55r9-5mx9-qq7r
when a client pushes an image zot's inline dedupe will try to find the blob path corresponding with the blob digest that it's currently pushed and if it's found in the cache then zot will make a symbolic link to that cache entry and report to the client that the blob already exists on the location. Before this patch authorization was not applied on this process meaning that a user could copy blobs without having permissions on the source repo. Added a rule which says that the client should have read permissions on the source repo before deduping, otherwise just Stat() the blob and return the corresponding status code. Signed-off-by: Petu Eusebiu <[email protected]> Co-authored-by: Petu Eusebiu <[email protected]>
1 parent 002ff05 commit aaee022

12 files changed

+492
-10
lines changed

pkg/api/controller_test.go

+116
Original file line numberDiff line numberDiff line change
@@ -5142,6 +5142,122 @@ func TestGetUsername(t *testing.T) {
51425142
})
51435143
}
51445144

5145+
func TestAuthorizationMountBlob(t *testing.T) {
5146+
Convey("Make a new controller", t, func() {
5147+
port := test.GetFreePort()
5148+
baseURL := test.GetBaseURL(port)
5149+
5150+
conf := config.New()
5151+
conf.HTTP.Port = port
5152+
// have two users: one for user Policy, and another for default policy
5153+
username1, _ := test.GenerateRandomString()
5154+
password1, _ := test.GenerateRandomString()
5155+
username2, _ := test.GenerateRandomString()
5156+
password2, _ := test.GenerateRandomString()
5157+
username1 = strings.ToLower(username1)
5158+
username2 = strings.ToLower(username2)
5159+
5160+
content := test.GetCredString(username1, password1) + test.GetCredString(username2, password2)
5161+
htpasswdPath := test.MakeHtpasswdFileFromString(content)
5162+
defer os.Remove(htpasswdPath)
5163+
5164+
conf.HTTP.Auth = &config.AuthConfig{
5165+
HTPasswd: config.AuthHTPasswd{
5166+
Path: htpasswdPath,
5167+
},
5168+
}
5169+
5170+
user1Repo := fmt.Sprintf("%s/**", username1)
5171+
user2Repo := fmt.Sprintf("%s/**", username2)
5172+
5173+
// config with all policy types, to test that the correct one is applied in each case
5174+
conf.HTTP.AccessControl = &config.AccessControlConfig{
5175+
Repositories: config.Repositories{
5176+
user1Repo: config.PolicyGroup{
5177+
Policies: []config.Policy{
5178+
{
5179+
Users: []string{username1},
5180+
Actions: []string{
5181+
constants.ReadPermission,
5182+
constants.CreatePermission,
5183+
},
5184+
},
5185+
},
5186+
},
5187+
user2Repo: config.PolicyGroup{
5188+
Policies: []config.Policy{
5189+
{
5190+
Users: []string{username2},
5191+
Actions: []string{
5192+
constants.ReadPermission,
5193+
constants.CreatePermission,
5194+
},
5195+
},
5196+
},
5197+
},
5198+
},
5199+
}
5200+
5201+
dir := t.TempDir()
5202+
5203+
ctlr := api.NewController(conf)
5204+
ctlr.Config.Storage.RootDirectory = dir
5205+
5206+
cm := test.NewControllerManager(ctlr)
5207+
cm.StartAndWait(port)
5208+
defer cm.StopServer()
5209+
5210+
userClient1 := resty.New()
5211+
userClient1.SetBasicAuth(username1, password1)
5212+
5213+
userClient2 := resty.New()
5214+
userClient2.SetBasicAuth(username2, password2)
5215+
5216+
img := CreateImageWith().RandomLayers(1, 2).DefaultConfig().Build()
5217+
5218+
repoName1 := username1 + "/" + "myrepo"
5219+
tag := "1.0"
5220+
5221+
// upload image with user1 on repoName1
5222+
err := UploadImageWithBasicAuth(img, baseURL, repoName1, tag, username1, password1)
5223+
So(err, ShouldBeNil)
5224+
5225+
repoName2 := username2 + "/" + "myrepo"
5226+
5227+
blobDigest := img.Manifest.Layers[0].Digest
5228+
5229+
/* a HEAD request by user2 on blob digest (found in user1Repo) should return 404
5230+
because user2 doesn't have permissions to read user1Repo */
5231+
resp, err := userClient2.R().Head(baseURL + fmt.Sprintf("/v2/%s/blobs/%s", repoName2, blobDigest))
5232+
So(err, ShouldBeNil)
5233+
So(resp.StatusCode(), ShouldEqual, http.StatusNotFound)
5234+
5235+
params := make(map[string]string)
5236+
params["mount"] = blobDigest.String()
5237+
5238+
// trying to mount a blob which can be found in cache, but user doesn't have permission
5239+
// should return 202 instead of 201
5240+
resp, err = userClient2.R().SetQueryParams(params).Post(baseURL + "/v2/" + repoName2 + "/blobs/uploads/")
5241+
So(err, ShouldBeNil)
5242+
So(resp.StatusCode(), ShouldEqual, http.StatusAccepted)
5243+
5244+
/* a HEAD request by user1 on blob digest (found in user1Repo) should return 200
5245+
because user1 has permission to read user1Repo */
5246+
resp, err = userClient1.R().Head(baseURL + fmt.Sprintf("/v2/%s/blobs/%s", username1+"/"+"mysecondrepo", blobDigest))
5247+
So(err, ShouldBeNil)
5248+
So(resp.StatusCode(), ShouldEqual, http.StatusOK)
5249+
5250+
// user2 can upload without dedupe
5251+
err = UploadImageWithBasicAuth(img, baseURL, repoName2, tag, username2, password2)
5252+
So(err, ShouldBeNil)
5253+
5254+
// trying to mount a blob which can be found in cache and user has permission should return 201 instead of 202
5255+
resp, err = userClient2.R().SetQueryParams(params).Post(baseURL + "/v2/" + repoName2 + "/blobs/uploads/")
5256+
So(err, ShouldBeNil)
5257+
So(resp.StatusCode(), ShouldEqual, http.StatusCreated)
5258+
})
5259+
}
5260+
51455261
func TestAuthorizationWithOnlyAnonymousPolicy(t *testing.T) {
51465262
Convey("Make a new controller", t, func() {
51475263
const TestRepo = "my-repos/repo"

pkg/api/routes.go

+74-3
Original file line numberDiff line numberDiff line change
@@ -873,6 +873,37 @@ func (rh *RouteHandler) DeleteManifest(response http.ResponseWriter, request *ht
873873
response.WriteHeader(http.StatusAccepted)
874874
}
875875

876+
// canMount checks if a user has read permission on cached blobs with this specific digest.
877+
// returns true if the user have permission to copy blob from cache.
878+
func canMount(userAc *reqCtx.UserAccessControl, imgStore storageTypes.ImageStore, digest godigest.Digest,
879+
) (bool, error) {
880+
canMount := true
881+
882+
// authz enabled
883+
if userAc != nil {
884+
canMount = false
885+
886+
repos, err := imgStore.GetAllDedupeReposCandidates(digest)
887+
if err != nil {
888+
// first write
889+
return false, err
890+
}
891+
892+
if len(repos) == 0 {
893+
canMount = false
894+
}
895+
896+
// check if user can read any repo which contain this blob
897+
for _, repo := range repos {
898+
if userAc.Can(constants.ReadPermission, repo) {
899+
canMount = true
900+
}
901+
}
902+
}
903+
904+
return canMount, nil
905+
}
906+
876907
// CheckBlob godoc
877908
// @Summary Check image blob/layer
878909
// @Description Check an image's blob/layer given a digest
@@ -905,7 +936,31 @@ func (rh *RouteHandler) CheckBlob(response http.ResponseWriter, request *http.Re
905936

906937
digest := godigest.Digest(digestStr)
907938

908-
ok, blen, err := imgStore.CheckBlob(name, digest)
939+
userAc, err := reqCtx.UserAcFromContext(request.Context())
940+
if err != nil {
941+
response.WriteHeader(http.StatusInternalServerError)
942+
943+
return
944+
}
945+
946+
userCanMount, err := canMount(userAc, imgStore, digest)
947+
if err != nil {
948+
rh.c.Log.Error().Err(err).Msg("unexpected error")
949+
}
950+
951+
var blen int64
952+
953+
if userCanMount {
954+
ok, blen, err = imgStore.CheckBlob(name, digest)
955+
} else {
956+
var lockLatency time.Time
957+
958+
imgStore.RLock(&lockLatency)
959+
defer imgStore.RUnlock(&lockLatency)
960+
961+
ok, blen, _, err = imgStore.StatBlob(name, digest)
962+
}
963+
909964
if err != nil {
910965
details := zerr.GetDetails(err)
911966
if errors.Is(err, zerr.ErrBadBlobDigest) { //nolint:gocritic // errorslint conflicts with gocritic:IfElseChain
@@ -1191,11 +1246,27 @@ func (rh *RouteHandler) CreateBlobUpload(response http.ResponseWriter, request *
11911246
}
11921247

11931248
mountDigest := godigest.Digest(mountDigests[0])
1249+
1250+
userAc, err := reqCtx.UserAcFromContext(request.Context())
1251+
if err != nil {
1252+
response.WriteHeader(http.StatusInternalServerError)
1253+
1254+
return
1255+
}
1256+
1257+
userCanMount, err := canMount(userAc, imgStore, mountDigest)
1258+
if err != nil {
1259+
rh.c.Log.Error().Err(err).Msg("unexpected error")
1260+
}
1261+
11941262
// zot does not support cross mounting directly and do a workaround creating using hard link.
11951263
// check blob looks for actual path (name+mountDigests[0]) first then look for cache and
11961264
// if found in cache, will do hard link and if fails we will start new upload.
1197-
_, _, err := imgStore.CheckBlob(name, mountDigest)
1198-
if err != nil {
1265+
if userCanMount {
1266+
_, _, err = imgStore.CheckBlob(name, mountDigest)
1267+
}
1268+
1269+
if err != nil || !userCanMount {
11991270
upload, err := imgStore.NewBlobUpload(name)
12001271
if err != nil {
12011272
details := zerr.GetDetails(err)

pkg/storage/cache/boltdb.go

+51
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,57 @@ func (d *BoltDBDriver) PutBlob(digest godigest.Digest, path string) error {
174174
return nil
175175
}
176176

177+
func (d *BoltDBDriver) GetAllBlobs(digest godigest.Digest) ([]string, error) {
178+
var blobPath strings.Builder
179+
180+
blobPaths := []string{}
181+
182+
if err := d.db.View(func(tx *bbolt.Tx) error {
183+
root := tx.Bucket([]byte(constants.BlobsCache))
184+
if root == nil {
185+
// this is a serious failure
186+
err := zerr.ErrCacheRootBucket
187+
d.log.Error().Err(err).Msg("failed to access root bucket")
188+
189+
return err
190+
}
191+
192+
bucket := root.Bucket([]byte(digest.String()))
193+
if bucket != nil {
194+
origin := bucket.Bucket([]byte(constants.OriginalBucket))
195+
blobPath.Write(d.getOne(origin))
196+
originBlob := blobPath.String()
197+
198+
blobPaths = append(blobPaths, originBlob)
199+
200+
deduped := bucket.Bucket([]byte(constants.DuplicatesBucket))
201+
if deduped != nil {
202+
cursor := deduped.Cursor()
203+
204+
for k, _ := cursor.First(); k != nil; k, _ = cursor.Next() {
205+
var blobPath strings.Builder
206+
207+
blobPath.Write(k)
208+
209+
duplicateBlob := blobPath.String()
210+
211+
if duplicateBlob != originBlob {
212+
blobPaths = append(blobPaths, duplicateBlob)
213+
}
214+
}
215+
216+
return nil
217+
}
218+
}
219+
220+
return zerr.ErrCacheMiss
221+
}); err != nil {
222+
return nil, err
223+
}
224+
225+
return blobPaths, nil
226+
}
227+
177228
func (d *BoltDBDriver) GetBlob(digest godigest.Digest) (string, error) {
178229
var blobPath strings.Builder
179230

pkg/storage/cache/boltdb_test.go

+43
Original file line numberDiff line numberDiff line change
@@ -122,4 +122,47 @@ func TestBoltDBCache(t *testing.T) {
122122
So(err, ShouldNotBeNil)
123123
So(val, ShouldBeEmpty)
124124
})
125+
126+
Convey("Test cache.GetAllBlos()", t, func() {
127+
dir := t.TempDir()
128+
129+
log := log.NewLogger("debug", "")
130+
So(log, ShouldNotBeNil)
131+
132+
_, err := storage.Create("boltdb", "failTypeAssertion", log)
133+
So(err, ShouldNotBeNil)
134+
135+
cacheDriver, _ := storage.Create("boltdb", cache.BoltDBDriverParameters{dir, "cache_test", false}, log)
136+
So(cacheDriver, ShouldNotBeNil)
137+
138+
err = cacheDriver.PutBlob("digest", "first")
139+
So(err, ShouldBeNil)
140+
141+
err = cacheDriver.PutBlob("digest", "second")
142+
So(err, ShouldBeNil)
143+
144+
err = cacheDriver.PutBlob("digest", "third")
145+
So(err, ShouldBeNil)
146+
147+
blobs, err := cacheDriver.GetAllBlobs("digest")
148+
So(err, ShouldBeNil)
149+
150+
So(blobs, ShouldResemble, []string{"first", "second", "third"})
151+
152+
err = cacheDriver.DeleteBlob("digest", "first")
153+
So(err, ShouldBeNil)
154+
155+
blobs, err = cacheDriver.GetAllBlobs("digest")
156+
So(err, ShouldBeNil)
157+
158+
So(blobs, ShouldResemble, []string{"second", "third"})
159+
160+
err = cacheDriver.DeleteBlob("digest", "third")
161+
So(err, ShouldBeNil)
162+
163+
blobs, err = cacheDriver.GetAllBlobs("digest")
164+
So(err, ShouldBeNil)
165+
166+
So(blobs, ShouldResemble, []string{"second"})
167+
})
125168
}

pkg/storage/cache/cacheinterface.go

+2
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ type Cache interface {
1111
// Retrieves the blob matching provided digest.
1212
GetBlob(digest godigest.Digest) (string, error)
1313

14+
GetAllBlobs(digest godigest.Digest) ([]string, error)
15+
1416
// Uploads blob to cachedb.
1517
PutBlob(digest godigest.Digest, path string) error
1618

pkg/storage/cache/dynamodb.go

+36
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,42 @@ func (d *DynamoDBDriver) GetBlob(digest godigest.Digest) (string, error) {
141141
return out.OriginalBlobPath, nil
142142
}
143143

144+
func (d *DynamoDBDriver) GetAllBlobs(digest godigest.Digest) ([]string, error) {
145+
blobPaths := []string{}
146+
147+
resp, err := d.client.GetItem(context.TODO(), &dynamodb.GetItemInput{
148+
TableName: aws.String(d.tableName),
149+
Key: map[string]types.AttributeValue{
150+
"Digest": &types.AttributeValueMemberS{Value: digest.String()},
151+
},
152+
})
153+
if err != nil {
154+
d.log.Error().Err(err).Str("tableName", d.tableName).Msg("failed to get blob")
155+
156+
return nil, err
157+
}
158+
159+
out := Blob{}
160+
161+
if resp.Item == nil {
162+
d.log.Debug().Err(zerr.ErrCacheMiss).Str("digest", string(digest)).Msg("failed to find blob in cache")
163+
164+
return nil, zerr.ErrCacheMiss
165+
}
166+
167+
_ = attributevalue.UnmarshalMap(resp.Item, &out)
168+
169+
blobPaths = append(blobPaths, out.OriginalBlobPath)
170+
171+
for _, item := range out.DuplicateBlobPath {
172+
if item != out.OriginalBlobPath {
173+
blobPaths = append(blobPaths, item)
174+
}
175+
}
176+
177+
return blobPaths, nil
178+
}
179+
144180
func (d *DynamoDBDriver) PutBlob(digest godigest.Digest, path string) error {
145181
if path == "" {
146182
d.log.Error().Err(zerr.ErrEmptyValue).Str("digest", digest.String()).

0 commit comments

Comments
 (0)