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.

Sign up for GitHub

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

Jump to bottom

Remove square brackets from ipv6 addresses in XFF #2182

Merged
merged 3 commits into from Nov 24, 2022
Merged

Remove square brackets from ipv6 addresses in XFF #2182

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7e723fb
Remove brackets from ipv6 addresses in XFF
42wim May 16, 2022
1a84dce
Remove brackets for HeaderXRealIP and add tests, remove ReplaceAll
42wim May 21, 2022
0c5648d
Add missing Trimspace
42wim Aug 13, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 6 additions & 1 deletion context.go
View file
Open in desktop
Expand Up @@ -282,11 +282,16 @@ func (c *context) RealIP() string {
if ip := c.request.Header.Get(HeaderXForwardedFor); ip != "" {
i := strings.IndexAny(ip, ",")
if i > 0 {
return strings.TrimSpace(ip[:i])
42wim marked this conversation as resolved.
Show resolved Hide resolved
xffip := strings.TrimSpace(ip[:i])
xffip = strings.TrimPrefix(xffip, "[")
xffip = strings.TrimSuffix(xffip, "]")
return xffip
42wim marked this conversation as resolved.
Show resolved Hide resolved
}
return ip
}
if ip := c.request.Header.Get(HeaderXRealIP); ip != "" {
ip = strings.TrimPrefix(ip, "[")
ip = strings.TrimSuffix(ip, "]")
return ip
}
ra, _, _ := net.SplitHostPort(c.request.RemoteAddr)
Expand Down
36 changes: 35 additions & 1 deletion context_test.go
View file
Open in desktop
Expand Up @@ -97,7 +97,6 @@ func (responseWriterErr) Write([]byte) (int, error) {
}

func (responseWriterErr) WriteHeader(statusCode int) {

}

func TestContext(t *testing.T) {
Expand Down Expand Up @@ -904,6 +903,30 @@ func TestContext_RealIP(t *testing.T) {
},
"127.0.0.1",
},
{
&context{
request: &http.Request{
Header: http.Header{HeaderXForwardedFor: []string{"[2001:db8:85a3:8d3:1319:8a2e:370:7348], 2001:db8::1, "}},
},
},
"2001:db8:85a3:8d3:1319:8a2e:370:7348",
},
{
&context{
request: &http.Request{
Header: http.Header{HeaderXForwardedFor: []string{"[2001:db8:85a3:8d3:1319:8a2e:370:7348],[2001:db8::1]"}},
},
},
"2001:db8:85a3:8d3:1319:8a2e:370:7348",
},
{
&context{
request: &http.Request{
Header: http.Header{HeaderXForwardedFor: []string{"2001:db8:85a3:8d3:1319:8a2e:370:7348"}},
},
},
"2001:db8:85a3:8d3:1319:8a2e:370:7348",
},
{
&context{
request: &http.Request{
Expand All @@ -914,6 +937,17 @@ func TestContext_RealIP(t *testing.T) {
},
"192.168.0.1",
},
{
&context{
request: &http.Request{
Header: http.Header{
"X-Real-Ip": []string{"[2001:db8::1]"},
},
},
},
"2001:db8::1",
},

{
&context{
request: &http.Request{
Expand Down
7 changes: 6 additions & 1 deletion ip.go
View file
Open in desktop
Expand Up @@ -227,6 +227,8 @@ func ExtractIPFromRealIPHeader(options ...TrustOption) IPExtractor {
return func(req *http.Request) string {
realIP := req.Header.Get(HeaderXRealIP)
if realIP != "" {
realIP = strings.TrimPrefix(realIP, "[")
realIP = strings.TrimSuffix(realIP, "]")
if ip := net.ParseIP(realIP); ip != nil && checker.trust(ip) {
return realIP
}
Expand All @@ -248,7 +250,10 @@ func ExtractIPFromXFFHeader(options ...TrustOption) IPExtractor {
}
ips := append(strings.Split(strings.Join(xffs, ","), ","), directIP)
for i := len(ips) - 1; i >= 0; i-- {
ip := net.ParseIP(strings.TrimSpace(ips[i]))
ips[i] = strings.TrimSpace(ips[i])
ips[i] = strings.TrimPrefix(ips[i], "[")
ips[i] = strings.TrimSuffix(ips[i], "]")
ip := net.ParseIP(ips[i])
if ip == nil {
// Unable to parse IP; cannot trust entire records
return directIP
Expand Down
78 changes: 78 additions & 0 deletions ip_test.go
View file
Open in desktop
Expand Up @@ -459,6 +459,7 @@ func TestExtractIPDirect(t *testing.T) {

func TestExtractIPFromRealIPHeader(t *testing.T) {
_, ipForRemoteAddrExternalRange, _ := net.ParseCIDR("203.0.113.199/24")
_, ipv6ForRemoteAddrExternalRange, _ := net.ParseCIDR("2001:db8::/64")

var testCases = []struct {
name string
Expand Down Expand Up @@ -493,6 +494,16 @@ func TestExtractIPFromRealIPHeader(t *testing.T) {
},
expectIP: "203.0.113.1",
},
{
name: "request is from external IP has valid + UNTRUSTED external X-Real-Ip header, extract IP from remote addr",
whenRequest: http.Request{
Header: http.Header{
HeaderXRealIP: []string{"[2001:db8::113:199]"}, // <-- this is untrusted
},
RemoteAddr: "[2001:db8::113:1]:8080",
},
expectIP: "2001:db8::113:1",
},
{
name: "request is from external IP has valid + TRUSTED X-Real-Ip header, extract IP from X-Real-Ip header",
givenTrustOptions: []TrustOption{ // case for "trust direct-facing proxy"
Expand All @@ -506,6 +517,19 @@ func TestExtractIPFromRealIPHeader(t *testing.T) {
},
expectIP: "203.0.113.199",
},
{
name: "request is from external IP has valid + TRUSTED X-Real-Ip header, extract IP from X-Real-Ip header",
givenTrustOptions: []TrustOption{ // case for "trust direct-facing proxy"
TrustIPRange(ipv6ForRemoteAddrExternalRange), // we trust external IP range "2001:db8::/64"
},
whenRequest: http.Request{
Header: http.Header{
HeaderXRealIP: []string{"[2001:db8::113:199]"},
},
RemoteAddr: "[2001:db8::113:1]:8080",
},
expectIP: "2001:db8::113:199",
},
{
name: "request is from external IP has XFF and valid + TRUSTED X-Real-Ip header, extract IP from X-Real-Ip header",
givenTrustOptions: []TrustOption{ // case for "trust direct-facing proxy"
Expand All @@ -520,6 +544,20 @@ func TestExtractIPFromRealIPHeader(t *testing.T) {
},
expectIP: "203.0.113.199",
},
{
name: "request is from external IP has XFF and valid + TRUSTED X-Real-Ip header, extract IP from X-Real-Ip header",
givenTrustOptions: []TrustOption{ // case for "trust direct-facing proxy"
TrustIPRange(ipv6ForRemoteAddrExternalRange), // we trust external IP range "2001:db8::/64"
},
whenRequest: http.Request{
Header: http.Header{
HeaderXRealIP: []string{"[2001:db8::113:199]"},
HeaderXForwardedFor: []string{"[2001:db8::113:198], [2001:db8::113:197]"}, // <-- should not affect anything
},
RemoteAddr: "[2001:db8::113:1]:8080",
},
expectIP: "2001:db8::113:199",
},
}

for _, tc := range testCases {
Expand All @@ -532,6 +570,7 @@ func TestExtractIPFromRealIPHeader(t *testing.T) {

func TestExtractIPFromXFFHeader(t *testing.T) {
_, ipForRemoteAddrExternalRange, _ := net.ParseCIDR("203.0.113.199/24")
_, ipv6ForRemoteAddrExternalRange, _ := net.ParseCIDR("2001:db8::/64")

var testCases = []struct {
name string
Expand Down Expand Up @@ -566,6 +605,16 @@ func TestExtractIPFromXFFHeader(t *testing.T) {
},
expectIP: "127.0.0.3",
},
{
name: "request trusts all IPs in XFF header, extract IP from furthest in XFF chain",
whenRequest: http.Request{
Header: http.Header{
HeaderXForwardedFor: []string{"[fe80::3], [fe80::2], [fe80::1]"},
},
RemoteAddr: "[fe80::1]:8080",
},
expectIP: "fe80::3",
},
{
name: "request is from external IP has valid + UNTRUSTED external XFF header, extract IP from remote addr",
whenRequest: http.Request{
Expand All @@ -576,6 +625,16 @@ func TestExtractIPFromXFFHeader(t *testing.T) {
},
expectIP: "203.0.113.1",
},
{
name: "request is from external IP has valid + UNTRUSTED external XFF header, extract IP from remote addr",
whenRequest: http.Request{
Header: http.Header{
HeaderXForwardedFor: []string{"[2001:db8::1]"}, // <-- this is untrusted
},
RemoteAddr: "[2001:db8::2]:8080",
},
expectIP: "2001:db8::2",
},
{
name: "request is from external IP is valid and has some IPs TRUSTED XFF header, extract IP from XFF header",
givenTrustOptions: []TrustOption{
Expand All @@ -595,6 +654,25 @@ func TestExtractIPFromXFFHeader(t *testing.T) {
},
expectIP: "203.0.100.100", // this is first trusted IP in XFF chain
},
{
name: "request is from external IP is valid and has some IPs TRUSTED XFF header, extract IP from XFF header",
givenTrustOptions: []TrustOption{
TrustIPRange(ipv6ForRemoteAddrExternalRange), // we trust external IP range "2001:db8::/64"
},
// from request its seems that request has been proxied through 6 servers.
// 1) 2001:db8:1::1:100 (this is external IP set by 2001:db8:2::100:100 which we do not trust - could be spoofed)
// 2) 2001:db8:2::100:100 (this is outside of our network but set by 2001:db8::113:199 which we trust to set correct IPs)
// 3) 2001:db8::113:199 (we trust, for example maybe our proxy from some other office)
// 4) fd12:3456:789a:1::1 (internal IP, some internal upstream loadbalancer ala SSL offloading with F5 products)
// 5) fe80::1 (is proxy on localhost. maybe we have Nginx in front of our Echo instance doing some routing)
whenRequest: http.Request{
Header: http.Header{
HeaderXForwardedFor: []string{"[2001:db8:1::1:100], [2001:db8:2::100:100], [2001:db8::113:199], [fd12:3456:789a:1::1]"},
},
RemoteAddr: "[fe80::1]:8080", // IP of proxy upstream of our APP
},
expectIP: "2001:db8:2::100:100", // this is first trusted IP in XFF chain
},
}

for _, tc := range testCases {
Expand Down