S3:get_object() - Simple BucketPolicy validation (#5476)

moto/iam/access_control.py

@@ -353,12 +353,14 @@ class IAMPolicy(object):
 
         self._policy_json = json.loads(policy_document)
 
-    def is_action_permitted(self, action):
+    def is_action_permitted(self, action, resource="*"):
         permitted = False
         if isinstance(self._policy_json["Statement"], list):
             for policy_statement in self._policy_json["Statement"]:
                 iam_policy_statement = IAMPolicyStatement(policy_statement)
-                permission_result = iam_policy_statement.is_action_permitted(action)
+                permission_result = iam_policy_statement.is_action_permitted(
+                    action, resource
+                )
                 if permission_result == PermissionResult.DENIED:
                     return permission_result
                 elif permission_result == PermissionResult.PERMITTED:
@@ -377,7 +379,7 @@ class IAMPolicyStatement(object):
     def __init__(self, statement):
         self._statement = statement
 
-    def is_action_permitted(self, action):
+    def is_action_permitted(self, action, resource="*"):
         is_action_concerned = False
 
         if "NotAction" in self._statement:
@@ -388,7 +390,8 @@ class IAMPolicyStatement(object):
                 is_action_concerned = True
 
         if is_action_concerned:
-            if self._statement["Effect"] == "Allow":
+            same_resource = self._match(self._statement["Resource"], resource)
+            if self._statement["Effect"] == "Allow" and same_resource:
                 return PermissionResult.PERMITTED
             else:  # Deny
                 return PermissionResult.DENIED

moto/moto_server/threaded_moto_server.py

@@ -31,7 +31,7 @@ class ThreadedMotoServer:
         self._thread = Thread(target=self._server_entry, daemon=True)
         self._thread.start()
         while not self._server_ready:
-            time.sleep(0.5)
+            time.sleep(0.1)
 
     def stop(self):
         self._server_ready = False

moto/s3/models.py

@@ -27,6 +27,7 @@ from moto.core.utils import (
     BackendDict,
 )
 from moto.cloudwatch.models import MetricDatum
+from moto.iam.access_control import IAMPolicy, PermissionResult
 from moto.moto_api import state_manager
 from moto.moto_api._internal.managed_state_model import ManagedState
 from moto.utilities.tagging_service import TaggingService
@@ -907,6 +908,13 @@ class FakeBucket(CloudFormationModel):
     def is_versioned(self):
         return self.versioning_status == "Enabled"
 
+    def allow_action(self, action, resource):
+        if self.policy is None:
+            return False
+        iam_policy = IAMPolicy(self.policy.decode())
+        result = iam_policy.is_action_permitted(action, resource)
+        return result == PermissionResult.PERMITTED
+
     def set_lifecycle(self, rules):
         self.rules = []
         for rule in rules:
@@ -1531,7 +1539,7 @@ class S3Backend(BaseBackend, CloudWatchMetricProvider):
     def list_buckets(self):
         return self.buckets.values()
 
-    def get_bucket(self, bucket_name):
+    def get_bucket(self, bucket_name) -> FakeBucket:
         try:
             return self.buckets[bucket_name]
         except KeyError:

moto/s3/responses.py

@@ -1155,10 +1155,13 @@ class S3Response(BaseResponse):
                 signed_url = "Signature=" in request.path
             key = self.backend.get_object(bucket_name, key_name)
 
-            if key:
-                if (key.acl and not key.acl.public_read) and not signed_url:
+            if key and not signed_url:
+                bucket = self.backend.get_bucket(bucket_name)
+                resource = f"arn:aws:s3:::{bucket_name}/{key_name}"
+                bucket_policy_allows = bucket.allow_action("s3:GetObject", resource)
+                if not bucket_policy_allows and (key.acl and not key.acl.public_read):
                     return 403, {}, ""
-            elif signed_url:
+            elif signed_url and not key:
                 # coming in from requests.get(s3.generate_presigned_url())
                 if self._invalid_headers(request.url, dict(request.headers)):
                     return 403, {}, S3_INVALID_PRESIGNED_PARAMETERS

tests/test_s3/test_s3_bucket_policy.py

@@ -0,0 +1,62 @@
+import boto3
+import json
+import requests
+import pytest
+import sure  # noqa # pylint: disable=unused-import
+
+from moto.moto_server.threaded_moto_server import ThreadedMotoServer
+
+
+class TestBucketPolicy:
+    @staticmethod
+    def setup_class(cls):
+        cls.server = ThreadedMotoServer(port="6000", verbose=False)
+        cls.server.start()
+
+    def setup(self) -> None:
+        self.client = boto3.client(
+            "s3", region_name="us-east-1", endpoint_url="http://localhost:6000"
+        )
+        self.client.create_bucket(Bucket="mybucket")
+        self.client.put_object(Bucket="mybucket", Key="test_txt", Body=b"mybytes")
+        self.key_name = "http://localhost:6000/mybucket/test_txt"
+
+    def teardown(self) -> None:
+        self.client.delete_object(Bucket="mybucket", Key="test_txt")
+        self.client.delete_bucket(Bucket="mybucket")
+
+    @staticmethod
+    def teardown_class(cls):
+        cls.server.stop()
+
+    @pytest.mark.parametrize(
+        "kwargs,status",
+        [
+            ({}, 200),
+            ({"resource": "arn:aws:s3:::mybucket/test_txt"}, 200),
+            ({"resource": "arn:aws:s3:::notmybucket/*"}, 403),
+            ({"resource": "arn:aws:s3:::mybucket/other*"}, 403),
+            ({"actions": ["s3:PutObject"]}, 403),
+            ({"effect": "Deny"}, 403),
+        ],
+    )
+    def test_policy_allow_all(self, kwargs, status):
+        self._put_policy(**kwargs)
+
+        requests.get(self.key_name).status_code.should.equal(status)
+
+    def _put_policy(
+        self, resource="arn:aws:s3:::mybucket/*", effect="Allow", actions=None
+    ):
+        policy = {
+            "Version": "2012-10-17",
+            "Statement": [
+                {
+                    "Effect": effect,
+                    "Principal": "*",
+                    "Action": actions or ["s3:GetObject"],
+                    "Resource": resource,
+                }
+            ],
+        }
+        self.client.put_bucket_policy(Bucket="mybucket", Policy=json.dumps(policy))