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

Review and Identify Unnecessary Config Rules to Reduce Security Hub Alerts #8961

Open
3 of 5 tasks
sukeshreddyg opened this issue Jan 15, 2025 · 14 comments
Open
3 of 5 tasks
Assignees
Labels

Comments

@sukeshreddyg
Copy link
Contributor

sukeshreddyg commented Jan 15, 2025

User Story

As a Security Engineer,
I want to review the existing config rules and identify which ones are unnecessary, so that we can discuss and decide on actions to reduce the number of alerts sent to Security Hub.

Value / Purpose

By identifying unnecessary config rules, we can reduce the volume of alerts in Security Hub, which will help improve the efficiency of alert management and prioritization of critical findings.

Context / Background

It has been observed that several config rules were created a long time ago, and many of them may no longer be relevant or necessary. These rules are generating a high number of security findings, which are sent to Security Hub. Reviewing and identifying which rules are unnecessary will help us streamline the process and address alert overload.

Useful Contacts

No response

Additional Information

No response

Definition of Done

  • A thorough review of all existing config rules is conducted.
  • A list of config rules that are potentially unnecessary or redundant is created.
  • The list is shared with the team for further discussion and evaluation.
  • If the team agrees, the identified config rules will be removed.
  • The list of unnecessary config rules is documented and made available for future action.
@mikereiddigital
Copy link
Contributor

Beginning the review with this - https://github.com/ministryofjustice/modernisation-platform-terraform-baselines/blob/main/modules/config/main.tf. This contains the config rules applied via the baselines module. No other MP-related config rules are listed in a search of the ministryofjustice github org using "aws_config_config_rule".

@mikereiddigital
Copy link
Contributor

mikereiddigital commented Jan 28, 2025

  1. Whether the rule is still required or not,
  2. Wether the rule is required but does not need notification sent to the Security Hub.

Will noncompliant resources issue a securityhub alert.

Also note:
AWS Config rules with the securityhub- prefix are pre-configured rules that AWS Security Hub enables as part of its Security Standards checks. These rules are automatically created in AWS Config when you enable Security Hub and enable specific Security Standards (e.g., CIS AWS Foundations Benchmark, NIST 800-53, PCI DSS).

@mikereiddigital
Copy link
Contributor

mikereiddigital commented Jan 29, 2025

Initial pass over the custom rules (using MP account as reference) - https://docs.google.com/spreadsheets/d/1LfAvUiKYMvM_yJbb2VrzLIVQ2kUNy3Not0YPAd07K4o/edit?usp=sharing

This afternoon will look at those custom rules (i.e. no SH equivalent) are being shared or not. Note these are only subscribed to by a PagerDuty sub. There is no native / out-of-the-box means for SecurityHub to subscribe to these config rule events.

@mikereiddigital
Copy link
Contributor

mikereiddigital commented Jan 29, 2025

Looking at the SH alert that is generating the bulk of the alerts to slack: "AWS Config should be enabled and use the service-linked role for resource recording"

https://docs.aws.amazon.com/securityhub/latest/userguide/config-controls.html#config-1

The ConfigurationRecorders detail is as follows:

~ » aws configservice describe-configuration-recorders --output json

{
"ConfigurationRecorders": [
{
"name": "config",
"roleARN": "arn:aws:iam::xxxxxxxxxxxx:role/AWSConfig",
"recordingGroup": {
"allSupported": true,
"includeGlobalResourceTypes": true,
"resourceTypes": [],
"exclusionByResourceTypes": {
"resourceTypes": []
},
"recordingStrategy": {
"useOnly": "ALL_SUPPORTED_RESOURCE_TYPES"
}
},
"recordingScope": "PAID"
}
]
}

I am investigating whether instead of the custom role, that we use the aws-service-role - AWSServiceRoleForConfig

https://docs.aws.amazon.com/config/latest/developerguide/using-service-linked-roles.html

Certainly in modernisation-platform this service-linked-role has not been created. (aws iam create-service-linked-role --aws-service-name config.amazonaws.com)

Or via terraform

resource "aws_iam_service_linked_role" "aws_config" {
  aws_service_name = "config.amazonaws.com"
}

@mikereiddigital
Copy link
Contributor

So in summary:

  • To discuss the list of config rules to keep in the above spreadsheet.
  • To switch the AWSConfig role to a linked service role.
  • To consider whether to create a custom lambda to push the AWSConfig rule findings to SecurityHub and have the alerts managed from there.

@mikereiddigital
Copy link
Contributor

mikereiddigital commented Jan 30, 2025

Update for today:

  1. Confirmed with AWS the approach for using a service-linked role to replace our custom role for AWSConfig.

  2. our terraform-baselines module contains the securityhub configuration for each account - https://github.com/ministryofjustice/modernisation-platform-terraform-baselines/tree/main/modules/securityhub.

  3. There are two separate SNS topics for SecurityHub output:

a. sechub_high_and_critical_findings which is an Event rule that alerts to pagerduty for NEW & CRITICAL issues only. So events outside of this will not have a slack alert.

b. securityhub_alarms which is defined in this module - https://github.com/ministryofjustice/modernisation-platform-terraform-baselines/blob/main/modules/securityhub-alarms that contains a number of cloudwatch alarms to cover events such as s3 bucket policy changes and sign-in failures. These do not appear to be routed to security hub.

