diff --git a/CHANGELOG.md b/CHANGELOG.md index 7a7348c..f7c5901 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,24 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and ## [Unreleased] +## [1.5.0] - 2026-10-28 + +### Deprecated + +- `logged.client-info.trust-forwarded-headers` (and `LoggedProperties.ClientInfo#isTrustForwardedHeaders`/`setTrustForwardedHeaders`). It trusts the leftmost `X-Forwarded-For` entry, which can be spoofed behind a proxy that appends to the header. Use Spring Boot's `server.forward-headers-strategy=native` instead: it resolves the client address from the right, skipping trusted proxies, and rewrites `getRemoteAddr()` for the whole application, which this library already uses with the setting off. The setting keeps working unchanged; while it is enabled, a single warning is logged at startup. + +### Added + +- Spring Boot configuration metadata (`META-INF/spring-configuration-metadata.json`) generated by `spring-boot-configuration-processor`, so every `logged.*` property now gets IDE completion, a description and its default value, and the deprecated property is flagged with its replacement. The processor runs at build time only; it is not a runtime dependency and is not passed on to applications. + +### Documentation + +- New README section "Behind a reverse proxy": why the leftmost `X-Forwarded-For` entry cannot be trusted, how to configure `server.forward-headers-strategy=native` and `server.tomcat.remoteip.internal-proxies`, when `framework` is appropriate, and how to verify the setup. + +### Compatibility + +- No behavior changes: with or without the deprecated setting, caller IP resolution works exactly as in 1.4.1. The only new output is the one-time startup warning when the deprecated setting is enabled. + ## [1.4.1] - 2026-10-28 ### Security diff --git a/README.md b/README.md index 952b477..f2af4e2 100644 --- a/README.md +++ b/README.md @@ -489,7 +489,7 @@ Every property below is optional and defaults to preserving the library's out-of |---|---|---| | `logged.enabled` | `true` | Global switch. Setting this to `false` removes the aspect entirely; `@Logged` methods run uninstrumented. | | `logged.metrics.enabled` | `true` | Setting this to `false` falls back to a no-op metrics recorder even when a `MeterRegistry` bean is present. | -| `logged.client-info.trust-forwarded-headers` | `false` | Whether the caller-IP resolver may trust the client-controlled `X-Forwarded-For` header. Only enable this after confirming the application sits behind a proxy that strips and re-sets this header itself; do **not** enable it behind a proxy that appends to the header — see [Caller resolution](#caller-resolution). | +| `logged.client-info.trust-forwarded-headers` | `false` | **Deprecated.** Makes the library read the client-controlled `X-Forwarded-For` header itself. Prefer Spring Boot's `server.forward-headers-strategy=native` and keep this off — see [Behind a reverse proxy](#behind-a-reverse-proxy). | | `logged.client-info.include-device` | `false` | Records a coarse device description for **every** `@Logged` method, not only those with `@Logged(includeDevice = true)`. Keep it off and opt in per method — see [Recording the caller's device](#recording-the-callers-device). | | `logged.client-info.caller` | `name` | How an authenticated caller's name is written: `name` (as is), `masked` (e-mail addresses become `j***@example.com`), or `custom` (a `CallerIdentityResolver` bean decides). A single method can override it with `@Logged(caller = ...)` — see [Masking the caller's e-mail (GDPR / KVKK)](#masking-the-callers-e-mail-gdpr--kvkk). | | `logged.mdc.enabled` | `true` | Whether `logged.traceId`/`logged.depth`/`logged.className`/`logged.methodName` are written to the current thread's MDC during a `@Logged` call — see [Structured logging (MDC)](#structured-logging-mdc). | @@ -497,7 +497,9 @@ Every property below is optional and defaults to preserving the library's out-of ```yaml logged: client-info: - trust-forwarded-headers: true + caller: masked + metrics: + enabled: true ``` ## Modules @@ -540,9 +542,41 @@ The default `IClientInfoPort` (active whenever Spring Security and Spring Web ar 2. Otherwise, the current HTTP request's `getRemoteAddr()` (`ip:203.0.113.5`). 3. `unknown`, if neither is available (for example, a scheduled job with no active request). -The client IP is read from `getRemoteAddr()`, **not** from `X-Forwarded-For`, by default. That header is controlled by the client and can be forged by anyone unless the application sits behind a proxy configured to strip and re-set it — a deployment detail this library cannot know on its own. If your application does sit behind such a proxy, set `logged.client-info.trust-forwarded-headers=true` explicitly, mirroring how Spring Security itself requires trusted proxies to be declared rather than assumed. +The client IP is read from `getRemoteAddr()`, **not** from `X-Forwarded-For`, by default. That header is controlled by the client and can be forged by anyone, so this library does not trust it on its own. Behind a reverse proxy, let Spring Boot resolve the real client address instead — see [Behind a reverse proxy](#behind-a-reverse-proxy). + +### Behind a reverse proxy + +Behind nginx, a cloud load balancer or a Kubernetes ingress, `getRemoteAddr()` is the proxy's address, not the user's. The user's address travels in `X-Forwarded-For`, which grows as the request passes each proxy: + +``` +X-Forwarded-For: , , +``` + +The entries on the right are written by your own proxies and can be trusted; the leftmost one can be anything the client chose to send. The correct client address is therefore found by reading **from the right**, skipping your own proxies. + +Spring Boot already does exactly this, and does it for the whole application rather than only for logging. Enable it in the application, keep `logged.client-info.trust-forwarded-headers` off, and this library picks up the resolved address automatically through `getRemoteAddr()`: + +```yaml +server: + forward-headers-strategy: native # Tomcat's RemoteIpValve resolves X-Forwarded-For + tomcat: + remoteip: + # Which addresses are your own proxies. The default already covers private + # networks (10.x, 172.16-31.x, 192.168.x, loopback, link-local), which fits + # nginx on the same host, an in-cluster ingress or a load balancer in your VPC. + # Override it only when a proxy has a public address (for example a CDN). + internal-proxies: "10\\.\\d{1,3}\\.\\d{1,3}\\.\\d{1,3}|192\\.168\\.\\d{1,3}\\.\\d{1,3}" +``` + +- With `native`, only proxies matching `internal-proxies` are skipped, so a forged leftmost entry is never taken for the client. On a recognized cloud platform Spring Boot enables this automatically. +- `forward-headers-strategy: framework` (Spring's `ForwardedHeaderFilter`) also rewrites the request, but it trusts the forwarded headers as sent. Use it only when every request is guaranteed to pass through a proxy that replaces those headers. +- Because Spring rewrites the request itself, everything that reads the client address — Spring Security, rate limiting, your own code and this library — sees the same, correct value. + +To check the setup, call a method annotated with `@Logged(includeIp = true)` through the proxy: the logged `ip=` must be your own address, not the proxy's. + +#### `logged.client-info.trust-forwarded-headers` (deprecated) -When enabled, the **first** (leftmost) `X-Forwarded-For` entry is used. That is only correct when your proxy replaces the header. Many proxies append to it instead (for example nginx with `$proxy_add_x_forwarded_for`, or a load balancer's default behavior); then the leftmost entry is whatever the client sent, and the logged IP can be spoofed. In that setup, leave this setting off. As a safety net, an entry that is not shaped like an IP address (only digits, hex letters, `.` and `:`, at most 45 characters) is ignored and `getRemoteAddr()` is used instead, so a forged header can never write arbitrary text into a log line. +This older setting makes the library read `X-Forwarded-For` itself, using the **first** (leftmost) entry. That is only correct when the proxy replaces the header; behind a proxy that appends to it (for example nginx with `$proxy_add_x_forwarded_for`, or a typical load balancer), the leftmost entry is whatever the client sent and the logged IP can be spoofed. It keeps working exactly as before, but is deprecated in favor of the setup above, and the application logs a one-time warning at startup while it is enabled. As a safety net, an entry that is not shaped like an IP address (only digits, hex letters, `.` and `:`, at most 45 characters) is ignored and `getRemoteAddr()` is used instead, so a forged header can never write arbitrary text into a log line. **Log injection.** Values that can originate outside the application — the caller identity (for example a username chosen at registration, a masked e-mail's domain, or a `CallerIdentityResolver` result), the IP and the device — are kept on a single line in the SLF4J summary line and in the MDC: line breaks and other control characters are written as visible escapes such as `\n` instead of starting a forged log line. Ordinary values are written unchanged. The JSON emitter already escapes every value. Trace depths received over HTTP, Kafka or RabbitMQ are clamped to `0..1000`. diff --git a/logged-benchmarks/pom.xml b/logged-benchmarks/pom.xml index 8153654..68f2e7a 100644 --- a/logged-benchmarks/pom.xml +++ b/logged-benchmarks/pom.xml @@ -7,7 +7,7 @@ com.fayupable logged-lib - 1.4.1 + 1.5.0 logged-benchmarks diff --git a/logged-core/pom.xml b/logged-core/pom.xml index 6e95b53..8299b07 100644 --- a/logged-core/pom.xml +++ b/logged-core/pom.xml @@ -7,7 +7,7 @@ com.fayupable logged-lib - 1.4.1 + 1.5.0 logged-core diff --git a/logged-spring/pom.xml b/logged-spring/pom.xml index bc4182f..20f5041 100644 --- a/logged-spring/pom.xml +++ b/logged-spring/pom.xml @@ -7,7 +7,7 @@ com.fayupable logged-lib - 1.4.1 + 1.5.0 logged-spring @@ -123,6 +123,23 @@ + + org.apache.maven.plugins + maven-compiler-plugin + + + + + org.springframework.boot + spring-boot-configuration-processor + ${spring-boot.version} + + + + org.apache.maven.plugins maven-surefire-plugin diff --git a/logged-spring/src/main/java/com/fayupable/logged/spring/config/LoggedAutoConfiguration.java b/logged-spring/src/main/java/com/fayupable/logged/spring/config/LoggedAutoConfiguration.java index 1bf99a1..2f7f1a8 100644 --- a/logged-spring/src/main/java/com/fayupable/logged/spring/config/LoggedAutoConfiguration.java +++ b/logged-spring/src/main/java/com/fayupable/logged/spring/config/LoggedAutoConfiguration.java @@ -16,6 +16,8 @@ import com.fayupable.logged.spring.security.SpringSecurityClientInfoAdapter; import io.micrometer.core.instrument.MeterRegistry; import jakarta.servlet.http.HttpServletRequest; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springframework.beans.factory.ObjectProvider; import org.springframework.boot.autoconfigure.AutoConfiguration; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; @@ -61,6 +63,8 @@ @EnableConfigurationProperties(LoggedProperties.class) public class LoggedAutoConfiguration { + private static final Logger log = LoggerFactory.getLogger(LoggedAutoConfiguration.class); + /** * Registered as a {@code static} bean method, per Spring's own guidance * for {@link org.springframework.beans.factory.config.BeanPostProcessor} @@ -92,14 +96,14 @@ public IClientInfoPort springSecurityClientInfoAdapter(LoggedProperties properti throw new IllegalStateException("logged.client-info.caller=custom requires a " + CallerIdentityResolver.class.getName() + " bean, but none is defined"); } - return new SpringSecurityClientInfoAdapter(clientInfo.isTrustForwardedHeaders(), clientInfo.getCaller(), resolver); + return new SpringSecurityClientInfoAdapter(trustForwardedHeaders(properties), clientInfo.getCaller(), resolver); } @Bean @ConditionalOnMissingBean(IClientInfoPort.class) @ConditionalOnClass(HttpServletRequest.class) public IClientInfoPort httpRequestClientInfoAdapter(LoggedProperties properties) { - return new HttpRequestClientInfoAdapter(properties.getClientInfo().isTrustForwardedHeaders()); + return new HttpRequestClientInfoAdapter(trustForwardedHeaders(properties)); } @Bean @@ -130,4 +134,21 @@ public LoggedAspect loggedAspect(InvocationEventEmitter eventEmitter, MetricsRec return new LoggedAspect(eventEmitter, metricsRecorder, clientInfoPort, properties.getMdc().isEnabled(), properties.getClientInfo().isIncludeDevice()); } -} \ No newline at end of file + + /** + * Reads the deprecated {@code logged.client-info.trust-forwarded-headers} + * setting, which is still honored, and logs a one-time warning when it is + * enabled. Only one client info adapter bean is ever created, so the + * warning appears once per application context. + */ + @SuppressWarnings("deprecation") // deprecated but still honored; this is its single read site + private static boolean trustForwardedHeaders(LoggedProperties properties) { + boolean trusted = properties.getClientInfo().isTrustForwardedHeaders(); + if (trusted) { + log.warn("logged.client-info.trust-forwarded-headers is deprecated: it trusts the leftmost " + + "X-Forwarded-For entry, which can be spoofed behind a proxy that appends to the header. " + + "Set server.forward-headers-strategy=native instead and turn this setting off."); + } + return trusted; + } +} diff --git a/logged-spring/src/main/java/com/fayupable/logged/spring/config/LoggedProperties.java b/logged-spring/src/main/java/com/fayupable/logged/spring/config/LoggedProperties.java index acf5acb..9dc8096 100644 --- a/logged-spring/src/main/java/com/fayupable/logged/spring/config/LoggedProperties.java +++ b/logged-spring/src/main/java/com/fayupable/logged/spring/config/LoggedProperties.java @@ -2,6 +2,7 @@ import com.fayupable.logged.core.annotation.CallerMode; import org.springframework.boot.context.properties.ConfigurationProperties; +import org.springframework.boot.context.properties.DeprecatedConfigurationProperty; /** * Configuration properties for this library, bound under the {@code logged} @@ -64,6 +65,14 @@ public static class ClientInfo { * confirming that such a proxy is actually in place; see * {@link com.fayupable.logged.spring.security.SpringSecurityClientInfoAdapter} * for the full explanation. + * + *

