From 38e63c24b8b6b0b557a49ddfc3765bd7a3e231b8 Mon Sep 17 00:00:00 2001 From: Gene Wood Date: Fri, 18 Aug 2023 15:23:31 -0700 Subject: [PATCH 1/3] Improve error logging --- .../functions/group_role_map_builder.py | 26 +++++++++++-------- .../tests/test_get_groups_from_policy.py | 6 ++--- 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/cloudformation/group_role_map_builder/functions/group_role_map_builder.py b/cloudformation/group_role_map_builder/functions/group_role_map_builder.py index 8846d97..70ea863 100644 --- a/cloudformation/group_role_map_builder/functions/group_role_map_builder.py +++ b/cloudformation/group_role_map_builder/functions/group_role_map_builder.py @@ -147,7 +147,7 @@ def flip_map(dict_of_lists: DictOfLists) -> DictOfLists: return group_arn_map -def get_groups_from_policy(policy, aws_account_id) -> list: +def get_groups_from_policy(policy, aws_account_id, role_name) -> list: # groups will be stored as a set to prevent duplicates and then return # a list when everything is finished policy_groups = set() @@ -158,11 +158,13 @@ def get_groups_from_policy(policy, aws_account_id) -> list: try: policy = json.loads(policy) except JSONDecodeError: - logger.error("InvalidPolicyError : Can't parse JSON") + logger.error(f"InvalidPolicyError : {aws_account_id} : " + f"{role_name} : Can't parse JSON") raise InvalidPolicyError if not isinstance(policy, dict): - logger.error("InvalidPolicyError : Policy is not dict") + logger.error("InvalidPolicyError : {aws_account_id} : {role_name} : " + "Policy is not dict") raise InvalidPolicyError # If policy lacks a statement, we can bail out @@ -203,7 +205,7 @@ def get_groups_from_policy(policy, aws_account_id) -> list: # StringNotLike, etc. are not supported if operator in UNSUPPORTED_OPERATORS: logger.error( - f'UnsupportedPolicyError : {aws_account_id} ' + f'UnsupportedPolicyError : {aws_account_id} : {role_name} ' f': Condition uses operator {operator}') raise UnsupportedPolicyError # Is a valid operator and contains a valid :amr entry @@ -216,17 +218,18 @@ def get_groups_from_policy(policy, aws_account_id) -> list: # Multiple operators are not supported if operator_count > 1: logger.error( - f'UnsupportedPolicyError : {aws_account_id} : Too many ' - f'({operator_count}) operators used') + f'UnsupportedPolicyError : {aws_account_id} : {role_name} : ' + f'Too many ({operator_count}) operators used') raise UnsupportedPolicyError # An absence of operators may mean all users are permitted which isn't # supported if operator_count == 0: logger.error( - f'UnsupportedPolicyError : {aws_account_id} : Statement has ' - 'no supported amr conditions, all users permitted access. At ' - f'least one supported amr condition is required : {statement}') + f'UnsupportedPolicyError : {aws_account_id} : {role_name} : ' + f'Statement has no supported amr conditions, all users ' + f'permitted access. At least one supported amr condition is ' + f'required : {statement}') raise UnsupportedPolicyError # For clarity: @@ -245,13 +248,14 @@ def get_groups_from_policy(policy, aws_account_id) -> list: if (operator in UNGLOBBABLE_OPERATORS and set('*?') & set(''.join(groups))): logger.error( - "InvalidPolicyError : Mismatched operator and " + f"InvalidPolicyError : {aws_account_id} : " + f"{role_name} : Mismatched operator and " f"wildcards. Operator {operator} and groups " f"{groups}") raise InvalidPolicyError logger.debug( f'Valid groups {groups} found in a policy in ' - f'{aws_account_id}') + f'{aws_account_id} in {role_name}') policy_groups.update(groups) return list(policy_groups) diff --git a/cloudformation/group_role_map_builder/tests/test_get_groups_from_policy.py b/cloudformation/group_role_map_builder/tests/test_get_groups_from_policy.py index a362332..c34779b 100644 --- a/cloudformation/group_role_map_builder/tests/test_get_groups_from_policy.py +++ b/cloudformation/group_role_map_builder/tests/test_get_groups_from_policy.py @@ -47,7 +47,7 @@ def test_all_policies(monkeypatch): parsed = policy["parsed"] if "Returns" in parsed: - result = get_groups_from_policy(raw, '123456789012') + result = get_groups_from_policy(raw, '123456789012', 'ExampleRole') assert (sorted(result) == sorted(parsed["Returns"])), ( "Expected {} to return {}. Instead it returned {} where " "VALID_AMRS is {} and VALID_FEDERATED_PRINCIPAL_KEYS is " @@ -73,7 +73,7 @@ def test_all_policies(monkeypatch): try: with raises(exception): - get_groups_from_policy(raw, '123456789012') + get_groups_from_policy(raw, '123456789012', 'ExampleRole') pytest.fail( 'Expected {} to raise exception {} but it did ' 'not'.format(policy["filename"], exception)) @@ -86,4 +86,4 @@ def test_all_policies(monkeypatch): # These should be things that aren't JSON else: with raises(InvalidPolicyError): - get_groups_from_policy(raw, '123456789012') + get_groups_from_policy(raw, '123456789012', 'ExampleRole') From 724e16f82108670019233030c4bda58d5eb34939 Mon Sep 17 00:00:00 2001 From: Gene Wood Date: Fri, 18 Aug 2023 15:23:46 -0700 Subject: [PATCH 2/3] Add role export feature This adds a (by default disabled) feature which produces a full export of every discovered AWS IAM Role that is in the group role map. This full export includes the policies and the policy payload for all inline and customer managed policies. --- .../functions/group_role_map_builder.py | 97 +++++++++++++++++-- .../group_role_map_builder.yaml | 10 ++ .../tests/test_build_group_role_map.py | 6 +- 3 files changed, 102 insertions(+), 11 deletions(-) diff --git a/cloudformation/group_role_map_builder/functions/group_role_map_builder.py b/cloudformation/group_role_map_builder/functions/group_role_map_builder.py index 70ea863..36c0951 100644 --- a/cloudformation/group_role_map_builder/functions/group_role_map_builder.py +++ b/cloudformation/group_role_map_builder/functions/group_role_map_builder.py @@ -22,9 +22,11 @@ 'S3_BUCKET_NAME': None, 'S3_FILE_PATH_GROUP_ROLE_MAP': 'access-group-iam-role-map.json', 'S3_FILE_PATH_ALIAS_MAP': 'account-aliases.json', + 'S3_FILE_PATH_ROLES_EXPORT': 'roles-export.json', 'S3_FILE_PATH_MANUAL_ALIAS_MAP': 'manual-account-aliases.json', 'VALID_AMRS': '', 'VALID_FEDERATED_PRINCIPAL_URLS': '', + 'EXPORT_ROLES': 'False' } COMMA_DELIMITED_VARIABLES = ['VALID_AMRS', 'VALID_FEDERATED_PRINCIPAL_URLS'] UNGLOBBABLE_OPERATORS = ("StringEquals", "ForAnyValue:StringEquals") @@ -50,7 +52,7 @@ SimpleDict = Dict[str, str] DictOfLists = Dict[str, list] -TupleOfDictOflists = Tuple[DictOfLists, DictOfLists] +CustomTuple = Tuple[DictOfLists, DictOfLists, List] class InvalidPolicyError(Exception): @@ -69,6 +71,10 @@ def get_setting(name): return value +def get_account_id(arn: str) -> str: + return arn.split(':')[4] + + def is_valid_identity_provider(arn: str, aws_account_id: str) -> bool: """Return whether or not the identity provider ARN is valid @@ -178,7 +184,9 @@ def get_groups_from_policy(policy, aws_account_id, role_name) -> list: 'Skipping policy statement with Effect {}'.format( statement.get("Effect"))) continue - if type(statement.get("Action", '')) == str and statement.get("Action", '').lower() != "sts:AssumeRoleWithWebIdentity".lower(): + if (type(statement.get("Action", '')) == str + and statement.get("Action", '').lower() != + "sts:AssumeRoleWithWebIdentity".lower()): # logger.debug( # 'Skipping policy statement with Action {}'.format( # statement.get("Action"))) @@ -261,6 +269,51 @@ def get_groups_from_policy(policy, aws_account_id, role_name) -> list: return list(policy_groups) +def get_role_policies(arn: str, credentials: Dict) -> List: + """Given an AWS IAM Role ARN and credential dictionary, return a list of policy dicts + + :param arn: The ARN of the AWS IAM Role + :param credentials: A dictionary of AWS API key arguments + :return: A List of dicts containing info on each policy + """ + policy_list = [] + client = boto3.client('iam', **credentials) + role_name = arn.split(':')[5].split('/')[1] + + # Inline policies + inline_policy_names = get_paginated_results( + 'iam', 'list_role_policies', 'PolicyNames', credentials, {'RoleName': role_name}) + for policy_name in inline_policy_names: + response = client.get_role_policy( + RoleName=role_name, + PolicyName=policy_name + ) + policy_list.append({ + 'policy_name': policy_name, + 'policy_type': 'inline', + 'policy_payload': response['PolicyDocument'] + }) + + # Managed policies + response = get_paginated_results( + 'iam', 'list_attached_role_policies', 'AttachedPolicies', credentials, {'RoleName': role_name}) + attached_policy_arns = [x['PolicyArn'] for x in response] + for policy_arn in attached_policy_arns: + policy_metadata = client.get_policy(PolicyArn=policy_arn) + policy_item = { + 'policy_name': policy_metadata['Policy']['PolicyName'], + 'policy_type': 'amazon_managed' if get_account_id(policy_arn) == 'aws' else 'customer_managed', + 'policy_arn': policy_arn + } + if policy_item['policy_type'] == 'customer_managed': + policy_item['policy_payload'] = client.get_policy_version( + PolicyArn=policy_arn, + VersionId=policy_metadata['Policy']['DefaultVersionId'] + )['PolicyVersion']['Document'] + policy_list.append(policy_item) + return policy_list + + def get_s3_file( s3_bucket: str, s3_key: str, @@ -333,7 +386,7 @@ def store_s3_file(s3_bucket: str, return False -def build_group_role_map(assumed_role_arns: List[str]) -> TupleOfDictOflists: +def build_group_role_map(assumed_role_arns: List[str]) -> CustomTuple: """Build map of IAM roles to OIDC groups used in assumption policies. Given a list of IAM Role ARNs to assume, iterate over those roles, @@ -354,12 +407,14 @@ def build_group_role_map(assumed_role_arns: List[str]) -> TupleOfDictOflists: } :param list assumed_role_arns: list of IAM role ARN strings - :return: a tuple of the map of IAM ARNs to related OIDC claimed group names - followed by the map of AWS account IDs to account aliases + :return: a tuple of the map of IAM Role ARNs to related OIDC claimed group names + followed by the map of AWS account IDs to account aliases, followed + by a summary of all policies in all federated IAM Roles """ assumed_role_credentials = {} role_group_map = {} alias_map = {} + roles_export = {} for assumed_role_arn in assumed_role_arns: aws_account_id = assumed_role_arn.split(':')[4] logger.debug(f'Fetching policies from {aws_account_id}') @@ -368,7 +423,15 @@ def build_group_role_map(assumed_role_arns: List[str]) -> TupleOfDictOflists: 'Version': '2012-10-17', 'Statement': [ {'Effect': 'Allow', - 'Action': ['iam:ListRoles', 'iam:ListAccountAliases'], + 'Action': [ + 'iam:ListRoles', + 'iam:ListRolePolicies', + 'iam:GetRolePolicy', + 'iam:ListAttachedRolePolicies', + 'iam:GetPolicyVersion', + 'iam:ListAccountAliases', + 'iam:GetPolicy' + ], 'Resource': '*'} ], } @@ -410,7 +473,18 @@ def build_group_role_map(assumed_role_arns: List[str]) -> TupleOfDictOflists: f'{role["RoleName"]} in AWS account {aws_account_id}') groups = get_groups_from_policy( role['AssumeRolePolicyDocument'], - aws_account_id) + aws_account_id, + role['RoleName'] + ) + if groups and get_setting('EXPORT_ROLES').lower() == 'true': + if get_account_id(role['Arn']) not in roles_export: + roles_export[get_account_id(role['Arn'])] = {} + roles_export[get_account_id(role['Arn'])][role['Arn']] = { + 'trusted_entities_payload': role['AssumeRolePolicyDocument'], + 'policies': get_role_policies( + role['Arn'], + assumed_role_credentials[get_account_id(role['Arn'])]) + } except UnsupportedPolicyError: # a policy intended to work with the right IdP but with # conditions beyond what we can handle @@ -435,7 +509,7 @@ def build_group_role_map(assumed_role_arns: List[str]) -> TupleOfDictOflists: continue role_group_map[role['Arn']] = groups alias_map[aws_account_id] = aliases - return flip_map(role_group_map), alias_map + return flip_map(role_group_map), alias_map, roles_export def get_security_audit_role_arns() -> List[str]: @@ -471,7 +545,7 @@ def lambda_handler(event, context): security_audit_role_arns = get_security_audit_role_arns() logger.debug( f'IAM Role ARNs fetched from table : {security_audit_role_arns}') - group_role_map, generated_alias_map = build_group_role_map( + group_role_map, generated_alias_map, roles_export = build_group_role_map( security_audit_role_arns) manual_alias_map = manual_alias_map = get_s3_file( get_setting('S3_BUCKET_NAME'), @@ -487,6 +561,11 @@ def lambda_handler(event, context): get_setting('S3_FILE_PATH_ALIAS_MAP'), alias_map, False) + if group_role_map_changed and roles_export: + store_s3_file( + get_setting('S3_BUCKET_NAME'), + get_setting('S3_FILE_PATH_ROLES_EXPORT'), + roles_export) if group_role_map_changed: logger.info( f'Group role map in S3 updated : {serialize_map(group_role_map)}') diff --git a/cloudformation/group_role_map_builder/group_role_map_builder.yaml b/cloudformation/group_role_map_builder/group_role_map_builder.yaml index 27730af..1f5846a 100644 --- a/cloudformation/group_role_map_builder/group_role_map_builder.yaml +++ b/cloudformation/group_role_map_builder/group_role_map_builder.yaml @@ -15,6 +15,7 @@ Metadata: Parameters: - S3BucketName - GroupRoleMapS3FilePath + - RolesExportS3FilePath - AccountAliasesS3FilePath - ManualAccountAliasesS3FilePath - Label: @@ -31,6 +32,8 @@ Metadata: default: S3 Bucket Name GroupRoleMapS3FilePath: default: Group Role Map S3 File Path + RolesExportS3FilePath: + default: Roles Export File Path AccountAliasesS3FilePath: default: AWS Account Alias Map S3 File Path ManualAccountAliasesS3FilePath: @@ -51,6 +54,10 @@ Parameters: Type: String Description: The path to the group role map file Default: access-group-iam-role-map.json + RolesExportS3FilePath: + Type: String + Description: The path to the role export file + Default: roles-export.json AccountAliasesS3FilePath: Type: String Description: The path to the account aliases map file @@ -115,6 +122,7 @@ Resources: - s3:PutObject Resource: - !Join ['', ['arn:aws:s3:::', !Ref 'S3BucketName', '/', !Ref 'GroupRoleMapS3FilePath']] + - !Join ['', ['arn:aws:s3:::', !Ref 'S3BucketName', '/', !Ref 'RolesExportS3FilePath']] - !Join ['', ['arn:aws:s3:::', !Ref 'S3BucketName', '/', !Ref 'AccountAliasesS3FilePath']] - !Join ['', ['arn:aws:s3:::', !Ref 'S3BucketName', '/', !Ref 'ManualAccountAliasesS3FilePath']] - Effect: Allow @@ -141,6 +149,7 @@ Resources: TABLE_REGION: !Ref StackEmissionDynamoDBTableRegion S3_BUCKET_NAME: !Ref S3BucketName S3_FILE_PATH_GROUP_ROLE_MAP: !Ref GroupRoleMapS3FilePath + S3_FILE_PATH_ROLES_EXPORT: !Ref RolesExportS3FilePath S3_FILE_PATH_ALIAS_MAP: !Ref AccountAliasesS3FilePath S3_FILE_PATH_MANUAL_ALIAS_MAP: !Ref ManualAccountAliasesS3FilePath VALID_FEDERATED_PRINCIPAL_URLS: !Ref ProviderUrls @@ -195,6 +204,7 @@ Resources: - s3:GetObject Resource: - !Join ['', ['arn:aws:s3:::', !Ref 'S3BucketName', '/', !Ref 'GroupRoleMapS3FilePath']] + - !Join ['', ['arn:aws:s3:::', !Ref 'S3BucketName', '/', !Ref 'RolesExportS3FilePath']] - !Join ['', ['arn:aws:s3:::', !Ref 'S3BucketName', '/', !Ref 'AccountAliasesS3FilePath']] - Effect: Allow Action: diff --git a/cloudformation/group_role_map_builder/tests/test_build_group_role_map.py b/cloudformation/group_role_map_builder/tests/test_build_group_role_map.py index 63885de..68d071a 100644 --- a/cloudformation/group_role_map_builder/tests/test_build_group_role_map.py +++ b/cloudformation/group_role_map_builder/tests/test_build_group_role_map.py @@ -52,13 +52,13 @@ def test_get_role_group_map(): AssumeRolePolicyDocument=assume_role_policy_document_with_conditions, Description='Test role with federated conditions', ) - groups, aliases = build_group_role_map([role_to_assume_arn]) + groups, aliases, roles_export = build_group_role_map([role_to_assume_arn]) assert len(groups) == 0 assert list(aliases.values()) == [[]] response = client.create_account_alias(AccountAlias='account-alias-test') - groups, aliases = build_group_role_map([role_to_assume_arn]) + groups, aliases, roles_export = build_group_role_map([role_to_assume_arn]) assert list(aliases.values()) == [['account-alias-test']] # Enable these tests once get_federated_groups_for_policy is written @@ -68,3 +68,5 @@ def test_get_role_group_map(): # TODO : Add a test to confirm that when 2 roles are encountered where one # is invalid/unsupported, the other role still gets processed + + # TODO : Add a test for the roles_export From f826fa69594aaeb7b264906c4ecebb2ef163ae24 Mon Sep 17 00:00:00 2001 From: Gene Wood Date: Fri, 18 Aug 2023 15:33:22 -0700 Subject: [PATCH 3/3] Update GitHub actions version to account for Nodejs 12 deprecation --- .github/workflows/build-publish.yml | 2 +- .github/workflows/test.yml | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/build-publish.yml b/.github/workflows/build-publish.yml index 0e5c37d..496c827 100644 --- a/.github/workflows/build-publish.yml +++ b/.github/workflows/build-publish.yml @@ -12,7 +12,7 @@ jobs: - name: Install build dependencies run: python3 -m pip install --upgrade wheel setuptools - name: Checkout code - uses: actions/checkout@v2 + uses: actions/checkout@v3 - name: Build package run: python3 setup.py sdist bdist_wheel --universal - name: Publish package to PyPI diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 626e72f..6bd6b54 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -24,7 +24,7 @@ jobs: steps: - uses: actions/checkout@v3 - name: Set up Python ${{ matrix.python-version }} - uses: actions/setup-python@v3 + uses: actions/setup-python@v4 with: python-version: ${{ matrix.python-version }} cache: 'pip' @@ -45,7 +45,7 @@ jobs: steps: - uses: actions/checkout@v3 - name: Set up Python ${{ matrix.python-version }} - uses: actions/setup-python@v2 + uses: actions/setup-python@v4 with: python-version: ${{ matrix.python-version }} cache: 'pip'