From 2068900a0e3775a961de820094dc9446c9ae5ba3 Mon Sep 17 00:00:00 2001 From: Priya Padmanaban <58094236+priyap286@users.noreply.github.com> Date: Tue, 11 May 2021 10:27:55 -0700 Subject: [PATCH] Revert "Filter as skips (#38)" (#39) This commit introduced an unwanted behavior with the some keyword while fixing for [*] clauses to fail when empty. This reverts commit 611de1f46e631d80c4268eddd4c27709d8edd74c. --- cfn-guard/src/commands/test.rs | 9 +- cfn-guard/src/commands/validate.rs | 6 +- cfn-guard/src/rules/evaluate.rs | 45 +- cfn-guard/src/rules/evaluate_tests.rs | 643 +++++++----------------- cfn-guard/src/rules/exprs.rs | 7 +- cfn-guard/src/rules/mod.rs | 2 - cfn-guard/src/rules/parser.rs | 36 +- cfn-guard/src/rules/parser_tests.rs | 15 +- cfn-guard/src/rules/path_value.rs | 30 +- cfn-guard/src/rules/path_value_tests.rs | 8 +- cfn-guard/tests/functional.rs | 2 +- guard2.0/all-tag-tests.yaml | 6 +- guard2.0/all-tags.guard | 6 +- guard2.0/api_gw_checks.guard | 24 +- guard2.0/api_gw_checks_tests.yaml | 2 +- guard2.0/ddb-gs-2-checks.guard | 51 +- guard2.0/ddb-gs-2-tests.yaml | 23 - 17 files changed, 289 insertions(+), 626 deletions(-) diff --git a/cfn-guard/src/commands/test.rs b/cfn-guard/src/commands/test.rs index f0b3ffe3c..1ec2f4882 100644 --- a/cfn-guard/src/commands/test.rs +++ b/cfn-guard/src/commands/test.rs @@ -94,7 +94,14 @@ or failure testing. match crate::rules::parser::rules_file(span) { Err(e) => println!("Parse Error on ruleset file {}", e), Ok(rules) => { - exit_code = test_with_data(&data_test_files, &rules, verbose)? + match test_with_data(&data_test_files, &rules, verbose) { + Ok(code) => { + exit_code = code; + }, + Err(_) => { + exit_code = 5; + } + } } } } diff --git a/cfn-guard/src/commands/validate.rs b/cfn-guard/src/commands/validate.rs index 6b65055a8..2995d294a 100644 --- a/cfn-guard/src/commands/validate.rs +++ b/cfn-guard/src/commands/validate.rs @@ -230,12 +230,8 @@ fn find_all_failing_clauses(context: &StatusContext) -> Vec<&StatusContext> { for each in &context.children { if each.status.map_or(false, |s| s == Status::FAIL) { match each.eval_type { - EvaluationType::Clause | - EvaluationType::BlockClause => { + EvaluationType::Clause => { failed.push(each); - if each.eval_type == EvaluationType::BlockClause { - failed.extend(find_all_failing_clauses(each)); - } }, EvaluationType::Filter | diff --git a/cfn-guard/src/rules/evaluate.rs b/cfn-guard/src/rules/evaluate.rs index 35f0a2c73..42ebc8175 100644 --- a/cfn-guard/src/rules/evaluate.rs +++ b/cfn-guard/src/rules/evaluate.rs @@ -138,7 +138,7 @@ fn compare(lhs: &Vec<&PathAwareValue>, where F: Fn(&PathAwareValue, &PathAwareValue) -> Result { if lhs.is_empty() || rhs.is_empty() { - return Ok((Status::SKIP, None, None)) + return Ok((Status::FAIL, None, None)) } let lhs_elem = lhs[0]; @@ -243,7 +243,7 @@ impl<'loc> Evaluate for GuardAccessClause<'loc> { let (lhs, retrieve_error) = - match resolve_query(true, + match resolve_query(clause.access_clause.query.match_all, &clause.access_clause.query.query, context, var_resolver) @@ -364,10 +364,7 @@ impl<'loc> Evaluate for GuardAccessClause<'loc> { Some(l[0].clone()) } else { None } } - ); - if r == Status::FAIL { - auto_reporter.message(message.to_string()); - } + ).message(message.to_string()); return Ok(r) } @@ -378,11 +375,9 @@ impl<'loc> Evaluate for GuardAccessClause<'loc> { .message(retrieve_error.map_or("".to_string(), |e| e)).get_status()) } else { - let status = if retrieve_error.is_none() { Status::SKIP } else { Status::FAIL }; - return Ok(auto_reporter.status(status) + return Ok(auto_reporter.status(Status::FAIL) .message(retrieve_error.map_or("".to_string(), |e| e)).get_status()) - }, - + } Some(l) => l, }; @@ -411,7 +406,7 @@ impl<'loc> Evaluate for GuardAccessClause<'loc> { let (rhs_resolved, rhs_query) = if let Some(expr) = &clause.access_clause.compare_with { match expr { LetValue::AccessClause(query) => - (Some(resolve_query(true, &query.query, context, var_resolver)?), Some(query.query.as_slice())), + (Some(resolve_query(query.match_all, &query.query, context, var_resolver)?), Some(query.query.as_slice())), _ => (None, None) } } else { @@ -526,10 +521,7 @@ impl<'loc> Evaluate for GuardAccessClause<'loc> { Some(msg) => msg, None => "(DEFAULT: NO_MESSAGE)" }; - auto_reporter.comparison(result, from, to); - if result == Status::FAIL { - auto_reporter.message(message.to_string()); - } + auto_reporter.comparison(result, from, to).message(message.to_string()); Ok(result) } } @@ -648,28 +640,27 @@ impl<'loc, T: Evaluate + 'loc> Evaluate for Conjunctions { impl<'loc> Evaluate for BlockGuardClause<'loc> { fn evaluate<'s>(&self, context: &'s PathAwareValue, var_resolver: &'s dyn EvaluationContext) -> Result { - let blk_context = format!("Block[{}]", self.location); - let mut report = AutoReport::new( - EvaluationType::BlockClause, - var_resolver, - &blk_context - ); let all = self.query.match_all; - let block_values = match resolve_query(true, &self.query.query, context, var_resolver) { + let block_values = match resolve_query(all, &self.query.query, context, var_resolver) { Err(Error(ErrorKind::RetrievalError(e))) | Err(Error(ErrorKind::IncompatibleRetrievalError(e))) => { + let context = format!("Block[{}]", self.location); + let mut report = AutoReport::new( + EvaluationType::Clause, + var_resolver, + &context + ); return Ok(report.message(e).status(Status::FAIL).get_status()) }, - Ok(v) => if v.is_empty() { - return Ok(report.message(format!("Query {} returned no results", SliceDisplay(&self.query.query))).status( - if self.not_empty { Status::FAIL } else { Status::SKIP }).get_status()) + Ok(v) => if v.is_empty() { // one or more + return Ok(Status::FAIL) } else { v }, Err(e) => return Err(e) }; - Ok(report.status(loop { + Ok(loop { let mut num_fail = 0; let mut num_pass = 0; for each in block_values { @@ -690,7 +681,7 @@ impl<'loc> Evaluate for BlockGuardClause<'loc> { if num_fail > 0 { break Status::FAIL } break Status::SKIP } - }).get_status()) + }) } } diff --git a/cfn-guard/src/rules/evaluate_tests.rs b/cfn-guard/src/rules/evaluate_tests.rs index cecb669ae..af968e7f9 100644 --- a/cfn-guard/src/rules/evaluate_tests.rs +++ b/cfn-guard/src/rules/evaluate_tests.rs @@ -72,6 +72,7 @@ fn guard_access_clause_tests() -> Result<()> { Principal.Service == /^lambda/ ].Action == "sts:AssumeRole""# )?; let status = clause.evaluate(&root, &dummy)?; + println!("Status = {:?}", status); assert_eq!(Status::PASS, status); let clause = GuardClause::try_from( @@ -80,7 +81,7 @@ fn guard_access_clause_tests() -> Result<()> { Principal.Service == /^notexists/ ].Action == "sts:AssumeRole""# )?; match clause.evaluate(&root, &dummy) { - Ok(Status::SKIP) => {}, + Ok(Status::FAIL) => {}, _rest => assert!(false) } Ok(()) @@ -283,6 +284,7 @@ fn test_iam_statement_clauses() -> Result<()> { // let clause = "Condition.*[ KEYS == /aws:[sS]ource(Vpc|VPC|Vpce|VPCE)/ ]"; let parsed = GuardClause::try_from(clause)?; let status = parsed.evaluate(&value, &reporter)?; + println!("Status {:?}", status); assert_eq!(Status::PASS, status); let clause = r#"Statement[ Condition EXISTS @@ -290,12 +292,14 @@ fn test_iam_statement_clauses() -> Result<()> { "#; let parsed = GuardClause::try_from(clause)?; let status = parsed.evaluate(&value, &reporter)?; + println!("Status {:?}", status); assert_eq!(Status::PASS, status); let value = Value::try_from(SAMPLE)?; let value = PathAwareValue::try_from(value)?; let parsed = GuardClause::try_from(clause)?; let status = parsed.evaluate(&value, &reporter)?; + println!("Status {:?}", status); assert_eq!(Status::FAIL, status); Ok(()) @@ -358,7 +362,7 @@ rule check_rest_api_private { let dummy = DummyEval{}; let reporter = Reporter(&dummy); let status = rule.evaluate(&value, &reporter)?; - assert_eq!(status, Status::PASS); + println!("{}", status); Ok(()) } @@ -396,15 +400,24 @@ fn testing_iam_role_prov_serve() -> Result<()> { let iam_roles = Resources.*[ Type == "AWS::IAM::Role" ] let ecs_tasks = Resources.*[ Type == "AWS::ECS::TaskDefinition" ] -rule deny_permissions_boundary_iam_role { +rule deny_permissions_boundary_iam_role when %iam_roles !EMPTY { # atleast one Tags contains a Key "TestRole" - %iam_roles { - Properties { - Tags[ Key == "TestRole" ] NOT EMPTY - PermissionBoundary NOT EXISTS - } - } -}"###; + %iam_roles.Properties.Tags[ Key == "TestRole" ] NOT EMPTY + %iam_roles.Properties.PermissionBoundary !EXISTS +} + +rule deny_task_role_no_permission_boundary when %ecs_tasks !EMPTY { + let task_role = %ecs_tasks.Properties.TaskRoleArn + + when %task_role.'Fn::GetAtt' EXISTS { + let role_name = %task_role.'Fn::GetAtt'[0] + let iam_roles_by_name = Resources.*[ KEYS == %role_name ] + %iam_roles_by_name !EMPTY + iam_roles_by_name.Properties.Tags !EMPTY + } or + %task_role == /aws:arn/ # either a direct string or +} + "###; let rules_file = RulesFile::try_from(rules)?; let value = PathAwareValue::try_from(resources)?; @@ -413,7 +426,7 @@ rule deny_permissions_boundary_iam_role { let root_context = RootScope::new(&rules_file, &value); let reporter = Reporter(&root_context); let status = rules_file.evaluate(&value, &reporter)?; - assert_eq!(status, Status::FAIL); + println!("{}", status); Ok(()) } @@ -502,33 +515,35 @@ fn testing_sg_rules_pro_serve() -> Result<()> { } } } -}]"###; +}] + + "###; let rules = r###" let sgs = Resources.*[ Type == "AWS::EC2::SecurityGroup" ] -rule deny_egress { +rule deny_egress when %sgs NOT EMPTY { # Ensure that none of the security group contain a rule # that has Cidr Ip set to any - %sgs { - Properties.SecurityGroupEgress[ CidrIp == "0.0.0.0/0" or - CidrIpv6 == "::/0" ] EMPTY - } -}"###; + %sgs.Properties.SecurityGroupEgress[ CidrIp == "0.0.0.0/0" or + CidrIpv6 == "::/0" ] EMPTY +} + + "###; let rules_file = RulesFile::try_from(rules)?; + let values = PathAwareValue::try_from(sgs)?; let samples = match values { PathAwareValue::List((_p, v)) => v, _ => unreachable!() }; - let statues = [Status::FAIL, Status::FAIL, Status::PASS, Status::PASS]; for (index, each) in samples.iter().enumerate() { let root_context = RootScope::new(&rules_file, each); let reporter = Reporter(&root_context); let status = rules_file.evaluate(each, &reporter)?; - assert_eq!(status, statues[index]); + println!("{}", format!("Status {} = {}", index, status).underline()); } let sample = r#"{ "Resources": {} }"#; @@ -537,18 +552,19 @@ rule deny_egress { rule deny_egress { # Ensure that none of the security group contain a rule # that has Cidr Ip set to any - Resources.*[ Type == "AWS::EC2::SecurityGroup" ] { - Properties.SecurityGroupEgress[ CidrIp == "0.0.0.0/0" or - CidrIpv6 == "::/0" ] EMPTY - } -}"###; + Resources.*[ Type == "AWS::EC2::SecurityGroup" ] + .Properties.SecurityGroupEgress[ CidrIp == "0.0.0.0/0" or + CidrIpv6 == "::/0" ] EMPTY +} + "###; let dummy = DummyEval{}; let rule_parsed = Rule::try_from(rule)?; let status = rule_parsed.evaluate(&value, &dummy)?; - assert_eq!(status, Status::FAIL); + println!("Status {:?}", status); Ok(()) + } #[test] @@ -702,7 +718,9 @@ fn test_s3_bucket_pro_serv() -> Result<()> { } } } -}]"###; +}] + + "###; let parsed_values = match PathAwareValue::try_from(values)? { PathAwareValue::List((_, v)) => v, @@ -711,33 +729,158 @@ fn test_s3_bucket_pro_serv() -> Result<()> { let rule = r###" rule deny_s3_public_bucket { - Resources.*[ Type == 'AWS::S3::Bucket' ] { - Properties { - BlockPublicAcls == true - BlockPublicPolicy == true - IgnorePublicAcls == true - RestrictPublicBuckets == true - } + AWS::S3::Bucket { # this is just a short form notation for Resources.*[ Type == "AWS::S3::Bucket" ] + Properties.BlockPublicAcls NOT EXISTS or + Properties.BlockPublicPolicy NOT EXISTS or + Properties.IgnorePublicAcls NOT EXISTS or + Properties.RestrictPublicBuckets NOT EXISTS or + + Properties.BlockPublicAcls == false or + Properties.BlockPublicPolicy == false or + Properties.IgnorePublicAcls == false or + Properties.RestrictPublicBuckets == false } -}"###; +} + + "###; - let statues = [ - Status::PASS, - Status::FAIL,Status::FAIL,Status::FAIL,Status::FAIL,Status::FAIL, - Status::FAIL, Status::FAIL, - Status::FAIL,Status::FAIL,Status::FAIL, - ]; let s3_rule = Rule::try_from(rule)?; let dummy = DummyEval{}; let reported = Reporter(&dummy); for (idx, each) in parsed_values.iter().enumerate() { let status = s3_rule.evaluate(each, &reported)?; println!("Status#{} = {}", idx, status); - assert_eq!(status, statues[idx]); } Ok(()) } +#[test] +fn ecs_iam_role_relationship_assetions() -> Result<()> { + let _template = r###" + # deny_task_role_no_permission_boundary is expected to be false so negate it to pass test +{ "Resources": { + "CounterTaskDef1468734E": { + "Type": "AWS::ECS::TaskDefinition", + "Properties": { + "ContainerDefinitions": [ + { + "Environment": [ + { + "Name": "COUNTER_TABLE_NAME", + "Value": { + "Ref": "CounterTableFE2C0268" + } + } + ], + "Essential": true, + "Image": { + "Fn::Sub": "${AWS::AccountId}.dkr.ecr.${AWS::Region}.${AWS::URLSuffix}/cdk-hnb659fds-container-assets-${AWS::AccountId}-${AWS::Region}:9a4832ed07fabf889e6df624dc8a8170008880d8db629312f85dba129920e0b1" + }, + "LogConfiguration": { + "LogDriver": "awslogs", + "Options": { + "awslogs-group": { + "Ref": "CounterTaskDefwebLogGroup437F46A3" + }, + "awslogs-stream-prefix": "Counter", + "awslogs-region": { + "Ref": "AWS::Region" + } + } + }, + "Name": "web", + "PortMappings": [ + { + "ContainerPort": 8080, + "Protocol": "tcp" + } + ] + } + ], + "Cpu": "256", + "ExecutionRoleArn": { + "Fn::GetAtt": [ + "CounterTaskDefExecutionRole5959CB2D", + "Arn" + ] + }, + "Family": "fooCounterTaskDef49BA9021", + "Memory": "512", + "NetworkMode": "awsvpc", + "RequiresCompatibilities": [ + "FARGATE" + ], + "TaskRoleArn": { + "Fn::GetAtt": [ + "CounterTaskRole71EBC3F8", + "Arn" + ] + } + }, + "Metadata": { + "aws:cdk:path": "foo/Counter/TaskDef/Resource" + } + }, + "CounterTaskRole71EBC3F8": { + "Type": "AWS::IAM::Role", + "Properties": { + "AssumeRolePolicyDocument": { + "Statement": [ + { + "Action": "sts:AssumeRole", + "Effect": "Allow", + "Principal": { + "Service": "ecs-tasks.amazonaws.com" + } + } + ], + "Version": "2012-10-17" + }, + "Tags": [{"Key": "TestRole", "Value": ""}], + "PermissionBoundary": "arn:aws:iam...", + "Policies": [ + { + "PolicyDocument": { + "Statement": [ + { + "Action": [ + "dynamodb:BatchGet*", + "dynamodb:DescribeStream", + "dynamodb:DescribeTable", + "dynamodb:Get*", + "dynamodb:Query", + "dynamodb:Scan", + "dynamodb:BatchWrite*", + "dynamodb:CreateTable", + "dynamodb:Delete*", + "dynamodb:Update*", + "dynamodb:PutItem" + ], + "Effect": "Allow", + "Resource": { + "Fn::GetAtt": [ + "CounterTableFE2C0268", + "Arn" + ] + } + } + ], + "Version": "2012-10-17" + }, + "PolicyName": "DynamoDBTableRWAccess" + } + ] + }, + "Metadata": { + "aws:cdk:path": "foo/CounterTaskRole/Default/Resource" + } + } + } +} + "###; + Ok(()) +} + struct VariableResolver<'a, 'b>(&'a dyn EvaluationContext, HashMap>); impl<'a, 'b> EvaluationContext for VariableResolver<'a, 'b> { @@ -925,12 +1068,7 @@ rule deny_permissions_boundary_iam_role when %iam_roles !EMPTY { #[test] fn test_rules_with_some_clauses() -> Result<()> { - let query = r#"Resources.*[ Type == 'AWS::IAM::Role' ].Properties[ - Tags !empty - some Tags[*] { - Key == /[A-Za-z0-9]+Role/ - } - ]"#; + let query = r#"some Resources.*[ Type == 'AWS::IAM::Role' ].Properties.Tags[ Key == /[A-Za-z0-9]+Role/ ]"#; let resources = r#" { "Resources": { "CounterTaskDefExecutionRole5959CB2D": { @@ -1082,33 +1220,6 @@ fn test_support_for_atleast_one_match_clause() -> Result<()> { println!("Selected = {:?}", selected); assert_eq!(selected.len(), 1); - let resources_str = r#"{ - Resources: { - ddbSelected: { - Type: 'AWS::DynamoDB::Table', - Properties: { - Tags: [ - { - Key: "PROD", - Value: "ProdApp" - } - ] - } - }, - ddbNotSelected: { - Type: 'AWS::DynamoDB::Table', - Properties: { - Tags: [] - } - } - } - }"#; - let resources = PathAwareValue::try_from(resources_str)?; - let selection_query = AccessQuery::try_from(selection_str)?; - let selected = resources.select(selection_query.match_all, &selection_query.query, &dummy)?; - println!("Selected = {:?}", selected); - assert_eq!(selected.len(), 1); - Ok(()) } @@ -1372,6 +1483,7 @@ fn embedded_when_clause_redshift_use_case_test() -> Result<()> { # Find all Redshift subnet group resource and extract all subnet Ids that are referenced # let local_subnet_refs = Resources.*[ Type == /Redshift::ClusterSubnetGroup/ ].Properties.SubnetIds[ Ref exists ].Ref + rule redshift_is_not_internet_accessible when %local_subnet_refs !empty { # @@ -1685,7 +1797,7 @@ fn test_for_not_in() -> Result<()> { fn test_rule_with_range_test() -> Result<()> { let rule_str = r#"rule check_parameter_validity { InputParameter.TcpBlockedPorts[*] { - this in r[0, 65535] <<[NON_COMPLIANT] Parameter TcpBlockedPorts has invalid value.>> + _ in r[0, 65535] <<[NON_COMPLIANT] Parameter TcpBlockedPorts has invalid value.>> } }"#; @@ -1785,382 +1897,3 @@ fn test_inner_when_skipped() -> Result<()> { Ok(()) } - -#[test] -fn empty_all_values_fails() -> Result<()> { - let resources_str = r#"{ Resources: {} }"#; - let resources = PathAwareValue::try_from(resources_str)?; - - let all_tags = GuardClause::try_from("Resources.*.Properties.Tags !empty")?; - let dummy = DummyEval{}; - let status = all_tags.evaluate(&resources, &dummy)?; - assert_eq!(status, Status::FAIL); - - let all_tags = GuardClause::try_from("Resources.* { Properties.Tags !empty }")?; - let dummy = DummyEval{}; - let status = all_tags.evaluate(&resources, &dummy)?; - assert_eq!(status, Status::FAIL); - - let all_tags = GuardClause::try_from("Resources.* !empty { Properties.Tags !empty }")?; - let dummy = DummyEval{}; - let status = all_tags.evaluate(&resources, &dummy)?; - assert_eq!(status, Status::FAIL); - - // - // Block level failures - // - let block_clause = GuardClause::try_from(r#"some Properties.Tags[*] { - Key == /PROD$/ - Value == /^App/ - }"#)?; - let values_str= r#" - Properties: - Tags: [] - "#; - let values = PathAwareValue::try_from(serde_yaml::from_str::(values_str)?)?; - let status = block_clause.evaluate(&values, &dummy)?; - assert_eq!(status, Status::FAIL); - - let values_str= r#" - Properties: - NoTags: Check - "#; - let values = PathAwareValue::try_from(serde_yaml::from_str::(values_str)?)?; - let status = block_clause.evaluate(&values, &dummy)?; - assert_eq!(status, Status::FAIL); - - let values_str= r#" - NoProperties: - NoTags: Check - "#; - let values = PathAwareValue::try_from(serde_yaml::from_str::(values_str)?)?; - let status = block_clause.evaluate(&values, &dummy)?; - assert_eq!(status, Status::FAIL); - - let values_str= r#"{}"#; - let values = PathAwareValue::try_from(serde_yaml::from_str::(values_str)?)?; - let status = block_clause.evaluate(&values, &dummy)?; - assert_eq!(status, Status::FAIL); - - let values_str= r#" - Properties: - Tags: - - Key: Beta - Value: BetaAppNoMatch - - Key: NotPRODEnding - Value: AppEvenIfThisMatches - "#; - let values = PathAwareValue::try_from(serde_yaml::from_str::(values_str)?)?; - let status = block_clause.evaluate(&values, &dummy)?; - assert_eq!(status, Status::FAIL); - - let values_str= r#" - Properties: - Tags: - - Key: EndingPROD - Value: BetaAppNoMatch - - Key: NotPRODEnding - Value: AppEvenIfThisMatches - "#; - let values = PathAwareValue::try_from(serde_yaml::from_str::(values_str)?)?; - let status = block_clause.evaluate(&values, &dummy)?; - assert_eq!(status, Status::FAIL); - - let values_str= r#" - Properties: - Tags: - - Key: SomePROD - Value: AppThisWorks - "#; - let values = PathAwareValue::try_from(serde_yaml::from_str::(values_str)?)?; - let status = block_clause.evaluate(&values, &dummy)?; - assert_eq!(status, Status::PASS); - - - let values_str= r#" - Properties: - Tags: - - Key: Beta - Value: BetaAppNoMatch - - Key: SomePROD - Value: AppThisWorks - "#; - let values = PathAwareValue::try_from(serde_yaml::from_str::(values_str)?)?; - let status = block_clause.evaluate(&values, &dummy)?; - assert_eq!(status, Status::PASS); - - // - // Block level ALL cases - // - let block_clause = GuardClause::try_from(r#"Properties.Tags[*] { - Key == /PROD$/ - Value == /^App/ - }"#)?; - - let values_str= r#" - Properties: - Tags: - - Key: Beta - Value: BetaAppNoMatch - - Key: SomePROD - Value: AppThisWorks - "#; - let values = PathAwareValue::try_from(serde_yaml::from_str::(values_str)?)?; - let status = block_clause.evaluate(&values, &dummy)?; - assert_eq!(status, Status::FAIL); - - let values_str= r#" - Properties: - Tags: - - Key: SomePROD - Value: AppThisWorks - "#; - let values = PathAwareValue::try_from(serde_yaml::from_str::(values_str)?)?; - let status = block_clause.evaluate(&values, &dummy)?; - assert_eq!(status, Status::PASS); - - let values_str= r#" - Properties: - Tags: - - Key: SomePROD - Value: AppThisWorks - - Key: AnotherSomePROD - Value: AppAnotherThisWorks - "#; - let values = PathAwareValue::try_from(serde_yaml::from_str::(values_str)?)?; - let status = block_clause.evaluate(&values, &dummy)?; - assert_eq!(status, Status::PASS); - - Ok(()) -} - -#[test] -fn filter_return_empty_direct_clause_skip() -> Result<()> { - - let dummy = DummyEval{}; - let clause_str = r#"some Properties.Tags[*].Key == /PROD$/"#; - let clause = GuardClause::try_from(clause_str)?; - - let values_str = r#" - Properties: - Tags: [] - "#; - let values = PathAwareValue::try_from(serde_yaml::from_str::(values_str)?)?; - let status = clause.evaluate(&values, &dummy)?; - assert_eq!(status, Status::FAIL); - - let values_str= r#" - Properties: - NoTags: Check - "#; - let values = PathAwareValue::try_from(serde_yaml::from_str::(values_str)?)?; - let status = clause.evaluate(&values, &dummy)?; - assert_eq!(status, Status::FAIL); - - let values_str= r#" - NoProperties: - NoTags: Check - "#; - let values = PathAwareValue::try_from(serde_yaml::from_str::(values_str)?)?; - let status = clause.evaluate(&values, &dummy)?; - assert_eq!(status, Status::FAIL); - - let values_str= r#"{}"#; - let values = PathAwareValue::try_from(serde_yaml::from_str::(values_str)?)?; - let status = clause.evaluate(&values, &dummy)?; - assert_eq!(status, Status::FAIL); - - let values_str = r#" - Properties: - Tags: - - Key: EndPROD - Value: AppStart - "#; - let values = PathAwareValue::try_from(serde_yaml::from_str::(values_str)?)?; - let status = clause.evaluate(&values, &dummy)?; - assert_eq!(status, Status::PASS); - - let rule_str = r#"rule not_the_same_as_block { - # - # These are 2 independent clauses, this is not the same as a block - # clause, each clause is evalulate separately. Hence when the input is - # - # Properties: - # Tags: - # - Key: EndPROD - # Value: NoTAppStart - # - Key: NotPRODEnd - # Value: AppStart - # - # This will PASS due to some clause - # - some Properties.Tags[*].Key == /PROD$/ - some Properties.Tags[*].Value == /^App/ - } - "#; - let rule = Rule::try_from(rule_str)?; - - let values_str = r#" - Properties: - Tags: - - Key: EndPROD - Value: AppStart - "#; - let values = PathAwareValue::try_from(serde_yaml::from_str::(values_str)?)?; - let status = clause.evaluate(&values, &dummy)?; - assert_eq!(status, Status::PASS); - - let values_str = r#" - Properties: - Tags: - - Key: EndPROD - Value: NotAppStart - - Key: NotPRODEnd - Value: AppStart - "#; - let values = PathAwareValue::try_from(serde_yaml::from_str::(values_str)?)?; - let status = clause.evaluate(&values, &dummy)?; - assert_eq!(status, Status::PASS); - - Ok(()) -} - -#[test] -fn filter_based_single_clause() -> Result<()> { - let clause = GuardClause::try_from( - r#"Properties.Tags[ Key == /PROD$/ ].Value == /^App/"#)?; - let dummy = DummyEval{}; - - let values_str = r#" - Properties: - Tags: [] - "#; - let values = PathAwareValue::try_from( - serde_yaml::from_str::(values_str)? - )?; - let status = clause.evaluate(&values, &dummy)?; - assert_eq!(status, Status::SKIP); - - let values_str = r#" - Properties: - Tags: - - Key: EndPROD - Value: NotAppStart - - Key: NotPRODEnd - Value: AppStart - "#; - let values = PathAwareValue::try_from(serde_yaml::from_str::(values_str)?)?; - let status = clause.evaluate(&values, &dummy)?; - assert_eq!(status, Status::FAIL); - - let values_str = r#" - Properties: - Tags: - - Key: EndPROD - Value: AppStart - "#; - let values = PathAwareValue::try_from(serde_yaml::from_str::(values_str)?)?; - let status = clause.evaluate(&values, &dummy)?; - assert_eq!(status, Status::PASS); - - let values_str = r#" - Properties: - Tags: - - Key: EndPROD - Value: AppStart - - Key: NotPRODEnd - Value: AppStart - "#; - let values = PathAwareValue::try_from(serde_yaml::from_str::(values_str)?)?; - let status = clause.evaluate(&values, &dummy)?; - assert_eq!(status, Status::PASS); - - Ok(()) -} - -#[test] -fn cross_ref_test() -> Result<()> { - let query_str = r#"Resources.*[ - Type in [/IAM::Policy/, /IAM::ManagedPolicy/] - some Properties.PolicyDocument.Statement[*] { - some Action[*] == 'cloudwatch:CreateLogGroup' - Effect == 'Allow' - } -]"#; - let query = AccessQuery::try_from(query_str)?; - let resources_str = r###" - Resources: - ReadOnlyPolicy: - Type: 'AWS::IAM::Policy' - Properties: - PolicyDocument: - Statement: - - Action: - - 'cloudwatch:Describe*' - - 'cloudwatch:List*' - Effect: Deny - Resource: '*' - - Action: - - 'cloudwatch:Describe*' - - 'cloudwatch:CreateLogGroup' - Effect: Allow - Resource: '*' - Version: 2012-10-17 - Description: '' - Roles: - - Ref: EcsTaskInstanceRole - ManagedPolicyName: OperatorPolicy - EcsTaskInstanceRole: - Type: 'AWS::IAM::Role' - Properties: - AssumeRolePolicyDocument: - Statement: - - Action: 'sts:AssumeRole' - Effect: Allow - Principal: - Service: 'ecs-tasks.amazonaws.com' - Version: 2012-10-17 - Description: This Admin Operator role - RoleName: EcsTaskInstanceRole - Tags: - - Key: Team - Value: IAM - - Key: IsPipeline - Value: 'false' - - Key: RoleARN - Value: !Sub 'arn:aws:iam::${AWS::AccountId}:role/EcsTaskInstanceRole' - - Key: VPC - Value: BOM-EgressVPC - - "###; - let resources = PathAwareValue::try_from(serde_yaml::from_str::(resources_str)?)?; - let dummy = DummyEval{}; - let selected = resources.select(query.match_all, &query.query, &dummy)?; - println!("{:?}", selected); - assert_eq!(selected.len(), 1); - - let rule_str = r###"rule certain_actions_forbid_for_ecs_role { - let policies_with_create_log_group = Resources.*[ - Type in [/IAM::Policy/, /IAM::ManagedPolicy/] - some Properties.PolicyDocument.Statement[*] { - some Action[*] == 'cloudwatch:CreateLogGroup' - Effect == 'Allow' - } - ] - - let role_refs = some %policies_with_create_log_group.Properties.Roles[*].Ref - Resources.%role_refs { - Type == 'AWS::IAM::Role' - Properties.AssumeRolePolicyDocument.Statement[*] { - Principal[*] { - Service != /^ecs-tasks/ - } - } - } -}"###; - let rule = RulesFile::try_from(rule_str)?; - let root = RootScope::new(&rule, &resources); - let status = rule.evaluate(&resources, &root)?; - assert_eq!(status, Status::FAIL); - Ok(()) -} \ No newline at end of file diff --git a/cfn-guard/src/rules/exprs.rs b/cfn-guard/src/rules/exprs.rs index 9d921e08a..b2f5fe1ea 100644 --- a/cfn-guard/src/rules/exprs.rs +++ b/cfn-guard/src/rules/exprs.rs @@ -54,7 +54,7 @@ pub(crate) struct LetExpr<'loc> { /// #[derive(PartialEq, Debug, Clone, Serialize, Deserialize, Hash)] pub(crate) enum QueryPart<'loc> { - This, + It, Key(String), MapKeyFilter(MapKeyFilterClause<'loc>), AllValues, @@ -112,8 +112,8 @@ impl<'loc> std::fmt::Display for QueryPart<'loc> { f.write_str("(map-key-filter-clause)")?; }, - QueryPart::This => { - f.write_str("this")?; + QueryPart::It => { + f.write_str("_")?; } } Ok(()) @@ -163,7 +163,6 @@ pub(crate) struct GuardNamedRuleClause<'loc> { #[derive(PartialEq, Debug, Clone, Serialize, Deserialize, Hash)] pub(crate) struct BlockGuardClause<'loc> { pub(crate) query: AccessQuery<'loc>, - pub(crate) not_empty: bool, pub(crate) block: Block<'loc, GuardClause<'loc>>, pub(crate) location: FileLocation<'loc> } diff --git a/cfn-guard/src/rules/mod.rs b/cfn-guard/src/rules/mod.rs index 1672c678c..5cfff872e 100644 --- a/cfn-guard/src/rules/mod.rs +++ b/cfn-guard/src/rules/mod.rs @@ -59,7 +59,6 @@ pub(crate) enum EvaluationType { ConditionBlock, Filter, Conjunction, - BlockClause, Clause } @@ -73,7 +72,6 @@ impl std::fmt::Display for EvaluationType { EvaluationType::ConditionBlock => f.write_str("ConditionBlock")?, EvaluationType::Filter => f.write_str("Filter")?, EvaluationType::Conjunction => f.write_str("Conjunction")?, - EvaluationType::BlockClause => f.write_str("BlockClause")?, EvaluationType::Clause => f.write_str("Clause")?, } Ok(()) diff --git a/cfn-guard/src/rules/parser.rs b/cfn-guard/src/rules/parser.rs index 36b6ce04c..106cafa37 100644 --- a/cfn-guard/src/rules/parser.rs +++ b/cfn-guard/src/rules/parser.rs @@ -795,23 +795,14 @@ fn some_keyword(input: Span) -> IResult { ))(input) } -fn self_reference(input: Span) -> IResult { - preceded(zero_or_more_ws_or_comment, - alt(( - value(QueryPart::This, tag("this")), - value(QueryPart::This, tag("THIS")), - )) - )(input) -} - // // access = (var_name / var_name_access) [dotted_access] // pub(crate) fn access(input: Span) -> IResult { map(tuple(( opt(some_keyword), - alt(( - self_reference, + alt( + (value(QueryPart::It, preceded(zero_or_more_ws_or_comment, char('_'))), map( alt((var_name_access_inclusive, property_name)), |p| QueryPart::Key(p)))), opt(dotted_access))), |(any, first, remainder)| { @@ -918,16 +909,6 @@ fn clause_with(input: Span, access: A) -> IResult clause_with_map(input, access, |g| GuardClause::Clause(g)) } -fn not_empty(input: Span) -> IResult { - preceded(zero_or_more_ws_or_comment, - value((), tuple(( - preceded(space0, not), - preceded( space0,empty) - )) - ) - )(input) -} - pub(crate) fn block_clause(input: Span) -> IResult { let location = FileLocation { file_name: input.extra, @@ -936,23 +917,12 @@ pub(crate) fn block_clause(input: Span) -> IResult { }; let (input, query) = access(input)?; - let (input, not_empty) = match &query.query.last().unwrap() { - QueryPart::MapKeyFilter(_) | - QueryPart::Filter(_) | - QueryPart::AllIndices | - QueryPart::AllValues => { - opt(not_empty)(input)? - }, - - _ => (input, None) - }; let (input, (assignments, conjunctions)) = block(clause)(input)?; Ok((input, GuardClause::BlockClause(BlockGuardClause { query, block: Block { assignments, conjunctions, }, - location, - not_empty: not_empty.map_or(false, |_| true) + location }))) } diff --git a/cfn-guard/src/rules/parser_tests.rs b/cfn-guard/src/rules/parser_tests.rs index 60e741345..6c0c32a1f 100644 --- a/cfn-guard/src/rules/parser_tests.rs +++ b/cfn-guard/src/rules/parser_tests.rs @@ -3613,7 +3613,7 @@ fn parse_rule_block_with_mixed_assignment() -> Result<(), Error> { request.kind.kind == "Pod" request.operation == "CREATE" let service_name = request.object.spec.serviceAccountName - %allowlist[ this.serviceAccount == %service_name ] !EMPTY + %allowlist[ _.serviceAccount == %service_name ] !EMPTY }"###; let rule = Rule::try_from(r)?; println!("{:?}", rule); @@ -3672,7 +3672,7 @@ impl EvaluationContext for DummyEval { #[test] fn select_any_one_from_list_clauses() -> Result<(), Error> { - let clause = "this == /\\{\\{resolve:secretsmanager/"; + let clause = "_ == /\\{\\{resolve:secretsmanager/"; let parsed = super::clause(from_str2(clause))?.1; let expected = GuardClause::Clause( GuardAccessClause { @@ -3685,7 +3685,7 @@ fn select_any_one_from_list_clauses() -> Result<(), Error> { compare_with: Some(LetValue::Value(Value::Regex("\\{\\{resolve:secretsmanager".to_string()))), comparator: (CmpOperator::Eq, false), custom_message: None, - query: AccessQuery{ query: vec![QueryPart::This], match_all: true } + query: AccessQuery{ query: vec![QueryPart::It], match_all: true } }, negation: false } @@ -3732,7 +3732,7 @@ fn select_any_one_from_list_clauses() -> Result<(), Error> { let _dummy = DummyEval{}; let _clause = GuardClause::try_from( - r#"Resources.*[ this.Type == "AWS::RDS::DBInstance" ].Properties.MasterUserPassword.'Fn::Join'[1][ this == /\{\{resolve:secretsmanager/ ] !EMPTY"#)?; + r#"Resources.*[ _.Type == "AWS::RDS::DBInstance" ].Properties.MasterUserPassword.'Fn::Join'[1][ _ == /\{\{resolve:secretsmanager/ ] !EMPTY"#)?; Ok(()) } @@ -3911,7 +3911,7 @@ fn some_clause_parse() -> Result<(), Error> { #[test] fn it_support_test() -> Result<(), Error> { - let query = r#"Tags[ some this == { Key: "Hi", Value: "There" }]"#; + let query = r#"Tags[ some _ == { Key: "Hi", Value: "There" }]"#; let parsed_query = AccessQuery::try_from(query)?; println!("{:?}", parsed_query); let expected = AccessQuery { @@ -3928,7 +3928,7 @@ fn it_support_test() -> Result<(), Error> { query: AccessQuery { match_all: false, query: vec![ - QueryPart::This + QueryPart::It ] }, custom_message: None, @@ -3979,7 +3979,6 @@ fn test_block_properties()-> Result<(), Error> { ], match_all: true }, - not_empty: false, block: Block { assignments: vec![], conjunctions: vec![ @@ -4105,7 +4104,7 @@ fn when_inside_when_parse_test() -> Result<(), Error> { # Find all routes that have a gateways associated with the route table and extract # all their references # - let gws_ids = Resources.*[ + let gws_ids = some Resources.*[ Type == 'AWS::EC2::Route' Properties.GatewayId.Ref exists Properties.RouteTableId.Ref in %route_tables diff --git a/cfn-guard/src/rules/path_value.rs b/cfn-guard/src/rules/path_value.rs index 23619283c..8f11d26f7 100644 --- a/cfn-guard/src/rules/path_value.rs +++ b/cfn-guard/src/rules/path_value.rs @@ -291,9 +291,9 @@ impl QueryResolver for PathAwareValue { } match &query[0] { - QueryPart::This => { + QueryPart::It => { self.select(all, &query[1..], resolver) - } + }, QueryPart::Key(key) => { match key.parse::() { @@ -417,9 +417,6 @@ impl QueryResolver for PathAwareValue { }, PathAwareValue::Map((_path, map)) => { - if map.is_empty() { - return self.map_some_or_error_all(all, query); - } let values: Vec<&PathAwareValue> = map.values.values().collect(); let mut resolved = Vec::with_capacity(values.len()); for each in values { @@ -497,8 +494,18 @@ impl QueryResolver for PathAwareValue { for each in vec { let mut filter = AutoReport::new(EvaluationType::Filter, resolver, &context); match conjunctions.evaluate(each, resolver) { - Err(Error(ErrorKind::RetrievalError(_))) | - Err(Error(ErrorKind::IncompatibleRetrievalError(_))) => continue, + Err(Error(ErrorKind::RetrievalError(e))) => { + if all { + return Err(Error::new(ErrorKind::RetrievalError(e))) + } + // Else treat is like a filter + }, + Err(Error(ErrorKind::IncompatibleRetrievalError(e))) => { + if all { + return Err(Error::new(ErrorKind::IncompatibleRetrievalError(e))) + } + // Else treat is like a filter + }, Err(e) => return Err(e), Ok(status) => { match status { @@ -525,12 +532,7 @@ impl QueryResolver for PathAwareValue { let mut filter = AutoReport::new(EvaluationType::Filter, resolver, &context); conjunctions.evaluate(self, resolver) .map_or_else( - |e| match e { - Error(ErrorKind::RetrievalError(_)) | - Error(ErrorKind::IncompatibleRetrievalError(_)) => Ok(vec![]), - - rest => Err(rest) - }, + |e| self.map_error_or_empty(all, e), |status| { match status { Status::PASS => { @@ -652,7 +654,7 @@ impl PathAwareValue { } pub(crate) fn accumulate<'v>(all: bool, query: &[QueryPart<'_>], elements: &'v Vec, resolver: &dyn EvaluationContext) -> Result, Error>{ - if elements.is_empty() && all { + if elements.is_empty() && !query.is_empty() && all { return Err(Error::new(ErrorKind::RetrievalError( format!("Remaining Query {}, No elements in array", SliceDisplay(query)) ))); diff --git a/cfn-guard/src/rules/path_value_tests.rs b/cfn-guard/src/rules/path_value_tests.rs index 93c944dd9..daff6b5e8 100644 --- a/cfn-guard/src/rules/path_value_tests.rs +++ b/cfn-guard/src/rules/path_value_tests.rs @@ -218,7 +218,7 @@ fn path_value_queries() -> Result<(), Error> { assert_eq!(selected.len(), 2); let get_att_refs = - r#"Resources.*.Properties[ SecurityGroupIds exists ].SecurityGroupIds[ 'Fn::GetAtt' exists ].'Fn::GetAtt'.*"#; + r#"SOME Resources.*.Properties.SecurityGroupIds[*].'Fn::GetAtt'.*"#; let resources_with_sgs = AccessQuery::try_from(get_att_refs)?; let selected = incoming.select(resources_with_sgs.match_all, &resources_with_sgs.query, &eval)?; assert_eq!(selected.len(), 2); @@ -227,7 +227,7 @@ fn path_value_queries() -> Result<(), Error> { // // Assignments // - let assignment = r#"let var = Resources.*.Properties.SecurityGroupIds[ 'Fn::GetAtt' EXISTS ].'Fn::GetAtt'.*"#; + let assignment = r#"let var = ANY Resources.*.Properties.SecurityGroupIds[*].'Fn::GetAtt'.*"#; let let_statement = LetExpr::try_from(assignment)?; println!("{:?}", let_statement); @@ -273,7 +273,7 @@ fn path_value_queries() -> Result<(), Error> { #[test] fn some_filter_tests() -> Result<(), Error> { - let query_str = r#"Resources.*.Properties.SecurityGroups[ 'Fn::GetAtt' exists ].'Fn::GetAtt'"#; + let query_str = r#"some Resources.*.Properties.SecurityGroups[*].'Fn::GetAtt'"#; let resources_str = r#"{ Resources: { ec2: { @@ -299,7 +299,7 @@ fn some_filter_tests() -> Result<(), Error> { #[test] fn it_support_evaluation_tests() -> Result<(), Error> { - let tags = r#"Tags[ this == { Key: "Hi", Value: "There" } ]"#; + let tags = r#"Tags[ _ == { Key: "Hi", Value: "There" } ]"#; let parsed_tags = AccessQuery::try_from(tags)?; let values = r#"{ Tags: [ diff --git a/cfn-guard/tests/functional.rs b/cfn-guard/tests/functional.rs index a531ba6cb..494d9b441 100644 --- a/cfn-guard/tests/functional.rs +++ b/cfn-guard/tests/functional.rs @@ -60,7 +60,7 @@ mod tests { { "eval_type": "Clause", "context": "Clause(Location[file:, line:1, column:14], Check: Type EQUALS String(\"AWS::ApiGateway::Method\"))", - "msg": "DEFAULT MESSAGE(PASS)", + "msg": "(DEFAULT: NO_MESSAGE)", "from": null, "to": null, "status": "PASS", diff --git a/guard2.0/all-tag-tests.yaml b/guard2.0/all-tag-tests.yaml index f5403a41f..1af0b1bb3 100644 --- a/guard2.0/all-tag-tests.yaml +++ b/guard2.0/all-tag-tests.yaml @@ -1,13 +1,9 @@ --- -- input: {} - expectations: - rules: - assert_all_resources_have_non_empty_tags: FAIL - input: Resources: {} expectations: rules: - assert_all_resources_have_non_empty_tags: FAIL + assert_all_resources_have_non_empty_tags: SKIP - input: Resources: compliant: diff --git a/guard2.0/all-tags.guard b/guard2.0/all-tags.guard index baf775e43..123768dc6 100644 --- a/guard2.0/all-tags.guard +++ b/guard2.0/all-tags.guard @@ -38,8 +38,6 @@ let resources = Resources.*[ # 2) FAIL is any one resource did have empty tags or did not have tags specified at all # 3) PASS when ALL resource do have non-empty tags # -rule assert_all_resources_have_non_empty_tags { - %resources { - Properties.Tags !empty - } +rule assert_all_resources_have_non_empty_tags when %resources !empty { + %resources.Properties.Tags !empty } diff --git a/guard2.0/api_gw_checks.guard b/guard2.0/api_gw_checks.guard index 40ab44a55..3521bbbcc 100644 --- a/guard2.0/api_gw_checks.guard +++ b/guard2.0/api_gw_checks.guard @@ -29,20 +29,18 @@ let api_gws = Resources.*[ Type == 'AWS::ApiGateway::RestApi' ] # ALL ApiGateway instances MUST have one IAM Condition key with aws:sourceVpc or aws:SourceVpc # 3) FAIL otherwise # -rule check_rest_api_is_private_and_has_access { - %api_gws { - Properties { - # - # ALL ApiGateways must be PRIVATE, checking with EndpointConfiguration - # - EndpointConfiguration == ["PRIVATE"] +rule check_rest_api_is_private_and_has_access when %api_gws !empty { + %api_gws.Properties { + # + # ALL ApiGateways must be PRIVATE, checking with EndpointConfiguration + # + EndpointConfiguration == ["PRIVATE"] - # ALL ApiGateways must have atleast one IAM statement that has Condition keys with - # aws:sourceVpc - # - some Policy.Statement[*] { - Condition[ keys == /aws:[sS]ource(Vpc|VPC|Vpce|VPCE)/ ] !empty - } + # ALL ApiGateways must have atleast one IAM statement that has Condition keys with + # aws:sourceVpc + # + some Policy.Statement[*] { + Condition[ keys == /aws:[sS]ource(Vpc|VPC|Vpce|VPCE)/ ] !empty } } } diff --git a/guard2.0/api_gw_checks_tests.yaml b/guard2.0/api_gw_checks_tests.yaml index 53b27a9b7..f6e0239cb 100644 --- a/guard2.0/api_gw_checks_tests.yaml +++ b/guard2.0/api_gw_checks_tests.yaml @@ -3,7 +3,7 @@ Resources: {} expectations: rules: - check_rest_api_is_private_and_has_access: FAIL + check_rest_api_is_private_and_has_access: SKIP - input: Resources: apiGw: diff --git a/guard2.0/ddb-gs-2-checks.guard b/guard2.0/ddb-gs-2-checks.guard index e00def069..724044d8e 100644 --- a/guard2.0/ddb-gs-2-checks.guard +++ b/guard2.0/ddb-gs-2-checks.guard @@ -22,14 +22,13 @@ let ddb = Resources.*[ Type == 'AWS::DynamoDB::Table' ] # b) PASS when all DDB Tables do have encryption turned on # c) FAIL if wasn't set for them # -rule dynamo_db_sse_on when assert_all_resources_have_non_empty_tags +rule dynamo_db_sse_on when %ddb !empty + assert_all_resources_have_non_empty_tags { # # Enusre ALL DynamoDB Tables have encryption at rest turned on # - %ddb { - Properties.SSESpecification.SSEEnabled == true - } + %ddb.Properties.SSESpecification.SSEEnabled == true } # @@ -61,38 +60,38 @@ rule dynamo_db_sse_on_for_prod_only when dynamo_db_sse_on # Key containing /PROD/ and Value starting with /^App/ # let only_prod_ddb = %ddb[ - Properties { + # + # At least one Tag exists that contains Key and Value + # needed on PROD DynamoDB Table + # + some Properties.Tags[*] { # - # At least one Tag exists that contains Key and Value - # needed on PROD DynamoDB Table + # contains at-least-one key with PROD # - some Tags[*] { - # - # contains at-least-one key with PROD - # - Key == /PROD/ + Key == /PROD/ - # - # Value that starts with App - # - Value == /^App/ - } + # + # Value that starts with App + # + Value == /^App/ } ] # # Skip the enforcement if there were no such DDB Tables # - %only_prod_ddb { - # - # Only permit allowed ones (currently just KMS) - # - Properties.SSESpecification.SSEType == %allowed_algorithms + when %only_prod_ddb !empty { + %only_prod_ddb { + # + # Only permit allowed ones (currently just KMS) + # + Properties.SSESpecification.SSEType == %allowed_algorithms - # - # Prod DDB Table must have retain - # - DeletionPolicy == 'Retain' + # + # Prod DDB Table must have retain + # + DeletionPolicy == 'Retain' + } } } diff --git a/guard2.0/ddb-gs-2-tests.yaml b/guard2.0/ddb-gs-2-tests.yaml index 81e8af06f..8ab8cd9b5 100644 --- a/guard2.0/ddb-gs-2-tests.yaml +++ b/guard2.0/ddb-gs-2-tests.yaml @@ -116,26 +116,3 @@ assert_all_resources_have_non_empty_tags: FAIL dynamo_db_sse_on: SKIP dynamo_db_sse_on_for_prod_only: SKIP -- input: - Resources: - ddb: - Type: 'AWS::DynamoDB::Table' - Properties: - # No Tags, will fail - SSESpecification: - SSEEnabled: true - Tags: [] - ddb2: - Type: 'AWS::DynamoDB::Table' - Properties: - Tags: - - Key: PROD-DDB - Value: AppSelected - SSESpecification: - SSEEnabled: true - SSEType: KMS - expectations: - rules: - assert_all_resources_have_non_empty_tags: FAIL - dynamo_db_sse_on: SKIP - dynamo_db_sse_on_for_prod_only: SKIP