Skip to content

Add a -bind-addr flag keeping default of 127.0.0.1#29

Open
mjrlee wants to merge 2 commits intoaws:mainfrom
mjrlee:main
Open

Add a -bind-addr flag keeping default of 127.0.0.1#29
mjrlee wants to merge 2 commits intoaws:mainfrom
mjrlee:main

Conversation

@mjrlee
Copy link
Copy Markdown

@mjrlee mjrlee commented Mar 3, 2023

This change introduces a -bind-addr flag that is useful when serving on another address. The flag is optional and the default behaviour has not been changed.

I have also hard coded in an allowlist of possible addresses to ensure that this is not accidentally set to a public IP. The addresses are localhost and 169.254.169.254.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@dwmw2
Copy link
Copy Markdown
Contributor

dwmw2 commented Jun 12, 2023

Hm, surely the default should be ::1. It's well over a quarter of a century since RFC1883 was published...

@dwmw2
Copy link
Copy Markdown
Contributor

dwmw2 commented Jun 12, 2023

We also prefer to call it an 'allowlist' these days, FWIW. Although I note that's only in the commit comment not the code itself.

@mjrlee
Copy link
Copy Markdown
Author

mjrlee commented Jun 15, 2023

Hm, surely the default should be ::1. It's well over a quarter of a century since RFC1883 was published...

Perhaps, but I think that's a different commit/PR - this keeps the default that's currently in main.

@dwmw2
Copy link
Copy Markdown
Contributor

dwmw2 commented Jun 19, 2023

It should at least be in the allowlist though?

@philipmw
Copy link
Copy Markdown

philipmw commented Dec 1, 2025

(I am not a maintainer of this package; just a customer.)

I have also hard coded in an allowlist of possible addresses to ensure that this is not accidentally set to a public IP. The addresses are localhost and 169.254.169.254.

IMO we should not do this. It adds complexity to the logic and unnecessarily restricts some use-cases. Already in this thread there's an argument about what should be in the allowlist. If I want to expose this service to my Kubernetes cluster, I think I need it to bind to a real network interface or to 0.0.0.0. With your PR as it is, I wouldn't be able to do this.

Could we just trust the operator to specify whatever address they need?

@camerondugan
Copy link
Copy Markdown
Contributor

Our main concern is to prevent unwanted exfiltration of credentials. If we were to allow a --bind-addr flag, we would need to ensure a ttl of 1 is set for outbound responses to mimic behavior of IMDSv2.

For the kubernetes cluster use case, this would likely mean without modifying the default ttl, you won't be able to have a singular pod serve credentials for the whole cluster. Running the credential helper as a sidecar within a pod would still work however.

@camerondugan
Copy link
Copy Markdown
Contributor

Another member of the team mentioned: for ::1, we should look into honoring AWS_USE_DUALSTACK_ENDPOINT and build the hostname as rolesanywhere.${region}.api.aws.

I would also add, that there might be some code generated go construct specifically for building endpoints for roles anywhere if we aren't already using it.

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.

4 participants