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

Applying permissions boundary to aws-cdk globally #3242

Closed
1 of 5 tasks
robertd opened this issue Jul 8, 2019 · 21 comments · Fixed by #12777
Closed
1 of 5 tasks

Applying permissions boundary to aws-cdk globally #3242

robertd opened this issue Jul 8, 2019 · 21 comments · Fixed by #12777
Assignees
Labels
@aws-cdk/aws-iam Related to AWS Identity and Access Management effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p1

Comments

@robertd
Copy link
Contributor

robertd commented Jul 8, 2019

Note: for support questions, please first reference our documentation, then use Stackoverflow. This repository's issues are intended for feature requests and bug reports.

  • I'm submitting a ...

    • 🪲 bug report
    • 🚀 feature request
    • 📚 construct library gap
    • ☎️ security issue or vulnerability => Please see policy
    • ❓ support request => Please see note at the top of this template.
  • What is the current behavior?
    It’s not possible to set permissions boundary globally for cdk, or anything similar along those lines

  • What is the expected behavior (or behavior of feature suggested)?

My Gitlab CI/CD pipeline runner uses an IAM role that has permissions boundary set (only what our devops team is willing to let us do on our own). However, cdk tries to automagically create or alter roles/permissions/security groups/etc between resources that don’t fall under rules set in our permissions boundary policy (i.e. any customer role must have /custom/ path, etc) . We end up with bunch of errors (cdk trying to create something that permissions boundary on IAM role for the gitlab runner doesn’t allow). What I’m asking is… it would be great if there a way to tell cdk to somehow use permissions boundary policy whenever it tries to automagically create relationships (IAM roles, policies, etc) between the resources.

  • What is the motivation / use case for changing the behavior or adding this feature?

We would be able to limit cdk based on pre-created permission boundaries created by our ops team.

  • Please tell us about your environment:

    • CDK CLI Version: 0.37.0
    • Module Version: 0.37.0
    • OS: Mac
    • Language: TypeScript
  • Other information (e.g. detailed explanation, stacktraces, related issues, suggestions how to fix, links for us to have context, eg. associated pull-request, stackoverflow, gitter, etc)

Conversation w/ @eladb on Gitter.

image

@robertd robertd added the needs-triage This issue or PR still needs to be triaged. label Jul 8, 2019
@matthewtapper
Copy link

We use aspects to achieve this.

import cdk = require('@aws-cdk/core');
import iam = require('@aws-cdk/aws-iam');

export class PermissionsBoundary implements cdk.IAspect {
  private readonly permissionsBoundaryArn: string;

  constructor(permissionBoundaryArn: string) {
    this.permissionsBoundaryArn = permissionBoundaryArn;
  }

  public visit(node: cdk.IConstruct): void {
    if (node instanceof iam.Role) {
        const roleResource = node.node.findChild('Resource') as iam.CfnRole;
        roleResource.addPropertyOverride('PermissionsBoundary', this.permissionsBoundaryArn);
    }
  }
}

Apply as follows:

import { PermissionsBoundary } from '@cultureamp/cdk-common'

const service = new ServiceStack(app, `${projectName}`)

service.node.apply(new PermissionsBoundary('arn:aws:iam::${cdk.Aws.accountId}:policy/hello-world-service-permission-boundary'))

@robertd
Copy link
Contributor Author

robertd commented Jul 9, 2019

Thanks @matthewtapper. Few minor changes in 0.39.0:

service.node.applyAspect(new PermissionsBoundary('arn:aws:iam::${cdk.Aws.ACCOUNT_ID}:policy/hello-world-service-permission-boundary'))

Still, it would be nice for this to be done on the aws-cdk level. :)

P.S.

Also, just putting this here for other folks... cdk.Aws.ACCOUNT_ID doesn't work if you're running cdk w/ aws profile i.e. cdk deploy --profile profile_name.

@mattaschmann
Copy link

