S3: add granting logging perms using bucket policy (#6715)

moto/s3/models.py

@@ -54,7 +54,7 @@ from .cloud_formation import cfn_to_api_encryption, is_replacement_update
 from . import notifications
 from .select_object_content import parse_query
 from .utils import _VersionedKeyStore, CaseInsensitiveDict
-from .utils import ARCHIVE_STORAGE_CLASSES, STORAGE_CLASS
+from .utils import ARCHIVE_STORAGE_CLASSES, STORAGE_CLASS, LOGGING_SERVICE_PRINCIPAL
 from ..events.notifications import send_notification as events_send_notification
 from ..settings import get_s3_default_key_buffer_size, S3_UPLOAD_PART_MIN_SIZE
 from ..settings import s3_allow_crossdomain_access
@@ -1196,22 +1196,38 @@ class FakeBucket(CloudFormationModel):
     def delete_cors(self) -> None:
         self.cors = []
 
-    def set_logging(
+    @staticmethod
-        self, logging_config: Optional[Dict[str, Any]], bucket_backend: "S3Backend"
+    def _log_permissions_enabled_policy(
-    ) -> None:
+        target_bucket: "FakeBucket", target_prefix: Optional[str]
-        if not logging_config:
+    ) -> bool:
-            self.logging = {}
+        target_bucket_policy = target_bucket.policy
-            return
+        if target_bucket_policy:
+            target_bucket_policy_json = json.loads(target_bucket_policy.decode())
+            for stmt in target_bucket_policy_json["Statement"]:
+                if (
+                    stmt.get("Principal", {}).get("Service")
+                    != LOGGING_SERVICE_PRINCIPAL
+                ):
+                    continue
+                if stmt.get("Effect", "") != "Allow":
+                    continue
+                if "s3:PutObject" not in stmt.get("Action", []):
+                    continue
+                if (
+                    stmt.get("Resource")
+                    != f"arn:aws:s3:::{target_bucket.name}/{target_prefix if target_prefix else ''}*"
+                    and stmt.get("Resource") != f"arn:aws:s3:::{target_bucket.name}/*"
+                    and stmt.get("Resource") != f"arn:aws:s3:::{target_bucket.name}"
+                ):
+                    continue
+                return True
 
-        # Target bucket must exist in the same account (assuming all moto buckets are in the same account):
+        return False
-        if not bucket_backend.buckets.get(logging_config["TargetBucket"]):
-            raise InvalidTargetBucketForLogging(
-                "The target bucket for logging does not exist."
-            )
 
-        # Does the target bucket have the log-delivery WRITE and READ_ACP permissions?
+    @staticmethod
+    def _log_permissions_enabled_acl(target_bucket: "FakeBucket") -> bool:
         write = read_acp = False
-        for grant in bucket_backend.buckets[logging_config["TargetBucket"]].acl.grants:  # type: ignore
+        for grant in target_bucket.acl.grants:  # type: ignore
             # Must be granted to: http://acs.amazonaws.com/groups/s3/LogDelivery
             for grantee in grant.grantees:
                 if grantee.uri == "http://acs.amazonaws.com/groups/s3/LogDelivery":
@@ -1226,20 +1242,39 @@ class FakeBucket(CloudFormationModel):
                         or "FULL_CONTROL" in grant.permissions
                     ):
                         read_acp = True
-
                     break
 
-        if not write or not read_acp:
+        return write and read_acp
+
+    def set_logging(
+        self, logging_config: Optional[Dict[str, Any]], bucket_backend: "S3Backend"
+    ) -> None:
+        if not logging_config:
+            self.logging = {}
+            return
+
+        # Target bucket must exist in the same account (assuming all moto buckets are in the same account):
+        target_bucket = bucket_backend.buckets.get(logging_config["TargetBucket"])
+        if not target_bucket:
             raise InvalidTargetBucketForLogging(
-                "You must give the log-delivery group WRITE and READ_ACP"
+                "The target bucket for logging does not exist."
-                " permissions to the target bucket"
+            )
+
+        target_prefix = self.logging.get("TargetPrefix", None)
+        has_policy_permissions = self._log_permissions_enabled_policy(
+            target_bucket=target_bucket, target_prefix=target_prefix
+        )
+        has_acl_permissions = self._log_permissions_enabled_acl(
+            target_bucket=target_bucket
+        )
+        if not (has_policy_permissions or has_acl_permissions):
+            raise InvalidTargetBucketForLogging(
+                "You must either provide the necessary permissions to the logging service using a bucket "
+                "policy or give the log-delivery group WRITE and READ_ACP permissions to the target bucket"
             )
 
         # Buckets must also exist within the same region:
