Skip to content

Commit cad442c

Browse files
fancycodefancycode
authored
Merge pull request #747 from strukturag/improve-real-ip
Improve detection of actual client IP.
2 parents e8ebfed + 040e663 commit cad442c

File tree

5 files changed

+193
-63
lines changed

5 files changed

+193
-63
lines changed

‎README.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,8 @@ interface on port `8080` below):
309309
# Enable proxying Websocket requests to the standalone signaling server.
310310
ProxyPass "/standalone-signaling/" "ws://127.0.0.1:8080/"
311311

312+
RequestHeader set X-Real-IP %{REMOTE_ADDR}s
313+
312314
RewriteEngine On
313315
# Websocket connections from the clients.
314316
RewriteRule ^/standalone-signaling/spreed/$ - [L]
@@ -344,6 +346,7 @@ myserver.domain.invalid {
344346
route /standalone-signaling/* {
345347
uri strip_prefix /standalone-signaling
346348
reverse_proxy http://127.0.0.1:8080
349+
header_up X-Real-IP {remote_host}
347350
}
348351
}
349352
```

‎hub.go‎

Lines changed: 35 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ import (
3939
"net"
4040
"net/http"
4141
"net/url"
42+
"slices"
4243
"strings"
4344
"sync"
4445
"sync/atomic"
@@ -2586,24 +2587,48 @@ func GetRealUserIP(r *http.Request, trusted *AllowedIps) string {
25862587
return addr
25872588
}
25882589

2590+
// Don't check any headers if the server can be reached by untrusted clients directly.
25892591
if trusted == nil || !trusted.Allowed(ip) {
25902592
return addr
25912593
}
25922594

2593-
if ip := r.Header.Get("X-Real-IP"); ip != "" {
2594-
return ip
2595+
if realIP := r.Header.Get("X-Real-IP"); realIP != "" {
2596+
if ip := net.ParseIP(realIP); len(ip) > 0 {
2597+
return realIP
2598+
}
25952599
}
25962600

2597-
if ip := r.Header.Get("X-Forwarded-For"); ip != "" {
2598-
// Result could be a list "clientip, proxy1, proxy2", so only use first element.
2599-
if pos := strings.Index(ip, ","); pos >= 0 {
2600-
ip = strings.TrimSpace(ip[:pos])
2601+
// See https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-For#selecting_an_ip_address
2602+
forwarded := strings.Split(strings.Join(r.Header.Values("X-Forwarded-For"), ","), ",")
2603+
if len(forwarded) > 0 {
2604+
slices.Reverse(forwarded)
2605+
var lastTrusted string
2606+
for _, hop := range forwarded {
2607+
hop = strings.TrimSpace(hop)
2608+
// Make sure to remove any port.
2609+
if host, _, err := net.SplitHostPort(hop); err == nil {
2610+
hop = host
2611+
}
2612+
2613+
ip := net.ParseIP(hop)
2614+
if len(ip) == 0 {
2615+
continue
2616+
}
2617+
2618+
if trusted.Allowed(ip) {
2619+
lastTrusted = hop
2620+
continue
2621+
}
2622+
2623+
return hop
26012624
}
2602-
// Make sure to remove any port.
2603-
if host, _, err := net.SplitHostPort(ip); err == nil {
2604-
ip = host
2625+
2626+
// If all entries in the "X-Forwarded-For" list are trusted, the left-most
2627+
// will be the client IP. This can happen if a subnet is trusted and the
2628+
// client also has an IP from this subnet.
2629+
if lastTrusted != "" {
2630+
return lastTrusted
26052631
}
2606-
return ip
26072632
}
26082633

26092634
return addr

‎hub_test.go‎

Lines changed: 151 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -3596,59 +3596,159 @@ func TestJoinRoomSwitchClient(t *testing.T) {
35963596
}
35973597

35983598
func TestGetRealUserIP(t *testing.T) {
3599-
REMOTE_ATTR := "192.168.1.2"
3600-
3601-
trustedProxies, err := ParseAllowedIps("192.168.0.0/16")
3602-
if err != nil {
3603-
t.Fatal(err)
3604-
}
3605-
3606-
request := &http.Request{
3607-
RemoteAddr: REMOTE_ATTR + ":23456",
3608-
}
3609-
if ip := GetRealUserIP(request, trustedProxies); ip != REMOTE_ATTR {
3610-
t.Errorf("Expected %s but got %s", REMOTE_ATTR, ip)
3611-
}
3612-
3613-
X_REAL_IP := "192.168.10.11"
3614-
request.Header = http.Header{
3615-
http.CanonicalHeaderKey("x-real-ip"): []string{X_REAL_IP},
3616-
}
3617-
if ip := GetRealUserIP(request, trustedProxies); ip != X_REAL_IP {
3618-
t.Errorf("Expected %s but got %s", X_REAL_IP, ip)
3619-
}
3620-
3621-
// "X-Real-IP" has preference before "X-Forwarded-For"
3622-
X_FORWARDED_FOR_IP := "192.168.20.21"
3623-
X_FORWARDED_FOR := X_FORWARDED_FOR_IP + ":12345, 192.168.30.32"
3624-
request.Header = http.Header{
3625-
http.CanonicalHeaderKey("x-real-ip"): []string{X_REAL_IP},
3626-
http.CanonicalHeaderKey("x-forwarded-for"): []string{X_FORWARDED_FOR},
3627-
}
3628-
if ip := GetRealUserIP(request, trustedProxies); ip != X_REAL_IP {
3629-
t.Errorf("Expected %s but got %s", X_REAL_IP, ip)
3630-
}
3631-
3632-
request.Header = http.Header{
3633-
http.CanonicalHeaderKey("x-forwarded-for"): []string{X_FORWARDED_FOR},
3634-
}
3635-
if ip := GetRealUserIP(request, trustedProxies); ip != X_FORWARDED_FOR_IP {
3636-
t.Errorf("Expected %s but got %s", X_FORWARDED_FOR_IP, ip)
3599+
testcases := []struct {
3600+
expected string
3601+
headers http.Header
3602+
trusted string
3603+
addr string
3604+
}{
3605+
{
3606+
"192.168.1.2",
3607+
nil,
3608+
"192.168.0.0/16",
3609+
"192.168.1.2:23456",
3610+
},
3611+
{
3612+
"10.11.12.13",
3613+
nil,
3614+
"192.168.0.0/16",
3615+
"10.11.12.13:23456",
3616+
},
3617+
{
3618+
"10.11.12.13",
3619+
http.Header{
3620+
http.CanonicalHeaderKey("x-real-ip"): []string{"10.11.12.13"},
3621+
},
3622+
"192.168.0.0/16",
3623+
"192.168.1.2:23456",
3624+
},
3625+
{
3626+
"11.12.13.14",
3627+
http.Header{
3628+
http.CanonicalHeaderKey("x-forwarded-for"): []string{"11.12.13.14, 192.168.30.32"},
3629+
},
3630+
"192.168.0.0/16",
3631+
"192.168.1.2:23456",
3632+
},
3633+
// "X-Real-IP" has preference before "X-Forwarded-For"
3634+
{
3635+
"10.11.12.13",
3636+
http.Header{
3637+
http.CanonicalHeaderKey("x-real-ip"): []string{"10.11.12.13"},
3638+
http.CanonicalHeaderKey("x-forwarded-for"): []string{"11.12.13.14, 192.168.30.32"},
3639+
},
3640+
"192.168.0.0/16",
3641+
"192.168.1.2:23456",
3642+
},
3643+
// Multiple "X-Forwarded-For" headers are merged.
3644+
{
3645+
"11.12.13.14",
3646+
http.Header{
3647+
http.CanonicalHeaderKey("x-forwarded-for"): []string{"11.12.13.14", "192.168.30.32"},
3648+
},
3649+
"192.168.0.0/16",
3650+
"192.168.1.2:23456",
3651+
},
3652+
// Headers are ignored if coming from untrusted clients.
3653+
{
3654+
"10.11.12.13",
3655+
http.Header{
3656+
http.CanonicalHeaderKey("x-real-ip"): []string{"11.12.13.14"},
3657+
},
3658+
"192.168.0.0/16",
3659+
"10.11.12.13:23456",
3660+
},
3661+
{
3662+
"10.11.12.13",
3663+
http.Header{
3664+
http.CanonicalHeaderKey("x-forwarded-for"): []string{"11.12.13.14, 192.168.30.32"},
3665+
},
3666+
"192.168.0.0/16",
3667+
"10.11.12.13:23456",
3668+
},
3669+
// X-Forwarded-For is filtered for trusted proxies.
3670+
{
3671+
"1.2.3.4",
3672+
http.Header{
3673+
http.CanonicalHeaderKey("x-forwarded-for"): []string{"11.12.13.14, 1.2.3.4"},
3674+
},
3675+
"192.168.0.0/16",
3676+
"192.168.1.2:23456",
3677+
},
3678+
{
3679+
"1.2.3.4",
3680+
http.Header{
3681+
http.CanonicalHeaderKey("x-forwarded-for"): []string{"11.12.13.14, 1.2.3.4, 192.168.2.3"},
3682+
},
3683+
"192.168.0.0/16",
3684+
"192.168.1.2:23456",
3685+
},
3686+
{
3687+
"10.11.12.13",
3688+
http.Header{
3689+
http.CanonicalHeaderKey("x-forwarded-for"): []string{"11.12.13.14, 1.2.3.4"},
3690+
},
3691+
"192.168.0.0/16",
3692+
"10.11.12.13:23456",
3693+
},
3694+
// Invalid IPs are ignored.
3695+
{
3696+
"192.168.1.2",
3697+
http.Header{
3698+
http.CanonicalHeaderKey("x-real-ip"): []string{"this-is-not-an-ip"},
3699+
},
3700+
"192.168.0.0/16",
3701+
"192.168.1.2:23456",
3702+
},
3703+
{
3704+
"11.12.13.14",
3705+
http.Header{
3706+
http.CanonicalHeaderKey("x-real-ip"): []string{"this-is-not-an-ip"},
3707+
http.CanonicalHeaderKey("x-forwarded-for"): []string{"11.12.13.14, 192.168.30.32"},
3708+
},
3709+
"192.168.0.0/16",
3710+
"192.168.1.2:23456",
3711+
},
3712+
{
3713+
"11.12.13.14",
3714+
http.Header{
3715+
http.CanonicalHeaderKey("x-real-ip"): []string{"this-is-not-an-ip"},
3716+
http.CanonicalHeaderKey("x-forwarded-for"): []string{"11.12.13.14, 192.168.30.32, proxy1"},
3717+
},
3718+
"192.168.0.0/16",
3719+
"192.168.1.2:23456",
3720+
},
3721+
{
3722+
"192.168.1.2",
3723+
http.Header{
3724+
http.CanonicalHeaderKey("x-forwarded-for"): []string{"this-is-not-an-ip"},
3725+
},
3726+
"192.168.0.0/16",
3727+
"192.168.1.2:23456",
3728+
},
3729+
{
3730+
"192.168.2.3",
3731+
http.Header{
3732+
http.CanonicalHeaderKey("x-forwarded-for"): []string{"this-is-not-an-ip, 192.168.2.3"},
3733+
},
3734+
"192.168.0.0/16",
3735+
"192.168.1.2:23456",
3736+
},
36373737
}
36383738

3639-
PUBLIC_IP := "1.2.3.4"
3640-
request.RemoteAddr = PUBLIC_IP + ":1234"
3641-
request.Header = http.Header{
3642-
http.CanonicalHeaderKey("x-real-ip"): []string{X_REAL_IP},
3643-
}
3644-
if ip := GetRealUserIP(request, trustedProxies); ip != PUBLIC_IP {
3645-
t.Errorf("Expected %s but got %s", PUBLIC_IP, ip)
3646-
}
3647-
request.Header = http.Header{
3648-
http.CanonicalHeaderKey("x-forwarded-for"): []string{X_FORWARDED_FOR},
3649-
}
3650-
if ip := GetRealUserIP(request, trustedProxies); ip != PUBLIC_IP {
3651-
t.Errorf("Expected %s but got %s", PUBLIC_IP, ip)
3739+
for _, tc := range testcases {
3740+
trustedProxies, err := ParseAllowedIps(tc.trusted)
3741+
if err != nil {
3742+
t.Errorf("invalid trusted proxies in %+v: %s", tc, err)
3743+
continue
3744+
}
3745+
request := &http.Request{
3746+
RemoteAddr: tc.addr,
3747+
Header: tc.headers,
3748+
}
3749+
if ip := GetRealUserIP(request, trustedProxies); ip != tc.expected {
3750+
t.Errorf("Expected %s for %+v but got %s", tc.expected, tc, ip)
3751+
}
36523752
}
36533753
}
36543754

‎proxy.conf.in‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,8 @@
99
#debug = false
1010

1111
# Comma separated list of trusted proxies (IPs or CIDR networks) that may set
12-
# the "X-Real-Ip" or "X-Forwarded-For" headers.
12+
# the "X-Real-Ip" or "X-Forwarded-For" headers. If both are provided, the
13+
# "X-Real-Ip" header will take precedence (if valid).
1314
# Leave empty to allow loopback and local addresses.
1415
#trustedproxies =
1516

‎server.conf.in‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,8 @@ debug = false
3535
#allowsubscribeany = false
3636

3737
# Comma separated list of trusted proxies (IPs or CIDR networks) that may set
38-
# the "X-Real-Ip" or "X-Forwarded-For" headers.
38+
# the "X-Real-Ip" or "X-Forwarded-For" headers. If both are provided, the
39+
# "X-Real-Ip" header will take precedence (if valid).
3940
# Leave empty to allow loopback and local addresses.
4041
#trustedproxies =
4142

0 commit comments

Comments
 (0)