Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(arn): improve resource ARNs in checks #3388

Merged
merged 16 commits into from
Mar 5, 2024
2 changes: 1 addition & 1 deletion docs/developer-guide/checks.md
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ All the checks MUST fill the `report.resource_id` and `report.resource_arn` with
- Resource ARN -- `report.resource_arn`
- AWS Account --> Root ARN `arn:aws:iam::123456789012:root`
- AWS Resource --> Resource ARN
- Root resource --> Root ARN `arn:aws:iam::123456789012:root`
- Root resource --> Root ARN `f"arn:{service_client.audited_partition}:<service_name>:{service_client.region}:{service_client.audited_account}:<resource_type>"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Name the result differently than root, maybe Service ARN or Resource Type ARN

- GCP
- Resource ID -- `report.resource_id`
- GCP Resource --> Resource ID
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ def execute(self):
report = Check_Report_AWS(self.metadata())
report.status = "FAIL"
report.status_extended = "No Backup Plan exist."
report.resource_arn = backup_client.audited_account_arn
report.resource_arn = f"arn:{backup_client.audited_partition}:backup:{backup_client.region}:{backup_client.audited_account}:backup-plan"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This applies to all the backup checks.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you can define those ARNs in a single place to use that also for the tests. Maybe it's better to have something in the lib/arn with the service generic ARNs.

Copy link
Member Author

@sergargar sergargar Feb 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, it would be a nice-to-have. But I see it complicated, since each check can have a different resource type and we need to use the information of each service.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done! I have added them in each service class.

report.resource_id = backup_client.audited_account
report.region = backup_client.region
findings.append(report)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ def execute(self):
report = Check_Report_AWS(self.metadata())
report.status = "FAIL"
report.status_extended = "No Backup Report Plan exist."
report.resource_arn = backup_client.audited_account_arn
report.resource_arn = f"arn:{backup_client.audited_partition}:backup:{backup_client.region}:{backup_client.audited_account}:report-plan"
report.resource_id = backup_client.audited_account
report.region = backup_client.region
if backup_client.backup_report_plans:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ def execute(self):
report = Check_Report_AWS(self.metadata())
report.status = "FAIL"
report.status_extended = "No Backup Vault exist."
report.resource_arn = backup_client.audited_account_arn
report.resource_arn = f"arn:{backup_client.audited_partition}:backup:{backup_client.region}:{backup_client.audited_account}:backup-vault"
report.resource_id = backup_client.audited_account
report.region = backup_client.region
if backup_client.backup_vaults:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ def execute(self):
report.status_extended = (
"No CloudTrail trails enabled and logging were found."
)
report.resource_arn = cloudtrail_client.audited_account_arn
report.resource_arn = f"arn:{cloudtrail_client.audited_partition}:cloudtrail:{cloudtrail_client.region}:{cloudtrail_client.audited_account}:trail"
report.resource_id = cloudtrail_client.audited_account
# If there are no trails logging it is needed to store the FAIL once all the trails have been checked
if report.status == "FAIL":
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ def execute(self):
)
report.region = cloudtrail_client.region
report.resource_id = cloudtrail_client.audited_account
report.resource_arn = cloudtrail_client.audited_account_arn
report.resource_arn = f"arn:{cloudtrail_client.audited_partition}:cloudtrail:{cloudtrail_client.region}:{cloudtrail_client.audited_account}:trail"

for trail in cloudtrail_client.trails:
if trail.is_logging:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ def execute(self):
):
report = Check_Report_AWS(self.metadata())
report.region = cloudtrail_client.region
report.resource_arn = cloudtrail_client.audited_account_arn
report.resource_arn = f"arn:{cloudtrail_client.audited_partition}:cloudtrail:{cloudtrail_client.region}:{cloudtrail_client.audited_account}:trail"
report.resource_id = cloudtrail_client.audited_account
report.status = "FAIL"
report.status_extended = "No CloudTrail trails have a data event to record all S3 object-level API operations."
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ def execute(self):
):
report = Check_Report_AWS(self.metadata())
report.region = cloudtrail_client.region
report.resource_arn = cloudtrail_client.audited_account_arn
report.resource_arn = f"arn:{cloudtrail_client.audited_partition}:cloudtrail:{cloudtrail_client.region}:{cloudtrail_client.audited_account}:trail"
report.resource_id = cloudtrail_client.audited_account
report.status = "FAIL"
report.status_extended = "No CloudTrail trails have a data event to record all S3 object-level API operations."
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ def execute(self):
"No CloudWatch log groups found with metric filters or alarms associated."
)
report.region = cloudwatch_client.region
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
report.region = cloudwatch_client.region
report.region = logs_client.region

report.resource_id = cloudtrail_client.audited_account
report.resource_arn = cloudtrail_client.audited_account_arn
report.resource_id = logs_client.audited_account
report.resource_arn = f"arn:{logs_client.audited_partition}:logs:{logs_client.region}:{logs_client.audited_account}:log-group"
report = check_cloudwatch_log_metric_filter(
pattern,
cloudtrail_client.trails,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ def execute(self):
"No CloudWatch log groups found with metric filters or alarms associated."
)
report.region = cloudwatch_client.region
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
report.region = cloudwatch_client.region
report.region = logs_client.region

report.resource_id = cloudtrail_client.audited_account
report.resource_arn = cloudtrail_client.audited_account_arn
report.resource_id = logs_client.audited_account
report.resource_arn = f"arn:{logs_client.audited_partition}:logs:{logs_client.region}:{logs_client.audited_account}:log-group"
report = check_cloudwatch_log_metric_filter(
pattern,
cloudtrail_client.trails,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ def execute(self):
"No CloudWatch log groups found with metric filters or alarms associated."
)
report.region = cloudwatch_client.region
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
report.region = cloudwatch_client.region
report.region = logs_client.region

report.resource_id = cloudtrail_client.audited_account
report.resource_arn = cloudtrail_client.audited_account_arn
report.resource_id = logs_client.audited_account
report.resource_arn = f"arn:{logs_client.audited_partition}:logs:{logs_client.region}:{logs_client.audited_account}:log-group"
report = check_cloudwatch_log_metric_filter(
pattern,
cloudtrail_client.trails,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ def execute(self):
"No CloudWatch log groups found with metric filters or alarms associated."
)
report.region = cloudwatch_client.region
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
report.region = cloudwatch_client.region
report.region = logs_client.region

report.resource_id = cloudtrail_client.audited_account
report.resource_arn = cloudtrail_client.audited_account_arn
report.resource_id = logs_client.audited_account
report.resource_arn = f"arn:{logs_client.audited_partition}:logs:{logs_client.region}:{logs_client.audited_account}:log-group"
report = check_cloudwatch_log_metric_filter(
pattern,
cloudtrail_client.trails,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ def execute(self):
report = Check_Report_AWS(self.metadata())
report.status = "PASS"
report.status_extended = "CloudWatch doesn't allow cross-account sharing."
report.resource_arn = iam_client.audited_account_arn
report.resource_arn = f"arn:{iam_client.audited_partition}:iam:{iam_client.region}:{iam_client.audited_account}:role"
report.resource_id = iam_client.audited_account
report.region = iam_client.region
for role in iam_client.roles:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ def execute(self):
"No CloudWatch log groups found with metric filters or alarms associated."
)
report.region = cloudwatch_client.region
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
report.region = cloudwatch_client.region
report.region = logs_client.region

report.resource_id = cloudtrail_client.audited_account
report.resource_arn = cloudtrail_client.audited_account_arn
report.resource_id = logs_client.audited_account
report.resource_arn = f"arn:{logs_client.audited_partition}:logs:{logs_client.region}:{logs_client.audited_account}:log-group"
report = check_cloudwatch_log_metric_filter(
pattern,
cloudtrail_client.trails,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ def execute(self):
"No CloudWatch log groups found with metric filters or alarms associated."
)
report.region = cloudwatch_client.region
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
report.region = cloudwatch_client.region
report.region = logs_client.region

report.resource_id = cloudtrail_client.audited_account
report.resource_arn = cloudtrail_client.audited_account_arn
report.resource_id = logs_client.audited_account
report.resource_arn = f"arn:{logs_client.audited_partition}:logs:{logs_client.region}:{logs_client.audited_account}:log-group"
report = check_cloudwatch_log_metric_filter(
pattern,
cloudtrail_client.trails,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ def execute(self):
"No CloudWatch log groups found with metric filters or alarms associated."
)
report.region = cloudwatch_client.region
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
report.region = cloudwatch_client.region
report.region = logs_client.region

report.resource_id = cloudtrail_client.audited_account
report.resource_arn = cloudtrail_client.audited_account_arn
report.resource_id = logs_client.audited_account
report.resource_arn = f"arn:{logs_client.audited_partition}:logs:{logs_client.region}:{logs_client.audited_account}:log-group"
report = check_cloudwatch_log_metric_filter(
pattern,
cloudtrail_client.trails,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ def execute(self):
"No CloudWatch log groups found with metric filters or alarms associated."
)
report.region = cloudwatch_client.region
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
report.region = cloudwatch_client.region
report.region = logs_client.region

report.resource_id = cloudtrail_client.audited_account
report.resource_arn = cloudtrail_client.audited_account_arn
report.resource_id = logs_client.audited_account
report.resource_arn = f"arn:{logs_client.audited_partition}:logs:{logs_client.region}:{logs_client.audited_account}:log-group"
report = check_cloudwatch_log_metric_filter(
pattern,
cloudtrail_client.trails,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ def execute(self):
"No CloudWatch log groups found with metric filters or alarms associated."
)
report.region = cloudwatch_client.region
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
report.region = cloudwatch_client.region
report.region = logs_client.region

report.resource_id = cloudtrail_client.audited_account
report.resource_arn = cloudtrail_client.audited_account_arn
report.resource_id = logs_client.audited_account
report.resource_arn = f"arn:{logs_client.audited_partition}:logs:{logs_client.region}:{logs_client.audited_account}:log-group"
report = check_cloudwatch_log_metric_filter(
pattern,
cloudtrail_client.trails,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ def execute(self):
"No CloudWatch log groups found with metric filters or alarms associated."
)
report.region = cloudwatch_client.region
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
report.region = cloudwatch_client.region
report.region = logs_client.region

report.resource_id = cloudtrail_client.audited_account
report.resource_arn = cloudtrail_client.audited_account_arn
report.resource_id = logs_client.audited_account
report.resource_arn = f"arn:{logs_client.audited_partition}:logs:{logs_client.region}:{logs_client.audited_account}:log-group"

report = check_cloudwatch_log_metric_filter(
pattern,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ def execute(self):
"No CloudWatch log groups found with metric filters or alarms associated."
)
report.region = cloudwatch_client.region
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
report.region = cloudwatch_client.region
report.region = logs_client.region

report.resource_id = cloudtrail_client.audited_account
report.resource_arn = cloudtrail_client.audited_account_arn
report.resource_id = logs_client.audited_account
report.resource_arn = f"arn:{logs_client.audited_partition}:logs:{logs_client.region}:{logs_client.audited_account}:log-group"
report = check_cloudwatch_log_metric_filter(
pattern,
cloudtrail_client.trails,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ def execute(self):
"No CloudWatch log groups found with metric filters or alarms associated."
)
report.region = cloudwatch_client.region
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
report.region = cloudwatch_client.region
report.region = logs_client.region

report.resource_id = cloudtrail_client.audited_account
report.resource_arn = cloudtrail_client.audited_account_arn
report.resource_id = logs_client.audited_account
report.resource_arn = f"arn:{logs_client.audited_partition}:logs:{logs_client.region}:{logs_client.audited_account}:log-group"
report = check_cloudwatch_log_metric_filter(
pattern,
cloudtrail_client.trails,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ def execute(self):
"No CloudWatch log groups found with metric filters or alarms associated."
)
report.region = cloudwatch_client.region
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
report.region = cloudwatch_client.region
report.region = logs_client.region

report.resource_id = cloudtrail_client.audited_account
report.resource_arn = cloudtrail_client.audited_account_arn
report.resource_id = logs_client.audited_account
report.resource_arn = f"arn:{logs_client.audited_partition}:logs:{logs_client.region}:{logs_client.audited_account}:log-group"
report = check_cloudwatch_log_metric_filter(
pattern,
cloudtrail_client.trails,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ def execute(self):
"No CloudWatch log groups found with metric filters or alarms associated."
)
report.region = cloudwatch_client.region
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
report.region = cloudwatch_client.region
report.region = logs_client.region

report.resource_id = cloudtrail_client.audited_account
report.resource_arn = cloudtrail_client.audited_account_arn
report.resource_id = logs_client.audited_account
report.resource_arn = f"arn:{logs_client.audited_partition}:logs:{logs_client.region}:{logs_client.audited_account}:log-group"
report = check_cloudwatch_log_metric_filter(
pattern,
cloudtrail_client.trails,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ def execute(self):
"No CloudWatch log groups found with metric filters or alarms associated."
)
report.region = cloudwatch_client.region
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
report.region = cloudwatch_client.region
report.region = logs_client.region

report.resource_id = cloudtrail_client.audited_account
report.resource_arn = cloudtrail_client.audited_account_arn
report.resource_id = logs_client.audited_account
report.resource_arn = f"arn:{logs_client.audited_partition}:logs:{logs_client.region}:{logs_client.audited_account}:log-group"
report = check_cloudwatch_log_metric_filter(
pattern,
cloudtrail_client.trails,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,7 @@ def execute(self):
for recorder in config_client.recorders:
report = Check_Report_AWS(self.metadata())
report.region = recorder.region
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
report.region = recorder.region
report.region = config.region

report.resource_arn = (
config_client.audited_account_arn
) # Config Recorders do not have ARNs
report.resource_arn = f"arn:{config_client.audited_partition}:config:{config_client.region}:{config_client.audited_account}:recorder" # Config Recorders do not have ARNs
report.resource_id = (
config_client.audited_account if not recorder.name else recorder.name
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ def execute(self):
report.status_extended = "No EBS Snapshot lifecycle policies found."
report.region = region
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check this region please.

report.resource_id = dlm_client.audited_account
report.resource_arn = dlm_client.audited_account_arn
report.resource_arn = f"arn:{dlm_client.audited_partition}:dlm:{dlm_client.region}:{dlm_client.audited_account}:policy"
if dlm_client.lifecycle_policies[region]:
report.status = "PASS"
report.status_extended = "EBS snapshot lifecycle policies found."
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ def execute(self):
report.status_extended = "DRS is not enabled for this region."
report.region = drs.region
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check this region please.

report.resource_tags = []
report.resource_arn = drs_client.audited_account_arn
report.resource_arn = f"arn:{drs_client.audited_partition}:drs:{drs_client.region}:{drs_client.audited_account}:recovery-job"
report.resource_id = drs_client.audited_account
if drs.status == "ENABLED":
report.status_extended = "DRS is enabled for this region without jobs."
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ def execute(self):
for ebs_encryption in ec2_client.ebs_encryption_by_default:
report = Check_Report_AWS(self.metadata())
report.region = ebs_encryption.region
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check this region please.

report.resource_arn = ec2_client.audited_account_arn
report.resource_arn = f"arn:{ec2_client.audited_partition}:ec2:{ec2_client.region}:{ec2_client.audited_account}:volume"
report.resource_id = ec2_client.audited_account
if ebs_encryption.status:
report.status = "PASS"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ def execute(self):
report = Check_Report_AWS(self.metadata())
report.region = region
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check this region please.

report.resource_id = emr_client.audited_account
report.resource_arn = emr_client.audited_account_arn
report.resource_arn = f"arn:{emr_client.audited_partition}:elasticmapreduce:{emr_client.region}:{emr_client.audited_account}:cluster"
if emr_client.block_public_access_configuration[
region
].block_public_security_group_rules:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ def execute(self):
findings = []
if fms_client.fms_admin_account:
report = Check_Report_AWS(self.metadata())
report.resource_arn = fms_client.audited_account_arn
report.resource_arn = f"arn:{fms_client.audited_partition}:fms:{fms_client.region}:{fms_client.audited_account}:policy"
report.resource_id = fms_client.audited_account
report.region = fms_client.region
report.status = "PASS"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ def execute(self):
if encryption.tables or not glue_client.audit_info.ignore_unused_services:
report = Check_Report_AWS(self.metadata())
report.resource_id = glue_client.audited_account
report.resource_arn = glue_client.audited_account_arn
report.resource_arn = f"arn:{glue_client.audited_partition}:glue:{glue_client.region}:{glue_client.audited_account}:data-catalog"
report.region = encryption.region
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check this region please.

report.status = "FAIL"
report.status_extended = (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ def execute(self):
if encryption.tables or not glue_client.audit_info.ignore_unused_services:
report = Check_Report_AWS(self.metadata())
report.resource_id = glue_client.audited_account
report.resource_arn = glue_client.audited_account_arn
report.resource_arn = f"arn:{glue_client.audited_partition}:glue:{glue_client.region}:{glue_client.audited_account}:data-catalog"
report.region = encryption.region
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check this region please.

report.status = "FAIL"
report.status_extended = (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ def execute(self) -> Check_Report_AWS:
findings = []
report = Check_Report_AWS(self.metadata())
report.region = iam_client.region
report.resource_arn = iam_client.audited_account_arn
report.resource_arn = f"arn:{iam_client.audited_partition}:iam:{iam_client.region}:{iam_client.audited_account}:password-policy"
report.resource_id = iam_client.audited_account
# Check if password policy exists
if iam_client.password_policy:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ def execute(self) -> Check_Report_AWS:
findings = []
report = Check_Report_AWS(self.metadata())
report.region = iam_client.region
report.resource_arn = iam_client.audited_account_arn
report.resource_arn = f"arn:{iam_client.audited_partition}:iam:{iam_client.region}:{iam_client.audited_account}:password-policy"
report.resource_id = iam_client.audited_account
# Check if password policy exists
if iam_client.password_policy:
Expand Down
Loading
Loading