diff --git a/weed/s3api/auth_credentials.go b/weed/s3api/auth_credentials.go index 2eb7d5ec5..6b4bea63c 100644 --- a/weed/s3api/auth_credentials.go +++ b/weed/s3api/auth_credentials.go @@ -2758,23 +2758,16 @@ func (iam *IdentityAccessManagement) bucketsNamedByAttachedPolicies(r *http.Requ return true, granted } -// AuthorizeCopySource verifies the caller is allowed to read the CopyObject / -// UploadPartCopy source. The Auth middleware only checks the destination -// (s3:PutObject) because routing keys on the request URL; without this call, -// an STS session token scoped to a prefix could copy from any other prefix in -// the same bucket. -// -// The source path is checked against both bucket policy and IAM/identity -// permissions, mirroring the normal request-routed flow but with a synthetic -// GetObject request so action resolution and ARN building target the source. -// Returns s3err.ErrNone when allowed or when auth is disabled. -func (iam *IdentityAccessManagement) AuthorizeCopySource(r *http.Request, identity *Identity, srcBucket, srcObject, srcVersionId string) s3err.ErrorCode { - if !iam.isEnabled() { - return s3err.ErrNone - } - if srcBucket == "" { - return s3err.ErrNone - } +// authorizeObjectKeyAction authorizes action on bucket/objectKey for an +// already-authenticated identity, for a key the request URL does not name: a +// copy/rename source, a DeleteObjects body key, or a POST Object form key. It +// evaluates the bucket policy (an explicit Deny wins) and then IAM/identity +// against a synthetic // request, so ResolveS3Action +// and buildResourceARN target the object rather than whatever the real URL and +// its query describe. versionId, when set, and the STS session token ride along +// for policy conditions. Returns ErrNone when auth is disabled (checked by the +// callers) or the identity is an admin. +func (iam *IdentityAccessManagement) authorizeObjectKeyAction(r *http.Request, identity *Identity, method string, action Action, bucket, objectKey, versionId string) s3err.ErrorCode { if identity == nil { return s3err.ErrAccessDenied } @@ -2782,72 +2775,8 @@ func (iam *IdentityAccessManagement) AuthorizeCopySource(r *http.Request, identi return s3err.ErrNone } - srcReq := r.Clone(r.Context()) - srcURL := &url.URL{ - Scheme: r.URL.Scheme, - Host: r.URL.Host, - Path: "/" + srcBucket + "/" + srcObject, - } - // Build the synthetic source query from scratch so leftover params like - // uploadId/partNumber on UploadPartCopy do not steer ResolveS3Action away - // from s3:GetObject. The session token must still flow through for - // presigned URLs that carry STS credentials in the query string. - srcQuery := make(url.Values) - if token := r.URL.Query().Get("X-Amz-Security-Token"); token != "" { - srcQuery.Set("X-Amz-Security-Token", token) - } - if srcVersionId != "" { - srcQuery.Set("versionId", srcVersionId) - } - if len(srcQuery) > 0 { - srcURL.RawQuery = srcQuery.Encode() - } - srcReq.URL = srcURL - srcReq.Method = http.MethodGet - srcReq.RequestURI = "" - srcReq.Body = nil - srcReq.GetBody = nil - srcReq.ContentLength = 0 - - action := s3_constants.ACTION_READ - - if iam.policyEngine != nil { - principal := buildPrincipalARN(identity, srcReq) - allowed, evaluated, err := iam.policyEngine.EvaluatePolicy(srcBucket, srcObject, action, principal, srcReq, identity.Claims, nil) - if err != nil { - glog.Errorf("CopyObject source policy evaluation failed for %s/%s: %v - denying", srcBucket, srcObject, err) - return s3err.ErrAccessDenied - } - if evaluated { - if allowed { - return s3err.ErrNone - } - return s3err.ErrAccessDenied - } - } - - return iam.VerifyActionPermission(srcReq, identity, Action(action), srcBucket, srcObject) -} - -// AuthorizeObjectDelete authorizes removing one key the request URL does not -// name: a key from a DeleteObjects body, or the source of a RenameObject. It is -// checked against a synthetic DELETE // so that ResolveS3Action and -// buildResourceARN target the object. Mirrors AuthorizeCopySource. -func (iam *IdentityAccessManagement) AuthorizeObjectDelete(r *http.Request, identity *Identity, bucket, objectKey, versionId string) s3err.ErrorCode { - if !iam.isEnabled() { - return s3err.ErrNone - } - if bucket == "" || objectKey == "" { - return s3err.ErrNone - } - if identity == nil { - return s3err.ErrAccessDenied - } - if identity.isAdmin() { - return s3err.ErrNone - } - - // Shallow copy: authorization only reads headers, and this runs once per key. + // Shallow copy: authorization only reads headers, so sharing the header map + // with the original request is safe, and this can run once per key. keyReq := new(http.Request) *keyReq = *r keyURL := &url.URL{ @@ -2855,8 +2784,9 @@ func (iam *IdentityAccessManagement) AuthorizeObjectDelete(r *http.Request, iden Host: r.URL.Host, Path: "/" + bucket + "/" + objectKey, } - // Build the query from scratch so the envelope's "delete" param can't steer - // ResolveS3Action; keep the STS token and per-key versionId for policy eval. + // Build the query from scratch so a param on the real request (delete, + // uploadId, partNumber, ...) cannot steer ResolveS3Action off the intended + // action; keep the STS token and per-key versionId for policy conditions. keyQuery := make(url.Values) if versionId != "" { keyQuery.Set("versionId", versionId) @@ -2870,19 +2800,17 @@ func (iam *IdentityAccessManagement) AuthorizeObjectDelete(r *http.Request, iden keyURL.RawQuery = keyQuery.Encode() } keyReq.URL = keyURL - keyReq.Method = http.MethodDelete + keyReq.Method = method keyReq.RequestURI = "" keyReq.Body = nil keyReq.GetBody = nil keyReq.ContentLength = 0 - action := s3_constants.ACTION_WRITE - if iam.policyEngine != nil { principal := buildPrincipalARN(identity, keyReq) - allowed, evaluated, err := iam.policyEngine.EvaluatePolicy(bucket, objectKey, action, principal, keyReq, identity.Claims, nil) + allowed, evaluated, err := iam.policyEngine.EvaluatePolicy(bucket, objectKey, string(action), principal, keyReq, identity.Claims, nil) if err != nil { - glog.Errorf("DeleteObjects key policy evaluation failed for %s/%s: %v - denying", bucket, objectKey, err) + glog.Errorf("policy evaluation failed for %s %s/%s: %v - denying", action, bucket, objectKey, err) return s3err.ErrAccessDenied } if evaluated { @@ -2893,7 +2821,48 @@ func (iam *IdentityAccessManagement) AuthorizeObjectDelete(r *http.Request, iden } } - return iam.VerifyActionPermission(keyReq, identity, Action(action), bucket, objectKey) + return iam.VerifyActionPermission(keyReq, identity, action, bucket, objectKey) +} + +// AuthorizeCopySource verifies the caller is allowed to read the CopyObject / +// UploadPartCopy source. The Auth middleware only checks the destination +// (s3:PutObject) because routing keys on the request URL; without this call, +// an STS session token scoped to a prefix could copy from any other prefix in +// the same bucket. Returns s3err.ErrNone when allowed or when auth is disabled. +func (iam *IdentityAccessManagement) AuthorizeCopySource(r *http.Request, identity *Identity, srcBucket, srcObject, srcVersionId string) s3err.ErrorCode { + if !iam.isEnabled() { + return s3err.ErrNone + } + if srcBucket == "" { + return s3err.ErrNone + } + return iam.authorizeObjectKeyAction(r, identity, http.MethodGet, s3_constants.ACTION_READ, srcBucket, srcObject, srcVersionId) +} + +// AuthorizeObjectDelete authorizes removing one key the request URL does not +// name: a key from a DeleteObjects body, or the source of a RenameObject. +func (iam *IdentityAccessManagement) AuthorizeObjectDelete(r *http.Request, identity *Identity, bucket, objectKey, versionId string) s3err.ErrorCode { + if !iam.isEnabled() { + return s3err.ErrNone + } + if bucket == "" || objectKey == "" { + return s3err.ErrNone + } + return iam.authorizeObjectKeyAction(r, identity, http.MethodDelete, s3_constants.ACTION_WRITE, bucket, objectKey, versionId) +} + +// AuthorizeObjectWrite authorizes writing one key the request URL does not name: +// a POST Object (presigned-POST / HTML-form) upload carries its key in the +// multipart form, so the Auth middleware only checked the coarse bucket-level +// Write action. This runs the same per-object authorization the PUT path applies. +func (iam *IdentityAccessManagement) AuthorizeObjectWrite(r *http.Request, identity *Identity, bucket, objectKey string) s3err.ErrorCode { + if !iam.isEnabled() { + return s3err.ErrNone + } + if bucket == "" || objectKey == "" { + return s3err.ErrNone + } + return iam.authorizeObjectKeyAction(r, identity, http.MethodPut, s3_constants.ACTION_WRITE, bucket, objectKey, "") } // authorizeWithIAM authorizes requests using the IAM integration policy engine diff --git a/weed/s3api/s3api_object_handlers_postpolicy.go b/weed/s3api/s3api_object_handlers_postpolicy.go index 8e9cbb076..66a89737c 100644 --- a/weed/s3api/s3api_object_handlers_postpolicy.go +++ b/weed/s3api/s3api_object_handlers_postpolicy.go @@ -139,6 +139,12 @@ func (s3a *S3ApiServer) PostPolicyBucketHandler(w http.ResponseWriter, r *http.R // Forward validated POST form fields to the underlying PUT as headers. applyPostPolicyFormHeaders(r, formValues) + // 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) + return + } + // Use fileSize, not r.ContentLength: the multipart body wrapping form // fields and boundaries inflates ContentLength relative to the // object body, which would mis-evaluate any size-filtered rule. diff --git a/weed/s3api/s3api_object_handlers_postpolicy_authz_test.go b/weed/s3api/s3api_object_handlers_postpolicy_authz_test.go new file mode 100644 index 000000000..1dc636507 --- /dev/null +++ b/weed/s3api/s3api_object_handlers_postpolicy_authz_test.go @@ -0,0 +1,172 @@ +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 ( + postAuthzAccessKey = "AKIATESTTESTTEST" + postAuthzSecretKey = "secret-key-for-tests" + postAuthzRegion = "us-east-1" + postAuthzService = "s3" + postAuthzBucket = "testbucket" + postAuthzPrincipal = "arn:aws:iam::000000000000:user/tester" +) + +// newPostAuthzServer builds an S3ApiServer whose non-admin "tester" identity +// holds the coarse Write action on the bucket and whose bucket policy denies it +// s3:PutObject under denied/: a writer confined by a bucket policy to part of a +// shared bucket. +func newPostAuthzServer(t *testing.T) *S3ApiServer { + t.Helper() + + iam := &IdentityAccessManagement{ + hashes: make(map[string]*sync.Pool), + hashCounters: make(map[string]*int32), + } + err := iam.loadS3ApiConfiguration(&iam_pb.S3ApiConfiguration{ + Identities: []*iam_pb.Identity{{ + Name: "tester", + Account: &iam_pb.Account{Id: "000000000000", DisplayName: "tester"}, + Credentials: []*iam_pb.Credential{{AccessKey: postAuthzAccessKey, SecretKey: postAuthzSecretKey}}, + Actions: []string{"Read:" + postAuthzBucket, "Write:" + postAuthzBucket, "List:" + postAuthzBucket}, + }}, + }) + require.NoError(t, err) + + policyEngine := NewBucketPolicyEngine() + err = policyEngine.engine.SetBucketPolicy(postAuthzBucket, fmt.Sprintf(`{ + "Version": "2012-10-17", + "Statement": [{ + "Sid": "DenyTesterDeniedPrefix", + "Effect": "Deny", + "Principal": {"AWS": "%s"}, + "Action": ["s3:PutObject"], + "Resource": ["arn:aws:s3:::%s/denied/*"] + }] + }`, postAuthzPrincipal, postAuthzBucket)) + require.NoError(t, err) + iam.policyEngine = policyEngine + + s3a := &S3ApiServer{ + option: &S3ApiServerOption{BucketsPath: "/buckets"}, + iam: iam, + policyEngine: policyEngine, + } + // Pre-populate the bucket registry so validateTableBucketObjectPath sees a + // non-table bucket without needing a live filer connection. + s3a.bucketRegistry = NewBucketRegistry(s3a) + s3a.bucketRegistry.setMetadataCache(&BucketMetaData{Name: postAuthzBucket, IsTableBucket: false}) + return s3a +} + +// newSignedPostRequest builds a signed multipart POST Object request whose POST +// policy conditions are satisfied by the form, so the handler passes signature +// and CheckPostPolicy verification and reaches the write-authorization stage. +func newSignedPostRequest(t *testing.T, key string) *http.Request { + t.Helper() + + now := time.Now().UTC() + amzDate := now.Format(iso8601Format) + yyyymmddStr := now.Format(yyyymmdd) + credential := fmt.Sprintf("%s/%s/%s/%s/aws4_request", postAuthzAccessKey, yyyymmddStr, postAuthzRegion, postAuthzService) + expiration := now.Add(1 * time.Hour).Format("2006-01-02T15:04:05.000Z") + + 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"]`+ + `]}`, + expiration, postAuthzBucket, key, credential, amzDate, + ) + encodedPolicy := base64.StdEncoding.EncodeToString([]byte(policyJSON)) + signature := getSignature(getSigningKey(postAuthzSecretKey, yyyymmddStr, postAuthzRegion, postAuthzService), encodedPolicy) + + var buf bytes.Buffer + writer := multipart.NewWriter(&buf) + require.NoError(t, writer.WriteField("bucket", postAuthzBucket)) + require.NoError(t, writer.WriteField("key", key)) + require.NoError(t, writer.WriteField("x-amz-credential", credential)) + require.NoError(t, writer.WriteField("x-amz-algorithm", "AWS4-HMAC-SHA256")) + require.NoError(t, writer.WriteField("x-amz-date", amzDate)) + require.NoError(t, writer.WriteField("policy", encodedPolicy)) + require.NoError(t, writer.WriteField("x-amz-signature", signature)) + 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, "/"+postAuthzBucket, &buf) + req.Header.Set("Content-Type", writer.FormDataContentType()) + return mux.SetURLVars(req, map[string]string{"bucket": postAuthzBucket}) +} + +// TestPostPolicyBucketHandlerHonorsBucketPolicyDeny drives a signed POST Object +// upload whose key a bucket policy denies the caller, and asserts the handler +// rejects it with 403 -- the same result the equivalent PUT gives, now that POST +// runs the shared per-key authorization instead of only the coarse Write check. +func TestPostPolicyBucketHandlerHonorsBucketPolicyDeny(t *testing.T) { + s3a := newPostAuthzServer(t) + + // Precondition: the policy really denies this principal the denied/ key, so a + // non-403 below is the POST path not consulting it, not a mis-configured policy. + putReq := httptest.NewRequest(http.MethodPut, "/"+postAuthzBucket+"/denied/secret.txt", nil) + allowed, evaluated, err := s3a.iam.policyEngine.EvaluatePolicy( + postAuthzBucket, "denied/secret.txt", s3_constants.ACTION_WRITE, postAuthzPrincipal, putReq, nil, nil) + require.NoError(t, err) + require.True(t, evaluated, "policy must match the denied key for the tester principal") + require.False(t, allowed, "policy must deny the tester principal on the denied key") + + rec := httptest.NewRecorder() + s3a.PostPolicyBucketHandler(rec, newSignedPostRequest(t, "denied/secret.txt")) + + assert.Equal(t, http.StatusForbidden, rec.Code, + "POST Object into a bucket-policy-denied key must be rejected, body: %s", rec.Body.String()) + assert.Contains(t, rec.Body.String(), "AccessDenied", + "response should identify AccessDenied, body: %s", rec.Body.String()) +} + +// TestAuthorizeObjectWrite covers the write-authorization decision the POST +// handler defers to: a bucket-policy Deny blocks the key, a key the policy does +// not deny clears (the coarse Write action still grants it), and admins are +// unaffected. Driving the full handler for the allowed case is not hermetic -- +// past authorization it reaches the lifecycle/write path that needs a filer -- +// so the counter-cases are checked at the authorization boundary directly. +func TestAuthorizeObjectWrite(t *testing.T) { + s3a := newPostAuthzServer(t) + tester, _, found := s3a.iam.lookupByAccessKey(postAuthzAccessKey) + require.True(t, found) + + req := httptest.NewRequest(http.MethodPost, "/"+postAuthzBucket, nil) + + assert.Equal(t, s3err.ErrAccessDenied, + s3a.iam.AuthorizeObjectWrite(req, tester, postAuthzBucket, "denied/secret.txt"), + "a bucket-policy-denied key must be denied") + assert.Equal(t, s3err.ErrNone, + s3a.iam.AuthorizeObjectWrite(req, tester, postAuthzBucket, "allowed/report.txt"), + "a key the policy does not deny must clear on the coarse Write grant") + + admin := &Identity{Name: "admin", Actions: []Action{s3_constants.ACTION_ADMIN}} + assert.Equal(t, s3err.ErrNone, + s3a.iam.AuthorizeObjectWrite(req, admin, postAuthzBucket, "denied/secret.txt"), + "an admin is not confined by the bucket policy") +}