The significant issue is that due to the linked regions across which SecurityHub will issue notifications, so where a critical issue occurs that are global (e.g. concerning an IAM user) then we will have an alert for each region. This goes some way to explain the large number of continued Config.1 rule alerts for the service-linked-role.

@mikereiddigital
Copy link
Contributor

Terraform plan in the sprinkler bootstrap for the amended config resources to use the service-linked role - https://github.com/ministryofjustice/modernisation-platform/actions/runs/13073077815/job/36478790311?pr=9146

@mikereiddigital
Copy link
Contributor

mikereiddigital commented Jan 31, 2025

Baselines branch containing the changes: https://github.com/ministryofjustice/modernisation-platform-terraform-baselines/blob/issue/8961/config.tf

So the apply into sprinkler bootstrap of the above failed with the following error:

Error: attaching IAM Policy (arn:aws:iam::aws:policy/AWSConfigPublishPolicy) to IAM Role (AWSServiceRoleForConfig): operation error IAM: AttachRolePolicy, https response error StatusCode: 404, RequestID: 45794ef9-a61c-43f6-adc8-1382625fe5c0, NoSuchEntity: The role with name AWSServiceRoleForConfig cannot be found.

Though attempts to use a depends_on generated a significant number of plan errors due to the arn of the new service-linked-role not being known until after apply. The changes to the Config recorder were applied and to undo that change the recorders in the various regions where config is also added, the recorder had to be manually amended to use the old role (which had been recreated by the above apply). Once that was done, the rollback could be applied.

Further investigation is needed to understand the above terraform error.

Edit:

So a possible solution is for the policy to be recreated as a bucket policy with AWS Config as the principal. I will look into this on Tuesday.

@mikereiddigital
Copy link
Contributor

mikereiddigital commented Feb 4, 2025

So, this branch covers the changes required. However it should be noted that the following changes from here will also be added when we create a new release. This accounts for the additional resources in the plans. Will raise at standup tomorrow morning to confirm this.

@mikereiddigital
Copy link
Contributor

mikereiddigital commented Feb 5, 2025

Testing the changes in sprinkler, the default scope of the service-linked role does not have the required permissions needed to write to the s3 bucket. To demonstrate this, using the IAM simulator feature of the cli:

aws iam simulate-principal-policy \
  --policy-source-arn arn:aws:iam::xxxxxxxxxxxx:role/aws-service-role/config.amazonaws.com/AWSServiceRoleForConfig \
  --action-names s3:PutObject s3:GetBucketAcl \
  --resource-arns arn:aws:s3:::<bucket-name-redacted>/AWSLogs/xxxxxxxxxxxx/Config/*

has the result:

EVALUATIONRESULTS       s3:PutObject    implicitDeny    arn:aws:s3:::<bucket>/AWSLogs/xxxxxxxxxxxx/Config/*
ORGANIZATIONSDECISIONDETAIL     False
EVALUATIONRESULTS       s3:GetBucketAcl implicitDeny    arn:aws:s3:::<bucket>/AWSLogs/xxxxxxxxxxxx/Config/*
ORGANIZATIONSDECISIONDETAIL     False

Additionally, it was noted that the role is unable to write using secure transport as the deny statement in the bucket module's policy prevents it.

Investigating how we can add these additional permissions.

Can also confirm that the Config service is no longer publishing messages over SNS. Investigating it shows that the topic is using the default policy. A new policy will need to be added to the module.

@mikereiddigital
Copy link
Contributor

Tracked the issue to this error:

Insufficient delivery policy to s3 bucket: config-20210729141032573100000001, unable to write to bucket, provided s3 key prefix is 'null', provided kms key is 'null'.

@mikereiddigital
Copy link
Contributor

mikereiddigital commented Feb 6, 2025

Will be raising a further ticket today to resolve the ongoing SNS message send failures. As context, according to the attached policy, the service-linked role (AWSServiceRoleForConfig) does not have the required sns:Publish permissions needed.

                "sns:GetDataProtectionPolicy",
                "sns:GetSMSSandboxAccountStatus",
                "sns:GetSubscriptionAttributes",
                "sns:GetTopicAttributes",
                "sns:ListSubscriptions",
                "sns:ListSubscriptionsByTopic",
                "sns:ListTagsForResource",
                "sns:ListTopics",

@mikereiddigital
Copy link
Contributor

mikereiddigital commented Feb 6, 2025

Call with AWS. The issue is due to the service-linked role not having permissions to use the default SNS key for the topic. Instead we need to add a CMK and a policy for it.

Create a multi-region CMK & policy and added it to the topic. No success. Note the following permissions the service-linked role has - it's missing encrypt and decrypt:

                "kms:DescribeKey",
                "kms:GetKeyPolicy",
                "kms:GetKeyRotationStatus",
                "kms:ListAliases",
                "kms:ListKeys",
                "kms:ListResourceTags",

This doc contains the policy scope and the reasoning for it - https://docs.aws.amazon.com/config/latest/developerguide/s3-kms-key-policy.html#granting-access-s3-kms-key

SNS Issue Resolved.

@mikereiddigital
Copy link
Contributor

Baselines PR containing the final set of changes - ministryofjustice/modernisation-platform-terraform-baselines#726

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: In Progress
Development

No branches or pull requests

3 participants