fix(auth): authorize attributes and archive reads with stored tags

Require both Attributes actions on the addressed version and GetObject permission for archive listings. Preserve conditional allows, missing-object masks and retryable storage errors.

Fixes #342

Signed-off-by: Feng Ruohang <rh@vonng.com>
This commit is contained in:
Feng Ruohang
2026-11-11 12:24:21 +08:00
parent 745fbf0437
commit f746d07076
5 changed files with 271 additions and 82 deletions
+1 -1
View File
@@ -202,7 +202,7 @@ func (api objectAPIHandlers) listObjectsV2Handler(ctx context.Context, w http.Re
if r.Header.Get(xMinIOExtract) == "true" && strings.Contains(prefix, archivePattern) {
// Initiate a list objects operation inside a zip file based in the input params
listObjectsV2Info, err = listObjectsV2InArchive(ctx, objectAPI, bucket, prefix, token, delimiter, maxKeys, startAfter, r.Header)
listObjectsV2Info, err = listObjectsV2InArchive(ctx, objectAPI, bucket, prefix, token, delimiter, maxKeys, startAfter, r)
} else {
// Initiate a list objects operation based on the input params.
// On success would return back ListObjectsInfo object to be
+170
View File
@@ -0,0 +1,170 @@
// Copyright (c) 2026 PGSTY
// SPDX-License-Identifier: AGPL-3.0-or-later
package cmd
import (
"archive/zip"
"bytes"
"fmt"
"net/http"
"net/http/httptest"
"net/url"
"strings"
"testing"
"github.com/minio/minio/internal/auth"
xhttp "github.com/minio/minio/internal/http"
)
func TestAPIAttributesAndArchiveStoredTags(t *testing.T) {
ExecObjectLayerAPITest(ExecObjectLayerAPITestArgs{
t: t, makeBucketOptions: MakeBucketOptions{VersioningEnabled: true},
objAPITest: func(obj ObjectLayer, backend, bucket string, router http.Handler, root auth.Credentials, t *testing.T) {
_, err := globalBucketMetadataSys.Update(t.Context(), bucket, bucketVersioningConfig, enabledBucketVersioningConfig)
if err != nil {
t.Fatal(err)
}
var archive bytes.Buffer
zw := zip.NewWriter(&archive)
entry, err := zw.Create("entry.txt")
if err != nil {
t.Fatal(err)
}
if _, err = entry.Write([]byte("private entry")); err != nil {
t.Fatal(err)
}
if err = zw.Close(); err != nil {
t.Fatal(err)
}
for _, indexed := range []bool{false, true} {
key := fmt.Sprintf("tagged-%v.zip", indexed)
put := func(tag string) ObjectInfo {
oi, err := obj.PutObject(t.Context(), bucket, key, mustGetPutObjReader(t, bytes.NewReader(archive.Bytes()), int64(archive.Len()), "", ""), ObjectOptions{Versioned: true, UserDefined: map[string]string{xhttp.AmzObjectTagging: "tier=" + tag}})
if err != nil {
t.Fatal(err)
}
return oi
}
old := put("public")
latest := put("restricted")
if indexed {
if _, err := updateObjectMetadataWithZipInfo(t.Context(), obj, bucket, key, ObjectOptions{VersionID: latest.VersionID}); err != nil {
t.Fatal(err)
}
}
for _, anonymous := range []bool{false, true} {
for _, effect := range []string{"Allow", "Deny"} {
actions := `"s3:GetObject","s3:GetObjectAttributes","s3:GetObjectVersion","s3:GetObjectVersionAttributes"`
condition := `"Condition":{"StringEquals":{"s3:ExistingObjectTag/tier":"restricted"}}`
statement := fmt.Sprintf(`{"Effect":"%s","Action":[%s],"Resource":["arn:aws:s3:::%s/*"],%s}`, effect, actions, bucket, condition)
if effect == "Deny" {
statement += fmt.Sprintf(`,{"Effect":"Allow","Action":[%s],"Resource":["arn:aws:s3:::%s/*"]}`, actions, bucket)
}
statement += fmt.Sprintf(`,{"Effect":"Allow","Action":["s3:ListBucket"],"Resource":["arn:aws:s3:::%s"]}`, bucket)
creds := newDeleteAuthzPolicyUser(t, backend, bucket, "["+statement+"]")
if anonymous {
document := `{"Version":"2012-10-17","Statement":[` + strings.ReplaceAll(statement, `"Effect":`, `"Principal":"*","Effect":`) + `]}`
if _, err := globalBucketMetadataSys.Update(t.Context(), bucket, bucketPolicyConfig, []byte(document)); err != nil {
t.Fatal(err)
}
}
for _, operation := range []string{"attributes", "version-attributes", http.MethodGet, http.MethodHead, "LIST"} {
t.Run(fmt.Sprintf("%s/indexed=%v/anonymous=%v/%s/%s", backend, indexed, anonymous, effect, operation), func(t *testing.T) {
verb := http.MethodGet
target := getGetObjectURL("", bucket, key+"/entry.txt")
headers := map[string]string{xMinIOExtract: "true", xhttp.AmzObjectTagging: "tier=public"}
want := http.StatusOK
if effect == "Deny" {
want = http.StatusForbidden
}
switch operation {
case "attributes", "version-attributes":
q := url.Values{"attributes": {""}}
if operation == "version-attributes" {
q.Set("versionId", old.VersionID)
if effect == "Allow" {
want = http.StatusForbidden
} else {
want = http.StatusOK
}
}
target = makeTestTargetURL("", bucket, key, q)
headers[xhttp.AmzObjectAttributes] = "ETag,ObjectSize"
case http.MethodHead:
verb = http.MethodHead
case "LIST":
target = "/" + bucket + "?list-type=2&prefix=" + key + "/"
}
req, err := newTestSignedRequestV4(verb, target, 0, nil, creds.AccessKey, creds.SecretKey, headers)
if err != nil {
t.Fatal(err)
}
if anonymous {
req.Header.Del("Authorization")
}
rec := httptest.NewRecorder()
router.ServeHTTP(rec, req)
if rec.Code != want {
t.Fatalf("status %d, want %d: %s", rec.Code, want, rec.Body.String())
}
if rec.Code == http.StatusOK && operation == http.MethodGet && rec.Body.String() != "private entry" {
t.Fatalf("wrong extracted content: %s", rec.Body.String())
}
if rec.Code == http.StatusForbidden && strings.Contains(rec.Body.String(), "private entry") {
t.Fatal("denied response disclosed content")
}
})
}
}
}
}
// Each Attributes action is necessary; a conditional Deny on either
// action must win even when the other action is unconditionally allowed.
for _, versioned := range []bool{false, true} {
for _, deniedAction := range []string{"GetObject", "GetObjectAttributes"} {
actions := `"s3:GetObject","s3:GetObjectAttributes"`
if versioned {
actions = `"s3:GetObjectVersion","s3:GetObjectVersionAttributes"`
deniedAction = strings.Replace(deniedAction, "GetObject", "GetObjectVersion", 1)
}
statements := fmt.Sprintf(`[{"Effect":"Allow","Action":[%s],"Resource":["arn:aws:s3:::%s/*"]},{"Effect":"Deny","Action":["s3:%s"],"Resource":["arn:aws:s3:::%s/*"],"Condition":{"StringEquals":{"s3:ExistingObjectTag/tier":"restricted"}}}]`, actions, bucket, deniedAction, bucket)
creds := newDeleteAuthzPolicyUser(t, backend, bucket, statements)
q := url.Values{"attributes": {""}}
if versioned {
oi, err := obj.GetObjectInfo(t.Context(), bucket, "tagged-true.zip", ObjectOptions{})
if err != nil {
t.Fatal(err)
}
q.Set("versionId", oi.VersionID)
}
req, err := newTestSignedRequestV4(http.MethodGet, makeTestTargetURL("", bucket, "tagged-true.zip", q), 0, nil, creds.AccessKey, creds.SecretKey, map[string]string{xhttp.AmzObjectAttributes: "ETag"})
if err != nil {
t.Fatal(err)
}
rec := httptest.NewRecorder()
router.ServeHTTP(rec, req)
if rec.Code != http.StatusForbidden {
t.Fatalf("%s Deny: %d %s", deniedAction, rec.Code, rec.Body.String())
}
}
}
listOnly := newDeleteAuthzPolicyUser(t, backend, bucket, fmt.Sprintf(`[{"Effect":"Allow","Action":["s3:ListBucket"],"Resource":["arn:aws:s3:::%s"]}]`, bucket))
for _, extract := range []bool{false, true} {
req, err := newTestSignedRequestV4(http.MethodGet, "/"+bucket+"?list-type=2&prefix=tagged-true.zip/", 0, nil, listOnly.AccessKey, listOnly.SecretKey, map[string]string{xMinIOExtract: fmt.Sprint(extract)})
if err != nil {
t.Fatal(err)
}
rec := httptest.NewRecorder()
router.ServeHTTP(rec, req)
want := http.StatusOK
if extract {
want = http.StatusForbidden
}
if rec.Code != want {
t.Fatalf("list-only extract=%v: %d %s", extract, rec.Code, rec.Body.String())
}
}
},
})
}
+10 -1
View File
@@ -54,7 +54,7 @@ func TestGetObjectAttributesReadQuorumLost(t *testing.T) {
t.Fatal(err)
}
const object, missing = "object", "missing"
if _, err := z.PutObject(ctx, bucket, object, mustGetPutObjReader(t, bytes.NewReader([]byte("data")), 4, "", ""), ObjectOptions{}); err != nil {
if _, err := z.PutObject(ctx, bucket, object, mustGetPutObjReader(t, bytes.NewReader([]byte("data")), 4, "", ""), ObjectOptions{UserDefined: map[string]string{xhttp.AmzObjectTagging: "tier=restricted"}}); err != nil {
t.Fatal(err)
}
@@ -63,6 +63,9 @@ func TestGetObjectAttributesReadQuorumLost(t *testing.T) {
withList := newDeleteAuthzPolicyUser(t, "", bucket, "["+read+
`, {"Effect": "Allow", "Action": ["s3:ListBucket"], "Resource": ["arn:aws:s3:::`+bucket+`"]}]`)
tagged := newDeleteAuthzPolicyUser(t, "", bucket, `[{"Effect":"Allow","Action":["s3:GetObject","s3:GetObjectAttributes"],"Resource":["arn:aws:s3:::`+bucket+`/*"],"Condition":{"StringEquals":{"s3:ExistingObjectTag/tier":"restricted"}}}]`)
unauthorized := newDeleteAuthzPolicyUser(t, "", bucket, `[{"Effect":"Allow","Action":["s3:GetObjectAttributes"],"Resource":["arn:aws:s3:::`+bucket+`/*"]}]`)
check := func(stage, user string, creds auth.Credentials, key string, want int) {
t.Helper()
target := makeTestTargetURL("", bucket, key, url.Values{"attributes": []string{""}})
@@ -85,7 +88,13 @@ func TestGetObjectAttributesReadQuorumLost(t *testing.T) {
check("healthy", "withoutList", withoutList, missing, http.StatusForbidden)
check("healthy", "withList", withList, missing, http.StatusNotFound)
check("healthy", "tagged", tagged, object, http.StatusOK)
check("healthy", "unauthorized", unauthorized, object, http.StatusForbidden)
defer objectReadFault(z, bucket, object, 10)()
check("read quorum lost", "withoutList", withoutList, object, http.StatusServiceUnavailable)
check("read quorum lost", "withList", withList, object, http.StatusServiceUnavailable)
check("read quorum lost", "tagged", tagged, object, http.StatusServiceUnavailable)
check("read quorum lost", "unauthorized", unauthorized, object, http.StatusForbidden)
}
+19 -12
View File
@@ -605,20 +605,13 @@ func (api objectAPIHandlers) getObjectAttributesHandler(ctx context.Context, obj
return
}
var s3Error APIErrorCode
actions := [2]policy.Action{policy.GetObjectAttributesAction, policy.GetObjectAction}
if opts.VersionID != "" {
s3Error = checkRequestAuthType(ctx, r, policy.GetObjectVersionAttributesAction, bucket, object)
if s3Error == ErrNone {
s3Error = checkRequestAuthType(ctx, r, policy.GetObjectVersionAction, bucket, object)
}
} else {
s3Error = checkRequestAuthType(ctx, r, policy.GetObjectAttributesAction, bucket, object)
if s3Error == ErrNone {
s3Error = checkRequestAuthType(ctx, r, policy.GetObjectAction, bucket, object)
}
actions = [2]policy.Action{policy.GetObjectVersionAttributesAction, policy.GetObjectVersionAction}
}
if s3Error != ErrNone {
logger.GetReqInfo(ctx).BucketName = bucket
logger.GetReqInfo(ctx).ObjectName = object
if s3Error := authenticateRequest(ctx, r, actions[0]); s3Error != ErrNone {
writeErrorResponse(ctx, w, errorCodes.ToAPIErr(s3Error), r.URL)
return
}
@@ -630,6 +623,20 @@ func (api objectAPIHandlers) getObjectAttributesHandler(ctx context.Context, obj
}
objInfo, err := objectAPI.GetObjectInfo(ctx, bucket, object, opts)
// Both actions must be authorized with the stored tags. Authorizing first
// with empty tags would reject conditional Allows and miss conditional Denies.
for _, action := range actions {
var s3Error APIErrorCode
if err != nil {
s3Error = authorizeRequestWithUnreadTags(ctx, r, action, err)
} else {
s3Error = authorizeRequestWithExistingTags(ctx, r, action, objInfo.UserTags)
}
if s3Error != ErrNone {
writeErrorResponse(ctx, w, errorCodes.ToAPIErr(s3Error), r.URL)
return
}
}
if err != nil {
// As with S3, a caller without s3:ListBucket on the bucket gets
// AccessDenied in place of NoSuchKey. A server error, such as lost
+71 -68
View File
@@ -28,10 +28,10 @@ import (
"sort"
"strings"
"github.com/minio/minio/internal/auth"
"github.com/minio/minio/internal/crypto"
xhttp "github.com/minio/minio/internal/http"
xioutil "github.com/minio/minio/internal/ioutil"
"github.com/minio/minio/internal/logger"
"github.com/minio/zipindex"
"github.com/pgsty/silo-pkg/v3/policy"
)
@@ -82,35 +82,9 @@ func (api objectAPIHandlers) getObjectInArchiveFileHandler(ctx context.Context,
getObjectInfo := objectAPI.GetObjectInfo
// Check for auth type to return S3 compatible error.
// type to return the correct error (NoSuchKey vs AccessDenied)
if s3Error := checkRequestAuthType(ctx, r, policy.GetObjectAction, bucket, zipPath); s3Error != ErrNone {
if getRequestAuthType(r) == authTypeAnonymous {
// As per "Permission" section in
// https://docs.aws.amazon.com/AmazonS3/latest/API/RESTObjectGET.html
// If the object you request does not exist,
// the error Amazon S3 returns depends on
// whether you also have the s3:ListBucket
// permission.
// * If you have the s3:ListBucket permission
// on the bucket, Amazon S3 will return an
// HTTP status code 404 ("no such key")
// error.
// * if you don’t have the s3:ListBucket
// permission, Amazon S3 will return an HTTP
// status code 403 ("access denied") error.`
if globalPolicySys.IsAllowed(policy.BucketPolicyArgs{
Action: policy.ListBucketAction,
BucketName: bucket,
ConditionValues: getConditionValues(r, "", auth.AnonymousCredentials),
IsOwner: false,
}) {
_, err = getObjectInfo(ctx, bucket, zipPath, opts)
if toAPIError(ctx, err).Code == "NoSuchKey" {
s3Error = ErrNoSuchKey
}
}
}
logger.GetReqInfo(ctx).BucketName = bucket
logger.GetReqInfo(ctx).ObjectName = zipPath
if s3Error := authenticateRequest(ctx, r, policy.GetObjectAction); s3Error != ErrNone {
writeErrorResponse(ctx, w, errorCodes.ToAPIErr(s3Error), r.URL)
return
}
@@ -128,6 +102,10 @@ func (api objectAPIHandlers) getObjectInArchiveFileHandler(ctx context.Context,
// Validate pre-conditions if any.
opts.CheckPrecondFn = func(oi ObjectInfo) bool {
if s3Error := authorizeRequestWithExistingTags(ctx, r, policy.GetObjectAction, oi.UserTags); s3Error != ErrNone {
writeErrorResponse(ctx, w, errorCodes.ToAPIErr(s3Error), r.URL)
return true
}
if _, err := DecryptObjectInfo(&oi, r); err != nil {
writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL)
return true
@@ -138,14 +116,21 @@ func (api objectAPIHandlers) getObjectInArchiveFileHandler(ctx context.Context,
zipObjInfo, err := getObjectInfo(ctx, bucket, zipPath, opts)
if err != nil {
err = archiveReadError(ctx, r, bucket, zipPath, err)
writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL)
return
}
if opts.CheckPrecondFn(zipObjInfo) {
return
}
zipInfo := zipObjInfo.ArchiveInfo(r.Header)
if len(zipInfo) == 0 {
zipInfo, _, err = readObjectZIPInfo(ctx, objectAPI, bucket, zipPath, opts)
}
if isErrPreconditionFailed(err) {
return
}
if err != nil {
writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL)
return
@@ -177,6 +162,9 @@ func (api objectAPIHandlers) getObjectInArchiveFileHandler(ctx context.Context,
end := min(file.Offset+int64(file.CompressedSize64)+64<<10, zipObjInfo.Size)
rs := &HTTPRangeSpec{Start: file.Offset, End: end}
gr, err := objectAPI.GetObjectNInfo(ctx, bucket, zipPath, rs, nil, opts)
if isErrPreconditionFailed(err) {
return
}
if err != nil {
writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL)
return
@@ -224,22 +212,40 @@ func (api objectAPIHandlers) getObjectInArchiveFileHandler(ctx context.Context,
}
// listObjectsV2InArchive generates S3 listing result ListObjectsV2Info from zip file, all parameters are already validated by the caller.
func listObjectsV2InArchive(ctx context.Context, objectAPI ObjectLayer, bucket, prefix, token, delimiter string, maxKeys int, startAfter string, h http.Header) (ListObjectsV2Info, error) {
func listObjectsV2InArchive(ctx context.Context, objectAPI ObjectLayer, bucket, prefix, token, delimiter string, maxKeys int, startAfter string, r *http.Request) (ListObjectsV2Info, error) {
zipPath, _, err := splitZipExtensionPath(prefix)
if err != nil {
// Return empty listing
return ListObjectsV2Info{}, nil
}
zipObjInfo, err := objectAPI.GetObjectInfo(ctx, bucket, zipPath, ObjectOptions{})
// The bucket listing permission does not grant access to ZIP contents.
logger.GetReqInfo(ctx).ObjectName = zipPath
opts := ObjectOptions{}
zipObjInfo, err := objectAPI.GetObjectInfo(ctx, bucket, zipPath, opts)
if err != nil {
// Return empty listing
return ListObjectsV2Info{}, nil
if s3Error := authorizeRequestWithUnreadTags(ctx, r, policy.GetObjectAction, err); s3Error != ErrNone {
return ListObjectsV2Info{}, PrefixAccessDenied{Bucket: bucket, Object: zipPath}
}
if isErrObjectNotFound(err) {
return ListObjectsV2Info{}, nil
}
return ListObjectsV2Info{}, err
}
zipInfo := zipObjInfo.ArchiveInfo(h)
if s3Error := authorizeRequestWithExistingTags(ctx, r, policy.GetObjectAction, zipObjInfo.UserTags); s3Error != ErrNone {
return ListObjectsV2Info{}, PrefixAccessDenied{Bucket: bucket, Object: zipPath}
}
var denied bool
opts.CheckPrecondFn = func(oi ObjectInfo) bool {
denied = authorizeRequestWithExistingTags(ctx, r, policy.GetObjectAction, oi.UserTags) != ErrNone
return denied
}
zipInfo := zipObjInfo.ArchiveInfo(r.Header)
if len(zipInfo) == 0 {
zipInfo, _, err = readObjectZIPInfo(ctx, objectAPI, bucket, zipPath, ObjectOptions{})
zipInfo, _, err = readObjectZIPInfo(ctx, objectAPI, bucket, zipPath, opts)
}
if denied {
return ListObjectsV2Info{}, PrefixAccessDenied{Bucket: bucket, Object: zipPath}
}
if err != nil {
return ListObjectsV2Info{}, err
@@ -372,42 +378,19 @@ func (api objectAPIHandlers) headObjectInArchiveFileHandler(ctx context.Context,
return
}
if s3Error := checkRequestAuthType(ctx, r, policy.GetObjectAction, bucket, zipPath); s3Error != ErrNone {
if getRequestAuthType(r) == authTypeAnonymous {
// As per "Permission" section in
// https://docs.aws.amazon.com/AmazonS3/latest/API/RESTObjectHEAD.html
// If the object you request does not exist,
// the error Amazon S3 returns depends on
// whether you also have the s3:ListBucket
// permission.
// * If you have the s3:ListBucket permission
// on the bucket, Amazon S3 will return an
// HTTP status code 404 ("no such key")
// error.
// * if you don’t have the s3:ListBucket
// permission, Amazon S3 will return an HTTP
// status code 403 ("access denied") error.`
if globalPolicySys.IsAllowed(policy.BucketPolicyArgs{
Action: policy.ListBucketAction,
BucketName: bucket,
ConditionValues: getConditionValues(r, "", auth.AnonymousCredentials),
IsOwner: false,
}) {
_, err = getObjectInfo(ctx, bucket, zipPath, opts)
if toAPIError(ctx, err).Code == "NoSuchKey" {
s3Error = ErrNoSuchKey
}
}
}
errCode := errorCodes.ToAPIErr(s3Error)
w.Header().Set(xMinIOErrCodeHeader, errCode.Code)
w.Header().Set(xMinIOErrDescHeader, "\""+errCode.Description+"\"")
writeErrorResponseHeadersOnly(w, errCode)
logger.GetReqInfo(ctx).BucketName = bucket
logger.GetReqInfo(ctx).ObjectName = zipPath
if s3Error := authenticateRequest(ctx, r, policy.GetObjectAction); s3Error != ErrNone {
writeErrorResponseHeadersOnly(w, errorCodes.ToAPIErr(s3Error))
return
}
// Validate pre-conditions if any.
opts.CheckPrecondFn = func(oi ObjectInfo) bool {
if s3Error := authorizeRequestWithExistingTags(ctx, r, policy.GetObjectAction, oi.UserTags); s3Error != ErrNone {
writeErrorResponseHeadersOnly(w, errorCodes.ToAPIErr(s3Error))
return true
}
return checkPreconditions(ctx, w, r, oi, opts)
}
@@ -424,14 +407,21 @@ func (api objectAPIHandlers) headObjectInArchiveFileHandler(ctx context.Context,
zipObjInfo, err := getObjectInfo(ctx, bucket, zipPath, opts)
if err != nil {
err = archiveReadError(ctx, r, bucket, zipPath, err)
writeErrorResponseHeadersOnly(w, toAPIError(ctx, err))
return
}
if opts.CheckPrecondFn(zipObjInfo) {
return
}
zipInfo := zipObjInfo.ArchiveInfo(r.Header)
if len(zipInfo) == 0 {
zipInfo, _, err = readObjectZIPInfo(ctx, objectAPI, bucket, zipPath, opts)
}
if isErrPreconditionFailed(err) {
return
}
if err != nil {
writeErrorResponseHeadersOnly(w, toAPIError(ctx, err))
return
@@ -470,6 +460,19 @@ func (api objectAPIHandlers) headObjectInArchiveFileHandler(ctx context.Context,
w.WriteHeader(http.StatusOK)
}
// archiveReadError preserves the anonymous ListBucket missing-object response.
// All other failures are disclosed only to callers authorized to read the ZIP.
func archiveReadError(ctx context.Context, r *http.Request, bucket, object string, err error) error {
if getRequestAuthType(r) == authTypeAnonymous && isErrObjectNotFound(err) &&
authorizeBucketRequest(ctx, r, policy.ListBucketAction) == ErrNone {
return err
}
if authorizeRequestWithUnreadTags(ctx, r, policy.GetObjectAction, err) != ErrNone {
return PrefixAccessDenied{Bucket: bucket, Object: object}
}
return err
}
// readObjectZIPInfo builds an index without modifying the object. A GET, HEAD
// or archive listing must remain read-only even when no index was stored.
func readObjectZIPInfo(ctx context.Context, objectAPI ObjectLayer, bucket, object string, opts ObjectOptions) ([]byte, ObjectInfo, error) {