aws-security-reference-architecture-examples icon indicating copy to clipboard operation
aws-security-reference-architecture-examples copied to clipboard

[BUG] SRA template errors with AWS Controls Library

Open lukenny opened this issue 11 months ago • 0 comments

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Describe the bug

When used CT.S3.PR.8 control "Require that Amazon S3 bucket requests use Secure Socket Layer" to check against the file sra-cloudtrail-org-bucket.yaml. It's complaining that the Deny "Action" should be "s3:*".

To Reproduce

Steps to reproduce the behavior:

Run the guard file CT.S3.PR.8 rule specification against the sra-cloudtrail-org-bucket.yaml, you should see the error below:

"error_message": "Check was not compliant as property [/Resources/rOrgTrailBucketPolicy/Properties/PolicyDocument/Statement/0/Action/0[L:138,C:16]] was not present in [(resolved, Path=[L:0,C:0] Value=[\"s3:*\",\"*\"])]"
"error_message": "Check was not compliant as property [/Resources/rOrgTrailBucketPolicy/Properties/PolicyDocument/Statement/0/Action/1[L:139,C:16]] was not present in [(resolved, Path=[L:0,C:0] Value=[\"s3:*\",\"*\"])]"
"error_message": "Check was not compliant as property [/Resources/rOrgTrailBucketPolicy/Properties/PolicyDocument/Statement/0/Action/2[L:140,C:16]] was not present in [(resolved, Path=[L:0,C:0] Value=[\"s3:*\",\"*\"])]"
# ###################################
##       Rule Specification        ##
#####################################
# 
# Rule Identifier:
#   s3_bucket_policy_ssl_requests_only_check
# 
# Description:
#   This control checks whether Amazon S3 bucket policies require requests to use Secure Socket Layer (SSL).
# 
# Reports on:
#   AWS::S3::BucketPolicy
# 
# Evaluates:
#   AWS CloudFormation, AWS CloudFormation hook
# 
# Rule Parameters:
#   None
# 
# Scenarios:
#   Scenario: 1
#     Given: The input document is an AWS CloudFormation or AWS CloudFormation hook document
#       And: The input document does not contain any S3 bucket policies
#      Then: SKIP
#   Scenario: 2
#     Given: The input document is an AWS CloudFormation or AWS CloudFormation hook document
#       And: The input document contains an S3 bucket policy
#       And: 'Policydocument' has not been provided
#      Then: FAIL
#   Scenario: 3
#     Given: The input document is an AWS CloudFormation or AWS CloudFormation hook document
#       And: The input document contains an S3 bucket policy
#       And: 'Policydocument' does not include a statement that denies Principal  ('*', AWS: '*')
#            all Actions ('s3:*', '*') over resource ('*' or bucketArn, bucketObjectArn) when the condition
#            "aws:SecureTransport" is "false"
#      Then: FAIL
#   Scenario: 4
#     Given: The input document is an AWS CloudFormation or AWS CloudFormation hook document
#       And: The input document contains an S3 bucket policy
#       And: 'Policydocument' includes a statement that denies Principal  ('*', AWS: '*')
#            all Actions ('s3:*', '*') over resource ('*' or bucketArn, bucketObjectArn) when the condition
#            "aws:SecureTransport" is "false"
#      Then: PASS

#
# Constants
#
let S3_BUCKET_POLICY_TYPE = "AWS::S3::BucketPolicy"
let INPUT_DOCUMENT = this

let S3_BUCKET_ARN_PATTERN = /^arn:aws[a-z0-9\-]*:s3:::[a-z0-9][a-z0-9\.-]*[a-z0-9]$/
let S3_BUCKET_OBJECT_ARN_PATTERN = /^arn:aws[a-z0-9\-]*:s3:::[a-z0-9][a-z0-9\.-]*[a-z0-9]\/\*$/

#
# Assignments
#
let s3_bucket_policies = Resources.*[ Type == %S3_BUCKET_POLICY_TYPE ]

#
# Primary Rules
#
rule s3_bucket_policy_ssl_requests_only_check when is_cfn_template(%INPUT_DOCUMENT)
                                                   %s3_bucket_policies not empty {
    check(%s3_bucket_policies.Properties)
        <<
        [CT.S3.PR.8]: Require that Amazon S3 buckets request to use Secure Socket Layer
            [FIX]: Configure an Amazon S3 bucket policy statement that denies access to all principals and actions for the S3 bucket and bucket objects when a secure transport protocol is not in use.
        >>
}

