Skip to content

fix(aws): recognize service-specific condition keys as restrictive in is_policy_public#10600

Merged
pedrooot merged 4 commits intoprowler-cloud:masterfrom
kagahd:fix/kms-policy-recognize-caller-account-condition
Apr 8, 2026
Merged

fix(aws): recognize service-specific condition keys as restrictive in is_policy_public#10600
pedrooot merged 4 commits intoprowler-cloud:masterfrom
kagahd:fix/kms-policy-recognize-caller-account-condition

Conversation

@kagahd
Copy link
Copy Markdown
Contributor

@kagahd kagahd commented Apr 7, 2026

Context

is_condition_block_restrictive() in prowler/providers/aws/services/iam/lib/policy.py maintains a list of IAM condition keys it considers restrictive. Several service-specific and global condition keys that effectively scope access were missing from this list, causing false positives in checks like kms_key_policy_is_not_public.

For example, a KMS key policy using kms:CallerAccount + kms:ViaService to restrict access to a specific account via a specific AWS service was incorrectly flagged as public because these condition keys were not recognized as restrictive — even though kms:CallerAccount is functionally equivalent to aws:SourceAccount.

Description

Add the following condition keys to valid_condition_options in is_condition_block_restrictive() (both StringEquals and StringLike operators):

  • kms:CallerAccount — functionally equivalent to aws:SourceAccount; restricts KMS key usage to a specific AWS account
  • kms:ViaService — restricts KMS key usage to calls made through a specific AWS service (e.g., glue.eu-central-1.amazonaws.com)
  • aws:CalledVia — global condition key that restricts to calls made through specific AWS services (confused deputy prevention)
  • aws:CalledViaFirst / aws:CalledViaLast — variants that match the first/last service in the call chain

This fixes false positives for any check that relies on is_policy_public() or is_condition_block_restrictive(), including but not limited to kms_key_policy_is_not_public.

Steps to review

  1. Review the added condition keys in prowler/providers/aws/services/iam/lib/policy.py (lines ~604–647) — they follow the same pattern as existing entries like s3:resourceaccount
  2. Review the 15 new tests in tests/providers/aws/services/iam/lib/policy_test.py:
    • 4 unit tests for aws:CalledVia/CalledViaFirst/CalledViaLast via is_condition_block_restrictive
    • 8 unit tests for kms:CallerAccount and kms:ViaService via is_condition_block_restrictive (valid/invalid, str/list)
    • 3 integration tests for is_policy_public with realistic KMS key policies
  3. Verify all 275 existing tests still pass (no regressions)

Checklist

SDK/CLI

  • Are there new checks included in this PR? No

License

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

kagahd and others added 2 commits April 7, 2026 14:39
…n is_policy_public

Add kms:CallerAccount, kms:ViaService, aws:CalledVia, aws:CalledViaFirst,
and aws:CalledViaLast to valid_condition_options in is_condition_block_restrictive.

These service-specific and global condition keys are functionally equivalent
to existing restrictive keys (e.g. kms:CallerAccount acts like aws:SourceAccount)
but were not recognized, causing false positives in checks like
kms_key_policy_is_not_public for policies that use them to scope access.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@kagahd kagahd requested review from a team as code owners April 7, 2026 12:44
@github-actions github-actions bot added provider/aws Issues/PRs related with the AWS provider community Opened by the Community labels Apr 7, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 7, 2026

Conflict Markers Resolved

All conflict markers have been successfully resolved in this pull request.

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 7, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 18.93%. Comparing base (abaacd7) to head (6c6ca4a).
⚠️ Report is 3 commits behind head on master.

❗ There is a different number of reports uploaded between BASE (abaacd7) and HEAD (6c6ca4a). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (abaacd7) HEAD (6c6ca4a)
api 1 0
Additional details and impacted files
@@             Coverage Diff             @@
##           master   #10600       +/-   ##
===========================================
- Coverage   93.55%   18.93%   -74.63%     
===========================================
  Files         225      841      +616     
  Lines       31652    23943     -7709     
===========================================
- Hits        29613     4534    -25079     
- Misses       2039    19409    +17370     
Flag Coverage Δ
api ?
prowler-py3.10-aws 18.93% <ø> (?)
prowler-py3.11-aws 18.93% <ø> (?)
prowler-py3.12-aws 18.93% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
prowler 18.93% <60.00%> (∅)
api ∅ <ø> (∅)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@kagahd
Copy link
Copy Markdown
Contributor Author

kagahd commented Apr 7, 2026

Re: codecov/project failure

The codecov/patch check passes — all modified and coverable lines are covered by tests, as confirmed by the Codecov report.

The codecov/project failure is due to a missing upload issue on fork PRs: HEAD is missing 9 coverage upload flags compared to BASE (oraclecloud, lib, and config flags for py3.10/3.11/3.12). This causes the reported project coverage to drop from 64% to 18%, which is not a real regression but an artifact of incomplete coverage data on fork PRs.

All other CI checks (sdk-tests across 3.10/3.11/3.12, code-quality, security-scans, etc.) pass.

Copy link
Copy Markdown
Member

@pedrooot pedrooot left a comment

Choose a reason for hiding this comment

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

Just one small comment and it's ready to go! 🔝

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@kagahd kagahd requested a review from pedrooot April 7, 2026 18:49
Copy link
Copy Markdown
Member

@pedrooot pedrooot left a comment

Choose a reason for hiding this comment

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

Thanks for this!! 🥇

@pedrooot pedrooot merged commit 89fe867 into prowler-cloud:master Apr 8, 2026
34 of 35 checks passed
HugoPBrito pushed a commit that referenced this pull request Apr 8, 2026
… is_policy_public (#10600)

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community Opened by the Community provider/aws Issues/PRs related with the AWS provider

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants