aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.58k stars 3.88k forks source link

aws_ec2: Support making BastionHost default role policy less permissive (no wildcards in Allow: Action: *) #20554

Open benjaminsoellner opened 2 years ago

benjaminsoellner commented 2 years ago

Describe the feature

The current deployment of a aws_ec2.BastionHost(...) deploys a CloudFormation template with an overly permissive default policy:

"bastionInstanceRoleDefaultPolicyF19D0FC0": {
   "Type": "AWS::IAM::Policy",
   "Properties": {
    "PolicyDocument": {
     "Statement": [
      {
       "Action": [
        "cloudformation:DescribeStackResource",
        "cloudformation:SignalResource"
       ],
       "Effect": "Allow",
       "Resource": {
        "Ref": "AWS::StackId"
       }
      },
      {
       "Action": [
        "ssmmessages:*",
        "ssm:UpdateInstanceInformation",
        "ec2messages:*"
       ],
       "Effect": "Allow",
       "Resource": "*"
      }
     ],
     "Version": "2012-10-17"
    },
    "PolicyName": "bastionInstanceRoleDefaultPolicyF19D0FC0",
    "Roles": [
     {
      "Ref": "bastionInstanceRole961823F4"
     }
    ]
   },
   "Metadata": ....
  },

The problem is when executing cfn_nag checks on the CloudFormation template:

cfn-nag check:
| FAIL F4
|
| Resource: ["bastionInstanceRoleDefaultPolicyF19D0FC0"]
| Line Numbers: [1604]
|
| IAM policy should not allow * action

Use Case

Cloud Formation templates shouldn't allow an "Resource": "*" policy.

Proposed Solution

The default policy should be changed to a less permissive policy, at least setting a custom role / policy like used for the aws_ec2.Instance construct should be allowed.

Other Information

No response

Acknowledgements

CDK version used

2.24.0

Environment details (OS name and version, etc.)

Ubuntu 18.4 / NodeJS 16.15.0 / Python 3.8.13

corymhall commented 2 years ago

It looks like these permissions may have been added to allow for users to connect to the bastion host via SSM. This should be something that is configurable though and these permissions probably need to be audited to make sure that they are least privilege.

We could potentially add a new property configureSsmAccess which defaults to true and sets up these permissions. To add more restrictive permissions we would probably need to introduce a feature flag.

benjaminsoellner commented 2 years ago

Seems I found a workaround patching the CloudFormation output to modify the policy document to conform to cfn_nag (see https://docs.aws.amazon.com/cdk/v2/guide/cfn_layer.html#cfn_layer_resource).

It's actually not the wildcard in the Resource but in Action (-> hence, title of issue changed). If instead of allowing blanket ssmmessages:* and ec2messages:* you instead would configure the specific actions required you should be ok (here is a doc which those should be: https://docs.aws.amazon.com/systems-manager/latest/userguide/systems-manager-setting-up-messageAPIs.html

In python in order to patch the Cfn output as long as this isn't implemented I would do (in Python):

        generated_policy = [child for child in bastion_host.role.node.children if type(child) is iam.Policy][0]
        generated_policy_cfn = generated_policy.node.default_child
        generated_policy_cfn.add_property_override("PolicyDocument.Statement.1.Action", [
            # as originally present
            "ssm:UpdateInstanceInformation",
            # see https://docs.aws.amazon.com/service-authorization/latest/reference/list_amazonsessionmanagermessagegatewayservice.html
            "ssmmessages:CreateControlChannel",
            "ssmmessages:CreateDataChannel",
            "ssmmessages:OpenControlChannel",
            "ssmmessages:OpenDataChannel",
            # see https://docs.aws.amazon.com/service-authorization/latest/reference/list_amazonmessagedeliveryservice.html
            "ec2messages:AcknowledgeMessage",
            "ec2messages:DeleteMessage",
            "ec2messages:FailMessage",
            "ec2messages:GetEndpoint",
            "ec2messages:GetMessages",
            "ec2messages:SendReply"
        ])

EDIT: of course technically even though this flies under the radar of cfn_nag this is equivalent to using a wildcard, so potentially the permissions should still be narrowed down so that they are least-privilege.

watany-dev commented 1 year ago

The official documentation mentions least privilege which solves exactly this. https://docs.aws.amazon.com/systems-manager/latest/userguide/getting-started-create-iam-instance-profile.html#create-iam-instance-profile-ssn-only

If the image below is okay, I can fix it.

{
    "Version": "2012-10-17",
    "Statement": [
        {
            "Effect": "Allow",
            "Action": [
                "ssm:UpdateInstanceInformation",
                "ssmmessages:CreateControlChannel",
                "ssmmessages:CreateDataChannel",
                "ssmmessages:OpenControlChannel",
                "ssmmessages:OpenDataChannel"
            ],
            "Resource": "*"
        },
        {
            "Effect": "Allow",
            "Action": [
                "s3:GetEncryptionConfiguration"
            ],
            "Resource": "*"
        }
    ]
}