fix: use X-Real-IP instead of X-Forwarded-For to prevent rate limit bypass

X-Forwarded-For can be spoofed by clients since nginx's
$proxy_add_x_forwarded_for appends to the client-supplied value.
X-Real-IP is set by nginx to $remote_addr which overwrites any
client-supplied header, making it immune to spoofing.

Author: d4rken

.gitignore

@@ -18,6 +18,8 @@ out/
 !**/src/main/**/out/
 !**/src/test/**/out/
 
+.kotlin
+
 ### Eclipse ###
 .apt_generated
 .classpath

.idea/kotlinc.xml

@@ -5,7 +5,7 @@ import io.ktor.server.request.*
 fun ApplicationRequest.clientIp(): String {
     val connectionIp = local.remoteAddress
     return if (IpHelper.isLoopback(connectionIp)) {
-        headers["X-Forwarded-For"]?.split(",")?.firstOrNull()?.trim() ?: connectionIp
+        headers["X-Real-IP"]?.trim() ?: connectionIp
     } else {
         connectionIp
     }

src/main/kotlin/eu/darken/octi/kserver/common/IpHelper.kt

@@ -11,13 +11,13 @@ class ClientIpTest {
 
     private fun createRequest(
         remoteAddress: String,
-        forwardedFor: String? = null,
+        realIp: String? = null,
     ): ApplicationRequest {
         val local = mockk<RequestConnectionPoint> {
             every { this@mockk.remoteAddress } returns remoteAddress
         }
         val headers = HeadersBuilder().apply {
-            if (forwardedFor != null) append("X-Forwarded-For", forwardedFor)
+            if (realIp != null) append("X-Real-IP", realIp)
         }.build()
         return mockk {
             every { this@mockk.local } returns local
@@ -32,31 +32,25 @@ class ClientIpTest {
     }
 
     @Test
-    fun `non-loopback ignores X-Forwarded-For`() {
-        val request = createRequest(remoteAddress = "203.0.113.5", forwardedFor = "10.0.0.1")
+    fun `non-loopback ignores X-Real-IP`() {
+        val request = createRequest(remoteAddress = "203.0.113.5", realIp = "10.0.0.1")
         request.clientIp() shouldBe "203.0.113.5"
     }
 
     @Test
-    fun `loopback IPv4 uses X-Forwarded-For`() {
-        val request = createRequest(remoteAddress = "127.0.0.1", forwardedFor = "198.51.100.7")
+    fun `loopback IPv4 uses X-Real-IP`() {
+        val request = createRequest(remoteAddress = "127.0.0.1", realIp = "198.51.100.7")
         request.clientIp() shouldBe "198.51.100.7"
     }
 
     @Test
-    fun `loopback IPv6 uses X-Forwarded-For`() {
-        val request = createRequest(remoteAddress = "0:0:0:0:0:0:0:1", forwardedFor = "198.51.100.7")
+    fun `loopback IPv6 uses X-Real-IP`() {
+        val request = createRequest(remoteAddress = "0:0:0:0:0:0:0:1", realIp = "198.51.100.7")
         request.clientIp() shouldBe "198.51.100.7"
     }
 
     @Test
-    fun `loopback takes first IP from X-Forwarded-For chain`() {
-        val request = createRequest(remoteAddress = "127.0.0.1", forwardedFor = "198.51.100.7, 10.0.0.1")
-        request.clientIp() shouldBe "198.51.100.7"
-    }
-
-    @Test
-    fun `loopback without X-Forwarded-For falls back to connection IP`() {
+    fun `loopback without X-Real-IP falls back to connection IP`() {
         val request = createRequest(remoteAddress = "127.0.0.1")
         request.clientIp() shouldBe "127.0.0.1"
     }

src/test/kotlin/eu/darken/octi/kserver/common/ClientIpTest.kt

@@ -58,15 +58,15 @@ class RateLimiterTest : TestRunner() {
     }
 
     @Test
-    fun `test different X-Forwarded-For from trusted proxy have separate rate limits`() = runTest2(
+    fun `test different X-Real-IP from trusted proxy have separate rate limits`() = runTest2(
         appConfig = baseConfig.copy(
             rateLimit = RateLimitConfig(limit = 2, resetTime = Duration.ofSeconds(5))
         )
     ) {
-        // Tests connect via loopback (trusted proxy), so X-Forwarded-For is used
+        // Tests connect via loopback (trusted proxy), so X-Real-IP is used
         repeat(2) {
             http.get("/v1/status") {
-                header("X-Forwarded-For", "192.168.1.1")
+                header("X-Real-IP", "192.168.1.1")
             }.apply {
                 status shouldBe HttpStatusCode.OK
             }
@@ -75,7 +75,7 @@ class RateLimiterTest : TestRunner() {
 
         // 192.168.1.1 is exhausted
         http.get("/v1/status") {
-            header("X-Forwarded-For", "192.168.1.1")
+            header("X-Real-IP", "192.168.1.1")
         }.apply {
             status shouldBe HttpStatusCode.TooManyRequests
         }
@@ -84,7 +84,7 @@ class RateLimiterTest : TestRunner() {
         // 192.168.1.2 should still have its own limit
         repeat(2) {
             http.get("/v1/status") {
-                header("X-Forwarded-For", "192.168.1.2")
+                header("X-Real-IP", "192.168.1.2")
             }.apply {
                 status shouldBe HttpStatusCode.OK
             }
@@ -101,15 +101,15 @@ class RateLimiterTest : TestRunner() {
         // Exhaust rate limit for an IP
         repeat(2) {
             http.get("/v1/status") {
-                header("X-Forwarded-For", "192.168.1.1")
+                header("X-Real-IP", "192.168.1.1")
             }.apply {
                 status shouldBe HttpStatusCode.OK
             }
             Thread.sleep(100)
         }
 
         http.get("/v1/status") {
-            header("X-Forwarded-For", "192.168.1.1")
+            header("X-Real-IP", "192.168.1.1")
         }.apply {
             status shouldBe HttpStatusCode.TooManyRequests
         }
@@ -121,7 +121,7 @@ class RateLimiterTest : TestRunner() {
         // Should be able to make requests again since entries were cleaned up
         repeat(2) {
             http.get("/v1/status") {
-                header("X-Forwarded-For", "192.168.1.1")
+                header("X-Real-IP", "192.168.1.1")
             }.apply {
                 status shouldBe HttpStatusCode.OK
             }