From c4ba29848701597c6ad97e608c1e9db3ca0debaa Mon Sep 17 00:00:00 2001
From: Mateusz Reszka <mateusz@mateuszreszka.xyz>
Date: Tue, 6 May 2025 14:44:33 +0200
Subject: [PATCH] Get IP from "standard" HTTP headers #3287

Reference:
https://github.com/element-hq/dendrite/issues/3287

Signed-off-by: Mateusz Reszka <mateusz@mateuszreszka.xyz>
---
 internal/httputil/rate_limiting.go |  6 +----
 ip/remote.go                       | 40 ++++++++++++++++++++++++++++++
 syncapi/sync/requestpool.go        | 16 ++----------
 3 files changed, 43 insertions(+), 19 deletions(-)
 create mode 100644 ip/remote.go

diff --git a/internal/httputil/rate_limiting.go b/internal/httputil/rate_limiting.go
index e0870dce..dc8dbb37 100644
--- a/internal/httputil/rate_limiting.go
+++ b/internal/httputil/rate_limiting.go
@@ -88,11 +88,7 @@ func (l *RateLimits) Limit(req *http.Request, device *userapi.Device) *util.JSON
 			caller = device.UserID + device.ID
 		}
 	} else {
-		if forwardedFor := req.Header.Get("X-Forwarded-For"); forwardedFor != "" {
-			caller = forwardedFor
-		} else {
-			caller = req.RemoteAddr
-		}
+		caller = ip.GetRemoteHeader(req, "")
 	}
 
 	// Look up the caller's channel, if they have one.
diff --git a/ip/remote.go b/ip/remote.go
new file mode 100644
index 00000000..40de5316
--- /dev/null
+++ b/ip/remote.go
@@ -0,0 +1,40 @@
+package ip
+
+import (
+	"net"
+	"net/http"
+	"strings"
+)
+
+// Gets real IP of the user. Order:
+//   - 'X-Forwarded-For' - de facto standard, which is supported by majority of reverse proxies
+//   - a custom header defined in the params
+//   - req.RemoteAddr
+//
+// Returns single IP address
+func GetRemoteHeader(req *http.Request, customHeaderName string) string {
+	header := req.RemoteAddr
+
+	// TODO: to discuss the order of precedence
+	possibleIPHeaders := []string{
+		req.Header.Get("X-Forwarded-For"),
+		req.Header.Get(customHeaderName),
+		req.RemoteAddr,
+	}
+
+	// pick first with meaningful data
+	for _, v := range possibleIPHeaders {
+		if v != "" {
+			header = v
+			break
+		}
+	}
+
+	// sometimes you get multiple addresses
+	addresses := strings.Split(header, ",")
+	if ip := net.ParseIP(addresses[0]); ip != nil {
+		header = addresses[0]
+	}
+
+	return header
+}
diff --git a/syncapi/sync/requestpool.go b/syncapi/sync/requestpool.go
index 3b80d487..2c4aa8ad 100644
--- a/syncapi/sync/requestpool.go
+++ b/syncapi/sync/requestpool.go
@@ -23,6 +23,7 @@ import (
 	"github.com/sirupsen/logrus"
 
 	"github.com/element-hq/dendrite/internal/sqlutil"
+	"github.com/element-hq/dendrite/ip"
 	roomserverAPI "github.com/element-hq/dendrite/roomserver/api"
 	"github.com/element-hq/dendrite/setup/config"
 	"github.com/element-hq/dendrite/syncapi/internal"
@@ -229,23 +230,10 @@ func (rp *RequestPool) updateLastSeen(req *http.Request, device *userapi.Device)
 		return
 	}
 
-	remoteAddr := req.RemoteAddr
-	if rp.cfg.RealIPHeader != "" {
-		if header := req.Header.Get(rp.cfg.RealIPHeader); header != "" {
-			// TODO: Maybe this isn't great but it will satisfy both X-Real-IP
-			// and X-Forwarded-For (which can be a list where the real client
-			// address is the first listed address). Make more intelligent?
-			addresses := strings.Split(header, ",")
-			if ip := net.ParseIP(addresses[0]); ip != nil {
-				remoteAddr = addresses[0]
-			}
-		}
-	}
-
 	lsreq := &userapi.PerformLastSeenUpdateRequest{
 		UserID:     device.UserID,
 		DeviceID:   device.ID,
-		RemoteAddr: remoteAddr,
+		RemoteAddr: ip.GetRemoteHeader(req, rp.cfg.RealIPHeader),
 		UserAgent:  req.UserAgent(),
 	}
 	lsres := &userapi.PerformLastSeenUpdateResponse{}