+1 for this, the IAM Roles that I create need to follow a permission boundary.

@robertd
Copy link
Contributor Author

robertd commented Jul 9, 2019

@mattaschmann I already have a pending PR to attach permissions boundary to an IAM role created through cdk #2919. Hopefully we’ll see it merged in soon.

@ozbillwang
Copy link

ozbillwang commented Jul 24, 2019

Thanks @robertd to raise the PR. I faced the same issue to add permission boundary policy with aws --profile . Did your PR fix the --profile issue to get proper aws account id?

Second, with your PR #2919 I found you add Inline policy directly (hard coded)

But in our cases, the permission boundary policy denies us to add any inline policies. Only managed policy allowed.

Third, we need apply another managed policy deny_common to any new roles. How should I implement it with this situation?

@aa2858
Copy link

aa2858 commented Aug 12, 2019

+1 for this issue. I work with security team that have created permission boundary for us to work within and providing this capability will be really nice.

👍

@pepastach
Copy link

pepastach commented Aug 20, 2019

Here's a solution in Python for CDK 1.4.0 inspired by @matthewtapper's code

Needless to say it's very ugly, since python CDK does not provide construct objects in aspects. We have to dig deep into JSII to resolve the objects. Hope it helps someone.

from jsii._reference_map import _refs
from jsii._utils import Singleton
import jsii

@jsii.implements(core.IAspect)
class PermissionBoundaryAspect:

    def __init__(self, permission_boundary: Union[aws_iam.ManagedPolicy, str]) -> None:
        """
        :param permission_boundary: Either aws_iam.ManagedPolicy object or managed policy's ARN as string
        """
        self.permission_boundary = permission_boundary

    def visit(self, construct_ref: core.IConstruct) -> None:
        """
        construct_ref only contains a string reference to an object. To get the actual object, we need to resolve it using JSII mapping.
        :param construct_ref: ObjRef object with string reference to the actual object.
        :return: None
        """
        kernel = Singleton._instances[jsii._kernel.Kernel]
        resolve = _refs.resolve(kernel, construct_ref)

        def _walk(obj):
            if isinstance(obj, aws_iam.Role):
                cfn_role = obj.node.find_child('Resource')
                policy_arn = self.permission_boundary if isinstance(self.permission_boundary, str) else self.permission_boundary.managed_policy_arn
                cfn_role.add_property_override('PermissionsBoundary', policy_arn)
            else:
                if hasattr(obj, 'permissions_node'):
                    for c in obj.permissions_node.children:
                        _walk(c)
                if obj.node.children:
                    for c in obj.node.children:
                        _walk(c)

        _walk(resolve)

Usage:

stack.node.apply_aspect(PermissionBoundaryAspect(managed_policy_arn))

@rix0rrr rix0rrr added feature-request A feature should be added or improved. and removed needs-triage This issue or PR still needs to be triaged. labels Aug 28, 2019
@rhboyd
Copy link
Contributor

rhboyd commented Sep 12, 2019

Huge shoutout for @pepastach for this. I was pulling my hair out trying to get Aspects to work in Python.

@tarunaroraonline
Copy link

Great stuff! So that gets me past applying permissions boundary. The permissions boundary I am applying requires that the custom roles and policies have the prefix ‘cust-‘. Is there a way using cdk to specify a prefix for the custom roles and policies being auto generated by cdk?

@rhboyd
Copy link
Contributor

rhboyd commented Oct 3, 2019

@tarunaroraonline yeah, use the Aspect and apply it to the Stack, it will apply whatever custom logic you want to each element.

@tarunaroraonline
Copy link

I am a python newbee ... The code seems to fail with a generic exception ... I am using Python 3.6.5 32-bit and cdk 1.6.1

Exception has occurred: NameError
name 'Union' is not defined
  File "C:\Users\xxx\source\repos\xxx\infra.app.xxx\infra\custom.py", line 13, in PermissionBoundaryAspect
    def __init__(self, permission_boundary: Union[aws_iam.ManagedPolicy, str]) -> None:
  File "C:\Users\xxx\source\repos\xxx\infra.app.xxx\infra\custom.py", line 10, in <module>
    @jsii.implements(core.IAspect)

The custom.py class ...

from aws_cdk import (
    core,
    aws_iam    
)

from jsii._reference_map import _refs
from jsii._utils import Singleton
import jsii

@jsii.implements(core.IAspect)
class PermissionBoundaryAspect:

    def __init__(self, permission_boundary: Union[aws_iam.ManagedPolicy, str]) -> None:
        """
        :param permission_boundary: Either aws_iam.ManagedPolicy object or managed policy's ARN as string
        """
        self.permission_boundary = permission_boundary

    def visit(self, construct_ref: core.IConstruct) -> None:
        """
        construct_ref only contains a string reference to an object. To get the actual object, we need to resolve it using JSII mapping.
        :param construct_ref: ObjRef object with string reference to the actual object.
        :return: None   
        """
        kernel = Singleton._instances[jsii._kernel.Kernel]
        resolve = _refs.resolve(kernel, construct_ref)

        def _walk(obj):
            if isinstance(obj, aws_iam.Role):
                cfn_role = obj.node.find_child('Resource')
                policy_arn = self.permission_boundary if isinstance(self.permission_boundary, str) else self.permission_boundary.managed_policy_arn
                cfn_role.add_property_override('PermissionsBoundary', policy_arn)
            else:
                if hasattr(obj, 'permissions_node'):
                    for c in obj.permissions_node.children:
                        _walk(c)
                if obj.node.children:
                    for c in obj.node.children:
                        _walk(c)

        _walk(resolve)

Any idea what I am missing?

@ghost
Copy link

ghost commented Oct 4, 2019

@tarunaroraonline You're trying to use Union but never imported it. It's in jsii which you did import, so just use jsii.Union in that line.

@jacques-
Copy link

I had to make small tweak to @pepastach 's version to make it work (had to add a check because not all obj have node attr), here is code:

@jsii.implements(core.IAspect)
class PermissionBoundaryAspect:

    def __init__(self, permission_boundary: Union[aws_iam.ManagedPolicy, str]) -> None:
        """
        :param permission_boundary: Either aws_iam.ManagedPolicy object or managed policy's ARN as string
        """
        self.permission_boundary = permission_boundary

    def visit(self, construct_ref: core.IConstruct) -> None:
        """
        construct_ref only contains a string reference to an object. To get the actual object, we need to resolve it using JSII mapping.
        :param construct_ref: ObjRef object with string reference to the actual object.
        :return: None
        """
        kernel = Singleton._instances[jsii._kernel.Kernel]
        resolve = _refs.resolve(kernel, construct_ref)

        def _walk(obj):
            if isinstance(obj, aws_iam.Role):
                cfn_role = obj.node.find_child('Resource')
                policy_arn = self.permission_boundary if isinstance(self.permission_boundary, str) else self.permission_boundary.managed_policy_arn
                cfn_role.add_property_override('PermissionsBoundary', policy_arn)
            else:
                if hasattr(obj, 'permissions_node'):
                    for c in obj.permissions_node.children:
                        _walk(c)
                if hasattr(obj, 'node') and obj.node.children:
                    for c in obj.node.children:
                        _walk(c)

        _walk(resolve)

@tarunaroraonline
Copy link

@pepastach @jacques- @matthewtapper this seems to be broken in aws-cdk 1.16.x with the introduction of changes to jsii 0.20.0... Anyone else got it to work with the changes? aws/jsii#980

@pepastach
Copy link

pepastach commented Nov 14, 2019

Hi everybody, here's a fix that should work with CDK based on JSII pre-0.20 as well as the newest ones.

https://gitlab.com/josef.stach/aws-cdk-permission-boundary-aspect

