PutBucketPolicy: specifying '*' as a principal results in 500

[evgeniiz321]

    policy_document = json.dumps(
        {
            "Version": "2012-10-17",
            "Statement": [
                {
                    "Effect": "Allow",
                    "Principal": "*",
                    "Action": "*",
                    "Resource": [resource1, resource2],
                }
            ],
        }
    )
    client.put_bucket_policy(Bucket=bucket_name, Policy=policy_document)

Results in

2023-10-19T01:49:14.040Z        error   handler/util.go:29      call method     {"status": 500, "request_id": "c8b88b4c-bc35-4fea-a1fc-f69cee176925", "method": "PutBucketPolicy", "bucket": "yournamehere-0ejrug068pe8wi6m-1", "object": "", "description": "could not parse bucket policy", "error": "json: cannot unmarshal string into Go struct field statement.Statement.Principal of type handler.principal"}

According to https://docs.aws.amazon.com/AmazonS3/latest/userguide/example-bucket-policies.html, https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_policies_elements_principal.html and https://gist.github.com/jstewmon/ee5d4b7ec0d8d60cbc303cb515272f8a#file-aws-iam-poilcy-schema-json-L129 "Principal": "*" is a valid principal.

Also take a look at the whole spec I provided above, looks like it is not parsed correctly. E.g. test_bucket_policy_put_obj_request_obj_tag fails with

2023-10-19T23:38:21.897Z        error   handler/util.go:29      call method     {"status": 500, "request_id": "be0dc325-c606-4a34-a33e-1042c4c684c4", "method": "PutBucketPolicy", "bucket": "yournamehere-4xmblafeakl1mnmp-1", "object": "", "description": "could not parse bucket policy", "error": "json: cannot unmarshal string into Go struct field statement.Statement.Action of type []string"}

But the schema provided is valid:

{"Version": "2012-10-17", "Statement": [{"Action": "s3:PutObject", "Principal": {"AWS": "*"}, "Effect": "Allow", "Resource": "arn:aws:s3:::yournamehere-4xmblafeakl1mnmp-1/*", "Condition": {"StringEquals": {"s3:RequestObjectTag/security": "public"}}}]}

For a complete list of failing tests due to an incorrect schema parsing see tests with this mark:

@pytest.mark.skip(reason="https://github.com/nspcc-dev/neofs-s3-gw/issues/863")

[smallhive]

Looks like such principal in not supported

[evgeniiz321]

I maybe wrong, but it seems to me, that it is said here that we should support Principal: '*', but don't support multiple 'Principal' inside single 'Statement'. Anyways - if we don't want to support the schema above - we should return some message and status code that makes sense, not 500.

[smallhive]

Correct, but object wildcard:

{
            "Version": "2012-10-17",
            "Statement": [
                {
                    "Effect": "Allow",
                    "Principal": {"AWS": "*"},
                    "Action": "*",
                    "Resource": [resource1, resource2],
                }
            ],
        }

not just string wildcard:

{
            "Version": "2012-10-17",
            "Statement": [
                {
                    "Effect": "Allow",
                    "Principal": "*",
                    "Action": "*",
                    "Resource": [resource1, resource2],
                }
            ],
        }

[roman-khimov]

This (and Action) can be fixed with custom MarshalJSON, it's OK to have it here for better compatibility. "Action": "*" and "Principal": "*" notations can be expected to be quite popular (they're just very simple for the purpose).

[smallhive]

PR fixes the wildcard issue, but the test still failed.
I need assistance to handle it. Right now tests failed with the next s3gate error

2024-06-04T14:30:12.450+0400    error   handler/util.go:29      call method     {"status": 403, "request_id": "ef71a550-9ffb-4551-8fde-763f78b04b9d", "method": "PutObject", "bucket": "yournamehere-1469jxiccnya4avo-1", "object": "testobj", "description": "could not get bucket settings", "error": "couldn't get node: access denied: rpc error: code = Unknown desc = access to operation GET is denied by extended ACL check: DENY eACL rule"}

The errors appears here when we are trying to get data from tree service.
On one hand, it is a valid error, because test sets only PutObject action.
On the other hand, we can't put an object because we can't read info about lockConfiguration from treeService

[roman-khimov]

Likely there are other valid uses for * that will have a broader set of actions, so this can be documented as a known limitation for now. Tree service is to be discontinued, so we won't even try fixing it.