diff --git a/weed/s3api/auth_credentials.go b/weed/s3api/auth_credentials.go index 46ff18366b..502283a944 100644 --- a/weed/s3api/auth_credentials.go +++ b/weed/s3api/auth_credentials.go @@ -3183,6 +3183,20 @@ func (iam *IdentityAccessManagement) AuthorizeObjectWrite(r *http.Request, ident return iam.authorizeObjectKeyAction(r, identity, http.MethodPut, s3_constants.ACTION_WRITE, bucket, objectKey, "") } +// AuthorizeObjectAclWrite authorizes setting an ACL on one key the request URL +// does not name: a POST Object upload promotes its acl form field and any +// x-amz-grant-* fields onto the upload's ACL headers, which the PUT path only +// applies after an s3:PutObjectAcl grant. +func (iam *IdentityAccessManagement) AuthorizeObjectAclWrite(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_ACP, bucket, objectKey, "") +} + // authorizeWithIAM authorizes requests using the IAM integration policy engine func (iam *IdentityAccessManagement) authorizeWithIAM(r *http.Request, identity *Identity, action Action, bucket string, object string) s3err.ErrorCode { ctx := r.Context() diff --git a/weed/s3api/s3api_object_handlers_postpolicy.go b/weed/s3api/s3api_object_handlers_postpolicy.go index 4fb604abbf..1e8705ba11 100644 --- a/weed/s3api/s3api_object_handlers_postpolicy.go +++ b/weed/s3api/s3api_object_handlers_postpolicy.go @@ -144,6 +144,14 @@ func (s3a *S3ApiServer) PostPolicyBucketHandler(w http.ResponseWriter, r *http.R s3err.WriteErrorResponse(w, r, refineDenialForBucketName(r, identity, errCode)) return } + // An acl or x-amz-grant-* form field rides onto the upload as ACL headers, + // which the PUT path only applies under s3:PutObjectAcl. + if postRequestsAcl(r) { + if errCode := s3a.iam.AuthorizeObjectAclWrite(r, identity, bucket, object); errCode != s3err.ErrNone { + s3err.WriteErrorResponse(w, r, refineDenialForBucketName(r, identity, errCode)) + return + } + } versioningState, err := s3a.getVersioningState(bucket) if err != nil { @@ -275,6 +283,17 @@ func applyPostPolicyFormHeaders(r *http.Request, formValues http.Header) { } } +// postRequestsAcl reports whether the request carries ACL headers, whether +// promoted from the POST form fields or supplied on the request itself. +func postRequestsAcl(r *http.Request) bool { + for _, header := range []string{s3_constants.AmzCannedAcl, s3_constants.AmzAclFullControl, s3_constants.AmzAclRead, s3_constants.AmzAclReadAcp, s3_constants.AmzAclWrite, s3_constants.AmzAclWriteAcp} { + if r.Header.Get(header) != "" { + return true + } + } + return false +} + // Extract form fields and file data from a HTTP POST Policy func extractPostPolicyFormValues(form *multipart.Form) (filePart io.ReadCloser, fileName, fileContentType string, fileSize int64, formValues http.Header, err error) { // / HTML Form values diff --git a/weed/s3api/s3api_object_handlers_postpolicy_authz_test.go b/weed/s3api/s3api_object_handlers_postpolicy_authz_test.go index 1dc6365071..553ff2f0a5 100644 --- a/weed/s3api/s3api_object_handlers_postpolicy_authz_test.go +++ b/weed/s3api/s3api_object_handlers_postpolicy_authz_test.go @@ -3,10 +3,12 @@ package s3api import ( "bytes" "encoding/base64" + "encoding/json" "fmt" "mime/multipart" "net/http" "net/http/httptest" + "sort" "sync" "testing" "time" @@ -26,6 +28,10 @@ const ( postAuthzService = "s3" postAuthzBucket = "testbucket" postAuthzPrincipal = "arn:aws:iam::000000000000:user/tester" + + postAclAccessKey = "AKIAACLTESTTEST" + postAclSecretKey = "secret-key-for-acl-tests" + postAclPrincipal = "arn:aws:iam::000000000000:user/aclwriter" ) // newPostAuthzServer builds an S3ApiServer whose non-admin "tester" identity @@ -45,6 +51,11 @@ func newPostAuthzServer(t *testing.T) *S3ApiServer { Account: &iam_pb.Account{Id: "000000000000", DisplayName: "tester"}, Credentials: []*iam_pb.Credential{{AccessKey: postAuthzAccessKey, SecretKey: postAuthzSecretKey}}, Actions: []string{"Read:" + postAuthzBucket, "Write:" + postAuthzBucket, "List:" + postAuthzBucket}, + }, { + Name: "aclwriter", + Account: &iam_pb.Account{Id: "000000000000", DisplayName: "aclwriter"}, + Credentials: []*iam_pb.Credential{{AccessKey: postAclAccessKey, SecretKey: postAclSecretKey}}, + Actions: []string{"Read:" + postAuthzBucket, "Write:" + postAuthzBucket, "WriteAcp:" + postAuthzBucket}, }}, }) require.NoError(t, err) @@ -58,8 +69,14 @@ func newPostAuthzServer(t *testing.T) *S3ApiServer { "Principal": {"AWS": "%s"}, "Action": ["s3:PutObject"], "Resource": ["arn:aws:s3:::%s/denied/*"] + }, { + "Sid": "DenyAclWriterAclPrefix", + "Effect": "Deny", + "Principal": {"AWS": "%s"}, + "Action": ["s3:PutObjectAcl"], + "Resource": ["arn:aws:s3:::%s/acl-denied/*"] }] - }`, postAuthzPrincipal, postAuthzBucket)) + }`, postAuthzPrincipal, postAuthzBucket, postAclPrincipal, postAuthzBucket)) require.NoError(t, err) iam.policyEngine = policyEngine @@ -79,26 +96,42 @@ func newPostAuthzServer(t *testing.T) *S3ApiServer { // 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 { + return newSignedPostRequestFor(t, key, postAuthzAccessKey, postAuthzSecretKey, nil) +} + +// extraFields are added to both the form and the policy conditions so +// CheckPostPolicy accepts them. +func newSignedPostRequestFor(t *testing.T, key, accessKey, secretKey string, extraFields map[string]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) + credential := fmt.Sprintf("%s/%s/%s/%s/aws4_request", accessKey, 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) + extraKeys := make([]string, 0, len(extraFields)) + for k := range extraFields { + extraKeys = append(extraKeys, k) + } + sort.Strings(extraKeys) + conditions := []any{ + []any{"eq", "$bucket", postAuthzBucket}, + []any{"eq", "$key", key}, + []any{"eq", "$x-amz-credential", credential}, + []any{"eq", "$x-amz-algorithm", "AWS4-HMAC-SHA256"}, + []any{"eq", "$x-amz-date", amzDate}, + } + for _, k := range extraKeys { + conditions = append(conditions, []any{"eq", "$" + k, extraFields[k]}) + } + policyJSON, err := json.Marshal(map[string]any{ + "expiration": expiration, + "conditions": conditions, + }) + require.NoError(t, err) + encodedPolicy := base64.StdEncoding.EncodeToString(policyJSON) + signature := getSignature(getSigningKey(secretKey, yyyymmddStr, postAuthzRegion, postAuthzService), encodedPolicy) var buf bytes.Buffer writer := multipart.NewWriter(&buf) @@ -107,6 +140,9 @@ func newSignedPostRequest(t *testing.T, key string) *http.Request { 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)) + for _, k := range extraKeys { + require.NoError(t, writer.WriteField(k, extraFields[k])) + } require.NoError(t, writer.WriteField("policy", encodedPolicy)) require.NoError(t, writer.WriteField("x-amz-signature", signature)) filePart, err := writer.CreateFormFile("file", "payload.txt") @@ -170,3 +206,71 @@ func TestAuthorizeObjectWrite(t *testing.T) { s3a.iam.AuthorizeObjectWrite(req, admin, postAuthzBucket, "denied/secret.txt"), "an admin is not confined by the bucket policy") } + +// TestPostPolicyBucketHandlerGatesAclFields drives signed POST uploads carrying +// an acl form field and asserts the handler rejects them the way the PUT path +// does: a writer without the WriteAcp action is refused, and so is a writer the +// bucket policy denies s3:PutObjectAcl on the target key. +func TestPostPolicyBucketHandlerGatesAclFields(t *testing.T) { + s3a := newPostAuthzServer(t) + + rec := httptest.NewRecorder() + s3a.PostPolicyBucketHandler(rec, newSignedPostRequestFor(t, "allowed/report.txt", + postAuthzAccessKey, postAuthzSecretKey, map[string]string{"acl": "public-read"})) + assert.Equal(t, http.StatusForbidden, rec.Code, + "POST with an acl field needs WriteAcp like PUT, body: %s", rec.Body.String()) + + rec = httptest.NewRecorder() + s3a.PostPolicyBucketHandler(rec, newSignedPostRequestFor(t, "acl-denied/secret.txt", + postAclAccessKey, postAclSecretKey, map[string]string{"acl": "public-read"})) + assert.Equal(t, http.StatusForbidden, rec.Code, + "POST with an acl field must honor the bucket-policy PutObjectAcl deny, body: %s", rec.Body.String()) + + for _, field := range []string{"x-amz-grant-read", "x-amz-grant-write", "x-amz-grant-read-acp", "x-amz-grant-write-acp", "x-amz-grant-full-control"} { + rec = httptest.NewRecorder() + s3a.PostPolicyBucketHandler(rec, newSignedPostRequestFor(t, "allowed/report.txt", + postAuthzAccessKey, postAuthzSecretKey, + map[string]string{field: `id="0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"`})) + assert.Equal(t, http.StatusForbidden, rec.Code, + "POST with an %s field needs WriteAcp like PUT, body: %s", field, rec.Body.String()) + } +} + +func TestPostRequestsAcl(t *testing.T) { + for _, header := range []string{ + s3_constants.AmzCannedAcl, s3_constants.AmzAclFullControl, s3_constants.AmzAclRead, + s3_constants.AmzAclReadAcp, s3_constants.AmzAclWrite, s3_constants.AmzAclWriteAcp, + } { + req := httptest.NewRequest(http.MethodPost, "/"+postAuthzBucket, nil) + req.Header.Set(header, "x") + assert.True(t, postRequestsAcl(req), "%s must trip the ACL gate", header) + } + assert.False(t, postRequestsAcl(httptest.NewRequest(http.MethodPost, "/"+postAuthzBucket, nil))) +} + +// TestAuthorizeObjectAclWrite covers the acl authorization decision at the +// boundary, for the same reason as TestAuthorizeObjectWrite. +func TestAuthorizeObjectAclWrite(t *testing.T) { + s3a := newPostAuthzServer(t) + tester, _, found := s3a.iam.lookupByAccessKey(postAuthzAccessKey) + require.True(t, found) + aclwriter, _, found := s3a.iam.lookupByAccessKey(postAclAccessKey) + require.True(t, found) + + req := httptest.NewRequest(http.MethodPost, "/"+postAuthzBucket, nil) + + assert.Equal(t, s3err.ErrAccessDenied, + s3a.iam.AuthorizeObjectAclWrite(req, tester, postAuthzBucket, "allowed/report.txt"), + "a writer without the WriteAcp action cannot attach an ACL") + assert.Equal(t, s3err.ErrNone, + s3a.iam.AuthorizeObjectAclWrite(req, aclwriter, postAuthzBucket, "allowed/report.txt"), + "the WriteAcp holder clears keys the policy does not deny") + assert.Equal(t, s3err.ErrAccessDenied, + s3a.iam.AuthorizeObjectAclWrite(req, aclwriter, postAuthzBucket, "acl-denied/secret.txt"), + "a bucket-policy PutObjectAcl deny must block the ACL-bearing upload") + + admin := &Identity{Name: "admin", Actions: []Action{s3_constants.ACTION_ADMIN}} + assert.Equal(t, s3err.ErrNone, + s3a.iam.AuthorizeObjectAclWrite(req, admin, postAuthzBucket, "acl-denied/secret.txt"), + "an admin is not confined by the bucket policy") +}