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

Enhancement/Update AWS CIS Benchmark to 1.2.0 #679

Merged
merged 59 commits into from
Aug 19, 2020

Conversation

prisas
Copy link
Contributor

@prisas prisas commented Mar 19, 2020

Changes:

  • Creates the new AWS CIS 1.2.0 json ruleset, the required new findings and updates the findings to the new output format
    • Including 23 new rules
  • Adds support for AWS CloudWatch Metric Filters
  • Adds support for AWS Peering Connections
  • Adds support for AWS Flow Logs (Subnet & VPC)

This PR is for issue #434

Phases:

  • Create all the new AWS CIS 1.2.0 IAM rules and update the format of the old ones
  • Create all the new AWS CIS 1.2.0 Logging rules and update the format of the old ones
  • Create all the new AWS CIS 1.2.0 Monitoring rules and update the format of the old ones
  • Create all the new AWS CIS 1.2.0 Networking rules and update the format of the old ones

TODOs for @j4v:

  • Complete finding 1.14 of the cis-1-2-0.json ruleset
    This finding is already created in the rule iam-root-account-no-hardware-mfa.json. The resource iam/credentialreports.py is already set to fetch the VirtualMfaDevices and parse it. The parsing of the returned data should get the serial-number of the mfa device and also a boolean containing whether the MFA device is hardware based or not.
    To check if a MFA device is hardware or virtual, the serial-number should be checked. If the serial-number contains "arn", then the device is virtaul. Otherwise, the device is hardware.
    Once done, issue Add check for hardware based MFA for the root account in AWS #681 can be closed.
  • Ensure there aren't any duplicate findings in pub/prop
    • Compare EC2 SG findings as there might be duplicates
  • Ensure new findings are added to default ruleset (+prop) where appropriate
  • Ensure new findings are added to detailed ruleset where appropriate
    • Review the arguments used in the detailed ruleset
      There are some findings that may have inappropriate arguments. For example, the iam-password-policy-minimum-length finding has the minimum length argument set to 8 when the CIS recommendation is 14.
  • Review peering connection implementation/partial
  • Review hidden flow logs implementation/partial
  • Fix findings 3.1 to 3.14 of cis-1-2-0.json ruleset not returning an empty list when clicked.
    Further information of this issue can be found here: Enhancement/Update AWS CIS Benchmark to 1.2.0 #679 (comment)

@prisas prisas requested a review from x4v13r64 March 19, 2020 10:51
@prisas prisas self-assigned this Mar 19, 2020
@prisas prisas added the component-provider-aws Affects AWS provider label Mar 19, 2020
@prisas prisas changed the title Added 2 rules for AWS CIS 1.2.0 and updated rules format Enhancement/Update AWS CIS Benchmark to 1.2.0 Mar 20, 2020
@prisas
Copy link
Contributor Author

prisas commented Mar 20, 2020

Updated all the old CIS Benchmark 1.0.0 IAM rules to the new format and added all the new CIS Bechmark 1.2.0 IAM rules.

There are still a couple of issues to fix or check but it should all work with some minor tweaks.

@x4v13r64 x4v13r64 mentioned this pull request Mar 22, 2020
11 tasks
@x4v13r64 x4v13r64 added the WIP label Mar 22, 2020
@x4v13r64 x4v13r64 added this to the 5.9.0 milestone Mar 22, 2020
@x4v13r64 x4v13r64 linked an issue Mar 22, 2020 that may be closed by this pull request
2 tasks
@x4v13r64 x4v13r64 changed the base branch from develop to issues/611 March 22, 2020 14:58
@prisas
Copy link
Contributor Author

prisas commented Mar 23, 2020

I just pushed a new commit where I updated all the old CIS Benchmark 1.0.0 Logging rules to the new format and added all the new CIS Bechmark 1.2.0 Logging rules.

There are a few minor issues to be fixed before it works as intended.

@codecov-io
Copy link

codecov-io commented Mar 23, 2020

Codecov Report

Merging #679 into develop will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop     #679   +/-   ##
========================================
  Coverage    65.03%   65.03%           
========================================
  Files           22       22           
  Lines         1530     1530           
========================================
  Hits           995      995           
  Misses         535      535           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7c576ae...33ae2b0. Read the comment docs.

@prisas
Copy link
Contributor Author

prisas commented Mar 24, 2020