-        if (
+        if target_bucket.region_name != self.region_name:
-            bucket_backend.buckets[logging_config["TargetBucket"]].region_name
-            != self.region_name
-        ):
             raise CrossLocationLoggingProhibitted()
 
         # Checks pass -- set the logging config:

moto/s3/utils.py

@@ -36,6 +36,7 @@ STORAGE_CLASS = [
     "ONEZONE_IA",
     "INTELLIGENT_TIERING",
 ] + ARCHIVE_STORAGE_CLASSES
+LOGGING_SERVICE_PRINCIPAL = "logging.s3.amazonaws.com"
 
 
 def bucket_name_from_url(url: str) -> Optional[str]:  # type: ignore

tests/test_s3/test_s3_logging.py

@@ -1,8 +1,17 @@
+import json
+
 import boto3
-from botocore.client import ClientError
 import pytest
+from botocore.client import ClientError
+from unittest import SkipTest
+from unittest.mock import patch
+
 
 from moto import mock_s3
+from moto import settings
+from moto.core import DEFAULT_ACCOUNT_ID
+from moto.s3 import s3_backends
+from moto.s3.models import FakeBucket
 from moto.s3.responses import DEFAULT_REGION_NAME
 
 
@@ -261,3 +270,322 @@ def test_log_file_is_created():
     assert any(c for c in contents if bucket_name in c)
     assert any(c for c in contents if "REST.GET.BUCKET" in c)
     assert any(c for c in contents if "REST.PUT.BUCKET" in c)
