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 <peusebiu@cisco.com> Co-authored-by: Petu Eusebiu <peusebiu@cisco.com>
2026-06-16 04:17:55 +08:00 · 2024-07-08 11:35:44 -07:00
parent 002ff05f6e
commit aaee0220e4
12 changed files with 492 additions and 10 deletions
@@ -5142,6 +5142,122 @@ func TestGetUsername(t *testing.T) {
 	})
 }

+func TestAuthorizationMountBlob(t *testing.T) {
+	Convey("Make a new controller", t, func() {
+		port := test.GetFreePort()
+		baseURL := test.GetBaseURL(port)
+
+		conf := config.New()
+		conf.HTTP.Port = port
+		// have two users: one for  user Policy, and another for default policy
+		username1, _ := test.GenerateRandomString()
+		password1, _ := test.GenerateRandomString()
+		username2, _ := test.GenerateRandomString()
+		password2, _ := test.GenerateRandomString()
+		username1 = strings.ToLower(username1)
+		username2 = strings.ToLower(username2)
+
+		content := test.GetCredString(username1, password1) + test.GetCredString(username2, password2)
+		htpasswdPath := test.MakeHtpasswdFileFromString(content)
+		defer os.Remove(htpasswdPath)
+
+		conf.HTTP.Auth = &config.AuthConfig{
+			HTPasswd: config.AuthHTPasswd{
+				Path: htpasswdPath,
+			},
+		}
+
+		user1Repo := fmt.Sprintf("%s/**", username1)
+		user2Repo := fmt.Sprintf("%s/**", username2)
+
+		// config with all policy types, to test that the correct one is applied in each case
+		conf.HTTP.AccessControl = &config.AccessControlConfig{
+			Repositories: config.Repositories{
+				user1Repo: config.PolicyGroup{
+					Policies: []config.Policy{
+						{
+							Users: []string{username1},
+							Actions: []string{
+								constants.ReadPermission,
+								constants.CreatePermission,
+							},
+						},
+					},
+				},
+				user2Repo: config.PolicyGroup{
+					Policies: []config.Policy{
+						{
+							Users: []string{username2},
+							Actions: []string{
+								constants.ReadPermission,
+								constants.CreatePermission,
+							},
+						},
+					},
+				},
+			},
+		}
+
+		dir := t.TempDir()
+
+		ctlr := api.NewController(conf)
+		ctlr.Config.Storage.RootDirectory = dir
+
+		cm := test.NewControllerManager(ctlr)
+		cm.StartAndWait(port)
+		defer cm.StopServer()
+
+		userClient1 := resty.New()
+		userClient1.SetBasicAuth(username1, password1)
+
+		userClient2 := resty.New()
+		userClient2.SetBasicAuth(username2, password2)
+
+		img := CreateImageWith().RandomLayers(1, 2).DefaultConfig().Build()
+
+		repoName1 := username1 + "/" + "myrepo"
+		tag := "1.0"
+
+		// upload image with user1 on repoName1
+		err := UploadImageWithBasicAuth(img, baseURL, repoName1, tag, username1, password1)
+		So(err, ShouldBeNil)
+
+		repoName2 := username2 + "/" + "myrepo"
+
+		blobDigest := img.Manifest.Layers[0].Digest
+
+		/* a HEAD request by user2 on blob digest (found in user1Repo) should return 404
+		because user2 doesn't have permissions to read user1Repo */
+		resp, err := userClient2.R().Head(baseURL + fmt.Sprintf("/v2/%s/blobs/%s", repoName2, blobDigest))
+		So(err, ShouldBeNil)
+		So(resp.StatusCode(), ShouldEqual, http.StatusNotFound)
+
+		params := make(map[string]string)
+		params["mount"] = blobDigest.String()
+
+		// trying to mount a blob which can be found in cache, but user doesn't have permission
+		// should return 202 instead of 201
+		resp, err = userClient2.R().SetQueryParams(params).Post(baseURL + "/v2/" + repoName2 + "/blobs/uploads/")
+		So(err, ShouldBeNil)
+		So(resp.StatusCode(), ShouldEqual, http.StatusAccepted)
+
+		/* a HEAD request by user1 on blob digest (found in user1Repo) should return 200
+		because user1 has permission to read user1Repo */
+		resp, err = userClient1.R().Head(baseURL + fmt.Sprintf("/v2/%s/blobs/%s", username1+"/"+"mysecondrepo", blobDigest))
+		So(err, ShouldBeNil)
+		So(resp.StatusCode(), ShouldEqual, http.StatusOK)
+
+		// user2 can upload without dedupe
+		err = UploadImageWithBasicAuth(img, baseURL, repoName2, tag, username2, password2)
+		So(err, ShouldBeNil)
+
+		// trying to mount a blob which can be found in cache and user has permission should return 201 instead of 202
+		resp, err = userClient2.R().SetQueryParams(params).Post(baseURL + "/v2/" + repoName2 + "/blobs/uploads/")
+		So(err, ShouldBeNil)
+		So(resp.StatusCode(), ShouldEqual, http.StatusCreated)
+	})
+}
+
 func TestAuthorizationWithOnlyAnonymousPolicy(t *testing.T) {
 	Convey("Make a new controller", t, func() {
 		const TestRepo = "my-repos/repo"
@@ -873,6 +873,37 @@ func (rh *RouteHandler) DeleteManifest(response http.ResponseWriter, request *ht
 	response.WriteHeader(http.StatusAccepted)
 }

+// canMount checks if a user has read permission on cached blobs with this specific digest.
+// returns true if the user have permission to copy blob from cache.
+func canMount(userAc *reqCtx.UserAccessControl, imgStore storageTypes.ImageStore, digest godigest.Digest,
+) (bool, error) {
+	canMount := true
+
+	// authz enabled
+	if userAc != nil {
+		canMount = false
+
+		repos, err := imgStore.GetAllDedupeReposCandidates(digest)
+		if err != nil {
+			// first write
+			return false, err
+		}
+
+		if len(repos) == 0 {
+			canMount = false
+		}
+
+		// check if user can read any repo which contain this blob
+		for _, repo := range repos {
+			if userAc.Can(constants.ReadPermission, repo) {
+				canMount = true
+			}
+		}
+	}
+
+	return canMount, nil
+}
+
 // CheckBlob godoc
 // @Summary Check image blob/layer
 // @Description Check an image's blob/layer given a digest
@@ -905,7 +936,31 @@ func (rh *RouteHandler) CheckBlob(response http.ResponseWriter, request *http.Re

 	digest := godigest.Digest(digestStr)

-	ok, blen, err := imgStore.CheckBlob(name, digest)
+	userAc, err := reqCtx.UserAcFromContext(request.Context())
+	if err != nil {
+		response.WriteHeader(http.StatusInternalServerError)
+
+		return
+	}
+
+	userCanMount, err := canMount(userAc, imgStore, digest)
+	if err != nil {
+		rh.c.Log.Error().Err(err).Msg("unexpected error")
+	}
+
+	var blen int64
+
+	if userCanMount {
+		ok, blen, err = imgStore.CheckBlob(name, digest)
+	} else {
+		var lockLatency time.Time
+
+		imgStore.RLock(&lockLatency)
+		defer imgStore.RUnlock(&lockLatency)
+
+		ok, blen, _, err = imgStore.StatBlob(name, digest)
+	}
+
 	if err != nil {
 		details := zerr.GetDetails(err)
 		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 *
 		}

 		mountDigest := godigest.Digest(mountDigests[0])
+
+		userAc, err := reqCtx.UserAcFromContext(request.Context())
+		if err != nil {
+			response.WriteHeader(http.StatusInternalServerError)
+
+			return
+		}
+
+		userCanMount, err := canMount(userAc, imgStore, mountDigest)
+		if err != nil {
+			rh.c.Log.Error().Err(err).Msg("unexpected error")
+		}
+
 		// zot does not support cross mounting directly and do a workaround creating using hard link.
 		// check blob looks for actual path (name+mountDigests[0]) first then look for cache and
 		// if found in cache, will do hard link and if fails we will start new upload.
-		_, _, err := imgStore.CheckBlob(name, mountDigest)
-		if err != nil {
+		if userCanMount {
+			_, _, err = imgStore.CheckBlob(name, mountDigest)
+		}
+
+		if err != nil || !userCanMount {
 			upload, err := imgStore.NewBlobUpload(name)
 			if err != nil {
 				details := zerr.GetDetails(err)