From a0b7920a559394908266ab7ee4de4c04e9233f8e Mon Sep 17 00:00:00 2001 From: Alan Kuo Date: Tue, 11 Jun 2024 20:43:06 +0000 Subject: [PATCH] Support CheckNoPublicAccess check and CheckAccessNotGranted check with resource type support --- Dockerfile | 2 +- README.md | 59 ++++++++++++++++++++++++++++++++++++++++++++--------- action.yaml | 4 +++- main.py | 43 ++++++++++++++++++++++++++++++-------- test_tf.py | 18 ++++++++-------- 5 files changed, 96 insertions(+), 30 deletions(-) diff --git a/Dockerfile b/Dockerfile index 7410ff3..3ea7815 100644 --- a/Dockerfile +++ b/Dockerfile @@ -5,7 +5,7 @@ FROM python:3.10 # Install tf-policy-validator # Cloning the `terraform-iam-policy-validator` repo for default config for terraform templates # ToDo: Once we start using the tags for releases, we should use the tag associated with the version we download by using flag --branch -RUN pip install tf-policy-validator==0.0.6 && git clone https://github.com/awslabs/terraform-iam-policy-validator.git +RUN pip install tf-policy-validator==0.0.8 && git clone https://github.com/awslabs/terraform-iam-policy-validator.git ENV TERRAFORM_CONFIG_DEFAULT=/terraform-iam-policy-validator/iam_check/config/default.yaml diff --git a/README.md b/README.md index 342489e..63cb490 100644 --- a/README.md +++ b/README.md @@ -20,24 +20,27 @@ See [action.yml](action.yaml) for the full documentation for this action's input VALIDATE_POLICY CHECK_NO_NEW_ACCESS CHECK_ACCESS_NOT_GRANTED + CHECK_NO_PUBLIC_ACCESS policy-check-type - Name of the policy check.
Note: Each value corresponds to an IAM Access Analyzer API.
- ValidatePolicy
- CheckNoNewAccess
- CheckAccessNotGranted - VALIDATE_POLICY, CHECK_NO_NEW_ACCESS, CHECK_ACCESS_NOT_GRANTED. + Name of the policy check.
Note: Each value corresponds to an IAM Access Analyzer API.
- ValidatePolicy
- CheckNoNewAccess
- CheckAccessNotGranted
- CheckNoPublicAccess + VALIDATE_POLICY, CHECK_NO_NEW_ACCESS, CHECK_ACCESS_NOT_GRANTED, CHECK_NO_PUBLIC_ACCESS. Yes ✅ ✅ ✅ + ✅ template-path - The path to the Terraform template. + The path to the CloudFormation template. FILE_PATH.json Yes ✅ ✅ ✅ + ✅ region @@ -47,6 +50,7 @@ See [action.yml](action.yaml) for the full documentation for this action's input ✅ ✅ ✅ + ✅ ignore-finding @@ -56,31 +60,47 @@ See [action.yml](action.yaml) for the full documentation for this action's input ✅ ✅ ✅ + ✅ actions - List of comma-separated actions. Example format - ACTION,ACTION,ACTION.

This attribute is only considered and required when policy-check-type is "CHECK_ACCESS_NOT_GRANTED". + List of comma-separated actions. Example format - ACTION,ACTION,ACTION.

This attribute is only considered when policy-check-type is "CHECK_ACCESS_NOT_GRANTED". At least one of "actions" or "resources" must be provided ACTION,ACTION,ACTION No ❌ ❌ ✅ + ❌ + + + resources + List of comma-separated resource ARNs. Example format - RESOURCE,RESOURCE,RESOURCE.

This attribute is only considered when policy-check-type is "CHECK_ACCESS_NOT_GRANTED". At least one of "actions" or "resources" must be provided + RESOURCE,RESOURCE,RESOURCE + No + ❌ + ❌ + ✅ + ❌ reference-policy A JSON formatted file that specifies the path to the reference policy that is used for a permissions comparison.

This attribute is only considered and required when policy-check-type is "CHECK_NO_NEW_ACCESS". + FILE_PATH.json No ❌ ✅ ❌ + ❌ reference-policy-type The policy type associated with the IAM policy under analysis and the reference policy. Valid values: IDENTITY, RESOURCE.

