Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Support for Trusted Proxy Detection on Forwarded Requests #44184

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading