Skip to content

Commit 3b555a5

Browse files
agmtthinkerou
authored andcommitted
ClientIP: check every proxy for trustiness (#2844)
1 parent fc5d6dd commit 3b555a5

File tree

2 files changed

+21
-20
lines changed

2 files changed

+21
-20
lines changed

context.go

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -759,7 +759,7 @@ func (c *Context) ClientIP() string {
759759

760760
if trusted && c.engine.ForwardedByClientIP && c.engine.RemoteIPHeaders != nil {
761761
for _, headerName := range c.engine.RemoteIPHeaders {
762-
ip, valid := validateHeader(c.requestHeader(headerName))
762+
ip, valid := c.engine.validateHeader(c.requestHeader(headerName))
763763
if valid {
764764
return ip
765765
}
@@ -768,6 +768,17 @@ func (c *Context) ClientIP() string {
768768
return remoteIP.String()
769769
}
770770

771+
func (e *Engine) isTrustedProxy(ip net.IP) bool {
772+
if e.trustedCIDRs != nil {
773+
for _, cidr := range e.trustedCIDRs {
774+
if cidr.Contains(ip) {
775+
return true
776+
}
777+
}
778+
}
779+
return false
780+
}
781+
771782
// RemoteIP parses the IP from Request.RemoteAddr, normalizes and returns the IP (without the port).
772783
// It also checks if the remoteIP is a trusted proxy or not.
773784
// In order to perform this validation, it will see if the IP is contained within at least one of the CIDR blocks
@@ -782,35 +793,25 @@ func (c *Context) RemoteIP() (net.IP, bool) {
782793
return nil, false
783794
}
784795

785-
if c.engine.trustedCIDRs != nil {
786-
for _, cidr := range c.engine.trustedCIDRs {
787-
if cidr.Contains(remoteIP) {
788-
return remoteIP, true
789-
}
790-
}
791-
}
792-
793-
return remoteIP, false
796+
return remoteIP, c.engine.isTrustedProxy(remoteIP)
794797
}
795798

796-
func validateHeader(header string) (clientIP string, valid bool) {
799+
func (e *Engine) validateHeader(header string) (clientIP string, valid bool) {
797800
if header == "" {
798801
return "", false
799802
}
800803
items := strings.Split(header, ",")
801-
for i, ipStr := range items {
802-
ipStr = strings.TrimSpace(ipStr)
804+
for i := len(items) - 1; i >= 0; i-- {
805+
ipStr := strings.TrimSpace(items[i])
803806
ip := net.ParseIP(ipStr)
804807
if ip == nil {
805808
return "", false
806809
}
807810

808-
// We need to return the first IP in the list, but,
809-
// we should not early return since we need to validate that
810-
// the rest of the header is syntactically valid
811-
if i == 0 {
812-
clientIP = ipStr
813-
valid = true
811+
// X-Forwarded-For is appended by proxy
812+
// Check IPs in reverse order and stop when find untrusted proxy
813+
if (i == 0) || (!e.isTrustedProxy(ip)) {
814+
return ipStr, true
814815
}
815816
}
816817
return

context_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1421,7 +1421,7 @@ func TestContextClientIP(t *testing.T) {
14211421

14221422
// Only trust RemoteAddr
14231423
_ = c.engine.SetTrustedProxies([]string{"40.40.40.40"})
1424-
assert.Equal(t, "20.20.20.20", c.ClientIP())
1424+
assert.Equal(t, "30.30.30.30", c.ClientIP())
14251425

14261426
// All steps are trusted
14271427
_ = c.engine.SetTrustedProxies([]string{"40.40.40.40", "30.30.30.30", "20.20.20.20"})

0 commit comments

Comments
 (0)