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

ADA-CP-AWS 1.0: first 13 rules #1687

Open
wants to merge 34 commits into
base: develop
Choose a base branch
from
Open

Conversation

rdegraaf
Copy link

Description

This MR adds a new ruleset for AWS, ada-cp-1.0 that incorporates the first 13 of the 40 rules from ADA-CP 1.0 that apply to AWS:

A few of the rules in the new ruleset are unmodified existing rules. However, most of them are new because either ScoutSuite did not have a similar rule or the existing rule did not match ADA-CP's requirements. In addition to new rules, I had to add and modify some comparison conditions, add facades to pull additional data, and add and modify some of the HTML display. I added new rules that did not have existing close equivalents into the "detailed" ruleset.

All new and modified rules have unit tests. Note that some of the unit tests fail in main; I did not fix the broken tests as I wasn't sure what they're supposed to do. I made some improvements to the unit tests for the rule engine; notably, it now fixes the time at which rules run (for testing rule data that includes timestamps) and allows additional rules to be added to extend the "default" ruleset while running tests.

Fixes #1653, #1654, #1655, #1656, #1657, #1660, #1683, #1684, #1685, #1686

Type of change

Select the relevant option(s):

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works (optional)
  • New and existing unit tests pass locally with my changes

(Note: some existing unit tests are broken in main; my code passes all new and existing unit tests that pass in main.

AWS doesn't have a programmatic interface for runtime deprecation dates.
Consequently, I used a horrible kludge: hard-coding dates copied from
https://docs.aws.amazon.com/lambda/latest/dg/lambda-runtimes.html.  If
the table hasn't been updated in the last 180 days, ScoutSuite will warn
the user.

Owing to this kludge, this rule has *not* been added to the default
ruleset. However, it is in the detailed ruleset.  If AWS ever adds a
programmatic interface, we can update this rule and add it to the
default ruleset.
This test handle only rule processing, not data acquisition (which is
where most of the logic is).

test_rules_processingengine.py only used rules from the default ruleset.
Since awslambda-runtime-deprecated is *not* in the default ruleset, I
repurposed some dead code relating to a test ruleset: the test ruleset
is now merged with the default ruleset when running this test.
Contains only one rule at the moment, more to come.
The existing rule did not work: the IAM facade only enumerated AWS-managed
permission policies that were attached to things so the rule would never
trigger.  Additionally, the existing rule allowed AWSSupportAccess to be
attached to a User or Group, whereas CIS and ADA specifically require it
to be attached to a Role.

Unfortunately, there is no way to call iam:ListPolicies for only a specific
permission policy; the only filtering is by attachment status (which
doesn't help) and by path (which also doesn't help since AWSSupportAccess
is at the root path).  So we have no choice but to pull all permission
policies, including the hundreds of AWS-managed policies that the account
doesn't use at all.  Then we end up pulling their contents and
attachments, which takes a long time.  I optimize this by skipping
unattached policies when querying for contents and attachments.  *That*
violated an assumption in the policy-parsing logic that all permission
policies are valid, so I patched that to skip empty policies (yielding
empty lists of statements, which seems to be handled safely everywhere
else).

With this done, I changed the rule's logic to fail if either
AWSSupportAccess is attached to something but not to a Role or if it is
not attached to anything at all.

I added the rule to the ADA-CP ruleset.  I dropped it from "danger" to
"warning" in the "detailed" ruleset because it's sort of a silly check as
it assumes that an account is being managed in a very specific way.
…er setup for all IAM users that have a console password"

This implementation is based off the description and Console
investigation procedure.  The CLI investigation procedure and
verification describe a very different check.
appdefensealliance/ASA-WG#69 tracks the
discrepancy in the ADA-CP spec.
…nistrative privileges are not attached"

The rule is intended to force operators to grant admin permission only
using the built-in "AdministratorAccess" permission policy, but the spec
currently ignores the possibility of an in-line permission policy granting
admin.  appdefensealliance/ASA-WG#17 tracks the
issue.
I had been using the existing rule "iam-root-account-with-active-keys"
for this requirement, but then noticed that the title and AWS CLI
investigation procedure for this ADA-CP requirement also prohibit
inactive root access keys; our existing rule only prohibits *active*
root access keys.  The AWS Console investigation procedure in the ADA-CP
requirement also allows inactive root access keys.  The issue is tracked
at appdefensealliance/ASA-WG#138.  Assuming
that they resolve it in favour of the title, I added this new rule.  If
they go the other way, we can remove this rule.
…ngth of 14 or greater"

This is an unmodified existing rule; I only added compliance and
reference details.
"Ensure there is only one active access key available for any single IAM
user" and "Ensure access keys are rotated every 90 days or less".

These are unmodified existing rules; I only added compliance and
reference details.
…euse"

For this one, I modified the existing rule
"iam-password-policy-reuse-enabled".  Previously, that rule only checked
that there the account was configured to prevent reuse of some number of
previous passwords; I added a parameter to put a lower bound on the
number of remembered passwords.  This required me to modify the existing
rulesets; a parameter of 1 is equivalent to the old check.

I also modified the data model and rule display somewhat: ScoutSuite
previously synthesized a boolean to indicate whether password reuse
prevention was on in addition to storing the number of past passwords to
remember.  But AWS doesn't have that boolean; reuse prevention is
disabled if there is no number of past passwords to remember.  It only
allows 1-24 passwords to be remembered, so remembering 0 is equivalent
to having the feature disabled.  So I changed the data model to store 0
if it's disabled and check based on that.  This makes the rule logic and
display simpler.

While I was changing the data display, I found that output for the
settings controlling whether users are allowed to change their own
passwords was gated on password reuse prevention being enabled.  There
is no such dependency within AWS, so I removed that condition.
Load the AWS IAM Account Summary, displayed in its own page.

Re-implemented 'iam-root-account-with-access-keys' using data from the
IAM account summary.
The new method should work reliably in all partitions.
With this change, all rules currently in `ada-cp-1.0.json` for AWS have at
least a minimal test case in `tests/test_rules_processingengine.py`, which
passes.

`tests/test_rules_ruleset.py` continues to fail, but I did not introduce
that bug.  Some of the other tests also throw warnings.
Used the "freezegun" module to fix the date at 2024-01-01 for all unit
tests.  Modified the times for the one existing, applicable test case to
suit this date.
…ter are disabled"

This rule is closely related to "iam-unused-credentials-not-disabled".
However, that rule applies only to credentials that have been used; both
ADA-CP and CIS 2.0 also apply to credentials that have *not* been used.
I haven't checked earlier versions of CIS, so maybe they are more
limited in scope; alternately, "iam-unused-credentials-not-disabled" may
be just wrong and should be replaced with this new rule.
…rative and daily tasks

This rule is closely related to "iam-root-account-used-recently.json".
However, that rule only applies to passwords; both ADA-CP 1.0 and CIS
2.0 also apply to API keys.  I haven't checked earlier versions of CIS,
so maybe they are more limited in scope; alternately,
"iam-root-account-used-recently" may just be wrong and should be
replaced with this new rule.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants