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

(ec2): BastionHostLinux should use AmazonSSMManagedInstanceCore policy #30834

Closed
2 tasks done
clueleaf opened this issue Jul 12, 2024 · 7 comments
Closed
2 tasks done
Labels
@aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p2

Comments

@clueleaf
Copy link
Contributor

Describe the feature

BastionHostLinux currently has the following instance role.

this.instance.addToRolePolicy(new PolicyStatement({
actions: [
'ssmmessages:*',
'ssm:UpdateInstanceInformation',
'ec2messages:*',
],
resources: ['*'],
}));

This has insufficient permissions to be managed by SSM. AmazonSSMManagedInstanceCore should be used instead. (Especially ssm permissions)

Use Case

Manage instance by SSM.

Proposed Solution

Use AmazonSSMManagedInstanceCore policy instead of inline policy.

Other Information

According to my research, The code above first appeared in #3697
It seems like the instance role was not discussed then.

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change

CDK version used

2.144.0

Environment details (OS name and version, etc.)

macOS

@clueleaf clueleaf added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Jul 12, 2024
@github-actions github-actions bot added the @aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud label Jul 12, 2024
@clueleaf clueleaf changed the title (ec2): BastionHostLinux should use AmazonSSMManagedInstanceCore role (ec2): BastionHostLinux should use AmazonSSMManagedInstanceCore policy Jul 12, 2024
@hoegertn
Copy link
Contributor

Hi, can you elaborate on the missing permissions? I explicitly did not use it because of the broad permissions regarding parameter store etc.

But we can change that if it is deemed safe.

@clueleaf
Copy link
Contributor Author

@hoegertn
Thank you for your response.
One thing I noticed is when I tried to scan the bastion host using Inspector, the State Manager association created automatically failed because of the lack of permission, which resulted in scan failure.
Screenshot 2024-07-12 at 21 26 01
Screenshot 2024-07-12 at 21 23 48

The execution history shows ssm:GetManifest was missing, which is included in AmazonSSMManagedInstanceCore. I'm not sure about other permissions, but I have a feeling that it would be safe to use AmazonSSMManagedInstanceCore so that bastion instances can be fully managed by SSM.

@pahud
Copy link
Contributor

pahud commented Jul 12, 2024

@clueleaf

Makes sense to me. Feel free to submit a PR for that if you like. Before that, I am afraid we'll need to manually attach that managed policy to the instance role like

    const bast = new ec2.BastionHostLinux(this, 'Bast', {
      vpc,
    });

    bast.role.addManagedPolicy(iam.ManagedPolicy.fromAwsManagedPolicyName('AmazonSSMManagedInstanceCore'));

@pahud pahud added p2 effort/small Small work item – less than a day of effort and removed needs-triage This issue or PR still needs to be triaged. labels Jul 12, 2024
@hoegertn
Copy link
Contributor

Please consider adding the missing statements instead of adding the ManagedPolicy. The ManagedPolicy might be a breaking change for users as some companies do not allow the use of managed policies as they ofter are to wide.

@clueleaf
Copy link
Contributor Author

The ManagedPolicy might be a breaking change for users as some companies do not allow the use of managed policies as they ofter are to wide.

Good point. Since we can work around by adding policies like @pahud shows, it is safe to keep the current state. Closing this issue.

@clueleaf clueleaf closed this as not planned Won't fix, can't repro, duplicate, stale Jul 13, 2024
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.

@aws-cdk-automation
Copy link
Collaborator

Comments on closed issues and PRs are hard for our team to see. If you need help, please open a new issue that references this one.

@aws aws locked as resolved and limited conversation to collaborators Jul 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
@aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p2
Projects
None yet
Development

No branches or pull requests

4 participants