Deprecated since 1.5.0: behind a proxy that appends to the + * header the leftmost entry this setting uses can be spoofed. Prefer + * Spring Boot's {@code server.forward-headers-strategy=native}, which + * resolves the client address from the right, skipping trusted + * proxies, and rewrites {@code getRemoteAddr()} for the whole + * application; this library then picks it up with this setting off. + * The setting keeps working unchanged. */ private boolean trustForwardedHeaders = false; @@ -96,10 +105,17 @@ public static class ClientInfo { */ private boolean includeDevice = false; + @Deprecated(since = "1.5.0") + @DeprecatedConfigurationProperty( + reason = "The leftmost X-Forwarded-For entry can be spoofed behind a proxy that appends to the " + + "header. Let Spring Boot resolve the client address instead and keep this off.", + replacement = "server.forward-headers-strategy", + since = "1.5.0") public boolean isTrustForwardedHeaders() { return trustForwardedHeaders; } + @Deprecated(since = "1.5.0") public void setTrustForwardedHeaders(boolean trustForwardedHeaders) { this.trustForwardedHeaders = trustForwardedHeaders; } diff --git a/logged-spring/src/test/java/com/fayupable/logged/spring/config/LoggedAutoConfigurationTest.java b/logged-spring/src/test/java/com/fayupable/logged/spring/config/LoggedAutoConfigurationTest.java index 0cab946..ffac875 100644 --- a/logged-spring/src/test/java/com/fayupable/logged/spring/config/LoggedAutoConfigurationTest.java +++ b/logged-spring/src/test/java/com/fayupable/logged/spring/config/LoggedAutoConfigurationTest.java @@ -1,5 +1,13 @@ package com.fayupable.logged.spring.config; +import org.springframework.web.context.request.ServletRequestAttributes; +import org.springframework.web.context.request.RequestContextHolder; +import org.springframework.mock.web.MockHttpServletRequest; +import org.slf4j.LoggerFactory; +import org.junit.jupiter.api.BeforeEach; +import ch.qos.logback.core.read.ListAppender; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.classic.Level; import org.springframework.aop.aspectj.annotation.AspectJProxyFactory; import com.fayupable.logged.core.model.MethodInvocationEvent; import com.fayupable.logged.core.annotation.Logged; @@ -345,4 +353,87 @@ void recordsDeviceWhenEnabled() { .run(context -> assertThat(deviceOfPlainCall(context)).isEqualTo("Windows / Chrome 129")); } } + + @Nested + @DisplayName("deprecated logged.client-info.trust-forwarded-headers") + class DeprecatedTrustForwardedHeaders { + + private ListAppender appender; + private ch.qos.logback.classic.Logger configLogger; + + @BeforeEach + void attachAppender() { + configLogger = (ch.qos.logback.classic.Logger) LoggerFactory.getLogger(LoggedAutoConfiguration.class); + appender = new ListAppender<>(); + appender.start(); + configLogger.addAppender(appender); + } + + @AfterEach + void detachAppender() { + configLogger.detachAppender(appender); + } + + private List deprecationWarnings() { + return appender.list.stream() + .filter(event -> event.getLevel() == Level.WARN) + .filter(event -> event.getFormattedMessage().contains("trust-forwarded-headers is deprecated")) + .toList(); + } + + @Test + @DisplayName("logs one startup warning when enabled, and still honors the setting") + @SuppressWarnings("deprecation") + void warnsOnceWhenEnabled() { + contextRunner.withPropertyValues("logged.client-info.trust-forwarded-headers=true").run(context -> { + assertThat(context).hasSingleBean(IClientInfoPort.class); + assertThat(context.getBean(LoggedProperties.class).getClientInfo().isTrustForwardedHeaders()).isTrue(); + assertThat(deprecationWarnings()).hasSize(1); + assertThat(deprecationWarnings().get(0).getFormattedMessage()) + .contains("server.forward-headers-strategy=native"); + }); + } + + @Test + @DisplayName("also warns once for the HTTP-only adapter") + void warnsOnceForHttpOnlyAdapter() { + contextRunner + .withClassLoader(new FilteredClassLoader(Authentication.class)) + .withPropertyValues("logged.client-info.trust-forwarded-headers=true") + .run(context -> { + assertThat(context.getBean(IClientInfoPort.class)).isInstanceOf(HttpRequestClientInfoAdapter.class); + assertThat(deprecationWarnings()).hasSize(1); + }); + } + + @Test + @DisplayName("passes the setting to the adapter: X-Forwarded-For is used only when enabled") + void passesSettingToAdapter() { + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setRemoteAddr("203.0.113.5"); + request.addHeader("X-Forwarded-For", "198.51.100.99"); + RequestContextHolder.setRequestAttributes(new ServletRequestAttributes(request)); + try { + contextRunner.withPropertyValues("logged.client-info.trust-forwarded-headers=true").run(context -> + assertThat(context.getBean(IClientInfoPort.class).resolveCallerIp()).isEqualTo("198.51.100.99")); + contextRunner.run(context -> + assertThat(context.getBean(IClientInfoPort.class).resolveCallerIp()).isEqualTo("203.0.113.5")); + } finally { + RequestContextHolder.resetRequestAttributes(); + } + } + + @Test + @DisplayName("logs nothing when the setting is left at its default") + void silentByDefault() { + contextRunner.run(context -> assertThat(deprecationWarnings()).isEmpty()); + } + + @Test + @DisplayName("logs nothing when the setting is explicitly false") + void silentWhenFalse() { + contextRunner.withPropertyValues("logged.client-info.trust-forwarded-headers=false") + .run(context -> assertThat(deprecationWarnings()).isEmpty()); + } + } } diff --git a/logged-test/pom.xml b/logged-test/pom.xml index 2a9ef0f..b30284b 100644 --- a/logged-test/pom.xml +++ b/logged-test/pom.xml @@ -7,7 +7,7 @@ com.fayupable logged-lib - 1.4.1 + 1.5.0 logged-test diff --git a/pom.xml b/pom.xml index 40cb957..8e5f673 100644 --- a/pom.xml +++ b/pom.xml @@ -6,7 +6,7 @@ com.fayupable logged-lib - 1.4.1 + 1.5.0 pom logged-lib