mirror of
https://github.com/pgsty/minio.git
synced 2026-09-05 18:16:16 +03:00
fix: authorize explicit version deletes with DeleteObjectVersion
Select DeleteObject or DeleteObjectVersion from each request's effective version ID for single and multi-delete. Authenticate multi-delete once, bind version conditions to each XML entry, and keep the established DeleteObject plus ReplicateDelete target contract with explicit version denies honored only on trusted replication.\n\nRefs: #58 Signed-off-by: Feng Ruohang <rh@vonng.com>
This commit is contained in:
+52
-16
@@ -439,6 +439,23 @@ func authorizeRequest(ctx context.Context, r *http.Request, action policy.Action
|
||||
return authorizeRequestWithExistingTags(ctx, r, action, "")
|
||||
}
|
||||
|
||||
func deleteObjectAction(versionID string) policy.Action {
|
||||
if versionID != "" {
|
||||
return policy.DeleteObjectVersionAction
|
||||
}
|
||||
return policy.DeleteObjectAction
|
||||
}
|
||||
|
||||
func actionUsesObjectVersion(action policy.Action) bool {
|
||||
switch action {
|
||||
case policy.DeleteObjectAction, policy.DeleteObjectVersionAction,
|
||||
policy.ReplicateDeleteAction, policy.BypassGovernanceRetentionAction:
|
||||
return true
|
||||
default:
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
func authorizeRequestWithExistingTags(ctx context.Context, r *http.Request, action policy.Action, existingTags string) (s3Err APIErrorCode) {
|
||||
return authorizeRequestWithTags(ctx, r, action, existingTags, nil)
|
||||
}
|
||||
@@ -457,7 +474,7 @@ func authorizeRequestWithTags(ctx context.Context, r *http.Request, action polic
|
||||
versionID := reqInfo.VersionID
|
||||
conditionValuesForAuth := func(locationConstraint string, credentials auth.Credentials) map[string][]string {
|
||||
values := getConditionValuesWithTags(r, locationConstraint, credentials, existingTags, requestTags)
|
||||
if action == policy.DeleteObjectAction {
|
||||
if actionUsesObjectVersion(action) {
|
||||
// DeleteObjects carries the effective version ID in each XML object,
|
||||
// not in the request query. Keep authorization scoped to that entry.
|
||||
if versionID == "" {
|
||||
@@ -503,21 +520,6 @@ func authorizeRequestWithTags(ctx context.Context, r *http.Request, action polic
|
||||
|
||||
return ErrAccessDenied
|
||||
}
|
||||
if action == policy.DeleteObjectAction && versionID != "" {
|
||||
if !globalIAMSys.IsAllowed(policy.Args{
|
||||
AccountName: cred.AccessKey,
|
||||
Groups: cred.Groups,
|
||||
Action: policy.Action(policy.DeleteObjectVersionAction),
|
||||
BucketName: bucket,
|
||||
ConditionValues: conditionValuesForAuth("", cred),
|
||||
ObjectName: object,
|
||||
IsOwner: owner,
|
||||
Claims: cred.Claims,
|
||||
DenyOnly: true,
|
||||
}) { // Request is not allowed if Deny action on DeleteObjectVersionAction
|
||||
return ErrAccessDenied
|
||||
}
|
||||
}
|
||||
if globalIAMSys.IsAllowed(policy.Args{
|
||||
AccountName: cred.AccessKey,
|
||||
Groups: cred.Groups,
|
||||
@@ -553,6 +555,40 @@ func authorizeRequestWithTags(ctx context.Context, r *http.Request, action polic
|
||||
return ErrAccessDenied
|
||||
}
|
||||
|
||||
// authorizeReplicationDelete preserves the established target-credential
|
||||
// contract for trusted replication: DeleteObject and ReplicateDelete must be
|
||||
// allowed, while an explicit DeleteObjectVersion deny still blocks a named
|
||||
// version. Ordinary S3 requests never use this compatibility path.
|
||||
func authorizeReplicationDelete(ctx context.Context, r *http.Request) APIErrorCode {
|
||||
if s3Err := authorizeRequest(ctx, r, policy.DeleteObjectAction); s3Err != ErrNone {
|
||||
return s3Err
|
||||
}
|
||||
reqInfo := logger.GetReqInfo(ctx)
|
||||
if reqInfo == nil {
|
||||
return ErrAccessDenied
|
||||
}
|
||||
if reqInfo.VersionID == "" {
|
||||
return ErrNone
|
||||
}
|
||||
cred := reqInfo.Cred
|
||||
values := getConditionValuesWithTags(r, "", cred, "", nil)
|
||||
values["versionid"] = []string{reqInfo.VersionID}
|
||||
if !globalIAMSys.IsAllowed(policy.Args{
|
||||
AccountName: cred.AccessKey,
|
||||
Groups: cred.Groups,
|
||||
Action: policy.DeleteObjectVersionAction,
|
||||
BucketName: reqInfo.BucketName,
|
||||
ConditionValues: values,
|
||||
ObjectName: reqInfo.ObjectName,
|
||||
IsOwner: reqInfo.Owner,
|
||||
Claims: cred.Claims,
|
||||
DenyOnly: true,
|
||||
}) {
|
||||
return ErrAccessDenied
|
||||
}
|
||||
return ErrNone
|
||||
}
|
||||
|
||||
// Check request auth type verifies the incoming http request
|
||||
// - validates the request signature
|
||||
// - validates the policy action if anonymous tests bucket policies if any,
|
||||
|
||||
+14
-5
@@ -463,10 +463,6 @@ func (api objectAPIHandlers) DeleteMultipleObjectsHandler(w http.ResponseWriter,
|
||||
// Make sure to update context to print ObjectNames for multi objects.
|
||||
ctx = updateReqContext(ctx, objects...)
|
||||
|
||||
// Call checkRequestAuthType to populate ReqInfo.AccessKey before GetBucketInfo()
|
||||
// Ignore errors here to preserve the S3 error behavior of GetBucketInfo()
|
||||
checkRequestAuthType(ctx, r, policy.DeleteObjectAction, bucket, "")
|
||||
|
||||
deleteObjectsFn := objectAPI.DeleteObjects
|
||||
|
||||
// Return Malformed XML as S3 spec if the number of objects is empty
|
||||
@@ -474,6 +470,17 @@ func (api objectAPIHandlers) DeleteMultipleObjectsHandler(w http.ResponseWriter,
|
||||
writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrMalformedXML), r.URL)
|
||||
return
|
||||
}
|
||||
reqInfo := logger.GetReqInfo(ctx)
|
||||
if reqInfo == nil {
|
||||
writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrAccessDenied), r.URL)
|
||||
return
|
||||
}
|
||||
reqInfo.BucketName = bucket
|
||||
reqInfo.ObjectName = ""
|
||||
if s3Err := authenticateRequest(ctx, r, policy.DeleteObjectAction); s3Err != ErrNone {
|
||||
writeErrorResponse(ctx, w, errorCodes.ToAPIErr(s3Err), r.URL)
|
||||
return
|
||||
}
|
||||
|
||||
objectsToDelete := map[ObjectToDelete]int{}
|
||||
getObjectInfoFn := objectAPI.GetObjectInfo
|
||||
@@ -499,7 +506,9 @@ func (api objectAPIHandlers) DeleteMultipleObjectsHandler(w http.ResponseWriter,
|
||||
vc, _ := globalBucketVersioningSys.Get(bucket)
|
||||
oss := make([]*objSweeper, len(deleteObjectsReq.Objects))
|
||||
for index, object := range deleteObjectsReq.Objects {
|
||||
if apiErrCode := checkRequestAuthTypeWithVID(ctx, r, policy.DeleteObjectAction, bucket, object.ObjectName, object.VersionID); apiErrCode != ErrNone {
|
||||
reqInfo.ObjectName = object.ObjectName
|
||||
reqInfo.VersionID = object.VersionID
|
||||
if apiErrCode := authorizeRequest(ctx, r, deleteObjectAction(object.VersionID)); apiErrCode != ErrNone {
|
||||
if apiErrCode == ErrSignatureDoesNotMatch || apiErrCode == ErrInvalidAccessKeyID {
|
||||
writeErrorResponse(ctx, w, errorCodes.ToAPIErr(apiErrCode), r.URL)
|
||||
return
|
||||
|
||||
+26
-13
@@ -1018,14 +1018,23 @@ func testAPIDeleteMultipleObjectsVersionIDNullCondition(obj ObjectLayer, instanc
|
||||
|
||||
policyBytes := fmt.Appendf(nil, `{
|
||||
"Version":"2012-10-17",
|
||||
"Statement":[{
|
||||
"Statement":[
|
||||
{
|
||||
"Effect":"Allow",
|
||||
"Principal":"*",
|
||||
"Action":"s3:DeleteObject",
|
||||
"Resource":"arn:aws:s3:::%s/*",
|
||||
"Condition":{"Null":{"s3:versionid":"true"}}
|
||||
}]
|
||||
}`, bucketName)
|
||||
},
|
||||
{
|
||||
"Effect":"Allow",
|
||||
"Principal":"*",
|
||||
"Action":"s3:DeleteObjectVersion",
|
||||
"Resource":"arn:aws:s3:::%s/*",
|
||||
"Condition":{"StringEquals":{"s3:versionid":"%s"}}
|
||||
}
|
||||
]
|
||||
}`, bucketName, bucketName, versionIDs["with-version-id"])
|
||||
policyReq, err := newTestSignedRequestV4(http.MethodPut, getPutPolicyURL("", bucketName), int64(len(policyBytes)),
|
||||
bytes.NewReader(policyBytes), credentials.AccessKey, credentials.SecretKey, nil)
|
||||
if err != nil {
|
||||
@@ -1071,29 +1080,30 @@ func testAPIDeleteMultipleObjectsVersionIDNullCondition(obj ObjectLayer, instanc
|
||||
t.Errorf("%s: %q was not a successful delete-marker creation: %+v", instanceType, objectName, response.DeletedObjects)
|
||||
}
|
||||
}
|
||||
if len(deleted) != 2 {
|
||||
if object, ok := deleted["with-version-id"]; !ok || object.VersionID != versionIDs["with-version-id"] {
|
||||
t.Errorf("%s: matching explicit version was not deleted: %+v", instanceType, response.DeletedObjects)
|
||||
}
|
||||
if len(deleted) != 3 {
|
||||
t.Errorf("%s: unexpected deleted objects: %+v", instanceType, response.DeletedObjects)
|
||||
}
|
||||
errorsByKey := make(map[string]DeleteError, len(response.Errors))
|
||||
for _, deleteErr := range response.Errors {
|
||||
errorsByKey[deleteErr.Key] = deleteErr
|
||||
}
|
||||
for objectName, versionID := range map[string]string{
|
||||
"with-version-id": versionIDs["with-version-id"],
|
||||
"with-null-version-id": nullVersionID,
|
||||
} {
|
||||
for objectName, versionID := range map[string]string{"with-null-version-id": nullVersionID} {
|
||||
deleteErr, ok := errorsByKey[objectName]
|
||||
if !ok || deleteErr.VersionID != versionID || deleteErr.Code != errorCodes[ErrAccessDenied].Code {
|
||||
t.Errorf("%s: %q did not return AccessDenied for version %q: %+v", instanceType, objectName, versionID, response.Errors)
|
||||
}
|
||||
}
|
||||
if len(errorsByKey) != 2 {
|
||||
if len(errorsByKey) != 1 {
|
||||
t.Errorf("%s: unexpected delete errors: %+v", instanceType, response.Errors)
|
||||
}
|
||||
|
||||
// A simple delete adds a marker and keeps the old version. The explicitly
|
||||
// named version must also remain because its policy condition did not match.
|
||||
for objectName, versionID := range versionIDs {
|
||||
// A simple delete adds a marker and keeps the old version. The null-version
|
||||
// delete remains denied because its per-entry condition does not match.
|
||||
for _, objectName := range []string{"without-version-id-before", "without-version-id-after", "with-null-version-id"} {
|
||||
versionID := versionIDs[objectName]
|
||||
if _, err = obj.GetObjectInfo(t.Context(), bucketName, objectName, ObjectOptions{VersionID: versionID}); err != nil {
|
||||
t.Errorf("%s: version %s of %q was not preserved: %v", instanceType, versionID, objectName, err)
|
||||
}
|
||||
@@ -1103,7 +1113,10 @@ func testAPIDeleteMultipleObjectsVersionIDNullCondition(obj ObjectLayer, instanc
|
||||
t.Errorf("%s: simple delete of %q did not hide the latest object behind a delete marker: %v", instanceType, objectName, err)
|
||||
}
|
||||
}
|
||||
for _, objectName := range []string{"with-version-id", "with-null-version-id"} {
|
||||
if _, err = obj.GetObjectInfo(t.Context(), bucketName, "with-version-id", ObjectOptions{VersionID: versionIDs["with-version-id"]}); !isErrVersionNotFound(err) && !isErrObjectNotFound(err) {
|
||||
t.Errorf("%s: matching explicit version still exists: %v", instanceType, err)
|
||||
}
|
||||
for _, objectName := range []string{"with-null-version-id"} {
|
||||
if info, err := obj.GetObjectInfo(t.Context(), bucketName, objectName, ObjectOptions{}); err != nil {
|
||||
t.Errorf("%s: denied version delete removed latest %q: %v", instanceType, objectName, err)
|
||||
} else if info.VersionID != versionIDs[objectName] {
|
||||
|
||||
@@ -12,15 +12,36 @@ package cmd
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"encoding/xml"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"net/url"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/minio/madmin-go/v3"
|
||||
"github.com/minio/minio/internal/auth"
|
||||
xhttp "github.com/minio/minio/internal/http"
|
||||
"github.com/minio/pkg/v3/policy"
|
||||
)
|
||||
|
||||
func TestDeleteObjectAction(t *testing.T) {
|
||||
for _, test := range []struct {
|
||||
versionID string
|
||||
want policy.Action
|
||||
}{
|
||||
{want: policy.DeleteObjectAction},
|
||||
{versionID: nullVersionID, want: policy.DeleteObjectVersionAction},
|
||||
{versionID: mustGetUUID(), want: policy.DeleteObjectVersionAction},
|
||||
{versionID: " ", want: policy.DeleteObjectVersionAction},
|
||||
} {
|
||||
if got := deleteObjectAction(test.versionID); got != test.want {
|
||||
t.Errorf("deleteObjectAction(%q) = %s, want %s", test.versionID, got, test.want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestAPIDeleteObjectVersionAuthorization(t *testing.T) {
|
||||
defer DetectTestLeak(t)()
|
||||
ExecObjectLayerAPITest(ExecObjectLayerAPITestArgs{
|
||||
@@ -31,6 +52,109 @@ func TestAPIDeleteObjectVersionAuthorization(t *testing.T) {
|
||||
})
|
||||
}
|
||||
|
||||
func TestAPIDeleteMultipleObjectsVersionAuthorization(t *testing.T) {
|
||||
defer DetectTestLeak(t)()
|
||||
ExecObjectLayerAPITest(ExecObjectLayerAPITestArgs{
|
||||
t: t,
|
||||
objAPITest: testAPIDeleteMultipleObjectsVersionAuthorization,
|
||||
endpoints: []string{"DeleteMultipleObjects"},
|
||||
makeBucketOptions: MakeBucketOptions{VersioningEnabled: true},
|
||||
})
|
||||
}
|
||||
|
||||
func testAPIDeleteMultipleObjectsVersionAuthorization(obj ObjectLayer, instanceType, bucket string,
|
||||
apiRouter http.Handler, _ auth.Credentials, t *testing.T,
|
||||
) {
|
||||
deleteOnly := newObjectAttributesAuthzUser(t, instanceType, bucket, `"s3:DeleteObject"`)
|
||||
versionOnly := newObjectAttributesAuthzUser(t, instanceType, bucket, `"s3:DeleteObjectVersion"`)
|
||||
payload := []byte("multi delete version authorization")
|
||||
|
||||
put := func(t *testing.T, object string, versioned bool) string {
|
||||
t.Helper()
|
||||
info, err := obj.PutObject(t.Context(), bucket, object,
|
||||
mustGetPutObjReader(t, bytes.NewReader(payload), int64(len(payload)), "", ""), ObjectOptions{Versioned: versioned})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
return info.VersionID
|
||||
}
|
||||
request := func(t *testing.T, prefix string, creds auth.Credentials) (DeleteObjectsResponse, map[string]string) {
|
||||
t.Helper()
|
||||
versions := map[string]string{
|
||||
prefix + "simple": put(t, prefix+"simple", true),
|
||||
prefix + "explicit": put(t, prefix+"explicit", true),
|
||||
prefix + "null": put(t, prefix+"null", false),
|
||||
}
|
||||
body := encodeResponse(DeleteObjectsRequest{Objects: []ObjectToDelete{
|
||||
{ObjectV: ObjectV{ObjectName: prefix + "simple"}},
|
||||
{ObjectV: ObjectV{ObjectName: prefix + "explicit", VersionID: versions[prefix+"explicit"]}},
|
||||
{ObjectV: ObjectV{ObjectName: prefix + "null", VersionID: nullVersionID}},
|
||||
{ObjectV: ObjectV{ObjectName: prefix + "bad", VersionID: "not-a-uuid"}},
|
||||
}})
|
||||
target := getDeleteMultipleObjectsURL("", bucket) + "&versionId=query-level-decoy"
|
||||
req, err := newTestSignedRequestV4(http.MethodPost, target, int64(len(body)), bytes.NewReader(body),
|
||||
creds.AccessKey, creds.SecretKey, nil)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
rec := httptest.NewRecorder()
|
||||
apiRouter.ServeHTTP(rec, req)
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("%s: multi-delete status %d: %s", instanceType, rec.Code, rec.Body.String())
|
||||
}
|
||||
var response DeleteObjectsResponse
|
||||
if err = xml.Unmarshal(rec.Body.Bytes(), &response); err != nil {
|
||||
t.Fatalf("decode response: %v: %s", err, rec.Body.String())
|
||||
}
|
||||
return response, versions
|
||||
}
|
||||
responseMap := func(response DeleteObjectsResponse) (map[string]DeletedObject, map[string]DeleteError) {
|
||||
deleted := make(map[string]DeletedObject, len(response.DeletedObjects))
|
||||
for _, object := range response.DeletedObjects {
|
||||
deleted[object.ObjectName] = object
|
||||
}
|
||||
errs := make(map[string]DeleteError, len(response.Errors))
|
||||
for _, deleteErr := range response.Errors {
|
||||
errs[deleteErr.Key] = deleteErr
|
||||
}
|
||||
return deleted, errs
|
||||
}
|
||||
|
||||
t.Run("DeleteObject only", func(t *testing.T) {
|
||||
prefix := "multi-delete-only/"
|
||||
response, versions := request(t, prefix, deleteOnly)
|
||||
deleted, errs := responseMap(response)
|
||||
if object, ok := deleted[prefix+"simple"]; !ok || !object.DeleteMarker {
|
||||
t.Fatalf("simple delete did not create a marker: %+v", response)
|
||||
}
|
||||
for _, object := range []string{"explicit", "null", "bad"} {
|
||||
if got := errs[prefix+object].Code; got != errorCodes[ErrAccessDenied].Code {
|
||||
t.Errorf("%s error = %q, want AccessDenied", object, got)
|
||||
}
|
||||
}
|
||||
if _, err := obj.GetObjectInfo(t.Context(), bucket, prefix+"explicit", ObjectOptions{VersionID: versions[prefix+"explicit"]}); err != nil {
|
||||
t.Fatalf("denied explicit delete removed its version: %v", err)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("DeleteObjectVersion only", func(t *testing.T) {
|
||||
prefix := "multi-version-only/"
|
||||
response, _ := request(t, prefix, versionOnly)
|
||||
deleted, errs := responseMap(response)
|
||||
for _, object := range []string{"explicit", "null"} {
|
||||
if _, ok := deleted[prefix+object]; !ok {
|
||||
t.Errorf("%s was not deleted: %+v", object, response)
|
||||
}
|
||||
}
|
||||
if got := errs[prefix+"simple"].Code; got != errorCodes[ErrAccessDenied].Code {
|
||||
t.Errorf("simple error = %q, want AccessDenied", got)
|
||||
}
|
||||
if got := errs[prefix+"bad"].Code; got != errorCodes[ErrNoSuchVersion].Code {
|
||||
t.Errorf("bad UUID error = %q, want NoSuchVersion", got)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
func testAPIDeleteObjectVersionAuthorization(obj ObjectLayer, instanceType, bucket string,
|
||||
apiRouter http.Handler, _ auth.Credentials, t *testing.T,
|
||||
) {
|
||||
@@ -71,7 +195,7 @@ func testAPIDeleteObjectVersionAuthorization(obj ObjectLayer, instanceType, buck
|
||||
if rec := remove(t, object, versionID, versionOnly); rec.Code != http.StatusNoContent {
|
||||
t.Fatalf("status %d, want 204: %s", rec.Code, rec.Body.String())
|
||||
}
|
||||
if _, err := obj.GetObjectInfo(t.Context(), bucket, object, ObjectOptions{VersionID: versionID}); !isErrObjectNotFound(err) {
|
||||
if _, err := obj.GetObjectInfo(t.Context(), bucket, object, ObjectOptions{VersionID: versionID}); !isErrObjectNotFound(err) && !isErrVersionNotFound(err) {
|
||||
t.Fatalf("explicit version still exists: %v", err)
|
||||
}
|
||||
})
|
||||
@@ -117,8 +241,142 @@ func testAPIDeleteObjectVersionAuthorization(obj ObjectLayer, instanceType, buck
|
||||
if rec := remove(t, object, nullVersionID, versionOnly); rec.Code != http.StatusNoContent {
|
||||
t.Fatalf("status %d, want 204: %s", rec.Code, rec.Body.String())
|
||||
}
|
||||
if _, err := obj.GetObjectInfo(t.Context(), bucket, object, ObjectOptions{VersionID: nullVersionID}); !isErrObjectNotFound(err) {
|
||||
if _, err := obj.GetObjectInfo(t.Context(), bucket, object, ObjectOptions{VersionID: nullVersionID}); !isErrObjectNotFound(err) && !isErrVersionNotFound(err) {
|
||||
t.Fatalf("null version still exists: %v", err)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("authorization precedes invalid version parsing", func(t *testing.T) {
|
||||
object := "delete-authz/invalid-version"
|
||||
if rec := remove(t, object, "not-a-uuid", deleteOnly); rec.Code != http.StatusForbidden {
|
||||
t.Fatalf("delete-only status %d, want 403: %s", rec.Code, rec.Body.String())
|
||||
}
|
||||
if rec := remove(t, object, "not-a-uuid", versionOnly); rec.Code != http.StatusBadRequest {
|
||||
t.Fatalf("version-only status %d, want 400: %s", rec.Code, rec.Body.String())
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("padded version uses the effective ID", func(t *testing.T) {
|
||||
object := "delete-authz/padded-version"
|
||||
versionID := put(t, object, true)
|
||||
if rec := remove(t, object, versionID+" ", versionOnly); rec.Code != http.StatusNoContent {
|
||||
t.Fatalf("status %d, want 204: %s", rec.Code, rec.Body.String())
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
func TestAPIDeleteObjectVersionDenyAndReplicationCompatibility(t *testing.T) {
|
||||
defer DetectTestLeak(t)()
|
||||
ExecObjectLayerAPITest(ExecObjectLayerAPITestArgs{
|
||||
t: t,
|
||||
objAPITest: testAPIDeleteObjectVersionDenyAndReplicationCompatibility,
|
||||
endpoints: []string{"DeleteObject"},
|
||||
makeBucketOptions: MakeBucketOptions{VersioningEnabled: true},
|
||||
})
|
||||
}
|
||||
|
||||
func testAPIDeleteObjectVersionDenyAndReplicationCompatibility(obj ObjectLayer, instanceType, bucket string,
|
||||
apiRouter http.Handler, _ auth.Credentials, t *testing.T,
|
||||
) {
|
||||
payload := []byte("delete version deny compatibility")
|
||||
put := func(t *testing.T, object string) string {
|
||||
t.Helper()
|
||||
info, err := obj.PutObject(t.Context(), bucket, object,
|
||||
mustGetPutObjReader(t, bytes.NewReader(payload), int64(len(payload)), "", ""), ObjectOptions{Versioned: true})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
return info.VersionID
|
||||
}
|
||||
request := func(t *testing.T, object, versionID string, creds auth.Credentials, replicationRequest bool) *httptest.ResponseRecorder {
|
||||
t.Helper()
|
||||
target := getDeleteObjectURL("", bucket, object)
|
||||
if versionID != "" {
|
||||
target += "?" + url.Values{xhttp.VersionID: {versionID}}.Encode()
|
||||
}
|
||||
var headers map[string]string
|
||||
if replicationRequest {
|
||||
headers = map[string]string{
|
||||
xhttp.MinIOSourceReplicationRequest: "true",
|
||||
xhttp.AmzBucketReplicationStatus: "REPLICA",
|
||||
xhttp.MinIOSourceDeleteMarker: "false",
|
||||
xhttp.MinIOSourceMTime: UTCNow().Format(time.RFC3339Nano),
|
||||
}
|
||||
}
|
||||
req, err := newTestSignedRequestV4(http.MethodDelete, target, 0, nil, creds.AccessKey, creds.SecretKey, headers)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
rec := httptest.NewRecorder()
|
||||
apiRouter.ServeHTTP(rec, req)
|
||||
return rec
|
||||
}
|
||||
|
||||
minimal := newDeleteAuthzPolicyUser(t, instanceType, bucket, `[
|
||||
{"Effect":"Allow","Action":["s3:DeleteObject","s3:ReplicateDelete"],"Resource":["arn:aws:s3:::`+bucket+`/*"]}
|
||||
]`)
|
||||
denied := newDeleteAuthzPolicyUser(t, instanceType, bucket, `[
|
||||
{"Effect":"Allow","Action":["s3:DeleteObject","s3:DeleteObjectVersion","s3:ReplicateDelete"],"Resource":["arn:aws:s3:::`+bucket+`/*"]},
|
||||
{"Effect":"Deny","Action":"s3:DeleteObjectVersion","Resource":"arn:aws:s3:::`+bucket+`/deny/*"}
|
||||
]`)
|
||||
|
||||
t.Run("ordinary explicit deny wins", func(t *testing.T) {
|
||||
object := "deny/ordinary"
|
||||
versionID := put(t, object)
|
||||
if rec := request(t, object, versionID, denied, false); rec.Code != http.StatusForbidden {
|
||||
t.Fatalf("status %d, want 403: %s", rec.Code, rec.Body.String())
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("version deny does not block a simple delete", func(t *testing.T) {
|
||||
object := "deny/simple"
|
||||
put(t, object)
|
||||
if rec := request(t, object, "", denied, false); rec.Code != http.StatusNoContent {
|
||||
t.Fatalf("status %d, want 204: %s", rec.Code, rec.Body.String())
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("replication keeps minimal target policy", func(t *testing.T) {
|
||||
object := "replication/minimal"
|
||||
versionID := put(t, object)
|
||||
if rec := request(t, object, versionID, minimal, true); rec.Code != http.StatusNoContent {
|
||||
t.Fatalf("status %d, want 204: %s", rec.Code, rec.Body.String())
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("replication preserves explicit version deny", func(t *testing.T) {
|
||||
object := "deny/replication"
|
||||
versionID := put(t, object)
|
||||
if rec := request(t, object, versionID, denied, true); rec.Code != http.StatusForbidden {
|
||||
t.Fatalf("status %d, want 403: %s", rec.Code, rec.Body.String())
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
func newDeleteAuthzPolicyUser(t *testing.T, instanceType, bucket, statements string) auth.Credentials {
|
||||
t.Helper()
|
||||
accessKey, secretKey, err := auth.GenerateCredentials()
|
||||
if err != nil {
|
||||
t.Fatalf("%s: generate credentials: %v", instanceType, err)
|
||||
}
|
||||
creds := auth.Credentials{AccessKey: accessKey, SecretKey: secretKey}
|
||||
if _, err = globalIAMSys.CreateUser(t.Context(), accessKey, madmin.AddOrUpdateUserReq{
|
||||
SecretKey: secretKey,
|
||||
Status: madmin.AccountEnabled,
|
||||
}); err != nil {
|
||||
t.Fatalf("%s: create delete authz user: %v", instanceType, err)
|
||||
}
|
||||
policyJSON := `{"Version":"2012-10-17","Statement":` + statements + `}`
|
||||
parsed, err := policy.ParseConfig(strings.NewReader(policyJSON))
|
||||
if err != nil {
|
||||
t.Fatalf("%s: parse delete authz policy: %v", instanceType, err)
|
||||
}
|
||||
policyName := "delete-version-authz-" + mustGetUUID()
|
||||
if _, err = globalIAMSys.SetPolicy(t.Context(), policyName, *parsed); err != nil {
|
||||
t.Fatalf("%s: install delete authz policy: %v", instanceType, err)
|
||||
}
|
||||
if _, err = globalIAMSys.PolicyDBSet(t.Context(), accessKey, policyName, regUser, false); err != nil {
|
||||
t.Fatalf("%s: attach delete authz policy: %v", instanceType, err)
|
||||
}
|
||||
return creds
|
||||
}
|
||||
|
||||
+19
-2
@@ -2821,7 +2821,14 @@ func (api objectAPIHandlers) DeleteObjectHandler(w http.ResponseWriter, r *http.
|
||||
return
|
||||
}
|
||||
|
||||
if s3Error := checkRequestAuthType(ctx, r, policy.DeleteObjectAction, bucket, object); s3Error != ErrNone {
|
||||
reqInfo := logger.GetReqInfo(ctx)
|
||||
if reqInfo == nil {
|
||||
writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrAccessDenied), r.URL)
|
||||
return
|
||||
}
|
||||
reqInfo.BucketName = bucket
|
||||
reqInfo.ObjectName = object
|
||||
if s3Error := authenticateRequest(ctx, r, policy.DeleteObjectAction); s3Error != ErrNone {
|
||||
writeErrorResponse(ctx, w, errorCodes.ToAPIErr(s3Error), r.URL)
|
||||
return
|
||||
}
|
||||
@@ -2835,12 +2842,22 @@ func (api objectAPIHandlers) DeleteObjectHandler(w http.ResponseWriter, r *http.
|
||||
writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrAccessDenied), r.URL)
|
||||
return
|
||||
}
|
||||
trustedReplication := markerExact && replicationPermitted
|
||||
var s3Error APIErrorCode
|
||||
if trustedReplication {
|
||||
s3Error = authorizeReplicationDelete(ctx, r)
|
||||
} else {
|
||||
s3Error = authorizeRequest(ctx, r, deleteObjectAction(reqInfo.VersionID))
|
||||
}
|
||||
if s3Error != ErrNone {
|
||||
writeErrorResponse(ctx, w, errorCodes.ToAPIErr(s3Error), r.URL)
|
||||
return
|
||||
}
|
||||
if _, ok := r.Header[xhttp.MinIOSourceReplicationCheck]; ok {
|
||||
// requests to just validate replication settings and permissions are not allowed to delete data
|
||||
writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrReplicationPermissionCheckError), r.URL)
|
||||
return
|
||||
}
|
||||
trustedReplication := markerExact && replicationPermitted
|
||||
replica := trustedReplication && rawReplica
|
||||
if hasReplicationRequestHeaders(r.Header) {
|
||||
ctx, r = applyReplicationTrust(ctx, r, trustedReplication, replica)
|
||||
|
||||
Reference in New Issue
Block a user