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

(aws-iam): PermissionsBoundary Not Added to Custom Resource roles #13310

Closed
joel-aws opened this issue Feb 28, 2021 · 2 comments · Fixed by #14754
Closed

(aws-iam): PermissionsBoundary Not Added to Custom Resource roles #13310

joel-aws opened this issue Feb 28, 2021 · 2 comments · Fixed by #14754
Assignees
Labels
@aws-cdk/aws-iam Related to AWS Identity and Access Management @aws-cdk/core Related to core CDK functionality @aws-cdk/custom-resources Related to AWS CDK Custom Resources bug This issue is a bug. effort/medium Medium work item – several days of effort needs-triage This issue or PR still needs to be triaged. p1

Comments

@joel-aws
Copy link
Contributor

joel-aws commented Feb 28, 2021

When applying a Permissions Boundary to a stack, not all roles in the CFn include PermissionsBoundary.

Reproduction Steps

from aws_cdk import core, aws_iam as iam, aws_s3 as s3

app = core.App()

class SupportFilesStack(core.Stack):
    def __init__(self, scope: core.Construct, construct_id: str, **kwargs) -> None:
        super().__init__(scope, construct_id, **kwargs)

        generic_role = iam.Role(
            self, "generic", assumed_by=iam.ServicePrincipal("lambda")
        )

        support_files_bucket = s3.Bucket(
            self,
            "support-files",
            removal_policy=core.RemovalPolicy.DESTROY,
            auto_delete_objects=True,
        )
        support_files_bucket.grant_read(generic_role)

        self.permission_boundary = iam.ManagedPolicy(
            self,
            "pb",
            statements=[
                iam.PolicyStatement(
                    effect=iam.Effect.ALLOW, actions=["*"], resources=["*"]
                )
            ],
        )

stack = SupportFilesStack(app, "SupportFiles")

iam.PermissionsBoundary.of(stack).apply(stack.permission_boundary)

app.synth()

What did you expect to happen?

In the outputted CFn, I expected all roles to have their Permissions Boundary set.

What actually happened?

Two roles were created: one defined and the other automatically created as a result of setting auto_delete_objects=True in the S3 construct. Only the first role had the Permissions Boundary set; the second did not.

Environment

  • CDK CLI Version : 1.91.0

Other

As pointed out by @ottokruse in #3242 (comment), you can work around this by creating an aspect that checks the following conditions:

def visit(self, node: core.IConstruct):
    if (
        core.CfnResource.is_cfn_resource(node)
        and node.cfn_resource_type == "AWS::IAM::Role"
    ):
        pass

This is 🐛 Bug Report

@joel-aws joel-aws added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Feb 28, 2021
@github-actions github-actions bot added the @aws-cdk/aws-iam Related to AWS Identity and Access Management label Feb 28, 2021
@robertd
Copy link
Contributor

robertd commented Feb 28, 2021

@joel-aws I believe it's because the role attached to the custom resource is not actually an iam.Role. A raw CfnResource is used to create the role.

Reference: #12323 (comment)

@rix0rrr rix0rrr changed the title (aws-iam): PermissionsBoundary Not Added to All Roles in a Stack (aws-iam): PermissionsBoundary Not Added to Custom Resource roles Mar 3, 2021
@rix0rrr rix0rrr added @aws-cdk/core Related to core CDK functionality @aws-cdk/custom-resources Related to AWS CDK Custom Resources effort/medium Medium work item – several days of effort p1 labels Mar 3, 2021
@mergify mergify bot closed this as completed in #14754 May 20, 2021
mergify bot pushed a commit that referenced this issue May 20, 2021
…14754)

The role created by `CustomResourceProvider` is a `CfnResource` with a manual type, not a `CfnRole` to avoid a cyclical dependency. But since `PermissionBoundary` assumes all role/user resources in scope are instances of `CfnRole` or `CfnUser`, a permission boundary is not correctly applied to the custom resource's role (or any other role or user created directly through `CfnResource`).

This PR solves the above problem by adding extra conditionals for the `CfnResource` case and adds permission boundaries through the `addPropertyOverride` escape hatch.

fixes #13310

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

hollanddd pushed a commit to hollanddd/aws-cdk that referenced this issue Aug 26, 2021
…ws#14754)

The role created by `CustomResourceProvider` is a `CfnResource` with a manual type, not a `CfnRole` to avoid a cyclical dependency. But since `PermissionBoundary` assumes all role/user resources in scope are instances of `CfnRole` or `CfnUser`, a permission boundary is not correctly applied to the custom resource's role (or any other role or user created directly through `CfnResource`).

This PR solves the above problem by adding extra conditionals for the `CfnResource` case and adds permission boundaries through the `addPropertyOverride` escape hatch.

fixes aws#13310

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-iam Related to AWS Identity and Access Management @aws-cdk/core Related to core CDK functionality @aws-cdk/custom-resources Related to AWS CDK Custom Resources bug This issue is a bug. effort/medium Medium work item – several days of effort needs-triage This issue or PR still needs to be triaged. p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants