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

Check for org ID restrictions in policies #1533

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

arielkr256
Copy link
Contributor

@arielkr256 arielkr256 commented Mar 6, 2025

Background

The policyuniverse library incorrectly considers policies with wildcard principals ("Principal": "*") as internet accessible, even when they have restrictive conditions like organization ID restrictions. The is_internet_accessible method in policyuniverse considers a policy statement internet accessible if ANY of its condition entries are internet accessible. ARN patterns with wildcards in the account number position (arn:aws:iam::*:role/...) are considered internet accessible by the _arn_internet_accessible method.

Changes

  • Modified the policy_is_internet_accessible function in the aws_resource_made_public.py rule to add an additional check for policies with organization ID restrictions. If a statement has a wildcard principal but also has organization ID conditions, we consider it not internet accessible.
  • Cleaned up the rule logic to be less complex and easier to understand.

Testing

  • several new unit tests based on recent false positives

@arielkr256 arielkr256 requested a review from a team as a code owner March 6, 2025 15:42
@arielkr256
Copy link
Contributor Author

There is an open issue/PR in policyuniverse to address this issue, but seeing as it's been there for 2 years I don't know that it will be updated any time soon.

@arielkr256 arielkr256 added the bug Something isn't working label Mar 6, 2025
policy_value = parameters.get("attributeValue", {})
return policy_is_internet_accessible(policy_value)

# Map of event names to policy locations in parameters
Copy link
Contributor

Choose a reason for hiding this comment

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

aws why are you like this

Copy link
Contributor

@LucySuddenly LucySuddenly left a comment

Choose a reason for hiding this comment

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

Limiting my approval to the following statement:

This appears to fix the incorrect trigger of this rule that I encountered a couple times recently

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants