From f14f3eecc963282ff2081530a42e3b5ba45f7949 Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Thu, 4 May 2023 12:40:18 +0200 Subject: [PATCH] s3_bucket - Ensure public_access settings are configured before policies (#1511) s3_bucket - Ensure public_access settings are configured before policies SUMMARY At the end of April Amazon updated various S3 bucket defaults. Buckets now have public_access blocked by default, and object_owner set to "BucketOwnerEnforced". https://aws.amazon.com/blogs/aws/heads-up-amazon-s3-security-changes-are-coming-in-april-of-2023/ This uncovered a race condition where we set the policy before setting the public_access configs. ISSUE TYPE Bugfix Pull Request COMPONENT NAME s3_bucket ADDITIONAL INFORMATION Reviewed-by: Alina Buzachis --- .../1511-s3_bucket-public_access.yml | 2 + plugins/modules/s3_bucket.py | 79 ++++++++++--------- .../s3_bucket/roles/s3_bucket/tasks/acl.yml | 1 + .../roles/s3_bucket/tasks/complex.yml | 2 + 4 files changed, 47 insertions(+), 37 deletions(-) create mode 100644 changelogs/fragments/1511-s3_bucket-public_access.yml diff --git a/changelogs/fragments/1511-s3_bucket-public_access.yml b/changelogs/fragments/1511-s3_bucket-public_access.yml new file mode 100644 index 00000000000..2206f2c0069 --- /dev/null +++ b/changelogs/fragments/1511-s3_bucket-public_access.yml @@ -0,0 +1,2 @@ +minor_changes: +- s3_bucket - ensure ``public_access`` is configured before updating policies (https://github.com/ansible-collections/amazon.aws/pull/1511). diff --git a/plugins/modules/s3_bucket.py b/plugins/modules/s3_bucket.py index 79aa3796161..f775ec212d0 100644 --- a/plugins/modules/s3_bucket.py +++ b/plugins/modules/s3_bucket.py @@ -83,6 +83,10 @@ description: - Configure public access block for S3 bucket. - This option cannot be used together with I(delete_public_access). + - | + Note: At the end of April 2023 Amazon updated the default settings to block public access by + default. While the defaults for this module remain unchanged, it is necessary to explicitly + pass the I(public_access) parameter to enable public access ACLs. suboptions: block_public_acls: description: Sets BlockPublicAcls value. @@ -122,6 +126,7 @@ if the object is uploaded with the bucket-owner-full-control canned ACL. - This option cannot be used together with a I(delete_object_ownership) definition. - C(BucketOwnerEnforced) has been added in version 3.2.0. + - "Note: At the end of April 2023 Amazon updated the default setting to C(BucketOwnerEnforced)." choices: [ 'BucketOwnerEnforced', 'BucketOwnerPreferred', 'ObjectWriter' ] type: str version_added: 2.0.0 @@ -475,6 +480,43 @@ def create_or_update_bucket(s3_client, module): result["requester_pays"] = requester_pays + # Public access clock configuration + current_public_access = {} + try: + current_public_access = get_bucket_public_access(s3_client, name) + except is_boto3_error_code(["NotImplemented", "XNotImplemented"]) as e: + if public_access is not None: + module.fail_json_aws(e, msg="Bucket public access settings are not supported by the current S3 Endpoint") + except is_boto3_error_code("AccessDenied") as e: + if public_access is not None: + module.fail_json_aws(e, msg="Failed to get bucket public access configuration") + module.debug("AccessDenied fetching bucket public access settings") + except ( + botocore.exceptions.BotoCoreError, + botocore.exceptions.ClientError, + ) as e: # pylint: disable=duplicate-except + module.fail_json_aws(e, msg="Failed to get bucket public access configuration") + else: + # -- Create / Update public access block + if public_access is not None: + camel_public_block = snake_dict_to_camel_dict(public_access, capitalize_first=True) + + if current_public_access == camel_public_block: + result["public_access_block"] = current_public_access + else: + put_bucket_public_access(s3_client, name, camel_public_block) + changed = True + result["public_access_block"] = camel_public_block + + # -- Delete public access block + if delete_public_access: + if current_public_access == {}: + result["public_access_block"] = current_public_access + else: + delete_bucket_public_access(s3_client, name) + changed = True + result["public_access_block"] = {} + # Policy try: current_policy = get_bucket_policy(s3_client, name) @@ -606,43 +648,6 @@ def create_or_update_bucket(s3_client, module): current_encryption = put_bucket_key_with_retry(module, s3_client, name, expected_encryption) changed = True result["encryption"] = current_encryption - # Public access clock configuration - current_public_access = {} - - try: - current_public_access = get_bucket_public_access(s3_client, name) - except is_boto3_error_code(["NotImplemented", "XNotImplemented"]) as e: - if public_access is not None: - module.fail_json_aws(e, msg="Bucket public access settings are not supported by the current S3 Endpoint") - except is_boto3_error_code("AccessDenied") as e: - if public_access is not None: - module.fail_json_aws(e, msg="Failed to get bucket public access configuration") - module.debug("AccessDenied fetching bucket public access settings") - except ( - botocore.exceptions.BotoCoreError, - botocore.exceptions.ClientError, - ) as e: # pylint: disable=duplicate-except - module.fail_json_aws(e, msg="Failed to get bucket public access configuration") - else: - # -- Create / Update public access block - if public_access is not None: - camel_public_block = snake_dict_to_camel_dict(public_access, capitalize_first=True) - - if current_public_access == camel_public_block: - result["public_access_block"] = current_public_access - else: - put_bucket_public_access(s3_client, name, camel_public_block) - changed = True - result["public_access_block"] = camel_public_block - - # -- Delete public access block - if delete_public_access: - if current_public_access == {}: - result["public_access_block"] = current_public_access - else: - delete_bucket_public_access(s3_client, name) - changed = True - result["public_access_block"] = {} # -- Bucket ownership try: diff --git a/tests/integration/targets/s3_bucket/roles/s3_bucket/tasks/acl.yml b/tests/integration/targets/s3_bucket/roles/s3_bucket/tasks/acl.yml index 1b5300ba584..f924af17368 100644 --- a/tests/integration/targets/s3_bucket/roles/s3_bucket/tasks/acl.yml +++ b/tests/integration/targets/s3_bucket/roles/s3_bucket/tasks/acl.yml @@ -12,6 +12,7 @@ - name: 'Create a simple bucket' s3_bucket: name: '{{ local_bucket_name }}' + object_ownership: BucketOwnerPreferred public_access: block_public_acls: true block_public_policy: true diff --git a/tests/integration/targets/s3_bucket/roles/s3_bucket/tasks/complex.yml b/tests/integration/targets/s3_bucket/roles/s3_bucket/tasks/complex.yml index 7d2786301c2..8b8a8bdca10 100644 --- a/tests/integration/targets/s3_bucket/roles/s3_bucket/tasks/complex.yml +++ b/tests/integration/targets/s3_bucket/roles/s3_bucket/tasks/complex.yml @@ -9,6 +9,8 @@ policy: "{{ lookup('template','policy.json') }}" requester_pays: yes versioning: yes + public_access: + block_public_acls: false tags: example: tag1 another: tag2