-
-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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 trusted whitelist proxy protocol #2234
Add trusted whitelist proxy protocol #2234
Conversation
7ab6ed0
to
264ef16
Compare
middlewares/ip_whitelister.go
Outdated
} | ||
|
||
func (whitelister *IPWhitelister) handle(w http.ResponseWriter, r *http.Request, next http.HandlerFunc) { | ||
remoteIP, err := ipFromRemoteAddr(r.RemoteAddr) | ||
func (whiteLister *IPWhiteLister) handle(w http.ResponseWriter, r *http.Request, next http.HandlerFunc) { |
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.
func(whiteLister *IPWhiteLister)
-> func(wl *IPWhiteLister)
middlewares/ip_whitelister.go
Outdated
reject(w) | ||
} | ||
|
||
func (whiteLister *IPWhiteLister) ServeHTTP(rw http.ResponseWriter, r *http.Request, next http.HandlerFunc) { |
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.
func(whiteLister *IPWhiteLister)
-> func(wl *IPWhiteLister)
server/server.go
Outdated
@@ -19,6 +19,7 @@ import ( | |||
"sync" | |||
"time" | |||
|
|||
"fmt" |
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.
Could you sort imports?
configuration/configuration.go
Outdated
|
||
var proxyProtocol *ProxyProtocol | ||
if len(result["ProxyProtocol"]) > 0 { | ||
trustedIPs := []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.
Seems unnecessary
server/server.go
Outdated
if !ok { | ||
return false, fmt.Errorf("Type error %v", addr) | ||
} | ||
contains, err := IPs.ContainsIP(ip.IP) |
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 replace this block:
contains, err := IPs.ContainsIP(ip.IP)
if err != nil {
return false, err
}
if contains {
return true, nil
}
return false, nil
by:
return IPs.ContainsIP(ip.IP)
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.
This one is gold 😂
configuration/configuration_test.go
Outdated
@@ -142,7 +142,7 @@ func TestEntryPoints_Set(t *testing.T) { | |||
}{ | |||
{ | |||
name: "all parameters", | |||
expression: "Name:foo Address:bar TLS:goo,gii TLS CA:car Redirect.EntryPoint:RedirectEntryPoint Redirect.Regex:RedirectRegex Redirect.Replacement:RedirectReplacement Compress:true WhiteListSourceRange:Range ProxyProtocol:true", | |||
expression: "Name:foo Address:bar TLS:goo,gii TLS CA:car Redirect.EntryPoint:RedirectEntryPoint Redirect.Regex:RedirectRegex Redirect.Replacement:RedirectReplacement Compress:true WhiteListSourceRange:Range ProxyProtocol:192.168.0.1", |
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.
Instead of ProxyProtocol:192.168.0.1
I think we can use ProxyProtocol.TrustedIPs:192.168.0.1
510e6b8
to
13869a1
Compare
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.
LGTM 😉
13869a1
to
143dbd2
Compare
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.
LGTM
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.
LGTM 👏
Signed-off-by: Emile Vauge <[email protected]>
Signed-off-by: Emile Vauge <[email protected]>
Signed-off-by: Emile Vauge <[email protected]>
143dbd2
to
21df2dc
Compare
Description
This PR adds mandatory trusted IP ranges declaration while enabling proxy protocol.
⚠️ This change will break any
1.4.0-rcx
config with proxy protocol enabled.You need to update your config from:
to
with
10.5.0.2
being the IP address of your front load balancer.I'm working on adding an integration test.