Merge pull request #44184 from cescoffier/x-forwarded-trusted-proxy-h…

…eader Add Support for Trusted Proxy Detection on Forwarded Requests
quarkusio · Oct 30, 2024 · 10f5926 · 10f5926
2 parents a6d6cd7 + 5069761
commit 10f5926
Show file tree

Hide file tree

Showing 7 changed files with 230 additions and 9 deletions.
diff --git a/...ertx-http/deployment/src/test/java/io/quarkus/vertx/http/ForwardedHandlerInitializer.java b/...ertx-http/deployment/src/test/java/io/quarkus/vertx/http/ForwardedHandlerInitializer.java
@@ -19,6 +19,17 @@ public void register(@Observes Router router) {
                         + "|" + rc.request().remoteAddress().toString()
                         + "|" + rc.request().uri()
                         + "|" + rc.request().absoluteURI()));
+        router.route("/trusted-proxy").handler(rc -> rc.response()
+                .end(rc.request().scheme() + "|" + rc.request().getHeader(HttpHeaders.HOST) + "|"
+                        + rc.request().remoteAddress().toString()
+                        + "|" + rc.request().getHeader("X-Forwarded-Trusted-Proxy")));
+        router.route("/path-trusted-proxy").handler(rc -> rc.response()
+                .end(rc.request().scheme()
+                        + "|" + rc.request().getHeader(HttpHeaders.HOST)
+                        + "|" + rc.request().remoteAddress().toString()
+                        + "|" + rc.request().uri()
+                        + "|" + rc.request().absoluteURI()
+                        + "|" + rc.request().getHeader("X-Forwarded-Trusted-Proxy")));
     }
 
 }
diff --git a/...nsions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/ForwardedHeaderTest.java b/...nsions/vertx-http/deployment/src/test/java/io/quarkus/vertx/http/ForwardedHeaderTest.java
@@ -30,6 +30,41 @@ public void test() {
                 .body(Matchers.equalTo("https|somehost|backend:4444"));
     }
 
