ec2-plugin icon indicating copy to clipboard operation
ec2-plugin copied to clipboard

[Documentation][Security] Enforce IMDSv2 by default & update groovy section to use up-to-date constructor

Open jdelnano opened this issue 2 years ago • 3 comments

Background

When recently setting up the EC2 plugin, I noticed the Groovy script detailed in the README used a deprecated SlaveTemplate constructor. The current link in the comment above the SlaveTemplate object instantiation points to an older version of the EC2 Plugin and doesn't include the ability to specify various additional settings such as metadataEndpointEnabled, metadataTokensRequired , metadataHopsLimit , hostKeyVerificationStrategy, etc.

What does this PR do?

  • This PR updates the Groovy script section of the README, bringing it up-to-date. The section now reflects the setting of additional properties for SlaveTemplate objects and updates the associated comment link to the SlaveTemplate's javadoc webpage.

  • Additionally, I've updated the metadataTokensRequired field to be true by default, enforcing the usage of IMDSv2 over IMDSv1 out-of-the-box when configuring an AMI. Explained here on Scott Piper's (an industry-leading AWS security expert) "wall of shame" repository, IMDSv2 should be used exclusively (over IMDSv1) as an AWS security best practice. IMDSv2 was released in response to the 2019 Capital One security breach, so I believe it is imperative that:

    • the field to enforce IMDSv2 usage be turned on by default
    • the README be updated to allow users to configure the plugin via groovy

Both of the above bullets are accomplished in this particular PR.

I've ensured that these changes run without issues, producing the updated Jenkins agent configuration in a configured cloud. Please see the short video below for a visual confirmation of the settings defined in the README.

https://user-images.githubusercontent.com/18095335/176363013-310e6ce1-42da-4bbb-8755-9aee2fe8fe43.mov

  • [x] Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • [x] Ensure that the pull request title represents the desired changelog entry
  • [x] Please describe what you did
  • [x] Link to relevant issues in GitHub or Jira
  • [x] Link to relevant pull requests, esp. upstream and downstream changes
  • [x] Ensure you have provided tests - that demonstrates feature works or fixes the issue

jdelnano avatar Jun 29 '22 06:06 jdelnano

@res0nance I apologize for the explicit tag, but I want to put this on your radar as I believe it is a significant security improvement (although the change itself is small).

jdelnano avatar Aug 01 '22 14:08 jdelnano

@res0nance I apologize for the explicit tag, but I want to put this on your radar as I believe it is a significant security improvement (although the change itself is small).

Hey, appreciate the PR, in theory this looks ok, I'll probably bundle this with other breaking changes. Since IMDSv2 is potentially breaking for some folks.

res0nance avatar Aug 01 '22 15:08 res0nance

Sounds good! Thanks for the speedy response.

jdelnano avatar Aug 01 '22 16:08 jdelnano