diff --git a/auth/bucket_policy.go b/auth/bucket_policy.go index 6d6d1495..c103832f 100644 --- a/auth/bucket_policy.go +++ b/auth/bucket_policy.go @@ -92,12 +92,12 @@ func (bp *BucketPolicy) isAllowed(principal string, action Action, resource stri return isAllowed } -// isPublic checks if the bucket policy statements contain -// an entity granting public access -func (bp *BucketPolicy) isPublic(resource string, action Action) bool { +// IsPublicFor checks if the bucket policy statements contain +// an entity granting public access to the given resource and action +func (bp *BucketPolicy) isPublicFor(resource string, action Action) bool { var isAllowed bool for _, statement := range bp.Statement { - if statement.isPublic(resource, action) { + if statement.isPublicFor(resource, action) { switch statement.Effect { case BucketPolicyAccessTypeAllow: isAllowed = true @@ -110,6 +110,18 @@ func (bp *BucketPolicy) isPublic(resource string, action Action) bool { return isAllowed } +// IsPublic checks if one of bucket policy statments grant +// public access to ALL users +func (bp *BucketPolicy) IsPublic() bool { + for _, statement := range bp.Statement { + if statement.isPublic() { + return true + } + } + + return false +} + type BucketPolicyItem struct { Effect BucketPolicyAccessType `json:"Effect"` Principals Principals `json:"Principal"` @@ -155,9 +167,16 @@ func (bpi *BucketPolicyItem) findMatch(principal string, action Action, resource return false } -// isPublic checks if the bucket policy statemant grants public access -func (bpi *BucketPolicyItem) isPublic(resource string, action Action) bool { - return bpi.Principals.IsPublic() && bpi.Actions.FindMatch(action) && bpi.Resources.FindMatch(resource) +// isPublicFor checks if the bucket policy statemant grants public access +// for given resource and action +func (bpi *BucketPolicyItem) isPublicFor(resource string, action Action) bool { + return bpi.Principals.isPublic() && bpi.Actions.FindMatch(action) && bpi.Resources.FindMatch(resource) +} + +// isPublic checks if the statement grants public access +// to ALL users +func (bpi *BucketPolicyItem) isPublic() bool { + return bpi.Principals.isPublic() } func getMalformedPolicyError(err error) error { @@ -168,17 +187,27 @@ func getMalformedPolicyError(err error) error { } } +// ParsePolicyDocument parses raw bytes to 'BucketPolicy' +func ParsePolicyDocument(data []byte) (*BucketPolicy, error) { + var policy BucketPolicy + if err := json.Unmarshal(data, &policy); err != nil { + var pe policyErr + if errors.As(err, &pe) { + return nil, getMalformedPolicyError(err) + } + return nil, getMalformedPolicyError(policyErrInvalidPolicy) + } + + return &policy, nil +} + func ValidatePolicyDocument(policyBin []byte, bucket string, iam IAMService) error { if len(policyBin) == 0 || policyBin[0] != '{' { return getMalformedPolicyError(policyErrInvalidFirstChar) } - var policy BucketPolicy - if err := json.Unmarshal(policyBin, &policy); err != nil { - var pe policyErr - if errors.As(err, &pe) { - return getMalformedPolicyError(err) - } - return getMalformedPolicyError(policyErrInvalidPolicy) + policy, err := ParsePolicyDocument(policyBin) + if err != nil { + return err } if len(policy.Statement) == 0 { @@ -222,7 +251,7 @@ func VerifyPublicBucketPolicy(policy []byte, bucket, object string, action Actio resource += "/" + object } - if !bucketPolicy.isPublic(resource, action) { + if !bucketPolicy.isPublicFor(resource, action) { return ErrAccessDenied } diff --git a/auth/bucket_policy_actions.go b/auth/bucket_policy_actions.go index 43ffea36..8812e1e7 100644 --- a/auth/bucket_policy_actions.go +++ b/auth/bucket_policy_actions.go @@ -86,6 +86,7 @@ const ( GetAccelerateConfigurationAction Action = "s3:GetAccelerateConfiguration" PutBucketWebsiteAction Action = "s3:PutBucketWebsite" GetBucketWebsiteAction Action = "s3:GetBucketWebsite" + GetBucketPolicyStatusAction Action = "s3:GetBucketPolicyStatus" AllActions Action = "s3:*" ) @@ -155,6 +156,7 @@ var supportedActionList = map[Action]struct{}{ GetAccelerateConfigurationAction: {}, PutBucketWebsiteAction: {}, GetBucketWebsiteAction: {}, + GetBucketPolicyStatusAction: {}, AllActions: {}, } diff --git a/auth/bucket_policy_principals.go b/auth/bucket_policy_principals.go index 306bb1a0..3f17d85e 100644 --- a/auth/bucket_policy_principals.go +++ b/auth/bucket_policy_principals.go @@ -124,7 +124,7 @@ func (p Principals) Contains(userAccess string) bool { // Bucket policy grants public access, if it contains // a wildcard match to all the users -func (p Principals) IsPublic() bool { +func (p Principals) isPublic() bool { _, ok := p["*"] return ok } diff --git a/metrics/actions.go b/metrics/actions.go index d0eeb9fe..ecc38069 100644 --- a/metrics/actions.go +++ b/metrics/actions.go @@ -115,6 +115,7 @@ var ( ActionPutBucketWebsite = "s3_PutBucketWebsite" ActionGetBucketWebsite = "s3_GetBucketWebsite" ActionDeleteBucketWebsite = "s3_DeleteBucketWebsite" + ActionGetBucketPolicyStatus = "s3_GetBucketPolicyStatus" // Admin actions ActionAdminCreateUser = "admin_CreateUser" @@ -493,4 +494,8 @@ func init() { Name: "DeleteBucketWebsite", Service: "s3", } + ActionMap[ActionGetBucketPolicyStatus] = Action{ + Name: "GetBucketPolicyStatus", + Service: "s3", + } } diff --git a/s3api/controllers/bucket-get.go b/s3api/controllers/bucket-get.go index 141b267e..c3116c50 100644 --- a/s3api/controllers/bucket-get.go +++ b/s3api/controllers/bucket-get.go @@ -238,6 +238,60 @@ func (c S3ApiController) GetBucketPolicy(ctx *fiber.Ctx) (*Response, error) { }, err } +func (c S3ApiController) GetBucketPolicyStatus(ctx *fiber.Ctx) (*Response, error) { + bucket := ctx.Params("bucket") + acct := utils.ContextKeyAccount.Get(ctx).(auth.Account) + isRoot := utils.ContextKeyIsRoot.Get(ctx).(bool) + isPublicBucket := utils.ContextKeyPublicBucket.IsSet(ctx) + parsedAcl := utils.ContextKeyParsedAcl.Get(ctx).(auth.ACL) + + err := auth.VerifyAccess(ctx.Context(), c.be, auth.AccessOptions{ + Readonly: c.readonly, + Acl: parsedAcl, + AclPermission: auth.PermissionRead, + IsRoot: isRoot, + Acc: acct, + Bucket: bucket, + Action: auth.GetBucketPolicyStatusAction, + IsBucketPublic: isPublicBucket, + }) + if err != nil { + return &Response{ + MetaOpts: &MetaOptions{ + BucketOwner: parsedAcl.Owner, + }, + }, err + } + + policyRaw, err := c.be.GetBucketPolicy(ctx.Context(), bucket) + if err != nil { + return &Response{ + MetaOpts: &MetaOptions{ + BucketOwner: parsedAcl.Owner, + }, + }, err + } + + policy, err := auth.ParsePolicyDocument(policyRaw) + if err != nil { + return &Response{ + MetaOpts: &MetaOptions{ + BucketOwner: parsedAcl.Owner, + }, + }, err + } + isPublic := policy.IsPublic() + + return &Response{ + Data: types.PolicyStatus{ + IsPublic: &isPublic, + }, + MetaOpts: &MetaOptions{ + BucketOwner: parsedAcl.Owner, + }, + }, nil +} + func (c S3ApiController) ListObjectVersions(ctx *fiber.Ctx) (*Response, error) { // url values bucket := ctx.Params("bucket") diff --git a/s3api/controllers/bucket-get_test.go b/s3api/controllers/bucket-get_test.go index 73354b9d..f45a45b7 100644 --- a/s3api/controllers/bucket-get_test.go +++ b/s3api/controllers/bucket-get_test.go @@ -505,6 +505,122 @@ func TestS3ApiController_GetBucketPolicy(t *testing.T) { } } +func TestS3ApiController_GetBucketPolicyStatus(t *testing.T) { + mockResp := ` + { + "Statement": [ + { + "Effect": "Allow", + "Principal": { + "AWS": "*" + }, + "Action": ["s3:GetObject"], + "Resource": "arn:aws:s3:::test/*" + } + ] + } + ` + isPublic := true + + tests := []struct { + name string + input testInput + output testOutput + }{ + { + name: "verify access fails", + input: testInput{ + beRes: []byte{}, + beErr: s3err.GetAPIError(s3err.ErrAccessDenied), + locals: accessDeniedLocals, + }, + output: testOutput{ + response: &Response{ + MetaOpts: &MetaOptions{ + BucketOwner: "root", + }, + }, + err: s3err.GetAPIError(s3err.ErrAccessDenied), + }, + }, + { + name: "fails to get bucket policy", + input: testInput{ + locals: defaultLocals, + beRes: []byte{}, + beErr: s3err.GetAPIError(s3err.ErrNoSuchBucket), + }, + output: testOutput{ + response: &Response{ + MetaOpts: &MetaOptions{ + BucketOwner: "root", + }, + }, + err: s3err.GetAPIError(s3err.ErrNoSuchBucket), + }, + }, + { + name: "fails to parse bucket policy", + input: testInput{ + locals: defaultLocals, + beRes: []byte("invalid policy"), + }, + output: testOutput{ + response: &Response{ + MetaOpts: &MetaOptions{ + BucketOwner: "root", + }, + }, + err: s3err.APIError{ + Code: "MalformedPolicy", + Description: "This policy contains invalid Json", + HTTPStatusCode: http.StatusBadRequest, + }, + }, + }, + { + name: "successful response", + input: testInput{ + locals: defaultLocals, + beRes: []byte(mockResp), + }, + output: testOutput{ + response: &Response{ + Data: types.PolicyStatus{ + IsPublic: &isPublic, + }, + MetaOpts: &MetaOptions{ + BucketOwner: "root", + }, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + be := &BackendMock{ + GetBucketPolicyFunc: func(contextMoqParam context.Context, bucket string) ([]byte, error) { + return tt.input.beRes.([]byte), tt.input.beErr + }, + } + + ctrl := S3ApiController{ + be: be, + } + + testController( + t, + ctrl.GetBucketPolicyStatus, + tt.output.response, + tt.output.err, + ctxInputs{ + locals: tt.input.locals, + body: tt.input.body, + }) + }) + } +} + func TestS3ApiController_ListObjectVersions(t *testing.T) { listVersionsResult := s3response.ListVersionsResult{ Name: utils.GetStringPtr("name"), diff --git a/s3api/router.go b/s3api/router.go index 6eafbc1b..51ba5d6a 100644 --- a/s3api/router.go +++ b/s3api/router.go @@ -737,6 +737,20 @@ func (sa *S3ApiRouter) Init(app *fiber.App, be backend.Backend, iam auth.IAMServ middlewares.ApplyBucketCORS(be), middlewares.ParseAcl(be), )) + bucketRouter.Get("", + middlewares.MatchQueryArgs("policyStatus"), + controllers.ProcessHandlers( + ctrl.GetBucketPolicyStatus, + metrics.ActionGetBucketPolicyStatus, + services, + middlewares.BucketObjectNameValidator(), + middlewares.AuthorizePublicBucketAccess(be, metrics.ActionGetBucketPolicyStatus, auth.GetBucketPolicyStatusAction, auth.PermissionRead), + middlewares.VerifyPresignedV4Signature(root, iam, region, debug), + middlewares.VerifyV4Signature(root, iam, region, debug), + middlewares.VerifyMD5Body(), + middlewares.ApplyBucketCORS(be), + middlewares.ParseAcl(be), + )) bucketRouter.Get("", middlewares.MatchQueryArgs("analytics", "id"), controllers.ProcessHandlers( diff --git a/tests/integration/group-tests.go b/tests/integration/group-tests.go index 05eab24a..ba59a7a8 100644 --- a/tests/integration/group-tests.go +++ b/tests/integration/group-tests.go @@ -520,6 +520,12 @@ func TestGetBucketPolicy(s *S3Conf) { GetBucketPolicy_success(s) } +func TestGetBucketPolicyStatus(s *S3Conf) { + GetBucketPolicyStatus_non_existing_bucket(s) + GetBucketPolicyStatus_no_such_bucket_policy(s) + GetBucketPolicyStatus_success(s) +} + func TestDeleteBucketPolicy(s *S3Conf) { DeleteBucketPolicy_non_existing_bucket(s) DeleteBucketPolicy_remove_before_setting(s) @@ -1342,6 +1348,9 @@ func GetIntTests() IntTests { "GetBucketPolicy_non_existing_bucket": GetBucketPolicy_non_existing_bucket, "GetBucketPolicy_not_set": GetBucketPolicy_not_set, "GetBucketPolicy_success": GetBucketPolicy_success, + "GetBucketPolicyStatus_non_existing_bucket": GetBucketPolicyStatus_non_existing_bucket, + "GetBucketPolicyStatus_no_such_bucket_policy": GetBucketPolicyStatus_no_such_bucket_policy, + "GetBucketPolicyStatus_success": GetBucketPolicyStatus_success, "DeleteBucketPolicy_non_existing_bucket": DeleteBucketPolicy_non_existing_bucket, "DeleteBucketPolicy_remove_before_setting": DeleteBucketPolicy_remove_before_setting, "DeleteBucketPolicy_success": DeleteBucketPolicy_success, diff --git a/tests/integration/tests.go b/tests/integration/tests.go index 9ee91227..77fa1517 100644 --- a/tests/integration/tests.go +++ b/tests/integration/tests.go @@ -13428,6 +13428,92 @@ func GetBucketPolicy_success(s *S3Conf) error { }) } +func GetBucketPolicyStatus_non_existing_bucket(s *S3Conf) error { + testName := "GetBucketPolicyStatus_non_existing_bucket" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err := s3client.GetBucketPolicyStatus(ctx, &s3.GetBucketPolicyStatusInput{ + Bucket: getPtr("non-existing-bucket"), + }) + cancel() + + return checkApiErr(err, s3err.GetAPIError(s3err.ErrNoSuchBucket)) + }) +} + +func GetBucketPolicyStatus_no_such_bucket_policy(s *S3Conf) error { + testName := "GetBucketPolicyStatus_no_such_bucket_policy" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err := s3client.GetBucketPolicyStatus(ctx, &s3.GetBucketPolicyStatusInput{ + Bucket: &bucket, + }) + cancel() + + return checkApiErr(err, s3err.GetAPIError(s3err.ErrNoSuchBucketPolicy)) + }) +} + +func GetBucketPolicyStatus_success(s *S3Conf) error { + testName := "GetBucketPolicyStatus_success" + return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + err := createUsers(s, []user{testuser1, testuser2}) + if err != nil { + return err + } + + for _, test := range []struct { + policy string + status bool + }{ + { + policy: genPolicyDoc("Allow", `"grt1"`, `["s3:DeleteBucket", "s3:GetBucketTagging"]`, fmt.Sprintf(`"arn:aws:s3:::%v"`, bucket)), + status: false, + }, + { + policy: genPolicyDoc("Allow", `"grt2"`, `"s3:GetObject"`, fmt.Sprintf(`"arn:aws:s3:::%v/obj"`, bucket)), + status: false, + }, + { + policy: genPolicyDoc("Allow", `"*"`, `"s3:PutObject"`, fmt.Sprintf(`"arn:aws:s3:::%v/*"`, bucket)), + status: true, + }, + { + policy: genPolicyDoc("Allow", `"*"`, `"s3:ListBucket"`, fmt.Sprintf(`"arn:aws:s3:::%v"`, bucket)), + status: true, + }, + } { + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + _, err := s3client.PutBucketPolicy(ctx, &s3.PutBucketPolicyInput{ + Bucket: &bucket, + Policy: &test.policy, + }) + cancel() + if err != nil { + return err + } + + ctx, cancel = context.WithTimeout(context.Background(), shortTimeout) + res, err := s3client.GetBucketPolicyStatus(ctx, &s3.GetBucketPolicyStatusInput{ + Bucket: &bucket, + }) + cancel() + if err != nil { + return err + } + if res.PolicyStatus.IsPublic == nil { + return fmt.Errorf("expected non nil policy status") + } + + if *res.PolicyStatus.IsPublic != test.status { + return fmt.Errorf("expected the policy public status to be %v, instead got %v", test.status, *res.PolicyStatus.IsPublic) + } + } + + return nil + }) +} + func DeleteBucketPolicy_non_existing_bucket(s *S3Conf) error { testName := "DeleteBucketPolicy_non_existing_bucket" return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {