-
Notifications
You must be signed in to change notification settings - Fork 846
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
Add dynamic config to control allowed Host headers #6750
Conversation
2a54283
to
f09b0fa
Compare
9b985f0
to
c169e01
Compare
common/dynamicconfig/constants.go
Outdated
|
||
FrontendHTTPAllowedHosts = NewGlobalTypedSetting( | ||
"frontend.httpAllowedHosts", | ||
[]string{"*"}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can't use a non-nil reference for the default value because of how the default converter works (it does a shallow copy and then asks mapstructure to fill it in). this has to be []string(nil)
and then handle the nil in application code (or in your second level converter)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you elaborate why this is a problem? I'm not sure I'm following. Are you saying that mapstructure
will append to this default?
Seems like a usability issue. You can't expect to be the gatekeeper for all DC changing PRs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know :( It should be commented somewhere but I'm not sure where, there's no obvious place for the "api" for defining settings.
I was planning to add a linter or a runtime check using reflection at static init time.
9a30eb6
to
f5bf8c3
Compare
What changed?
Add a dynamic config to restrict which HTTP
Host
header values are allowed in HTTP API requests.Why?
Prevent DNS rebinding attacks.
How did you test it?
Added a functional test.