Skip to content

Commit

Permalink
Add Support for Trusted Proxy Detection on Forwarded Requests
Browse files Browse the repository at this point in the history
This commit introduces a way to determine if a request behind a proxy has been forwarded by a trusted proxy. It implements a custom header (`X-Forwarded-Trusted-Proxy`) that allows request processing to verify the presence of this header, indicating the request originated from a trusted source.

To prevent forgery, any incoming request containing this custom header has it removed before further processing.
  • Loading branch information
cescoffier committed Oct 30, 2024
1 parent 0e4dc19 commit 5069761
Show file tree
Hide file tree
Showing 7 changed files with 230 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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")));
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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|");
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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>,
Expand Down
Original file line number Diff line number Diff line change
@@ -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"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);

Expand All @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit 5069761

Please sign in to comment.