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

Enhance IMDS access detection to support cases where IMDSv2 is enforced (closes #8) #9

Merged
merged 3 commits into from
Jul 12, 2023

Conversation

christophetd
Copy link
Collaborator

@christophetd christophetd commented Jul 11, 2023

Before:

Now:

  • We have 2 separate tests, one for IMDSv1 and one for IMDSv2
  • The IMDSv1 test ensures that the response contains something - if not, it's likely that IMDSV2 is enforced and the endpoint returns a 401 error with empty content (so we're "secure")
  • The IMDSv2 test works similarly, but handle timeouts differently due to how IMDS responds when IMDSv2 is enforced and max-response-hop is set to 1 (the TCP connection succeeds, but the subsequent socket read times out)

Sample result on an EKS cluster that has IMDSv2 enforced (but not blocked through a NetworkPolicy or response-max-hop=1)

image

Copy link

@Frichetten Frichetten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow it's been a long time since I've looked at Go 😄 . From what I've deciphered it looks good to me.

I was going to nitpick that I didn't think Curl would hang indefinitely for dropped packets from IMDSv2 but based on the curl documentation it appears there is no default timeout.

@christophetd
Copy link
Collaborator Author

Yep I added the timeout after testing and seeing that it hangs. Thanks for the review!

@christophetd christophetd merged commit 37f9c32 into main Jul 12, 2023
2 checks passed
@christophetd christophetd deleted the enhance-imds-testing branch July 12, 2023 07:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants