allow the usage of an encrypted AMI

[v-rosa]

Currently its not possible to use an AMI encrypted using an external KMS key.

Something like this would fix this issue. https://github.com/v-rosa/terraform-aws-github-runner/tree/allow-using-encrypted-ami

Would be great to have this feature, let me know if these changes are OK for a PR.

Co-authored with @rsmolinski

[reedloden]

(random lurker here that just saw this issue)

Wouldn't it make sense to go ahead and submit as a PR versus waiting to hear if you should? Saves an unneeded step. :-)

[github-actions]

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed if no further activity occurs. Thank you for your contributions.

[reedloden]

not stale :-)

[marko-fabry]

Hello @v-rosa , I have tried your shared encrypted AMI feature in the 2.4.0 release but I was not able to get the scale-up lambda to actually run an EC2 instance without adding following IAM permission to the role:

statement {
    sid    = "AllowKMSCreateGrant"
    effect = "Allow"
    actions = [
      "kms:CreateGrant",
    ]
    resources = [
      var. ami_kms_key_arn
    ]
    condition {
      test     = "Bool"
      variable = "aws:ViaAWSService"
      values   = ["true"]
    }
  }

Did you maybe set some special permissions on the KMS key, so that this is not needed?

[v-rosa]

Hello @marko-fabry indeed you are right, we're missing the Kms:CreateGrant permission I have it in my example, not sure why I missed it in the initial PR.

Even though the Kms key policy allows a given identity to use the key, the KMS service also requires the permissions to be set on the user/role allowed to use the key.

Edit: this not applies for grants, it can be set either on the key policy or IAM policy. See:

Principals can get permission to create grants from a key policy or IAM policy. Principals who get kms:CreateGrant permission from a policy can create grants for any grant operation on the KMS key. These principals are not required to have the permission that they are granting on the key. When you allow kms:CreateGrant permission in a policy, you can use policy conditions to limit this permission.

https://docs.aws.amazon.com/kms/latest/developerguide/grants.html

I will try to open a PR today to fix this. Thanks for the report.

[marko-fabry]

Thank you :) Much appreciated.

[v-rosa]

Meanwhile I started to load more context (I'm in PTO ehehe) it might work only with the KMS key policy. Can you share you KMS key policy uses to encrypt the AMI?

[v-rosa]

In you KMS key policy try to add the following statement:

            "Effect": "Allow",
            "Principal": {
                "AWS": [
                    "arn:aws:iam::<account-number>:<root-or-iam-role>"
                ]
            },
            "Action": "kms:CreateGrant",
            "Resource": "*"
        }

And try again to spin up some runners. With this permission on the key policy I could create runners without specifically add Kms:CreateGrant on the lambda policy.

Later the CreateGrant permission in the KMS key policy should be more restricted, see https://docs.aws.amazon.com/kms/latest/developerguide/grant-manage.html#grant-authorization

[marko-fabry]

Hey @v-rosa , sorry for late response.

I tried adding the policy statement you provided without the statement that allowed the kms:Grant on the role, but the runners don't spin up. I see errors in CloudTrail like this:

...
    "eventSource": "kms.amazonaws.com",
    "eventName": "CreateGrant",
    "awsRegion": "us-east-1",
    "sourceIPAddress": "ec2-frontend-api.amazonaws.com",
    "userAgent": "ec2-frontend-api.amazonaws.com",
    "errorCode": "AccessDenied",
    "errorMessage": "User: arn:aws:sts::123456789:assumed-role/scale-up-lambda-role/scale-up is not authorized to perform: kms:CreateGrant on resource: arn:aws:kms:us-east-1:123456789:key/abcdef123456789 because no identity-based policy allows the kms:CreateGrant action",
...

So I guess the allow on the lambda role is actually necessary.

[marko-fabry]

@v-rosa I added a PR - #3049 :)

[v-rosa]

Seems that a new hot fix was released, closing this issue.