+
+
+@mock_s3
+def test_invalid_bucket_logging_when_permissions_are_false():
+    if settings.TEST_SERVER_MODE:
+        raise SkipTest("Can't patch permission logic in ServerMode")
+
+    s3_client = boto3.client("s3", region_name=DEFAULT_REGION_NAME)
+    bucket_name = "mybucket"
+    log_bucket = "logbucket"
+    s3_client.create_bucket(Bucket=bucket_name)
+    s3_client.create_bucket(Bucket=log_bucket)
+    with patch(
+        "moto.s3.models.FakeBucket._log_permissions_enabled_policy", return_value=False
+    ), patch(
+        "moto.s3.models.FakeBucket._log_permissions_enabled_acl", return_value=False
+    ):
+        with pytest.raises(ClientError) as err:
+            s3_client.put_bucket_logging(
+                Bucket=bucket_name,
+                BucketLoggingStatus={
+                    "LoggingEnabled": {"TargetBucket": log_bucket, "TargetPrefix": ""}
+                },
+            )
+        assert err.value.response["Error"]["Code"] == "InvalidTargetBucketForLogging"
+        assert "log-delivery" in err.value.response["Error"]["Message"]
+
+
+@mock_s3
+def test_valid_bucket_logging_when_permissions_are_true():
+    if settings.TEST_SERVER_MODE:
+        raise SkipTest("Can't patch permission logic in ServerMode")
+
+    s3_client = boto3.client("s3", region_name=DEFAULT_REGION_NAME)
+    bucket_name = "mybucket"
+    log_bucket = "logbucket"
+    s3_client.create_bucket(Bucket=bucket_name)
+    s3_client.create_bucket(Bucket=log_bucket)
+    with patch(
+        "moto.s3.models.FakeBucket._log_permissions_enabled_policy", return_value=True
+    ), patch(
+        "moto.s3.models.FakeBucket._log_permissions_enabled_acl", return_value=True
+    ):
+        s3_client.put_bucket_logging(
+            Bucket=bucket_name,
+            BucketLoggingStatus={
+                "LoggingEnabled": {
+                    "TargetBucket": log_bucket,
+                    "TargetPrefix": f"{bucket_name}/",
+                }
+            },
+        )
+        result = s3_client.get_bucket_logging(Bucket=bucket_name)
+        assert result["LoggingEnabled"]["TargetBucket"] == log_bucket
+        assert result["LoggingEnabled"]["TargetPrefix"] == f"{bucket_name}/"
+
+
+@mock_s3
+def test_bucket_policy_not_set():
+    if settings.TEST_SERVER_MODE:
+        raise SkipTest("Can't patch permission logic in ServerMode")
+
+    s3_client = boto3.client("s3", region_name=DEFAULT_REGION_NAME)
+    s3_backend = s3_backends[DEFAULT_ACCOUNT_ID]["global"]
+
+    log_bucket = "log_bucket"
+    s3_client.create_bucket(Bucket=log_bucket)
+    log_bucket_obj = s3_backend.get_bucket(log_bucket)
+
+    assert (
+        FakeBucket._log_permissions_enabled_policy(
+            target_bucket=log_bucket_obj, target_prefix=""
+        )
+        is False
+    )
+
+
+@mock_s3
+def test_bucket_policy_principal():
+    if settings.TEST_SERVER_MODE:
+        raise SkipTest("Can't patch permission logic in ServerMode")
+
+    s3_client = boto3.client("s3", region_name=DEFAULT_REGION_NAME)
+    s3_backend = s3_backends[DEFAULT_ACCOUNT_ID]["global"]
+
+    log_bucket = "log_bucket"
+    s3_client.create_bucket(Bucket=log_bucket)
+    log_bucket_obj = s3_backend.get_bucket(log_bucket)
+
+    invalid_principal_policy = {
+        "Version": "2012-10-17",
+        "Statement": [
+            {
+                "Sid": "S3ServerAccessLogsPolicy",
+                "Effect": "Allow",
+                "Principal": {"Service": "not_logging.s3.amazonaws.com"},
+                "Action": ["s3:PutObject"],
+                "Resource": f"arn:aws:s3:::{log_bucket}/*",
+            }
+        ],
+    }
+    s3_client.put_bucket_policy(
+        Bucket=log_bucket, Policy=json.dumps(invalid_principal_policy)
+    )
+    assert (
+        FakeBucket._log_permissions_enabled_policy(
+            target_bucket=log_bucket_obj, target_prefix=""
+        )
+        is False
+    )
+
+    s3_client.delete_bucket_policy(Bucket=log_bucket)
+    valid_principal_policy = {
+        "Version": "2012-10-17",
+        "Statement": [
+            {
+                "Sid": "S3ServerAccessLogsPolicy",
+                "Effect": "Allow",
+                "Principal": {"Service": "logging.s3.amazonaws.com"},
+                "Action": ["s3:PutObject"],
+                "Resource": f"arn:aws:s3:::{log_bucket}/*",
+            }
+        ],
+    }
+    s3_client.put_bucket_policy(
+        Bucket=log_bucket, Policy=json.dumps(valid_principal_policy)
+    )
+    assert FakeBucket._log_permissions_enabled_policy(
+        target_bucket=log_bucket_obj, target_prefix=""
+    )
+
+
+@mock_s3
+def test_bucket_policy_effect():
+    if settings.TEST_SERVER_MODE:
+        raise SkipTest("Can't patch permission logic in ServerMode")
+
+    s3_client = boto3.client("s3", region_name=DEFAULT_REGION_NAME)
+    s3_backend = s3_backends[DEFAULT_ACCOUNT_ID]["global"]
+
+    log_bucket = "log_bucket"
+    s3_client.create_bucket(Bucket=log_bucket)
+    log_bucket_obj = s3_backend.get_bucket(log_bucket)
+    deny_effect_policy = {
+        "Version": "2012-10-17",
+        "Statement": [
+            {
+                "Sid": "S3ServerAccessLogsPolicy",
+                "Effect": "Deny",
+                "Principal": {"Service": "logging.s3.amazonaws.com"},
+                "Action": ["s3:PutObject"],
+                "Resource": f"arn:aws:s3:::{log_bucket}/*",
+            }
+        ],
+    }
+    s3_client.put_bucket_policy(
+        Bucket=log_bucket, Policy=json.dumps(deny_effect_policy)
+    )
+    assert (
+        FakeBucket._log_permissions_enabled_policy(
+            target_bucket=log_bucket_obj, target_prefix=""
+        )
+        is False
+    )
+
+    s3_client.delete_bucket_policy(Bucket=log_bucket)
+    allow_effect_policy = {
+        "Version": "2012-10-17",
+        "Statement": [
+            {
+                "Sid": "S3ServerAccessLogsPolicy",
+                "Effect": "Allow",
+                "Principal": {"Service": "logging.s3.amazonaws.com"},
+                "Action": ["s3:PutObject"],
+                "Resource": f"arn:aws:s3:::{log_bucket}/*",
+            }
+        ],
+    }
+    s3_client.put_bucket_policy(
+        Bucket=log_bucket, Policy=json.dumps(allow_effect_policy)
+    )
+    assert FakeBucket._log_permissions_enabled_policy(
+        target_bucket=log_bucket_obj, target_prefix=""
+    )
+
+
+@mock_s3
+def test_bucket_policy_action():
+    if settings.TEST_SERVER_MODE:
+        raise SkipTest("Can't patch permission logic in ServerMode")
+
+    s3_client = boto3.client("s3", region_name=DEFAULT_REGION_NAME)
+    s3_backend = s3_backends[DEFAULT_ACCOUNT_ID]["global"]
+
+    log_bucket = "log_bucket"
+    s3_client.create_bucket(Bucket=log_bucket)
+    log_bucket_obj = s3_backend.get_bucket(log_bucket)
+    non_put_object_policy = {
+        "Version": "2012-10-17",
+        "Statement": [
+            {
+                "Sid": "S3ServerAccessLogsPolicy",
+                "Effect": "Allow",
+                "Principal": {"Service": "logging.s3.amazonaws.com"},
+                "Action": ["s3:GetObject"],
+                "Resource": f"arn:aws:s3:::{log_bucket}/*",
+            }
+        ],
+    }
+    s3_client.put_bucket_policy(
+        Bucket=log_bucket, Policy=json.dumps(non_put_object_policy)
+    )
+    assert (
+        FakeBucket._log_permissions_enabled_policy(
+            target_bucket=log_bucket_obj, target_prefix=""
+        )
+        is False
+    )
+
+    s3_client.delete_bucket_policy(Bucket=log_bucket)
+    put_object_policy = {
+        "Version": "2012-10-17",
+        "Statement": [
+            {
+                "Sid": "S3ServerAccessLogsPolicy",
+                "Effect": "Allow",
+                "Principal": {"Service": "logging.s3.amazonaws.com"},
+                "Action": ["s3:PutObject"],
+                "Resource": f"arn:aws:s3:::{log_bucket}/*",
+            }
+        ],
+    }
+    s3_client.put_bucket_policy(Bucket=log_bucket, Policy=json.dumps(put_object_policy))
+    assert FakeBucket._log_permissions_enabled_policy(
+        target_bucket=log_bucket_obj, target_prefix=""
+    )
+
+
+@mock_s3
+def test_bucket_policy_resource():
+    if settings.TEST_SERVER_MODE:
+        raise SkipTest("Can't patch permission logic in ServerMode")
+
+    s3_client = boto3.client("s3", region_name=DEFAULT_REGION_NAME)
+    s3_backend = s3_backends[DEFAULT_ACCOUNT_ID]["global"]
+
+    log_bucket = "log_bucket"
+    s3_client.create_bucket(Bucket=log_bucket)
+    log_bucket_obj = s3_backend.get_bucket(log_bucket)
+    entire_bucket_policy = {
+        "Version": "2012-10-17",
+        "Statement": [
+            {
+                "Sid": "S3ServerAccessLogsPolicy",
+                "Effect": "Allow",
+                "Principal": {"Service": "logging.s3.amazonaws.com"},
+                "Action": ["s3:PutObject"],
+                "Resource": f"arn:aws:s3:::{log_bucket}/*",
+            }
+        ],
+    }
+    s3_client.put_bucket_policy(
+        Bucket=log_bucket, Policy=json.dumps(entire_bucket_policy)
+    )
+    assert FakeBucket._log_permissions_enabled_policy(
+        target_bucket=log_bucket_obj, target_prefix=""
+    )
+    assert FakeBucket._log_permissions_enabled_policy(
+        target_bucket=log_bucket_obj, target_prefix="prefix"
+    )
+
+    s3_client.delete_bucket_policy(Bucket=log_bucket)
+    bucket_level_policy = {
+        "Version": "2012-10-17",
+        "Statement": [
+            {
+                "Sid": "S3ServerAccessLogsPolicy",
+                "Effect": "Allow",
+                "Principal": {"Service": "logging.s3.amazonaws.com"},
+                "Action": ["s3:PutObject"],
+                "Resource": f"arn:aws:s3:::{log_bucket}",
+            }
+        ],
+    }
+    s3_client.put_bucket_policy(
+        Bucket=log_bucket, Policy=json.dumps(bucket_level_policy)
+    )
+    assert FakeBucket._log_permissions_enabled_policy(
+        target_bucket=log_bucket_obj, target_prefix=""
+    )
+    assert FakeBucket._log_permissions_enabled_policy(
+        target_bucket=log_bucket_obj, target_prefix="prefix"
+    )
+
+    s3_client.delete_bucket_policy(Bucket=log_bucket)
+    specfic_prefix_policy = {
+        "Version": "2012-10-17",
+        "Statement": [
+            {
+                "Sid": "S3ServerAccessLogsPolicy",
+                "Effect": "Allow",
+                "Principal": {"Service": "logging.s3.amazonaws.com"},
+                "Action": ["s3:PutObject"],
+                "Resource": f"arn:aws:s3:::{log_bucket}/prefix*",
+            }
+        ],
+    }
+    s3_client.put_bucket_policy(
+        Bucket=log_bucket, Policy=json.dumps(specfic_prefix_policy)
+    )
+    assert (
+        FakeBucket._log_permissions_enabled_policy(
+            target_bucket=log_bucket_obj, target_prefix=""
+        )
+        is False
+    )
+    assert FakeBucket._log_permissions_enabled_policy(
+        target_bucket=log_bucket_obj, target_prefix="prefix"
+    )