From f40267db446bdc7cc7fb3718b35e08a0005b42f4 Mon Sep 17 00:00:00 2001 From: Javier de la Torre Date: Wed, 21 Oct 2026 17:08:33 -0400 Subject: [PATCH] s3: answer InvalidBucketName to a denied request on a malformed bucket name (#11893) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * s3: answer InvalidBucketName to a denied request on a malformed bucket name The store authorizes a request before it looks at the bucket, so a caller denied on a bucket whose name breaks the S3 naming rules (upper case, an underscore, too short, two adjacent periods, a non-ASCII letter) gets 403 AccessDenied, where AWS S3 answers 400 InvalidBucketName. Clients and compatibility suites that check the error code read the 403 as a permission problem. Auth and AuthPostPolicy now turn an AccessDenied for a resolved identity into InvalidBucketName when the bucket name fails VerifyS3BucketName or uses a character outside a-z, 0-9, '.' and '-' (VerifyS3BucketName checks with unicode.IsLower/IsNumber, which accept letters such as 'ü'). The answer depends on the name alone, so it reveals nothing about which buckets exist. An authentication failure, an allowed request and a denial on a well-formed name answer as before. Test: TestAuthDeniedInvalidBucketName drives iam.Auth with signed and anonymous requests; on master every malformed name answers 403, with the change 400, while the identity's own bucket stays allowed and a well-formed foreign name stays 403. TestRefineDenialForBucketName covers the rule table. * s3: refine POST-policy and deferred anonymous-read denials on a malformed bucket name Two denial paths bypass the refinement in Auth and AuthPostPolicy: - A browser POST upload carries its credentials in the form, so AuthPostPolicy delegates it to PostPolicyBucketHandler, which verifies the form and writes AccessDenied itself when the signer may not write the bucket or the per-key authorization denies the object. Both denials now go through refineDenialForBucketName with the verified identity. The coarse Write check used to run inside doesPolicySignatureV2Match and doesPolicySignatureV4Match before the signature was compared, so a form with a wrong signature was told whether the key may write the bucket. It now runs in doesPolicySignatureMatch after the signature verifies, as on the header-signed path; a form that fails verification answers SignatureDoesNotMatch (or MissingFields) and is never refined. - Unsigned GET/HEAD object reads are deferred by AuthWithPublicRead to the object handlers, where writeAnonymousAwareNotFound masks a miss as AccessDenied. That denial is refined with the anonymous identity, so a missing malformed bucket answers InvalidBucketName when an anonymous identity is configured, as Auth does. A denial on an entry that was found is left alone: the bucket exists. Test: TestPostPolicyDeniedInvalidBucketName drives AuthPostPolicy with signed forms (Write denied, per-key policy deny, bad signature, well-formed name); TestAnonymousReadDeniedInvalidBucketName drives AuthWithPublicRead with GET and HEAD, with and without an anonymous identity. Both fail before this change. * s3: refine a denied anonymous read of an existing key on a malformed bucket name A deferred anonymous GET/HEAD refined only its masked miss to InvalidBucketName, so in a bucket whose name breaks the S3 rules (one VerifyS3BucketName still accepts, such as a non-ASCII name) a missing key answered 400 and an existing private key 403, telling the caller which keys exist. The entry check now refines its denial the same way, so both answer alike. Without an anonymous identity no identity resolves and both stay 403, as in Auth. The deny-precedence case in TestAnonymousObjectACLConfiguredIdentity runs on the fixture bucket "b", shorter than S3's three-character minimum, so its denial now answers InvalidBucketName. --------- Co-authored-by: Javier de la Torre --- weed/s3api/auth_credentials.go | 36 +++ weed/s3api/auth_invalid_bucket_name_test.go | 250 ++++++++++++++++++ weed/s3api/auth_signature_v2.go | 6 - weed/s3api/auth_signature_v4.go | 5 - weed/s3api/s3api_object_anonymous_read.go | 6 +- .../s3api/s3api_object_anonymous_read_test.go | 4 +- .../s3api/s3api_object_handlers_postpolicy.go | 22 +- weed/s3api/s3api_server.go | 5 +- 8 files changed, 315 insertions(+), 19 deletions(-) create mode 100644 weed/s3api/auth_invalid_bucket_name_test.go diff --git a/weed/s3api/auth_credentials.go b/weed/s3api/auth_credentials.go index 8a0491e7b1..bb05bb5335 100644 --- a/weed/s3api/auth_credentials.go +++ b/weed/s3api/auth_credentials.go @@ -25,6 +25,7 @@ import ( "github.com/seaweedfs/seaweedfs/weed/pb/iam_pb" "github.com/seaweedfs/seaweedfs/weed/s3api/policy_engine" "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" + "github.com/seaweedfs/seaweedfs/weed/s3api/s3bucket" "github.com/seaweedfs/seaweedfs/weed/s3api/s3err" "github.com/seaweedfs/seaweedfs/weed/util/wildcard" "github.com/seaweedfs/seaweedfs/weed/wdclient" @@ -1680,6 +1681,7 @@ func (iam *IdentityAccessManagement) Auth(f http.HandlerFunc, action Action) htt // succeeds but authZ denies, so the denied request still audits its // requester; a failed authN resolves no identity at all. identity, errCode, _ := iam.authRequestWithAuthType(r, action) + errCode = refineDenialForBucketName(r, identity, errCode) if errCode != s3err.ErrNone { glog.V(3).Infof("auth error: %v", errCode) } @@ -1719,6 +1721,7 @@ func (iam *IdentityAccessManagement) AuthPostPolicy(f http.HandlerFunc, action A } } + errCode = refineDenialForBucketName(r, identity, errCode) if errCode != s3err.ErrNone { glog.V(3).Infof("auth error: %v", errCode) } @@ -1727,6 +1730,39 @@ func (iam *IdentityAccessManagement) AuthPostPolicy(f http.HandlerFunc, action A } } +// refineDenialForBucketName answers 400 InvalidBucketName, as AWS S3 does, +// when an authenticated caller is denied on a bucket whose name breaks the S3 +// naming rules. The rules are public and the answer depends on the name alone, +// so it tells the caller nothing about which buckets exist. Authentication +// failures (no identity resolved) and every other answer are left unchanged. +func refineDenialForBucketName(r *http.Request, identity *Identity, errCode s3err.ErrorCode) s3err.ErrorCode { + if errCode != s3err.ErrAccessDenied || identity == nil { + return errCode + } + bucket, _ := s3_constants.GetBucketAndObject(r) + if bucket == "" { + return errCode + } + if !isASCIIBucketName(bucket) || s3bucket.VerifyS3BucketName(bucket) != nil { + return s3err.ErrInvalidBucketName + } + return errCode +} + +// isASCIIBucketName reports whether name uses only the characters S3 allows +// in a bucket name. VerifyS3BucketName checks with unicode.IsLower and +// unicode.IsNumber, which also accept letters such as 'ü' and digits such as +// '²' that S3 refuses. +func isASCIIBucketName(name string) bool { + for i := 0; i < len(name); i++ { + c := name[i] + if !(c >= 'a' && c <= 'z' || c >= '0' && c <= '9' || c == '.' || c == '-') { + return false + } + } + return true +} + // recordIdentityInContext stores the authenticated identity, its name and its // principal ARN in the request context. An STS session's name is only an opaque // session subject, so the ARN is what carries the assumed role and session name diff --git a/weed/s3api/auth_invalid_bucket_name_test.go b/weed/s3api/auth_invalid_bucket_name_test.go new file mode 100644 index 0000000000..e702d49001 --- /dev/null +++ b/weed/s3api/auth_invalid_bucket_name_test.go @@ -0,0 +1,250 @@ +package s3api + +import ( + "bytes" + "encoding/base64" + "fmt" + "mime/multipart" + "net/http" + "net/http/httptest" + "sync" + "testing" + "time" + + "github.com/gorilla/mux" + "github.com/seaweedfs/seaweedfs/weed/pb/iam_pb" + "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" + "github.com/seaweedfs/seaweedfs/weed/s3api/s3err" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const ( + invalidNameTestAccessKey = "INVALIDNAMEKEY000001" + invalidNameTestSecretKey = "invalid-name-secret-for-tests-only" + invalidNameTestBucket = "readable-bucket" +) + +// newInvalidNameIAM has one identity that may read invalidNameTestBucket only +// and a configured anonymous identity with no actions. +func newInvalidNameIAM(t *testing.T) *IdentityAccessManagement { + t.Helper() + iam := &IdentityAccessManagement{hashes: make(map[string]*sync.Pool), hashCounters: make(map[string]*int32)} + require.NoError(t, iam.loadS3ApiConfiguration(&iam_pb.S3ApiConfiguration{ + Identities: []*iam_pb.Identity{ + {Name: "reader", Actions: []string{"Read:" + invalidNameTestBucket}, + Credentials: []*iam_pb.Credential{{AccessKey: invalidNameTestAccessKey, SecretKey: invalidNameTestSecretKey}}}, + {Name: s3_constants.AccountAnonymousId}, + }, + })) + require.True(t, iam.isEnabled()) + return iam +} + +func invalidNameRequest(t *testing.T, bucket string, sign bool) *http.Request { + t.Helper() + req := mustNewRequest(http.MethodGet, fmt.Sprintf("http://127.0.0.1:8333/%s/key", bucket), 0, nil, t) + if sign { + require.NoError(t, signRequestV4(req, invalidNameTestAccessKey, invalidNameTestSecretKey)) + } + return mux.SetURLVars(req, map[string]string{"bucket": bucket, "object": "key"}) +} + +func serveAuth(iam *IdentityAccessManagement, r *http.Request) (int, bool) { + ran := false + rec := httptest.NewRecorder() + iam.Auth(func(w http.ResponseWriter, r *http.Request) { ran = true; w.WriteHeader(http.StatusOK) }, s3_constants.ACTION_READ).ServeHTTP(rec, r) + return rec.Code, ran +} + +// A denied request on a malformed bucket name answers 400 InvalidBucketName, +// as AWS S3 does; a denied request on a well-formed name stays 403. +func TestAuthDeniedInvalidBucketName(t *testing.T) { + iam := newInvalidNameIAM(t) + + code, ran := serveAuth(iam, invalidNameRequest(t, invalidNameTestBucket, true)) + assert.True(t, ran, "the identity's own bucket is allowed") + assert.Equal(t, http.StatusOK, code) + + code, ran = serveAuth(iam, invalidNameRequest(t, "other-bucket", true)) + assert.False(t, ran) + assert.Equal(t, http.StatusForbidden, code, "a well-formed name the caller may not read stays AccessDenied") + + for _, bucket := range []string{"Bad_Bucket", "ab", "bucket..name", "bücket"} { + for _, sign := range []bool{true, false} { + code, ran = serveAuth(iam, invalidNameRequest(t, bucket, sign)) + assert.False(t, ran, bucket) + assert.Equal(t, http.StatusBadRequest, code, "bucket %q signed=%v", bucket, sign) + } + } +} + +func TestRefineDenialForBucketName(t *testing.T) { + reader := &Identity{Name: "reader"} + req := func(bucket string) *http.Request { + return mux.SetURLVars(httptest.NewRequest(http.MethodGet, "/", nil), map[string]string{"bucket": bucket}) + } + cases := []struct { + name string + bucket string + identity *Identity + in, want s3err.ErrorCode + }{ + {"space in the name", "bucket name", reader, s3err.ErrAccessDenied, s3err.ErrInvalidBucketName}, + {"non-ASCII letter", "bücket", reader, s3err.ErrAccessDenied, s3err.ErrInvalidBucketName}, + {"superscript digit", "b²cket", reader, s3err.ErrAccessDenied, s3err.ErrInvalidBucketName}, + {"too short", "ab", reader, s3err.ErrAccessDenied, s3err.ErrInvalidBucketName}, + {"well-formed name", "other-bucket", reader, s3err.ErrAccessDenied, s3err.ErrAccessDenied}, + {"no bucket in the request", "", reader, s3err.ErrAccessDenied, s3err.ErrAccessDenied}, + {"allowed", "Bad_Bucket", reader, s3err.ErrNone, s3err.ErrNone}, + {"authentication failed", "Bad_Bucket", nil, s3err.ErrAccessDenied, s3err.ErrAccessDenied}, + {"bad signature", "Bad_Bucket", nil, s3err.ErrSignatureDoesNotMatch, s3err.ErrSignatureDoesNotMatch}, + } + for _, c := range cases { + assert.Equal(t, c.want, refineDenialForBucketName(req(c.bucket), c.identity, c.in), c.name) + } +} + +// signedPostForm builds a multipart POST Object request for bucket, its policy +// signed with secret under postAuthzAccessKey. +func signedPostForm(t *testing.T, bucket, key, secret string) *http.Request { + t.Helper() + now := time.Now().UTC() + amzDate := now.Format(iso8601Format) + day := now.Format(yyyymmdd) + credential := fmt.Sprintf("%s/%s/%s/%s/aws4_request", postAuthzAccessKey, day, postAuthzRegion, postAuthzService) + policyJSON := fmt.Sprintf(`{"expiration":"%s","conditions":[["eq","$bucket","%s"],["eq","$key","%s"],`+ + `["eq","$x-amz-credential","%s"],["eq","$x-amz-algorithm","AWS4-HMAC-SHA256"],["eq","$x-amz-date","%s"]]}`, + now.Add(time.Hour).Format("2006-01-02T15:04:05.000Z"), bucket, key, credential, amzDate) + encodedPolicy := base64.StdEncoding.EncodeToString([]byte(policyJSON)) + signature := getSignature(getSigningKey(secret, day, postAuthzRegion, postAuthzService), encodedPolicy) + + var buf bytes.Buffer + writer := multipart.NewWriter(&buf) + for _, field := range [][2]string{{"bucket", bucket}, {"key", key}, {"x-amz-credential", credential}, + {"x-amz-algorithm", "AWS4-HMAC-SHA256"}, {"x-amz-date", amzDate}, {"policy", encodedPolicy}, {"x-amz-signature", signature}} { + require.NoError(t, writer.WriteField(field[0], field[1])) + } + filePart, err := writer.CreateFormFile("file", "payload.txt") + require.NoError(t, err) + _, err = filePart.Write([]byte("payload")) + require.NoError(t, err) + require.NoError(t, writer.Close()) + + req := httptest.NewRequest(http.MethodPost, "/"+bucket, &buf) + req.Header.Set("Content-Type", writer.FormDataContentType()) + return mux.SetURLVars(req, map[string]string{"bucket": bucket}) +} + +// A browser POST upload authenticates in the handler, after AuthPostPolicy has +// delegated it, so its denials are refined there: a verified form denied on a +// malformed bucket name answers 400, a bad signature or a well-formed name 403. +func TestPostPolicyDeniedInvalidBucketName(t *testing.T) { + s3a := newPostAuthzServer(t) + for _, bucket := range []string{"Bad_Bucket", "other-bucket"} { + s3a.bucketRegistry.setMetadataCache(&BucketMetaData{Name: bucket}) + } + // A second identity may write Bad_Bucket, except under denied/. + require.NoError(t, s3a.iam.loadS3ApiConfiguration(&iam_pb.S3ApiConfiguration{ + Identities: []*iam_pb.Identity{ + {Name: "tester", Actions: []string{"Write:" + postAuthzBucket}, + Credentials: []*iam_pb.Credential{{AccessKey: postAuthzAccessKey, SecretKey: postAuthzSecretKey}}}, + }, + })) + require.NoError(t, s3a.policyEngine.engine.SetBucketPolicy("Bad_Bucket", `{"Version":"2012-10-17","Statement":[`+ + `{"Effect":"Deny","Principal":"*","Action":["s3:PutObject"],"Resource":["arn:aws:s3:::Bad_Bucket/denied/*"]}]}`)) + + serve := func(r *http.Request) *httptest.ResponseRecorder { + rec := httptest.NewRecorder() + s3a.iam.AuthPostPolicy(s3a.PostPolicyBucketHandler, s3_constants.ACTION_WRITE).ServeHTTP(rec, r) + return rec + } + + rec := serve(signedPostForm(t, "Bad_Bucket", "k", postAuthzSecretKey)) + assert.Equal(t, http.StatusBadRequest, rec.Code, rec.Body.String()) + assert.Contains(t, rec.Body.String(), "InvalidBucketName") + + rec = serve(signedPostForm(t, "Bad_Bucket", "k", "not-the-secret")) + assert.Equal(t, http.StatusForbidden, rec.Code, rec.Body.String()) + assert.Contains(t, rec.Body.String(), "SignatureDoesNotMatch", "a form that fails verification is not refined") + + rec = serve(signedPostForm(t, "other-bucket", "k", postAuthzSecretKey)) + assert.Equal(t, http.StatusForbidden, rec.Code, rec.Body.String()) + assert.Contains(t, rec.Body.String(), "AccessDenied") + + // The per-key authorization after the coarse Write check is refined too. + require.NoError(t, s3a.iam.loadS3ApiConfiguration(&iam_pb.S3ApiConfiguration{ + Identities: []*iam_pb.Identity{ + {Name: "tester", Actions: []string{"Write:Bad_Bucket"}, + Credentials: []*iam_pb.Credential{{AccessKey: postAuthzAccessKey, SecretKey: postAuthzSecretKey}}}, + }, + })) + rec = serve(signedPostForm(t, "Bad_Bucket", "denied/k", postAuthzSecretKey)) + assert.Equal(t, http.StatusBadRequest, rec.Code, rec.Body.String()) + assert.Contains(t, rec.Body.String(), "InvalidBucketName") +} + +// Unsigned GET/HEAD object reads are authorized inside the object handlers, not +// by Auth; their denial on a malformed bucket name answers 400 as well. +func TestAnonymousReadDeniedInvalidBucketName(t *testing.T) { + read := func(s3a *S3ApiServer, method, bucket string) int { + r := httptest.NewRequest(method, "/"+bucket+"/key", nil) + r = mux.SetURLVars(r, map[string]string{"bucket": bucket, "object": "key"}) + handler := s3a.GetObjectHandler + if method == http.MethodHead { + handler = s3a.HeadObjectHandler + } + rr := httptest.NewRecorder() + s3a.AuthWithPublicRead(handler, s3_constants.ACTION_READ)(rr, r) + return rr.Code + } + for _, method := range []string{http.MethodGet, http.MethodHead} { + s3a, _ := newAnonymousReadTestServer(t) + s3a.iam.identityAnonymous = &Identity{Name: s3_constants.AccountAnonymousId} + assert.Equal(t, http.StatusBadRequest, read(s3a, method, "Bad_Bucket"), method) + assert.Equal(t, http.StatusForbidden, read(s3a, method, "other-bucket"), "%s: a well-formed missing bucket stays masked", method) + + // No anonymous identity configured: no identity resolves, as in Auth. + s3a, _ = newAnonymousReadTestServer(t) + assert.Equal(t, http.StatusForbidden, read(s3a, method, "Bad_Bucket"), method) + } +} + +// An existing private key and a missing key in the same malformed-name bucket +// must answer alike, or the status code tells an anonymous caller which keys +// exist. +func TestAnonymousReadDeniedInvalidBucketNameExistingKey(t *testing.T) { + const bucket = "bücket" + read := func(s3a *S3ApiServer, method, object string) int { + r := httptest.NewRequest(method, "/"+bucket+"/"+object, nil) + r = mux.SetURLVars(r, map[string]string{"bucket": bucket, "object": object}) + handler := s3a.GetObjectHandler + if method == http.MethodHead { + handler = s3a.HeadObjectHandler + } + rr := httptest.NewRecorder() + s3a.AuthWithPublicRead(handler, s3_constants.ACTION_READ)(rr, r) + return rr.Code + } + setup := func(anonymous bool) *S3ApiServer { + s3a, f := newAnonymousReadTestServer(t) + if anonymous { + s3a.iam.identityAnonymous = &Identity{Name: s3_constants.AccountAnonymousId} + } + s3a.bucketConfigCache.Set(bucket, &BucketConfig{Name: bucket, Ownership: s3_constants.OwnershipObjectWriter}) + f.entries["/buckets/"+bucket+"/private"] = anonymousReadEntry([]byte(`[]`)) + f.entries["/buckets/"+bucket+"/public"] = anonymousReadEntry(anonymousReadGrant("READ")) + return s3a + } + for _, method := range []string{http.MethodGet, http.MethodHead} { + s3a := setup(true) + assert.Equal(t, http.StatusOK, read(s3a, method, "public"), "%s: a public key is still served", method) + assert.Equal(t, http.StatusBadRequest, read(s3a, method, "missing"), method) + assert.Equal(t, http.StatusBadRequest, read(s3a, method, "private"), "%s: an existing private key answers like a missing one", method) + + // No anonymous identity configured: both stay 403, as in Auth. + s3a = setup(false) + assert.Equal(t, http.StatusForbidden, read(s3a, method, "missing"), method) + assert.Equal(t, http.StatusForbidden, read(s3a, method, "private"), method) + } +} diff --git a/weed/s3api/auth_signature_v2.go b/weed/s3api/auth_signature_v2.go index 257316816d..72fdf9b421 100644 --- a/weed/s3api/auth_signature_v2.go +++ b/weed/s3api/auth_signature_v2.go @@ -29,7 +29,6 @@ import ( "time" "github.com/seaweedfs/seaweedfs/weed/glog" - "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" "github.com/seaweedfs/seaweedfs/weed/s3api/s3err" ) @@ -95,11 +94,6 @@ func (iam *IdentityAccessManagement) doesPolicySignatureV2Match(formValues http. return nil, s3err.ErrAccessDenied } - bucket := formValues.Get("bucket") - if !identity.CanDo(s3_constants.ACTION_WRITE, bucket, "") { - return nil, s3err.ErrAccessDenied - } - policy := formValues.Get("Policy") if policy == "" { return nil, s3err.ErrMissingFields diff --git a/weed/s3api/auth_signature_v4.go b/weed/s3api/auth_signature_v4.go index 6b6e3f05f1..2289d7f77d 100644 --- a/weed/s3api/auth_signature_v4.go +++ b/weed/s3api/auth_signature_v4.go @@ -858,11 +858,6 @@ func (iam *IdentityAccessManagement) doesPolicySignatureV4Match(formValues http. return nil, s3err.ErrAccessDenied } - bucket := formValues.Get("bucket") - if !identity.CanDo(s3_constants.ACTION_WRITE, bucket, "") { - return nil, s3err.ErrAccessDenied - } - // Get signing key. signingKey := getSigningKey(cred.SecretKey, credHeader.scope.date.Format(yyyymmdd), credHeader.scope.region, credHeader.scope.service) diff --git a/weed/s3api/s3api_object_anonymous_read.go b/weed/s3api/s3api_object_anonymous_read.go index 8146896453..e5d1c21c28 100644 --- a/weed/s3api/s3api_object_anonymous_read.go +++ b/weed/s3api/s3api_object_anonymous_read.go @@ -171,10 +171,12 @@ func (s3a *S3ApiServer) authorizeAnonymousBucketList(r *http.Request, bucket str // writeAnonymousAwareNotFound keeps missing-bucket and missing-key misses // indistinguishable from denied reads for anonymous callers: a 404 only -// reaches the wire when the bucket is anonymously listable. +// reaches the wire when the bucket is anonymously listable. The denial is +// refined like Auth's, so a malformed bucket name answers InvalidBucketName. func (s3a *S3ApiServer) writeAnonymousAwareNotFound(w http.ResponseWriter, r *http.Request, bucket string, code s3err.ErrorCode) { if isAnonymousObjectRead(r) && s3a.authorizeAnonymousBucketList(r, bucket) != s3err.ErrNone { - s3err.WriteErrorResponse(w, r, s3err.ErrAccessDenied) + identity, _ := s3_constants.GetIdentityFromContext(r).(*Identity) + s3err.WriteErrorResponse(w, r, refineDenialForBucketName(r, identity, s3err.ErrAccessDenied)) return } s3err.WriteErrorResponse(w, r, code) diff --git a/weed/s3api/s3api_object_anonymous_read_test.go b/weed/s3api/s3api_object_anonymous_read_test.go index db8e68192d..2f27c78acf 100644 --- a/weed/s3api/s3api_object_anonymous_read_test.go +++ b/weed/s3api/s3api_object_anonymous_read_test.go @@ -299,7 +299,9 @@ func TestAnonymousObjectACLConfiguredIdentity(t *testing.T) { }{ {"legacy anonymous permission", false, []Action{s3_constants.ACTION_READ}, "", false, 200}, {"identity policy allow", false, nil, `{"Effect":"Allow","Action":"s3:GetObject","Resource":"arn:aws:s3:::b/*"}`, false, 200}, - {"identity deny beats object ACL", true, nil, `{"Effect":"Deny","Action":"s3:GetObject","Resource":"arn:aws:s3:::b/*"}`, false, 403}, + // The fixture bucket "b" is shorter than S3's three-character minimum, + // so a denial for a resolved identity answers InvalidBucketName. + {"identity deny beats object ACL", true, nil, `{"Effect":"Deny","Action":"s3:GetObject","Resource":"arn:aws:s3:::b/*"}`, false, 400}, {"disabled identity cannot grant", false, []Action{s3_constants.ACTION_READ}, "", true, 403}, {"public ACL does not require identity", true, nil, "", true, 200}, } { diff --git a/weed/s3api/s3api_object_handlers_postpolicy.go b/weed/s3api/s3api_object_handlers_postpolicy.go index 835d5008bb..3d5cffbc8e 100644 --- a/weed/s3api/s3api_object_handlers_postpolicy.go +++ b/weed/s3api/s3api_object_handlers_postpolicy.go @@ -82,7 +82,7 @@ func (s3a *S3ApiServer) PostPolicyBucketHandler(w http.ResponseWriter, r *http.R // Verify policy signature. identity, errCode := s3a.iam.doesPolicySignatureMatch(formValues) if errCode != s3err.ErrNone { - s3err.WriteErrorResponse(w, r, errCode) + s3err.WriteErrorResponse(w, r, refineDenialForBucketName(r, identity, errCode)) return } if identity != nil { @@ -140,7 +140,7 @@ func (s3a *S3ApiServer) PostPolicyBucketHandler(w http.ResponseWriter, r *http.R // Authorize the object like the PUT path; the coarse Write check above does not consult the bucket policy. if errCode := s3a.iam.AuthorizeObjectWrite(r, identity, bucket, object); errCode != s3err.ErrNone { - s3err.WriteErrorResponse(w, r, errCode) + s3err.WriteErrorResponse(w, r, refineDenialForBucketName(r, identity, errCode)) return } @@ -362,10 +362,24 @@ func getRedirectPostRawQuery(bucket, key, etag string) string { } // Check to see if Policy is signed correctly, returning the signing identity. +// The identity's Write permission on the bucket is checked only once the +// signature verifies, as on the header-signed path, so a form that fails +// verification learns nothing about the key's permissions. A verified identity +// that may not write is returned with ErrAccessDenied. func (iam *IdentityAccessManagement) doesPolicySignatureMatch(formValues http.Header) (*Identity, s3err.ErrorCode) { + var identity *Identity + var errCode s3err.ErrorCode // For SignV2 - Signature field will be valid if _, ok := formValues["Signature"]; ok { - return iam.doesPolicySignatureV2Match(formValues) + identity, errCode = iam.doesPolicySignatureV2Match(formValues) + } else { + identity, errCode = iam.doesPolicySignatureV4Match(formValues) } - return iam.doesPolicySignatureV4Match(formValues) + if errCode != s3err.ErrNone { + return nil, errCode + } + if !identity.CanDo(s3_constants.ACTION_WRITE, formValues.Get("bucket"), "") { + return identity, s3err.ErrAccessDenied + } + return identity, s3err.ErrNone } diff --git a/weed/s3api/s3api_server.go b/weed/s3api/s3api_server.go index 126cb124de..f7c0e477cf 100644 --- a/weed/s3api/s3api_server.go +++ b/weed/s3api/s3api_server.go @@ -661,7 +661,10 @@ func (s3a *S3ApiServer) checkPolicyWithEntry(r *http.Request, bucket, object, ac // Returns s3err.ErrNone if allowed, or an error code if denied or on error. func (s3a *S3ApiServer) recheckPolicyWithObjectEntry(r *http.Request, bucket, object, action string, objectEntry map[string][]byte, handlerName string) s3err.ErrorCode { if isAnonymousObjectRead(r) { - return s3a.authorizeAnonymousObjectRead(r, bucket, object, objectEntry) + // Refined like writeAnonymousAwareNotFound's masked miss, so a denied + // existing key and a missing key answer alike on a malformed name. + identity, _ := GetIdentityFromContext(r).(*Identity) + return refineDenialForBucketName(r, identity, s3a.authorizeAnonymousObjectRead(r, bucket, object, objectEntry)) } identityRaw := GetIdentityFromContext(r) var identity *Identity