Merge pull request #747 from strukturag/improve-real-ip

Improve detection of actual client IP.
This commit is contained in:
Joachim Bauch 2024-05-23 21:16:16 +02:00 committed by GitHub
commit cad442c486
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 193 additions and 63 deletions

View file

@ -309,6 +309,8 @@ interface on port `8080` below):
# Enable proxying Websocket requests to the standalone signaling server. # Enable proxying Websocket requests to the standalone signaling server.
ProxyPass "/standalone-signaling/" "ws://127.0.0.1:8080/" ProxyPass "/standalone-signaling/" "ws://127.0.0.1:8080/"
RequestHeader set X-Real-IP %{REMOTE_ADDR}s
RewriteEngine On RewriteEngine On
# Websocket connections from the clients. # Websocket connections from the clients.
RewriteRule ^/standalone-signaling/spreed/$ - [L] RewriteRule ^/standalone-signaling/spreed/$ - [L]
@ -344,6 +346,7 @@ myserver.domain.invalid {
route /standalone-signaling/* { route /standalone-signaling/* {
uri strip_prefix /standalone-signaling uri strip_prefix /standalone-signaling
reverse_proxy http://127.0.0.1:8080 reverse_proxy http://127.0.0.1:8080
header_up X-Real-IP {remote_host}
} }
} }
``` ```

45
hub.go
View file

@ -39,6 +39,7 @@ import (
"net" "net"
"net/http" "net/http"
"net/url" "net/url"
"slices"
"strings" "strings"
"sync" "sync"
"sync/atomic" "sync/atomic"
@ -2586,24 +2587,48 @@ func GetRealUserIP(r *http.Request, trusted *AllowedIps) string {
return addr return addr
} }
// Don't check any headers if the server can be reached by untrusted clients directly.
if trusted == nil || !trusted.Allowed(ip) { if trusted == nil || !trusted.Allowed(ip) {
return addr return addr
} }
if ip := r.Header.Get("X-Real-IP"); ip != "" { if realIP := r.Header.Get("X-Real-IP"); realIP != "" {
return ip if ip := net.ParseIP(realIP); len(ip) > 0 {
return realIP
}
} }
if ip := r.Header.Get("X-Forwarded-For"); ip != "" { // See https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-For#selecting_an_ip_address
// Result could be a list "clientip, proxy1, proxy2", so only use first element. forwarded := strings.Split(strings.Join(r.Header.Values("X-Forwarded-For"), ","), ",")
if pos := strings.Index(ip, ","); pos >= 0 { if len(forwarded) > 0 {
ip = strings.TrimSpace(ip[:pos]) slices.Reverse(forwarded)
var lastTrusted string
for _, hop := range forwarded {
hop = strings.TrimSpace(hop)
// Make sure to remove any port.
if host, _, err := net.SplitHostPort(hop); err == nil {
hop = host
}
ip := net.ParseIP(hop)
if len(ip) == 0 {
continue
}
if trusted.Allowed(ip) {
lastTrusted = hop
continue
}
return hop
} }
// Make sure to remove any port.
if host, _, err := net.SplitHostPort(ip); err == nil { // If all entries in the "X-Forwarded-For" list are trusted, the left-most
ip = host // will be the client IP. This can happen if a subnet is trusted and the
// client also has an IP from this subnet.
if lastTrusted != "" {
return lastTrusted
} }
return ip
} }
return addr return addr

View file

@ -3596,59 +3596,159 @@ func TestJoinRoomSwitchClient(t *testing.T) {
} }
func TestGetRealUserIP(t *testing.T) { func TestGetRealUserIP(t *testing.T) {
REMOTE_ATTR := "192.168.1.2" testcases := []struct {
expected string
trustedProxies, err := ParseAllowedIps("192.168.0.0/16") headers http.Header
if err != nil { trusted string
t.Fatal(err) addr string
}{
{
"192.168.1.2",
nil,
"192.168.0.0/16",
"192.168.1.2:23456",
},
{
"10.11.12.13",
nil,
"192.168.0.0/16",
"10.11.12.13:23456",
},
{
"10.11.12.13",
http.Header{
http.CanonicalHeaderKey("x-real-ip"): []string{"10.11.12.13"},
},
"192.168.0.0/16",
"192.168.1.2:23456",
},
{
"11.12.13.14",
http.Header{
http.CanonicalHeaderKey("x-forwarded-for"): []string{"11.12.13.14, 192.168.30.32"},
},
"192.168.0.0/16",
"192.168.1.2:23456",
},
// "X-Real-IP" has preference before "X-Forwarded-For"
{
"10.11.12.13",
http.Header{
http.CanonicalHeaderKey("x-real-ip"): []string{"10.11.12.13"},
http.CanonicalHeaderKey("x-forwarded-for"): []string{"11.12.13.14, 192.168.30.32"},
},
"192.168.0.0/16",
"192.168.1.2:23456",
},
// Multiple "X-Forwarded-For" headers are merged.
{
"11.12.13.14",
http.Header{
http.CanonicalHeaderKey("x-forwarded-for"): []string{"11.12.13.14", "192.168.30.32"},
},
"192.168.0.0/16",
"192.168.1.2:23456",
},
// Headers are ignored if coming from untrusted clients.
{
"10.11.12.13",
http.Header{
http.CanonicalHeaderKey("x-real-ip"): []string{"11.12.13.14"},
},
"192.168.0.0/16",
"10.11.12.13:23456",
},
{
"10.11.12.13",
http.Header{
http.CanonicalHeaderKey("x-forwarded-for"): []string{"11.12.13.14, 192.168.30.32"},
},
"192.168.0.0/16",
"10.11.12.13:23456",
},
// X-Forwarded-For is filtered for trusted proxies.
{
"1.2.3.4",
http.Header{
http.CanonicalHeaderKey("x-forwarded-for"): []string{"11.12.13.14, 1.2.3.4"},
},
"192.168.0.0/16",
"192.168.1.2:23456",
},
{
"1.2.3.4",
http.Header{
http.CanonicalHeaderKey("x-forwarded-for"): []string{"11.12.13.14, 1.2.3.4, 192.168.2.3"},
},
"192.168.0.0/16",
"192.168.1.2:23456",
},
{
"10.11.12.13",
http.Header{
http.CanonicalHeaderKey("x-forwarded-for"): []string{"11.12.13.14, 1.2.3.4"},
},
"192.168.0.0/16",
"10.11.12.13:23456",
},
// Invalid IPs are ignored.
{
"192.168.1.2",
http.Header{
http.CanonicalHeaderKey("x-real-ip"): []string{"this-is-not-an-ip"},
},
"192.168.0.0/16",
"192.168.1.2:23456",
},
{
"11.12.13.14",
http.Header{
http.CanonicalHeaderKey("x-real-ip"): []string{"this-is-not-an-ip"},
http.CanonicalHeaderKey("x-forwarded-for"): []string{"11.12.13.14, 192.168.30.32"},
},
"192.168.0.0/16",
"192.168.1.2:23456",
},
{
"11.12.13.14",
http.Header{
http.CanonicalHeaderKey("x-real-ip"): []string{"this-is-not-an-ip"},
http.CanonicalHeaderKey("x-forwarded-for"): []string{"11.12.13.14, 192.168.30.32, proxy1"},
},
"192.168.0.0/16",
"192.168.1.2:23456",
},
{
"192.168.1.2",
http.Header{
http.CanonicalHeaderKey("x-forwarded-for"): []string{"this-is-not-an-ip"},
},
"192.168.0.0/16",
"192.168.1.2:23456",
},
{
"192.168.2.3",
http.Header{
http.CanonicalHeaderKey("x-forwarded-for"): []string{"this-is-not-an-ip, 192.168.2.3"},
},
"192.168.0.0/16",
"192.168.1.2:23456",
},
} }
request := &http.Request{ for _, tc := range testcases {
RemoteAddr: REMOTE_ATTR + ":23456", trustedProxies, err := ParseAllowedIps(tc.trusted)
} if err != nil {
if ip := GetRealUserIP(request, trustedProxies); ip != REMOTE_ATTR { t.Errorf("invalid trusted proxies in %+v: %s", tc, err)
t.Errorf("Expected %s but got %s", REMOTE_ATTR, ip) continue
} }
request := &http.Request{
X_REAL_IP := "192.168.10.11" RemoteAddr: tc.addr,
request.Header = http.Header{ Header: tc.headers,
http.CanonicalHeaderKey("x-real-ip"): []string{X_REAL_IP}, }
} if ip := GetRealUserIP(request, trustedProxies); ip != tc.expected {
if ip := GetRealUserIP(request, trustedProxies); ip != X_REAL_IP { t.Errorf("Expected %s for %+v but got %s", tc.expected, tc, ip)
t.Errorf("Expected %s but got %s", X_REAL_IP, ip) }
}
// "X-Real-IP" has preference before "X-Forwarded-For"
X_FORWARDED_FOR_IP := "192.168.20.21"
X_FORWARDED_FOR := X_FORWARDED_FOR_IP + ":12345, 192.168.30.32"
request.Header = http.Header{
http.CanonicalHeaderKey("x-real-ip"): []string{X_REAL_IP},
http.CanonicalHeaderKey("x-forwarded-for"): []string{X_FORWARDED_FOR},
}
if ip := GetRealUserIP(request, trustedProxies); ip != X_REAL_IP {
t.Errorf("Expected %s but got %s", X_REAL_IP, ip)
}
request.Header = http.Header{
http.CanonicalHeaderKey("x-forwarded-for"): []string{X_FORWARDED_FOR},
}
if ip := GetRealUserIP(request, trustedProxies); ip != X_FORWARDED_FOR_IP {
t.Errorf("Expected %s but got %s", X_FORWARDED_FOR_IP, ip)
}
PUBLIC_IP := "1.2.3.4"
request.RemoteAddr = PUBLIC_IP + ":1234"
request.Header = http.Header{
http.CanonicalHeaderKey("x-real-ip"): []string{X_REAL_IP},
}
if ip := GetRealUserIP(request, trustedProxies); ip != PUBLIC_IP {
t.Errorf("Expected %s but got %s", PUBLIC_IP, ip)
}
request.Header = http.Header{
http.CanonicalHeaderKey("x-forwarded-for"): []string{X_FORWARDED_FOR},
}
if ip := GetRealUserIP(request, trustedProxies); ip != PUBLIC_IP {
t.Errorf("Expected %s but got %s", PUBLIC_IP, ip)
} }
} }

View file

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

View file

@ -35,7 +35,8 @@ debug = false
#allowsubscribeany = false #allowsubscribeany = false
# Comma separated list of trusted proxies (IPs or CIDR networks) that may set # Comma separated list of trusted proxies (IPs or CIDR networks) that may set
# the "X-Real-Ip" or "X-Forwarded-For" headers. # the "X-Real-Ip" or "X-Forwarded-For" headers. If both are provided, the
# "X-Real-Ip" header will take precedence (if valid).
# Leave empty to allow loopback and local addresses. # Leave empty to allow loopback and local addresses.
#trustedproxies = #trustedproxies =