diff --git a/README.md b/README.md index 1c9ad8a..fcba4a8 100644 --- a/README.md +++ b/README.md @@ -309,6 +309,8 @@ interface on port `8080` below): # Enable proxying Websocket requests to the standalone signaling server. ProxyPass "/standalone-signaling/" "ws://127.0.0.1:8080/" + RequestHeader set X-Real-IP %{REMOTE_ADDR}s + RewriteEngine On # Websocket connections from the clients. RewriteRule ^/standalone-signaling/spreed/$ - [L] @@ -344,6 +346,7 @@ myserver.domain.invalid { route /standalone-signaling/* { uri strip_prefix /standalone-signaling reverse_proxy http://127.0.0.1:8080 + header_up X-Real-IP {remote_host} } } ``` diff --git a/hub.go b/hub.go index 08ccac6..8a5b5cb 100644 --- a/hub.go +++ b/hub.go @@ -39,6 +39,7 @@ import ( "net" "net/http" "net/url" + "slices" "strings" "sync" "sync/atomic" @@ -2586,24 +2587,48 @@ func GetRealUserIP(r *http.Request, trusted *AllowedIps) string { return addr } + // Don't check any headers if the server can be reached by untrusted clients directly. if trusted == nil || !trusted.Allowed(ip) { return addr } - if ip := r.Header.Get("X-Real-IP"); ip != "" { - return ip + if realIP := r.Header.Get("X-Real-IP"); realIP != "" { + if ip := net.ParseIP(realIP); len(ip) > 0 { + return realIP + } } - if ip := r.Header.Get("X-Forwarded-For"); ip != "" { - // Result could be a list "clientip, proxy1, proxy2", so only use first element. - if pos := strings.Index(ip, ","); pos >= 0 { - ip = strings.TrimSpace(ip[:pos]) + // See https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-For#selecting_an_ip_address + forwarded := strings.Split(strings.Join(r.Header.Values("X-Forwarded-For"), ","), ",") + if len(forwarded) > 0 { + 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 { - ip = host + + // If all entries in the "X-Forwarded-For" list are trusted, the left-most + // 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 diff --git a/hub_test.go b/hub_test.go index 2ebc64f..d8793c6 100644 --- a/hub_test.go +++ b/hub_test.go @@ -3596,59 +3596,159 @@ func TestJoinRoomSwitchClient(t *testing.T) { } func TestGetRealUserIP(t *testing.T) { - REMOTE_ATTR := "192.168.1.2" - - trustedProxies, err := ParseAllowedIps("192.168.0.0/16") - if err != nil { - t.Fatal(err) + testcases := []struct { + expected string + headers http.Header + trusted string + 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{ - RemoteAddr: REMOTE_ATTR + ":23456", - } - if ip := GetRealUserIP(request, trustedProxies); ip != REMOTE_ATTR { - t.Errorf("Expected %s but got %s", REMOTE_ATTR, ip) - } - - X_REAL_IP := "192.168.10.11" - request.Header = http.Header{ - http.CanonicalHeaderKey("x-real-ip"): []string{X_REAL_IP}, - } - if ip := GetRealUserIP(request, trustedProxies); ip != X_REAL_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) + for _, tc := range testcases { + trustedProxies, err := ParseAllowedIps(tc.trusted) + if err != nil { + t.Errorf("invalid trusted proxies in %+v: %s", tc, err) + continue + } + request := &http.Request{ + RemoteAddr: tc.addr, + Header: tc.headers, + } + if ip := GetRealUserIP(request, trustedProxies); ip != tc.expected { + t.Errorf("Expected %s for %+v but got %s", tc.expected, tc, ip) + } } } diff --git a/proxy.conf.in b/proxy.conf.in index 0f33cf5..5cbb3c4 100644 --- a/proxy.conf.in +++ b/proxy.conf.in @@ -9,7 +9,8 @@ #debug = false # 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. #trustedproxies = diff --git a/server.conf.in b/server.conf.in index 6b61b7b..b748cac 100644 --- a/server.conf.in +++ b/server.conf.in @@ -35,7 +35,8 @@ debug = false #allowsubscribeany = false # 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. #trustedproxies =