Skip to content

Commit

Permalink
Merge pull request #573 from 0xdabbad00/detect_listallmybuckets_by_co…
Browse files Browse the repository at this point in the history
…mpute_resources

Make unexpected principal for admin its own finding
  • Loading branch information
0xdabbad00 authored Oct 4, 2019
2 parents 9233bfb + 4c7db68 commit e10c866
Show file tree
Hide file tree
Showing 8 changed files with 127 additions and 17 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"JobId": "a2b53fd9-88bc-d0b5-1b9a-af9c4af22371"
}
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,44 @@
],
"RoleDetailList": [
{
"Arn": "arn:aws:iam::123456789012:role/service-role/bad_role",
"Arn": "arn:aws:iam::123456789012:role/exfiller",
"AssumeRolePolicyDocument": {
"Statement": [
{
"Action": "sts:AssumeRole",
"Effect": "Allow",
"Principal": {
"Service": "ec2.amazonaws.com"
}
}
],
"Version": "2012-10-17"
},
"AttachedManagedPolicies": [],
"CreateDate": "2018-10-19T21:45:27+00:00",
"InstanceProfileList": [],
"Path": "/",
"RoleId": "AROAJIWYG5SMZL45EWWGG",
"RoleName": "exfiller",
"RolePolicyList": [
{
"PolicyDocument": {
"Statement": [
{
"Action": "s3:*",
"Effect": "Allow",
"Resource": "*"
}
],
"Version": "2012-10-17"
},
"PolicyName": "tmp"
}
],
"Tags": []
},
{
"Arn": "arn:aws:iam::123456789012:role/bad_role",
"AssumeRolePolicyDocument": {
"Statement": [
{
Expand All @@ -238,7 +275,7 @@
],
"CreateDate": "2018-11-20T17:14:45+00:00",
"InstanceProfileList": [],
"Path": "/service-role/",
"Path": "",
"RoleId": "AROAIBATWWYQXZTTALNCE",
"RoleName": "bad_role",
"RolePolicyList": [
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"JobCompletionDate": "2019-05-03T21:56:43.085000+00:00",
"JobCreationDate": "2019-05-03T21:56:43.001000+00:00",
"JobStatus": "COMPLETED",
"ServicesLastAccessed": [
{
"LastAuthenticated": "2019-04-15T04:19:00+00:00",
"LastAuthenticatedEntity": "arn:aws:iam::123456789012:role/service-role/level1",
"ServiceName": "Amazon CloudWatch Logs",
"ServiceNamespace": "logs",
"TotalAuthenticatedEntities": 1
}
]
}
15 changes: 15 additions & 0 deletions audit_config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,21 @@ IAM_UNEXPECTED_FORMAT:
is_global: True
group: IAM

IAM_UNEXPECTED_ADMIN_PRINCIPAL:
title: IAM role with admin privileges can be assumed by unexpected principals
description: Admins in an account should be assumed by people. This rule detects IAM Roles that can be granted to EC2s and other services, that has admin privileges.
severity: High
is_global: True
group: IAM

IAM_UNEXPECTED_S3_EXFIL_PRINCIPAL:
title: IAM role with s3 listing and get privileges can be assumed by unexpected principals
description: The ability to list s3 buckets, and get objects from them, should be restricted largely to people as compromising an EC2 with this privilege could lead to exfiltration of data.
severity: High
is_global: True
group: IAM


IAM_NAME_DOES_NOT_INDICATE_ADMIN:
title: Name does not indicate admin
description: This IAM Group grants admin privileges, but the name does not indicate it is for admins.
Expand Down
45 changes: 43 additions & 2 deletions shared/audit.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,11 @@ def __len__(self):

def finding_is_filtered(finding, conf, minimum_severity="LOW"):
minimum_severity = minimum_severity.upper()
severity_choices = ['CRITICAL', 'HIGH', 'MEDIUM', 'LOW', 'INFO', 'MUTE']
severity_choices = ["CRITICAL", "HIGH", "MEDIUM", "LOW", "INFO", "MUTE"]
finding_severity = conf["severity"].upper()
if severity_choices.index(finding_severity) > severity_choices.index(minimum_severity):
if severity_choices.index(finding_severity) > severity_choices.index(
minimum_severity
):
return True

for resource_to_ignore in conf.get("ignore_resources", []):
Expand Down Expand Up @@ -205,6 +207,45 @@ def audit_guardduty(findings, region):

def audit_iam(findings, region):
find_admins_in_account(region, findings)
# By default we get the findings for the admins, but we can also look for specific
# privileges, so we'll look for who has s3:ListAllMyBuckets and then only use those
# findings that are for a compute resource having this privilege

s3_listing_findings = Findings()
s3_get_findings = Findings()

# TODO Running find_admins_in_account is really slow, and now we're running it 3 times.
# So figure out a way to run it once.
find_admins_in_account(
region, s3_listing_findings, privs_to_look_for=["s3:ListAllMyBuckets"]
)
find_admins_in_account(region, s3_get_findings, privs_to_look_for=["s3:GetObject"])

for f in s3_listing_findings:
if f.issue_id != "IAM_UNEXPECTED_ADMIN_PRINCIPAL":
continue

services = make_list(f.resource_details.get("Principal", {}).get("Service", ""))
for service in services:
if service in ["config.amazonaws.com", "trustedadvisor.amazonaws.com"]:
continue

# If we are here then we have a principal that can list S3 buckets,
# and is associated with an unexpected service,
# so check if they can read data from them as well

for fget in s3_get_findings:
if (
fget.issue_id == "IAM_UNEXPECTED_ADMIN_PRINCIPAL"
and fget.resource_id == f.resource_id
):
# If we are here, then the principal can list S3 buckets and get objects
# from them, and is not an unexpected service, so record this as a finding
f.issue_id = "IAM_UNEXPECTED_S3_EXFIL_PRINCIPAL"
findings.add(f)

# Don't record this multiple times if multiple services are listed
break


def audit_cloudtrail(findings, region):
Expand Down
23 changes: 11 additions & 12 deletions shared/iam_audit.py
Original file line number Diff line number Diff line change
Expand Up @@ -237,12 +237,12 @@ def find_admins_in_account(
# Identify roles that allow admin access
for role in iam["RoleDetailList"]:
location["role"] = role["Arn"]
reasons = []
reasons_for_being_admin = []

# Check if this role is an admin
for policy in role["AttachedManagedPolicies"]:
if policy["PolicyArn"] in admin_policies:
reasons.append(
reasons_for_being_admin.append(
"Attached managed policy: {}".format(policy["PolicyArn"])
)
if policy["PolicyArn"] in KNOWN_BAD_POLICIES:
Expand Down Expand Up @@ -274,7 +274,7 @@ def find_admins_in_account(
# of this detection rule is to find unexpected admins
continue

reasons.append("Custom policy: {}".format(policy["PolicyName"]))
reasons_for_being_admin.append("Custom policy: {}".format(policy["PolicyName"]))
findings.add(
Finding(
region,
Expand Down Expand Up @@ -316,13 +316,13 @@ def find_admins_in_account(
continue

if stmt["Action"] == "sts:AssumeRole":
if len(reasons) != 0:
if len(reasons_for_being_admin) != 0:
# Admin assumption should be done by users or roles, not by AWS services
if "AWS" not in stmt["Principal"] or len(stmt["Principal"]) != 1:
findings.add(
Finding(
region,
"IAM_UNEXPECTED_FORMAT",
"IAM_UNEXPECTED_ADMIN_PRINCIPAL",
role["Arn"],
resource_details={
"comment": "Unexpected Principal in AssumeRolePolicyDocument for an admin",
Expand All @@ -345,7 +345,7 @@ def find_admins_in_account(
)
)

if len(reasons) != 0:
if len(reasons_for_being_admin) != 0:
record_admin(admins, account.name, "role", role["RoleName"])
# TODO Should check users or other roles allowed to assume this role to show they are admins
location.pop("role", None)
Expand Down Expand Up @@ -408,12 +408,12 @@ def find_admins_in_account(
# Check users
for user in iam["UserDetailList"]:
location["user"] = user["UserName"]
reasons = []
reasons_for_being_admin = []

# Check the different ways in which the user could be an admin
for policy in user["AttachedManagedPolicies"]:
if policy["PolicyArn"] in admin_policies:
reasons.append(
reasons_for_being_admin.append(
"Attached managed policy: {}".format(policy["PolicyArn"])
)
if policy["PolicyArn"] in KNOWN_BAD_POLICIES:
Expand All @@ -438,7 +438,7 @@ def find_admins_in_account(
privs_to_look_for,
include_restricted,
):
reasons.append("Custom user policy: {}".format(policy["PolicyName"]))
reasons_for_being_admin.append("Custom user policy: {}".format(policy["PolicyName"]))
findings.add(
Finding(
region,
Expand All @@ -452,11 +452,10 @@ def find_admins_in_account(
)
for group in user["GroupList"]:
if group in admin_groups:
reasons.append("In admin group: {}".format(group))
reasons_for_being_admin.append("In admin group: {}".format(group))

# Log them if they are an admin
if len(reasons) != 0:
# log_info('User is admin', location, reasons)
if len(reasons_for_being_admin) != 0:
record_admin(admins, account.name, "user", user["UserName"])

location.pop("user", None)
Expand Down
3 changes: 2 additions & 1 deletion tests/unit/test_audit.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ def test_audit(self):
"IAM_CUSTOM_POLICY_ALLOWS_ADMIN",
"IAM_KNOWN_BAD_POLICY",
"IAM_ROLE_ALLOWS_ASSUMPTION_FROM_ANYWHERE",
"EC2_OLD"
"EC2_OLD",
"IAM_UNEXPECTED_S3_EXFIL_PRINCIPAL"
]
),
)

0 comments on commit e10c866

Please sign in to comment.