Merge pull request #2368 from versity/sis/s3-conditional-write-condition-keys

feat: add s3:if-match and s3:if-none-match policy condition keys
This commit is contained in:
Ben McClelland
2026-09-09 08:57:30 -07:00
committed by GitHub
12 changed files with 1499 additions and 19 deletions
+6 -4
View File
@@ -116,7 +116,7 @@ func VerifyAccess(ctx fiber.Ctx, be backend.Backend, opts AccessOptions) error {
return err
}
errs, err := objectsAccessErrors(ctx.RequestCtx(), be, opts, []string{opts.Object}, requestConditionContext(ctx))
errs, err := objectsAccessErrors(ctx.RequestCtx(), be, opts, []string{opts.Object}, requestConditionContext(ctx, opts.Actions))
if err != nil {
return err
}
@@ -163,7 +163,9 @@ func VerifyObjectsAccess(ctx fiber.Ctx, be backend.Backend, opts AccessOptions,
}
rctx := ctx.RequestCtx()
condCtx := requestConditionContext(ctx)
// A DeleteObjects batch reads no If-Match/If-None-Match, so no
// conditional-write key applies to any object in it.
condCtx := requestConditionContext(ctx, nil)
keys := make([]string, len(objects))
for i, obj := range objects {
@@ -528,7 +530,7 @@ func VerifyPublicAccess(ctx fiber.Ctx, be backend.Backend, action Action, permis
return err
}
if err == nil {
err = VerifyPublicBucketPolicy(policy, bucket, object, requestConditionContext(ctx), be.NormalizeObjectKey, action)
err = VerifyPublicBucketPolicy(policy, bucket, object, requestConditionContext(ctx, []Action{action}), be.NormalizeObjectKey, action)
if errors.Is(err, errExplicitDeny) {
// Explicit public-policy Deny has higher precedence than any
// public ACL grant, so do not continue to ACL fallback.
@@ -619,7 +621,7 @@ func verifyIdentityOnlyAccess(ctx fiber.Ctx, pe PolicyEvaluator, acc Account, ac
Acc: acc,
Bucket: resource,
Actions: []Action{action},
}, []string{""}, nil, requestConditionContext(ctx))
}, []string{""}, nil, requestConditionContext(ctx, []Action{action}))
if err != nil {
return err
}
+29 -4
View File
@@ -69,6 +69,28 @@ func isVersionedAction(a Action) bool {
}
}
// isConditionalWriteAction is s3:if-match's applicable-action set:
// s3:PutObject — which also covers CompleteMultipartUpload, authorized as
// s3:PutObject — and s3:DeleteObject, S3's conditional delete. Note that
// s3:DeleteObjectVersion is not in the set: a versioned delete names the
// version to remove rather than overwriting the current one.
//
// addConditionalWriteKeys gates the runtime key on the same predicate, so
// narrowing or widening this set moves both ends at once.
func isConditionalWriteAction(a Action) bool {
return a == PutObjectAction || a == DeleteObjectAction
}
// isConditionalCreateAction is s3:if-none-match's applicable-action set:
// s3:PutObject alone. If-None-Match asserts the object doesn't exist yet,
// which only an upload can require — S3 rejects the header on DeleteObject.
//
// addConditionalWriteKeys gates the runtime key on the same predicate, so
// narrowing or widening this set moves both ends at once.
func isConditionalCreateAction(a Action) bool {
return a == PutObjectAction
}
// bucketPolicyConditionKeys is the fixed catalogue of condition keys this
// gateway's S3 bucket-policy Condition support recognizes, each mapped to
// the actions it may be used with. Keys are looked up case-insensitively
@@ -81,10 +103,8 @@ func isVersionedAction(a Action) bool {
// s3:RequestObjectTagKeys), object-lock keys, s3:x-amz-server-side-encryption
// (the gateway never reads that header, so enforcing it would be
// misleading), and aws:MultiFactorAuthAge (no MFA concept here) are out of
// scope. A Condition naming one of those is still accepted at write time
// the key just never appears in the runtime context, so any Condition
// depending on it simply never matches, the same as any other key this
// package doesn't populate.
// scope: a Condition naming one of those is rejected at write time, the
// same as any other key absent from this catalogue.
var bucketPolicyConditionKeys = map[string]conditionKeyRule{
// Generic keys: AWS accepts these with any action.
"aws:sourceip": {appliesTo: anyAction, ipSemantic: true},
@@ -104,6 +124,11 @@ var bucketPolicyConditionKeys = map[string]conditionKeyRule{
"s3:max-keys": {appliesTo: isListAction},
"s3:x-amz-acl": {appliesTo: isAclPutAction},
"s3:versionid": {appliesTo: isVersionedAction},
// Conditional-write keys, carrying the request's If-Match /
// If-None-Match header value.
"s3:if-match": {appliesTo: isConditionalWriteAction},
"s3:if-none-match": {appliesTo: isConditionalCreateAction},
}
// lookupConditionKeyRule finds key's rule case-insensitively.
+64
View File
@@ -137,6 +137,70 @@ func TestValidateBucketPolicyCondition(t *testing.T) {
raw: `{"StringEquals":{"s3:x-amz-acl":"public-read"}}`,
actions: actionSet(AllActions),
},
{
name: "s3:if-match accepted for PutObject",
raw: `{"StringEquals":{"s3:if-match":"abc123"}}`,
actions: actionSet(PutObjectAction),
},
{
// S3's conditional delete takes If-Match, so the key applies
// to DeleteObject as well as PutObject.
name: "s3:if-match accepted for DeleteObject",
raw: `{"StringEquals":{"s3:if-match":"abc123"}}`,
actions: actionSet(DeleteObjectAction),
},
{
name: "s3:if-match accepted for PutObject and DeleteObject together",
raw: `{"StringEquals":{"s3:if-match":"abc123"}}`,
actions: actionSet(PutObjectAction, DeleteObjectAction),
},
{
name: "s3:if-match rejected for GetObject",
raw: `{"StringEquals":{"s3:if-match":"abc123"}}`,
actions: actionSet(GetObjectAction),
wantErr: policyErrConditionActionMismatch,
},
{
// A versioned delete names the version to remove and takes no
// If-Match, so the key doesn't extend to it.
name: "s3:if-match rejected for DeleteObjectVersion",
raw: `{"StringEquals":{"s3:if-match":"abc123"}}`,
actions: actionSet(DeleteObjectVersionAction),
wantErr: policyErrConditionActionMismatch,
},
{
name: "s3:if-none-match accepted for PutObject",
raw: `{"Null":{"s3:if-none-match":"false"}}`,
actions: actionSet(PutObjectAction),
},
{
// If-None-Match asserts the object doesn't exist yet, which
// only an upload can require - unlike s3:if-match, DeleteObject
// doesn't take it.
name: "s3:if-none-match rejected for DeleteObject",
raw: `{"Null":{"s3:if-none-match":"false"}}`,
actions: actionSet(DeleteObjectAction),
wantErr: policyErrConditionActionMismatch,
},
{
name: "s3:if-none-match rejected for PutObject and DeleteObject together",
raw: `{"Null":{"s3:if-none-match":"false"}}`,
actions: actionSet(PutObjectAction, DeleteObjectAction),
wantErr: policyErrConditionActionMismatch,
},
{
name: "conditional-write keys recognized case-insensitively",
raw: `{"StringEquals":{"S3:IF-MATCH":"abc123"},"Null":{"s3:If-None-Match":"false"}}`,
actions: actionSet(PutObjectAction),
},
{
// The hyphens and the s3: prefix are part of the key name;
// dropping either names a key that doesn't exist.
name: "s3:ifmatch is not a recognized key",
raw: `{"StringEquals":{"s3:ifmatch":"abc123"}}`,
actions: actionSet(PutObjectAction),
wantErr: policyErrInvalidConditionKey,
},
{
name: "aws:SourceIp with a valid CIDR",
raw: `{"IpAddress":{"aws:SourceIp":"10.0.0.0/8"}}`,
+117 -1
View File
@@ -15,6 +15,7 @@
package auth
import (
"slices"
"strconv"
"time"
@@ -28,7 +29,11 @@ import (
// aws:username, aws:PrincipalTag/*, …) are deliberately absent: the S3
// gateway has no way to know them, so the IAM service fills them in itself
// when it evaluates an identity policy.
func requestConditionContext(ctx fiber.Ctx) map[string][]string {
//
// actions is the action set this request is being authorized under, used by
// the keys whose applicability AWS defines per action. Pass nil where no
// such key can apply.
func requestConditionContext(ctx fiber.Ctx, actions []Action) map[string][]string {
now := time.Now().UTC()
condCtx := map[string][]string{
"aws:CurrentTime": {now.Format(time.RFC3339)},
@@ -63,6 +68,117 @@ func requestConditionContext(ctx fiber.Ctx) map[string][]string {
if versionID := ctx.Query("versionId"); versionID != "" {
condCtx["s3:VersionId"] = []string{versionID}
}
addConditionalWriteKeys(ctx, actions, condCtx)
return condCtx
}
// addConditionalWriteKeys populates s3:if-match and s3:if-none-match from
// the request's If-Match/If-None-Match headers, each only on a request
// whose action the key applies to. The applicability rules are the ones
// PutBucketPolicy validates a Condition against, so a key can never reach
// the request context on an action a policy isn't allowed to name it on.
// The value is the header with its surrounding ETag quotes removed, so a
// policy compares against the bare ETag whichever form the client sent.
func addConditionalWriteKeys(ctx fiber.Ctx, actions []Action, condCtx map[string][]string) {
action := conditionalWriteAction(ctx, actions)
if isConditionalWriteAction(action) {
if ifMatch := trimETagQuotes(ctx.Get("If-Match")); ifMatch != "" {
condCtx["s3:if-match"] = []string{ifMatch}
}
}
if isConditionalCreateAction(action) {
if ifNoneMatch := trimETagQuotes(ctx.Get("If-None-Match")); ifNoneMatch != "" {
condCtx["s3:if-none-match"] = []string{ifNoneMatch}
}
}
}
// nonConditionalWriteSubresources names the query parameters that route an
// object PUT or DELETE to a handler other than PutObject/DeleteObject:
// tagging, retention, legal-hold and ACL writes, plus UploadPart and
// AbortMultipartUpload. UploadPart and UploadPartCopy are authorized as
// s3:PutObject just like PutObject itself, so only the route tells them
// apart.
var nonConditionalWriteSubresources = []string{"acl", "tagging", "retention", "legal-hold", "uploadId"}
// conditionalWriteAction reports the action ctx is authorized under, for
// the requests whose If-Match/If-None-Match the gateway enforces: PutObject
// and CompleteMultipartUpload, both authorized as s3:PutObject, and
// DeleteObject, which a versionId turns into s3:DeleteObjectVersion exactly
// as the handler does. It returns the empty action for everything else.
//
// Everything else ignores those headers, and a policy must never grant on a
// precondition that won't be checked — otherwise a form upload, a
// DeleteObjects batch, a copy or an upload part could satisfy a statement
// demanding a conditional write by sending a header that changes nothing.
// Reads are the same case: GET and HEAD take these headers as ordinary HTTP
// cache preconditions. Excluding a request leaves both keys absent, which
// denies it under such a policy rather than letting it through.
//
// The request shape alone doesn't identify an object write: a bucket
// sub-resource write is a PUT or DELETE carrying none of the object
// sub-resources, so it has to be recognized by what it is authorized as.
// actions is that set, and the shape's action must be in it — the two
// disagree exactly when the request routes somewhere else, as
// PutBucketVersioning, CreateBucket and DeleteBucket all do.
func conditionalWriteAction(ctx fiber.Ctx, actions []Action) Action {
action := conditionalWriteRouteAction(ctx)
if action == "" || !slices.Contains(actions, action) {
return ""
}
return action
}
// conditionalWriteRouteAction is conditionalWriteAction's request-shape
// half: the action this method, query and copy-source header would route
// to, before checking what the request is actually authorized as.
func conditionalWriteRouteAction(ctx fiber.Ctx) Action {
// A copy carries its preconditions in the X-Amz-Copy-Source-If-*
// headers, which name the source object and populate neither key. Both
// a copy and a plain upload are authorized as s3:PutObject, so only
// this header tells them apart.
if ctx.Get("X-Amz-Copy-Source") != "" {
return ""
}
query := ctx.Request().URI().QueryArgs()
switch string(ctx.Request().Header.Method()) {
case fiber.MethodPut:
if slices.ContainsFunc(nonConditionalWriteSubresources, query.Has) {
return ""
}
return PutObjectAction
case fiber.MethodDelete:
if slices.ContainsFunc(nonConditionalWriteSubresources, query.Has) {
return ""
}
// A delete naming a version removes that version rather than
// overwriting the current one, so it is authorized as
// s3:DeleteObjectVersion — an action neither key applies to. The
// gateway still enforces the precondition against the named
// version; a policy simply has no vocabulary to require it there,
// and the key stays absent so such a statement denies instead.
if query.Has("versionId") {
return DeleteObjectVersionAction
}
return DeleteObjectAction
case fiber.MethodPost:
// CompleteMultipartUpload is the only POST that enforces them.
if query.Has("uploadId") {
return PutObjectAction
}
return ""
default:
return ""
}
}
// trimETagQuotes strips one leading and one trailing double quote from an
// ETag-valued header, leaving any other value (notably If-None-Match's "*")
// untouched.
func trimETagQuotes(s string) string {
if len(s) >= 2 && s[0] == '"' && s[len(s)-1] == '"' {
return s[1 : len(s)-1]
}
return s
}
+304
View File
@@ -0,0 +1,304 @@
// Copyright 2026 Versity Software
// This file is licensed under the Apache License, Version 2.0
// (the "License"); you may not use this file except in compliance
// with the License. You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing,
// software distributed under the License is distributed on an
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.
package auth
import (
"testing"
"github.com/gofiber/fiber/v3"
"github.com/stretchr/testify/assert"
"github.com/valyala/fasthttp"
)
func TestRequestConditionContextConditionalWriteKeys(t *testing.T) {
tests := []struct {
name string
method string
ifMatch string
ifNoneMatch string
copySource string
// uri defaults to an object path when empty.
uri string
query string
actions []Action
want map[string][]string
}{
{
name: "no conditional headers",
method: fiber.MethodPut,
actions: []Action{PutObjectAction},
want: map[string][]string{},
},
{
name: "If-Match quotes are stripped",
method: fiber.MethodPut,
ifMatch: `"abc123"`,
actions: []Action{PutObjectAction},
want: map[string][]string{"s3:if-match": {"abc123"}},
},
{
// Both wire forms land on the same context value, so a policy
// always compares against the bare ETag.
name: "unquoted If-Match is taken as-is",
method: fiber.MethodPut,
ifMatch: "abc123",
actions: []Action{PutObjectAction},
want: map[string][]string{"s3:if-match": {"abc123"}},
},
{
// A multi-ETag If-Match is one value, not a list: the gateway
// compares the whole header against the object's ETag, and the
// key carries exactly what that comparison sees.
name: "a multi-ETag If-Match is one value",
method: fiber.MethodPut,
ifMatch: `"abc123", "def456"`,
actions: []Action{PutObjectAction},
want: map[string][]string{"s3:if-match": {`abc123", "def456`}},
},
{
name: "If-None-Match wildcard",
method: fiber.MethodPut,
ifNoneMatch: "*",
actions: []Action{PutObjectAction},
want: map[string][]string{"s3:if-none-match": {"*"}},
},
{
name: "both headers on one request",
method: fiber.MethodPut,
ifMatch: `"abc123"`,
ifNoneMatch: "*",
actions: []Action{PutObjectAction},
want: map[string][]string{
"s3:if-match": {"abc123"},
"s3:if-none-match": {"*"},
},
},
{
// An upload carrying tagging or lock headers is authorized
// under those actions too; s3:PutObject is still among them.
name: "an upload authorized under several actions",
method: fiber.MethodPut,
ifMatch: `"abc123"`,
actions: []Action{PutObjectAction, PutObjectTaggingAction, PutObjectRetentionAction},
want: map[string][]string{"s3:if-match": {"abc123"}},
},
{
name: "CompleteMultipartUpload populates the keys",
method: fiber.MethodPost,
query: "uploadId=abc",
ifMatch: `"abc123"`,
actions: []Action{PutObjectAction},
want: map[string][]string{"s3:if-match": {"abc123"}},
},
{
name: "DeleteObject populates s3:if-match",
method: fiber.MethodDelete,
ifMatch: `"abc123"`,
actions: []Action{DeleteObjectAction},
want: map[string][]string{"s3:if-match": {"abc123"}},
},
{
// A delete is authorized as s3:DeleteObject, which
// s3:if-none-match doesn't apply to — DeleteObject reads no
// If-None-Match, so the header changes nothing.
name: "a delete populates no s3:if-none-match",
method: fiber.MethodDelete,
ifMatch: `"abc123"`,
ifNoneMatch: "*",
actions: []Action{DeleteObjectAction},
want: map[string][]string{"s3:if-match": {"abc123"}},
},
{
// A versioned delete is authorized as s3:DeleteObjectVersion,
// which neither key applies to, even though the gateway does
// check the precondition against the named version.
name: "a versioned delete populates neither key",
method: fiber.MethodDelete,
query: "versionId=v1",
ifMatch: `"abc123"`,
ifNoneMatch: "*",
actions: []Action{DeleteObjectVersionAction},
want: map[string][]string{},
},
{
// A bucket sub-resource write is a bare PUT carrying none of
// the object sub-resources, so only the action it authorizes
// under separates it from an upload.
name: "PutBucketVersioning populates neither key",
method: fiber.MethodPut,
uri: "/bucket",
query: "versioning=",
ifMatch: `"abc123"`,
actions: []Action{PutBucketVersioningAction},
want: map[string][]string{},
},
{
name: "PutBucketPolicy populates neither key",
method: fiber.MethodPut,
uri: "/bucket",
query: "policy=",
ifMatch: `"abc123"`,
ifNoneMatch: "*",
actions: []Action{PutBucketPolicyAction},
want: map[string][]string{},
},
{
name: "CreateBucket populates neither key",
method: fiber.MethodPut,
uri: "/bucket",
ifMatch: `"abc123"`,
ifNoneMatch: "*",
actions: []Action{CreateBucketAction},
want: map[string][]string{},
},
{
name: "DeleteBucket populates neither key",
method: fiber.MethodDelete,
uri: "/bucket",
ifMatch: `"abc123"`,
actions: []Action{DeleteBucketAction},
want: map[string][]string{},
},
{
// The object-lock paths authorize s3:BypassGovernanceRetention,
// an action neither key applies to, on the very requests that
// do carry a precondition.
name: "a governance bypass populates neither key",
method: fiber.MethodPut,
ifMatch: `"abc123"`,
ifNoneMatch: "*",
actions: []Action{BypassGovernanceRetentionAction},
want: map[string][]string{},
},
{
name: "no actions populates neither key",
method: fiber.MethodPut,
ifMatch: `"abc123"`,
ifNoneMatch: "*",
want: map[string][]string{},
},
{
// A form upload and a DeleteObjects batch are POSTs that never
// look at the headers.
name: "a form upload populates neither key",
method: fiber.MethodPost,
ifMatch: `"abc123"`,
ifNoneMatch: "*",
actions: []Action{PutObjectAction},
want: map[string][]string{},
},
{
name: "a DeleteObjects batch populates neither key",
method: fiber.MethodPost,
uri: "/bucket",
query: "delete=",
ifMatch: `"abc123"`,
actions: []Action{DeleteObjectAction},
want: map[string][]string{},
},
{
name: "an upload part populates neither key",
method: fiber.MethodPut,
query: "partNumber=1&uploadId=abc",
ifMatch: `"abc123"`,
actions: []Action{PutObjectAction},
want: map[string][]string{},
},
{
name: "a tagging write populates neither key",
method: fiber.MethodPut,
query: "tagging=",
ifMatch: `"abc123"`,
actions: []Action{PutObjectTaggingAction},
want: map[string][]string{},
},
{
name: "an ACL write populates neither key",
method: fiber.MethodPut,
query: "acl=",
ifMatch: `"abc123"`,
actions: []Action{PutObjectAclAction},
want: map[string][]string{},
},
{
// On a read the same headers are ordinary HTTP cache
// preconditions, not conditional writes.
name: "GET populates neither key",
method: fiber.MethodGet,
ifMatch: `"abc123"`,
ifNoneMatch: `"abc123"`,
actions: []Action{GetObjectAction},
want: map[string][]string{},
},
{
name: "HEAD populates neither key",
method: fiber.MethodHead,
ifMatch: `"abc123"`,
ifNoneMatch: `"abc123"`,
actions: []Action{GetObjectAction},
want: map[string][]string{},
},
{
// A copy takes its preconditions from the
// X-Amz-Copy-Source-If-* headers, so a plain one is ignored.
// It is authorized as s3:PutObject like any other upload, so
// only the copy-source header separates the two.
name: "a copy populates neither key",
method: fiber.MethodPut,
ifMatch: `"abc123"`,
ifNoneMatch: "*",
copySource: "/src-bucket/src-key",
actions: []Action{PutObjectAction},
want: map[string][]string{},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
app := fiber.New()
fctx := &fasthttp.RequestCtx{}
fctx.Request.Header.SetMethod(tt.method)
uri := tt.uri
if uri == "" {
uri = "/bucket/object"
}
if tt.query != "" {
uri += "?" + tt.query
}
fctx.Request.SetRequestURI(uri)
if tt.ifMatch != "" {
fctx.Request.Header.Set("If-Match", tt.ifMatch)
}
if tt.ifNoneMatch != "" {
fctx.Request.Header.Set("If-None-Match", tt.ifNoneMatch)
}
if tt.copySource != "" {
fctx.Request.Header.Set("X-Amz-Copy-Source", tt.copySource)
}
ctx := app.AcquireCtx(fctx)
defer app.ReleaseCtx(ctx)
condCtx := requestConditionContext(ctx, tt.actions)
for _, key := range []string{"s3:if-match", "s3:if-none-match"} {
if want, ok := tt.want[key]; ok {
assert.Equal(t, want, condCtx[key])
continue
}
assert.NotContains(t, condCtx, key)
}
})
}
}
+2 -2
View File
@@ -234,7 +234,7 @@ func IsObjectLockRetentionPutAllowed(ctx fiber.Ctx, be backend.Backend, iam IAMS
// or switching it to COMPLIANCE — with the bypass header. That needs
// s3:BypassGovernanceRetention, via the bucket policy and/or (when
// configured) the IAM identity policy.
if err := verifyBypassGovernancePermission(ctx.RequestCtx(), be, iam, acc, bucket, object, BypassRequested, false, requestConditionContext(ctx)); err != nil {
if err := verifyBypassGovernancePermission(ctx.RequestCtx(), be, iam, acc, bucket, object, BypassRequested, false, requestConditionContext(ctx, []Action{BypassGovernanceRetentionAction})); err != nil {
debuglogger.Logf("the user is missing 's3:BypassGovernanceRetention' permission: %v", err)
return err
}
@@ -393,7 +393,7 @@ func CheckObjectAccess(ctx fiber.Ctx, bucket string, acc Account, objects []type
return err
}
condCtx := requestConditionContext(ctx)
condCtx := requestConditionContext(ctx, []Action{BypassGovernanceRetentionAction})
for _, obj := range objects {
if err := state.checkObject(rctx, be, iam, acc, bucket, obj, bypass, isBucketPublic, condCtx); err != nil {
return err
+324
View File
@@ -1677,3 +1677,327 @@ func AccessControl_bucket_policy_condition_not_ip_address_deny(s *S3Conf) error
return checkApiErr(err, s3err.GetAPIError(s3err.ErrAccessDenied))
})
}
// AccessControl_bucket_policy_condition_if_none_match_required is AWS's
// documented "enforce conditional writes" pattern: an unconditional Allow
// paired with a Deny that fires whenever the key is absent, forcing every
// upload to carry If-None-Match.
func AccessControl_bucket_policy_condition_if_none_match_required(s *S3Conf) error {
testName := "AccessControl_bucket_policy_condition_if_none_match_required"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
testuser := getUser("user")
if err := createUsers(s, []user{testuser}); err != nil {
return err
}
userClient := s.getUserClient(testuser)
if err := putBucketPolicyDoc(s, bucket,
bucketStatement{
Effect: "Allow",
Principal: testuser.access,
Action: "s3:PutObject",
Resource: fmt.Sprintf("arn:aws:s3:::%s/*", bucket),
},
bucketStatement{
Effect: "Deny",
Principal: testuser.access,
Action: "s3:PutObject",
Resource: fmt.Sprintf("arn:aws:s3:::%s/*", bucket),
Condition: json.RawMessage(`{"Null":{"s3:if-none-match":"true"}}`),
},
); err != nil {
return err
}
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
_, err := userClient.PutObject(ctx, &s3.PutObjectInput{
Bucket: &bucket,
Key: getPtr("unconditional"),
Body: bytes.NewReader([]byte("data")),
})
cancel()
if err := checkApiErr(err, s3err.GetExplicitDenyAccessErr(testuser.access, "s3:PutObject",
fmt.Sprintf("arn:aws:s3:::%s/unconditional", bucket), "a resource-based policy")); err != nil {
return fmt.Errorf("an upload without If-None-Match must be denied: %w", err)
}
ctx, cancel = context.WithTimeout(context.Background(), shortTimeout)
_, err = userClient.PutObject(ctx, &s3.PutObjectInput{
Bucket: &bucket,
Key: getPtr("conditional"),
Body: bytes.NewReader([]byte("data")),
IfNoneMatch: getPtr("*"),
})
cancel()
if err != nil {
return fmt.Errorf("an upload carrying If-None-Match must be allowed: %w", err)
}
// The same request once the key exists is still authorized; it now
// fails on the precondition itself, which the policy has no say in.
ctx, cancel = context.WithTimeout(context.Background(), shortTimeout)
_, err = userClient.PutObject(ctx, &s3.PutObjectInput{
Bucket: &bucket,
Key: getPtr("conditional"),
Body: bytes.NewReader([]byte("data")),
IfNoneMatch: getPtr("*"),
})
cancel()
return checkApiErr(err, s3err.GetAPIError(s3err.ErrPreconditionFailed))
})
}
// AccessControl_bucket_policy_condition_if_none_match_value covers the
// key's value rather than its presence: the only value S3 accepts in an
// If-None-Match write is "*", and that is what the key carries verbatim -
// under StringEquals "*" is a literal, not a wildcard.
func AccessControl_bucket_policy_condition_if_none_match_value(s *S3Conf) error {
testName := "AccessControl_bucket_policy_condition_if_none_match_value"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
testuser := getUser("user")
if err := createUsers(s, []user{testuser}); err != nil {
return err
}
userClient := s.getUserClient(testuser)
for i, tc := range []struct {
name string
condition string
wantAllow bool
}{
{"StringEquals on the literal wildcard matches", `{"StringEquals":{"s3:if-none-match":"*"}}`, true},
{"StringEquals on any other value does not", `{"StringEquals":{"s3:if-none-match":"abc123"}}`, false},
{"Null:false matches a present key", `{"Null":{"s3:if-none-match":"false"}}`, true},
{"Null:true does not match a present key", `{"Null":{"s3:if-none-match":"true"}}`, false},
{"StringLike wildcard matches", `{"StringLike":{"s3:if-none-match":"*"}}`, true},
} {
if err := putBucketPolicyDoc(s, bucket, bucketStatement{
Effect: "Allow",
Principal: testuser.access,
Action: "s3:PutObject",
Resource: fmt.Sprintf("arn:aws:s3:::%s/*", bucket),
Condition: json.RawMessage(tc.condition),
}); err != nil {
return fmt.Errorf("%s: %w", tc.name, err)
}
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
_, err := userClient.PutObject(ctx, &s3.PutObjectInput{
Bucket: &bucket,
Key: getPtr(fmt.Sprintf("obj-%v", i)),
Body: bytes.NewReader([]byte("data")),
IfNoneMatch: getPtr("*"),
})
cancel()
if tc.wantAllow {
if err != nil {
return fmt.Errorf("%s: expected success, got %w", tc.name, err)
}
continue
}
if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrAccessDenied)); err != nil {
return fmt.Errorf("%s: %w", tc.name, err)
}
}
return nil
})
}
// AccessControl_bucket_policy_condition_if_match_value covers
// s3:if-match's value shape: the key carries the ETag with the surrounding
// quotes stripped, so a policy compares against the bare ETag whichever
// form the client puts on the wire.
func AccessControl_bucket_policy_condition_if_match_value(s *S3Conf) error {
testName := "AccessControl_bucket_policy_condition_if_match_value"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
testuser := getUser("user")
if err := createUsers(s, []user{testuser}); err != nil {
return err
}
userClient := s.getUserClient(testuser)
etag, err := putObjectAndGetETag(s3client, bucket, "obj")
if err != nil {
return err
}
bare := strings.Trim(etag, `"`)
for _, tc := range []struct {
name string
condition func() string
ifMatch *string
wantAllow bool
azureUnsupp bool
}{
{"quoted header matches a bare policy value",
func() string { return fmt.Sprintf(`{"StringEquals":{"s3:if-match":%q}}`, bare) }, &etag, true, false},
{"unquoted header matches the same bare policy value",
func() string { return fmt.Sprintf(`{"StringEquals":{"s3:if-match":%q}}`, bare) }, &bare, true, false},
{"a policy value carrying the quotes never matches",
func() string { return fmt.Sprintf(`{"StringEquals":{"s3:if-match":%q}}`, etag) }, &etag, false, true},
{"a different ETag does not match",
func() string { return `{"StringEquals":{"s3:if-match":"0123456789abcdef0123456789abcdef"}}` }, &etag, false, false},
{"StringLike wildcard matches",
func() string { return `{"StringLike":{"s3:if-match":"*"}}` }, &etag, true, false},
{"Null:false matches a present key",
func() string { return `{"Null":{"s3:if-match":"false"}}` }, &etag, true, false},
} {
if tc.azureUnsupp && s.azureTests {
continue
}
if err := putBucketPolicyDoc(s, bucket, bucketStatement{
Effect: "Allow",
Principal: testuser.access,
Action: "s3:PutObject",
Resource: fmt.Sprintf("arn:aws:s3:::%s/*", bucket),
Condition: json.RawMessage(tc.condition()),
}); err != nil {
return fmt.Errorf("%s: %w", tc.name, err)
}
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
_, err := userClient.PutObject(ctx, &s3.PutObjectInput{
Bucket: &bucket,
Key: getPtr("obj"),
Body: bytes.NewReader([]byte("data")),
IfMatch: tc.ifMatch,
})
cancel()
if tc.wantAllow {
if err != nil {
return fmt.Errorf("%s: expected success, got %w", tc.name, err)
}
// The overwrite may have changed the ETag, so every
// subsequent case has to condition on the current one.
if etag, err = headObjectETag(s3client, bucket, "obj"); err != nil {
return err
}
bare = strings.Trim(etag, `"`)
continue
}
if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrAccessDenied)); err != nil {
return fmt.Errorf("%s: %w", tc.name, err)
}
}
return nil
})
}
// AccessControl_bucket_policy_condition_if_match_delete_object covers the
// half of s3:if-match that isn't an upload: S3's conditional delete, which
// is the only other action a bucket policy may name the key on.
func AccessControl_bucket_policy_condition_if_match_delete_object(s *S3Conf) error {
testName := "AccessControl_bucket_policy_condition_if_match_delete_object"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
testuser := getUser("user")
if err := createUsers(s, []user{testuser}); err != nil {
return err
}
userClient := s.getUserClient(testuser)
etag, err := putObjectAndGetETag(s3client, bucket, "obj")
if err != nil {
return err
}
// A delete that doesn't name the object's ETag is not authorized
// at all, so it never reaches the precondition check.
if err := putBucketPolicyDoc(s, bucket, bucketStatement{
Effect: "Allow",
Principal: testuser.access,
Action: "s3:DeleteObject",
Resource: fmt.Sprintf("arn:aws:s3:::%s/*", bucket),
Condition: json.RawMessage(fmt.Sprintf(`{"StringEquals":{"s3:if-match":%q}}`, strings.Trim(etag, `"`))),
}); err != nil {
return err
}
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
_, err = userClient.DeleteObject(ctx, &s3.DeleteObjectInput{
Bucket: &bucket,
Key: getPtr("obj"),
})
cancel()
if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrAccessDenied)); err != nil {
return fmt.Errorf("an unconditional delete must be denied: %w", err)
}
ctx, cancel = context.WithTimeout(context.Background(), shortTimeout)
_, err = userClient.DeleteObject(ctx, &s3.DeleteObjectInput{
Bucket: &bucket,
Key: getPtr("obj"),
IfMatch: &etag,
})
cancel()
if err != nil {
return fmt.Errorf("a delete naming the object's ETag must be allowed: %w", err)
}
return nil
})
}
// AccessControl_bucket_policy_condition_if_match_versioned_delete pins the
// edge of s3:if-match's action set. A delete naming a version removes that
// version rather than overwriting the current one, so it is authorized as
// s3:DeleteObjectVersion — an action the key doesn't apply to, which is why
// PutBucketPolicy rejects a statement pairing the two. The key has to stay
// absent from the request context to match, or a wildcard-action statement
// would grant on a precondition the policy language can't name.
func AccessControl_bucket_policy_condition_if_match_versioned_delete(s *S3Conf) error {
testName := "AccessControl_bucket_policy_condition_if_match_versioned_delete"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
testuser := getUser("user")
if err := createUsers(s, []user{testuser}); err != nil {
return err
}
userClient := s.getUserClient(testuser)
etag, err := putObjectAndGetETag(s3client, bucket, "obj")
if err != nil {
return err
}
// s3:* is the only action a bucket policy can name s3:if-match on
// and still reach a versioned delete: a wildcard is exempt from
// the applicability check an explicit s3:DeleteObjectVersion fails.
if err := putBucketPolicyDoc(s, bucket, bucketStatement{
Effect: "Allow",
Principal: testuser.access,
Action: "s3:*",
Resource: fmt.Sprintf("arn:aws:s3:::%s/*", bucket),
Condition: json.RawMessage(`{"Null":{"s3:if-match":"false"}}`),
}); err != nil {
return err
}
// "null" is the version id every object carries until versioning is
// enabled, so this is a versioned delete on any backend.
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
_, err = userClient.DeleteObject(ctx, &s3.DeleteObjectInput{
Bucket: &bucket,
Key: getPtr("obj"),
VersionId: getPtr("null"),
IfMatch: &etag,
})
cancel()
if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrAccessDenied)); err != nil {
return fmt.Errorf("a versioned delete must leave s3:if-match absent: %w", err)
}
// The same header on the same object, minus the version, is a plain
// s3:DeleteObject and does populate the key.
ctx, cancel = context.WithTimeout(context.Background(), shortTimeout)
_, err = userClient.DeleteObject(ctx, &s3.DeleteObjectInput{
Bucket: &bucket,
Key: getPtr("obj"),
IfMatch: &etag,
})
cancel()
if err != nil {
return fmt.Errorf("an unversioned delete carrying If-Match must be allowed: %w", err)
}
return nil
})
}
+50
View File
@@ -807,6 +807,19 @@ func PutBucketPolicy_condition_action_mismatch(s *S3Conf) error {
// an explicit multi-action list requires every action to
// support the key, even though s3:PutObject alone would.
{`["s3:GetObject","s3:PutObject"]`, `{"StringEquals":{"s3:x-amz-acl":"public-read"}}`},
// s3:if-match applies to the conditional-write actions only,
// so a read action is rejected...
{`"s3:GetObject"`, `{"StringEquals":{"s3:if-match":"abc123"}}`},
// ... and so is a versioned delete, which names the version to
// remove rather than taking an If-Match.
{`"s3:DeleteObjectVersion"`, `{"StringEquals":{"s3:if-match":"abc123"}}`},
// ... as is a bucket-level write, which reads no If-Match at
// all.
{`"s3:PutBucketVersioning"`, `{"StringEquals":{"s3:if-match":"abc123"}}`},
// s3:if-none-match is narrower still: only s3:PutObject can
// require that the object not already exist.
{`"s3:DeleteObject"`, `{"Null":{"s3:if-none-match":"false"}}`},
{`["s3:PutObject","s3:DeleteObject"]`, `{"Null":{"s3:if-none-match":"false"}}`},
} {
doc := fmt.Sprintf(`{"Statement":[{"Effect":"Allow","Principal":"*","Action":%s,
"Resource":"arn:aws:s3:::%s/*","Condition":%s}]}`, tc.action, bucket, tc.condition)
@@ -826,6 +839,43 @@ func PutBucketPolicy_condition_action_mismatch(s *S3Conf) error {
})
}
func PutBucketPolicy_condition_conditional_write_keys(s *S3Conf) error {
testName := "PutBucketPolicy_condition_conditional_write_keys"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
for _, tc := range []struct {
action string
condition string
}{
{`"s3:PutObject"`, `{"StringEquals":{"s3:if-match":"abc123"}}`},
{`"s3:PutObject"`, `{"Null":{"s3:if-none-match":"false"}}`},
// s3:if-match also covers S3's conditional delete.
{`"s3:DeleteObject"`, `{"StringEquals":{"s3:if-match":"abc123"}}`},
{`["s3:PutObject","s3:DeleteObject"]`, `{"StringEquals":{"s3:if-match":"abc123"}}`},
// a wildcard action is exempt from the applicability check
{`"s3:*"`, `{"Null":{"s3:if-none-match":"true"}}`},
// key names are case-insensitive
{`"s3:PutObject"`, `{"StringEquals":{"S3:IF-MATCH":"abc123"}}`},
{`"s3:PutObject"`, `{"Null":{"s3:If-None-Match":"false"}}`},
// both keys in one statement
{`"s3:PutObject"`, `{"Null":{"s3:if-match":"true","s3:if-none-match":"false"}}`},
} {
doc := fmt.Sprintf(`{"Statement":[{"Effect":"Allow","Principal":"*","Action":%s,
"Resource":"arn:aws:s3:::%s/*","Condition":%s}]}`, tc.action, bucket, tc.condition)
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
_, err := s3client.PutBucketPolicy(ctx, &s3.PutBucketPolicyInput{
Bucket: &bucket,
Policy: &doc,
})
cancel()
if err != nil {
return fmt.Errorf("action %s with condition %s: %w", tc.action, tc.condition, err)
}
}
return nil
})
}
func PutBucketPolicy_condition_invalid_ip(s *S3Conf) error {
testName := "PutBucketPolicy_condition_invalid_ip"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
+28
View File
@@ -633,6 +633,7 @@ func TestPutBucketPolicy(ts *TestState) {
ts.Run(PutBucketPolicy_condition_invalid_operator)
ts.Run(PutBucketPolicy_condition_invalid_key)
ts.Run(PutBucketPolicy_condition_action_mismatch)
ts.Run(PutBucketPolicy_condition_conditional_write_keys)
ts.Run(PutBucketPolicy_condition_invalid_ip)
}
@@ -1744,6 +1745,14 @@ func TestS3IAMAccessControl(ts *TestState) {
ts.Run(S3IAMAccessControl_condition_principal_tag)
ts.Run(S3IAMAccessControl_condition_on_deny_statement)
ts.Run(S3IAMAccessControl_condition_multiple_keys_anded)
ts.Run(S3IAMAccessControl_condition_if_none_match_required)
ts.Run(S3IAMAccessControl_condition_conditional_write_values)
ts.Run(S3IAMAccessControl_condition_if_match_delete_object)
ts.Run(S3IAMAccessControl_condition_if_match_versioned_delete)
ts.Run(S3IAMAccessControl_condition_conditional_write_keys_ignore_reads)
ts.Run(S3IAMAccessControl_condition_conditional_write_keys_ignore_copies)
ts.Run(S3IAMAccessControl_condition_if_match_bucket_level_write)
ts.Run(S3IAMAccessControl_condition_if_match_ignores_delete_objects)
ts.Run(S3IAMAccessControl_inactive_and_deleted_credentials)
ts.Run(S3IAMAccessControl_access_key_last_used_records_s3)
ts.Run(S3IAMPrincipal_accepted_forms)
@@ -1875,6 +1884,11 @@ func TestAccessControl(ts *TestState) {
ts.Run(AccessControl_bucket_policy_condition_bool_operator)
ts.Run(AccessControl_bucket_policy_condition_binary_operator)
ts.Run(AccessControl_bucket_policy_condition_null_operator)
ts.Run(AccessControl_bucket_policy_condition_if_none_match_required)
ts.Run(AccessControl_bucket_policy_condition_if_none_match_value)
ts.Run(AccessControl_bucket_policy_condition_if_match_value)
ts.Run(AccessControl_bucket_policy_condition_if_match_delete_object)
ts.Run(AccessControl_bucket_policy_condition_if_match_versioned_delete)
}
func TestPublicBuckets(ts *TestState) {
@@ -2201,6 +2215,14 @@ func GetIntTests() IntTests {
"S3IAMAccessControl_condition_principal_tag": S3IAMAccessControl_condition_principal_tag,
"S3IAMAccessControl_condition_on_deny_statement": S3IAMAccessControl_condition_on_deny_statement,
"S3IAMAccessControl_condition_multiple_keys_anded": S3IAMAccessControl_condition_multiple_keys_anded,
"S3IAMAccessControl_condition_if_none_match_required": S3IAMAccessControl_condition_if_none_match_required,
"S3IAMAccessControl_condition_conditional_write_values": S3IAMAccessControl_condition_conditional_write_values,
"S3IAMAccessControl_condition_if_match_delete_object": S3IAMAccessControl_condition_if_match_delete_object,
"S3IAMAccessControl_condition_if_match_versioned_delete": S3IAMAccessControl_condition_if_match_versioned_delete,
"S3IAMAccessControl_condition_conditional_write_keys_ignore_reads": S3IAMAccessControl_condition_conditional_write_keys_ignore_reads,
"S3IAMAccessControl_condition_conditional_write_keys_ignore_copies": S3IAMAccessControl_condition_conditional_write_keys_ignore_copies,
"S3IAMAccessControl_condition_if_match_bucket_level_write": S3IAMAccessControl_condition_if_match_bucket_level_write,
"S3IAMAccessControl_condition_if_match_ignores_delete_objects": S3IAMAccessControl_condition_if_match_ignores_delete_objects,
"S3IAMAccessControl_inactive_and_deleted_credentials": S3IAMAccessControl_inactive_and_deleted_credentials,
"S3IAMSession_bucket_policy_role_arn_covers_every_session": S3IAMSession_bucket_policy_role_arn_covers_every_session,
"S3IAMSession_bucket_policy_names_one_session": S3IAMSession_bucket_policy_names_one_session,
@@ -3167,6 +3189,7 @@ func GetIntTests() IntTests {
"PutBucketPolicy_condition_invalid_operator": PutBucketPolicy_condition_invalid_operator,
"PutBucketPolicy_condition_invalid_key": PutBucketPolicy_condition_invalid_key,
"PutBucketPolicy_condition_action_mismatch": PutBucketPolicy_condition_action_mismatch,
"PutBucketPolicy_condition_conditional_write_keys": PutBucketPolicy_condition_conditional_write_keys,
"PutBucketPolicy_condition_invalid_ip": PutBucketPolicy_condition_invalid_ip,
"GetBucketPolicy_non_existing_bucket": GetBucketPolicy_non_existing_bucket,
"GetBucketPolicy_not_set": GetBucketPolicy_not_set,
@@ -3388,6 +3411,11 @@ func GetIntTests() IntTests {
"AccessControl_bucket_policy_condition_bool_operator": AccessControl_bucket_policy_condition_bool_operator,
"AccessControl_bucket_policy_condition_binary_operator": AccessControl_bucket_policy_condition_binary_operator,
"AccessControl_bucket_policy_condition_null_operator": AccessControl_bucket_policy_condition_null_operator,
"AccessControl_bucket_policy_condition_if_none_match_required": AccessControl_bucket_policy_condition_if_none_match_required,
"AccessControl_bucket_policy_condition_if_none_match_value": AccessControl_bucket_policy_condition_if_none_match_value,
"AccessControl_bucket_policy_condition_if_match_value": AccessControl_bucket_policy_condition_if_match_value,
"AccessControl_bucket_policy_condition_if_match_delete_object": AccessControl_bucket_policy_condition_if_match_delete_object,
"AccessControl_bucket_policy_condition_if_match_versioned_delete": AccessControl_bucket_policy_condition_if_match_versioned_delete,
"PublicBucket_default_private_bucket": PublicBucket_default_private_bucket,
"PublicBucket_public_bucket_policy": PublicBucket_public_bucket_policy,
"PublicBucket_public_object_policy": PublicBucket_public_object_policy,
+511
View File
@@ -15,7 +15,9 @@
package integration
import (
"bytes"
"context"
"encoding/json"
"fmt"
"strings"
"time"
@@ -2103,6 +2105,515 @@ func S3IAMAccessControl_access_key_last_used_records_s3(s *S3Conf) error {
})
}
// S3IAMAccessControl_condition_if_none_match_required is AWS's documented
// "enforce conditional writes" pattern expressed as an identity policy: an
// unconditional Allow paired with a Deny that fires whenever the key is
// absent.
func S3IAMAccessControl_condition_if_none_match_required(s *S3Conf) error {
testName := "S3IAMAccessControl_condition_if_none_match_required"
return s3IAMActionHandler(s, testName, func(root *iam.Client, bucket string) error {
user, cleanup, err := newS3IAMUser(root, s, map[string]string{
"p": policyDoc(
accessStatement{Effect: "Allow", Action: actS3PutObject, Resource: objectsArn(bucket)},
accessStatement{
Effect: "Deny", Action: actS3PutObject, Resource: objectsArn(bucket),
Condition: cond("Null", "s3:if-none-match", "true"),
},
),
})
if err != nil {
return err
}
defer cleanup()
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
_, err = user.client.PutObject(ctx, &s3.PutObjectInput{
Bucket: &bucket,
Key: getPtr("unconditional"),
Body: bytes.NewReader([]byte("data")),
})
cancel()
if err := checkApiErr(err, wantExplicitIdentityDeny(user.arn, actS3PutObject, objectArn(bucket, "unconditional"))); err != nil {
return fmt.Errorf("an upload without If-None-Match must be denied: %w", err)
}
ctx, cancel = context.WithTimeout(context.Background(), shortTimeout)
_, err = user.client.PutObject(ctx, &s3.PutObjectInput{
Bucket: &bucket,
Key: getPtr("conditional"),
Body: bytes.NewReader([]byte("data")),
IfNoneMatch: getPtr("*"),
})
cancel()
if err != nil {
return fmt.Errorf("an upload carrying If-None-Match must be allowed: %w", err)
}
// Still authorized once the key exists; only the precondition
// itself fails now, which the policy has no say in.
ctx, cancel = context.WithTimeout(context.Background(), shortTimeout)
_, err = user.client.PutObject(ctx, &s3.PutObjectInput{
Bucket: &bucket,
Key: getPtr("conditional"),
Body: bytes.NewReader([]byte("data")),
IfNoneMatch: getPtr("*"),
})
cancel()
return checkApiErr(err, s3err.GetAPIError(s3err.ErrPreconditionFailed))
})
}
// S3IAMAccessControl_condition_conditional_write_values covers both keys'
// values on an upload: s3:if-match carries the ETag with its surrounding
// quotes stripped, and s3:if-none-match carries the only value S3 accepts
// on a write, the literal "*".
func S3IAMAccessControl_condition_conditional_write_values(s *S3Conf) error {
testName := "S3IAMAccessControl_condition_conditional_write_values"
return s3IAMActionHandler(s, testName, func(root *iam.Client, bucket string) error {
etag, err := putObjectAndGetETag(s.GetClient(), bucket, "obj")
if err != nil {
return err
}
bare := strings.Trim(etag, `"`)
user, cleanup, err := newS3IAMUser(root, s, nil)
if err != nil {
return err
}
defer cleanup()
for i, tc := range []struct {
name string
condition json.RawMessage
// ifMatch and ifNoneMatch are the headers the client sends;
// S3 rejects a request carrying both.
ifMatch *string
ifNoneMatch *string
wantAllow bool
}{
{
name: "quoted If-Match header matches a bare policy value",
condition: cond("StringEquals", "s3:if-match", bare),
ifMatch: &etag,
wantAllow: true,
},
{
name: "unquoted If-Match header matches the same bare policy value",
condition: cond("StringEquals", "s3:if-match", bare),
ifMatch: &bare,
wantAllow: true,
},
{
name: "a policy value carrying the quotes never matches",
condition: cond("StringEquals", "s3:if-match", etag),
ifMatch: &etag,
},
{
name: "a different ETag does not match",
condition: cond("StringEquals", "s3:if-match", "0123456789abcdef0123456789abcdef"),
ifMatch: &etag,
},
{
name: "s3:if-match is absent without the header",
condition: cond("Null", "s3:if-match", "true"),
wantAllow: true,
},
{
name: "an absent s3:if-match cannot satisfy StringEquals",
condition: cond("StringEquals", "s3:if-match", bare),
},
{
// Key names are case-insensitive, values are not.
name: "key name case is ignored",
condition: cond("StringEquals", "S3:IF-MATCH", bare),
ifMatch: &etag,
wantAllow: true,
},
{
name: "s3:if-none-match carries the literal wildcard",
condition: cond("StringEquals", "s3:if-none-match", "*"),
ifNoneMatch: getPtr("*"),
wantAllow: true,
},
{
// "*" is a literal under StringEquals, so it cannot stand
// in for an arbitrary value.
name: "s3:if-none-match is absent without the header",
condition: cond("StringEquals", "s3:if-none-match", "*"),
},
{
name: "one key is absent while the other is present",
condition: cond("Null", "s3:if-match", "true"),
ifNoneMatch: getPtr("*"),
wantAllow: true,
},
} {
// An If-None-Match write has to target a key that doesn't exist
// yet, or it is authorized and then fails the precondition -
// every other case conditions on "obj"'s own ETag.
key := "obj"
if tc.ifNoneMatch != nil {
key = fmt.Sprintf("absent-%v", i)
}
if err := func() error {
if err := putS3IAMUserPolicy(root, user, "p", policyDoc(accessStatement{
Effect: "Allow", Action: actS3PutObject, Resource: objectsArn(bucket),
Condition: tc.condition,
})); err != nil {
return err
}
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
_, err := user.client.PutObject(ctx, &s3.PutObjectInput{
Bucket: &bucket,
Key: &key,
Body: bytes.NewReader([]byte("data")),
IfMatch: tc.ifMatch,
IfNoneMatch: tc.ifNoneMatch,
})
cancel()
if !tc.wantAllow {
return checkApiErr(err, wantImplicitDeny(user.arn, actS3PutObject, objectArn(bucket, key)))
}
if err != nil {
return fmt.Errorf("expected the request to be allowed: %w", err)
}
// An overwrite may have changed the ETag, so the remaining
// cases have to condition on the current one.
if etag, err = headObjectETag(s.GetClient(), bucket, "obj"); err != nil {
return err
}
bare = strings.Trim(etag, `"`)
return nil
}(); err != nil {
return fmt.Errorf("%s: %w", tc.name, err)
}
}
return nil
})
}
// S3IAMAccessControl_condition_if_match_delete_object covers the half of
// s3:if-match that isn't an upload: S3's conditional delete.
func S3IAMAccessControl_condition_if_match_delete_object(s *S3Conf) error {
testName := "S3IAMAccessControl_condition_if_match_delete_object"
return s3IAMActionHandler(s, testName, func(root *iam.Client, bucket string) error {
etag, err := putObjectAndGetETag(s.GetClient(), bucket, "obj")
if err != nil {
return err
}
user, cleanup, err := newS3IAMUser(root, s, map[string]string{
"p": policyDoc(accessStatement{
Effect: "Allow", Action: actS3DeleteObject, Resource: objectsArn(bucket),
Condition: cond("StringEquals", "s3:if-match", strings.Trim(etag, `"`)),
}),
})
if err != nil {
return err
}
defer cleanup()
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
_, err = user.client.DeleteObject(ctx, &s3.DeleteObjectInput{Bucket: &bucket, Key: getPtr("obj")})
cancel()
if err := checkApiErr(err, wantImplicitDeny(user.arn, actS3DeleteObject, objectArn(bucket, "obj"))); err != nil {
return fmt.Errorf("an unconditional delete must be denied: %w", err)
}
ctx, cancel = context.WithTimeout(context.Background(), shortTimeout)
_, err = user.client.DeleteObject(ctx, &s3.DeleteObjectInput{
Bucket: &bucket,
Key: getPtr("obj"),
IfMatch: &etag,
})
cancel()
if err != nil {
return fmt.Errorf("a delete naming the object's ETag must be allowed: %w", err)
}
return nil
})
}
// S3IAMAccessControl_condition_if_match_versioned_delete pins the edge of
// s3:if-match's action set on the identity-policy side, where nothing
// validates a Condition's key against the action it names. A delete naming
// a version is authorized as s3:DeleteObjectVersion, which the key doesn't
// apply to, so it stays absent and a statement demanding it can never be
// satisfied
func S3IAMAccessControl_condition_if_match_versioned_delete(s *S3Conf) error {
testName := "S3IAMAccessControl_condition_if_match_versioned_delete"
return s3IAMActionHandler(s, testName, func(root *iam.Client, bucket string) error {
etag, err := putObjectAndGetETag(s.GetClient(), bucket, "obj")
if err != nil {
return err
}
user, cleanup, err := newS3IAMUser(root, s, map[string]string{
"p": policyDoc(accessStatement{
Effect: "Allow", Action: actS3DeleteObjectVersion, Resource: objectsArn(bucket),
Condition: cond("StringEquals", "s3:if-match", strings.Trim(etag, `"`)),
}),
})
if err != nil {
return err
}
defer cleanup()
// "null" is the version id every object carries until versioning is
// enabled, so this is a versioned delete on any backend.
deleteVersion := func(ifMatch string) error {
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
defer cancel()
_, err := user.client.DeleteObject(ctx, &s3.DeleteObjectInput{
Bucket: &bucket,
Key: getPtr("obj"),
VersionId: getPtr("null"),
IfMatch: &ifMatch,
})
return err
}
if err := checkApiErr(deleteVersion(etag),
wantImplicitDeny(user.arn, actS3DeleteObjectVersion, objectArn(bucket, "obj"))); err != nil {
return fmt.Errorf("a versioned delete must leave s3:if-match absent: %w", err)
}
// Absent, not merely different: the same request satisfies a
// statement requiring the key to be absent.
if err := putS3IAMUserPolicy(root, user, "p", policyDoc(accessStatement{
Effect: "Allow", Action: actS3DeleteObjectVersion, Resource: objectsArn(bucket),
Condition: cond("Null", "s3:if-match", "true"),
})); err != nil {
return err
}
// Authorized now, and the header still decides the delete: a
// mismatch fails the precondition the policy had no say in.
if err := checkApiErr(deleteVersion("0123456789abcdef0123456789abcdef"),
s3err.GetAPIError(s3err.ErrPreconditionFailed)); err != nil {
return fmt.Errorf("the precondition itself must still be enforced: %w", err)
}
if err := deleteVersion(etag); err != nil {
return fmt.Errorf("a versioned delete naming the object's ETag must be allowed: %w", err)
}
return nil
})
}
// S3IAMAccessControl_condition_if_match_bucket_level_write pins the last
// shape a bare PUT can take. A bucket sub-resource write carries none of
// the object sub-resources, so the request alone looks exactly like an
// upload; only the action it is authorized under separates the two. The
// gateway reads no If-Match there, so the key has to stay absent rather
// than satisfy a statement demanding a conditional write.
func S3IAMAccessControl_condition_if_match_bucket_level_write(s *S3Conf) error {
testName := "S3IAMAccessControl_condition_if_match_bucket_level_write"
return s3IAMActionHandler(s, testName, func(root *iam.Client, bucket string) error {
etag, err := putObjectAndGetETag(s.GetClient(), bucket, "obj")
if err != nil {
return err
}
bareETag := strings.Trim(etag, `"`)
user, cleanup, err := newS3IAMUser(root, s, map[string]string{
"p": policyDoc(accessStatement{
Effect: "Allow", Action: actS3PutBucketOwnershipControls, Resource: bucketArn(bucket),
Condition: cond("StringEquals", "s3:if-match", bareETag),
}),
})
if err != nil {
return err
}
defer cleanup()
putOwnership := func() error {
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
defer cancel()
_, err := user.client.PutBucketOwnershipControls(ctx, &s3.PutBucketOwnershipControlsInput{
Bucket: &bucket,
OwnershipControls: &types.OwnershipControls{
Rules: []types.OwnershipControlsRule{
{ObjectOwnership: types.ObjectOwnershipBucketOwnerPreferred},
},
},
}, withRequestHeader("If-Match", etag))
return err
}
if err := checkApiErr(putOwnership(),
wantImplicitDeny(user.arn, actS3PutBucketOwnershipControls, bucketArn(bucket))); err != nil {
return fmt.Errorf("a bucket-level write must leave s3:if-match absent: %w", err)
}
// Absent, not merely different: the same request satisfies a
// statement requiring the key to be absent.
if err := putS3IAMUserPolicy(root, user, "p", policyDoc(accessStatement{
Effect: "Allow", Action: actS3PutBucketOwnershipControls, Resource: bucketArn(bucket),
Condition: cond("Null", "s3:if-match", "true"),
})); err != nil {
return err
}
if err := putOwnership(); err != nil {
return fmt.Errorf("a bucket-level write must be allowed once the statement stops demanding the key: %w", err)
}
return nil
})
}
// S3IAMAccessControl_condition_conditional_write_keys_ignore_copies pins
// the other half of that rule. A copy takes its preconditions from the
// x-amz-copy-source-if-* headers, so the gateway ignores a plain If-Match or
// If-None-Match on one — and a policy demanding a conditional write must
// therefore keep denying copies rather than be satisfied by a header that
// changes nothing.
func S3IAMAccessControl_condition_conditional_write_keys_ignore_copies(s *S3Conf) error {
testName := "S3IAMAccessControl_condition_conditional_write_keys_ignore_copies"
return s3IAMActionHandler(s, testName, func(root *iam.Client, bucket string) error {
if _, err := putObjectAndGetETag(s.GetClient(), bucket, "src"); err != nil {
return err
}
user, cleanup, err := newS3IAMUser(root, s, map[string]string{
"p": policyDoc(
accessStatement{Effect: "Allow", Action: actS3GetObject, Resource: objectsArn(bucket)},
accessStatement{Effect: "Allow", Action: actS3PutObject, Resource: objectsArn(bucket)},
accessStatement{
Effect: "Deny", Action: actS3PutObject, Resource: objectsArn(bucket),
Condition: cond("Null", "s3:if-none-match", "true"),
},
),
})
if err != nil {
return err
}
defer cleanup()
for _, tc := range []struct {
name string
ifNoneMatch *string
}{
{"a copy without the header", nil},
{"a copy carrying the header", getPtr("*")},
} {
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
_, err := user.client.CopyObject(ctx, &s3.CopyObjectInput{
Bucket: &bucket,
Key: getPtr("dst"),
CopySource: getPtr(bucket + "/src"),
IfNoneMatch: tc.ifNoneMatch,
})
cancel()
if err := checkApiErr(err, wantExplicitIdentityDeny(user.arn, actS3PutObject, objectArn(bucket, "dst"))); err != nil {
return fmt.Errorf("%s: %w", tc.name, err)
}
}
return nil
})
}
// S3IAMAccessControl_condition_if_match_ignores_delete_objects covers the
// batch delete. DeleteObjects carries one request-level header for the
// whole batch and the gateway never applies it to any key, so letting it
// populate s3:if-match would authorize deleting every object in the batch
// against an ETag nothing checks.
func S3IAMAccessControl_condition_if_match_ignores_delete_objects(s *S3Conf) error {
testName := "S3IAMAccessControl_condition_if_match_ignores_delete_objects"
return s3IAMActionHandler(s, testName, func(root *iam.Client, bucket string) error {
etag, err := putObjectAndGetETag(s.GetClient(), bucket, "obj")
if err != nil {
return err
}
user, cleanup, err := newS3IAMUser(root, s, map[string]string{
"p": policyDoc(accessStatement{
Effect: "Allow", Action: actS3DeleteObject, Resource: objectsArn(bucket),
Condition: cond("StringEquals", "s3:if-match", strings.Trim(etag, `"`)),
}),
})
if err != nil {
return err
}
defer cleanup()
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
res, err := user.client.DeleteObjects(ctx, &s3.DeleteObjectsInput{
Bucket: &bucket,
Delete: &types.Delete{Objects: objectIdentifiers("obj")},
})
cancel()
if err != nil {
return err
}
if len(res.Errors) != 1 || getString(res.Errors[0].Code) != "AccessDenied" {
return fmt.Errorf("expected the batch delete to be denied, instead got %+v", res)
}
return nil
})
}
// S3IAMAccessControl_condition_conditional_write_keys_ignore_reads pins the
// keys to writes. GET and HEAD take If-Match/If-None-Match too, as ordinary
// HTTP cache preconditions, and neither may populate the condition context
// there — otherwise a browser revalidating its cache would decide whether a
// read is authorized.
func S3IAMAccessControl_condition_conditional_write_keys_ignore_reads(s *S3Conf) error {
testName := "S3IAMAccessControl_condition_conditional_write_keys_ignore_reads"
return s3IAMActionHandler(s, testName, func(root *iam.Client, bucket string) error {
etag, err := putObjectAndGetETag(s.GetClient(), bucket, "obj")
if err != nil {
return err
}
user, cleanup, err := newS3IAMUser(root, s, nil)
if err != nil {
return err
}
defer cleanup()
for _, tc := range []struct {
name string
condition json.RawMessage
wantAllow bool
}{
{"s3:if-match stays absent on a conditional read", cond("Null", "s3:if-match", "true"), true},
{"s3:if-none-match stays absent on a conditional read", cond("Null", "s3:if-none-match", "true"), true},
{"a read can never satisfy a present-key condition", cond("Null", "s3:if-match", "false"), false},
{"nor an equality against the ETag it sent", cond("StringEquals", "s3:if-match", strings.Trim(etag, `"`)), false},
} {
if err := func() error {
if err := putS3IAMUserPolicy(root, user, "p", policyDoc(accessStatement{
Effect: "Allow", Action: actS3GetObject, Resource: objectsArn(bucket),
Condition: tc.condition,
})); err != nil {
return err
}
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
_, err := user.client.GetObject(ctx, &s3.GetObjectInput{
Bucket: &bucket,
Key: getPtr("obj"),
IfMatch: &etag,
})
cancel()
if !tc.wantAllow {
return checkApiErr(err, wantImplicitDeny(user.arn, actS3GetObject, objectArn(bucket, "obj")))
}
if err != nil {
return fmt.Errorf("expected the request to be allowed: %w", err)
}
return nil
}(); err != nil {
return fmt.Errorf("%s: %w", tc.name, err)
}
}
return nil
})
}
// containsBucket reports whether buckets names bucket, so a listing can be
// asserted without depending on what else other tests left behind.
func containsBucket(buckets []types.Bucket, bucket string) bool {
+9 -8
View File
@@ -38,14 +38,15 @@ import (
)
const (
actS3GetObject = "s3:GetObject"
actS3PutObject = "s3:PutObject"
actS3DeleteObject = "s3:DeleteObject"
actS3DeleteObjectVersion = "s3:DeleteObjectVersion"
actS3ListBucket = "s3:ListBucket"
actS3CreateBucket = "s3:CreateBucket"
actS3ListAllMyBuckets = "s3:ListAllMyBuckets"
actS3BypassGovernance = "s3:BypassGovernanceRetention"
actS3GetObject = "s3:GetObject"
actS3PutObject = "s3:PutObject"
actS3DeleteObject = "s3:DeleteObject"
actS3DeleteObjectVersion = "s3:DeleteObjectVersion"
actS3ListBucket = "s3:ListBucket"
actS3CreateBucket = "s3:CreateBucket"
actS3ListAllMyBuckets = "s3:ListAllMyBuckets"
actS3BypassGovernance = "s3:BypassGovernanceRetention"
actS3PutBucketOwnershipControls = "s3:PutBucketOwnershipControls"
)
const (
+55
View File
@@ -1017,6 +1017,61 @@ func putObjects(client *s3.Client, objs []string, bucket string) ([]types.Object
return contents, nil
}
// putObjectAndGetETag uploads an object and returns its ETag as the wire
// carries it, quotes included, for the conditional-write tests that have to
// name a real ETag in an If-Match header or a policy condition.
func putObjectAndGetETag(client *s3.Client, bucket, key string) (string, error) {
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
defer cancel()
res, err := client.PutObject(ctx, &s3.PutObjectInput{
Bucket: &bucket,
Key: &key,
Body: bytes.NewReader([]byte("data")),
})
if err != nil {
return "", err
}
return getString(res.ETag), nil
}
// withRequestHeader sets a raw header on an SDK request, for the cases no
// input field reaches — an If-Match on a bucket-level write, say. The
// middleware runs right before signing so the header is signed like any
// other.
func withRequestHeader(key, value string) func(*s3.Options) {
return func(o *s3.Options) {
o.APIOptions = append(o.APIOptions, func(stack *middleware.Stack) error {
return stack.Finalize.Insert(
middleware.FinalizeMiddlewareFunc("SetRequestHeader",
func(ctx context.Context, in middleware.FinalizeInput, next middleware.FinalizeHandler) (
out middleware.FinalizeOutput, md middleware.Metadata, err error,
) {
if req, ok := in.Request.(*smithyhttp.Request); ok {
req.Header.Set(key, value)
}
return next.HandleFinalize(ctx, in)
}),
"Signing",
middleware.Before,
)
})
}
}
// headObjectETag re-reads an object's current ETag, which an overwrite may
// have changed.
func headObjectETag(client *s3.Client, bucket, key string) (string, error) {
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
defer cancel()
res, err := client.HeadObject(ctx, &s3.HeadObjectInput{Bucket: &bucket, Key: &key})
if err != nil {
return "", err
}
return getString(res.ETag), nil
}
func listObjects(client *s3.Client, bucket, prefix, delimiter string, maxKeys int32) ([]types.Object, []types.CommonPrefix, error) {
var contents []types.Object
var commonPrefixes []types.CommonPrefix