from typing import Union

import jsii
from aws_cdk import aws_iam, core
from jsii._reference_map import _refs
from jsii._utils import Singleton


@jsii.implements(core.IAspect)
class PermissionBoundaryAspect:
    """
    This aspect finds all aws_iam.Role objects in a node (ie. CDK stack) and sets permission boundary to the given ARN.
    """

    def __init__(self, permission_boundary: Union[aws_iam.ManagedPolicy, str]) -> None:
        """
        :param permission_boundary: Either aws_iam.ManagedPolicy object or managed policy's ARN string
        """
        self.permission_boundary = permission_boundary

    def visit(self, construct_ref: core.IConstruct) -> None:
        """
        construct_ref only contains a string reference to an object. To get the actual object, we need to resolve it using JSII mapping.
        :param construct_ref: ObjRef object with string reference to the actual object.
        :return: None
        """
        if isinstance(construct_ref, jsii._kernel.ObjRef) and hasattr(construct_ref, 'ref'):
            kernel = Singleton._instances[jsii._kernel.Kernel]  # The same object is available as: jsii.kernel
            resolve = _refs.resolve(kernel, construct_ref)
        else:
            resolve = construct_ref

        def _walk(obj):
            if isinstance(obj, aws_iam.Role):
                cfn_role = obj.node.find_child('Resource')
                policy_arn = self.permission_boundary if isinstance(self.permission_boundary, str) else self.permission_boundary.managed_policy_arn
                cfn_role.add_property_override('PermissionsBoundary', policy_arn)
            else:
                if hasattr(obj, 'permissions_node'):
                    for c in obj.permissions_node.children:
                        _walk(c)
                if hasattr(obj, 'node') and obj.node.children:
                    for c in obj.node.children:
                        _walk(c)

        _walk(resolve)

@ottokruse
Copy link

We used the aspect solution to implement 'global' permissionBoundary as outlined by @matthewtapper above (THANKS) but did find the following code more reliable, as instanceof iam.Role doesn't always return true for IAM roles. (We're still puzzled why, but have observed this issue also when checking for instanceof ec2.SecurityGroup in another context).

import cdk = require('@aws-cdk/core');

export class PermissionsBoundary implements cdk.IAspect {
    private readonly permissionsBoundaryArn: string;

    constructor(permissionBoundaryArn: string) {
        this.permissionsBoundaryArn = permissionBoundaryArn;
    }

    public visit(node: cdk.IConstruct): void {
        if (cdk.CfnResource.isCfnResource(node) && node.cfnResourceType === 'AWS::IAM::Role') {
            node.addPropertyOverride('PermissionsBoundary', this.permissionsBoundaryArn);
        }
    }
}

@rix0rrr rix0rrr added @aws-cdk/aws-iam Related to AWS Identity and Access Management effort/medium Medium work item – several days of effort labels Jan 23, 2020
@Marakai
Copy link

Marakai commented Apr 23, 2020

Hi everybody, here's a fix that should work with CDK based on JSII pre-0.20 as well as the newest ones.

https://gitlab.com/josef.stach/aws-cdk-permission-boundary-aspect

...snip...

Works like a charm, many thanks! Still would be nice to have the functionality cleanly embedded in CDK.

@rix0rrr rix0rrr added the p1 label Aug 12, 2020
ajkerrigan pushed a commit to mdsol/document-understanding-solution that referenced this issue Aug 12, 2020
Apply an aspect to each CDK template (backend and client) which adds a
permission boundary to any IAM role. This makes it easier to
delegate deploy permissions without requiring unbounded access
to manage IAM roles.

Reference:

aws/aws-cdk#3242
@jacques-
Copy link

I've been using the aspect in the code snippets above in all our code without trouble till one of our stacks (with dozens of nested stacks) grew a bit too complicated. Now I'm hitting the following during the _walk:

