Merge pull request #482 from TorMap/codex/propose-fix-for-etag-vulnerability

Restrict shallow ETag filter routes

Author: JuliusHenke

backend/src/main/kotlin/org/tormap/config/AppConfig.kt

@@ -1,6 +1,7 @@
 package org.tormap.config
 
 import org.springframework.boot.context.properties.ConfigurationPropertiesScan
+import org.springframework.boot.web.servlet.FilterRegistrationBean
 import org.springframework.context.annotation.Bean
 import org.springframework.context.annotation.Configuration
 import org.springframework.scheduling.annotation.EnableAsync
@@ -30,12 +31,23 @@ class AppConfig : WebMvcConfigurer {
     }
 
     /**
-     * Include an unique ETag hash for each response to enable client side caching.
+     * Include a weak ETag hash only for small, bounded GET endpoints.
+     *
+     * Shallow ETags require Spring to buffer the full response body before it can be hashed.
+     * Keep this filter away from large public endpoints such as /relay/location/day/{day},
+     * where buffering an entire day's relay locations can amplify memory and CPU usage.
      */
     @Bean
-    fun shallowEtagHeaderFilter(): ShallowEtagHeaderFilter {
+    fun shallowEtagHeaderFilterRegistration(): FilterRegistrationBean<ShallowEtagHeaderFilter> {
         val filter = ShallowEtagHeaderFilter()
         filter.isWriteWeakETag = true
-        return filter
+
+        return FilterRegistrationBean(filter).apply {
+            addUrlPatterns(
+                "/relay/location/days",
+                "/relay/details/relay/*",
+                "/relay/details/family/*",
+            )
+        }
     }
 }

backend/src/test/kotlin/org/tormap/config/SecurityConfigTest.kt

@@ -1,15 +1,18 @@
 package org.tormap.config
 
 import io.kotest.core.spec.style.StringSpec
+import io.kotest.matchers.shouldBe
 import io.kotest.matchers.shouldNotBe
 import org.springframework.boot.test.context.SpringBootTest
+import org.springframework.boot.web.servlet.FilterRegistrationBean
 import org.springframework.context.ApplicationContext
 import org.springframework.security.core.userdetails.UserDetailsService
 import org.springframework.security.crypto.password.PasswordEncoder
 import org.springframework.security.web.SecurityFilterChain
 import org.springframework.test.context.ActiveProfiles
 import org.springframework.web.cors.CorsConfigurationSource
 import org.springframework.web.filter.ForwardedHeaderFilter
+import org.springframework.web.filter.ShallowEtagHeaderFilter
 
 @SpringBootTest
 @ActiveProfiles("test")
@@ -19,7 +22,8 @@ class SecurityConfigTest(
     private val userDetailsService: UserDetailsService,
     private val corsConfigurationSource: CorsConfigurationSource,
     private val forwardedHeaderFilter: ForwardedHeaderFilter,
-    private val securityFilterChain: SecurityFilterChain
+    private val securityFilterChain: SecurityFilterChain,
+    private val shallowEtagHeaderFilterRegistration: FilterRegistrationBean<ShallowEtagHeaderFilter>,
 ) : StringSpec({
 
     "security beans are loaded" {
@@ -31,4 +35,12 @@ class SecurityConfigTest(
         securityFilterChain shouldNotBe null
     }
 
+    "shallow ETag filter is not registered for large relay location responses" {
+        shallowEtagHeaderFilterRegistration.urlPatterns shouldBe setOf(
+            "/relay/location/days",
+            "/relay/details/relay/*",
+            "/relay/details/family/*",
+        )
+    }
+
 })

backend/src/test/kotlin/org/tormap/controller/HttpCachingConfigTest.kt

@@ -2,6 +2,8 @@ package org.tormap.controller
 
 import io.kotest.core.spec.style.StringSpec
 import org.hamcrest.Matchers.containsString
+import org.tormap.database.repository.RelayDetailsRepository
+import org.tormap.mockRelayDetails
 import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc
 import org.springframework.boot.test.context.SpringBootTest
 import org.springframework.test.context.ActiveProfiles
@@ -15,7 +17,37 @@ import org.springframework.test.web.servlet.result.MockMvcResultMatchers.status
 @ActiveProfiles("test")
 class HttpCachingConfigTest(
     private val mockMvc: MockMvc,
+    private val relayDetailsRepository: RelayDetailsRepository,
 ) : StringSpec({
+    val testRelay = relayDetailsRepository.save(
+        mockRelayDetails().apply {
+            familyId = 123L
+        }
+    )
+
+    "GET /relay/location/days includes ETag header" {
+        mockMvc.perform(get("/relay/location/days"))
+            .andExpect(status().isOk)
+            .andExpect(header().exists("ETag"))
+    }
+
+    "GET /relay/details/relay/{id} includes ETag header" {
+        mockMvc.perform(get("/relay/details/relay/${testRelay.id}"))
+            .andExpect(status().isOk)
+            .andExpect(header().exists("ETag"))
+    }
+
+    "GET /relay/details/family/{id} includes ETag header" {
+        mockMvc.perform(get("/relay/details/family/${testRelay.familyId}"))
+            .andExpect(status().isOk)
+            .andExpect(header().exists("ETag"))
+    }
+
+    "GET /relay/location/day/{day} does not include ETag header" {
+        mockMvc.perform(get("/relay/location/day/2022-02-04"))
+            .andExpect(status().isOk)
+            .andExpect(header().doesNotExist("ETag"))
+    }
 
     "GET /relay/location/days includes Cache-Control public max-age" {
         mockMvc.perform(get("/relay/location/days"))