rule s3_bucket_policy_ssl_requests_only_check when is_cfn_hook(%INPUT_DOCUMENT, %S3_BUCKET_POLICY_TYPE) {
    check(%INPUT_DOCUMENT.%S3_BUCKET_POLICY_TYPE.resourceProperties)
        <<
        [CT.S3.PR.8]: Require that Amazon S3 buckets request to use Secure Socket Layer
            [FIX]: Configure an Amazon S3 bucket policy statement that denies access to all principals and actions for the S3 bucket and bucket objects when a secure transport protocol is not in use.
        >>
}

#
# Parameterized Rules
#
rule check(s3_bucket_policy) {
    %s3_bucket_policy {
        # Scenario 2
        PolicyDocument exists
        PolicyDocument is_struct

        PolicyDocument {
            Statement exists
            Statement is_list or
            Statement is_struct

            #Scenario 3 and 4
            some Statement[*] {
                check_statement_ssl_requests_only(this)
            }
        }
    }
}

rule check_statement_ssl_requests_only(statement) {
    %statement{
        check_all_required_statement_properties(this)

        Effect == "Deny"
        Action[*] in ["s3:*", "*"]

        Principal == "*" or
        Principal {
            AWS exists
            AWS == "*"
        }

        Resource[*] == "*" or
        check_resource_for_bucket_arns(Resource) or
        check_resource_for_bucket_arn_refs(Resource)

        Condition is_struct
        Condition == {
            "Bool": {
                "aws:SecureTransport": "false"
            }
        }

    }
}

rule check_all_required_statement_properties(statement) {
    %statement {
        Effect exists
        Action exists
        Principal exists
        Condition exists
        Resource exists
    }
}

rule check_resource_for_bucket_arns(resource) {
    %resource {
        this is_list
        this not empty
        some this[*] == %S3_BUCKET_ARN_PATTERN
        some this[*] == %S3_BUCKET_OBJECT_ARN_PATTERN
    }
}

rule check_resource_for_bucket_arn_refs(resource) {
    %resource {
        this is_list
        this not empty
        some this[*] {
            check_local_bucket_arn_reference(%INPUT_DOCUMENT, this, "AWS::S3::Bucket")
        }
        some this[*] {
            check_local_bucket_object_arn_reference(%INPUT_DOCUMENT, this, "AWS::S3::Bucket")
        }
    }
}

rule check_local_bucket_arn_reference(doc, reference_properties, referenced_resource_type) {
    %reference_properties {
        'Fn::GetAtt' {
            check_get_att_bucket_arn(this)
        }
    }
}

rule check_local_bucket_object_arn_reference(doc, reference_properties, referenced_resource_type) {
    %reference_properties {
        'Fn::Join' {
            this is_list
            this not empty
            this[1][0] {
                'Fn::GetAtt' {
                    check_get_att_bucket_arn(this)
                }
            }
            this[1][1] == "/*"
        }
    }
}

rule check_get_att_bucket_arn(get_att){
    %get_att {
        this is_list
        this not empty
        this[1] == "Arn"
        query_for_resource(%doc, this[0], %referenced_resource_type)
            <<Local Stack reference was invalid>>
    }
}

#
# Utility Rules
#
rule is_cfn_template(doc) {
    %doc {
        AWSTemplateFormatVersion exists  or
        Resources exists
    }
}

rule is_cfn_hook(doc, RESOURCE_TYPE) {
    %doc.%RESOURCE_TYPE.resourceProperties exists
}

rule query_for_resource(doc, resource_key, referenced_resource_type) {
    let referenced_resource = %doc.Resources[ keys == %resource_key ]
    %referenced_resource not empty
    %referenced_resource {
        Type == %referenced_resource_type
    }
}

Expected behavior

It shouldn't error out if it conforms with the [CT.S3.PR8 rule specification] (https://docs.aws.amazon.com/controltower/latest/userguide/s3-rules.html#ct-s3-pr-8-description)

Deployment Environment (please complete the following information)

  • Deployment Framework - Customizations for Control Tower and CloudFormation StackSets
  • Deployment Framework Version v.2.7.0

Additional context

Can the deny action be "s3:" instead of GetObject, s3:ListBucket and S3:PutObject

Action: - s3:GetObject* - s3:ListBucket - s3:PutObject

lukenny avatar Feb 27 '24 17:02 lukenny