RangeError: Maximum call stack size exceeded
    at KernelHost.processRequest (/Users/xx/.venv/lib/python3.7/site-packages/jsii/_embedded/jsii/jsii-runtime.js:7495:18)
    at KernelHost.completeCallback (/Users/xx/.venv/lib/python3.7/site-packages/jsii/_embedded/jsii/jsii-runtime.js:7428:25)
    at KernelHost.processRequest (/Users/xx/.venv/lib/python3.7/site-packages/jsii/_embedded/jsii/jsii-runtime.js:7498:16)
    at KernelHost.completeCallback (/Users/xx/.venv/lib/python3.7/site-packages/jsii/_embedded/jsii/jsii-runtime.js:7428:25)
    at KernelHost.processRequest (/Users/xx/.venv/lib/python3.7/site-packages/jsii/_embedded/jsii/jsii-runtime.js:7498:16)
    at KernelHost.completeCallback (/Users/xx/.venv/lib/python3.7/site-packages/jsii/_embedded/jsii/jsii-runtime.js:7428:25)
    at KernelHost.processRequest (/Users/xx/.venv/lib/python3.7/site-packages/jsii/_embedded/jsii/jsii-runtime.js:7498:16)
    at KernelHost.completeCallback (/Users/xx/.venv/lib/python3.7/site-packages/jsii/_embedded/jsii/jsii-runtime.js:7428:25)
    at KernelHost.processRequest (/Users/xx/.venv/lib/python3.7/site-packages/jsii/_embedded/jsii/jsii-runtime.js:7498:16)
    at Kernel._wrapSandboxCode (/Users/xx/.venv/lib/python3.7/site-packages/jsii/_embedded/jsii/jsii-runtime.js:8408:19)
    at ret._ensureSync (/Users/xx/.venv/lib/python3.7/site-packages/jsii/_embedded/jsii/jsii-runtime.js:7756:25)
    at Kernel._ensureSync (/Users/xx/.venv/lib/python3.7/site-packages/jsii/_embedded/jsii/jsii-runtime.js:8381:20)
    at Kernel.invoke (/Users/xx/.venv/lib/python3.7/site-packages/jsii/_embedded/jsii/jsii-runtime.js:7755:26)
    at KernelHost.processRequest (/Users/xx/.venv/lib/python3.7/site-packages/jsii/_embedded/jsii/jsii-runtime.js:7456:28)
    at KernelHost.run (/Users/xx/.venv/lib/python3.7/site-packages/jsii/_embedded/jsii/jsii-runtime.js:7394:14)
    at Immediate.setImmediate [as _onImmediate] (/Users/xx/.venv/lib/python3.7/site-packages/jsii/_embedded/jsii/jsii-runtime.js:7397:37)
    at runCallback (timers.js:705:18)
    at tryOnImmediate (timers.js:676:5)
    at processImmediate (timers.js:658:5)

Unfortunately it is hard to reproduce without sharing lots of internal code.

I don't think this is a circular recursion because I can make the stack simpler by reducing the number of similar nested stacks slightly, and then it still works.

I'd be grateful for any hints or suggestions on where to look next.

rix0rrr added a commit that referenced this issue Jan 29, 2021
Allow configuring Permissions Boundaries for an entire subtree using
Aspects, add a sample policy which can be used to reduce future
misconfiguration risk for untrusted CodeBuild projects as an example.

Fixes #3242.
@mergify mergify bot closed this as completed in #12777 Feb 1, 2021
mergify bot pushed a commit that referenced this issue Feb 1, 2021
Allow configuring Permissions Boundaries for an entire subtree using
Aspects, add a sample policy which can be used to reduce future
misconfiguration risk for untrusted CodeBuild projects as an example.

Addresses one part of aws/aws-cdk-rfcs#5.

Fixes #3242.

ALSO IN THIS COMMIT:

