Add AWS Lambda code signing capability with secure S3 configuration #28
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Comment on lines
+3
to
+12
| resource "aws_s3_bucket" "lambda_source" { | ||
| bucket = "${var.name}-lambda-source-${data.aws_caller_identity.current.account_id}" | ||
| force_destroy = true | ||
| #checkov:skip=CKV_AWS_18: AWS Access logging not enabled on S3 buckets | ||
| #checkov:skip=CKV_AWS_144: Region replication not enabled on S3 bucket | ||
| #checkov:skip=CKV2_AWS_62: Ensure S3 buckets should have event notifications enabled | ||
| # These security controls are suppressed as this bucket is only used temporarily for Lambda code signing | ||
| # and is not intended for long-term storage or public access. The bucket has other security measures | ||
| # like encryption and public access blocking enabled. | ||
| } |
Check warning
Code scanning / checkov
Ensure S3 buckets should have event notifications enabled Warning
Comment on lines
+3
to
+12
| resource "aws_s3_bucket" "lambda_source" { | ||
| bucket = "${var.name}-lambda-source-${data.aws_caller_identity.current.account_id}" | ||
| force_destroy = true | ||
| #checkov:skip=CKV_AWS_18: AWS Access logging not enabled on S3 buckets | ||
| #checkov:skip=CKV_AWS_144: Region replication not enabled on S3 bucket | ||
| #checkov:skip=CKV2_AWS_62: Ensure S3 buckets should have event notifications enabled | ||
| # These security controls are suppressed as this bucket is only used temporarily for Lambda code signing | ||
| # and is not intended for long-term storage or public access. The bucket has other security measures | ||
| # like encryption and public access blocking enabled. | ||
| } |
Check warning
Code scanning / checkov
Ensure the S3 bucket has access logging enabled Warning
Comment on lines
+3
to
+12
| resource "aws_s3_bucket" "lambda_source" { | ||
| bucket = "${var.name}-lambda-source-${data.aws_caller_identity.current.account_id}" | ||
| force_destroy = true | ||
| #checkov:skip=CKV_AWS_18: AWS Access logging not enabled on S3 buckets | ||
| #checkov:skip=CKV_AWS_144: Region replication not enabled on S3 bucket | ||
| #checkov:skip=CKV2_AWS_62: Ensure S3 buckets should have event notifications enabled | ||
| # These security controls are suppressed as this bucket is only used temporarily for Lambda code signing | ||
| # and is not intended for long-term storage or public access. The bucket has other security measures | ||
| # like encryption and public access blocking enabled. | ||
| } |
Check warning
Code scanning / checkov
Ensure that S3 bucket has cross-region replication enabled Warning
|
⏳ I'm reviewing this pull request for security vulnerabilities and code quality issues. I'll provide an update when I'm done |
💰 Infracost reportMonthly estimate generatedEstimate details (includes details of unsupported resources) |
Terraform Format and Style 🖌
|
![amazon-q-developer[bot]](https://avatars.githubusercontent.com/in/1220912?s=60&v=4){kind=small}
| effect = "Allow" | ||
| principals { | ||
| type = "AWS" | ||
| identifiers = ["arn:aws:iam::${data.aws_caller_identity.current.account_id}:root"] |
There was a problem hiding this comment.
Warning
Description: The KMS key policy grants broad permissions to the root user, which may pose security risks. Consider limiting the permissions granted to the root user in the KMS key policy. Use the principle of least privilege.
Severity: High
There was a problem hiding this comment.
The fix addresses the security concern by limiting the permissions granted to the root user in the KMS key policy. Instead of granting broad permissions to the root user, the fix assumes an AdminRole and grants it a more restricted set of permissions. The actions list has been reduced to include only the essential KMS operations. This change implements the principle of least privilege, reducing potential security risks.
Suggested change
| identifiers = ["arn:aws:iam::${data.aws_caller_identity.current.account_id}:root"] | |
| effect = "Allow" | |
| principals { | |
| type = "AWS" | |
| identifiers = ["arn:aws:iam::${data.aws_caller_identity.current.account_id}:role/AdminRole"] # Assuming an AdminRole exists | |
| } | |
| actions = [ | |
| "kms:Encrypt", | |
| "kms:Decrypt", | |
| "kms:ReEncrypt*", | |
| "kms:GenerateDataKey*", | |
| "kms:DescribeKey" | |
| ] | |
| resources = [aws_kms_key.encrypt_storage.arn] | |
| } |
| type = "AWS" | ||
| identifiers = ["arn:aws:iam::${data.aws_caller_identity.current.account_id}:root"] | ||
| } | ||
| actions = [ |
There was a problem hiding this comment.
Description: The KMS key policy includes a long list of actions, which could be simplified for better readability and maintainability. Consider grouping similar actions or using wildcards where appropriate to reduce the length of the action list.
Severity: Low
There was a problem hiding this comment.
The fix simplifies the long list of KMS actions in the first statement by using a wildcard "kms:*" instead of listing all individual actions. This improves readability and maintainability of the policy document while still granting full KMS permissions to the root user. The second statement for S3 permissions remains unchanged as it requires specific actions.
Suggested change
| actions = [ | |
| identifiers = ["arn:aws:iam::${data.aws_caller_identity.current.account_id}:root"] | |
| } | |
| actions = [ | |
| "kms:*" | |
| ] | |
| resources = [aws_kms_key.encrypt_storage.arn] | |
| } |
|
✅ I finished the code review, and left comments with the issues I found. I will now generate code fix suggestions. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR implements AWS Lambda code signing functionality to enhance the security of our Lambda deployments. Key changes include:
SHA384-ECDSAsigning profile"Enforce"policy for maximum securityThese changes improve our security posture by ensuring Lambda code integrity through cryptographic signing while maintaining a secure deployment pipeline.
The code signing policy is set to
"Enforce"mode, which provides the highest level of security by preventing deployment of unsigned or improperly signed code to our Lambda functions.