cloudformation-guard icon indicating copy to clipboard operation
cloudformation-guard copied to clipboard

Check if a proper bucket policy exists for a bucket

Open robertjan-b opened this issue 4 years ago • 7 comments

I'm looking for guidance. I want to check if a bucket has a proper bucket policy. For example we want to check if encryption in transit is enabled for a bucket (TLS). If there is a bucket in the template, we need to make sure that also a bucket policy is attached to the bucket. How can I check this with cfn-guard?

  MyBucket:
    Type: AWS::S3::Bucket
  BucketPolicy:
    Type: 'AWS::S3::BucketPolicy'
    Properties:
      Bucket: !Ref MyBucket
      PolicyDocument:
        Version: '2012-10-17'
        Statement:
          - Sid: AllowSSLRequestsOnly # enforce encryption in transit
            Principal: '*'
            Action: 's3:*'
            Effect: Deny
            Resource:
              - !Sub '${MyBucket.Arn}'
              - !Sub '${MyBucket.Arn}/*'
            Condition:
              Bool:
                aws:SecureTransport: false```

robertjan-b avatar Jul 26 '21 11:07 robertjan-b

Thank you for your report. Here is a rule that does portion of the assessment

let policies = Resources.*[ Type == 'AWS::S3::BucketPolicy' ]
let bucket_references = some %policies.Properties.Bucket.Ref

rule ensure_buckets_have_tls_enabled when %policies !empty {
    %policies.Properties.PolicyDocument {
        # 
        # At least one Statement must contain secure transport condition 
        # 
        some Statement[*].Condition.Bool.'aws:SecureTransport' == true
    }

    let buckets = Resources.%bucket_references

    when %buckets not empty {
        %buckets.Type == 'AWS::S3::Bucket'
    }
}

#
# Feature being worked on to perform the check that all S3 buckets do have a 
# bucket policy associated using key/index captures
#
# let buckets = Resources[ bucket_name | Type == 'AWS::S3::BucketPolicy' ]
# rule enure_all_buckets_have_policy_associated when %buckets not empty {
#     %bucket_names == %bucket_references <<Not all S3 buckets have policy associated>>
# }

The commented out portion is under development, where the second assertion that all S3 buckets have a policy associated is being checked.

Test rules file

---
- name: Testing TLS false
  input:
    Resources:
      s3:
        Type: AWS::S3::Bucket
      policy:
        Type: 'AWS::S3::BucketPolicy'
        Properties:
          Bucket: { Ref: MyBucket }
          PolicyDocument:
            Version: '2012-10-17'
            Statement:
              - Sid: AllowSSLRequestsOnly # enforce encryption in transit
                Principal: '*'
                Action: 's3:*'
                Effect: Deny
                Resource:
                  - !Sub '${MyBucket.Arn}'
                  - !Sub '${MyBucket.Arn}/*'
                Condition:
                  Bool:
                    aws:SecureTransport: false
  expectations:
    rules:
      ensure_buckets_have_tls_enabled: FAIL

- name: Testing TLS true PASS
  input:
    Resources:
      s3:
        Type: AWS::S3::Bucket
      policy:
        Type: 'AWS::S3::BucketPolicy'
        Properties:
          Bucket: { Ref: MyBucket }
          PolicyDocument:
            Version: '2012-10-17'
            Statement:
              - Sid: AllowSSLRequestsOnly # enforce encryption in transit
                Principal: '*'
                Action: 's3:*'
                Effect: Deny
                Resource:
                  - !Sub '${MyBucket.Arn}'
                  - !Sub '${MyBucket.Arn}/*'
                Condition:
                  Bool:
                    aws:SecureTransport: true
  expectations:
    rules:
      ensure_buckets_have_tls_enabled: PASS

- name: Testing TLS attribute missing, FAIL
  input:
    Resources:
      s3:
        Type: AWS::S3::Bucket
      policy:
        Type: 'AWS::S3::BucketPolicy'
        Properties:
          Bucket: { Ref: MyBucket }
          PolicyDocument:
            Version: '2012-10-17'
            Statement:
              - Sid: AllowSSLRequestsOnly # enforce encryption in transit
                Principal: '*'
                Action: 's3:*'
                Effect: Deny
                Resource:
                  - !Sub '${MyBucket.Arn}'
                  - !Sub '${MyBucket.Arn}/*'
  expectations:
    rules:
      ensure_buckets_have_tls_enabled: FAIL

- name: Testing TLS attribute missing, FAIL
  input:
    Resources:
      s3:
        Type: AWS::S3::Bucket
      policy:
        Type: 'AWS::S3::BucketPolicy'
        Properties:
          Bucket: { Ref: MyBucket }
          PolicyDocument:
            Version: '2012-10-17'
            Statement:
              - Sid: AllowSSLRequestsOnly # enforce encryption in transit
                Principal: '*'
                Action: 's3:*'
                Effect: Deny
                Resource:
                  - !Sub '${MyBucket.Arn}'
                  - !Sub '${MyBucket.Arn}/*'
                Condition:
                  somethingelse: true
  expectations:
    rules:
      ensure_buckets_have_tls_enabled: FAIL

Sample run of test

$ ~/.guard/bin/cfn-guard test -r rule -t rule-tests.yaml 
PASS Expected Rule = ensure_buckets_have_tls_enabled, Status = FAIL, Got Status = FAIL
PASS Expected Rule = ensure_buckets_have_tls_enabled, Status = PASS, Got Status = PASS
PASS Expected Rule = ensure_buckets_have_tls_enabled, Status = FAIL, Got Status = FAIL
PASS Expected Rule = ensure_buckets_have_tls_enabled, Status = FAIL, Got Status = FAIL

dchakrav-github avatar Jul 29 '21 17:07 dchakrav-github

@robertjan-b does the above address your need? I will raise a separate issue to track the variable capture need.

dchakrav-github avatar Aug 04 '21 21:08 dchakrav-github

The first part has an issue. If one of the two checks in a rule does not have a match, it will return a SKIP. It will then do the second check and if that one returns a PASS, the whole rule gets a PASS. In the above example the Reference to the bucket must existing the resource name:

  input:
    Resources:
      MyBucket: #should be the resource name
        Type: AWS::S3::Bucket
      policy:
        Type: 'AWS::S3::BucketPolicy'
        Properties:
          Bucket: { Ref: MyBucket }

This returns a SKIP if there is no match:

    when %buckets not empty {
        %buckets.Type == 'AWS::S3::Bucket'
    }

How do I make sure to get a FAIL if there is no match @dchakrav-github ?

The feature that is being worked on would solve my issue completely. If the above works then I'm sure that each bucket policy has a proper statement and is connected to a bucket, but I need to make sure that all buckets have an associated bucket policy.

robertjan-b avatar Aug 17 '21 15:08 robertjan-b

@robertjan-b you are right without the feature you are not completely unblocked. Wanted to demonstrate a working rule and test case, hence the delay. This is based on an internal build targeting a new release of the tool that addresses this.

For your use case here is the rule example

let s3_buckets = Resources[ bucket_name | Type == 'AWS::S3::Bucket' ]
let s3_bucket_policies = Resources[ Type == 'AWS::S3::BucketPolicy' ]

rule ensure_all_s3_buckets_have_policy_associated when %s3_buckets not empty {
    #
    # Select all buckets policies that have a Ref. The template might contain 
    # policies against an S3 ARN as well, we skip selecting those, hence the 
    # some, at-least-one-or-more
    # 
    let s3_bucket_refs = some %s3_bucket_policies.Properties.Bucket.Ref

    #
    # If there are S3 buckets present, then BucketPolicies MUST be present
    # in the same stack 
    #
    %s3_bucket_refs not empty 
        <<Bucket policies must be defined within the same stack template>>

    # 
    # Equality works in this case as there is one-to-one correspondence
    #
    %bucket_name == %s3_bucket_refs 
        <<ALL S3 buckets do not have a reference to a policy>>
}

the test cases for this

---
- name: s3 bucket and policy reference success
  input:
    Resources:
      MyBucket: #should be the resource name
        Type: AWS::S3::Bucket
      policy:
        Type: 'AWS::S3::BucketPolicy'
        Properties:
          Bucket: { Ref: MyBucket }
  expectations:
    rules:
      ensure_all_s3_buckets_have_policy_associated: PASS

- name: s3 bucket no policies defined, FAIL
  input:
    Resources:
      MyBucket: #should be the resource name
        Type: AWS::S3::Bucket
  expectations:
    rules:
      ensure_all_s3_buckets_have_policy_associated: FAIL

- name: s3 bucket and policy reference, one without reference, FAIL
  input:
    Resources:
      MyBucket2: #should be the resource name
        Type: AWS::S3::Bucket
      MyBucket: #should be the resource name
        Type: AWS::S3::Bucket
      policy:
        Type: 'AWS::S3::BucketPolicy'
        Properties:
          Bucket: { Ref: MyBucket }
  expectations:
    rules:
      ensure_all_s3_buckets_have_policy_associated: FAIL

- name: s3 bucket and policy reference, policy with hard reference, PASS
  input:
    Resources:
      MyBucket: #should be the resource name
        Type: AWS::S3::Bucket
      policy:
        Type: 'AWS::S3::BucketPolicy'
        Properties:
          Bucket: { Ref: MyBucket }
      policyWithHardRef:
        Type: 'AWS::S3::BucketPolicy'
        Properties:
            Bucket: 'aws:arn:s3:'
  expectations:
    rules:
      ensure_all_s3_buckets_have_policy_associated: PASS

Sample run

./target/debug/cfn-guard test -n -r ~/demo/samples/s3-policy/rule.guard -t ~/demo/samples/s3-policy/tests.yaml 
Test Case #1
Name: "s3 bucket and policy reference success"
  PASS Rules:
    ensure_all_s3_buckets_have_policy_associated: Expected = PASS, Evaluated = PASS

Test Case #2
Name: "s3 bucket no policies defined, FAIL"
  PASS Rules:
    ensure_all_s3_buckets_have_policy_associated: Expected = FAIL, Evaluated = FAIL

Test Case #3
Name: "s3 bucket and policy reference, one without reference, FAIL"
  PASS Rules:
    ensure_all_s3_buckets_have_policy_associated: Expected = FAIL, Evaluated = FAIL

Test Case #4
Name: "s3 bucket and policy reference, policy with hard reference, PASS"
  PASS Rules:
    ensure_all_s3_buckets_have_policy_associated: Expected = PASS, Evaluated = PASS


dchakrav-github avatar Sep 02 '21 23:09 dchakrav-github

thanks @dchakrav-github , then I'm looking forward to the next release.

robertjan-b avatar Sep 03 '21 12:09 robertjan-b

Any updates to this feature ?

cjoyv avatar Feb 23 '22 04:02 cjoyv

Looking forward for this feature too.

jozefcunderlik avatar Jul 11 '22 08:07 jozefcunderlik

Before you read any further: this only works for CloudFormation templates generated by CDK:

# get the logical ids of the Buckets refereced in BucketPolicies
let local_bucket_ids = some Resources.*[ Type == /S3::BucketPolicy/ ].Properties.Bucket.Ref
# get the cdk-path of the Buckets found
let bucket_with_policy_paths = Resources.%local_bucket_ids.Metadata."aws:cdk:path"
# all buckets
let local_buckets = Resources.*[ Type == /S3::Bucket$/ ]

rule bucket_has_policy when %local_buckets !empty {
    %local_buckets {
        Metadata."aws:cdk:path" in %bucket_with_policy_paths
    }
}

In this rule I use the fact that CDK emits cdk-paths of the Metadata of each resource.

Jacco avatar Jan 27 '23 23:01 Jacco

Hi @robertjan-b I am going to go ahead and close this issue since the latest release has the code required to make @dchakrav-github latest example work.

Thanks

joshfried-aws avatar Jun 22 '23 12:06 joshfried-aws