Fix a bug in the `assert` library, where `haveResource()` would *never* match
any resource that didn't have a `Properties` block (even if we tested for no property
in particular, or the absence of properties). This fix caused two ECS tests to fail,
which were asserting the wrong thing anyway (both were asserting `notTo(haveResource(...))`
where they actually meant to assert `to(haveResource())`.

----

*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

github-actions bot commented Feb 1, 2021

⚠️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.

NovakGu pushed a commit to NovakGu/aws-cdk that referenced this issue Feb 18, 2021
Allow configuring Permissions Boundaries for an entire subtree using
Aspects, add a sample policy which can be used to reduce future
misconfiguration risk for untrusted CodeBuild projects as an example.

Addresses one part of aws/aws-cdk-rfcs#5.

Fixes aws#3242.

ALSO IN THIS COMMIT:

Fix a bug in the `assert` library, where `haveResource()` would *never* match
any resource that didn't have a `Properties` block (even if we tested for no property
in particular, or the absence of properties). This fix caused two ECS tests to fail,
which were asserting the wrong thing anyway (both were asserting `notTo(haveResource(...))`
where they actually meant to assert `to(haveResource())`.

----

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

wmarcuse commented Apr 14, 2021

For CDK version 1.9.0 + I needed to adjust the Python code from @pepastach to get it to work, first added and extra try_find_child() on the node and also the stack.node.apply_aspect() method is deprecated by AWS.

from aws_cdk import (
    aws_iam as iam,
    core,
)
import jsii
from jsii._reference_map import _refs
from jsii._utils import Singleton

from typing import Union


@jsii.implements(core.IAspect)
class PermissionBoundaryAspect:
    """
    This aspect finds all aws_iam.Role objects in a node (ie. CDK stack) and sets
    permission boundary to the given ARN.
    """

    def __init__(self, permission_boundary: Union[iam.ManagedPolicy, str]) -> None:
        """
        This initialization method sets the permission boundary attribute.

        :param permission_boundary: The provided permission boundary
        :type permission_boundary: iam.ManagedPolicy|str
        """
        self.permission_boundary = permission_boundary
        print(self.permission_boundary)

    def visit(self, construct_ref: core.IConstruct) -> None:
        """
        construct_ref only contains a string reference to an object.
        To get the actual object, we need to resolve it using JSII mapping.
        :param construct_ref: ObjRef object with string reference to the actual object.
        :return: None
        """
        if isinstance(construct_ref, jsii._kernel.ObjRef) and hasattr(
            construct_ref, "ref"
        ):
            kernel = Singleton._instances[
                jsii._kernel.Kernel
            ]  # The same object is available as: jsii.kernel
            resolve = _refs.resolve(kernel, construct_ref)
        else:
            resolve = construct_ref

        def _walk(obj):
            if obj.node.try_find_child("Resource") is not None:
                if isinstance(obj, iam.Role):
                    cfn_role = obj.node.find_child("Resource")
                    policy_arn = (
                        self.permission_boundary
                        if isinstance(self.permission_boundary, str)
                        else self.permission_boundary.managed_policy_arn
                    )
                    cfn_role.add_property_override("PermissionsBoundary", policy_arn)
            else:
                if hasattr(obj, "permissions_node"):
                    for c in obj.permissions_node.children:
                        _walk(c)
                if hasattr(obj, "node") and obj.node.children:
                    for c in obj.node.children:
                        _walk(c)

        _walk(resolve)

And the new implementation API for the stack is:

    core.Aspects.of(stack).add(
        PermissionBoundaryAspect(
            f"arn:aws:iam::{target_environment.account}:policy/my-permissions-boundary"
        )
    )

@robertd
Copy link
Contributor Author

robertd commented Sep 15, 2023

https://aws.amazon.com/blogs/devops/secure-cdk-deployments-with-iam-permission-boundaries/

In cdk.json file:

{
  "context": {
     "@aws-cdk/core:permissionsBoundary": {
       "name": "developer-policy"
     }
  }
}

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 effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.