This attribute is only considered and required when policy-check-type is "CHECK_NO_NEW_ACCESS" + REFERENCE_POLICY_TYPE No ❌ ✅ ❌ + ❌ treat-finding-type-as-blocking @@ -90,14 +110,17 @@ See [action.yml](action.yaml) for the full documentation for this action's input ✅ ❌ ❌ + ❌ treat-findings-as-non-blocking - By default, the tool will exit with a non-zero exit code when it detects any findings. Set this flag to exit with an exit code of 0 when it detects findings. You can use this to run new checks in a shadow or log only mode before enforcing them.

This attribute is considered only when policy-check-type is "CHECK_NO_NEW_ACCESS" or "CHECK_ACCESS_NOT_GRANTED". + By default, the tool will exit with a non-zero exit code when it detects any findings. Set this flag to exit with an exit code of 0 when it detects findings. You can use this to run new checks in a shadow or log only mode before enforcing them.

This attribute is considered only when policy-check-type is "CHECK_NO_NEW_ACCESS", "CHECK_ACCESS_NOT_GRANTED", or "CHECK_NO_PUBLIC_ACCESS. + No ❌ ✅ ✅ + ✅ allow-external-principals @@ -107,14 +130,17 @@ See [action.yml](action.yaml) for the full documentation for this action's input ✅ ❌ ❌ + ❌ allow-dynamic-ref-without-version Override the default behavior and allow dynamic SSM references without version numbers. The version number ensures that the SSM parameter value that was validated is the one that is deployed. + No ✅ ✅ ✅ + ✅ exclude-resource-types @@ -124,6 +150,7 @@ See [action.yml](action.yaml) for the full documentation for this action's input ✅ ✅ ✅ + ✅ @@ -135,7 +162,7 @@ See [action.yml](action.yaml) for the full documentation for this action's input - Setting up the role: Role used in the GitHub workflow should have necessary permissions required - to be called from the GitHub workflows - setup OpenID Connect(OIDC) provider and IAM role & Trust policy as described in step 1 & 2 in [this](https://aws.amazon.com/blogs/security/use-iam-roles-to-connect-github-actions-to-actions-in-aws/) blog - - to call the AWS APIs for the policy checks - ValidatePolicy, CheckNoNewAccess, CheckAccessNotGranted. Refer [this](https://docs.aws.amazon.com/IAM/latest/UserGuide/access-analyzer-checks-validating-policies.html) page for more details + - to call the AWS APIs for the policy checks - ValidatePolicy, CheckNoNewAccess, CheckAccessNotGranted, CheckNoPublicAccess. Refer [this](https://docs.aws.amazon.com/IAM/latest/UserGuide/access-analyzer-checks-validating-policies.html) page for more details ``` - name: Checkout Repo @@ -143,10 +170,9 @@ See [action.yml](action.yaml) for the full documentation for this action's input - name: Configure AWS Credentials uses: aws-actions/configure-aws-credentials@v4 with: - role-to-assume: ${{ secrets.POLICY_VALIDATOR_ROLE }} # Role with permissions to invoke access-analyzer:ValidatePolicy,access-analyzer:CheckNoNewAccess, access-analyzer:CheckAccessNotGranted + role-to-assume: ${{ secrets.POLICY_VALIDATOR_ROLE }} # Role with permissions to invoke access-analyzer:ValidatePolicy,access-analyzer:CheckNoNewAccess, access-analyzer:CheckAccessNotGranted, access-analyzer:CheckNoPublicAccess aws-region: aws-example-region ``` - #### Getting started using starter workflows To get started quickly, add a starter workflow to the `.github/workflows` directory of your repository. In order to do that, do the following - @@ -191,13 +217,26 @@ Please find the starter workflow [here](https://github.com/actions/starter-workf #### Using for the `CHECK_ACCESS_NOT_GRANTED` CHECK ``` - - name: Run CHECK_ACCESS_NOT_GRANTED check - id: run-check-no-new-access + - name: Run CHECK_ACCESS_NOT_GRANTED check + id: run-check-access-not-granted uses: aws-actions/terraform-aws-iam-policy-validator@v1.0.1 with: policy-check-type: 'CHECK_ACCESS_NOT_GRANTED' template-path: file-path-to-the-cfn-templates actions: "action1, action2.." + resources: "resource1, resource2.." + region: aws-example-region +``` + +#### Using for the `CHECK_NO_PUBLIC_ACCESS` CHECK + +``` + - name: Run CHECK_NO_PUBLIC_ACCESS check + id: run-check-no-public-access + uses: aws-actions/cloudformation-aws-iam-policy-validator@v1.0.1 + with: + policy-check-type: 'CHECK_NO_PUBLIC_ACCESS' + template-path: file-path-to-the-cfn-templates region: aws-example-region ``` diff --git a/action.yaml b/action.yaml index 63f8c78..623efcb 100644 --- a/action.yaml +++ b/action.yaml @@ -16,7 +16,9 @@ inputs: ignore-finding: description: 'Allow validation failures to be ignored. Specify as a comma separated list of findings to be ignored. Can be individual finding codes (e.g. "PASS_ROLE_WITH_STAR_IN_RESOURCE"), a specific resource name (e.g. "MyResource"), or a combination of both separated by a period.(e.g. "MyResource.PASS_ROLE_WITH_STAR_IN_RESOURCE"). Names of finding codes may change in IAM Access Analyzer over time. Valid options: FINDING_CODE,RESOURCE_NAME,RESOURCE_NAME.FINDING_CODE' actions: - description: 'List of comma-separated actions. Example format - ACTION,ACTION,ACTION. This attribute is considered and required when policy-check-type is "CHECK_ACCESS_NOT_GRANTED"' + description: 'List of comma-separated actions. Example format - ACTION,ACTION,ACTION. This attribute is considered when policy-check-type is "CHECK_ACCESS_NOT_GRANTED". At least one of "actions" or "resources" must be specified.' + resources: + description: 'List of comma-separated resource ARNs. Example format - RESOURCE,RESOURCE,RESOURCE. This attribute is considered when policy-check-type is "CHECK_ACCESS_NOT_GRANTED" At least one of "actions" or "resources" must be specified.' reference-policy: description: 'A JSON formatted file that specifies the path to the reference policy that is used for a permissions comparison. This attribute is considered and required when policy-check-type is "CHECK_NO_NEW_ACCESS"' reference-policy-type: diff --git a/main.py b/main.py index 79dc12c..fcef979 100644 --- a/main.py +++ b/main.py @@ -7,6 +7,7 @@ VALIDATE_POLICY = "VALIDATE_POLICY" CHECK_NO_NEW_ACCESS = "CHECK_NO_NEW_ACCESS" CHECK_ACCESS_NOT_GRANTED = "CHECK_ACCESS_NOT_GRANTED" +CHECK_NO_PUBLIC_ACCESS = "CHECK_NO_PUBLIC_ACCESS" CLI_POLICY_VALIDATOR = "tf-policy-validator" @@ -26,7 +27,10 @@ "INPUT_REFERENCE-POLICY-TYPE", } -CHECK_ACCESS_NOT_GRANTED_SPECIFIC_REQUIRED_INPUTS = {"INPUT_ACTIONS"} +# Use tuple to specify that at least one of the enclosed inputs is required. +CHECK_ACCESS_NOT_GRANTED_SPECIFIC_REQUIRED_INPUTS = {("INPUT_ACTIONS", "INPUT_RESOURCES")} + +CHECK_NO_PUBLIC_ACCESS_SPECIFIC_REQUIRED_INPUTS = set() # excluding the "INPUT_POLICY-CHECK-TYPE". Contains only other required inputs in cfn-policy-validator COMMON_OPTIONAL_INPUTS = { @@ -46,11 +50,15 @@ # Excluding the TREAT-FINDINGS-AS-NON-BLOCKING which is a flag and needs special handling CHECK_ACCESS_NOT_GRANTED_SPECIFIC_OPTIONAL_INPUTS = set() +# Excluding the TREAT-FINDINGS-AS-NON-BLOCKING which is a flag and needs special handling +CHECK_NO_PUBLIC_ACCESS_SPECIFIC_OPTIONAL_INPUTS = set() + VALID_POLICY_CHECK_TYPES = [ VALIDATE_POLICY, CHECK_NO_NEW_ACCESS, CHECK_ACCESS_NOT_GRANTED, + CHECK_NO_PUBLIC_ACCESS ] # Name of the output defined in the GitHub action schema @@ -99,6 +107,10 @@ def get_required_inputs(policy_check): check_specific_required_inputs = ( CHECK_ACCESS_NOT_GRANTED_SPECIFIC_REQUIRED_INPUTS ) + elif policy_check == CHECK_NO_PUBLIC_ACCESS: + check_specific_required_inputs = ( + CHECK_NO_PUBLIC_ACCESS_SPECIFIC_REQUIRED_INPUTS + ) required_inputs = COMMON_REQUIRED_INPUTS.union(check_specific_required_inputs) return required_inputs @@ -114,6 +126,10 @@ def get_optional_inputs(policy_check): check_specific_optional_inputs = ( CHECK_ACCESS_NOT_GRANTED_SPECIFIC_OPTIONAL_INPUTS ) + elif policy_check == CHECK_NO_PUBLIC_ACCESS: + check_specific_optional_inputs = ( + CHECK_NO_PUBLIC_ACCESS_SPECIFIC_OPTIONAL_INPUTS + ) optional_inputs = check_specific_optional_inputs.union(COMMON_OPTIONAL_INPUTS) return optional_inputs @@ -147,19 +163,30 @@ def get_sub_command(inputFields, areRequiredFields): flags = [] for input in inputFields: - # The default values to these environment variable when passed to docker is empty string through GitHub Actions - if os.environ[input] != "": - flag_name = get_flag_name(input) - flags.extend(["--{}".format(flag_name), os.environ[input]]) - elif areRequiredFields: - raise ValueError("Missing value for required field: {}", input) + # Checking that at least one of a set of required fields is provided + if isinstance(input, tuple): + provided = False + for field in input: + if os.environ[field] != "": + flag_name = get_flag_name(field) + flags.extend(["--{}".format(flag_name), os.environ[field]]) + provided = True + if provided == False: + raise ValueError(f"Missing value for at least one of the required fields: {str(input)}") + else: + # The default values to these environment variable when passed to docker is empty string through GitHub Actions + if os.environ[input] != "": + flag_name = get_flag_name(input) + flags.extend(["--{}".format(flag_name), os.environ[input]]) + elif areRequiredFields: + raise ValueError("Missing value for required field: {}", input) return flags def get_treat_findings_as_non_blocking_flag(policy_check): # This is specific to custom checks - CheckAccessNotGranted & CheckNoNewAccess - if policy_check in (CHECK_ACCESS_NOT_GRANTED, CHECK_NO_NEW_ACCESS): + if policy_check in (CHECK_ACCESS_NOT_GRANTED, CHECK_NO_NEW_ACCESS, CHECK_NO_PUBLIC_ACCESS): val = os.environ[TREAT_FINDINGS_AS_NON_BLOCKING] if val == "True": return ["--{}".format(get_flag_name(TREAT_FINDINGS_AS_NON_BLOCKING))] diff --git a/test_tf.py b/test_tf.py index 468f891..0257f1d 100644 --- a/test_tf.py +++ b/test_tf.py @@ -6,7 +6,7 @@ from main import get_policy_check_type, get_required_inputs, get_optional_inputs, get_sub_command from main import get_treat_findings_as_non_blocking_flag, build_command, execute_command, set_output -from main import CLI_POLICY_VALIDATOR, POLICY_CHECK_TYPE, VALIDATE_POLICY, CHECK_NO_NEW_ACCESS, CHECK_ACCESS_NOT_GRANTED +from main import CLI_POLICY_VALIDATOR, POLICY_CHECK_TYPE, VALIDATE_POLICY, CHECK_NO_NEW_ACCESS, CHECK_ACCESS_NOT_GRANTED, CHECK_NO_PUBLIC_ACCESS from main import COMMON_REQUIRED_INPUTS, TREAT_FINDINGS_AS_NON_BLOCKING from unittest.mock import patch @@ -24,7 +24,7 @@ def test_get_type_INVALID_POLICY_CHECK(self): self.assertRaises(ValueError, get_policy_check_type) # case 3, 4, 5: test_get_type_WITH_VALIDATE_POLICY: success with valid POLICY_CHECK_TYPE - @parameterized.expand([VALIDATE_POLICY, CHECK_NO_NEW_ACCESS, CHECK_ACCESS_NOT_GRANTED]) + @parameterized.expand([VALIDATE_POLICY, CHECK_NO_NEW_ACCESS, CHECK_ACCESS_NOT_GRANTED, CHECK_NO_PUBLIC_ACCESS]) def test_get_type_WITH_VALID_POLICY_CHECK(self, policy_check_type): os.environ[POLICY_CHECK_TYPE] = policy_check_type policy_type = get_policy_check_type() @@ -37,15 +37,15 @@ def test_get_required_input_INVALID_POLICY_CHECK(self): self.assertEqual(get_required_inputs(policy_check), "") # case 7, 8, 9: test_get_required_input_WITH_VALIDATE_POLICY: success as a valid POLICY_CHECK_TYPE is provided: VALIDATE_POLICY - @parameterized.expand([VALIDATE_POLICY, CHECK_NO_NEW_ACCESS, CHECK_ACCESS_NOT_GRANTED]) + @parameterized.expand([VALIDATE_POLICY, CHECK_NO_NEW_ACCESS, CHECK_ACCESS_NOT_GRANTED, CHECK_NO_PUBLIC_ACCESS]) def test_get_required_input_WITH_VALID_POLICY_CHECK(self, policy_check_type): result = get_required_inputs(policy_check_type) - if policy_check_type == VALIDATE_POLICY: - self.assertEqual(result, {"INPUT_TEMPLATE-PATH", "INPUT_REGION"}) - elif policy_check_type == CHECK_NO_NEW_ACCESS: + if policy_check_type == CHECK_NO_NEW_ACCESS: self.assertEqual(result, {"INPUT_TEMPLATE-PATH", "INPUT_REGION", "INPUT_REFERENCE-POLICY", "INPUT_REFERENCE-POLICY-TYPE"}) + elif policy_check_type == CHECK_ACCESS_NOT_GRANTED: + self.assertEqual(result, {"INPUT_TEMPLATE-PATH", "INPUT_REGION", ("INPUT_ACTIONS", "INPUT_RESOURCES")}) else: - self.assertEqual(result, {"INPUT_TEMPLATE-PATH", "INPUT_REGION", "INPUT_ACTIONS"}) + self.assertEqual(result, {"INPUT_TEMPLATE-PATH", "INPUT_REGION"}) # case 10: test_get_optional_input_INVALID_POLICY_CHECK: failure expected because an invalid policy_check_type is provided @unittest.expectedFailure @@ -55,14 +55,12 @@ def test_get_optional_input_INVALID_POLICY_CHECK(self): assertEqual(result, {"INPUT_IGNORE-FINDING", "INPUT_ALLOW-DYNAMIC-REF-WITHOUT-VERSION", "INPUT_EXCLUDE-RESOURCE-TYPES"}) # case 11, 12, 13: test_get_optional_input_WITH_VALID_POLICY_CHECK: success as a valid POLICY_CHECK_TYPE is provided - @parameterized.expand([VALIDATE_POLICY, CHECK_NO_NEW_ACCESS, CHECK_ACCESS_NOT_GRANTED]) + @parameterized.expand([VALIDATE_POLICY, CHECK_NO_NEW_ACCESS, CHECK_ACCESS_NOT_GRANTED, CHECK_NO_NEW_ACCESS]) def test_get_optional_input_WITH_VALID_POLICY_CHECK(self, policy_check_type): result = get_optional_inputs(policy_check_type) if policy_check_type == VALIDATE_POLICY: self.assertEqual(result, {"INPUT_ALLOW-EXTERNAL-PRINCIPALS", "INPUT_TREAT-FINDING-TYPE-AS-BLOCKING", "INPUT_IGNORE-FINDING", "INPUT_ALLOW-DYNAMIC-REF-WITHOUT-VERSION", "INPUT_EXCLUDE-RESOURCE-TYPES"}) - elif policy_check_type == CHECK_NO_NEW_ACCESS: - self.assertEqual(result, {"INPUT_IGNORE-FINDING", "INPUT_ALLOW-DYNAMIC-REF-WITHOUT-VERSION", "INPUT_EXCLUDE-RESOURCE-TYPES"}) else: self.assertEqual(result, {"INPUT_IGNORE-FINDING", "INPUT_ALLOW-DYNAMIC-REF-WITHOUT-VERSION", "INPUT_EXCLUDE-RESOURCE-TYPES"})