From ed038bf37c2e034ff2c7aa3941574d675da4f25a Mon Sep 17 00:00:00 2001 From: nathanmc Date: Wed, 17 Jun 2020 11:35:24 -0400 Subject: [PATCH 01/21] Remove unneeded Copyright date. --- cfn-guard-lambda/src/main.rs | 2 +- cfn-guard-rulegen/src/lib.rs | 2 +- cfn-guard-rulegen/src/main.rs | 2 +- cfn-guard/src/guard_types.rs | 2 +- cfn-guard/src/lib.rs | 2 +- cfn-guard/src/main.rs | 2 +- cfn-guard/src/parser.rs | 2 +- cfn-guard/src/util.rs | 2 +- cfn-guard/tests/functional.rs | 2 +- 9 files changed, 9 insertions(+), 9 deletions(-) diff --git a/cfn-guard-lambda/src/main.rs b/cfn-guard-lambda/src/main.rs index 9ff8581d3..42c88babd 100644 --- a/cfn-guard-lambda/src/main.rs +++ b/cfn-guard-lambda/src/main.rs @@ -1,4 +1,4 @@ -// © 2020 Amazon Web Services, Inc. or its affiliates. All Rights Reserved. This AWS Content is provided subject to the terms of the AWS Customer Agreement available at http://aws.amazon.com/agreement or other written agreement between Customer and either Amazon Web Services, Inc. or Amazon Web Services EMEA SARL or both. +// © Amazon Web Services, Inc. or its affiliates. All Rights Reserved. This AWS Content is provided subject to the terms of the AWS Customer Agreement available at http://aws.amazon.com/agreement or other written agreement between Customer and either Amazon Web Services, Inc. or Amazon Web Services EMEA SARL or both. use std::error::Error; diff --git a/cfn-guard-rulegen/src/lib.rs b/cfn-guard-rulegen/src/lib.rs index e34ae4a75..43d3f2da3 100644 --- a/cfn-guard-rulegen/src/lib.rs +++ b/cfn-guard-rulegen/src/lib.rs @@ -1,4 +1,4 @@ -// © 2020 Amazon Web Services, Inc. or its affiliates. All Rights Reserved. This AWS Content is provided subject to the terms of the AWS Customer Agreement available at http://aws.amazon.com/agreement or other written agreement between Customer and either Amazon Web Services, Inc. or Amazon Web Services EMEA SARL or both. +// © Amazon Web Services, Inc. or its affiliates. All Rights Reserved. This AWS Content is provided subject to the terms of the AWS Customer Agreement available at http://aws.amazon.com/agreement or other written agreement between Customer and either Amazon Web Services, Inc. or Amazon Web Services EMEA SARL or both. use log::{self, debug, info, trace}; use serde_json::Value; diff --git a/cfn-guard-rulegen/src/main.rs b/cfn-guard-rulegen/src/main.rs index 82f8ed4ac..b88f674b7 100644 --- a/cfn-guard-rulegen/src/main.rs +++ b/cfn-guard-rulegen/src/main.rs @@ -1,4 +1,4 @@ -// © 2020 Amazon Web Services, Inc. or its affiliates. All Rights Reserved. This AWS Content is provided subject to the terms of the AWS Customer Agreement available at http://aws.amazon.com/agreement or other written agreement between Customer and either Amazon Web Services, Inc. or Amazon Web Services EMEA SARL or both. +// © Amazon Web Services, Inc. or its affiliates. All Rights Reserved. This AWS Content is provided subject to the terms of the AWS Customer Agreement available at http://aws.amazon.com/agreement or other written agreement between Customer and either Amazon Web Services, Inc. or Amazon Web Services EMEA SARL or both. use std::process; #[macro_use] diff --git a/cfn-guard/src/guard_types.rs b/cfn-guard/src/guard_types.rs index fecf0ba3b..24a77f934 100644 --- a/cfn-guard/src/guard_types.rs +++ b/cfn-guard/src/guard_types.rs @@ -1,4 +1,4 @@ -// © 2020 Amazon Web Services, Inc. or its affiliates. All Rights Reserved. This AWS Content is provided subject to the terms of the AWS Customer Agreement available at http://aws.amazon.com/agreement or other written agreement between Customer and either Amazon Web Services, Inc. or Amazon Web Services EMEA SARL or both. +// © Amazon Web Services, Inc. or its affiliates. All Rights Reserved. This AWS Content is provided subject to the terms of the AWS Customer Agreement available at http://aws.amazon.com/agreement or other written agreement between Customer and either Amazon Web Services, Inc. or Amazon Web Services EMEA SARL or both. // Structs, Enums and Impls pub mod enums { diff --git a/cfn-guard/src/lib.rs b/cfn-guard/src/lib.rs index 4c1e0e793..9d8a2baf5 100644 --- a/cfn-guard/src/lib.rs +++ b/cfn-guard/src/lib.rs @@ -1,4 +1,4 @@ -// © 2020 Amazon Web Services, Inc. or its affiliates. All Rights Reserved. This AWS Content is provided subject to the terms of the AWS Customer Agreement available at http://aws.amazon.com/agreement or other written agreement between Customer and either Amazon Web Services, Inc. or Amazon Web Services EMEA SARL or both. +// © Amazon Web Services, Inc. or its affiliates. All Rights Reserved. This AWS Content is provided subject to the terms of the AWS Customer Agreement available at http://aws.amazon.com/agreement or other written agreement between Customer and either Amazon Web Services, Inc. or Amazon Web Services EMEA SARL or both. use log::{self, debug, error, info, trace}; use serde_json::Value; diff --git a/cfn-guard/src/main.rs b/cfn-guard/src/main.rs index e7d5bcea7..6a6039ef3 100644 --- a/cfn-guard/src/main.rs +++ b/cfn-guard/src/main.rs @@ -1,4 +1,4 @@ -// © 2020 Amazon Web Services, Inc. or its affiliates. All Rights Reserved. This AWS Content is provided subject to the terms of the AWS Customer Agreement available at http://aws.amazon.com/agreement or other written agreement between Customer and either Amazon Web Services, Inc. or Amazon Web Services EMEA SARL or both. +// © Amazon Web Services, Inc. or its affiliates. All Rights Reserved. This AWS Content is provided subject to the terms of the AWS Customer Agreement available at http://aws.amazon.com/agreement or other written agreement between Customer and either Amazon Web Services, Inc. or Amazon Web Services EMEA SARL or both. use std::process; #[macro_use] diff --git a/cfn-guard/src/parser.rs b/cfn-guard/src/parser.rs index 4eee2ee19..29fe50fe2 100644 --- a/cfn-guard/src/parser.rs +++ b/cfn-guard/src/parser.rs @@ -1,4 +1,4 @@ -// © 2020 Amazon Web Services, Inc. or its affiliates. All Rights Reserved. This AWS Content is provided subject to the terms of the AWS Customer Agreement available at http://aws.amazon.com/agreement or other written agreement between Customer and either Amazon Web Services, Inc. or Amazon Web Services EMEA SARL or both. +// © Amazon Web Services, Inc. or its affiliates. All Rights Reserved. This AWS Content is provided subject to the terms of the AWS Customer Agreement available at http://aws.amazon.com/agreement or other written agreement between Customer and either Amazon Web Services, Inc. or Amazon Web Services EMEA SARL or both. use std::collections::HashMap; use std::env; diff --git a/cfn-guard/src/util.rs b/cfn-guard/src/util.rs index d2cfbedb2..b44796953 100644 --- a/cfn-guard/src/util.rs +++ b/cfn-guard/src/util.rs @@ -1,4 +1,4 @@ -// © 2020 Amazon Web Services, Inc. or its affiliates. All Rights Reserved. This AWS Content is provided subject to the terms of the AWS Customer Agreement available at http://aws.amazon.com/agreement or other written agreement between Customer and either Amazon Web Services, Inc. or Amazon Web Services EMEA SARL or both. +// © Amazon Web Services, Inc. or its affiliates. All Rights Reserved. This AWS Content is provided subject to the terms of the AWS Customer Agreement available at http://aws.amazon.com/agreement or other written agreement between Customer and either Amazon Web Services, Inc. or Amazon Web Services EMEA SARL or both. use log::{self, debug, error, trace}; use crate::{structs, enums}; diff --git a/cfn-guard/tests/functional.rs b/cfn-guard/tests/functional.rs index aaf278c69..a07b45a08 100644 --- a/cfn-guard/tests/functional.rs +++ b/cfn-guard/tests/functional.rs @@ -1,4 +1,4 @@ -// © 2020 Amazon Web Services, Inc. or its affiliates. All Rights Reserved. This AWS Content is provided subject to the terms of the AWS Customer Agreement available at http://aws.amazon.com/agreement or other written agreement between Customer and either Amazon Web Services, Inc. or Amazon Web Services EMEA SARL or both. +// © Amazon Web Services, Inc. or its affiliates. All Rights Reserved. This AWS Content is provided subject to the terms of the AWS Customer Agreement available at http://aws.amazon.com/agreement or other written agreement between Customer and either Amazon Web Services, Inc. or Amazon Web Services EMEA SARL or both. // Tests use std::env; From 07c08d8de9ef7c1b96f3360693331dd06c3c9633 Mon Sep 17 00:00:00 2001 From: nathanmc Date: Thu, 18 Jun 2020 20:16:26 -0400 Subject: [PATCH 02/21] Fix "resource with no properties" bug; tests --- cfn-guard-rulegen/src/lib.rs | 6 +++- cfn-guard-rulegen/tests/functional.rs | 48 +++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 1 deletion(-) create mode 100644 cfn-guard-rulegen/tests/functional.rs diff --git a/cfn-guard-rulegen/src/lib.rs b/cfn-guard-rulegen/src/lib.rs index 43d3f2da3..05a011632 100644 --- a/cfn-guard-rulegen/src/lib.rs +++ b/cfn-guard-rulegen/src/lib.rs @@ -44,7 +44,10 @@ fn gen_rules(cfn_resources: HashMap) -> Vec { for (name, cfn_resource) in cfn_resources { trace!("{} is {:?}", name, &cfn_resource); let props: HashMap = - serde_json::from_value(cfn_resource["Properties"].clone()).unwrap(); + match serde_json::from_value(cfn_resource["Properties"].clone()) { + Ok(s) => s, + Err(_) => continue + }; for (prop_name, prop_val) in props { let stripped_val = match prop_val.as_str() { Some(v) => String::from(v), @@ -79,3 +82,4 @@ fn gen_rules(cfn_resources: HashMap) -> Vec { } rule_set.into_iter().collect() } + diff --git a/cfn-guard-rulegen/tests/functional.rs b/cfn-guard-rulegen/tests/functional.rs new file mode 100644 index 000000000..55a1214f8 --- /dev/null +++ b/cfn-guard-rulegen/tests/functional.rs @@ -0,0 +1,48 @@ +// © Amazon Web Services, Inc. or its affiliates. All Rights Reserved. This AWS Content is provided subject to the terms of the AWS Customer Agreement available at http://aws.amazon.com/agreement or other written agreement between Customer and either Amazon Web Services, Inc. or Amazon Web Services EMEA SARL or both. + +// Tests +// use cfn_guard_rulegen; + +mod tests { + + #[test] + fn test_simple_mixed_template() { + let template_contents = String::from(r#" +Resources: + LambdaRoleHelper: + Type: 'AWS::IAM::Role' + Properties: + AssumeRolePolicyDocument: | + { + "Statement": [ + { + "Effect": "Allow", + "Principal": { + "Service": [ + "notlambda.amazonaws.com" + ] + } + } + ] + } +"#); + assert_eq!( + cfn_guard_rulegen::run_gen(&template_contents ), + vec![String::from(r#"AWS::IAM::Role AssumeRolePolicyDocument == { "Statement": [ { "Effect": "Allow", "Principal": { "Service": [ "notlambda.amazonaws.com" ] } } ]}"#)] + ); + } + +#[test] +fn test_no_properties_template() { + let template_contents = String::from(r#" +Resources: + LambdaRoleHelper: + Type: 'AWS::IAM::Role' +"#); + let empty_vec: Vec = vec![]; + assert_eq!( + cfn_guard_rulegen::run_gen(&template_contents ), + empty_vec + ); + } +} From f42c59c9a88978b7323af162e9f48999fa187f22 Mon Sep 17 00:00:00 2001 From: nathanmc Date: Thu, 18 Jun 2020 20:28:21 -0400 Subject: [PATCH 03/21] Set up regexes to only compile on the first pass --- cfn-guard-lambda/Cargo.lock | 1 + cfn-guard/Cargo.lock | 1 + cfn-guard/Cargo.toml | 1 + cfn-guard/src/main.rs | 1 + cfn-guard/src/parser.rs | 43 +++++++++++++++++-------------------- 5 files changed, 24 insertions(+), 23 deletions(-) diff --git a/cfn-guard-lambda/Cargo.lock b/cfn-guard-lambda/Cargo.lock index 05cab7c94..4659bb5df 100644 --- a/cfn-guard-lambda/Cargo.lock +++ b/cfn-guard-lambda/Cargo.lock @@ -91,6 +91,7 @@ name = "cfn-guard" version = "0.5.0" dependencies = [ "clap", + "lazy_static", "log", "regex", "serde", diff --git a/cfn-guard/Cargo.lock b/cfn-guard/Cargo.lock index 46f77571e..17a26113c 100644 --- a/cfn-guard/Cargo.lock +++ b/cfn-guard/Cargo.lock @@ -52,6 +52,7 @@ name = "cfn-guard" version = "0.5.0" dependencies = [ "clap", + "lazy_static", "log", "regex", "serde", diff --git a/cfn-guard/Cargo.toml b/cfn-guard/Cargo.toml index 25a9c118b..787cd0e8b 100644 --- a/cfn-guard/Cargo.toml +++ b/cfn-guard/Cargo.toml @@ -7,6 +7,7 @@ edition = "2018" serde = { version = "1.0.92", features = ["derive"] } serde_json = "1.0" serde_yaml = "0.8.9" +lazy_static = "1.4.0" log = "0.4.6" clap = "2.33.0" simple_logger = "1.3.0" diff --git a/cfn-guard/src/main.rs b/cfn-guard/src/main.rs index 6a6039ef3..04e0fa29c 100644 --- a/cfn-guard/src/main.rs +++ b/cfn-guard/src/main.rs @@ -3,6 +3,7 @@ use std::process; #[macro_use] extern crate log; +extern crate lazy_static; extern crate simple_logger; use clap::{App, Arg}; use log::Level; diff --git a/cfn-guard/src/parser.rs b/cfn-guard/src/parser.rs index 29fe50fe2..aa62dcc1b 100644 --- a/cfn-guard/src/parser.rs +++ b/cfn-guard/src/parser.rs @@ -10,13 +10,18 @@ use serde_json::Value; use crate::guard_types::enums::{CompoundType, LineType, OpCode, RValueType}; use crate::guard_types::structs::{CompoundRule, ParsedRuleSet, Rule}; use crate::util::expand_wildcard_props; - -// TODO: Move consts to lazy_static -// const assign_reg: Regex = Regex::new(r"let (?P\w+) *= *(?P.*)").unwrap(); -// const rule_reg: Regex = Regex::new(r"(?P\S+) +(?P[\w\.\*]+) +(?P\S+) +(?P[^\n\r]+)").unwrap(); -// const rule_with_optional_message_reg: Regex = Regex::new( -// r"(?P\S+) +(?P[\w\.\*]+) +(?P\S+) +(?P[^\n\r]+) +<{2} *(?P.*)").unwrap(); -// const comment_reg: Regex = Regex::new(r#"#(?P.*)"#).unwrap(); +use lazy_static::lazy_static; + +// This sets it up so the regexen only get compiled once +// See: https://docs.rs/regex/1.3.9/regex/#example-avoid-compiling-the-same-regex-in-a-loop +lazy_static! { + static ref ASSIGN_REG: Regex = Regex::new(r"let (?P\w+) *= *(?P.*)").unwrap(); + static ref RULE_REG: Regex = Regex::new(r"(?P\S+) +(?P[\w\.\*]+) +(?P\S+) +(?P[^\n\r]+)").unwrap(); + static ref COMMENT_REG: Regex = Regex::new(r#"#(?P.*)"#).unwrap(); + static ref WILDCARD_OR_RULE_REG: Regex = Regex::new(r"(\S+) (\S+\*\S+) (==) (.+)").unwrap(); + static ref RULE_WITH_OPTIONAL_MESSAGE_REG: Regex = Regex::new( + r"(?P\S+) +(?P[\w\.\*]+) +(?P\S+) +(?P[^\n\r]+) +<{2} *(?P.*)").unwrap(); + } pub(crate) fn parse_rules( rules_file_contents: &str, @@ -25,6 +30,7 @@ pub(crate) fn parse_rules( debug!("Entered parse_rules"); trace!("Parse rules entered with rules_file_contents: {:#?}", &rules_file_contents); trace!("Parse rules entered with cfn_resources: {:#?}", &cfn_resources); + let mut rule_set: Vec = vec![]; let mut variables = HashMap::new(); @@ -71,30 +77,24 @@ pub(crate) fn parse_rules( } fn find_line_type(line: &str) -> LineType { - let assign_reg: Regex = Regex::new(r"let (?P\w+) *= *(?P.*)").unwrap(); - let rule_reg: Regex = Regex::new(r"(?P\S+) +(?P[\w\.\*]+) +(?P\S+) +(?P[^\n\r]+)").unwrap(); - let comment_reg: Regex = Regex::new(r#"#(?P.*)"#).unwrap(); - - if assign_reg.is_match(line) { + if ASSIGN_REG.is_match(line) { return LineType::Assignment; }; - if rule_reg.is_match(line) { + if RULE_REG.is_match(line) { return LineType::Rule; }; - if comment_reg.is_match(line) { + if COMMENT_REG.is_match(line) { return LineType::Comment; }; panic!("BAD RULE: {:?}", line) } fn process_assignment(line: &str) -> Captures { - let assign_reg: Regex = Regex::new(r"let (?P\w+) *= *(?P.*)").unwrap(); - assign_reg.captures(line).unwrap() + ASSIGN_REG.captures(line).unwrap() } fn is_or_rule(line: &str) -> bool { - let wildcard_or_rule_reg: Regex = Regex::new(r"(\S+) (\S+\*\S+) (==) (.+)").unwrap(); - line.contains("|OR|") || wildcard_or_rule_reg.is_match(line) + line.contains("|OR|") || WILDCARD_OR_RULE_REG.is_match(line) } fn process_or_rule(line: &str, cfn_resources: &HashMap) -> CompoundRule { @@ -120,13 +120,10 @@ fn process_and_rule(line: &str, cfn_resources: &HashMap) -> Compo fn destructure_rule(rule_text: &str, cfn_resources: &HashMap) -> Vec { trace!("Entered destructure_rule"); - let rule_with_optional_message_reg: Regex = Regex::new( - r"(?P\S+) +(?P[\w\.\*]+) +(?P\S+) +(?P[^\n\r]+) +<{2} *(?P.*)").unwrap(); - let rule_reg: Regex = Regex::new(r"(?P\S+) +(?P[\w\.\*]+) +(?P\S+) +(?P[^\n\r]+)").unwrap(); let mut rules: Vec = vec![]; - let caps = match rule_with_optional_message_reg.captures(rule_text) { + let caps = match RULE_WITH_OPTIONAL_MESSAGE_REG.captures(rule_text) { Some(c) => c, - None => rule_reg.captures(rule_text).unwrap() + None => RULE_REG.captures(rule_text).unwrap() }; trace!("Parsed rule's captures are: {:#?}", &caps); From b108688eb546e47af30377eb383d9ecdfa24d048 Mon Sep 17 00:00:00 2001 From: nathanmc Date: Thu, 18 Jun 2020 20:37:32 -0400 Subject: [PATCH 04/21] Flatten cfn-guard-lambda Makefile into fail, pass, err targets --- cfn-guard-lambda/Makefile | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/cfn-guard-lambda/Makefile b/cfn-guard-lambda/Makefile index 1713bea07..2750aeab8 100644 --- a/cfn-guard-lambda/Makefile +++ b/cfn-guard-lambda/Makefile @@ -119,12 +119,9 @@ endif test: build aws lambda update-function-code --function-name $(project_name) --zip-file fileb://./lambda.zip - aws lambda invoke --function-name $(project_name) --payload $(request_payload_fail) output.json - cat output.json | jq '.' - aws lambda invoke --function-name $(project_name) --payload $(request_payload_pass) output.json - cat output.json | jq '.' - aws lambda invoke --function-name $(project_name) --payload $(request_payload_err) output.json - cat output.json | jq '.' + $(MAKE) fail + $(MAKE) pass + $(MAKE) err clean: @sh -c "if test -f bootstrap; then rm bootstrap; fi" @@ -159,22 +156,25 @@ build: clean chmod +x bootstrap zip lambda.zip bootstrap -install: build - aws lambda create-function --function-name $(project_name) --handler doesnt.matter --zip-file fileb://./lambda.zip --runtime provided --role $(role_arn) --environment Variables={RUST_BACKTRACE=1} +fail: aws lambda invoke --function-name $(project_name) --payload $(request_payload_fail) output.json cat output.json | jq '.' - aws lambda invoke --function-name $(project_name) --payload $(request_payload_pass) output.json - cat output.json | jq '.' - aws lambda invoke --function-name $(project_name) --payload $(request_payload_err) output.json - cat output.json | jq '.' -invoke: - aws lambda invoke --function-name $(project_name) --payload $(request_payload_fail) output.json - cat output.json | jq '.' +pass: aws lambda invoke --function-name $(project_name) --payload $(request_payload_pass) output.json cat output.json | jq '.' + +err: aws lambda invoke --function-name $(project_name) --payload $(request_payload_err) output.json cat output.json | jq '.' +install: build + aws lambda create-function --function-name $(project_name) --handler doesnt.matter --zip-file fileb://./lambda.zip --runtime provided --role $(role_arn) --environment Variables={RUST_BACKTRACE=1} + $(MAKE) fail + $(MAKE) pass + $(MAKE) err + +invoke: fail pass err + uninstall: aws lambda delete-function --function-name $(project_name) From b82328cd02474e9f30bbb0d32dbd58e78b10cbaf Mon Sep 17 00:00:00 2001 From: nathanmc Date: Thu, 18 Jun 2020 21:01:24 -0400 Subject: [PATCH 05/21] Add top-level test target to Makefile --- Makefile | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Makefile b/Makefile index d37489bd7..d60189fac 100644 --- a/Makefile +++ b/Makefile @@ -15,6 +15,11 @@ cfn-guard-lambda_update: clean: if test -f cloudformation-guard.tar.gz; then rm cloudformation-guard.tar.gz; fi +test: + cd cfn-guard; cargo test + cd cfn-guard-rulegen; cargo test + cd cfn-guard-lambda; make test + release_with_binaries: clean cfn-guard cfn-guard-rulegen tar czvf cloudformation-guard.tar.gz -X Exclude.txt . From 2f246e5f34299e16e41f977d6b50d1f0d88df871 Mon Sep 17 00:00:00 2001 From: nathanmc Date: Thu, 18 Jun 2020 21:02:04 -0400 Subject: [PATCH 06/21] rustfmt --- cfn-guard/src/guard_types.rs | 1 - cfn-guard/src/lib.rs | 101 ++++++++++++-------- cfn-guard/src/main.rs | 9 +- cfn-guard/src/parser.rs | 112 ++++++++++++---------- cfn-guard/src/util.rs | 100 +++++++++++--------- cfn-guard/tests/functional.rs | 173 +++++++++++++++++++++------------- 6 files changed, 292 insertions(+), 204 deletions(-) diff --git a/cfn-guard/src/guard_types.rs b/cfn-guard/src/guard_types.rs index 24a77f934..afaadf197 100644 --- a/cfn-guard/src/guard_types.rs +++ b/cfn-guard/src/guard_types.rs @@ -56,4 +56,3 @@ pub mod structs { pub(crate) rule_set: Vec, } } - diff --git a/cfn-guard/src/lib.rs b/cfn-guard/src/lib.rs index 9d8a2baf5..5399426e4 100644 --- a/cfn-guard/src/lib.rs +++ b/cfn-guard/src/lib.rs @@ -16,7 +16,11 @@ pub mod util; pub use crate::guard_types::{enums, structs}; use crate::util::fix_stringified_bools; -pub fn run(template_file: &str, rules_file: &str, strict_checks: bool) -> Result<(Vec, usize), Box> { +pub fn run( + template_file: &str, + rules_file: &str, + strict_checks: bool, +) -> Result<(Vec, usize), Box> { debug!("Entered run"); let template_contents = fs::read_to_string(template_file)?; let rules_file_contents = fs::read_to_string(rules_file)?; @@ -37,7 +41,11 @@ pub fn run(template_file: &str, rules_file: &str, strict_checks: bool) -> Result Ok((outcome, exit_code)) } -pub fn run_check(template_file_contents: &str, rules_file_contents: &str, strict_checks: bool) -> (Vec, usize) { +pub fn run_check( + template_file_contents: &str, + rules_file_contents: &str, + strict_checks: bool, +) -> (Vec, usize) { info!("Loading CloudFormation Template and Rule Set"); debug!("Entered run_check"); @@ -48,22 +56,29 @@ pub fn run_check(template_file_contents: &str, rules_file_contents: &str, strict cleaned_template_file_contents ); - let cleaned_rules_file_contents= fix_stringified_bools(rules_file_contents); + let cleaned_rules_file_contents = fix_stringified_bools(rules_file_contents); trace!( "Cleaned rules file contents are:\n'{}'", cleaned_rules_file_contents ); debug!("Deserializing CloudFormation template"); - let cfn_template: HashMap = match serde_json::from_str(&cleaned_template_file_contents) { - Ok(s) => s, - Err(_) => match serde_yaml::from_str(&cleaned_template_file_contents) { - Ok(y) => y, - Err(e) => { - return (vec![format!("ERROR: Template file format was unreadable as json or yaml: {}", e)], 1); - } - }, - }; + let cfn_template: HashMap = + match serde_json::from_str(&cleaned_template_file_contents) { + Ok(s) => s, + Err(_) => match serde_yaml::from_str(&cleaned_template_file_contents) { + Ok(y) => y, + Err(e) => { + return ( + vec![format!( + "ERROR: Template file format was unreadable as json or yaml: {}", + e + )], + 1, + ); + } + }, + }; trace!("CFN Template is '{:#?}'", &cfn_template); let cfn_resources: HashMap = @@ -101,7 +116,12 @@ fn check_resources( let mut cfn_resource_map: HashMap = HashMap::new(); cfn_resource_map.insert(name.clone(), cfn_resource.clone()); for rule in &c_rule.rule_list { - match apply_rule(&cfn_resource_map, &rule, &parsed_rule_set.variables, strict_checks) { + match apply_rule( + &cfn_resource_map, + &rule, + &parsed_rule_set.variables, + strict_checks, + ) { Some(rule_result) => { pass_fail.insert("fail"); temp_results.extend(rule_result); @@ -120,11 +140,16 @@ fn check_resources( } enums::CompoundType::AND => { for rule in &c_rule.rule_list { - match apply_rule(&cfn_resources, &rule, &parsed_rule_set.variables, strict_checks) { + match apply_rule( + &cfn_resources, + &rule, + &parsed_rule_set.variables, + strict_checks, + ) { Some(rule_result) => { result.extend(rule_result); } - None => () + None => (), } } } @@ -152,17 +177,17 @@ fn apply_rule( ); let target_field: Vec<&str> = rule.field.split('.').collect(); match util::get_resource_prop_value(&cfn_resource["Properties"], &target_field) { - Err(e) => if strict_checks { - rule_result.push(match &rule.custom_msg { - Some(c) => format!( - "[{}] failed because {}", - name, - c), - None => format!( + Err(e) => { + if strict_checks { + rule_result.push(match &rule.custom_msg { + Some(c) => format!("[{}] failed because {}", name, c), + None => format!( "[{}] failed because it does not contain the required property of [{}]", name, e - )}) - }, + ), + }) + } + } Ok(val) => { debug!("Template val is {:?}", val); match util::deref_rule_value(rule, variables) { @@ -216,16 +241,16 @@ fn apply_rule_operation( res_name, &rule.field, util::format_value(&val), - c), + c + ), None => format!( "[{}] failed because [{}] is [{}] and the permitted value is [{}]", res_name, &rule.field, util::format_value(&val), rule_val.to_string() - ) - } - ) + ), + }) } else { info!("Result: PASS"); None @@ -272,13 +297,14 @@ fn apply_rule_operation( res_name, &rule.field, util::format_value(&val), - c), + c + ), None => format!( "[{}] failed because [{}] is [{}] and that value is not permitted", res_name, &rule.field, util::format_value(&val) - ) + ), }) } else { info!("Result: PASS"); @@ -332,14 +358,15 @@ fn apply_rule_operation( res_name, &rule.field, util::format_value(&val), - c), + c + ), None => format!( "[{}] failed because [{}] is not in {} for [{}]", res_name, util::format_value(&val), rule_val.to_string(), &rule.field - ) + ), }) } else { info!("Result: PASS"); @@ -363,14 +390,15 @@ fn apply_rule_operation( res_name, &rule.field, util::format_value(&val), - c), + c + ), None => format!( "[{}] failed because [{}] is in {} which is not permitted for [{}]", res_name, util::format_value(&val), rule_val.to_string(), &rule.field - ) + ), }) } else { info!("Result: PASS"); @@ -379,8 +407,3 @@ fn apply_rule_operation( } } } - - - - - diff --git a/cfn-guard/src/main.rs b/cfn-guard/src/main.rs index 04e0fa29c..a6912d588 100644 --- a/cfn-guard/src/main.rs +++ b/cfn-guard/src/main.rs @@ -44,7 +44,7 @@ fn main() { Arg::with_name("strict-checks") .short("s") .long("strict-checks") - .help("Fail resources if they're missing the property that a rule checks") + .help("Fail resources if they're missing the property that a rule checks"), ) .get_matches(); @@ -67,7 +67,12 @@ fn main() { &template_file, &rule_set_file ); - let (result, exit_code) = cfn_guard::run(template_file, rule_set_file, matches.is_present("strict-checks")).unwrap_or_else(|err| { + let (result, exit_code) = cfn_guard::run( + template_file, + rule_set_file, + matches.is_present("strict-checks"), + ) + .unwrap_or_else(|err| { println!("Problem checking template: {}", err); process::exit(1); }); diff --git a/cfn-guard/src/parser.rs b/cfn-guard/src/parser.rs index aa62dcc1b..4ec721c15 100644 --- a/cfn-guard/src/parser.rs +++ b/cfn-guard/src/parser.rs @@ -15,21 +15,27 @@ use lazy_static::lazy_static; // This sets it up so the regexen only get compiled once // See: https://docs.rs/regex/1.3.9/regex/#example-avoid-compiling-the-same-regex-in-a-loop lazy_static! { - static ref ASSIGN_REG: Regex = Regex::new(r"let (?P\w+) *= *(?P.*)").unwrap(); - static ref RULE_REG: Regex = Regex::new(r"(?P\S+) +(?P[\w\.\*]+) +(?P\S+) +(?P[^\n\r]+)").unwrap(); - static ref COMMENT_REG: Regex = Regex::new(r#"#(?P.*)"#).unwrap(); - static ref WILDCARD_OR_RULE_REG: Regex = Regex::new(r"(\S+) (\S+\*\S+) (==) (.+)").unwrap(); - static ref RULE_WITH_OPTIONAL_MESSAGE_REG: Regex = Regex::new( - r"(?P\S+) +(?P[\w\.\*]+) +(?P\S+) +(?P[^\n\r]+) +<{2} *(?P.*)").unwrap(); - } + static ref ASSIGN_REG: Regex = Regex::new(r"let (?P\w+) *= *(?P.*)").unwrap(); + static ref RULE_REG: Regex = Regex::new(r"(?P\S+) +(?P[\w\.\*]+) +(?P\S+) +(?P[^\n\r]+)").unwrap(); + static ref COMMENT_REG: Regex = Regex::new(r#"#(?P.*)"#).unwrap(); + static ref WILDCARD_OR_RULE_REG: Regex = Regex::new(r"(\S+) (\S+\*\S+) (==) (.+)").unwrap(); + static ref RULE_WITH_OPTIONAL_MESSAGE_REG: Regex = Regex::new( + r"(?P\S+) +(?P[\w\.\*]+) +(?P\S+) +(?P[^\n\r]+) +<{2} *(?P.*)").unwrap(); +} pub(crate) fn parse_rules( rules_file_contents: &str, cfn_resources: &HashMap, ) -> ParsedRuleSet { debug!("Entered parse_rules"); - trace!("Parse rules entered with rules_file_contents: {:#?}", &rules_file_contents); - trace!("Parse rules entered with cfn_resources: {:#?}", &cfn_resources); + trace!( + "Parse rules entered with rules_file_contents: {:#?}", + &rules_file_contents + ); + trace!( + "Parse rules entered with cfn_resources: {:#?}", + &cfn_resources + ); let mut rule_set: Vec = vec![]; let mut variables = HashMap::new(); @@ -104,7 +110,7 @@ fn process_or_rule(line: &str, cfn_resources: &HashMap) -> Compou let mut rules: Vec = vec![]; for b in branches { rules.append(destructure_rule(b.trim(), cfn_resources).as_mut()); - }; + } CompoundRule { compound_type: CompoundType::OR, rule_list: rules, @@ -123,7 +129,7 @@ fn destructure_rule(rule_text: &str, cfn_resources: &HashMap) -> let mut rules: Vec = vec![]; let caps = match RULE_WITH_OPTIONAL_MESSAGE_REG.captures(rule_text) { Some(c) => c, - None => RULE_REG.captures(rule_text).unwrap() + None => RULE_REG.captures(rule_text).unwrap(), }; trace!("Parsed rule's captures are: {:#?}", &caps); @@ -131,12 +137,16 @@ fn destructure_rule(rule_text: &str, cfn_resources: &HashMap) -> if caps["resource_property"].contains("*") { for (_name, value) in cfn_resources { if caps["resource_type"] == value["Type"] { - match expand_wildcard_props( &value["Properties"],caps["resource_property"].to_string(), String::from("")) { + match expand_wildcard_props( + &value["Properties"], + caps["resource_property"].to_string(), + String::from(""), + ) { Some(p) => { props.append(&mut p.clone()); trace!("Expanded props are {:#?}", &props); - }, - None => () + } + None => (), } } } @@ -145,45 +155,43 @@ fn destructure_rule(rule_text: &str, cfn_resources: &HashMap) -> }; for p in props { - rules.push( - Rule { - resource_type: caps["resource_type"].to_string(), - field: p.to_string(), - operation: { - match &caps["operator"] { - "==" => OpCode::Require, - "!=" => OpCode::RequireNot, - "IN" => OpCode::In, - "NOT_IN" => OpCode::NotIn, + rules.push(Rule { + resource_type: caps["resource_type"].to_string(), + field: p.to_string(), + operation: { + match &caps["operator"] { + "==" => OpCode::Require, + "!=" => OpCode::RequireNot, + "IN" => OpCode::In, + "NOT_IN" => OpCode::NotIn, + _ => panic!(format!("Bad Rule Operator: {}", &caps["operator"])), + } + }, + rule_vtype: { + let rv = caps["rule_value"].chars().nth(0).unwrap(); + match rv { + '[' => match &caps["operator"] { + "==" | "!=" => RValueType::Value, + "IN" | "NOT_IN" => RValueType::List, _ => panic!(format!("Bad Rule Operator: {}", &caps["operator"])), - } - }, - rule_vtype: { - let rv = caps["rule_value"].chars().nth(0).unwrap(); - match rv { - '[' => match &caps["operator"] { - "==" | "!=" => RValueType::Value, - "IN" | "NOT_IN" => RValueType::List, - _ => panic!(format!("Bad Rule Operator: {}", &caps["operator"])), - }, - '/' => RValueType::Regex, - '%' => RValueType::Variable, - _ => RValueType::Value, - } - }, - value: { - let rv = caps["rule_value"].chars().nth(0).unwrap(); - match rv { - '/' => caps["rule_value"].trim_matches('/').to_string(), - _ => caps["rule_value"].to_string().trim().to_string(), - } - }, - custom_msg: match caps.name("custom_msg") { - Some(s) => Some(s.as_str().to_string()), - None => None - }, - } - ) + }, + '/' => RValueType::Regex, + '%' => RValueType::Variable, + _ => RValueType::Value, + } + }, + value: { + let rv = caps["rule_value"].chars().nth(0).unwrap(); + match rv { + '/' => caps["rule_value"].trim_matches('/').to_string(), + _ => caps["rule_value"].to_string().trim().to_string(), + } + }, + custom_msg: match caps.name("custom_msg") { + Some(s) => Some(s.as_str().to_string()), + None => None, + }, + }) } trace!("Destructured rules are: {:#?}", &rules); diff --git a/cfn-guard/src/util.rs b/cfn-guard/src/util.rs index b44796953..2a0f06b05 100644 --- a/cfn-guard/src/util.rs +++ b/cfn-guard/src/util.rs @@ -1,15 +1,20 @@ // © Amazon Web Services, Inc. or its affiliates. All Rights Reserved. This AWS Content is provided subject to the terms of the AWS Customer Agreement available at http://aws.amazon.com/agreement or other written agreement between Customer and either Amazon Web Services, Inc. or Amazon Web Services EMEA SARL or both. +use crate::{enums, structs}; +use lazy_static::lazy_static; use log::{self, debug, error, trace}; -use crate::{structs, enums}; -use std::collections::HashMap; +use regex::{Captures, Regex}; use serde_json::Value; -use regex::{Regex, Captures}; +use std::collections::HashMap; +// This sets it up so the regex only gets compiled once +// See: https://docs.rs/regex/1.3.9/regex/#example-avoid-compiling-the-same-regex-in-a-loop +lazy_static! { + static ref STRINGIFIED_BOOLS: Regex = + Regex::new(r"[:=]\s*([fF]alse|[tT]rue)\s*([,}]+|$)").unwrap(); +} pub fn fix_stringified_bools(fstr: &str) -> String { - let regex_pattern = r"[:=]\s*([fF]alse|[tT]rue)\s*([,}]+|$)"; - let re = Regex::new(regex_pattern).unwrap(); - let after = re.replace_all(fstr, |caps: &Captures| { + let after = STRINGIFIED_BOOLS.replace_all(fstr, |caps: &Captures| { format!("{}", &caps[0].to_lowercase()) }); after.to_string() @@ -137,11 +142,17 @@ pub fn deref_rule_value<'a>( } } -pub fn expand_wildcard_props(props: &Value, address: String, accumulator: String) -> Option> { - trace!("Entering expand_wildcard_props() with props: {:#?} , address: {:#?} , accumulator: {:#?}", +pub fn expand_wildcard_props( + props: &Value, + address: String, + accumulator: String, +) -> Option> { + trace!( + "Entering expand_wildcard_props() with props: {:#?} , address: {:#?} , accumulator: {:#?}", &props, &address, - &accumulator); + &accumulator + ); let mut segments = address.split("*").collect::>(); trace!("Segments are {:#?}", &segments); let segment = segments.remove(0); @@ -151,28 +162,26 @@ pub fn expand_wildcard_props(props: &Value, address: String, accumulator: String let s = segment.trim_end_matches(".").trim_start_matches("."); let steps = s.split(".").collect::>(); match get_resource_prop_value(props, &steps) { - Ok(v) => { - match v.as_array() { - Some(result_array) => { - trace!("Value is an array"); - let mut counter = 0; - for r in result_array { - trace!("Counter is {:#?}", counter); - let next_segment = segments.join("*"); - trace!("next_segment is {:#?}", &next_segment); - let temp_address = format!("{}{}{}",accumulator, segment, counter); - trace!("temp_address is {:#?}", &temp_address); - match expand_wildcard_props(&r, next_segment, temp_address) { - Some(result) => expanded_props.append(&mut result.clone()), - None => return None - } - counter += 1; + Ok(v) => match v.as_array() { + Some(result_array) => { + trace!("Value is an array"); + let mut counter = 0; + for r in result_array { + trace!("Counter is {:#?}", counter); + let next_segment = segments.join("*"); + trace!("next_segment is {:#?}", &next_segment); + let temp_address = format!("{}{}{}", accumulator, segment, counter); + trace!("temp_address is {:#?}", &temp_address); + match expand_wildcard_props(&r, next_segment, temp_address) { + Some(result) => expanded_props.append(&mut result.clone()), + None => return None, } - }, - None => expanded_props.push(format!("{}{}", accumulator, segment)) + counter += 1; + } } + None => expanded_props.push(format!("{}{}", accumulator, segment)), }, - Err(_) => return None + Err(_) => return None, } Some(expanded_props) } else { @@ -181,14 +190,13 @@ pub fn expand_wildcard_props(props: &Value, address: String, accumulator: String } } - mod tests { #[cfg(test)] - use std::collections::HashMap; + use crate::util::expand_wildcard_props; #[cfg(test)] use serde_yaml; #[cfg(test)] - use crate::util::expand_wildcard_props; + use std::collections::HashMap; #[test] fn test_wildcard_expansion() { @@ -222,23 +230,29 @@ Resources: Action: - 'sts:AssumeRole' "#; - let cfn_template: HashMap = serde_yaml::from_str(&iam_template).unwrap(); + let cfn_template: HashMap = + serde_yaml::from_str(&iam_template).unwrap(); let mut wildcard = String::from("AssumeRolePolicyDocument.Statement.*.Effect"); let root = &cfn_template["Resources"]["LambdaRoleHelper"]["Properties"]; - let mut expanded_wildcards = expand_wildcard_props(&root, wildcard, String::from("")).unwrap(); - assert_eq!(expanded_wildcards, - vec![String::from("AssumeRolePolicyDocument.Statement.0.Effect"), - String::from("AssumeRolePolicyDocument.Statement.1.Effect"), - String::from("AssumeRolePolicyDocument.Statement.2.Effect"), - ] + let mut expanded_wildcards = + expand_wildcard_props(&root, wildcard, String::from("")).unwrap(); + assert_eq!( + expanded_wildcards, + vec![ + String::from("AssumeRolePolicyDocument.Statement.0.Effect"), + String::from("AssumeRolePolicyDocument.Statement.1.Effect"), + String::from("AssumeRolePolicyDocument.Statement.2.Effect"), + ] ); wildcard = String::from("AssumeRolePolicyDocument.Statement.*.Action.*"); expanded_wildcards = expand_wildcard_props(&root, wildcard, String::from("")).unwrap(); - assert_eq!(expanded_wildcards, - vec![String::from("AssumeRolePolicyDocument.Statement.0.Action.0"), - String::from("AssumeRolePolicyDocument.Statement.1.Action.0"), - String::from("AssumeRolePolicyDocument.Statement.2.Action.0"), - ] + assert_eq!( + expanded_wildcards, + vec![ + String::from("AssumeRolePolicyDocument.Statement.0.Action.0"), + String::from("AssumeRolePolicyDocument.Statement.1.Action.0"), + String::from("AssumeRolePolicyDocument.Statement.2.Action.0"), + ] ); } } diff --git a/cfn-guard/tests/functional.rs b/cfn-guard/tests/functional.rs index a07b45a08..f2f670699 100644 --- a/cfn-guard/tests/functional.rs +++ b/cfn-guard/tests/functional.rs @@ -1,9 +1,9 @@ // © Amazon Web Services, Inc. or its affiliates. All Rights Reserved. This AWS Content is provided subject to the terms of the AWS Customer Agreement available at http://aws.amazon.com/agreement or other written agreement between Customer and either Amazon Web Services, Inc. or Amazon Web Services EMEA SARL or both. // Tests +use cfn_guard; use std::env; use std::fs; -use cfn_guard; mod tests { use super::*; @@ -30,7 +30,8 @@ mod tests { #[test] fn test_lax_boolean_correction() { - let mut template_contents = String::from(r#" + let mut template_contents = String::from( + r#" { "Resources": { "NewVolume" : { @@ -50,7 +51,8 @@ mod tests { } } } - }"#); + }"#, + ); let mut rules_file_contents = String::from("AWS::EC2::Volume Encrypted == true"); assert_eq!( cfn_guard::run_check(&template_contents, &rules_file_contents, true), @@ -62,7 +64,8 @@ mod tests { (vec![], 0) ); - template_contents = String::from(r#" + template_contents = String::from( + r#" { "Resources": { "NewVolume" : { @@ -82,8 +85,9 @@ mod tests { } } } - }"#); - rules_file_contents = String::from("AWS::EC2::Volume Encrypted == false"); + }"#, + ); + rules_file_contents = String::from("AWS::EC2::Volume Encrypted == false"); assert_eq!( cfn_guard::run_check(&template_contents, &rules_file_contents, true), (vec![], 0) @@ -95,7 +99,8 @@ mod tests { (vec![], 0) ); - template_contents = String::from(r#" + template_contents = String::from( + r#" { "Resources": { "NewVolume" : { @@ -115,7 +120,8 @@ mod tests { } } } - }"#); + }"#, + ); rules_file_contents = String::from("AWS::EC2::Volume Encrypted == false"); assert_eq!( cfn_guard::run_check(&template_contents, &rules_file_contents, true), @@ -128,7 +134,8 @@ mod tests { (vec![], 0) ); - template_contents = String::from(r#" + template_contents = String::from( + r#" { "Resources": { "NewVolume" : { @@ -146,7 +153,8 @@ mod tests { } } } - }"#); + }"#, + ); rules_file_contents = String::from("AWS::EC2::Volume Encrypted == false"); assert_eq!( cfn_guard::run_check(&template_contents, &rules_file_contents, false), @@ -162,53 +170,76 @@ mod tests { #[test] fn test_fail_on_regex_require_not_match() { - let template_contents = - fs::read_to_string("tests/ebs_volume_template.json") - .unwrap_or_else(|err| format!("{}", err)); + let template_contents = fs::read_to_string("tests/ebs_volume_template.json") + .unwrap_or_else(|err| format!("{}", err)); let rules_file_content = String::from(r#"AWS::EC2::Volume Encrypted != /true/"#); assert_eq!( cfn_guard::run_check(&template_contents, &rules_file_content, true), - (vec![String::from(r#"[NewVolume2] failed because [Encrypted] is [true] and the pattern [true] is not permitted"#), - String::from(r#"[NewVolume] failed because [Encrypted] is [true] and the pattern [true] is not permitted"#)], - 2) + ( + vec![ + String::from( + r#"[NewVolume2] failed because [Encrypted] is [true] and the pattern [true] is not permitted"# + ), + String::from( + r#"[NewVolume] failed because [Encrypted] is [true] and the pattern [true] is not permitted"# + ) + ], + 2 + ) ); } #[test] fn test_fail_on_regex_require_not_match_custom_message() { - let template_contents = - fs::read_to_string("tests/ebs_volume_template.json") - .unwrap_or_else(|err| format!("{}", err)); - let rules_file_content = String::from(r#"AWS::EC2::Volume Encrypted != /true/ << lorem ipsum"#); + let template_contents = fs::read_to_string("tests/ebs_volume_template.json") + .unwrap_or_else(|err| format!("{}", err)); + let rules_file_content = + String::from(r#"AWS::EC2::Volume Encrypted != /true/ << lorem ipsum"#); assert_eq!( cfn_guard::run_check(&template_contents, &rules_file_content, true), - (vec![String::from(r#"[NewVolume2] failed because [Encrypted] is [true] and lorem ipsum"#), - String::from(r#"[NewVolume] failed because [Encrypted] is [true] and lorem ipsum"#)], - 2) + ( + vec![ + String::from( + r#"[NewVolume2] failed because [Encrypted] is [true] and lorem ipsum"# + ), + String::from( + r#"[NewVolume] failed because [Encrypted] is [true] and lorem ipsum"# + ) + ], + 2 + ) ); } #[test] fn test_fail_require_not_custom_message() { - let template_contents = - fs::read_to_string("tests/ebs_volume_template.json") - .unwrap_or_else(|err| format!("{}", err)); - let rules_file_content = String::from(r#"AWS::EC2::Volume Encrypted != true << lorem ipsum"#); + let template_contents = fs::read_to_string("tests/ebs_volume_template.json") + .unwrap_or_else(|err| format!("{}", err)); + let rules_file_content = + String::from(r#"AWS::EC2::Volume Encrypted != true << lorem ipsum"#); assert_eq!( cfn_guard::run_check(&template_contents, &rules_file_content, true), - (vec![String::from(r#"[NewVolume2] failed because [Encrypted] is [true] and lorem ipsum"#), - String::from(r#"[NewVolume] failed because [Encrypted] is [true] and lorem ipsum"#)], - 2) + ( + vec![ + String::from( + r#"[NewVolume2] failed because [Encrypted] is [true] and lorem ipsum"# + ), + String::from( + r#"[NewVolume] failed because [Encrypted] is [true] and lorem ipsum"# + ) + ], + 2 + ) ); } #[test] fn test_bad_template() { - let template_contents = - fs::read_to_string("tests/broken_template_file.json") - .unwrap_or_else(|err| format!("{}", err)); - let rules_file_contents = fs::read_to_string("tests/ebs_volume_rule_set_custom_msg.passing") + let template_contents = fs::read_to_string("tests/broken_template_file.json") .unwrap_or_else(|err| format!("{}", err)); + let rules_file_contents = + fs::read_to_string("tests/ebs_volume_rule_set_custom_msg.passing") + .unwrap_or_else(|err| format!("{}", err)); assert_eq!( cfn_guard::run_check(&template_contents, &rules_file_contents, true), (vec![String::from("ERROR: Template file format was unreadable as json or yaml: invalid type: string \"THIS IS MEANT TO BE INVALID\", expected a map at line 1 column 1")], 1) @@ -217,11 +248,11 @@ mod tests { #[test] fn test_custom_fail_message_pass() { - let template_contents = - fs::read_to_string("tests/ebs_volume_template.json") - .unwrap_or_else(|err| format!("{}", err)); - let rules_file_contents = fs::read_to_string("tests/ebs_volume_rule_set_custom_msg.passing") + let template_contents = fs::read_to_string("tests/ebs_volume_template.json") .unwrap_or_else(|err| format!("{}", err)); + let rules_file_contents = + fs::read_to_string("tests/ebs_volume_rule_set_custom_msg.passing") + .unwrap_or_else(|err| format!("{}", err)); assert_eq!( cfn_guard::run_check(&template_contents, &rules_file_contents, true), (vec![], 0) @@ -237,11 +268,11 @@ mod tests { // Since an |OR| is a join of two discrete rules, you can see how the first half lacks a custom message. // I decided to leave that detail in the results to underscore the behavior so that it doesn't get // lost in the shuffle. - let template_contents = - fs::read_to_string("tests/ebs_volume_template.json") - .unwrap_or_else(|err| format!("{}", err)); - let rules_file_contents = fs::read_to_string("tests/ebs_volume_rule_set_custom_msg.failing") + let template_contents = fs::read_to_string("tests/ebs_volume_template.json") .unwrap_or_else(|err| format!("{}", err)); + let rules_file_contents = + fs::read_to_string("tests/ebs_volume_rule_set_custom_msg.failing") + .unwrap_or_else(|err| format!("{}", err)); let mut outcome = vec![ String::from("[NewVolume2] failed because [Encrypted] is [true] and enc lorem ipsum"), String::from("[NewVolume2] failed because [Size] is [99] and or lorem ipsum"), @@ -263,7 +294,8 @@ mod tests { #[test] fn test_not_in_list_fail() { - let template_contents = String::from(r#" + let template_contents = String::from( + r#" { "Resources": { "NewVolume" : { @@ -283,7 +315,8 @@ mod tests { } } } - }"#); + }"#, + ); let rules_file_contents = fs::read_to_string("tests/test_not_in_list_fail.ruleset") .unwrap_or_else(|err| format!("{}", err)); @@ -298,7 +331,8 @@ mod tests { #[test] fn test_in_list_fail_custom_message() { - let template_contents = String::from(r#" + let template_contents = String::from( + r#" { "Resources": { "NewVolume" : { @@ -318,10 +352,12 @@ mod tests { } } } - }"#); + }"#, + ); - let rules_file_contents = fs::read_to_string("tests/test_in_list_fail_custom_message.ruleset") - .unwrap_or_else(|err| format!("{}", err)); + let rules_file_contents = + fs::read_to_string("tests/test_in_list_fail_custom_message.ruleset") + .unwrap_or_else(|err| format!("{}", err)); assert_eq!( cfn_guard::run_check(&template_contents, &rules_file_contents, true), (vec![ @@ -890,9 +926,8 @@ AWS::EC2::Volume Size == 101 |OR| AWS::EC2::Volume Size == 99"#, #[test] fn test_diff_wildcard_type_pass() { - let template_contents = - fs::read_to_string("tests/aws-waf-security-automations.template") - .unwrap_or_else(|err| format!("{}", err)); + let template_contents = fs::read_to_string("tests/aws-waf-security-automations.template") + .unwrap_or_else(|err| format!("{}", err)); let rules_file_contents = fs::read_to_string("tests/wildcard_iam_rule_set.passing") .unwrap_or_else(|err| format!("{}", err)); assert_eq!( @@ -903,12 +938,10 @@ AWS::EC2::Volume Size == 101 |OR| AWS::EC2::Volume Size == 99"#, #[test] fn test_diff_wildcard_type_fail() { - let template_contents = - fs::read_to_string("tests/aws-waf-security-automations.template") - .unwrap_or_else(|err| format!("{}", err)); - let rules_file_contents = - fs::read_to_string("tests/wildcard_not_in_iam_rule_set.failing") - .unwrap_or_else(|err| format!("{}", err)); + let template_contents = fs::read_to_string("tests/aws-waf-security-automations.template") + .unwrap_or_else(|err| format!("{}", err)); + let rules_file_contents = fs::read_to_string("tests/wildcard_not_in_iam_rule_set.failing") + .unwrap_or_else(|err| format!("{}", err)); assert_eq!( cfn_guard::run_check(&template_contents, &rules_file_contents, true), (vec![String::from("[LambdaRoleHelper] failed because [lambda.amazonaws.com] is in [lambda.amazonaws.com, ec2.amazonaws.com] which is not permitted for [AssumeRolePolicyDocument.Statement.0.Principal.Service.0]"), ], @@ -950,11 +983,14 @@ AWS::EC2::Volume Size == 101 |OR| AWS::EC2::Volume Size == 99"#, // That shows up as an empty "error: test failed" // Try running with "cargo test -- --nocapture" // If you see "Could not load value"... then it's the process exit - let template_contents = - fs::read_to_string("tests/test_do_not_fail_when_type_lacks_property_for_wildcard.template") - .unwrap_or_else(|err| format!("{}", err)); - let rules_file_contents = fs::read_to_string("tests/test_do_not_fail_when_type_lacks_property_for_wildcard.ruleset") - .unwrap_or_else(|err| format!("{}", err)); + let template_contents = fs::read_to_string( + "tests/test_do_not_fail_when_type_lacks_property_for_wildcard.template", + ) + .unwrap_or_else(|err| format!("{}", err)); + let rules_file_contents = fs::read_to_string( + "tests/test_do_not_fail_when_type_lacks_property_for_wildcard.ruleset", + ) + .unwrap_or_else(|err| format!("{}", err)); assert_eq!( cfn_guard::run_check(&template_contents, &rules_file_contents, true), (vec![String::from("[NewVolume2] failed because it does not contain the required property of [Tags]"), @@ -971,11 +1007,14 @@ AWS::EC2::Volume Size == 101 |OR| AWS::EC2::Volume Size == 99"#, // That shows up as an empty "error: test failed" // Try running with "cargo test -- --nocapture" // If you see "Could not load value"... then it's the process exit - let template_contents = - fs::read_to_string("tests/test_do_not_fail_when_type_lacks_property_for_wildcard.template") - .unwrap_or_else(|err| format!("{}", err)); - let rules_file_contents = fs::read_to_string("tests/test_do_not_fail_when_type_lacks_property_for_wildcard.ruleset") - .unwrap_or_else(|err| format!("{}", err)); + let template_contents = fs::read_to_string( + "tests/test_do_not_fail_when_type_lacks_property_for_wildcard.template", + ) + .unwrap_or_else(|err| format!("{}", err)); + let rules_file_contents = fs::read_to_string( + "tests/test_do_not_fail_when_type_lacks_property_for_wildcard.ruleset", + ) + .unwrap_or_else(|err| format!("{}", err)); assert_eq!( cfn_guard::run_check(&template_contents, &rules_file_contents, false), (vec![String::from("[NewVolume] failed because [Tags.0.Key] is [uaid] and the permitted value is [uai]"), From c25aa97dbf79386dd76d31542936026e8258d649 Mon Sep 17 00:00:00 2001 From: nathanmc Date: Fri, 19 Jun 2020 13:10:55 -0400 Subject: [PATCH 07/21] Deduplicate destructure_rule() result --- cfn-guard/src/guard_types.rs | 18 ++++++++++++++++++ cfn-guard/src/parser.rs | 24 +++++++++++------------- cfn-guard/tests/functional.rs | 2 -- 3 files changed, 29 insertions(+), 15 deletions(-) diff --git a/cfn-guard/src/guard_types.rs b/cfn-guard/src/guard_types.rs index afaadf197..535d0c042 100644 --- a/cfn-guard/src/guard_types.rs +++ b/cfn-guard/src/guard_types.rs @@ -10,6 +10,9 @@ pub mod enums { } #[derive(Debug)] + #[derive(Hash)] + #[derive(PartialEq, Eq)] + #[derive(Clone)] pub enum OpCode { Require, RequireNot, @@ -18,6 +21,9 @@ pub mod enums { } #[derive(Debug)] + #[derive(Hash)] + #[derive(PartialEq, Eq)] + #[derive(Clone)] pub enum RValueType { Value, List, @@ -25,6 +31,7 @@ pub mod enums { Variable, } #[derive(Debug)] + #[derive(Clone)] pub enum CompoundType { OR, AND, @@ -35,6 +42,9 @@ pub mod structs { use std::collections::HashMap; #[derive(Debug)] + #[derive(Hash)] + #[derive(Eq)] + #[derive(Clone)] pub struct Rule { pub(crate) resource_type: String, pub(crate) field: String, @@ -44,6 +54,14 @@ pub mod structs { pub(crate) custom_msg: Option, } + impl PartialEq for Rule { + fn eq(&self, other: &Rule) -> bool { + let self_string: String = format!("{:#?}", self); + let other_string: String = format!("{:#?}", other); + self_string == other_string + } + } + #[derive(Debug)] pub struct CompoundRule { pub(crate) compound_type: super::enums::CompoundType, diff --git a/cfn-guard/src/parser.rs b/cfn-guard/src/parser.rs index 4ec721c15..a5dcb2007 100644 --- a/cfn-guard/src/parser.rs +++ b/cfn-guard/src/parser.rs @@ -1,6 +1,6 @@ // © Amazon Web Services, Inc. or its affiliates. All Rights Reserved. This AWS Content is provided subject to the terms of the AWS Customer Agreement available at http://aws.amazon.com/agreement or other written agreement between Customer and either Amazon Web Services, Inc. or Amazon Web Services EMEA SARL or both. -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::env; use log::{self, debug, trace}; @@ -126,7 +126,7 @@ fn process_and_rule(line: &str, cfn_resources: &HashMap) -> Compo fn destructure_rule(rule_text: &str, cfn_resources: &HashMap) -> Vec { trace!("Entered destructure_rule"); - let mut rules: Vec = vec![]; + let mut rules_hash: HashSet = HashSet::new(); let caps = match RULE_WITH_OPTIONAL_MESSAGE_REG.captures(rule_text) { Some(c) => c, None => RULE_REG.captures(rule_text).unwrap(), @@ -134,19 +134,16 @@ fn destructure_rule(rule_text: &str, cfn_resources: &HashMap) -> trace!("Parsed rule's captures are: {:#?}", &caps); let mut props: Vec = vec![]; - if caps["resource_property"].contains("*") { + if caps["resource_property"].contains('*') { for (_name, value) in cfn_resources { if caps["resource_type"] == value["Type"] { - match expand_wildcard_props( + if let Some(p) = expand_wildcard_props( &value["Properties"], caps["resource_property"].to_string(), String::from(""), ) { - Some(p) => { - props.append(&mut p.clone()); - trace!("Expanded props are {:#?}", &props); - } - None => (), + props.append(&mut p.clone()); + trace!("Expanded props are {:#?}", &props); } } } @@ -155,7 +152,7 @@ fn destructure_rule(rule_text: &str, cfn_resources: &HashMap) -> }; for p in props { - rules.push(Rule { + rules_hash.insert(Rule { resource_type: caps["resource_type"].to_string(), field: p.to_string(), operation: { @@ -168,7 +165,7 @@ fn destructure_rule(rule_text: &str, cfn_resources: &HashMap) -> } }, rule_vtype: { - let rv = caps["rule_value"].chars().nth(0).unwrap(); + let rv = caps["rule_value"].chars().next().unwrap(); match rv { '[' => match &caps["operator"] { "==" | "!=" => RValueType::Value, @@ -181,7 +178,7 @@ fn destructure_rule(rule_text: &str, cfn_resources: &HashMap) -> } }, value: { - let rv = caps["rule_value"].chars().nth(0).unwrap(); + let rv = caps["rule_value"].chars().next().unwrap(); match rv { '/' => caps["rule_value"].trim_matches('/').to_string(), _ => caps["rule_value"].to_string().trim().to_string(), @@ -191,9 +188,10 @@ fn destructure_rule(rule_text: &str, cfn_resources: &HashMap) -> Some(s) => Some(s.as_str().to_string()), None => None, }, - }) + }); } + let rules = rules_hash.into_iter().collect::>(); trace!("Destructured rules are: {:#?}", &rules); rules } diff --git a/cfn-guard/tests/functional.rs b/cfn-guard/tests/functional.rs index f2f670699..46eecdbec 100644 --- a/cfn-guard/tests/functional.rs +++ b/cfn-guard/tests/functional.rs @@ -970,8 +970,6 @@ AWS::EC2::Volume Size == 101 |OR| AWS::EC2::Volume Size == 99"#, assert_eq!( cfn_guard::run_check(&template_contents, &rules_file_contents, true), (vec![String::from("[EndpointCloudWatchRoleC3C64E0F] failed because [AssumeRolePolicyDocument.Statement.0.Action] is [sts:AssumeRole] and that value is not permitted"), - String::from("[EndpointCloudWatchRoleC3C64E0F] failed because [AssumeRolePolicyDocument.Statement.0.Action] is [sts:AssumeRole] and that value is not permitted"), - String::from("[HelloHandlerServiceRole11EF7C63] failed because [AssumeRolePolicyDocument.Statement.0.Action] is [sts:AssumeRole] and that value is not permitted"), String::from("[HelloHandlerServiceRole11EF7C63] failed because [AssumeRolePolicyDocument.Statement.0.Action] is [sts:AssumeRole] and that value is not permitted")], 2) ); From 12ce2e52ee90866b11f1679b82747b3d879ef900 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A1bor=20Lipt=C3=A1k?= Date: Thu, 18 Jun 2020 10:40:03 -0400 Subject: [PATCH 08/21] Add Travis build --- .travis.yml | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 .travis.yml diff --git a/.travis.yml b/.travis.yml new file mode 100644 index 000000000..cab8b6ac8 --- /dev/null +++ b/.travis.yml @@ -0,0 +1,8 @@ +language: rust + +script: + - cd cfn-guard && cargo build --verbose --all && cargo test --verbose --all +script: + - cd cfn-guard-lambda && cargo build --verbose --all && cargo test --verbose --all +script: + - cd cfn-guard-rulegen && cargo build --verbose --all && cargo test --verbose --all \ No newline at end of file From 4c13ea94b8f702f0cc1e73d9cee8e8a13360f445 Mon Sep 17 00:00:00 2001 From: nathanmc Date: Fri, 19 Jun 2020 15:04:30 -0400 Subject: [PATCH 09/21] Clippy cleanup --- cfn-guard/src/guard_types.rs | 26 ++++---------------------- cfn-guard/src/lib.rs | 19 +++++-------------- cfn-guard/src/main.rs | 2 +- cfn-guard/src/util.rs | 16 +++++----------- 4 files changed, 15 insertions(+), 48 deletions(-) diff --git a/cfn-guard/src/guard_types.rs b/cfn-guard/src/guard_types.rs index 535d0c042..e007c0db2 100644 --- a/cfn-guard/src/guard_types.rs +++ b/cfn-guard/src/guard_types.rs @@ -9,10 +9,7 @@ pub mod enums { Rule, } - #[derive(Debug)] - #[derive(Hash)] - #[derive(PartialEq, Eq)] - #[derive(Clone)] + #[derive(Debug, Hash, PartialEq, Eq, Clone)] pub enum OpCode { Require, RequireNot, @@ -20,18 +17,14 @@ pub mod enums { NotIn, } - #[derive(Debug)] - #[derive(Hash)] - #[derive(PartialEq, Eq)] - #[derive(Clone)] + #[derive(Debug, Hash, PartialEq, Eq, Clone)] pub enum RValueType { Value, List, Regex, Variable, } - #[derive(Debug)] - #[derive(Clone)] + #[derive(Debug, Clone)] pub enum CompoundType { OR, AND, @@ -41,10 +34,7 @@ pub mod enums { pub mod structs { use std::collections::HashMap; - #[derive(Debug)] - #[derive(Hash)] - #[derive(Eq)] - #[derive(Clone)] + #[derive(Debug, Hash, Eq, PartialEq, Clone)] pub struct Rule { pub(crate) resource_type: String, pub(crate) field: String, @@ -54,14 +44,6 @@ pub mod structs { pub(crate) custom_msg: Option, } - impl PartialEq for Rule { - fn eq(&self, other: &Rule) -> bool { - let self_string: String = format!("{:#?}", self); - let other_string: String = format!("{:#?}", other); - self_string == other_string - } - } - #[derive(Debug)] pub struct CompoundRule { pub(crate) compound_type: super::enums::CompoundType, diff --git a/cfn-guard/src/lib.rs b/cfn-guard/src/lib.rs index 5399426e4..80c9d399a 100644 --- a/cfn-guard/src/lib.rs +++ b/cfn-guard/src/lib.rs @@ -33,7 +33,7 @@ pub fn run( trace!( "Rules file is '{}' and its contents are: {}", rules_file, - format!("{}", rules_file_contents) + rules_file_contents.to_string() ); let (outcome, exit_code) = run_check(&template_contents, &rules_file_contents, strict_checks); @@ -140,16 +140,13 @@ fn check_resources( } enums::CompoundType::AND => { for rule in &c_rule.rule_list { - match apply_rule( + if let Some(rule_result) = apply_rule( &cfn_resources, &rule, &parsed_rule_set.variables, strict_checks, ) { - Some(rule_result) => { - result.extend(rule_result); - } - None => (), + result.extend(rule_result); } } } @@ -345,10 +342,7 @@ fn apply_rule_operation( let value_vec = util::convert_list_var_to_vec(rule_val); let val_as_string = match val.as_str() { Some(s) => s.to_string(), - None => { - let serde_string = serde_json::to_string(val).unwrap(); - serde_string - } + None => serde_json::to_string(val).unwrap(), }; if !value_vec.contains(&util::strip_ws_nl(val_as_string)) { info!("Result: FAIL"); @@ -377,10 +371,7 @@ fn apply_rule_operation( let value_vec = util::convert_list_var_to_vec(rule_val); let val_as_string = match val.as_str() { Some(s) => s.to_string(), - None => { - let serde_string = serde_json::to_string(val).unwrap(); - serde_string - } + None => serde_json::to_string(val).unwrap(), }; if value_vec.contains(&util::strip_ws_nl(val_as_string)) { info!("Result: FAIL"); diff --git a/cfn-guard/src/main.rs b/cfn-guard/src/main.rs index a6912d588..0fd64508a 100644 --- a/cfn-guard/src/main.rs +++ b/cfn-guard/src/main.rs @@ -52,7 +52,7 @@ fn main() { 0 => Level::Error, 1 => Level::Info, 2 => Level::Debug, - 3 | _ => Level::Trace, + _ => Level::Trace, }; simple_logger::init_with_level(log_level).unwrap(); diff --git a/cfn-guard/src/util.rs b/cfn-guard/src/util.rs index 2a0f06b05..f474ea01c 100644 --- a/cfn-guard/src/util.rs +++ b/cfn-guard/src/util.rs @@ -14,9 +14,7 @@ lazy_static! { Regex::new(r"[:=]\s*([fF]alse|[tT]rue)\s*([,}]+|$)").unwrap(); } pub fn fix_stringified_bools(fstr: &str) -> String { - let after = STRINGIFIED_BOOLS.replace_all(fstr, |caps: &Captures| { - format!("{}", &caps[0].to_lowercase()) - }); + let after = STRINGIFIED_BOOLS.replace_all(fstr, |caps: &Captures| caps[0].to_lowercase()); after.to_string() } @@ -153,20 +151,19 @@ pub fn expand_wildcard_props( &address, &accumulator ); - let mut segments = address.split("*").collect::>(); + let mut segments = address.split('*').collect::>(); trace!("Segments are {:#?}", &segments); let segment = segments.remove(0); trace!("Processing segment {:#?}", &segment); if segment != "" { let mut expanded_props: Vec = vec![]; - let s = segment.trim_end_matches(".").trim_start_matches("."); - let steps = s.split(".").collect::>(); + let s = segment.trim_end_matches('.').trim_start_matches('.'); + let steps = s.split('.').collect::>(); match get_resource_prop_value(props, &steps) { Ok(v) => match v.as_array() { Some(result_array) => { trace!("Value is an array"); - let mut counter = 0; - for r in result_array { + for (counter, r) in result_array.iter().enumerate() { trace!("Counter is {:#?}", counter); let next_segment = segments.join("*"); trace!("next_segment is {:#?}", &next_segment); @@ -176,7 +173,6 @@ pub fn expand_wildcard_props( Some(result) => expanded_props.append(&mut result.clone()), None => return None, } - counter += 1; } } None => expanded_props.push(format!("{}{}", accumulator, segment)), @@ -194,8 +190,6 @@ mod tests { #[cfg(test)] use crate::util::expand_wildcard_props; #[cfg(test)] - use serde_yaml; - #[cfg(test)] use std::collections::HashMap; #[test] From a3295e39ccb422a6e96bc7186282144848d5adda Mon Sep 17 00:00:00 2001 From: nathanmc Date: Fri, 19 Jun 2020 15:53:03 -0400 Subject: [PATCH 10/21] Make the runtime arguments more conspicuous --- cfn-guard-rulegen/README.md | 42 ++++++++++++++++++------------------- cfn-guard/README.md | 40 +++++++++++++++-------------------- 2 files changed, 38 insertions(+), 44 deletions(-) diff --git a/cfn-guard-rulegen/README.md b/cfn-guard-rulegen/README.md index 39e7fdce4..7a9170515 100644 --- a/cfn-guard-rulegen/README.md +++ b/cfn-guard-rulegen/README.md @@ -2,6 +2,27 @@ A CLI tool to automatically generate [CloudFormation Guard](https://github.com/aws-cloudformation/cloudformation-guard) rules from CloudFormation Templates. +### Runtime Arguments + +Rulegen uses the Rust Clap library to parse arguments. Its `--help` output will show you what options are available: + +``` +$> cfn-guard-rulegen --help + +CloudFormation Guard RuleGen +Generate CloudFormation Guard rules from a CloudFormation template + +USAGE: + cfn-guard-rulegen [FLAGS]