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

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

Conversation

jdelnano
Copy link
Contributor

@jdelnano jdelnano commented Jun 29, 2022

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.

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

@jdelnano jdelnano changed the title Update SlaveTemplate groovy section to use up-to-date constructor & specify more settings [Documentation] Update SlaveTemplate groovy section to use up-to-date constructor & specify more settings Jun 29, 2022
@jdelnano jdelnano force-pushed the jdelnano/update-readme-to-improve-groovy-snippet branch from 55fde70 to 8b9c564 Compare August 1, 2022 14:22
@jdelnano jdelnano changed the title [Documentation] Update SlaveTemplate groovy section to use up-to-date constructor & specify more settings [Documentation][Default Settings] Enforce IMDSv2 by default & update groovy section to use up-to-date constructor Aug 1, 2022
@jdelnano jdelnano changed the title [Documentation][Default Settings] Enforce IMDSv2 by default & update groovy section to use up-to-date constructor [Documentation][Security] Enforce IMDSv2 by default & update groovy section to use up-to-date constructor Aug 1, 2022
@jdelnano
Copy link
Contributor Author

jdelnano commented Aug 1, 2022

@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).

@res0nance res0nance added the documentation Documentation update label Aug 1, 2022
@res0nance
Copy link
Contributor

@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.

@jdelnano
Copy link
Contributor Author

jdelnano commented Aug 1, 2022

Sounds good! Thanks for the speedy response.

@res0nance res0nance added the enhancement Feature additions or enhancements label Aug 24, 2022
@res0nance res0nance merged commit 90aca58 into jenkinsci:master Aug 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Documentation update enhancement Feature additions or enhancements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants