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

add IAM role for secret manager EC2 access #405

Merged
merged 1 commit into from
Jan 24, 2025

Conversation

DaMandal0rian
Copy link
Member

@DaMandal0rian DaMandal0rian commented Jan 24, 2025

PR Type

enhancement


Description

  • Added IAM role and policy for EC2 access to AWS Secrets Manager.

  • Updated EC2 module to include IAM instance profile for secret access.

  • Modified lifecycle configurations to ignore changes to vpc_security_group_ids.

  • Introduced new Terraform resources for IAM role, policy, and instance profile.


Changes walkthrough 📝

Relevant files
Enhancement
main.tf
Update EC2 module with IAM instance profile                           

auto-drive/main.tf

  • Added iam_instance_profile to EC2 module configuration.
  • Ensures EC2 instances can access AWS Secrets Manager.
  • +1/-0     
    secret.tf
    Define IAM role, policy, and instance profile                       

    auto-drive/secret.tf

  • Created IAM role for EC2 and Lambda services.
  • Added policy to allow access to a specific secret in AWS Secrets
    Manager.
  • Attached policy to the IAM role.
  • Created IAM instance profile for the role.
  • +50/-0   
    main.tf
    Adjust lifecycle to ignore security group changes               

    templates/terraform/aws/ec2/main.tf

  • Updated lifecycle block to ignore changes to vpc_security_group_ids.
  • Ensures stability in EC2 instance configurations.
  • +2/-0     
    main.tf
    Update RDS lifecycle to ignore security group changes       

    templates/terraform/aws/rds/modules/db_instance/main.tf

  • Added lifecycle block to ignore changes to vpc_security_group_ids.
  • Ensures stability in RDS instance configurations.
  • +6/-0     

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
    🧪 No relevant tests
    🔒 Security concerns

    Sensitive information exposure:
    The IAM policy grants access to a specific secret in AWS Secrets Manager. Ensure that the secret ARN is accurate and does not expose sensitive data unintentionally. Additionally, verify that the trust policy for the IAM role does not allow unintended services to assume the role.

    ⚡ Recommended focus areas for review

    Policy Resource Restriction

    The aws_iam_policy resource for Secrets Manager allows access to a specific secret. Ensure that the secret ARN is correct and that it does not unintentionally expose sensitive information or grant overly broad permissions.

    resource "aws_iam_policy" "secrets_manager_policy" {
      name        = "SecretsManagerReadPolicy"
      description = "Policy to allow reading a specific secret from AWS Secrets Manager"
    
      policy = jsonencode({
        Version = "2012-10-17"
        Statement = [
          {
            Effect   = "Allow"
            Action   = "secretsmanager:GetSecretValue"
            Resource = "arn:aws:secretsmanager:${var.region}:${data.aws_caller_identity.current.account_id}:secret:my-app-secret"
          }
        ]
      })
    }
    Role Trust Policy Validation

    The aws_iam_role resource includes both EC2 and Lambda services in its trust policy. Verify that both services require access and that this does not introduce unnecessary permissions.

    resource "aws_iam_role" "auto_secret_role" {
      name = "AutoDriveSecretsManagerAppRole"
    
      assume_role_policy = jsonencode({
        Version = "2012-10-17"
        Statement = [
          {
            Effect = "Allow"
            Principal = {
              Service = [
                "ec2.amazonaws.com",
                "lambda.amazonaws.com"
              ]
            }
            Action = "sts:AssumeRole"
          }
        ]
      })

    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Security
    Restrict Secrets Manager access scope

    Ensure that the Resource field in the aws_iam_policy for Secrets Manager is
    parameterized or restricted to specific secrets to avoid unintentional access to
    other secrets in the account.

    auto-drive/secret.tf [34]

    -Resource = "arn:aws:secretsmanager:${var.region}:${data.aws_caller_identity.current.account_id}:secret:my-app-secret"
    +Resource = "arn:aws:secretsmanager:${var.region}:${data.aws_caller_identity.current.account_id}:secret:${var.secret_name}"
    Suggestion importance[1-10]: 9

    Why: The suggestion improves security by parameterizing the secret name in the IAM policy, reducing the risk of unintentional access to other secrets. This is a critical enhancement for maintaining least privilege access.

    9
    Restrict IAM role assumption conditions

    Add a condition to the assume_role_policy to restrict the role assumption to
    specific EC2 or Lambda instances to enhance security.

    auto-drive/secret.tf [16]

     Action = "sts:AssumeRole"
    +Condition = {
    +  StringEquals = {
    +    "ec2:InstanceProfile" = "arn:aws:iam::${data.aws_caller_identity.current.account_id}:instance-profile/SpecificProfileName"
    +  }
    +}
    Suggestion importance[1-10]: 8

    Why: Adding a condition to restrict role assumption to specific EC2 or Lambda instances enhances security by preventing unauthorized entities from assuming the role. This is a valuable improvement, though it requires careful implementation to avoid breaking functionality.

    8
    Avoid ignoring security group changes

    Avoid ignoring changes to vpc_security_group_ids in the lifecycle block unless
    absolutely necessary, as it may lead to security misconfigurations going unnoticed.

    templates/terraform/aws/ec2/main.tf [199]

     ignore_changes = [
       ami,
       private_ip,
       associate_public_ip_address,
    -  vpc_security_group_ids,
     ]
    Suggestion importance[1-10]: 7

    Why: Ignoring changes to vpc_security_group_ids can lead to unnoticed security misconfigurations. While the suggestion is valid, it may require further consideration of the operational impact before implementation.

    7
    Prevent ignoring RDS security group changes

    Avoid ignoring changes to vpc_security_group_ids in the lifecycle block for the RDS
    instance, as it may lead to potential security risks or misconfigurations.

    templates/terraform/aws/rds/modules/db_instance/main.tf [82]

    -ignore_changes = [
    -  vpc_security_group_ids,
    -]
    +# Removed ignore_changes for vpc_security_group_ids
    Suggestion importance[1-10]: 7

    Why: Ignoring changes to vpc_security_group_ids for the RDS instance could result in security risks. The suggestion is valid and improves security, but it may need to be balanced against operational requirements.

    7

    @DaMandal0rian DaMandal0rian merged commit 5f11fc5 into main Jan 24, 2025
    1 check passed
    @DaMandal0rian DaMandal0rian deleted the add-iam-secret-manager-role branch January 24, 2025 18:56
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    2 participants