Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(query): add missing name check in S3Bucket for AWS CloudFormation #5642

Merged
merged 11 commits into from
Aug 1, 2022
10 changes: 10 additions & 0 deletions assets/libraries/cloudformation.rego
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,16 @@ getBucketName(resource) = name {
name := resource.Properties.Bucket.Ref
}

has_bucket_associated(resources, bucketPolicyRef){
resource := resources[_]
resource.Type == "AWS::S3::Bucket"
resource.Properties.BucketName == getBucketName(bucketPolicyRef)
} else {
resource := resources[name]
resource.Type == "AWS::S3::Bucket"
name == getBucketName(bucketPolicyRef)
}

get_encryption(resource) = encryption {
resource.Properties.Encrypted == true
encryption := "encrypted"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ CxPolicy[result] {
policyStatements := [policyStatement |
resourcePolicy := input.document[indexBucket].Resources[_]
resourcePolicy.Type == "AWS::S3::BucketPolicy"
check_ref(resourcePolicy.Properties.Bucket, nameBucket)
check_ref(resourcePolicy.Properties.Bucket, resourceBucket, nameBucket)
policy := resourcePolicy.Properties.PolicyDocument
st := common_lib.get_statement(common_lib.get_policy(policy))
policyStatement := st[_]
Expand All @@ -36,8 +36,10 @@ CxPolicy[result] {
}
}

check_ref(obj, name) {
obj.Ref == name
check_ref(obj, bucketResource , logicName) {
obj.Ref == logicName
} else {
obj == name
obj == logicName
} else {
obj == bucketResource.Properties.BucketName
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ CxPolicy[result] {
st := common_lib.get_statement(common_lib.get_policy(policy))
statement := st[_]
common_lib.is_allow_effect(statement)
common_lib.equalsOrInArray(statement.Resource, "*")
common_lib.equalsOrInArray(statement.Principal, "*")
cf_lib.checkAction(statement.Action, "delete")

result := {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ CxPolicy[result] {
statement := st[_]

common_lib.is_allow_effect(statement)
common_lib.equalsOrInArray(statement.Resource, "*")
common_lib.equalsOrInArray(statement.Principal, "*")
cf_lib.checkAction(statement.Action, "get")

result := {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,7 @@
"PolicyDocument": {
"Statement": [
{
"Action": [
"DeleteObject",
"GetObject"
],
"Action": ["DeleteObject", "GetObject"],
"Effect": "Allow",
"Resource": "*",
"Principal": "*",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ CxPolicy[result] {
st := common_lib.get_statement(common_lib.get_policy(policy))
statement := st[_]
common_lib.is_allow_effect(statement)
common_lib.equalsOrInArray(statement.Resource, "*")
common_lib.equalsOrInArray(statement.Principal, "*")
cf_lib.checkAction(statement.Action, "list")

result := {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ CxPolicy[result] {
st := common_lib.get_statement(common_lib.get_policy(policy))
statement := st[_]
common_lib.is_allow_effect(statement)
common_lib.equalsOrInArray(statement.Resource, "*")
common_lib.equalsOrInArray(statement.Principal, "*")
cf_lib.checkAction(statement.Action, "put")

result := {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ CxPolicy[result] {
st := common_lib.get_statement(common_lib.get_policy(policy))
statement := st[_]
common_lib.is_allow_effect(statement)
common_lib.equalsOrInArray(statement.Resource, "*")
common_lib.equalsOrInArray(statement.Principal, "*")
cf_lib.checkAction(statement.Action, "restore")

result := {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,27 +1,25 @@
package Cx

import data.generic.common as common_lib
import data.generic.cloudformation as cf_lib
import data.generic.common as common_lib

CxPolicy[result] {
document := input.document[i]
resources := document.Resources

resources[resourceName].Type == "AWS::S3::Bucket"

bucketName := resourceName

not bucketHasPolicy(bucketName, resources)
not bucketHasPolicy(resources[resourceName], resourceName, resources)

result := {
"documentId": document.id,
"resourceType": resources[resourceName].Type,
"resourceName": cf_lib.get_resource_name(resources[resourceName], resourceName),
"searchKey": sprintf("Resources.%s", [bucketName]),
"searchKey": sprintf("Resources.%s", [resourceName]),
"issueType": "MissingAttribute",
"keyExpectedValue": sprintf("Resources.%s bucket has a policy that enforces SSL", [bucketName]),
"keyActualValue": sprintf("Resources.%s bucket doesn't have a policy", [bucketName]),
"searchLine": common_lib.build_search_line(["Resources", bucketName], []),
"keyExpectedValue": sprintf("Resources.%s bucket has a policy that enforces SSL", [resourceName]),
"keyActualValue": sprintf("Resources.%s bucket doesn't have a policy", [resourceName]),
"searchLine": common_lib.build_search_line(["Resources", resourceName], []),
}
}

Expand All @@ -31,33 +29,39 @@ CxPolicy[result] {

resources[resourceName].Type == "AWS::S3::Bucket"

bucketName := resourceName

bucketHasPolicy(bucketName, resources)
not bucketHasPolicyWithValidSslVerification(bucketName, resources)
bucketHasPolicy(resources[resourceName], resourceName, resources)
not bucketHasPolicyWithValidSslVerification(resources[resourceName], resourceName, resources)

result := {
"documentId": document.id,
"resourceType": resources[resourceName].Type,
"resourceName": cf_lib.get_resource_name(resources[resourceName], resourceName),
"searchKey": sprintf("Resources.%s", [bucketName]),
"searchKey": sprintf("Resources.%s", [resourceName]),
"issueType": "IncorrectValue",
"keyExpectedValue": sprintf("Resources.%s bucket has a policy that enforces SSL", [bucketName]),
"keyActualValue": sprintf("Resources.%s bucket doesn't have a policy or has a policy that doesn't enforce SSL", [bucketName]),
"searchLine": common_lib.build_search_line(["Resources", bucketName], []),
"keyExpectedValue": sprintf("Resources.%s bucket has a policy that enforces SSL", [resourceName]),
"keyActualValue": sprintf("Resources.%s bucket doesn't have a policy or has a policy that doesn't enforce SSL", [resourceName]),
"searchLine": common_lib.build_search_line(["Resources", resourceName], []),
}
}

bucketHasPolicy(bucketName, resources) {
resources[_].Type == "AWS::S3::BucketPolicy"
cf_lib.getBucketName(resources[_]) == bucketName
bucketHasPolicy(bucket, bucketLogicalName, resources) {
resources[a].Type == "AWS::S3::BucketPolicy"
cf_lib.getBucketName(resources[a]) == bucket.Properties.BucketName
} else {
resources[a].Type == "AWS::S3::BucketPolicy"
cf_lib.getBucketName(resources[a]) == bucketLogicalName
}

bucketHasPolicyWithValidSslVerification(bucketName, resources) {
resources[_].Type == "AWS::S3::BucketPolicy"
cf_lib.getBucketName(resources[_]) == bucketName
bucketHasPolicyWithValidSslVerification(bucket, bucketLogicalName, resources) {
resources[a].Type == "AWS::S3::BucketPolicy"
cf_lib.getBucketName(resources[a]) == bucket.Properties.BucketName

isValidSslPolicyStatement(resources[a].Properties.PolicyDocument.Statement)
} else {
resources[a].Type == "AWS::S3::BucketPolicy"
cf_lib.getBucketName(resources[a]) == bucketLogicalName

isValidSslPolicyStatement(resources[_].Properties.PolicyDocument.Statement)
isValidSslPolicyStatement(resources[a].Properties.PolicyDocument.Statement)
}

isUnsafeAction("s3:*") = true
Expand All @@ -70,13 +74,13 @@ equalsFalse(false) = true

isValidSslPolicyStatement(stmt) {
is_array(stmt)
st := stmt[s]
st := stmt[s]
st.Effect == "Deny"
isUnsafeAction(st.Action)
equalsFalse(st.Condition.Bool["aws:SecureTransport"])
} else {
is_object(stmt)
stmt.Effect == "Deny"
stmt.Effect == "Deny"
isUnsafeAction(stmt.Action)
equalsFalse(stmt.Condition.Bool["aws:SecureTransport"])
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ Resources:
S3Bucket:
Type: AWS::S3::Bucket
Properties:
BucketName: S3Bucket
AccessControl: PublicRead
WebsiteConfiguration:
IndexDocument: index.html
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ Resources:
S3Bucket2:
Type: AWS::S3::Bucket
Properties:
BucketName: S3Bucket2
AccessControl: PublicRead
WebsiteConfiguration:
IndexDocument: index.html
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"S3Bucket": {
"Type": "AWS::S3::Bucket",
"Properties": {
"BucketName": "S3Bucket",
"AccessControl": "PublicRead",
"WebsiteConfiguration": {
"IndexDocument": "index.html",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ Resources:
S3Bucket33:
Type: AWS::S3::Bucket
Properties:
BucketName: S3Bucket33
AccessControl: PublicRead
WebsiteConfiguration:
IndexDocument: index.html
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"S3Bucket2": {
"Type": "AWS::S3::Bucket",
"Properties": {
"BucketName": "S3Bucket2",
"AccessControl": "PublicRead",
"WebsiteConfiguration": {
"IndexDocument": "index.html",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
"S3Bucket88": {
"DeletionPolicy": "Retain",
"Properties": {
"BucketName": "S3Bucket88",
"AccessControl": "PublicRead",
"WebsiteConfiguration": {
"ErrorDocument": "error.html",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ Resources:
S3Bucket88:
Type: AWS::S3::Bucket
Properties:
BucketName: S3Bucket88
AccessControl: PublicRead
WebsiteConfiguration:
IndexDocument: index.html
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ Resources:
S3Bucket:
Type: AWS::S3::Bucket
Properties:
BucketName: S3Bucket
AccessControl: PublicRead
WebsiteConfiguration:
IndexDocument: index.html
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
"S3Bucket33": {
"DeletionPolicy": "Retain",
"Properties": {
"BucketName": "S3Bucket33",
"AccessControl": "PublicRead",
"WebsiteConfiguration": {
"ErrorDocument": "error.html",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ Resources:
S3Bucket2:
Type: AWS::S3::Bucket
Properties:
BucketName: S3Bucket2
AccessControl: PublicRead
WebsiteConfiguration:
IndexDocument: index.html
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ Resources:
S3Bucket3:
Type: AWS::S3::Bucket
Properties:
BucketName: S3Bucket3
AccessControl: PublicRead
WebsiteConfiguration:
IndexDocument: index.html
Expand All @@ -11,6 +12,7 @@ Resources:
S3Bucket4:
Type: AWS::S3::Bucket
Properties:
BucketName: S3Bucket4
AccessControl: PublicRead
WebsiteConfiguration:
IndexDocument: index.html
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,16 @@ Resources:
S3Bucket5:
Type: AWS::S3::Bucket
Properties:
BucketName: S3Bucket5
AccessControl: PublicRead
WebsiteConfiguration:
IndexDocument: index.html
ErrorDocument: error.html
DeletionPolicy: Retain
S3Bucket6:
S3Bucket6:
Type: AWS::S3::Bucket
Properties:
BucketName: S3Bucket6
AccessControl: PublicRead
WebsiteConfiguration:
IndexDocument: index.html
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
"S3Bucket": {
"Type": "AWS::S3::Bucket",
"Properties": {
"BucketName": "S3Bucket",
"AccessControl": "PublicRead",
"WebsiteConfiguration": {
"IndexDocument": "index.html",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
"Resources": {
"S3Bucket2": {
"Properties": {
"BucketName": "S3Bucket2",
"AccessControl": "PublicRead",
"WebsiteConfiguration": {
"IndexDocument": "index.html",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
"S3Bucket3": {
"Type": "AWS::S3::Bucket",
"Properties": {
"BucketName": "S3Bucket3",
"AccessControl": "PublicRead",
"WebsiteConfiguration": {
"IndexDocument": "index.html",
Expand All @@ -46,6 +47,7 @@
"S3Bucket4": {
"Type": "AWS::S3::Bucket",
"Properties": {
"BucketName": "S3Bucket4",
"AccessControl": "PublicRead",
"WebsiteConfiguration": {
"IndexDocument": "index.html",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
},
"S3Bucket6": {
"Properties": {
"BucketName": "S3Bucket6",
"AccessControl": "PublicRead",
"WebsiteConfiguration": {
"ErrorDocument": "error.html",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ Resources:
S3Bucket33:
Type: AWS::S3::Bucket
Properties:
BucketName: S3Bucket33,
AccessControl: PublicRead
WebsiteConfiguration:
IndexDocument: index.html
Expand All @@ -24,6 +25,6 @@ Resources:
Resource: !Join
- ''
- - 'arn:aws:s3:::'
- !Ref S3Bucket3
- !Ref S3Bucket33
- /*
Bucket: !Ref S3Bucket33
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
{
"queryName": "S3 Bucket Without SSL In Write Actions",
"severity": "HIGH",
"line": 11,
"line": 12,
"fileName": "positive3.yaml"
},
{
Expand All @@ -26,7 +26,7 @@
{
"queryName": "S3 Bucket Without SSL In Write Actions",
"severity": "HIGH",
"line": 11,
"line": 12,
"fileName": "positive4.yaml"
},
{
Expand All @@ -44,7 +44,7 @@
{
"queryName": "S3 Bucket Without SSL In Write Actions",
"severity": "HIGH",
"line": 46,
"line": 47,
"fileName": "positive7.json"
},
{
Expand Down