From 8c48b082cf300e2fddf4f66472a7337b5fa7037b Mon Sep 17 00:00:00 2001 From: nathanmc Date: Wed, 19 Aug 2020 18:36:42 -0400 Subject: [PATCH 01/22] Cargo bump minor to 0.7.0 --- cfn-guard-lambda/Cargo.lock | 2 +- cfn-guard-lambda/Cargo.toml | 2 +- cfn-guard-rulegen-lambda/Cargo.lock | 4 ++-- cfn-guard-rulegen-lambda/Cargo.toml | 2 +- cfn-guard-rulegen/Cargo.toml | 2 +- cfn-guard/Cargo.toml | 2 +- 6 files changed, 7 insertions(+), 7 deletions(-) diff --git a/cfn-guard-lambda/Cargo.lock b/cfn-guard-lambda/Cargo.lock index 8f9091765..4079e3701 100644 --- a/cfn-guard-lambda/Cargo.lock +++ b/cfn-guard-lambda/Cargo.lock @@ -88,7 +88,7 @@ checksum = "4785bdd1c96b2a846b2bd7cc02e86b6b3dbf14e7e53446c4f54c92a361040822" [[package]] name = "cfn-guard" -version = "0.6.0" +version = "0.7.0" dependencies = [ "clap", "lazy_static", diff --git a/cfn-guard-lambda/Cargo.toml b/cfn-guard-lambda/Cargo.toml index a9130376f..fc8863cf2 100644 --- a/cfn-guard-lambda/Cargo.toml +++ b/cfn-guard-lambda/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "cfn-guard-lambda" -version = "0.6.0" +version = "0.7.0" edition = "2018" [dependencies] diff --git a/cfn-guard-rulegen-lambda/Cargo.lock b/cfn-guard-rulegen-lambda/Cargo.lock index 6cb6134a2..1177b83d8 100644 --- a/cfn-guard-rulegen-lambda/Cargo.lock +++ b/cfn-guard-rulegen-lambda/Cargo.lock @@ -89,7 +89,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" [[package]] name = "cfn-guard-rulegen" -version = "0.6.0" +version = "0.7.0" dependencies = [ "clap 2.33.0 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.4.7 (registry+https://github.com/rust-lang/crates.io-index)", @@ -103,7 +103,7 @@ dependencies = [ name = "cfn-guard-rulegen-lambda" version = "0.6.0" dependencies = [ - "cfn-guard-rulegen 0.6.0", + "cfn-guard-rulegen 0.7.0", "lambda_runtime 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.4.7 (registry+https://github.com/rust-lang/crates.io-index)", "serde 1.0.95 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/cfn-guard-rulegen-lambda/Cargo.toml b/cfn-guard-rulegen-lambda/Cargo.toml index d995eb978..1bb3caca8 100644 --- a/cfn-guard-rulegen-lambda/Cargo.toml +++ b/cfn-guard-rulegen-lambda/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "cfn-guard-rulegen-lambda" -version = "0.6.0" +version = "0.7.0" edition = "2018" [dependencies] diff --git a/cfn-guard-rulegen/Cargo.toml b/cfn-guard-rulegen/Cargo.toml index fbc22b4be..c1d27ed9b 100644 --- a/cfn-guard-rulegen/Cargo.toml +++ b/cfn-guard-rulegen/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "cfn-guard-rulegen" -version = "0.6.0" +version = "0.7.0" edition = "2018" [dependencies] diff --git a/cfn-guard/Cargo.toml b/cfn-guard/Cargo.toml index 86aade01f..49df84671 100644 --- a/cfn-guard/Cargo.toml +++ b/cfn-guard/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "cfn-guard" -version = "0.6.0" +version = "0.7.0" edition = "2018" [dependencies] From 21f58ec5b5d1095a5fcfe8ae09ae588cb674497c Mon Sep 17 00:00:00 2001 From: nathanmc Date: Wed, 19 Aug 2020 18:40:08 -0400 Subject: [PATCH 02/22] Merge cfn-guard and cfn-guard-rulegen binaries into cfn-guard as two subcommands: - cfn-guard check ... - cfn-guard rulegen ... --- cfn-guard-lambda/Cargo.lock | 15 ++- cfn-guard-rulegen-lambda/Cargo.lock | 2 +- cfn-guard-rulegen/Cargo.lock | 2 +- cfn-guard/Cargo.lock | 15 ++- cfn-guard/Cargo.toml | 1 + cfn-guard/src/main.rs | 156 +++++++++++++++++----------- 6 files changed, 127 insertions(+), 64 deletions(-) diff --git a/cfn-guard-lambda/Cargo.lock b/cfn-guard-lambda/Cargo.lock index 4079e3701..dd651e4e7 100644 --- a/cfn-guard-lambda/Cargo.lock +++ b/cfn-guard-lambda/Cargo.lock @@ -90,6 +90,7 @@ checksum = "4785bdd1c96b2a846b2bd7cc02e86b6b3dbf14e7e53446c4f54c92a361040822" name = "cfn-guard" version = "0.7.0" dependencies = [ + "cfn-guard-rulegen", "clap", "lazy_static", "log", @@ -102,7 +103,7 @@ dependencies = [ [[package]] name = "cfn-guard-lambda" -version = "0.6.0" +version = "0.7.0" dependencies = [ "cfn-guard", "lambda_runtime", @@ -113,6 +114,18 @@ dependencies = [ "simple_logger", ] +[[package]] +name = "cfn-guard-rulegen" +version = "0.7.0" +dependencies = [ + "clap", + "log", + "serde", + "serde_json", + "serde_yaml", + "simple_logger", +] + [[package]] name = "chrono" version = "0.4.11" diff --git a/cfn-guard-rulegen-lambda/Cargo.lock b/cfn-guard-rulegen-lambda/Cargo.lock index 1177b83d8..49644d9eb 100644 --- a/cfn-guard-rulegen-lambda/Cargo.lock +++ b/cfn-guard-rulegen-lambda/Cargo.lock @@ -101,7 +101,7 @@ dependencies = [ [[package]] name = "cfn-guard-rulegen-lambda" -version = "0.6.0" +version = "0.7.0" dependencies = [ "cfn-guard-rulegen 0.7.0", "lambda_runtime 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/cfn-guard-rulegen/Cargo.lock b/cfn-guard-rulegen/Cargo.lock index 18a368308..b25f3064f 100644 --- a/cfn-guard-rulegen/Cargo.lock +++ b/cfn-guard-rulegen/Cargo.lock @@ -40,7 +40,7 @@ checksum = "4785bdd1c96b2a846b2bd7cc02e86b6b3dbf14e7e53446c4f54c92a361040822" [[package]] name = "cfn-guard-rulegen" -version = "0.6.0" +version = "0.7.0" dependencies = [ "clap", "log", diff --git a/cfn-guard/Cargo.lock b/cfn-guard/Cargo.lock index a4e46b5e9..654102138 100644 --- a/cfn-guard/Cargo.lock +++ b/cfn-guard/Cargo.lock @@ -49,8 +49,9 @@ checksum = "4785bdd1c96b2a846b2bd7cc02e86b6b3dbf14e7e53446c4f54c92a361040822" [[package]] name = "cfn-guard" -version = "0.6.0" +version = "0.7.0" dependencies = [ + "cfn-guard-rulegen", "clap", "lazy_static", "log", @@ -61,6 +62,18 @@ dependencies = [ "simple_logger", ] +[[package]] +name = "cfn-guard-rulegen" +version = "0.7.0" +dependencies = [ + "clap", + "log", + "serde", + "serde_json", + "serde_yaml", + "simple_logger", +] + [[package]] name = "chrono" version = "0.4.11" diff --git a/cfn-guard/Cargo.toml b/cfn-guard/Cargo.toml index 49df84671..b666c2faa 100644 --- a/cfn-guard/Cargo.toml +++ b/cfn-guard/Cargo.toml @@ -12,3 +12,4 @@ log = "0.4.6" clap = "2.33.0" simple_logger = "1.3.0" regex = "1.1.9" +cfn-guard-rulegen = {path = "../cfn-guard-rulegen" } diff --git a/cfn-guard/src/main.rs b/cfn-guard/src/main.rs index 772b2f440..6072d174a 100644 --- a/cfn-guard/src/main.rs +++ b/cfn-guard/src/main.rs @@ -5,46 +5,64 @@ use std::process; extern crate log; extern crate lazy_static; extern crate simple_logger; -use clap::{crate_version, App, Arg}; +use cfn_guard_rulegen; +use clap::{crate_version, App, AppSettings, Arg, SubCommand}; use log::Level; fn main() { let matches = App::new("CloudFormation Guard") + .setting(AppSettings::ArgRequiredElseHelp) .version(crate_version!()) - .about("Check CloudFormation templates against rules") - .arg( - Arg::with_name("template") - .short("t") - .long("template") - .value_name("TEMPLATE_FILE") - .help("CloudFormation Template") - .required(true), + .subcommand( + SubCommand::with_name("check") + .about("Check CloudFormation templates against rules") + .arg( + Arg::with_name("template") + .short("t") + .long("template") + .value_name("TEMPLATE_FILE") + .help("CloudFormation Template") + .required(true), + ) + .arg( + Arg::with_name("rule_set") + .short("r") + .long("rule_set") + .value_name("RULE_SET_FILE") + .help("Rules to check the template against") + .required(true), + ) + .arg( + Arg::with_name("warn-only") + .short("w") + .long("warn_only") + .help( + "Show results but return an exit code of 0 regardless of rule violations", + ), + ) + .arg( + Arg::with_name("strict-checks") + .short("s") + .long("strict-checks") + .help("Fail resources if they're missing the property that a rule checks"), + ) + .arg( + Arg::with_name("v") + .short("v") + .multiple(true) + .help("Sets the level of verbosity - add v's to increase output"), + ), ) - .arg( - Arg::with_name("rule_set") - .short("r") - .long("rule_set") - .value_name("RULE_SET_FILE") - .help("Rules to check the template against") - .required(true), - ) - .arg( - Arg::with_name("v") - .short("v") - .multiple(true) - .help("Sets the level of verbosity - add v's to increase output"), - ) - .arg( - Arg::with_name("warn-only") - .short("w") - .long("warn_only") - .help("Show results but return an exit code of 0 regardless of rule violations"), - ) - .arg( - Arg::with_name("strict-checks") - .short("s") - .long("strict-checks") - .help("Fail resources if they're missing the property that a rule checks"), + .subcommand( + SubCommand::with_name("rulegen") + .about("Autogenerate rules from an existing CloudFormation template") + .arg(Arg::with_name("TEMPLATE").index(1).required(true)) + .arg( + Arg::with_name("v") + .short("v") + .multiple(true) + .help("Sets the level of verbosity - add v's to increase output"), + ), ) .get_matches(); @@ -59,35 +77,53 @@ fn main() { debug!("Parameters are {:#?}", matches); - let template_file = matches.value_of("template").unwrap(); - let rule_set_file = matches.value_of("rule_set").unwrap(); + if let Some(matches) = matches.subcommand_matches("rulegen") { + let template_file = matches.value_of("TEMPLATE").unwrap(); + let mut result = cfn_guard_rulegen::run(template_file).unwrap_or_else(|err| { + println!("Problem generating rules: {}", err); + process::exit(1); + }); + + if !result.is_empty() { + result.sort(); + for res in result.iter() { + println!("{}", res); + } + } + process::exit(0); + } else { + if let Some(matches) = matches.subcommand_matches("check") { + let template_file = matches.value_of("template").unwrap(); + let rule_set_file = matches.value_of("rule_set").unwrap(); - info!( - "CloudFormation Guard is checking the template '{}' against the rules in '{}'", - &template_file, &rule_set_file - ); + info!( + "CloudFormation Guard is checking the template '{}' against the rules in '{}'", + &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| { - println!("Problem checking template: {}", err); - process::exit(1); - }); + 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); + }); - if !result.is_empty() { - for res in result.iter() { - println!("{}", res); + if !result.is_empty() { + for res in result.iter() { + println!("{}", res); + } + println!("Number of failures: {}", result.len()); + if matches.is_present("warn-only") { + process::exit(0); + } else { + process::exit(exit_code as i32); + } + } else { + info!("All CloudFormation resources passed"); + } } - println!("Number of failures: {}", result.len()); - if matches.is_present("warn-only") { - process::exit(0); - } else { - process::exit(exit_code as i32); - } - } else { - info!("All CloudFormation resources passed"); } } From 168a6725cfad6c795eafb77d5c50f76247d17b99 Mon Sep 17 00:00:00 2001 From: nathanmc Date: Wed, 19 Aug 2020 18:44:46 -0400 Subject: [PATCH 03/22] Rename cfn-guard-lambda "strict_checks" to "strictChecks" to match the other fields --- cfn-guard-lambda/Makefile | 6 +++--- cfn-guard-lambda/src/main.rs | 1 + 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/cfn-guard-lambda/Makefile b/cfn-guard-lambda/Makefile index 455b19bdb..8ceb1d290 100644 --- a/cfn-guard-lambda/Makefile +++ b/cfn-guard-lambda/Makefile @@ -5,7 +5,7 @@ project_name = cfn-guard-lambda role_arn := ${CFN_GUARD_LAMBDA_ROLE_ARN} request_payload_fail = '{ "template": "{\n \"Resources\": {\n \"NewVolume\" : {\n \"Type\" : \"AWS::EC2::Volume\",\n \"Properties\" : {\n \"Size\" : 100,\n \"Encrypted\": true,\n \"AvailabilityZone\" : \"us-east-1b\"\n }\n },\n \"NewVolume2\" : {\n \"Type\" : \"AWS::EC2::Volume\",\n \"Properties\" : {\n \"Size\" : 99,\n \"Encrypted\": true,\n \"AvailabilityZone\" : \"us-east-1b\"\n }\n } }\n}",\ "ruleSet": "let require_encryption = true\nlet disallowed_azs = [us-east-1a,us-east-1b,us-east-1c]\n\nAWS::EC2::Volume AvailabilityZone NOT_IN %disallowed_azs\nAWS::EC2::Volume Encrypted != %require_encryption\nAWS::EC2::Volume Size == 101 |OR| AWS::EC2::Volume Size == 99\nAWS::IAM::Role AssumeRolePolicyDocument.Version == 2012-10-18\nAWS::EC2::Volume Lorem == true\nAWS::EC2::Volume Encrypted == %ipsum\nAWS::EC2::Volume AvailabilityZone != /us-east-.*/",\ - "strict_checks": true}' + "strictChecks": true}' #====================================================================== # Request Payload Fail: @@ -49,7 +49,7 @@ request_payload_fail = '{ "template": "{\n \"Resources\": {\n \"NewVol request_payload_pass = '{ "template": "{\n \"Resources\": {\n \"NewVolume\" : {\n \"Type\" : \"AWS::EC2::Volume\",\n \"Properties\" : {\n \"Size\" : 100,\n \"Encrypted\": true,\n \"AvailabilityZone\" : \"us-east-1b\"\n }\n },\n \"NewVolume2\" : {\n \"Type\" : \"AWS::EC2::Volume\",\n \"Properties\" : {\n \"Size\" : 99,\n \"Encrypted\": true,\n \"AvailabilityZone\" : \"us-east-1b\"\n }\n } }\n}",\ "ruleSet": "let require_encryption = true",\ - "strict_checks": true}' + "strictChecks": true}' #====================================================================== # Request Payload Pass @@ -83,7 +83,7 @@ request_payload_pass = '{ "template": "{\n \"Resources\": {\n \"NewVol request_payload_err = '{ "template": "{\n \"Resources\": \n \"NewVolume\" : {\n \"Type\" : \"AWS::EC2::Volume\",\n \"Properties\" : {\n \"Size\" : 100,\n \"Encrypted\": true,\n \"AvailabilityZone\" : \"us-east-1b\"\n }\n },\n \"NewVolume2\" : {\n \"Type\" : \"AWS::EC2::Volume\",\n \"Properties\" : {\n \"Size\" : 99,\n \"Encrypted\": true,\n \"AvailabilityZone\" : \"us-east-1b\"\n }\n } }\n}",\ "ruleSet": "let require_encryption = true",\ - "strict_checks": true}' + "strictChecks": true}' #====================================================================== # Request Payload Fail diff --git a/cfn-guard-lambda/src/main.rs b/cfn-guard-lambda/src/main.rs index 442e0ecfa..616e9b99b 100644 --- a/cfn-guard-lambda/src/main.rs +++ b/cfn-guard-lambda/src/main.rs @@ -14,6 +14,7 @@ struct CustomEvent { template: String, #[serde(rename = "ruleSet")] rule_set: String, + #[serde(rename = "strictChecks")] strict_checks: bool, } From 338932f8110d1e8dd19470f9539835667099201c Mon Sep 17 00:00:00 2001 From: nathanmc Date: Thu, 20 Aug 2020 13:11:28 -0400 Subject: [PATCH 04/22] Rename lambda "exit_status" to "exitStatus" to make interface consistent --- cfn-guard-lambda/src/main.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/cfn-guard-lambda/src/main.rs b/cfn-guard-lambda/src/main.rs index 616e9b99b..072b71c3f 100644 --- a/cfn-guard-lambda/src/main.rs +++ b/cfn-guard-lambda/src/main.rs @@ -21,6 +21,7 @@ struct CustomEvent { #[derive(Serialize)] struct CustomOutput { message: Vec, + #[serde(rename = "exitStatus")] exit_status: String, } From d5c1ea34157e84b73fe78a9afc9c5cc59112375e Mon Sep 17 00:00:00 2001 From: nathanmc Date: Thu, 20 Aug 2020 13:27:37 -0400 Subject: [PATCH 05/22] Update lambda README to reflect new request/response field naming in camelCase --- cfn-guard-lambda/README.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/cfn-guard-lambda/README.md b/cfn-guard-lambda/README.md index 42b3fdd32..82e082fab 100644 --- a/cfn-guard-lambda/README.md +++ b/cfn-guard-lambda/README.md @@ -74,7 +74,7 @@ aws lambda update-function-code --function-name cfn-guard-lambda --zip-file file "State": "Active", "LastUpdateStatus": "Successful" } -aws lambda invoke --function-name cfn-guard-lambda --payload '{ "template": "{\n \"Resources\": {\n \"NewVolume\" : {\n \"Type\" : \"AWS::EC2::Volume\",\n \"Properties\" : {\n \"Size\" : 100,\n \"Encrypted\": true,\n \"AvailabilityZone\" : \"us-east-1b\"\n }\n },\n \"NewVolume2\" : {\n \"Type\" : \"AWS::EC2::Volume\",\n \"Properties\" : {\n \"Size\" : 99,\n \"Encrypted\": true,\n \"AvailabilityZone\" : \"us-east-1b\"\n }\n } }\n}", "ruleSet": "let require_encryption = true\nlet disallowed_azs = [us-east-1a,us-east-1b,us-east-1c]\n\nAWS::EC2::Volume AvailabilityZone NOT_IN %disallowed_azs\nAWS::EC2::Volume Encrypted != %require_encryption\nAWS::EC2::Volume Size == 101 |OR| AWS::EC2::Volume Size == 99\nAWS::IAM::Role AssumeRolePolicyDocument.Version == 2012-10-18\nAWS::EC2::Volume Lorem == true\nAWS::EC2::Volume Encrypted == %ipsum\nAWS::EC2::Volume AvailabilityZone != /us-east-.*/", "strict_checks": true}' output.json +aws lambda invoke --function-name cfn-guard-lambda --payload '{ "template": "{\n \"Resources\": {\n \"NewVolume\" : {\n \"Type\" : \"AWS::EC2::Volume\",\n \"Properties\" : {\n \"Size\" : 100,\n \"Encrypted\": true,\n \"AvailabilityZone\" : \"us-east-1b\"\n }\n },\n \"NewVolume2\" : {\n \"Type\" : \"AWS::EC2::Volume\",\n \"Properties\" : {\n \"Size\" : 99,\n \"Encrypted\": true,\n \"AvailabilityZone\" : \"us-east-1b\"\n }\n } }\n}", "ruleSet": "let require_encryption = true\nlet disallowed_azs = [us-east-1a,us-east-1b,us-east-1c]\n\nAWS::EC2::Volume AvailabilityZone NOT_IN %disallowed_azs\nAWS::EC2::Volume Encrypted != %require_encryption\nAWS::EC2::Volume Size == 101 |OR| AWS::EC2::Volume Size == 99\nAWS::IAM::Role AssumeRolePolicyDocument.Version == 2012-10-18\nAWS::EC2::Volume Lorem == true\nAWS::EC2::Volume Encrypted == %ipsum\nAWS::EC2::Volume AvailabilityZone != /us-east-.*/", "strictChecks": true}' output.json { "StatusCode": 200, "ExecutedVersion": "$LATEST" @@ -95,9 +95,9 @@ cat output.json | jq '.' "[NewVolume] failed because it does not contain the required property of [Lorem]", "[NewVolume] failed because there is no value defined for [%ipsum] to check [Encrypted] against" ], - "exit_status": "FAIL" + "exitStatus": "FAIL" } -aws lambda invoke --function-name cfn-guard-lambda --payload '{ "template": "{\n \"Resources\": {\n \"NewVolume\" : {\n \"Type\" : \"AWS::EC2::Volume\",\n \"Properties\" : {\n \"Size\" : 100,\n \"Encrypted\": true,\n \"AvailabilityZone\" : \"us-east-1b\"\n }\n },\n \"NewVolume2\" : {\n \"Type\" : \"AWS::EC2::Volume\",\n \"Properties\" : {\n \"Size\" : 99,\n \"Encrypted\": true,\n \"AvailabilityZone\" : \"us-east-1b\"\n }\n } }\n}", "ruleSet": "let require_encryption = true", "strict_checks": true}' output.json +aws lambda invoke --function-name cfn-guard-lambda --payload '{ "template": "{\n \"Resources\": {\n \"NewVolume\" : {\n \"Type\" : \"AWS::EC2::Volume\",\n \"Properties\" : {\n \"Size\" : 100,\n \"Encrypted\": true,\n \"AvailabilityZone\" : \"us-east-1b\"\n }\n },\n \"NewVolume2\" : {\n \"Type\" : \"AWS::EC2::Volume\",\n \"Properties\" : {\n \"Size\" : 99,\n \"Encrypted\": true,\n \"AvailabilityZone\" : \"us-east-1b\"\n }\n } }\n}", "ruleSet": "let require_encryption = true", "strictChecks": true}' output.json { "StatusCode": 200, "ExecutedVersion": "$LATEST" @@ -105,9 +105,9 @@ aws lambda invoke --function-name cfn-guard-lambda --payload '{ "template": "{\n cat output.json | jq '.' { "message": [], - "exit_status": "PASS" + "exitStatus": "PASS" } -aws lambda invoke --function-name cfn-guard-lambda --payload '{ "template": "{\n \"Resources\": \n \"NewVolume\" : {\n \"Type\" : \"AWS::EC2::Volume\",\n \"Properties\" : {\n \"Size\" : 100,\n \"Encrypted\": true,\n \"AvailabilityZone\" : \"us-east-1b\"\n }\n },\n \"NewVolume2\" : {\n \"Type\" : \"AWS::EC2::Volume\",\n \"Properties\" : {\n \"Size\" : 99,\n \"Encrypted\": true,\n \"AvailabilityZone\" : \"us-east-1b\"\n }\n } }\n}", "ruleSet": "let require_encryption = true", "strict_checks": true}' output.json +aws lambda invoke --function-name cfn-guard-lambda --payload '{ "template": "{\n \"Resources\": \n \"NewVolume\" : {\n \"Type\" : \"AWS::EC2::Volume\",\n \"Properties\" : {\n \"Size\" : 100,\n \"Encrypted\": true,\n \"AvailabilityZone\" : \"us-east-1b\"\n }\n },\n \"NewVolume2\" : {\n \"Type\" : \"AWS::EC2::Volume\",\n \"Properties\" : {\n \"Size\" : 99,\n \"Encrypted\": true,\n \"AvailabilityZone\" : \"us-east-1b\"\n }\n } }\n}", "ruleSet": "let require_encryption = true", "strictChecks": true}' output.json { "StatusCode": 200, "ExecutedVersion": "$LATEST" @@ -117,7 +117,7 @@ cat output.json | jq '.' "message": [ "ERROR: Template file format was unreadable as json or yaml: while parsing a flow mapping, did not find expected ',' or '}' at line 3 column 21" ], - "exit_status": "ERR" + "exitStatus": "ERR" } ``` ## Calling the Lambda Function @@ -125,7 +125,7 @@ cat output.json | jq '.' Requests to `cfn-guard-lambda` require the following 3 fields: * `template` - The string version of the YAML or JSON CloudFormation Template * `ruleSet` - The string version of the rule set file -* `strict_checks` - A boolean indicating whether to apply [strict checks](../cfn-guard/README.md#about) +* `strictChecks` - A boolean indicating whether to apply [strict checks](../cfn-guard/README.md#about) #### Example There are example payloads in the [Makefile](Makefile). Here's one we use to test a rule set that should not pass: @@ -133,7 +133,7 @@ There are example payloads in the [Makefile](Makefile). Here's one we use to te ``` request_payload_fail = '{ "template": "{\n \"Resources\": {\n \"NewVolume\" : {\n \"Type\" : \"AWS::EC2::Volume\",\n \"Properties\" : {\n \"Size\" : 100,\n \"Encrypted\": true,\n \"AvailabilityZone\" : \"us-east-1b\"\n }\n },\n \"NewVolume2\" : {\n \"Type\" : \"AWS::EC2::Volume\",\n \"Properties\" : {\n \"Size\" : 99,\n \"Encrypted\": true,\n \"AvailabilityZone\" : \"us-east-1b\"\n }\n } }\n}",\ "ruleSet": "let require_encryption = true\nlet disallowed_azs = [us-east-1a,us-east-1b,us-east-1c]\n\nAWS::EC2::Volume AvailabilityZone NOT_IN %disallowed_azs\nAWS::EC2::Volume Encrypted != %require_encryption\nAWS::EC2::Volume Size == 101 |OR| AWS::EC2::Volume Size == 99\nAWS::IAM::Role AssumeRolePolicyDocument.Version == 2012-10-18\nAWS::EC2::Volume Lorem == true\nAWS::EC2::Volume Encrypted == %ipsum\nAWS::EC2::Volume AvailabilityZone != /us-east-.*/",\ - "strict_checks": true}' + "strictChecks": true}' #====================================================================== # Request Payload Fail: From d96bbc07cfe344c26ea5aedfb12443da592a17cf Mon Sep 17 00:00:00 2001 From: nathanmc Date: Fri, 21 Aug 2020 12:08:14 -0400 Subject: [PATCH 06/22] Fix source file copyright headers --- NOTICE | 1 + cfn-guard-lambda/src/main.rs | 3 ++- cfn-guard-rulegen-lambda/src/main.rs | 3 ++- cfn-guard-rulegen/src/lib.rs | 3 ++- cfn-guard-rulegen/src/main.rs | 3 ++- cfn-guard/src/guard_types.rs | 3 ++- cfn-guard/src/lib.rs | 3 ++- cfn-guard/src/main.rs | 3 ++- cfn-guard/src/parser.rs | 3 ++- cfn-guard/src/util.rs | 3 ++- cfn-guard/tests/functional.rs | 3 ++- 11 files changed, 21 insertions(+), 10 deletions(-) diff --git a/NOTICE b/NOTICE index 616fc5889..ba8da6855 100644 --- a/NOTICE +++ b/NOTICE @@ -1 +1,2 @@ +CloudFormation Guard Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. diff --git a/cfn-guard-lambda/src/main.rs b/cfn-guard-lambda/src/main.rs index 072b71c3f..b2da516d6 100644 --- a/cfn-guard-lambda/src/main.rs +++ b/cfn-guard-lambda/src/main.rs @@ -1,4 +1,5 @@ -// © 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. +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 use std::error::Error; diff --git a/cfn-guard-rulegen-lambda/src/main.rs b/cfn-guard-rulegen-lambda/src/main.rs index 7ae4b01e3..faa93639e 100644 --- a/cfn-guard-rulegen-lambda/src/main.rs +++ b/cfn-guard-rulegen-lambda/src/main.rs @@ -1,4 +1,5 @@ -// © 2019 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. +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 use std::error::Error; use cfn_guard_rulegen; diff --git a/cfn-guard-rulegen/src/lib.rs b/cfn-guard-rulegen/src/lib.rs index 4670649f9..146834dbb 100644 --- a/cfn-guard-rulegen/src/lib.rs +++ b/cfn-guard-rulegen/src/lib.rs @@ -1,4 +1,5 @@ -// © 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. +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 use log::{self, debug, error, info, trace}; use serde_json::Value; diff --git a/cfn-guard-rulegen/src/main.rs b/cfn-guard-rulegen/src/main.rs index b13499621..925521175 100644 --- a/cfn-guard-rulegen/src/main.rs +++ b/cfn-guard-rulegen/src/main.rs @@ -1,4 +1,5 @@ -// © 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. +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 use std::process; #[macro_use] diff --git a/cfn-guard/src/guard_types.rs b/cfn-guard/src/guard_types.rs index 4017b92e5..fea506598 100644 --- a/cfn-guard/src/guard_types.rs +++ b/cfn-guard/src/guard_types.rs @@ -1,4 +1,5 @@ -// © 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. +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 // Structs, Enums and Impls pub mod enums { diff --git a/cfn-guard/src/lib.rs b/cfn-guard/src/lib.rs index 3e06a911f..e6677d57e 100644 --- a/cfn-guard/src/lib.rs +++ b/cfn-guard/src/lib.rs @@ -1,4 +1,5 @@ -// © 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. +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 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 6072d174a..691103211 100644 --- a/cfn-guard/src/main.rs +++ b/cfn-guard/src/main.rs @@ -1,4 +1,5 @@ -// © 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. +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 use std::process; #[macro_use] diff --git a/cfn-guard/src/parser.rs b/cfn-guard/src/parser.rs index 312750b47..d7fe4d096 100644 --- a/cfn-guard/src/parser.rs +++ b/cfn-guard/src/parser.rs @@ -1,4 +1,5 @@ -// © 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. +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 use std::collections::{HashMap, HashSet}; use std::env; diff --git a/cfn-guard/src/util.rs b/cfn-guard/src/util.rs index 925d736e6..0ff29df68 100644 --- a/cfn-guard/src/util.rs +++ b/cfn-guard/src/util.rs @@ -1,4 +1,5 @@ -// © 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. +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 use crate::{enums, structs}; use lazy_static::lazy_static; diff --git a/cfn-guard/tests/functional.rs b/cfn-guard/tests/functional.rs index 88929d7ff..45cdc40e8 100644 --- a/cfn-guard/tests/functional.rs +++ b/cfn-guard/tests/functional.rs @@ -1,4 +1,5 @@ -// © 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. +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 // Tests use cfn_guard; From 499044b9d77fb692725d3655688068d89edcd0dc Mon Sep 17 00:00:00 2001 From: nathanmc Date: Fri, 21 Aug 2020 19:15:50 -0400 Subject: [PATCH 07/22] Fix logging for sub-commands --- cfn-guard/src/main.rs | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/cfn-guard/src/main.rs b/cfn-guard/src/main.rs index 691103211..b2f34c49e 100644 --- a/cfn-guard/src/main.rs +++ b/cfn-guard/src/main.rs @@ -67,18 +67,17 @@ fn main() { ) .get_matches(); - let log_level = match matches.occurrences_of("v") { - 0 => Level::Error, - 1 => Level::Info, - 2 => Level::Debug, - _ => Level::Trace, - }; - - simple_logger::init_with_level(log_level).unwrap(); + if let Some(matches) = matches.subcommand_matches("rulegen") { + let log_level = match matches.occurrences_of("v") { + 0 => Level::Error, + 1 => Level::Info, + 2 => Level::Debug, + _ => Level::Trace, + }; - debug!("Parameters are {:#?}", matches); + simple_logger::init_with_level(log_level).unwrap(); - if let Some(matches) = matches.subcommand_matches("rulegen") { + debug!("Parameters are {:#?}", matches); let template_file = matches.value_of("TEMPLATE").unwrap(); let mut result = cfn_guard_rulegen::run(template_file).unwrap_or_else(|err| { println!("Problem generating rules: {}", err); @@ -94,6 +93,16 @@ fn main() { process::exit(0); } else { if let Some(matches) = matches.subcommand_matches("check") { + let log_level = match matches.occurrences_of("v") { + 0 => Level::Error, + 1 => Level::Info, + 2 => Level::Debug, + _ => Level::Trace, + }; + + simple_logger::init_with_level(log_level).unwrap(); + + debug!("Parameters are {:#?}", matches); let template_file = matches.value_of("template").unwrap(); let rule_set_file = matches.value_of("rule_set").unwrap(); From c3064d5003d96f809e84eb52989cf4d88ae6ba28 Mon Sep 17 00:00:00 2001 From: nathanmc Date: Fri, 21 Aug 2020 21:07:26 -0400 Subject: [PATCH 08/22] add basic pipe operator and functional test --- cfn-guard/src/lib.rs | 6 ++- cfn-guard/src/parser.rs | 2 +- cfn-guard/src/util.rs | 22 ++++++---- cfn-guard/tests/functional.rs | 46 +++++++++++++++++++++ cfn-guard/tests/pipe-operator-template.yaml | 24 +++++++++++ 5 files changed, 91 insertions(+), 9 deletions(-) create mode 100644 cfn-guard/tests/pipe-operator-template.yaml diff --git a/cfn-guard/src/lib.rs b/cfn-guard/src/lib.rs index e6677d57e..c8cf24ab3 100644 --- a/cfn-guard/src/lib.rs +++ b/cfn-guard/src/lib.rs @@ -421,7 +421,11 @@ fn apply_rule_operation( enums::OpCode::Require => { match rule.rule_vtype { enums::RValueType::Value | enums::RValueType::Variable => { - if util::format_value(&val) == util::strip_ws_nl(rule_val.to_string()) { + let f_template_val = util::format_value(&val); + trace!("f_template_val is {}", f_template_val); + let f_rule_val = util::strip_ws_nl(rule_val.to_string()); + trace!("f_rule_val is {}", f_rule_val); + if f_template_val == f_rule_val { info!("Result: PASS"); None } else { diff --git a/cfn-guard/src/parser.rs b/cfn-guard/src/parser.rs index d7fe4d096..06a10b54f 100644 --- a/cfn-guard/src/parser.rs +++ b/cfn-guard/src/parser.rs @@ -17,7 +17,7 @@ use lazy_static::lazy_static; // 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\S+) +(?P.+)").unwrap(); - static ref RULE_REG: Regex = Regex::new(r"^(?P\S+) +(?P[\w\.\*]+) +(?P==|!=|<|>|<=|>=|IN|NOT_IN) +(?P[^\n\r]+)").unwrap(); + static ref RULE_REG: Regex = Regex::new(r"^(?P\S+) +(?P[\w\.\|\*]+) +(?P==|!=|<|>|<=|>=|IN|NOT_IN) +(?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*) (==|IN) (.+)").unwrap(); static ref RULE_WITH_OPTIONAL_MESSAGE_REG: Regex = Regex::new( diff --git a/cfn-guard/src/util.rs b/cfn-guard/src/util.rs index 0ff29df68..0add2fa83 100644 --- a/cfn-guard/src/util.rs +++ b/cfn-guard/src/util.rs @@ -82,14 +82,22 @@ pub fn get_resource_prop_value(props: &Value, field: &[&str]) -> Result() { + let next_field = field_list.remove(0); //<= Exit here on empty element + match next_field { + "" => return Ok(props.clone()), + "." => get_resource_prop_value(&props, &field_list), + "|" => { + let sub_structure: HashMap = + serde_json::from_str(&props.as_str().unwrap()).unwrap(); + trace!("sub_structure is {:#?}", sub_structure); + let next_field = field_list.remove(0); + trace!("next_field is {}", next_field); + get_resource_prop_value(&sub_structure[next_field], &field_list) + } + _ => match next_field.parse::() { Ok(n) => { trace!( "next_field is {:?} and field_list is now {:?}", @@ -125,7 +133,7 @@ pub fn get_resource_prop_value(props: &Value, field: &[&str]) -> Result Err(next_field.to_string()), } } - } + }, } } diff --git a/cfn-guard/tests/functional.rs b/cfn-guard/tests/functional.rs index 45cdc40e8..8a014571a 100644 --- a/cfn-guard/tests/functional.rs +++ b/cfn-guard/tests/functional.rs @@ -1649,4 +1649,50 @@ AWS::EC2::Volume Size == 101 |OR| AWS::EC2::Volume Size == 99"#, (vec![], 0) ) } + #[test] + fn test_pipe_operator() { + let template_contents = fs::read_to_string("tests/pipe-operator-template.yaml") + .unwrap_or_else(|err| format!("{}", err)); + + let mut rules_file_contents = String::from( + r#"AWS::IAM::Role AssumeRolePolicyDocument.|.Statement.0 == { "Effect": "Allow", "Principal": { "Service": [ "notlambda.amazonaws.com" ] } } "#, + ); + + assert_eq!( + cfn_guard::run_check(&template_contents, &rules_file_contents, true).unwrap(), + (vec![], 0) + ); + + rules_file_contents = String::from( + r#"AWS::IAM::Role AssumeRolePolicyDocument.|.Statement.* == { "Effect": "Allow", "Principal": { "Service": [ "notlambda.amazonaws.com" ] } } "#, + ); + assert_eq!( + cfn_guard::run_check(&template_contents, &rules_file_contents, true).unwrap(), + (vec![], 0) + ); + + rules_file_contents = String::from( + r#"AWS::IAM::Role AssumeRolePolicyDocument.|.Statement.0 == {"Effect": "Allow","Principal":{"Service":["notlambda.amazonaws.com"]}}"#, + ); + assert_eq!( + cfn_guard::run_check(&template_contents, &rules_file_contents, true).unwrap(), + (vec![], 0) + ); + + rules_file_contents = String::from( + r#"AWS::IAM::Role AssumeRolePolicyDocument.|.Statement.0.Effect == Allow"#, + ); + assert_eq!( + cfn_guard::run_check(&template_contents, &rules_file_contents, true).unwrap(), + (vec![], 0) + ); + + rules_file_contents = String::from( + r#"AWS::IAM::Role AssumeRolePolicyDocument.|.Statement.*.Effect == Allow"#, + ); + assert_eq!( + cfn_guard::run_check(&template_contents, &rules_file_contents, true).unwrap(), + (vec![], 0) + ) + } } diff --git a/cfn-guard/tests/pipe-operator-template.yaml b/cfn-guard/tests/pipe-operator-template.yaml new file mode 100644 index 000000000..5f32e4dca --- /dev/null +++ b/cfn-guard/tests/pipe-operator-template.yaml @@ -0,0 +1,24 @@ +AWSTemplateFormatVersion: "2010-09-09" +Description: "My API Gateway and Lambda function" + +Parameters: + lambdaFunctionName: + Type: "String" + Default: "governance-lambda" +Resources: + LambdaRoleHelper: + Type: 'AWS::IAM::Role' + Properties: + AssumeRolePolicyDocument: | + { + "Statement": [ + { + "Effect": "Allow", + "Principal": { + "Service": [ + "notlambda.amazonaws.com" + ] + } + } + ] + } From 8eedb63721210de8c44eddeb50d3309772b4b92f Mon Sep 17 00:00:00 2001 From: nathanmc Date: Fri, 21 Aug 2020 21:09:01 -0400 Subject: [PATCH 09/22] Replace trim() call with " " -> "" to remove spaces from equality checks --- cfn-guard/src/util.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cfn-guard/src/util.rs b/cfn-guard/src/util.rs index 0add2fa83..20cbfaab0 100644 --- a/cfn-guard/src/util.rs +++ b/cfn-guard/src/util.rs @@ -32,7 +32,7 @@ pub fn format_value(v: &Value) -> String { pub fn strip_ws_nl(v: String) -> String { trace!("Removing spaces and newline characters from '{}'", &v); - v.trim().replace("\n", "") + v.replace("\n", "").replace(" ", "") } pub fn convert_list_var_to_vec(rule_val: &str) -> Vec { From 0259199e37c1df5da1b6450bdb683517f8b50fc2 Mon Sep 17 00:00:00 2001 From: nathanmc Date: Mon, 24 Aug 2020 15:47:21 -0400 Subject: [PATCH 10/22] Move "double unwrap" implementation to proper pattern matching --- cfn-guard/src/util.rs | 32 +++++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/cfn-guard/src/util.rs b/cfn-guard/src/util.rs index 20cbfaab0..b27dedac8 100644 --- a/cfn-guard/src/util.rs +++ b/cfn-guard/src/util.rs @@ -85,18 +85,32 @@ pub fn get_resource_prop_value(props: &Value, field: &[&str]) -> Result return Ok(props.clone()), "." => get_resource_prop_value(&props, &field_list), - "|" => { - let sub_structure: HashMap = - serde_json::from_str(&props.as_str().unwrap()).unwrap(); - trace!("sub_structure is {:#?}", sub_structure); - let next_field = field_list.remove(0); - trace!("next_field is {}", next_field); - get_resource_prop_value(&sub_structure[next_field], &field_list) - } + "|" => match props.as_str() { + Some(p) => match serde_json::from_str::(p) { + Ok(s) => { + trace!("sub structure is {:#?}", s); + if !field_list.is_empty() { + let nf = field_list.remove(0); + trace!("next_field is {}", nf); + match match_props(&s, &nf) { + Ok(v) => { + trace!("next_props is {:#?}", v); + get_resource_prop_value(&v, &field_list) + } + Err(_) => return Err(format!("Invalid address: {}", nf)), + } + } else { + Ok(s) + } + } + Err(e) => return Err(e.to_string()), + }, + None => return Err(format!("Could not convert properties to string")), + }, _ => match next_field.parse::() { Ok(n) => { trace!( From 1f31789ce6726d540d2c76fe549fd34d248703a0 Mon Sep 17 00:00:00 2001 From: nathanmc Date: Mon, 24 Aug 2020 15:48:19 -0400 Subject: [PATCH 11/22] Add pipe-operator functional tests for addresses ending in "|" as well as test failure of negated rules --- cfn-guard/tests/functional.rs | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/cfn-guard/tests/functional.rs b/cfn-guard/tests/functional.rs index 8a014571a..7621084cb 100644 --- a/cfn-guard/tests/functional.rs +++ b/cfn-guard/tests/functional.rs @@ -1693,6 +1693,31 @@ AWS::EC2::Volume Size == 101 |OR| AWS::EC2::Volume Size == 99"#, assert_eq!( cfn_guard::run_check(&template_contents, &rules_file_contents, true).unwrap(), (vec![], 0) + ); + + rules_file_contents = String::from( + r#"AWS::IAM::Role AssumeRolePolicyDocument.| == {"Statement":[{"Effect": "Allow","Principal":{"Service":["notlambda.amazonaws.com"]}}]}"#, + ); + assert_eq!( + cfn_guard::run_check(&template_contents, &rules_file_contents, true).unwrap(), + (vec![], 0) + ); + + rules_file_contents = String::from( + r#"AWS::IAM::Role AssumeRolePolicyDocument.| != {"Statement":[{"Effect": "Allow","Principal":{"Service":["notlambda.amazonaws.com"]}}]} + AWS::IAM::Role AssumeRolePolicyDocument.|.Statement != [{"Effect": "Allow","Principal":{"Service":["notlambda.amazonaws.com"]}}] + AWS::IAM::Role AssumeRolePolicyDocument.|.Statement.0 != {"Effect": "Allow","Principal":{"Service":["notlambda.amazonaws.com"]}} + AWS::IAM::Role AssumeRolePolicyDocument.|.Statement.*.Effect != Allow"#, + ); + assert_eq!( + cfn_guard::run_check(&template_contents, &rules_file_contents, true).unwrap(), + ( + vec![String::from("[LambdaRoleHelper] failed because [AssumeRolePolicyDocument.|.Statement.0.Effect] is [Allow] and that value is not permitted"), + String::from(r#"[LambdaRoleHelper] failed because [AssumeRolePolicyDocument.|.Statement.0] is [{"Effect":"Allow","Principal":{"Service":["notlambda.amazonaws.com"]}}] and that value is not permitted"#), + String::from(r#"[LambdaRoleHelper] failed because [AssumeRolePolicyDocument.|.Statement] is [[{"Effect":"Allow","Principal":{"Service":["notlambda.amazonaws.com"]}}]] and that value is not permitted"#), + String::from(r#"[LambdaRoleHelper] failed because [AssumeRolePolicyDocument.|] is [{"Statement":[{"Effect":"Allow","Principal":{"Service":["notlambda.amazonaws.com"]}}]}] and that value is not permitted"#)], + 2 + ) ) } } From 9dad197fc39f2fdc7f532cf7d8f96da9213c4a59 Mon Sep 17 00:00:00 2001 From: nathanmc Date: Fri, 28 Aug 2020 11:01:29 -0400 Subject: [PATCH 12/22] Add functional test for whitespace display fix --- cfn-guard/tests/functional.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/cfn-guard/tests/functional.rs b/cfn-guard/tests/functional.rs index 7621084cb..ffe58f7a0 100644 --- a/cfn-guard/tests/functional.rs +++ b/cfn-guard/tests/functional.rs @@ -1720,4 +1720,20 @@ AWS::EC2::Volume Size == 101 |OR| AWS::EC2::Volume Size == 99"#, ) ) } + #[test] + fn test_correct_whitespace() { + let template_contents = String::from( + r#"Resources: + NewVolume: + Type: AWS::EC2::Volume + Properties : + Description: This is a description"#, + ); + let rules_file_contents = + String::from(r#"AWS::EC2::Volume Description == This is NOT a description"#); + assert_eq!( + cfn_guard::run_check(&template_contents, &rules_file_contents, true).unwrap(), + (vec![], 0) + ); + } } From 5a0d460bdc22a65dc95a372eda2d61d950f8f797 Mon Sep 17 00:00:00 2001 From: nathanmc Date: Fri, 28 Aug 2020 11:38:54 -0400 Subject: [PATCH 13/22] Fix string formatting in result values, update functional test to test for PASS when the only difference is whitespace --- cfn-guard/src/lib.rs | 25 +++++++++++++++++++++---- cfn-guard/src/util.rs | 2 ++ cfn-guard/tests/functional.rs | 3 ++- 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/cfn-guard/src/lib.rs b/cfn-guard/src/lib.rs index c8cf24ab3..4b795e4bf 100644 --- a/cfn-guard/src/lib.rs +++ b/cfn-guard/src/lib.rs @@ -421,6 +421,7 @@ fn apply_rule_operation( enums::OpCode::Require => { match rule.rule_vtype { enums::RValueType::Value | enums::RValueType::Variable => { + // Rule and template values are stripped of whitespace here for comparison let f_template_val = util::format_value(&val); trace!("f_template_val is {}", f_template_val); let f_rule_val = util::strip_ws_nl(rule_val.to_string()); @@ -435,16 +436,32 @@ fn apply_rule_operation( "[{}] failed because [{}] is [{}] and {}", res_name, &rule.field, - util::format_value(&val), + { + if val.is_string() { + //This is necessary to remove extraneous quotes when converting a string + String::from(val.as_str().unwrap()) + } else { + //Quotes not added for non-String SerDe values + val.to_string() + } + }, c ), - None => format!( + None => { + format!( "[{}] failed because [{}] is [{}] and the permitted value is [{}]", res_name, &rule.field, - util::format_value(&val), + {if val.is_string() { + //This is necessary to remove extraneous quotes when converting a string + String::from(val.as_str().unwrap()) + } else { + //Quotes not added for non-String SerDe values + val.to_string() + }}, rule_val.to_string() - ), + ) + } }) } } diff --git a/cfn-guard/src/util.rs b/cfn-guard/src/util.rs index b27dedac8..e62ddffc2 100644 --- a/cfn-guard/src/util.rs +++ b/cfn-guard/src/util.rs @@ -22,8 +22,10 @@ pub fn fix_stringified_bools(fstr: &str) -> String { pub fn format_value(v: &Value) -> String { let formatted_value = if v.is_string() { + //This is necessary to remove extraneous quotes when converting a string strip_ws_nl(String::from(v.as_str().unwrap())) } else { + //Quotes not added for non-String SerDe values strip_ws_nl(v.to_string()) }; trace!("formatted_value is '{}'", formatted_value); diff --git a/cfn-guard/tests/functional.rs b/cfn-guard/tests/functional.rs index ffe58f7a0..449a5e2e4 100644 --- a/cfn-guard/tests/functional.rs +++ b/cfn-guard/tests/functional.rs @@ -1722,6 +1722,7 @@ AWS::EC2::Volume Size == 101 |OR| AWS::EC2::Volume Size == 99"#, } #[test] fn test_correct_whitespace() { + // Value comparisons during rule eval remove whitespace but the result message should not to avoid confusing the user let template_contents = String::from( r#"Resources: NewVolume: @@ -1730,7 +1731,7 @@ AWS::EC2::Volume Size == 101 |OR| AWS::EC2::Volume Size == 99"#, Description: This is a description"#, ); let rules_file_contents = - String::from(r#"AWS::EC2::Volume Description == This is NOT a description"#); + String::from(r#"AWS::EC2::Volume Description == This is a description"#); // inserted a space on either side of 'a' assert_eq!( cfn_guard::run_check(&template_contents, &rules_file_contents, true).unwrap(), (vec![], 0) From 9217bdc1b7123c257e60d82268a4bee02b1e02c6 Mon Sep 17 00:00:00 2001 From: nathanmc Date: Fri, 28 Aug 2020 11:52:49 -0400 Subject: [PATCH 14/22] Add a `Troubleshooting FAQ` section and address the "failed match because of typo in property name" case --- cfn-guard/README.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/cfn-guard/README.md b/cfn-guard/README.md index a07b5439d..fb7a023e6 100644 --- a/cfn-guard/README.md +++ b/cfn-guard/README.md @@ -769,6 +769,12 @@ And the rule: ``` Whenever your rules aren't behaving as expected, this is great way to see why. +## Troubleshooting FAQ + +**Q: I keep trying to force a failure with a bad rule value and I'm not getting any results** + +A: This is almost always due to fact that there's a typo in the property name you're trying to check for in your rule. Turn on `--strict-checks` and you'll get an error if the names don't match. This is an easy way to spot typos. + # To Build and Run From 2262319b29744cce92d15c3bd0b25e83fcf10fa6 Mon Sep 17 00:00:00 2001 From: nathanmc Date: Fri, 28 Aug 2020 14:03:53 -0400 Subject: [PATCH 15/22] Add stricter regex for comments to prevent comments mistakenly being added at the end of rules from making the rule get treated as a comment. --- cfn-guard/src/parser.rs | 2 +- cfn-guard/tests/functional.rs | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/cfn-guard/src/parser.rs b/cfn-guard/src/parser.rs index 06a10b54f..6546ad91e 100644 --- a/cfn-guard/src/parser.rs +++ b/cfn-guard/src/parser.rs @@ -18,7 +18,7 @@ use lazy_static::lazy_static; lazy_static! { static ref ASSIGN_REG: Regex = Regex::new(r"let (?P\w+) +(?P\S+) +(?P.+)").unwrap(); static ref RULE_REG: Regex = Regex::new(r"^(?P\S+) +(?P[\w\.\|\*]+) +(?P==|!=|<|>|<=|>=|IN|NOT_IN) +(?P[^\n\r]+)").unwrap(); - static ref COMMENT_REG: Regex = Regex::new(r#"#(?P.*)"#).unwrap(); + static ref COMMENT_REG: Regex = Regex::new(r#"^#(?P.*)"#).unwrap(); static ref WILDCARD_OR_RULE_REG: Regex = Regex::new(r"(\S+) (\S*\*\S*) (==|IN) (.+)").unwrap(); static ref RULE_WITH_OPTIONAL_MESSAGE_REG: Regex = Regex::new( r"^(?P\S+) +(?P[\w\.\*]+) +(?P==|!=|<|>|<=|>=|IN|NOT_IN) +(?P[^\n\r]+) +<{2} *(?P.*)").unwrap(); diff --git a/cfn-guard/tests/functional.rs b/cfn-guard/tests/functional.rs index 449a5e2e4..f2a6f6539 100644 --- a/cfn-guard/tests/functional.rs +++ b/cfn-guard/tests/functional.rs @@ -1737,4 +1737,38 @@ AWS::EC2::Volume Size == 101 |OR| AWS::EC2::Volume Size == 99"#, (vec![], 0) ); } + + #[test] + fn test_for_inline_comment() { + let template_contents = String::from( + r#"Resources: + NewVolume: + Type: AWS::EC2::Volume + Properties : + Description: This is a description"#, + ); + let rules_file_contents = String::from( + r#"AWS::EC2::Volume Description == This is a description # this comment shouldn't be here"#, + ); + assert_eq!( + cfn_guard::run_check(&template_contents, &rules_file_contents, true).unwrap(), + (vec![String::from("[NewVolume] failed because [Description] is [This is a description] and the permitted value is [This is a description # this comment shouldn't be here]")], 2) + ); + } + + #[test] + fn test_for_comments_with_whitespace() { + let template_contents = String::from( + r#"Resources: + NewVolume: + Type: AWS::EC2::Volume + Properties : + Description: This is a description"#, + ); + let rules_file_contents = String::from(r#" # Comment!! :-o"#); + assert_eq!( + cfn_guard::run_check(&template_contents, &rules_file_contents, true).unwrap(), + (vec![], 0) + ); + } } From b80e5af96f27c87e6ea1a90b5daedc1acf954edf Mon Sep 17 00:00:00 2001 From: nathanmc Date: Fri, 28 Aug 2020 17:37:21 -0400 Subject: [PATCH 16/22] Move explicit | operator to implicit conversion --- cfn-guard-lambda/Cargo.lock | 106 +++++++++++ cfn-guard/Cargo.lock | 174 ++++++++++++++++++ cfn-guard/src/lib.rs | 4 +- cfn-guard/src/util.rs | 50 ++--- cfn-guard/tests/functional.rs | 42 ++--- ...te.yaml => stringified-json-template.yaml} | 0 6 files changed, 328 insertions(+), 48 deletions(-) rename cfn-guard/tests/{pipe-operator-template.yaml => stringified-json-template.yaml} (100%) diff --git a/cfn-guard-lambda/Cargo.lock b/cfn-guard-lambda/Cargo.lock index dd651e4e7..229056b28 100644 --- a/cfn-guard-lambda/Cargo.lock +++ b/cfn-guard-lambda/Cargo.lock @@ -27,6 +27,12 @@ dependencies = [ "winapi 0.3.8", ] +[[package]] +name = "ascii" +version = "0.9.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "eab1c04a571841102f5345a8fc0f6bb3d31c315dec879b5c6e42e40ce7ffa34e" + [[package]] name = "atty" version = "0.2.14" @@ -80,6 +86,12 @@ dependencies = [ "iovec", ] +[[package]] +name = "cesu8" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6d43a04d8753f35258c91f8ec639f792891f748a1edbd759cf1dcea3382ad83c" + [[package]] name = "cfg-if" version = "0.1.10" @@ -92,6 +104,7 @@ version = "0.7.0" dependencies = [ "cfn-guard-rulegen", "clap", + "jni", "lazy_static", "log", "regex", @@ -172,6 +185,19 @@ dependencies = [ "winapi 0.3.8", ] +[[package]] +name = "combine" +version = "3.8.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "da3da6baa321ec19e1cc41d31bf599f00c783d0517095cdaf0332e3fe8d20680" +dependencies = [ + "ascii", + "byteorder", + "either", + "memchr", + "unreachable", +] + [[package]] name = "crossbeam-deque" version = "0.7.3" @@ -232,6 +258,16 @@ version = "1.5.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bb1f6b1ce1c140482ea30ddd3335fc0024ac7ee112895426e0a629a6c20adfe3" +[[package]] +name = "error-chain" +version = "0.12.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2d2f06b9cac1506ece98fe3231e3cc9c4410ec3d5b1f24ae1c8946f0742cdefc" +dependencies = [ + "backtrace", + "version_check", +] + [[package]] name = "failure" version = "0.1.8" @@ -408,6 +444,26 @@ version = "0.4.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b8b7a7c0c47db5545ed3fef7468ee7bb5b74691498139e4b3f6a20685dc6dd8e" +[[package]] +name = "jni" +version = "0.14.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1981310da491a4f0f815238097d0d43d8072732b5ae5f8bd0d8eadf5bf245402" +dependencies = [ + "cesu8", + "combine", + "error-chain", + "jni-sys", + "log", + "walkdir", +] + +[[package]] +name = "jni-sys" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8eaf4bc02d17cbdd7ff4c7438cafcdf7fb9a4613313ad11b4f8fefe7d3fa0130" + [[package]] name = "kernel32-sys" version = "0.2.2" @@ -743,6 +799,15 @@ version = "1.0.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "71d301d4193d031abdd79ff7e3dd721168a9572ef3fe51a1517aba235bd8f86e" +[[package]] +name = "same-file" +version = "1.0.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "93fc1dc3aaa9bfed95e02e6eadabb4baf7e3078b0bd1b4d7b6b0b68378900502" +dependencies = [ + "winapi-util", +] + [[package]] name = "scopeguard" version = "1.1.0" @@ -1147,12 +1212,44 @@ version = "0.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "826e7639553986605ec5979c7dd957c7895e93eabed50ab2ffa7f6128a75097c" +[[package]] +name = "unreachable" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "382810877fe448991dfc7f0dd6e3ae5d58088fd0ea5e35189655f84e6814fa56" +dependencies = [ + "void", +] + [[package]] name = "vec_map" version = "0.8.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f1bddf1187be692e79c5ffeab891132dfb0f236ed36a43c7ed39f1165ee20191" +[[package]] +name = "version_check" +version = "0.9.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b5a972e5669d67ba988ce3dc826706fb0a8b01471c088cb0b6110b805cc36aed" + +[[package]] +name = "void" +version = "1.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6a02e4885ed3bc0f2de90ea6dd45ebcbb66dacffe03547fadbb0eeae2770887d" + +[[package]] +name = "walkdir" +version = "2.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "777182bc735b6424e1a57516d35ed72cb8019d85c8c9bf536dccb3445c1a2f7d" +dependencies = [ + "same-file", + "winapi 0.3.8", + "winapi-util", +] + [[package]] name = "want" version = "0.2.0" @@ -1192,6 +1289,15 @@ version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ac3b87c63620426dd9b991e5ce0329eff545bccbbb34f3be09ff6fb6ab51b7b6" +[[package]] +name = "winapi-util" +version = "0.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "70ec6ce85bb158151cae5e5c87f95a8e97d2c0c4b001223f33a334e3ce5de178" +dependencies = [ + "winapi 0.3.8", +] + [[package]] name = "winapi-x86_64-pc-windows-gnu" version = "0.4.0" diff --git a/cfn-guard/Cargo.lock b/cfn-guard/Cargo.lock index 654102138..e2736a974 100644 --- a/cfn-guard/Cargo.lock +++ b/cfn-guard/Cargo.lock @@ -1,5 +1,20 @@ # This file is automatically @generated by Cargo. # It is not intended for manual editing. +[[package]] +name = "addr2line" +version = "0.13.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1b6a2d3371669ab3ca9797670853d61402b03d0b4b9ebf33d677dfa720203072" +dependencies = [ + "gimli", +] + +[[package]] +name = "adler" +version = "0.2.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ee2a4ec343196209d6594e19543ae87a39f96d5534d7174822a3ad825dd6ed7e" + [[package]] name = "aho-corasick" version = "0.7.10" @@ -18,6 +33,12 @@ dependencies = [ "winapi", ] +[[package]] +name = "ascii" +version = "0.9.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "eab1c04a571841102f5345a8fc0f6bb3d31c315dec879b5c6e42e40ce7ffa34e" + [[package]] name = "atty" version = "0.2.14" @@ -35,12 +56,38 @@ version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f8aac770f1885fd7e387acedd76065302551364496e46b3dd00860b2f8359b9d" +[[package]] +name = "backtrace" +version = "0.3.50" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "46254cf2fdcdf1badb5934448c1bcbe046a56537b3987d96c51a7afc5d03f293" +dependencies = [ + "addr2line", + "cfg-if", + "libc", + "miniz_oxide", + "object", + "rustc-demangle", +] + [[package]] name = "bitflags" version = "1.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cf1de2fe8c75bc145a2f577add951f8134889b4795d47466a54a5c846d691693" +[[package]] +name = "byteorder" +version = "1.3.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "08c48aae112d48ed9f069b33538ea9e3e90aa263cfa3d1c24309612b1f7472de" + +[[package]] +name = "cesu8" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6d43a04d8753f35258c91f8ec639f792891f748a1edbd759cf1dcea3382ad83c" + [[package]] name = "cfg-if" version = "0.1.10" @@ -53,6 +100,7 @@ version = "0.7.0" dependencies = [ "cfn-guard-rulegen", "clap", + "jni", "lazy_static", "log", "regex", @@ -111,12 +159,47 @@ dependencies = [ "winapi", ] +[[package]] +name = "combine" +version = "3.8.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "da3da6baa321ec19e1cc41d31bf599f00c783d0517095cdaf0332e3fe8d20680" +dependencies = [ + "ascii", + "byteorder", + "either", + "memchr", + "unreachable", +] + [[package]] name = "dtoa" version = "0.4.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4358a9e11b9a09cf52383b451b49a169e8d797b68aa02301ff586d70d9661ea3" +[[package]] +name = "either" +version = "1.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cd56b59865bce947ac5958779cfa508f6c3b9497cc762b7e24a12d11ccde2c4f" + +[[package]] +name = "error-chain" +version = "0.12.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2d2f06b9cac1506ece98fe3231e3cc9c4410ec3d5b1f24ae1c8946f0742cdefc" +dependencies = [ + "backtrace", + "version_check", +] + +[[package]] +name = "gimli" +version = "0.22.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "aaf91faf136cb47367fa430cd46e37a788775e7fa104f8b4bcb3861dc389b724" + [[package]] name = "hermit-abi" version = "0.1.13" @@ -132,6 +215,26 @@ version = "0.4.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b8b7a7c0c47db5545ed3fef7468ee7bb5b74691498139e4b3f6a20685dc6dd8e" +[[package]] +name = "jni" +version = "0.14.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1981310da491a4f0f815238097d0d43d8072732b5ae5f8bd0d8eadf5bf245402" +dependencies = [ + "cesu8", + "combine", + "error-chain", + "jni-sys", + "log", + "walkdir", +] + +[[package]] +name = "jni-sys" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8eaf4bc02d17cbdd7ff4c7438cafcdf7fb9a4613313ad11b4f8fefe7d3fa0130" + [[package]] name = "lazy_static" version = "1.4.0" @@ -165,6 +268,15 @@ version = "2.3.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3728d817d99e5ac407411fa471ff9800a778d88a24685968b36824eaf4bee400" +[[package]] +name = "miniz_oxide" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4d7559a8a40d0f97e1edea3220f698f78b1c5ab67532e49f68fde3910323b722" +dependencies = [ + "adler", +] + [[package]] name = "num-integer" version = "0.1.43" @@ -184,6 +296,12 @@ dependencies = [ "autocfg", ] +[[package]] +name = "object" +version = "0.20.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1ab52be62400ca80aa00285d25253d7f7c437b7375c4de678f5405d3afe82ca5" + [[package]] name = "proc-macro2" version = "1.0.18" @@ -220,12 +338,27 @@ version = "0.6.18" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "26412eb97c6b088a6997e05f69403a802a92d520de2f8e63c2b65f9e0f47c4e8" +[[package]] +name = "rustc-demangle" +version = "0.1.16" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4c691c0e608126e00913e33f0ccf3727d5fc84573623b8d65b2df340b5201783" + [[package]] name = "ryu" version = "1.0.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "71d301d4193d031abdd79ff7e3dd721168a9572ef3fe51a1517aba235bd8f86e" +[[package]] +name = "same-file" +version = "1.0.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "93fc1dc3aaa9bfed95e02e6eadabb4baf7e3078b0bd1b4d7b6b0b68378900502" +dependencies = [ + "winapi-util", +] + [[package]] name = "serde" version = "1.0.111" @@ -339,12 +472,44 @@ version = "0.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "826e7639553986605ec5979c7dd957c7895e93eabed50ab2ffa7f6128a75097c" +[[package]] +name = "unreachable" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "382810877fe448991dfc7f0dd6e3ae5d58088fd0ea5e35189655f84e6814fa56" +dependencies = [ + "void", +] + [[package]] name = "vec_map" version = "0.8.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f1bddf1187be692e79c5ffeab891132dfb0f236ed36a43c7ed39f1165ee20191" +[[package]] +name = "version_check" +version = "0.9.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b5a972e5669d67ba988ce3dc826706fb0a8b01471c088cb0b6110b805cc36aed" + +[[package]] +name = "void" +version = "1.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6a02e4885ed3bc0f2de90ea6dd45ebcbb66dacffe03547fadbb0eeae2770887d" + +[[package]] +name = "walkdir" +version = "2.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "777182bc735b6424e1a57516d35ed72cb8019d85c8c9bf536dccb3445c1a2f7d" +dependencies = [ + "same-file", + "winapi", + "winapi-util", +] + [[package]] name = "winapi" version = "0.3.8" @@ -361,6 +526,15 @@ version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ac3b87c63620426dd9b991e5ce0329eff545bccbbb34f3be09ff6fb6ab51b7b6" +[[package]] +name = "winapi-util" +version = "0.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "70ec6ce85bb158151cae5e5c87f95a8e97d2c0c4b001223f33a334e3ce5de178" +dependencies = [ + "winapi", +] + [[package]] name = "winapi-x86_64-pc-windows-gnu" version = "0.4.0" diff --git a/cfn-guard/src/lib.rs b/cfn-guard/src/lib.rs index 00f852a58..8596cbe3d 100644 --- a/cfn-guard/src/lib.rs +++ b/cfn-guard/src/lib.rs @@ -369,13 +369,13 @@ fn apply_rule( } }; match util::get_resource_prop_value(property_root, &address) { - Err(e) => { + Err(_) => { 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 + name, &rule.field ), }) } diff --git a/cfn-guard/src/util.rs b/cfn-guard/src/util.rs index e62ddffc2..921a23e6b 100644 --- a/cfn-guard/src/util.rs +++ b/cfn-guard/src/util.rs @@ -91,28 +91,6 @@ pub fn get_resource_prop_value(props: &Value, field: &[&str]) -> Result return Ok(props.clone()), "." => get_resource_prop_value(&props, &field_list), - "|" => match props.as_str() { - Some(p) => match serde_json::from_str::(p) { - Ok(s) => { - trace!("sub structure is {:#?}", s); - if !field_list.is_empty() { - let nf = field_list.remove(0); - trace!("next_field is {}", nf); - match match_props(&s, &nf) { - Ok(v) => { - trace!("next_props is {:#?}", v); - get_resource_prop_value(&v, &field_list) - } - Err(_) => return Err(format!("Invalid address: {}", nf)), - } - } else { - Ok(s) - } - } - Err(e) => return Err(e.to_string()), - }, - None => return Err(format!("Could not convert properties to string")), - }, _ => match next_field.parse::() { Ok(n) => { trace!( @@ -129,7 +107,7 @@ pub fn get_resource_prop_value(props: &Value, field: &[&str]) -> Result Err(n.to_string()), + Err(_) => destringify_json(props, &n, &mut field_list), } } Err(_) => { @@ -146,10 +124,34 @@ pub fn get_resource_prop_value(props: &Value, field: &[&str]) -> Result Err(next_field.to_string()), + Err(_) => destringify_json(props, &next_field, &mut field_list), + } + } + }, + } +} + +fn destringify_json<'a>( + props: &'a Value, + field: &'a dyn serde_json::value::Index, + field_list: &'a mut Vec<&str>, +) -> Result { + match props.as_str() { + Some(p) => match serde_json::from_str::(p) { + Ok(s) => { + trace!("sub structure is {:#?}", s); + // trace!("next_field is {}", field); + match match_props(&s, field) { + Ok(v) => { + trace!("next_props is {:#?}", v); + get_resource_prop_value(&v, &field_list) + } + Err(_) => return Err(format!("Invalid address")), } } + Err(e) => return Err(e.to_string()), }, + None => return Err(format!("Could not convert properties to string")), } } diff --git a/cfn-guard/tests/functional.rs b/cfn-guard/tests/functional.rs index f2a6f6539..3c9bfb6d1 100644 --- a/cfn-guard/tests/functional.rs +++ b/cfn-guard/tests/functional.rs @@ -1198,8 +1198,8 @@ AWS::EC2::Volume Size == 101 |OR| AWS::EC2::Volume Size == 99"#, .unwrap_or_else(|err| format!("{}", err)); assert_eq!( cfn_guard::run_check(&template_contents, &rules_file_contents, true).unwrap(), - (vec![String::from("[NewVolume2] failed because it does not contain the required property of [Tags]"), - String::from("[NewVolume2] failed because it does not contain the required property of [Tags]"), + (vec![String::from("[NewVolume2] failed because it does not contain the required property of [Tags.0.Key]"), + String::from("[NewVolume2] failed because it does not contain the required property of [Tags.1.Key]"), String::from("[NewVolume] failed because [Tags.0.Key] is [uaid] and the permitted value is [uai]"), String::from("[NewVolume] failed because [Tags.1.Key] is [tag2] and the permitted value is [uai]")], 2) @@ -1650,12 +1650,12 @@ AWS::EC2::Volume Size == 101 |OR| AWS::EC2::Volume Size == 99"#, ) } #[test] - fn test_pipe_operator() { - let template_contents = fs::read_to_string("tests/pipe-operator-template.yaml") + fn test_stringifed_json_descent() { + let template_contents = fs::read_to_string("tests/stringified-json-template.yaml") .unwrap_or_else(|err| format!("{}", err)); let mut rules_file_contents = String::from( - r#"AWS::IAM::Role AssumeRolePolicyDocument.|.Statement.0 == { "Effect": "Allow", "Principal": { "Service": [ "notlambda.amazonaws.com" ] } } "#, + r#"AWS::IAM::Role AssumeRolePolicyDocument.Statement.0 == { "Effect": "Allow", "Principal": { "Service": [ "notlambda.amazonaws.com" ] } } "#, ); assert_eq!( @@ -1664,7 +1664,7 @@ AWS::EC2::Volume Size == 101 |OR| AWS::EC2::Volume Size == 99"#, ); rules_file_contents = String::from( - r#"AWS::IAM::Role AssumeRolePolicyDocument.|.Statement.* == { "Effect": "Allow", "Principal": { "Service": [ "notlambda.amazonaws.com" ] } } "#, + r#"AWS::IAM::Role AssumeRolePolicyDocument.Statement.* == { "Effect": "Allow", "Principal": { "Service": [ "notlambda.amazonaws.com" ] } } "#, ); assert_eq!( cfn_guard::run_check(&template_contents, &rules_file_contents, true).unwrap(), @@ -1672,31 +1672,29 @@ AWS::EC2::Volume Size == 101 |OR| AWS::EC2::Volume Size == 99"#, ); rules_file_contents = String::from( - r#"AWS::IAM::Role AssumeRolePolicyDocument.|.Statement.0 == {"Effect": "Allow","Principal":{"Service":["notlambda.amazonaws.com"]}}"#, + r#"AWS::IAM::Role AssumeRolePolicyDocument.Statement.0 == {"Effect": "Allow","Principal":{"Service":["notlambda.amazonaws.com"]}}"#, ); assert_eq!( cfn_guard::run_check(&template_contents, &rules_file_contents, true).unwrap(), (vec![], 0) ); - rules_file_contents = String::from( - r#"AWS::IAM::Role AssumeRolePolicyDocument.|.Statement.0.Effect == Allow"#, - ); + rules_file_contents = + String::from(r#"AWS::IAM::Role AssumeRolePolicyDocument.Statement.0.Effect == Allow"#); assert_eq!( cfn_guard::run_check(&template_contents, &rules_file_contents, true).unwrap(), (vec![], 0) ); - rules_file_contents = String::from( - r#"AWS::IAM::Role AssumeRolePolicyDocument.|.Statement.*.Effect == Allow"#, - ); + rules_file_contents = + String::from(r#"AWS::IAM::Role AssumeRolePolicyDocument.Statement.*.Effect == Allow"#); assert_eq!( cfn_guard::run_check(&template_contents, &rules_file_contents, true).unwrap(), (vec![], 0) ); rules_file_contents = String::from( - r#"AWS::IAM::Role AssumeRolePolicyDocument.| == {"Statement":[{"Effect": "Allow","Principal":{"Service":["notlambda.amazonaws.com"]}}]}"#, + r#"AWS::IAM::Role AssumeRolePolicyDocument == {"Statement":[{"Effect": "Allow","Principal":{"Service":["notlambda.amazonaws.com"]}}]}"#, ); assert_eq!( cfn_guard::run_check(&template_contents, &rules_file_contents, true).unwrap(), @@ -1704,18 +1702,18 @@ AWS::EC2::Volume Size == 101 |OR| AWS::EC2::Volume Size == 99"#, ); rules_file_contents = String::from( - r#"AWS::IAM::Role AssumeRolePolicyDocument.| != {"Statement":[{"Effect": "Allow","Principal":{"Service":["notlambda.amazonaws.com"]}}]} - AWS::IAM::Role AssumeRolePolicyDocument.|.Statement != [{"Effect": "Allow","Principal":{"Service":["notlambda.amazonaws.com"]}}] - AWS::IAM::Role AssumeRolePolicyDocument.|.Statement.0 != {"Effect": "Allow","Principal":{"Service":["notlambda.amazonaws.com"]}} - AWS::IAM::Role AssumeRolePolicyDocument.|.Statement.*.Effect != Allow"#, + r#"AWS::IAM::Role AssumeRolePolicyDocument != {"Statement":[{"Effect": "Allow","Principal":{"Service":["notlambda.amazonaws.com"]}}]} + AWS::IAM::Role AssumeRolePolicyDocument.Statement != [{"Effect": "Allow","Principal":{"Service":["notlambda.amazonaws.com"]}}] + AWS::IAM::Role AssumeRolePolicyDocument.Statement.0 != {"Effect": "Allow","Principal":{"Service":["notlambda.amazonaws.com"]}} + AWS::IAM::Role AssumeRolePolicyDocument.Statement.*.Effect != Allow"#, ); assert_eq!( cfn_guard::run_check(&template_contents, &rules_file_contents, true).unwrap(), ( - vec![String::from("[LambdaRoleHelper] failed because [AssumeRolePolicyDocument.|.Statement.0.Effect] is [Allow] and that value is not permitted"), - String::from(r#"[LambdaRoleHelper] failed because [AssumeRolePolicyDocument.|.Statement.0] is [{"Effect":"Allow","Principal":{"Service":["notlambda.amazonaws.com"]}}] and that value is not permitted"#), - String::from(r#"[LambdaRoleHelper] failed because [AssumeRolePolicyDocument.|.Statement] is [[{"Effect":"Allow","Principal":{"Service":["notlambda.amazonaws.com"]}}]] and that value is not permitted"#), - String::from(r#"[LambdaRoleHelper] failed because [AssumeRolePolicyDocument.|] is [{"Statement":[{"Effect":"Allow","Principal":{"Service":["notlambda.amazonaws.com"]}}]}] and that value is not permitted"#)], + vec![String::from("[LambdaRoleHelper] failed because [AssumeRolePolicyDocument.Statement.0.Effect] is [Allow] and that value is not permitted"), + String::from(r#"[LambdaRoleHelper] failed because [AssumeRolePolicyDocument.Statement.0] is [{"Effect":"Allow","Principal":{"Service":["notlambda.amazonaws.com"]}}] and that value is not permitted"#), + String::from(r#"[LambdaRoleHelper] failed because [AssumeRolePolicyDocument.Statement] is [[{"Effect":"Allow","Principal":{"Service":["notlambda.amazonaws.com"]}}]] and that value is not permitted"#), + String::from(r#"[LambdaRoleHelper] failed because [AssumeRolePolicyDocument] is [{"Statement":[{"Effect":"Allow","Principal":{"Service":["notlambda.amazonaws.com"]}}]}] and that value is not permitted"#)], 2 ) ) diff --git a/cfn-guard/tests/pipe-operator-template.yaml b/cfn-guard/tests/stringified-json-template.yaml similarity index 100% rename from cfn-guard/tests/pipe-operator-template.yaml rename to cfn-guard/tests/stringified-json-template.yaml From a94642fe3e08375e6a9139b188456e3641c18e1f Mon Sep 17 00:00:00 2001 From: nathanmc Date: Fri, 28 Aug 2020 17:37:43 -0400 Subject: [PATCH 17/22] rustfmt --- cfn-guard/src/lib.rs | 35 +++++++++++++++++++++-------------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/cfn-guard/src/lib.rs b/cfn-guard/src/lib.rs index 8596cbe3d..c9b396f21 100644 --- a/cfn-guard/src/lib.rs +++ b/cfn-guard/src/lib.rs @@ -50,7 +50,7 @@ pub fn run( } } -pub extern fn run_check( +pub extern "C" fn run_check( template_file_contents: &str, rules_file_contents: &str, strict_checks: bool, @@ -777,26 +777,33 @@ pub extern "system" fn Java_com_amazonaws_cfnguard_javawrapper_CfnGuardWrapper_r rules_contents: JString, strict_checks: JString, ) -> jstring { - let template_string: String = - env.get_string(template_contents).expect("Couldn't get java string for template_contents").into(); - let rules_string: String = - env.get_string(rules_contents).expect("Couldn't get java string for rules_contents").into(); + let template_string: String = env + .get_string(template_contents) + .expect("Couldn't get java string for template_contents") + .into(); + let rules_string: String = env + .get_string(rules_contents) + .expect("Couldn't get java string for rules_contents") + .into(); // Anything but "true" is treated as false. - let strict_checks_string: String = - env.get_string(strict_checks).expect("Couldn't get java string for strict_checks").into(); - let strict_checks_bool = - match strict_checks_string.parse() { - Ok(res) => res, - Err(_e) => false, - }; + let strict_checks_string: String = env + .get_string(strict_checks) + .expect("Couldn't get java string for strict_checks") + .into(); + let strict_checks_bool = match strict_checks_string.parse() { + Ok(res) => res, + Err(_e) => false, + }; - let outcome_string: String = + let outcome_string: String = match run_check(&template_string, &rules_string, strict_checks_bool) { Ok(res) => res.0.join("\n"), Err(e) => e.to_string(), }; - let result_jni_string = env.new_string(outcome_string).expect("Couldn't cast check outcome to JNI JString"); + let result_jni_string = env + .new_string(outcome_string) + .expect("Couldn't cast check outcome to JNI JString"); return result_jni_string.into_inner(); } From 2d35b299864632fb55a40bda6c4a66a9bdb72f78 Mon Sep 17 00:00:00 2001 From: nathanmc Date: Fri, 28 Aug 2020 18:01:18 -0400 Subject: [PATCH 18/22] Update the README's to mirror the folding of cfn-guard-rulegen into the cfn-guard binary --- README.md | 24 +++--- cfn-guard-rulegen/README.md | 158 ------------------------------------ cfn-guard/README.md | 78 +++++++++++++----- 3 files changed, 72 insertions(+), 188 deletions(-) delete mode 100644 cfn-guard-rulegen/README.md diff --git a/README.md b/README.md index 71e51ac59..0054aec61 100644 --- a/README.md +++ b/README.md @@ -2,9 +2,11 @@ This repo contains source code for the following tools: -* `CloudFormation Guard` A CLI tool that checks AWS CloudFormation templates for policy compliance using a simple, policy-as-code, declarative syntax -* `CloudFormation Guard Lambda` is the AWS Lambda version of `CloudFormation Guard` -* `CloudFormation Guard Rulegen` automatically generates CloudFormation Guard rules from existing CloudFormation templates +* `CloudFormation Guard` A CLI tool that + * Checks AWS CloudFormation templates for policy compliance using a simple, policy-as-code, declarative syntax + * Can autogenerate rules from existing CloudFormation templates +* `CloudFormation Guard Lambda` is the AWS Lambda version of CloudFormation Guard's `check` functionality +* `CloudFormation Guard Rulegen Lambda` is the AWS Lambda version of CloudFormation Guard's `rulegen` functionality ## How it works @@ -48,7 +50,7 @@ AWS::EC2::Volume Size <= 100 You can check the template to ensure that it adheres to the rules. ``` -$> cfn-guard -t Examples/ebs_volume_template.json -r Examples/ebs_volume_template.ruleset +$> cfn-guard check -t Examples/ebs_volume_template.json -r Examples/ebs_volume_template.ruleset [NewVolume2] failed because [Encrypted] is [false] and the permitted value is [true] [NewVolume] failed because [Encrypted] is [false] and the permitted value is [true] @@ -61,7 +63,7 @@ Number of failures: 3 CloudFormation Guard can be used to evaluate security best practices for infrastructure deployed via CloudFormation. A number of example rules are included: ``` -$> cfn-guard -t Examples/security_template.json -r Examples/security_rules.ruleset +$> cfn-guard check -t Examples/security_template.json -r Examples/security_rules.ruleset "[AmazonMQBroker] failed because [AutoMinorVersionUpgrade] is [false] and Version upgrades should be enabled to receive security updates" "[AmazonMQBroker] failed because [EncryptionOptions.UseAwsOwnedKey] is [true] and CMKs should be used instead of AWS-provided KMS keys" "[AmazonMQBroker] failed because [EngineVersion] is [5.15.9] and Broker engine version should be at least 5.15.10" @@ -71,12 +73,12 @@ $> cfn-guard -t Examples/security_template.json -r Examples/security_rules.rules More details on how to write rules and how the tool can work with build systems can be found [here](cfn-guard/README.md). ### Automatically Generating Rules -You can also use the `CloudFormation Guard Rulegen` tool to automatically generate rules from known-good CloudFormation templates. +You can also use the `CloudFormation Guard` tool to automatically generate rules from known-good CloudFormation templates. -Using the same template as above, `cfn-guard-rulegen` would produce: +Using the same template as above, `cfn-guard rulegen` would produce: ``` -$> cfn-guard-rulegen Examples/ebs_volume_template.json +$> cfn-guard rulegen Examples/ebs_volume_template.json AWS::EC2::Volume Encrypted == false AWS::EC2::Volume Size == 101 |OR| AWS::EC2::Volume Size == 99 AWS::EC2::Volume AvailabilityZone == us-west-2b |OR| AWS::EC2::Volume AvailabilityZone == us-west-2c @@ -84,9 +86,9 @@ AWS::EC2::Volume AvailabilityZone == us-west-2b |OR| AWS::EC2::Volume Availabili From there, you can pipe them into a file and add, edit or remove rules as you need. -### Checking templates using the Lambda +### Using the tool as an AWS Lambda -Everything that can be checked from the command-line version of the tool can be checked using [the Lambda version](./cfn-guard-lambda/README.md). +Everything that can be checked from the command-line version of the tool can be checked using [the Lambda version](./cfn-guard-lambda/README.md). The same is true for the [rulegen functionality](./cfn-guard-rulegen-lambda/README.md). ## Setting it up @@ -134,7 +136,7 @@ Details on how to build the tools and use them are available in each tool's READ [CloudFormation Guard Lambda](cfn-guard-lambda/README.md) -[CloudFormation Guard Rulegen](cfn-guard-rulegen/README.md) +[CloudFormation Guard Rulegen Lambda](cfn-guard-rulegen-lambda/README.md) ## Using the Makefile diff --git a/cfn-guard-rulegen/README.md b/cfn-guard-rulegen/README.md deleted file mode 100644 index 7a9170515..000000000 --- a/cfn-guard-rulegen/README.md +++ /dev/null @@ -1,158 +0,0 @@ -# [PREVIEW] CloudFormation Guard Rulegen - -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]