Current errors to be fixed:

  • iam_unusued_credentials_not_disabled. Check the error on condition 'olderThan' for values where last_used is NoneType.
  • iam-no-support-role. Is this the correct way to check if the policy AWSSupportAccess (created by default) is attached to at least 1 user?
  • iam-managed-policy-allows-full-privileges. Fix error: list index out of range.
  • cloudtrail-no-logging. Need to add an extra verification (recommended by CIS): Condition to verify there is at least one Event Selector for a Trail with IncludeManagementEvents set to true and ReadWriteType set to All
  • Recommendation 2.3. Should we create a new finding exclusive to CloudTrail S3 buckets or reuse the already created ones?
  • Recommendation 2.6. Should be specific to CloudTrail S3 buckets, currently the cis 1.2.0 ruleset has a general bucket logging finding in this recommendation. Finish finding cloudtrail-s3-bucket-no-logging to verify the bucket has the principal cloudtrail.amazonaws.com and logging disabled.
  • vpc-subnet-without-flow-log. Would like to make the flow log counter higllighted in red when there are 0 flow logs (services.vpc.regions.id.vpcs.id.subnets.html partial)
  • Recommendation 4.4 ec2-routes-tables-full-peering. Complete finding conditions with the newly created object of VPCs route_tables. Also modify description to better explain the end user that this finding depends on their requirements for each instance

@x4v13r64 x4v13r64 self-assigned this Apr 6, 2020
….2.0

# Conflicts:
#	ScoutSuite/output/data/html/partials/aws/services.kms.regions.id.keys.html
#	ScoutSuite/providers/aws/resources/iam/credentialreports.py
#	ScoutSuite/providers/aws/rules/findings/cloudtrail-no-log-file-validation.json
#	ScoutSuite/providers/aws/rules/findings/cloudtrail-no-logging.json
#	ScoutSuite/providers/aws/rules/findings/config-recorder-not-configured.json
#	ScoutSuite/providers/aws/rules/findings/ec2-default-security-group-in-use.json
#	ScoutSuite/providers/aws/rules/findings/ec2-default-security-group-with-rules.json
#	ScoutSuite/providers/aws/rules/findings/ec2-security-group-opens-known-port-to-all.json
#	ScoutSuite/providers/aws/rules/findings/iam-password-policy-expiration-threshold.json
#	ScoutSuite/providers/aws/rules/findings/iam-password-policy-minimum-length.json
#	ScoutSuite/providers/aws/rules/findings/iam-password-policy-no-lowercase-required.json
#	ScoutSuite/providers/aws/rules/findings/iam-password-policy-no-number-required.json
#	ScoutSuite/providers/aws/rules/findings/iam-password-policy-no-symbol-required.json
#	ScoutSuite/providers/aws/rules/findings/iam-password-policy-no-uppercase-required.json
#	ScoutSuite/providers/aws/rules/findings/iam-password-policy-reuse-enabled.json
#	ScoutSuite/providers/aws/rules/findings/iam-root-account-no-mfa.json
#	ScoutSuite/providers/aws/rules/findings/iam-root-account-used-recently.json
#	ScoutSuite/providers/aws/rules/findings/iam-root-account-with-active-keys.json
#	ScoutSuite/providers/aws/rules/findings/iam-user-no-key-rotation.json
#	ScoutSuite/providers/aws/rules/findings/iam-user-with-policies.json
#	ScoutSuite/providers/aws/rules/findings/iam-user-without-mfa.json
#	ScoutSuite/providers/aws/rules/findings/s3-bucket-no-logging.json
#	ScoutSuite/providers/aws/rules/findings/vpc-subnet-without-flow-log.json
@x4v13r64
Copy link
Collaborator

Finally ready to merge! Thanks @paurisa for the great work on this.

@x4v13r64 x4v13r64 marked this pull request as ready for review August 19, 2020 16:09
@x4v13r64 x4v13r64 merged commit c84e889 into develop Aug 19, 2020
@x4v13r64 x4v13r64 deleted the Enhancement/Update_AWS_CIS_Benchmark_to_1.2.0 branch August 19, 2020 16:09
@prisas
Copy link
Contributor Author

prisas commented Aug 19, 2020

No problem, it is a pleasure to contribute to this useful project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component-provider-aws Affects AWS provider enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update AWS CIS Benchmark Ruleset
5 participants