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

ACK RDS IRSA role requires access to alias/secretsmanager KMS key or fails to create DB. #45

Closed
bdellegrazie opened this issue Jul 25, 2023 · 6 comments

Comments

@bdellegrazie
Copy link
Contributor

bdellegrazie commented Jul 25, 2023

Description

When using the ACK Controller for RDS, I encountered a problem using the Secrets Manager feature of RDS.

In my circumstance, I was using a custom KMS key and gave, to the IRSA role, permission to create the secret,
access the KMS key and allow the associated grants as documented in AWS documentation.

However the controller still failed citing insufficient permissions on the KMS key.

Upon using CloudTrail I discovered the controller was still performing kms:DescribeKey on the default KMS key (alias/secretsmanager), even though I had supplied a specific one for secret in the resource.

Once I permitted kms:DescribeKey on the default KMS key for secrets manager, everything started working properly.

I have three questions:

  1. Can we please update the documentation to ensure that this is reflected in the required IAM permissions for the controller to avoid others having the same issues if using SecretsManager facility.
  2. Is this a bug of the controller? should it only perform kms:DescribeKey on the key supplied in the CRD?
    If so, where should I report this?
  3. Should an additional policy be created as an example for one that can be attached to the IAM IRSA role to fix this?

Versions

  • Module version 2.0.1

Steps to reproduce the behavior:

Expected behavior

CRD for dummy single instance small RDS:

---
apiVersion: rds.services.k8s.aws/v1alpha1
kind: DBInstance
metadata:
  name: testdb
spec:
  allocatedStorage: 10
  autoMinorVersionUpgrade: true
  backupRetentionPeriod: 1
  dbInstanceClass: db.t4g.micro
  dbInstanceIdentifier: testdb
  dbSubnetGroupName: <pre-existing-group>
  deletionProtection: false
  engine: postgres
  engineVersion: "14"
  kmsKeyID: <alias or ARN of pre-existing KMS key>
  manageMasterUserPassword: true
  masterUserSecretKMSKeyID: <alias or ARN of pre-existing KMS key>
  masterUsername: "postgres"
  multiAZ: false
  networkType: IPV4
  publiclyAccessible: false
  storageEncrypted: true
  storageType: gp2
  vpcSecurityGroupIDs:
    - <pre-existing security group ID>

Extra IAM policy added to IRSA role:

data "aws_iam_policy_document" "this" {
  statement {
    sid = "AllowKMSUseByRDS"
    actions = [
      "kms:CreateGrant",
      "kms:DescribeKey",
      "kms:ListGrants",
      "kms:RevokeGrant",
    ]

    resources = local.kms_keys  # array of both custom KMS keys for EBS and secrets manager

    condition {
      test     = "StringEquals"
      variable = "kms:ViaService"
      values   = local.rds_services
    }
  }

  statement {
    sid = "AllowKMSUseForSMByIRSA"
    actions = [
      "kms:Encrypt",
      "kms:Decrypt",
      "kms:DescribeKey",
      "kms:GenerateDataKey",
      "kms:CreateGrant",
    ]

    resources = var.secretsmanager_kms_keys
  }

  statement {
    sid = "AllowSMUseByIRSA"
    actions = [
      "secretsmanager:CreateSecret",
      "secretsmanager:DeleteSecret",
      "secretsmanager:RotateSecret",
      "secretsmanager:TagResource",
    ]
    resources = [local.account_sm_arn]
  }
}

Expects to create DB and create a secret with the postgres randomly generated password.

Actual behavior

Fails with insufficient permissions for KMS key (KMS key ARN for custom secrets manager KMS key)

Additional context

Further examination in CloudTrail sees the a failure on kms:DescribeKey but for the default KMS key alias for secrets manager (alias/secretsmanager)

Modifying the policy to allow access (full or just kms:DescribeKey) to the default KMS key results in success, an example such statement is below:

  # Must grant DescribeKey to all KMS keys or ACK controller fails, even if the default KMS key is not used
  statement {
    sid = "AllowKMSDescribeKeyForRDS"
    actions = [
      "kms:DescribeKey",
    ]

    resources = ["arn:${local.partition}:kms:*:${local.account_id}:key/*"]

    condition {
      test     = "StringEquals"
      variable = "kms:ViaService"
      values   = local.rds_services
    }
  }

Note 1: I tried using ResourceAliases condition to limit the kms:DescribeKey permission to just the default secretsmanager KMS key but that, surprisingly, didn't work.

@bdellegrazie
Copy link
Contributor Author

bdellegrazie commented Jul 25, 2023

Sorry I slightly updated the main comment to reduce additional context and simplify the additional IAM statement
In the kms:DescribeKey condition I tried using a ResourceAliases condition to limit DescribeKey to the default one but that didn't work. In the end I had to grant DescribeKey to all the account's keys or the default KMS key ARN directly.

@github-actions
Copy link

This issue has been automatically marked as stale because it has been open 30 days
with no activity. Remove stale label or comment or this issue will be closed in 10 days

@github-actions github-actions bot added the stale label Aug 26, 2023
@bdellegrazie
Copy link
Contributor Author

Remove stale please

@github-actions github-actions bot removed the stale label Aug 29, 2023
@luong-komorebi
Copy link
Contributor

Simple solution for anyone looking to quickly mitigate this lack-of-permission situation:
Add this to the module

module "..." {
  source = "aws-ia/eks-ack-addons/aws"
  # [....]
  # rds is missing KMS access https://github.com/aws-ia/terraform-aws-eks-ack-addons/issues/45
  rds = {
    role_policies = {
      AmazonRDSFullAccess = "arn:${data.aws_partition.current.partition}:iam::aws:policy/AmazonRDSFullAccess"
      SecretsManagerReadWrite = "arn:${data.aws_partition.current.partition}:iam::aws:policy/SecretsManagerReadWrite"
    }
  }
}

data "aws_partition" "current" {}

Copy link

github-actions bot commented Aug 7, 2024

This issue has been automatically marked as stale because it has been open 30 days
with no activity. Remove stale label or comment or this issue will be closed in 10 days

@github-actions github-actions bot added the stale label Aug 7, 2024
Copy link

Issue closed due to inactivity.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Aug 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

No branches or pull requests

3 participants