+    @Test
+    public void testWithoutTrustedProxyHeader() {
+        assertThat(RestAssured.get("/forward").asString()).startsWith("http|");
+        RestAssured.given()
+                .header("Forwarded", "by=proxy;for=backend:4444;host=somehost;proto=https")
+                .get("/trusted-proxy")
+                .then()
+                .body(Matchers.equalTo("https|somehost|backend:4444|null"));
+    }
+
+    @Test
+    public void testThatTrustedProxyHeaderCannotBeForged() {
+        assertThat(RestAssured.get("/forward").asString()).startsWith("http|");
+        RestAssured.given()
+                .header("Forwarded", "by=proxy;for=backend:4444;host=somehost;proto=https")
+                .header("X-Forwarded-Trusted-Proxy", "true")
+                .get("/trusted-proxy")
+                .then()
+                .body(Matchers.equalTo("https|somehost|backend:4444|null"));
+
+        RestAssured.given()
+                .header("Forwarded", "by=proxy;for=backend:4444;host=somehost;proto=https")
+                .header("X-Forwarded-Trusted-Proxy", "hello")
+                .get("/trusted-proxy")
+                .then()
+                .body(Matchers.equalTo("https|somehost|backend:4444|null"));
+
+        RestAssured.given()
+                .header("Forwarded", "by=proxy;for=backend:4444;host=somehost;proto=https")
+                .header("X-Forwarded-Trusted-Proxy", "false")
+                .get("/trusted-proxy")
+                .then()
+                .body(Matchers.equalTo("https|somehost|backend:4444|null"));
+    }
+
     @Test
     public void testForwardedForWithSequenceOfProxies() {
         assertThat(RestAssured.get("/forward").asString()).startsWith("http|");

diff --git a/...-http/deployment/src/test/java/io/quarkus/vertx/http/proxy/TrustedForwarderProxyTest.java b/...-http/deployment/src/test/java/io/quarkus/vertx/http/proxy/TrustedForwarderProxyTest.java
@@ -1,5 +1,7 @@
 package io.quarkus.vertx.http.proxy;
 
+import static org.assertj.core.api.Assertions.assertThat;
+
 import org.hamcrest.Matchers;
 import org.jboss.shrinkwrap.api.asset.StringAsset;
 import org.junit.jupiter.api.Test;
@@ -31,6 +33,51 @@ public void testHeadersAreUsed() {
                 .body(Matchers.equalTo("http|somehost2|backend2:5555|/path|http://somehost2/path"));
     }
 
+    @Test
+    public void testHeadersAreUsedWithTrustedProxyHeader() {
+        RestAssured.given()
+                .header("Forwarded", "proto=http;for=backend2:5555;host=somehost2")
+                .get("/path-trusted-proxy")
+                .then()
+                .body(Matchers
+                        .equalTo("http|somehost2|backend2:5555|/path-trusted-proxy|http://somehost2/path-trusted-proxy|null"));
+    }
+
+    @Test
+    public void testWithoutTrustedProxyHeader() {
+        assertThat(RestAssured.get("/forward").asString()).startsWith("http|");
+        RestAssured.given()
+                .header("Forwarded", "by=proxy;for=backend:4444;host=somehost;proto=https")
+                .get("/trusted-proxy")
+                .then()
+                .body(Matchers.equalTo("https|somehost|backend:4444|null"));
+    }
+
+    @Test
+    public void testThatTrustedProxyHeaderCannotBeForged() {
+        assertThat(RestAssured.get("/forward").asString()).startsWith("http|");
+        RestAssured.given()
+                .header("Forwarded", "by=proxy;for=backend:4444;host=somehost;proto=https")
+                .header("X-Forwarded-Trusted-Proxy", "true")
+                .get("/trusted-proxy")
+                .then()
+                .body(Matchers.equalTo("https|somehost|backend:4444|null"));
+
+        RestAssured.given()
+                .header("Forwarded", "by=proxy;for=backend:4444;host=somehost;proto=https")
+                .header("X-Forwarded-Trusted-Proxy", "hello")
+                .get("/trusted-proxy")
+                .then()
+                .body(Matchers.equalTo("https|somehost|backend:4444|null"));
+
+        RestAssured.given()
+                .header("Forwarded", "by=proxy;for=backend:4444;host=somehost;proto=https")
+                .header("X-Forwarded-Trusted-Proxy", "false")
+                .get("/trusted-proxy")
+                .then()
+                .body(Matchers.equalTo("https|somehost|backend:4444|null"));
+    }
+
     /**
      * As described on <a href=
      * "https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Forwarded">https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Forwarded</a>,

diff --git a/...rtx-http/deployment/src/test/java/io/quarkus/vertx/http/proxy/TrustedProxyHeaderTest.java b/...rtx-http/deployment/src/test/java/io/quarkus/vertx/http/proxy/TrustedProxyHeaderTest.java
@@ -0,0 +1,95 @@
+package io.quarkus.vertx.http.proxy;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import org.hamcrest.Matchers;
+import org.jboss.shrinkwrap.api.asset.StringAsset;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import io.quarkus.test.QuarkusUnitTest;
+import io.quarkus.vertx.http.ForwardedHandlerInitializer;
+import io.restassured.RestAssured;
+
+/**
+ * Test the trusted-proxy header
+ */
+public class TrustedProxyHeaderTest {
+
+    @RegisterExtension
+    static final QuarkusUnitTest config = new QuarkusUnitTest()
+            .withApplicationRoot((jar) -> jar
+                    .addClasses(ForwardedHandlerInitializer.class)
+                    .addAsResource(new StringAsset("""
+                            quarkus.http.proxy.proxy-address-forwarding=true
+                            quarkus.http.proxy.allow-forwarded=true
+                            quarkus.http.proxy.enable-forwarded-host=true
+                            quarkus.http.proxy.enable-forwarded-prefix=true
+                            quarkus.http.proxy.allow-forwarded=true
+                            quarkus.http.proxy.enable-trusted-proxy-header=true
+                            quarkus.http.proxy.trusted-proxies=localhost
+                            """),
+                            "application.properties"));
+
+    @Test
+    public void testHeadersAreUsed() {
+        RestAssured.given()
+                .header("Forwarded", "proto=http;for=backend2:5555;host=somehost2")
+                .get("/path-trusted-proxy")
+                .then()
+                .body(Matchers
+                        .equalTo("http|somehost2|backend2:5555|/path-trusted-proxy|http://somehost2/path-trusted-proxy|true"));
+    }
+
+    @Test
+    public void testTrustedProxyHeader() {
+        assertThat(RestAssured.get("/forward").asString()).startsWith("http|");
+        RestAssured.given()
+                .header("Forwarded", "by=proxy;for=backend:4444;host=somehost;proto=https")
+                .get("/trusted-proxy")
+                .then()
+                .body(Matchers.equalTo("https|somehost|backend:4444|true"));
+    }
+
+    @Test
+    public void testThatTrustedProxyHeaderCannotBeForged() {
+        assertThat(RestAssured.get("/forward").asString()).startsWith("http|");
+        RestAssured.given()
+                .header("Forwarded", "by=proxy;for=backend:4444;host=somehost;proto=https")
+                .header("X-Forwarded-Trusted-Proxy", "true")
+                .get("/trusted-proxy")
+                .then()
+                .body(Matchers.equalTo("https|somehost|backend:4444|true"));
+
+        RestAssured.given()
+                .header("Forwarded", "by=proxy;for=backend:4444;host=somehost;proto=https")
+                .header("X-Forwarded-Trusted-Proxy", "hello")
+                .get("/trusted-proxy")
+                .then()
+                .body(Matchers.equalTo("https|somehost|backend:4444|true"));
+
+        RestAssured.given()
+                .header("Forwarded", "by=proxy;for=backend:4444;host=somehost;proto=https")
+                .header("X-Forwarded-Trusted-Proxy", "false")
+                .get("/trusted-proxy")
+                .then()
+                .body(Matchers.equalTo("https|somehost|backend:4444|true"));
+    }
+
+    /**
+     * As described on <a href=
+     * "https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Forwarded">https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Forwarded</a>,
+     * the syntax should be case-insensitive.
+     * <p>
+     * Kong, for example, uses `Proto` instead of `proto` and `For` instead of `for`.
+     */
+    @Test
+    public void testHeadersAreUsedWhenUsingCasedCharacters() {
+        RestAssured.given()
+                .header("Forwarded", "Proto=http;For=backend2:5555;Host=somehost2")
+                .get("/path-trusted-proxy")
+                .then()
+                .body(Matchers
+                        .equalTo("http|somehost2|backend2:5555|/path-trusted-proxy|http://somehost2/path-trusted-proxy|true"));
+    }
+}
diff --git a/...sions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/ForwardedParser.java b/...sions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/ForwardedParser.java
@@ -40,6 +40,7 @@ class ForwardedParser {
     private static final AsciiString X_FORWARDED_PROTO = AsciiString.cached("X-Forwarded-Proto");
     private static final AsciiString X_FORWARDED_PORT = AsciiString.cached("X-Forwarded-Port");
     private static final AsciiString X_FORWARDED_FOR = AsciiString.cached("X-Forwarded-For");
+    private static final AsciiString X_FORWARDED_TRUSTED_PROXY = AsciiString.cached("X-Forwarded-Trusted-Proxy");
 
     private static final Pattern FORWARDED_HOST_PATTERN = Pattern.compile("host=\"?([^;,\"]+)\"?", Pattern.CASE_INSENSITIVE);
     private static final Pattern FORWARDED_PROTO_PATTERN = Pattern.compile("proto=\"?([^;,\"]+)\"?", Pattern.CASE_INSENSITIVE);
@@ -128,7 +129,8 @@ private void calculate() {
         setHostAndPort(delegate.host(), port);
         uri = delegate.uri();
 
-        if (trustedProxyCheck.isProxyAllowed()) {
+        boolean isProxyAllowed = trustedProxyCheck.isProxyAllowed();
+        if (isProxyAllowed) {
             String forwarded = delegate.getHeader(FORWARDED);
             if (forwardingProxyOptions.allowForwarded && forwarded != null) {
                 Matcher matcher = FORWARDED_PROTO_PATTERN.matcher(forwarded);
@@ -193,6 +195,21 @@ private void calculate() {
         authority = HostAndPort.create(host, port >= 0 ? port : -1);
         host = host + (port >= 0 ? ":" + port : "");
         delegate.headers().set(HOST_HEADER, host);
+        // TODO Add a test
+        if (forwardingProxyOptions.enableTrustedProxyHeader) {
+            // Verify that the header was not already set.
+            if (delegate.headers().contains(X_FORWARDED_TRUSTED_PROXY)) {
+                log.warn("The header " + X_FORWARDED_TRUSTED_PROXY + " was already set. Overwriting it.");
+            }
+            delegate.headers().set(X_FORWARDED_TRUSTED_PROXY, Boolean.toString(isProxyAllowed));
+        } else {
+            // Verify that the header was not already set - to avoid forgery.
+            if (delegate.headers().contains(X_FORWARDED_TRUSTED_PROXY)) {
+                log.warn("The header " + X_FORWARDED_TRUSTED_PROXY + " was already set. Removing it.");
+                delegate.headers().remove(X_FORWARDED_TRUSTED_PROXY);
+            }
+        }
+
         absoluteURI = scheme + "://" + host + uri;
         log.debug("Recalculated absoluteURI to " + absoluteURI);
     }

diff --git a/...ertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/ForwardingProxyOptions.java b/...ertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/ForwardingProxyOptions.java
@@ -15,14 +15,16 @@ public class ForwardingProxyOptions {
     final AsciiString forwardedHostHeader;
     final AsciiString forwardedPrefixHeader;
     public final TrustedProxyCheckBuilder trustedProxyCheckBuilder;
+    final boolean enableTrustedProxyHeader;
 
     public ForwardingProxyOptions(final boolean proxyAddressForwarding,
-            final boolean allowForwarded,
-            final boolean allowXForwarded,
-            final boolean enableForwardedHost,
-            final AsciiString forwardedHostHeader,
-            final boolean enableForwardedPrefix,
-            final AsciiString forwardedPrefixHeader,
+            boolean allowForwarded,
+            boolean allowXForwarded,
+            boolean enableForwardedHost,
+            boolean enableTrustedProxyHeader,
+            AsciiString forwardedHostHeader,
+            boolean enableForwardedPrefix,
+            AsciiString forwardedPrefixHeader,
             TrustedProxyCheckBuilder trustedProxyCheckBuilder) {
         this.proxyAddressForwarding = proxyAddressForwarding;
         this.allowForwarded = allowForwarded;
@@ -32,15 +34,16 @@ public ForwardingProxyOptions(final boolean proxyAddressForwarding,
         this.forwardedHostHeader = forwardedHostHeader;
         this.forwardedPrefixHeader = forwardedPrefixHeader;
         this.trustedProxyCheckBuilder = trustedProxyCheckBuilder;
+        this.enableTrustedProxyHeader = enableTrustedProxyHeader;
     }
 
     public static ForwardingProxyOptions from(ProxyConfig proxy) {
         final boolean proxyAddressForwarding = proxy.proxyAddressForwarding;
         final boolean allowForwarded = proxy.allowForwarded;
         final boolean allowXForwarded = proxy.allowXForwarded.orElse(!allowForwarded);
-
         final boolean enableForwardedHost = proxy.enableForwardedHost;
         final boolean enableForwardedPrefix = proxy.enableForwardedPrefix;
+        final boolean enableTrustedProxyHeader = proxy.enableTrustedProxyHeader;
         final AsciiString forwardedPrefixHeader = AsciiString.cached(proxy.forwardedPrefixHeader);
         final AsciiString forwardedHostHeader = AsciiString.cached(proxy.forwardedHostHeader);
 
@@ -50,6 +53,7 @@ public static ForwardingProxyOptions from(ProxyConfig proxy) {
                 || parts.isEmpty() ? null : TrustedProxyCheckBuilder.builder(parts);
 
         return new ForwardingProxyOptions(proxyAddressForwarding, allowForwarded, allowXForwarded, enableForwardedHost,
-                forwardedHostHeader, enableForwardedPrefix, forwardedPrefixHeader, proxyCheckBuilder);
+                enableTrustedProxyHeader, forwardedHostHeader, enableForwardedPrefix, forwardedPrefixHeader,
+                proxyCheckBuilder);
     }
 }
diff --git a/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/ProxyConfig.java b/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/ProxyConfig.java
@@ -76,6 +76,18 @@ public class ProxyConfig {
     @ConfigItem(defaultValue = "X-Forwarded-Prefix")
     public String forwardedPrefixHeader;
 
+    /**
+     * Adds the header `X-Forwarded-Trusted-Proxy` if the request is forwarded by a trusted proxy.
+     * The value is `true` if the request is forwarded by a trusted proxy, otherwise `null`.
+     * <p>
+     * The forwarded parser detects forgery attempts and if the incoming request contains this header, it will be removed
+     * from the request.
+     * <p>
+     * The `X-Forwarded-Trusted-Proxy` header is a custom header, not part of the standard `Forwarded` header.
+     */
+    @ConfigItem(defaultValue = "false")
+    public boolean enableTrustedProxyHeader;
+
     /**
      * Configure the list of trusted proxy addresses.
      * Received `Forwarded`, `X-Forwarded` or `X-Forwarded-*` headers from any other proxy address will be ignored.