Skip to content

Commit 7544796

Browse files
42wim42wim
authoredNov 24, 2022
Remove square brackets from ipv6 addresses in XFF (#2182)
Remove square brackets from ipv6 addresses in XFF
·
v4.13.3v4.10.0
1 parent 36ff0b3 commit 7544796

File tree

4 files changed

+125
-3
lines changed

4 files changed

+125
-3
lines changed
 

‎context.go

+6-1
Original file line numberDiff line numberDiff line change
@@ -282,11 +282,16 @@ func (c *context) RealIP() string {
282282
if ip := c.request.Header.Get(HeaderXForwardedFor); ip != "" {
283283
i := strings.IndexAny(ip, ",")
284284
if i > 0 {
285-
return strings.TrimSpace(ip[:i])
285+
xffip := strings.TrimSpace(ip[:i])
286+
xffip = strings.TrimPrefix(xffip, "[")
287+
xffip = strings.TrimSuffix(xffip, "]")
288+
return xffip
286289
}
287290
return ip
288291
}
289292
if ip := c.request.Header.Get(HeaderXRealIP); ip != "" {
293+
ip = strings.TrimPrefix(ip, "[")
294+
ip = strings.TrimSuffix(ip, "]")
290295
return ip
291296
}
292297
ra, _, _ := net.SplitHostPort(c.request.RemoteAddr)

‎context_test.go

+35-1
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,6 @@ func (responseWriterErr) Write([]byte) (int, error) {
9797
}
9898

9999
func (responseWriterErr) WriteHeader(statusCode int) {
100-
101100
}
102101

103102
func TestContext(t *testing.T) {
@@ -904,6 +903,30 @@ func TestContext_RealIP(t *testing.T) {
904903
},
905904
"127.0.0.1",
906905
},
906+
{
907+
&context{
908+
request: &http.Request{
909+
Header: http.Header{HeaderXForwardedFor: []string{"[2001:db8:85a3:8d3:1319:8a2e:370:7348], 2001:db8::1, "}},
910+
},
911+
},
912+
"2001:db8:85a3:8d3:1319:8a2e:370:7348",
913+
},
914+
{
915+
&context{
916+
request: &http.Request{
917+
Header: http.Header{HeaderXForwardedFor: []string{"[2001:db8:85a3:8d3:1319:8a2e:370:7348],[2001:db8::1]"}},
918+
},
919+
},
920+
"2001:db8:85a3:8d3:1319:8a2e:370:7348",
921+
},
922+
{
923+
&context{
924+
request: &http.Request{
925+
Header: http.Header{HeaderXForwardedFor: []string{"2001:db8:85a3:8d3:1319:8a2e:370:7348"}},
926+
},
927+
},
928+
"2001:db8:85a3:8d3:1319:8a2e:370:7348",
929+
},
907930
{
908931
&context{
909932
request: &http.Request{
@@ -914,6 +937,17 @@ func TestContext_RealIP(t *testing.T) {
914937
},
915938
"192.168.0.1",
916939
},
940+
{
941+
&context{
942+
request: &http.Request{
943+
Header: http.Header{
944+
"X-Real-Ip": []string{"[2001:db8::1]"},
945+
},
946+
},
947+
},
948+
"2001:db8::1",
949+
},
950+
917951
{
918952
&context{
919953
request: &http.Request{

‎ip.go

+6-1
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,8 @@ func ExtractIPFromRealIPHeader(options ...TrustOption) IPExtractor {
227227
return func(req *http.Request) string {
228228
realIP := req.Header.Get(HeaderXRealIP)
229229
if realIP != "" {
230+
realIP = strings.TrimPrefix(realIP, "[")
231+
realIP = strings.TrimSuffix(realIP, "]")
230232
if ip := net.ParseIP(realIP); ip != nil && checker.trust(ip) {
231233
return realIP
232234
}
@@ -248,7 +250,10 @@ func ExtractIPFromXFFHeader(options ...TrustOption) IPExtractor {
248250
}
249251
ips := append(strings.Split(strings.Join(xffs, ","), ","), directIP)
250252
for i := len(ips) - 1; i >= 0; i-- {
251-
ip := net.ParseIP(strings.TrimSpace(ips[i]))
253+
ips[i] = strings.TrimSpace(ips[i])
254+
ips[i] = strings.TrimPrefix(ips[i], "[")
255+
ips[i] = strings.TrimSuffix(ips[i], "]")
256+
ip := net.ParseIP(ips[i])
252257
if ip == nil {
253258
// Unable to parse IP; cannot trust entire records
254259
return directIP

‎ip_test.go

+78
Original file line numberDiff line numberDiff line change
@@ -459,6 +459,7 @@ func TestExtractIPDirect(t *testing.T) {
459459

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

463464
var testCases = []struct {
464465
name string
@@ -493,6 +494,16 @@ func TestExtractIPFromRealIPHeader(t *testing.T) {
493494
},
494495
expectIP: "203.0.113.1",
495496
},
497+
{
498+
name: "request is from external IP has valid + UNTRUSTED external X-Real-Ip header, extract IP from remote addr",
499+
whenRequest: http.Request{
500+
Header: http.Header{
501+
HeaderXRealIP: []string{"[2001:db8::113:199]"}, // <-- this is untrusted
502+
},
503+
RemoteAddr: "[2001:db8::113:1]:8080",
504+
},
505+
expectIP: "2001:db8::113:1",
506+
},
496507
{
497508
name: "request is from external IP has valid + TRUSTED X-Real-Ip header, extract IP from X-Real-Ip header",
498509
givenTrustOptions: []TrustOption{ // case for "trust direct-facing proxy"
@@ -506,6 +517,19 @@ func TestExtractIPFromRealIPHeader(t *testing.T) {
506517
},
507518
expectIP: "203.0.113.199",
508519
},
520+
{
521+
name: "request is from external IP has valid + TRUSTED X-Real-Ip header, extract IP from X-Real-Ip header",
522+
givenTrustOptions: []TrustOption{ // case for "trust direct-facing proxy"
523+
TrustIPRange(ipv6ForRemoteAddrExternalRange), // we trust external IP range "2001:db8::/64"
524+
},
525+
whenRequest: http.Request{
526+
Header: http.Header{
527+
HeaderXRealIP: []string{"[2001:db8::113:199]"},
528+
},
529+
RemoteAddr: "[2001:db8::113:1]:8080",
530+
},
531+
expectIP: "2001:db8::113:199",
532+
},
509533
{
510534
name: "request is from external IP has XFF and valid + TRUSTED X-Real-Ip header, extract IP from X-Real-Ip header",
511535
givenTrustOptions: []TrustOption{ // case for "trust direct-facing proxy"
@@ -520,6 +544,20 @@ func TestExtractIPFromRealIPHeader(t *testing.T) {
520544
},
521545
expectIP: "203.0.113.199",
522546
},
547+
{
548+
name: "request is from external IP has XFF and valid + TRUSTED X-Real-Ip header, extract IP from X-Real-Ip header",
549+
givenTrustOptions: []TrustOption{ // case for "trust direct-facing proxy"
550+
TrustIPRange(ipv6ForRemoteAddrExternalRange), // we trust external IP range "2001:db8::/64"
551+
},
552+
whenRequest: http.Request{
553+
Header: http.Header{
554+
HeaderXRealIP: []string{"[2001:db8::113:199]"},
555+
HeaderXForwardedFor: []string{"[2001:db8::113:198], [2001:db8::113:197]"}, // <-- should not affect anything
556+
},
557+
RemoteAddr: "[2001:db8::113:1]:8080",
558+
},
559+
expectIP: "2001:db8::113:199",
560+
},
523561
}
524562

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

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

536575
var testCases = []struct {
537576
name string
@@ -566,6 +605,16 @@ func TestExtractIPFromXFFHeader(t *testing.T) {
566605
},
567606
expectIP: "127.0.0.3",
568607
},
608+
{
609+
name: "request trusts all IPs in XFF header, extract IP from furthest in XFF chain",
610+
whenRequest: http.Request{
611+
Header: http.Header{
612+
HeaderXForwardedFor: []string{"[fe80::3], [fe80::2], [fe80::1]"},
613+
},
614+
RemoteAddr: "[fe80::1]:8080",
615+
},
616+
expectIP: "fe80::3",
617+
},
569618
{
570619
name: "request is from external IP has valid + UNTRUSTED external XFF header, extract IP from remote addr",
571620
whenRequest: http.Request{
@@ -576,6 +625,16 @@ func TestExtractIPFromXFFHeader(t *testing.T) {
576625
},
577626
expectIP: "203.0.113.1",
578627
},
628+
{
629+
name: "request is from external IP has valid + UNTRUSTED external XFF header, extract IP from remote addr",
630+
whenRequest: http.Request{
631+
Header: http.Header{
632+
HeaderXForwardedFor: []string{"[2001:db8::1]"}, // <-- this is untrusted
633+
},
634+
RemoteAddr: "[2001:db8::2]:8080",
635+
},
636+
expectIP: "2001:db8::2",
637+
},
579638
{
580639
name: "request is from external IP is valid and has some IPs TRUSTED XFF header, extract IP from XFF header",
581640
givenTrustOptions: []TrustOption{
@@ -595,6 +654,25 @@ func TestExtractIPFromXFFHeader(t *testing.T) {
595654
},
596655
expectIP: "203.0.100.100", // this is first trusted IP in XFF chain
597656
},
657+
{
658+
name: "request is from external IP is valid and has some IPs TRUSTED XFF header, extract IP from XFF header",
659+
givenTrustOptions: []TrustOption{
660+
TrustIPRange(ipv6ForRemoteAddrExternalRange), // we trust external IP range "2001:db8::/64"
661+
},
662+
// from request its seems that request has been proxied through 6 servers.
663+
// 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)
664+
// 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)
665+
// 3) 2001:db8::113:199 (we trust, for example maybe our proxy from some other office)
666+
// 4) fd12:3456:789a:1::1 (internal IP, some internal upstream loadbalancer ala SSL offloading with F5 products)
667+
// 5) fe80::1 (is proxy on localhost. maybe we have Nginx in front of our Echo instance doing some routing)
668+
whenRequest: http.Request{
669+
Header: http.Header{
670+
HeaderXForwardedFor: []string{"[2001:db8:1::1:100], [2001:db8:2::100:100], [2001:db8::113:199], [fd12:3456:789a:1::1]"},
671+
},
672+
RemoteAddr: "[fe80::1]:8080", // IP of proxy upstream of our APP
673+
},
674+
expectIP: "2001:db8:2::100:100", // this is first trusted IP in XFF chain
675+
},
598676
}
599677

600678
for _, tc := range testCases {

0 commit comments

Comments
 (0)