From 6483c32ce7a422b67461d94486639d7f725c003d Mon Sep 17 00:00:00 2001 From: Scott Piper Date: Mon, 8 Nov 2021 13:12:25 -0700 Subject: [PATCH] Ran black --- auditor/resources/alarm_forwarder/main.py | 14 +++-- commands/access_check.py | 15 +++-- commands/amis.py | 6 +- commands/api_endpoints.py | 4 +- commands/audit.py | 6 +- commands/collect.py | 49 ++++++++++----- commands/configure.py | 4 +- commands/iam_report.py | 10 ++- commands/prepare.py | 14 +++-- commands/public.py | 2 +- commands/sg_ips.py | 12 +++- commands/weboftrust.py | 76 ++++++++++++++--------- shared/audit.py | 27 +++++--- shared/common.py | 22 +++---- shared/find_unused.py | 52 +++++++++++++--- shared/iam_audit.py | 70 ++++++++++++++------- shared/organization.py | 42 ++++++------- shared/public.py | 10 +-- tests/__init__.py | 2 +- tests/unit/test_audit.py | 2 +- tests/unit/test_find_unused.py | 34 +++++----- utils/strings.py | 19 +++--- utils/toslack.py | 23 ++++--- 23 files changed, 316 insertions(+), 199 deletions(-) diff --git a/auditor/resources/alarm_forwarder/main.py b/auditor/resources/alarm_forwarder/main.py index 5677d1073..0435c86a2 100644 --- a/auditor/resources/alarm_forwarder/main.py +++ b/auditor/resources/alarm_forwarder/main.py @@ -1,11 +1,13 @@ import boto3 import os + def handler(event, context): - sns = boto3.client('sns') - for record in event['Records']: + sns = boto3.client("sns") + for record in event["Records"]: sns.publish( - TopicArn=str(os.environ['ALARM_SNS']), - Message=record['Sns']['Message'], - Subject=record['Sns']['Subject']) - return True \ No newline at end of file + TopicArn=str(os.environ["ALARM_SNS"]), + Message=record["Sns"]["Message"], + Subject=record["Sns"]["Subject"], + ) + return True diff --git a/commands/access_check.py b/commands/access_check.py index 1f447de7d..35be45cde 100644 --- a/commands/access_check.py +++ b/commands/access_check.py @@ -20,7 +20,7 @@ def replace_principal_variables(reference, principal): """ - Given a resource reference string (ie. the Resource string from an IAM policy) and a prinicipal, replace any variables in the resource string that are principal related. + Given a resource reference string (ie. the Resource string from an IAM policy) and a prinicipal, replace any variables in the resource string that are principal related. """ reference = reference.lower() for tag in principal.tags: @@ -189,22 +189,22 @@ class Principal: @property def tags(self): - """ aws:PrincipalTag """ + """aws:PrincipalTag""" return self._tags @property def mytype(self): - """ aws:PrincipalType """ + """aws:PrincipalType""" return self._type @property def username(self): - """ aws:username """ + """aws:username""" return self._username @property def userid(self): - """ aws:userid """ + """aws:userid""" return self._userid def __init__(self, mytype, tags, username="", userid=""): @@ -422,7 +422,7 @@ def access_check_command(accounts, config, args): def get_managed_policy(iam, policy_arn): """ - Given the IAM data for an account and the ARN for a policy, + Given the IAM data for an account and the ARN for a policy, return the policy document """ for policy in iam["Policies"]: @@ -457,8 +457,7 @@ def is_allowed(privilege_prefix, privilege_name, statements): def get_allowed_privileges( privilege_matches, privileged_statements, boundary_statements ): - """ - """ + """ """ allowed_privileges = [] for privilege in privilege_matches: if boundary_statements is not None: diff --git a/commands/amis.py b/commands/amis.py index 72ca188ae..35c9e6ae5 100644 --- a/commands/amis.py +++ b/commands/amis.py @@ -77,8 +77,8 @@ def amis(args, accounts, config): ) if args.instance_filter != "": resource_filter += "|{}".format(args.instance_filter) - - if 'Reservations' not in instances: + + if "Reservations" not in instances: print(f"** skipping: {account.name} in {region_name}") continue @@ -86,7 +86,7 @@ def amis(args, accounts, config): account_images = query_aws(account, "ec2-describe-images", region) resource_filter = ".Images[]" - if 'Images' not in account_images: + if "Images" not in account_images: print(f"** skipping: {account.name} in {region_name}") continue account_images = pyjq.all(resource_filter, account_images) diff --git a/commands/api_endpoints.py b/commands/api_endpoints.py index 9a2053a6c..4efa053a0 100644 --- a/commands/api_endpoints.py +++ b/commands/api_endpoints.py @@ -50,6 +50,8 @@ def api_endpoints(accounts, config): def run(arguments): - print("*** DEPRECARTED: Not enough of data is collected for this command to run successfully ***\n\n") + print( + "*** DEPRECARTED: Not enough of data is collected for this command to run successfully ***\n\n" + ) _, accounts, config = parse_arguments(arguments) api_endpoints(accounts, config) diff --git a/commands/audit.py b/commands/audit.py index b4e16acb9..91886f489 100644 --- a/commands/audit.py +++ b/commands/audit.py @@ -23,7 +23,7 @@ def audit_command(accounts, config, args): if args.json: finding = json.loads(str(finding)) - finding['finding_type_metadata']= conf + finding["finding_type_metadata"] = conf print(json.dumps(finding, sort_keys=True)) elif args.markdown: print( @@ -35,7 +35,7 @@ def audit_command(accounts, config, args): finding.region.name, conf["description"], finding.resource_id, - str(finding.resource_details).replace('\n', '\\n') + str(finding.resource_details).replace("\n", "\\n"), ) ) else: @@ -68,7 +68,7 @@ def run(arguments): "--minimum_severity", help="Only report issues that are greater than this. Default: INFO", default="INFO", - choices=['CRITICAL', 'HIGH', 'MEDIUM', 'LOW', 'INFO', 'MUTE'] + choices=["CRITICAL", "HIGH", "MEDIUM", "LOW", "INFO", "MUTE"], ) args, accounts, config = parse_arguments(arguments, parser) diff --git a/commands/collect.py b/commands/collect.py index 5376824df..073914f4e 100644 --- a/commands/collect.py +++ b/commands/collect.py @@ -18,6 +18,7 @@ MAX_RETRIES = 3 + def snakecase(s): return s.replace("-", "_") @@ -183,7 +184,7 @@ def call_function(outputfile, handler, method_to_call, parameters, check, summar ): print(" - Securityhub is not enabled") elif "AWSOrganizationsNotInUseException" in str(e): - print(' - Your account is not a member of an organization.') + print(" - Your account is not a member of an organization.") else: print("ClientError: {}".format(e), flush=True) call_summary["exception"] = e @@ -223,12 +224,12 @@ def collect(arguments): # Identify the default region used by global services such as IAM default_region = os.environ.get("AWS_REGION", "us-east-1") - if 'gov-' in default_region: - default_region = 'us-gov-west-1' - elif 'cn-' in default_region: - default_region = 'cn-north-1' + if "gov-" in default_region: + default_region = "us-gov-west-1" + elif "cn-" in default_region: + default_region = "cn-north-1" else: - default_region = 'us-east-1' + default_region = "us-east-1" regions_filter = None if len(arguments.regions_filter) > 0: @@ -290,7 +291,9 @@ def collect(arguments): region_list = ec2.describe_regions() if regions_filter is not None: - filtered_regions = [r for r in region_list["Regions"] if r["RegionName"] in regions_filter] + filtered_regions = [ + r for r in region_list["Regions"] if r["RegionName"] in regions_filter + ] region_list["Regions"] = filtered_regions with open("account-data/{}/describe-regions.json".format(account_dir), "w+") as f: @@ -344,8 +347,9 @@ def collect(arguments): ) continue handler = session.client( - runner["Service"], region_name=region["RegionName"], - config=Config(retries={'max_attempts': arguments.max_attempts}) + runner["Service"], + region_name=region["RegionName"], + config=Config(retries={"max_attempts": arguments.max_attempts}), ) filepath = "account-data/{}/{}/{}-{}".format( @@ -382,7 +386,9 @@ def collect(arguments): # For each cluster, read the `ecs list-tasks` for clusterArn in list_clusters["clusterArns"]: cluster_path = ( - action_path + "/" + urllib.parse.quote_plus(clusterArn) + action_path + + "/" + + urllib.parse.quote_plus(clusterArn) ) make_directory(cluster_path) @@ -418,7 +424,10 @@ def collect(arguments): runner.get("Check", None), summary, ) - elif runner["Service"] == "route53" and runner["Request"] == "list-hosted-zones-by-vpc": + elif ( + runner["Service"] == "route53" + and runner["Request"] == "list-hosted-zones-by-vpc" + ): action_path = filepath make_directory(action_path) @@ -432,7 +441,9 @@ def collect(arguments): # For each region for collect_region in describe_regions["Regions"]: cluster_path = ( - action_path + "/" + urllib.parse.quote_plus(collect_region["RegionName"]) + action_path + + "/" + + urllib.parse.quote_plus(collect_region["RegionName"]) ) make_directory(cluster_path) @@ -440,7 +451,7 @@ def collect(arguments): describe_vpcs_file = "account-data/{}/{}/{}".format( account_dir, collect_region["RegionName"], - "ec2-describe-vpcs.json" + "ec2-describe-vpcs.json", ) if os.path.isfile(describe_vpcs_file): @@ -451,13 +462,17 @@ def collect(arguments): outputfile = ( action_path + "/" - + urllib.parse.quote_plus(collect_region["RegionName"]) + + urllib.parse.quote_plus( + collect_region["RegionName"] + ) + "/" + urllib.parse.quote_plus(vpc["VpcId"]) ) call_parameters = {} - call_parameters["VPCRegion"] = collect_region["RegionName"] + call_parameters["VPCRegion"] = collect_region[ + "RegionName" + ] call_parameters["VPCId"] = vpc["VpcId"] call_function( outputfile, @@ -588,7 +603,7 @@ def run(arguments): required=False, type=int, dest="max_attempts", - default=4 + default=4, ) parser.add_argument( "--regions", @@ -596,7 +611,7 @@ def run(arguments): required=False, type=str, dest="regions_filter", - default="" + default="", ) args = parser.parse_args(arguments) diff --git a/commands/configure.py b/commands/configure.py index 53faf8269..17b3d58fd 100644 --- a/commands/configure.py +++ b/commands/configure.py @@ -68,9 +68,9 @@ def condition(x, y): current_account_ids = set(map(lambda entry: entry["id"], current_accounts)) for organization_account in organization_accounts: # Don't overwrite any account already in the configuration file - if organization_account['id'] not in current_account_ids: + if organization_account["id"] not in current_account_ids: config["accounts"].append(organization_account) - + with open(arguments.config_file, "w+") as f: f.write(json.dumps(config, indent=4, sort_keys=True)) diff --git a/commands/iam_report.py b/commands/iam_report.py index d66fde97c..0edd5c3aa 100644 --- a/commands/iam_report.py +++ b/commands/iam_report.py @@ -226,7 +226,7 @@ def __init__(self, auth, auth_graph): auth_graph[policy_node.key()] = policy_node for group_name in auth.get("GroupList", []): - group_key = self.key()[0:26] + "group" + auth['Path'] + group_name + group_key = self.key()[0:26] + "group" + auth["Path"] + group_name group_node = auth_graph[group_key] group_node.add_parent(self) self.add_child(group_node) @@ -632,7 +632,11 @@ def iam_report(accounts, config, args): with open("{}.json".format(REPORT_OUTPUT_FILE), "w") as f: json.dump(t, f) - print("Report written to {}.{}".format(REPORT_OUTPUT_FILE, args.requested_output.value)) + print( + "Report written to {}.{}".format( + REPORT_OUTPUT_FILE, args.requested_output.value + ) + ) def run(arguments): @@ -654,7 +658,7 @@ def run(arguments): help="Set the output type for the report. [json | html]. Default: html", default=OutputFormat.html, type=OutputFormat, - dest="requested_output" + dest="requested_output", ) parser.set_defaults(show_graph=False) args, accounts, config = parse_arguments(arguments, parser) diff --git a/commands/prepare.py b/commands/prepare.py index 1d4888358..2ad315fd3 100644 --- a/commands/prepare.py +++ b/commands/prepare.py @@ -327,7 +327,11 @@ def add_node_to_subnets(region, node, nodes): # Add a new node (potentially the same one) back to the dictionary for vpc in region.children: - if len(node.subnets) == 0 and node._parent and vpc.local_id == node._parent.local_id: + if ( + len(node.subnets) == 0 + and node._parent + and vpc.local_id == node._parent.local_id + ): # VPC Gateway Endpoints (S3 and DynamoDB) reside in a VPC, not a subnet # So set the relationship between the VPC and the node nodes[node.arn] = node @@ -658,16 +662,16 @@ def build_data_structure(account_data, config, outputfilter): def prepare(account, config, outputfilter): - """ NO LONGER MAINTAINED + """NO LONGER MAINTAINED Collect the data and write it to a file """ log("WARNING: This functionality is no longer maintained") cytoscape_json = build_data_structure(account, config, outputfilter) if not outputfilter["node_data"]: - filtered_cytoscape_json=[] + filtered_cytoscape_json = [] for node in cytoscape_json: filtered_node = node.copy() - filtered_node['data']['node_data'] = {} + filtered_node["data"]["node_data"] = {} filtered_cytoscape_json.append(filtered_node) cytoscape_json = filtered_cytoscape_json with open("web/data.json", "w") as outfile: @@ -675,7 +679,7 @@ def prepare(account, config, outputfilter): def run(arguments): - """ NO LONGER MAINTAINED """ + """NO LONGER MAINTAINED""" log("WARNING: This functionality is no longer maintained") # Parse arguments parser = argparse.ArgumentParser() diff --git a/commands/public.py b/commands/public.py index 5e8bf9a6f..38f777e97 100644 --- a/commands/public.py +++ b/commands/public.py @@ -16,7 +16,7 @@ def public(accounts, config): all_accounts.append(public_node) for warning in warnings: print("WARNING: {}".format(warning), file=sys.stderr) - + print(json.dumps(all_accounts, indent=4, sort_keys=True)) diff --git a/commands/sg_ips.py b/commands/sg_ips.py index ce72c9d87..f4da0eec6 100644 --- a/commands/sg_ips.py +++ b/commands/sg_ips.py @@ -3,14 +3,22 @@ from netaddr import IPNetwork import pyjq -from shared.common import parse_arguments, query_aws, get_regions, is_external_cidr, is_unblockable_cidr +from shared.common import ( + parse_arguments, + query_aws, + get_regions, + is_external_cidr, + is_unblockable_cidr, +) from shared.nodes import Account, Region # TODO: Considering removing this command. The warnings now live in the audit code. # The creation of the map and table of locations is all this does now, which is both # not very valuable, and is difficult to setup (requires matplotlib, basemap data, and geoip data) -__description__ = "[Deprecated] Find all IPs are that are given trusted access via Security Groups" +__description__ = ( + "[Deprecated] Find all IPs are that are given trusted access via Security Groups" +) def get_cidrs_for_account(account, cidrs): diff --git a/commands/weboftrust.py b/commands/weboftrust.py index b1b05d97b..d5391803f 100644 --- a/commands/weboftrust.py +++ b/commands/weboftrust.py @@ -5,7 +5,13 @@ import pyjq import urllib.parse -from shared.common import parse_arguments, make_list, query_aws, get_regions, get_account_by_id +from shared.common import ( + parse_arguments, + make_list, + query_aws, + get_regions, + get_account_by_id, +) __description__ = "Create Web Of Trust diagram for accounts" @@ -174,9 +180,7 @@ def get_iam_trusts(account, nodes, connections, connections_to_get): ) saml_providers = query_aws( - account, - "iam-list-saml-providers", - Region(account, {"RegionName": "us-east-1"}) + account, "iam-list-saml-providers", Region(account, {"RegionName": "us-east-1"}) )["SAMLProviderList"] for role in pyjq.all(".RoleDetailList[]", iam): @@ -195,19 +199,19 @@ def get_iam_trusts(account, nodes, connections, connections_to_get): # WoT will show us the direction of that trust for further inspection. # this enables cross_account_admin_sts (STS between accounts) for saml in saml_providers: - if saml['Arn'] == federated_principal: - saml_provider_arn = saml['Arn'] - elif get_account_by_id(account_id=federated_principal.split(':')[4]): - if get_account_by_id(account_id=saml['Arn'].split(':')[4]): - saml_provider_arn = saml['Arn'] - - if 'saml-provider/okta' in saml_provider_arn.lower(): + if saml["Arn"] == federated_principal: + saml_provider_arn = saml["Arn"] + elif get_account_by_id( + account_id=federated_principal.split(":")[4] + ): + if get_account_by_id( + account_id=saml["Arn"].split(":")[4] + ): + saml_provider_arn = saml["Arn"] + + if "saml-provider/okta" in saml_provider_arn.lower(): node = Account( - json_blob={ - "id": "okta", - "name": "okta", - "type": "Okta" - } + json_blob={"id": "okta", "name": "okta", "type": "Okta"} ) assume_role_nodes.add(node) elif "saml-provider/onelogin" in saml_provider_arn.lower(): @@ -248,19 +252,15 @@ def get_iam_trusts(account, nodes, connections, connections_to_get): assume_role_nodes.add(node) elif "saml-provider/adfs" in saml_provider_arn.lower(): node = Account( - json_blob={ - "id": "adfs", - "name": "adfs", - "type": "ADFS" - } + json_blob={"id": "adfs", "name": "adfs", "type": "ADFS"} ) assume_role_nodes.add(node) elif "saml-provider/auth0" in saml_provider_arn.lower(): node = Account( json_blob={ - "id": "auth0", - "name": "auth0", - "type": "auth0" + "id": "auth0", + "name": "auth0", + "type": "auth0", } ) assume_role_nodes.add(node) @@ -282,7 +282,10 @@ def get_iam_trusts(account, nodes, connections, connections_to_get): } ) assume_role_nodes.add(node) - elif "cognito-identity.amazonaws.com" in saml_provider_arn.lower(): + elif ( + "cognito-identity.amazonaws.com" + in saml_provider_arn.lower() + ): continue elif "www.amazon.com" in saml_provider_arn.lower(): node = Account( @@ -295,18 +298,27 @@ def get_iam_trusts(account, nodes, connections, connections_to_get): continue else: raise Exception( - "Unknown federation provider: {}".format(saml_provider_arn.lower()) + "Unknown federation provider: {}".format( + saml_provider_arn.lower() + ) ) except (StopIteration, IndexError): - if "cognito-identity.amazonaws.com" in federated_principal.lower(): + if ( + "cognito-identity.amazonaws.com" + in federated_principal.lower() + ): # TODO: Should show this somehow continue elif ":oidc-provider/" in federated_principal.lower(): # TODO: handle OpenID Connect identity providers # https://docs.aws.amazon.com/IAM/latest/UserGuide/id_roles_providers_create_oidc.html continue - raise Exception('Principal {} is not a configured SAML provider'.format(federated_principal)) + raise Exception( + "Principal {} is not a configured SAML provider".format( + federated_principal + ) + ) if principal.get("AWS", None): principal = principal["AWS"] if not isinstance(principal, list): @@ -480,8 +492,12 @@ def weboftrust(args, accounts, config): if not was_scanned: for vendor in vendor_accounts: if n.id in vendor["accounts"]: - if 'source' not in vendor: - print("WARNING: Unconfirmed vendor: {} ({})".format(vendor['name'], n.id)) + if "source" not in vendor: + print( + "WARNING: Unconfirmed vendor: {} ({})".format( + vendor["name"], n.id + ) + ) n.name = vendor["name"] n.type = vendor.get("type", vendor["name"]) diff --git a/shared/audit.py b/shared/audit.py index c7c7c6899..0154a7020 100644 --- a/shared/audit.py +++ b/shared/audit.py @@ -558,7 +558,10 @@ def audit_route53(findings, region): regions = pyjq.all(".Regions[].RegionName", regions_json) for region_name in regions: vpc_json = query_aws(region.account, "ec2-describe-vpcs", region_name) - vpcs = pyjq.all('.Vpcs[]? | select(.OwnerId=="{}").VpcId'.format(region.account.local_id), vpc_json) + vpcs = pyjq.all( + '.Vpcs[]? | select(.OwnerId=="{}").VpcId'.format(region.account.local_id), + vpc_json, + ) for vpc in vpcs: hosted_zone_file = f"account-data/{region.account.name}/{region.name}/route53-list-hosted-zones-by-vpc/{region_name}/{vpc}" hosted_zones_json = json.load(open(hosted_zone_file)) @@ -567,8 +570,15 @@ def audit_route53(findings, region): if hosted_zone.get("Owner", {}).get("OwningAccount", "") != "": if hosted_zone["Owner"]["OwningAccount"] != region.account.local_id: findings.add( - Finding(region, "FOREIGN_HOSTED_ZONE", hosted_zone, - resource_datails={"vpc_id": vpc, "vpc_regions": region_name}) + Finding( + region, + "FOREIGN_HOSTED_ZONE", + hosted_zone, + resource_datails={ + "vpc_id": vpc, + "vpc_regions": region_name, + }, + ) ) @@ -722,7 +732,7 @@ def audit_es(findings, region): ) # Find the entity we need policy_string = policy_file_json["DomainStatus"]["AccessPolicies"] - if policy_string == '': + if policy_string == "": policy_string = "{}" # Load the string value as json policy = json.loads(policy_string) @@ -732,7 +742,8 @@ def audit_es(findings, region): # they are VPC-only, in which case they have an "Endpoints" (plural) array containing a "vpc" element if ( policy_file_json["DomainStatus"].get("Endpoint", "") != "" - or policy_file_json["DomainStatus"].get("Endpoints", {}).get("vpc", "") == "" + or policy_file_json["DomainStatus"].get("Endpoints", {}).get("vpc", "") + == "" ): if policy.is_internet_accessible() or policy_string == "{}": findings.add( @@ -850,7 +861,9 @@ def audit_elbv1(findings, region): region, "elb", "describe-load-balancer-attributes", lb_name ) - for attribute in attributes_json.get("LoadBalancerAttributes", [])['AdditionalAttributes']: + for attribute in attributes_json.get("LoadBalancerAttributes", [])[ + "AdditionalAttributes" + ]: if ( attribute["Key"] == "elb.http.desyncmitigationmode" and attribute["Value"] != "strictest" @@ -940,7 +953,7 @@ def audit_sg(findings, region): "SG description": sg["Description"], "SG tags": sg.get("Tags", {}), "cidr1": cidr, - "cidr2": cidr_seen + "cidr2": cidr_seen, }, ) ) diff --git a/shared/common.py b/shared/common.py index e41e08cf3..f10338361 100644 --- a/shared/common.py +++ b/shared/common.py @@ -79,7 +79,6 @@ class InvalidAccountData(Exception): """Raised when collect results are missing or malformed""" - class Finding(object): """Used for auditing""" @@ -210,11 +209,7 @@ def get_account_by_id(account_id, config=None, config_filename="config.json"): config_filename ) ) - exit( - 'ERROR: Account ID "{}" not found in {}'.format( - account_id, config_filename - ) - ) + exit('ERROR: Account ID "{}" not found in {}'.format(account_id, config_filename)) def parse_arguments(arguments, parser=None): @@ -342,7 +337,7 @@ def get_us_east_1(account): def iso_date(d): - """ Convert ISO format date string such as 2018-04-08T23:33:20+00:00""" + """Convert ISO format date string such as 2018-04-08T23:33:20+00:00""" time_format = "%Y-%m-%dT%H:%M:%S" return datetime.datetime.strptime(d.split("+")[0], time_format) @@ -363,7 +358,9 @@ def get_collection_date(account): ) if not json_blob: raise InvalidAccountData( - "File iam-get-credential-report.json does not exist or is not well-formed. Likely cause is you did not run the collect command for account {}".format(account.name) + "File iam-get-credential-report.json does not exist or is not well-formed. Likely cause is you did not run the collect command for account {}".format( + account.name + ) ) # GeneratedTime looks like "2019-01-30T15:43:24+00:00" @@ -399,9 +396,13 @@ def get_access_advisor_active_counts(account, max_age=90): principal_auth["Arn"], ) if job_details is None: - print("Missing data for arn {} in {}".format(principal_auth["Arn"], account.name)) + print( + "Missing data for arn {} in {}".format( + principal_auth["Arn"], account.name + ) + ) continue - + job_id = job_details["JobId"] json_last_access_details = get_parameter_file( region, "iam", "get-service-last-accessed-details", job_id @@ -439,4 +440,3 @@ def get_current_policy_doc(policy): if doc["IsDefaultVersion"]: return doc["Document"] raise Exception("No default document version in policy {}".format(policy["Arn"])) - diff --git a/shared/find_unused.py b/shared/find_unused.py index 8e69603f7..9e407f8ca 100644 --- a/shared/find_unused.py +++ b/shared/find_unused.py @@ -67,7 +67,9 @@ def find_unused_elastic_ips(region): unused_ips = [] ips = query_aws(region.account, "ec2-describe-addresses", region) for ip in pyjq.all(".Addresses[]? | select(.AssociationId == null)", ips): - unused_ips.append({"id": ip.get("AllocationId", "Un-allocated IP"), "ip": ip["PublicIp"]}) + unused_ips.append( + {"id": ip.get("AllocationId", "Un-allocated IP"), "ip": ip["PublicIp"]} + ) return unused_ips @@ -86,18 +88,48 @@ def find_unused_network_interfaces(region): return unused_network_interfaces + def find_unused_elastic_load_balancers(region): unused_elastic_load_balancers = [] - elastic_load_balancers = query_aws(region.account, "elb-describe-load-balancers", region) - for elastic_load_balancer in pyjq.all(".LoadBalancerDescriptions[]? | select(.Instances == [])", elastic_load_balancers): - unused_elastic_load_balancers.append({"LoadBalancerName": elastic_load_balancer["LoadBalancerName"], "Type": "classic"}) - - elastic_load_balancers_v2 = query_aws(region.account, "elbv2-describe-load-balancers", region) - for elastic_load_balancer in pyjq.all(".LoadBalancers[]?", elastic_load_balancers_v2): - target_groups = get_parameter_file(region, "elbv2", "describe-target-groups", elastic_load_balancer["LoadBalancerArn"]) - unused_elastic_load_balancers.append({"LoadBalancerName": elastic_load_balancer["LoadBalancerName"], "Type": elastic_load_balancer["Type"]}) + elastic_load_balancers = query_aws( + region.account, "elb-describe-load-balancers", region + ) + for elastic_load_balancer in pyjq.all( + ".LoadBalancerDescriptions[]? | select(.Instances == [])", + elastic_load_balancers, + ): + unused_elastic_load_balancers.append( + { + "LoadBalancerName": elastic_load_balancer["LoadBalancerName"], + "Type": "classic", + } + ) + + elastic_load_balancers_v2 = query_aws( + region.account, "elbv2-describe-load-balancers", region + ) + for elastic_load_balancer in pyjq.all( + ".LoadBalancers[]?", elastic_load_balancers_v2 + ): + target_groups = get_parameter_file( + region, + "elbv2", + "describe-target-groups", + elastic_load_balancer["LoadBalancerArn"], + ) + unused_elastic_load_balancers.append( + { + "LoadBalancerName": elastic_load_balancer["LoadBalancerName"], + "Type": elastic_load_balancer["Type"], + } + ) for target_group in pyjq.all(".TargetGroups[]?", target_groups): - target_healths = get_parameter_file(region, "elbv2", "describe-target-health", target_group["TargetGroupArn"]) + target_healths = get_parameter_file( + region, + "elbv2", + "describe-target-health", + target_group["TargetGroupArn"], + ) instances = pyjq.one(".TargetHealthDescriptions? | length", target_healths) if instances > 0: unused_elastic_load_balancers.pop() diff --git a/shared/iam_audit.py b/shared/iam_audit.py index fbf9dd74d..a9a33c714 100644 --- a/shared/iam_audit.py +++ b/shared/iam_audit.py @@ -20,8 +20,8 @@ "arn:aws:iam::aws:policy/service-role/AmazonEC2RoleforSSM": "Use AmazonSSMManagedInstanceCore instead and add privs as needed", "arn:aws:iam::aws:policy/service-role/AmazonMachineLearningRoleforRedshiftDataSource": "Use AmazonMachineLearningRoleforRedshiftDataSourceV3 instead", "arn:aws:iam::aws:policy/service-role/AmazonEC2SpotFleetRole": "Use AmazonEC2SpotFleetTaggingRole instead", - "arn:aws:iam::aws:policy/service-role/AWSLambdaReadOnlyAccess" : "Use AWSLambda_ReadOnlyAccess instead", - "arn:aws:iam::aws:policy/service-role/AWSLambdaFullAccess" :"Use AWSLambda_FullAccess instead", + "arn:aws:iam::aws:policy/service-role/AWSLambdaReadOnlyAccess": "Use AWSLambda_ReadOnlyAccess instead", + "arn:aws:iam::aws:policy/service-role/AWSLambdaFullAccess": "Use AWSLambda_FullAccess instead", } @@ -199,13 +199,18 @@ def find_admins_in_account( analyzed_policy = analyze_policy_string(json.dumps(policy_doc)) for f in analyzed_policy.findings: findings.add( - Finding( - region, - "IAM_LINTER", - policy["Arn"], - resource_details={"issue": str(f.issue), "severity": str(f.severity), "location": str(f.location), "policy": policy_doc}, - ) + Finding( + region, + "IAM_LINTER", + policy["Arn"], + resource_details={ + "issue": str(f.issue), + "severity": str(f.severity), + "location": str(f.location), + "policy": policy_doc, + }, ) + ) policy_action_counts[policy["Arn"]] = policy_action_count(policy_doc, location) @@ -275,13 +280,18 @@ def find_admins_in_account( analyzed_policy = analyze_policy_string(json.dumps(policy_doc)) for f in analyzed_policy.findings: findings.add( - Finding( - region, - "IAM_LINTER", - role["Arn"], - resource_details={"issue": str(f.issue), "severity": str(f.severity), "location": str(f.location), "policy": policy_doc}, - ) + Finding( + region, + "IAM_LINTER", + role["Arn"], + resource_details={ + "issue": str(f.issue), + "severity": str(f.severity), + "location": str(f.location), + "policy": policy_doc, + }, ) + ) if is_admin_policy( policy_doc, @@ -291,13 +301,15 @@ def find_admins_in_account( privs_to_look_for, include_restricted, ): - if ':role/OrganizationAccountAccessRole' in role['Arn']: + if ":role/OrganizationAccountAccessRole" in role["Arn"]: # AWS creates this role and adds an inline policy to it granting full # privileges, so this just causes false positives as the purpose # of this detection rule is to find unexpected admins continue - reasons_for_being_admin.append("Custom policy: {}".format(policy["PolicyName"])) + reasons_for_being_admin.append( + "Custom policy: {}".format(policy["PolicyName"]) + ) findings.add( Finding( region, @@ -353,7 +365,10 @@ def find_admins_in_account( }, ) ) - elif stmt["Action"] in ["sts:AssumeRoleWithSAML", "sts:AssumeRoleWithWebIdentity"]: + elif stmt["Action"] in [ + "sts:AssumeRoleWithSAML", + "sts:AssumeRoleWithWebIdentity", + ]: continue else: findings.add( @@ -457,13 +472,18 @@ def find_admins_in_account( analyzed_policy = analyze_policy_string(json.dumps(policy_doc)) for f in analyzed_policy.findings: findings.add( - Finding( - region, - "IAM_LINTER", - user["Arn"], - resource_details={"issue": str(f.issue), "severity": str(f.severity), "location": str(f.location), "policy": policy_doc}, - ) + Finding( + region, + "IAM_LINTER", + user["Arn"], + resource_details={ + "issue": str(f.issue), + "severity": str(f.severity), + "location": str(f.location), + "policy": policy_doc, + }, ) + ) if is_admin_policy( policy_doc, @@ -473,7 +493,9 @@ def find_admins_in_account( privs_to_look_for, include_restricted, ): - reasons_for_being_admin.append("Custom user policy: {}".format(policy["PolicyName"])) + reasons_for_being_admin.append( + "Custom user policy: {}".format(policy["PolicyName"]) + ) findings.add( Finding( region, diff --git a/shared/organization.py b/shared/organization.py index 76e04c442..b182b899a 100644 --- a/shared/organization.py +++ b/shared/organization.py @@ -5,25 +5,25 @@ # maximum allowed value, c.f. https://docs.aws.amazon.com/organizations/latest/APIReference/API_ListAccounts.html#API_ListAccounts_RequestSyntax MAX_NUM_RESULTS = 20 + def get_organization_accounts(): - organizations_client = boto3.client('organizations') - has_more = True - next_token = None - accounts = [] - - while has_more: - if next_token: - response = organizations_client.list_accounts(MaxResults=MAX_NUM_RESULTS, NextToken=next_token) - else: - response = organizations_client.list_accounts(MaxResults=MAX_NUM_RESULTS) - - for account in response.get('Accounts', []): - accounts.append({ - "name": slugify(account['Name']), - "id": account['Id'] - }) - - next_token = response.get('NextToken', None) - has_more = (next_token is not None) - - return accounts \ No newline at end of file + organizations_client = boto3.client("organizations") + has_more = True + next_token = None + accounts = [] + + while has_more: + if next_token: + response = organizations_client.list_accounts( + MaxResults=MAX_NUM_RESULTS, NextToken=next_token + ) + else: + response = organizations_client.list_accounts(MaxResults=MAX_NUM_RESULTS) + + for account in response.get("Accounts", []): + accounts.append({"name": slugify(account["Name"]), "id": account["Id"]}) + + next_token = response.get("NextToken", None) + has_more = next_token is not None + + return accounts diff --git a/shared/public.py b/shared/public.py index 7f606b92a..5e290cff9 100644 --- a/shared/public.py +++ b/shared/public.py @@ -153,7 +153,7 @@ def get_public_nodes(account, config, use_cache=False): ) # I would need to redo this code in order to get the name of the security group public_sgs[sg_group_allowing_all_protocols] = {"public_ports": "0-65535"} - + # from_port and to_port mean the beginning and end of a port range # We only care about TCP (6) and UDP (17) # For more info see https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/security-group-rules-reference.html @@ -163,15 +163,11 @@ def get_public_nodes(account, config, use_cache=False): for ip_permission in sg.get("IpPermissions", []): selection = 'select((.IpProtocol=="tcp") or (.IpProtocol=="udp")) | select(.IpRanges[].CidrIp=="0.0.0.0/0")' sg_port_ranges.extend( - pyjq.all( - "{}| [.FromPort,.ToPort]".format(selection), ip_permission - ) + pyjq.all("{}| [.FromPort,.ToPort]".format(selection), ip_permission) ) selection = 'select(.IpProtocol=="-1") | select(.IpRanges[].CidrIp=="0.0.0.0/0")' sg_port_ranges.extend( - pyjq.all( - "{}| [0,65535]".format(selection), ip_permission - ) + pyjq.all("{}| [0,65535]".format(selection), ip_permission) ) public_sgs[sg["GroupId"]] = { "GroupId": sg["GroupId"], diff --git a/tests/__init__.py b/tests/__init__.py index c218472ee..b72c80931 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -1,2 +1,2 @@ # Allow for python3 style. -from __future__ import (absolute_import, division, print_function) +from __future__ import absolute_import, division, print_function diff --git a/tests/unit/test_audit.py b/tests/unit/test_audit.py index 6321161bb..a12413db9 100644 --- a/tests/unit/test_audit.py +++ b/tests/unit/test_audit.py @@ -48,7 +48,7 @@ def test_audit(self): "IAM_LINTER", "EC2_IMDSV2_NOT_ENFORCED", "REQUEST_SMUGGLING", - "ELBV1_DESYNC_MITIGATION" + "ELBV1_DESYNC_MITIGATION", ] ), ) diff --git a/tests/unit/test_find_unused.py b/tests/unit/test_find_unused.py index 78045522a..e9e31c933 100644 --- a/tests/unit/test_find_unused.py +++ b/tests/unit/test_find_unused.py @@ -9,7 +9,11 @@ class TestFindUnused(TestCase): mock_account = type("account", (object,), {"name": "a"}) - mock_region = type("region", (object,), {"account": mock_account, "name": "a", "region": mock_account }) + mock_region = type( + "region", + (object,), + {"account": mock_account, "name": "a", "region": mock_account}, + ) def test_find_unused_elastic_ips_empty(self): def mocked_query_side_effect(account, query, region): @@ -281,7 +285,7 @@ def mocked_query_side_effect(account, query, region): "AvailabilityZones": [ "eu-west-1b", "eu-west-1c", - "eu-west-1a" + "eu-west-1a", ], "BackendServerDescriptions": [], "CanonicalHostedZoneName": "some-elb-450109654.eu-west-1.elb.amazonaws.com", @@ -293,7 +297,7 @@ def mocked_query_side_effect(account, query, region): "Interval": 300, "Target": "HTTP:8033/ping", "Timeout": 4, - "UnhealthyThreshold": 2 + "UnhealthyThreshold": 2, }, "Instances": [], "ListenerDescriptions": [ @@ -303,46 +307,42 @@ def mocked_query_side_effect(account, query, region): "InstanceProtocol": "HTTP", "LoadBalancerPort": 443, "Protocol": "HTTPS", - "SSLCertificateId": "arn:aws:acm:eu-west-1:123456789011:certificate/1e43d2f8-8f31-4a29-ba3e-fce3d2c6c0ed" + "SSLCertificateId": "arn:aws:acm:eu-west-1:123456789011:certificate/1e43d2f8-8f31-4a29-ba3e-fce3d2c6c0ed", }, - "PolicyNames": [ - "ELBSecurityPolicy-2016-08" - ] + "PolicyNames": ["ELBSecurityPolicy-2016-08"], }, { "Listener": { "InstancePort": 8033, "InstanceProtocol": "HTTP", "LoadBalancerPort": 80, - "Protocol": "HTTP" + "Protocol": "HTTP", }, - "PolicyNames": [] - } + "PolicyNames": [], + }, ], "LoadBalancerName": "some-elb", "Policies": { "AppCookieStickinessPolicies": [], "LBCookieStickinessPolicies": [], - "OtherPolicies": [ - "ELBSecurityPolicy-2019-08" - ] + "OtherPolicies": ["ELBSecurityPolicy-2019-08"], }, "Scheme": "internet-facing", "SecurityGroups": [ "sg-82e9fce5", "sg-d2be0aaa", - "sg-b1e9fcd6" + "sg-b1e9fcd6", ], "SourceSecurityGroup": { "GroupName": "default-http-healthcheck", - "OwnerAlias": "123456789011" + "OwnerAlias": "123456789011", }, "Subnets": [ "subnet-67c99110", "subnet-a684b4c3", - "subnet-ef6516b6" + "subnet-ef6516b6", ], - "VPCId": "vpc-1234567" + "VPCId": "vpc-1234567", } ] } diff --git a/utils/strings.py b/utils/strings.py index 0f16efaf6..32e67c99d 100644 --- a/utils/strings.py +++ b/utils/strings.py @@ -1,12 +1,13 @@ # Slugifies a string, e.g. "My IAM account" => "my-iam-account" def slugify(str): - allowed_characters = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_" - new_string = [] - for i in range(len(str)): - char = str[i] - if char not in allowed_characters: - char = '-' - new_string.append(char) - - return ''.join(new_string).strip().lower() + allowed_characters = ( + "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_" + ) + new_string = [] + for i in range(len(str)): + char = str[i] + if char not in allowed_characters: + char = "-" + new_string.append(char) + return "".join(new_string).strip().lower() diff --git a/utils/toslack.py b/utils/toslack.py index fa0204672..25a37148d 100644 --- a/utils/toslack.py +++ b/utils/toslack.py @@ -6,28 +6,31 @@ # Usage: Set the environment variable SLACK_WEBHOOK to https://hooks.slack.com/services/XXXX/YYYYY -webhook_url = os.environ['SLACK_WEBHOOK'] +webhook_url = os.environ["SLACK_WEBHOOK"] for line in sys.stdin: line.rstrip() - line = line.replace('\\n', '\n') - #print(line) + line = line.replace("\\n", "\n") + # print(line) - slack_data = {'text': line} + slack_data = {"text": line} response = requests.post( - webhook_url, data=json.dumps(slack_data), - headers={'Content-Type': 'application/json'} + webhook_url, + data=json.dumps(slack_data), + headers={"Content-Type": "application/json"}, ) if response.status_code == 429: # Rate-limited. Sleep and retry time.sleep(5) response = requests.post( - webhook_url, data=json.dumps(slack_data), - headers={'Content-Type': 'application/json'} + webhook_url, + data=json.dumps(slack_data), + headers={"Content-Type": "application/json"}, ) if response.status_code != 200: raise ValueError( - 'Request to slack returned an error %s, the response is:\n%s' - % (response.status_code, response.text)) + "Request to slack returned an error %s, the response is:\n%s" + % (response.status_code, response.text) + )