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

Enable forwarded for host header configuration #10138

Merged
merged 1 commit into from
Jun 24, 2020
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 @@ -18,7 +18,8 @@ public class ForwardedForHeaderTest {
static final QuarkusUnitTest config = new QuarkusUnitTest()
.setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class)
.addClasses(ForwardedHandlerInitializer.class)
.addAsResource(new StringAsset("quarkus.http.proxy-address-forwarding=true\n"),
.addAsResource(new StringAsset("quarkus.http.proxy-address-forwarding=true\n" +
"quarkus.http.proxy.enable-forwarded-host=true\n"),
"application.properties"));

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ class ForwardedParser {
private static final AsciiString FORWARDED = AsciiString.cached("Forwarded");
private static final AsciiString X_FORWARDED_SSL = AsciiString.cached("X-Forwarded-Ssl");
private static final AsciiString X_FORWARDED_PROTO = AsciiString.cached("X-Forwarded-Proto");
private static final AsciiString X_FORWARDED_HOST = AsciiString.cached("X-Forwarded-Host");
private static final AsciiString X_FORWARDED_PORT = AsciiString.cached("X-Forwarded-Port");
private static final AsciiString X_FORWARDED_FOR = AsciiString.cached("X-Forwarded-For");

Expand All @@ -47,7 +46,7 @@ class ForwardedParser {
private static final Pattern FORWARDED_FOR_PATTERN = Pattern.compile("for=\"?([^;,\"]+)\"?");

private final HttpServerRequest delegate;
private final boolean allowForward;
private final ForwardingProxyOptions forwardingProxyOptions;

private boolean calculated;
private String host;
Expand All @@ -56,9 +55,9 @@ class ForwardedParser {
private String absoluteURI;
private SocketAddress remoteAddress;

ForwardedParser(HttpServerRequest delegate, boolean allowForward) {
ForwardedParser(HttpServerRequest delegate, ForwardingProxyOptions forwardingProxyOptions) {
this.delegate = delegate;
this.allowForward = allowForward;
this.forwardingProxyOptions = forwardingProxyOptions;
}

public String scheme() {
Expand Down Expand Up @@ -104,7 +103,7 @@ private void calculate() {
boolean isForwardedSslOn = forwardedSsl != null && forwardedSsl.equalsIgnoreCase("on");

String forwarded = delegate.getHeader(FORWARDED);
if (allowForward && forwarded != null) {
if (forwardingProxyOptions.allowForwarded && forwarded != null) {
String forwardedToUse = forwarded.split(",")[0];
Matcher matcher = FORWARDED_PROTO_PATTERN.matcher(forwardedToUse);
if (matcher.find()) {
Expand All @@ -124,7 +123,7 @@ private void calculate() {
if (matcher.find()) {
remoteAddress = parseFor(matcher.group(1).trim(), remoteAddress.port());
}
} else if (!allowForward) {
} else if (!forwardingProxyOptions.allowForwarded) {
String protocolHeader = delegate.getHeader(X_FORWARDED_PROTO);
if (protocolHeader != null) {
scheme = protocolHeader.split(",")[0];
Expand All @@ -134,9 +133,11 @@ private void calculate() {
port = -1;
}

String hostHeader = delegate.getHeader(X_FORWARDED_HOST);
if (hostHeader != null) {
setHostAndPort(hostHeader.split(",")[0], port);
if (forwardingProxyOptions.enableForwardedHost) {
String hostHeader = delegate.getHeader(forwardingProxyOptions.forwardedHostHeader);
if (hostHeader != null) {
setHostAndPort(hostHeader.split(",")[0], port);
}
}

String portHeader = delegate.getHeader(X_FORWARDED_PORT);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ class ForwardedServerRequestWrapper implements HttpServerRequest {
private String uri;
private String absoluteURI;

ForwardedServerRequestWrapper(HttpServerRequest request, boolean allowForwarded) {
ForwardedServerRequestWrapper(HttpServerRequest request, ForwardingProxyOptions forwardingProxyOptions) {
delegate = request;
forwardedParser = new ForwardedParser(delegate, allowForwarded);
forwardedParser = new ForwardedParser(delegate, forwardingProxyOptions);
}

void changeTo(HttpMethod method, String uri) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package io.quarkus.vertx.http.runtime;

import io.netty.util.AsciiString;

public class ForwardingProxyOptions {
boolean proxyAddressForwarding;
boolean allowForwarded;
boolean enableForwardedHost;
AsciiString forwardedHostHeader;

public ForwardingProxyOptions(final boolean proxyAddressForwarding,
final boolean allowForwarded,
final boolean enableForwardedHost,
final AsciiString forwardedHostHeader) {
this.proxyAddressForwarding = proxyAddressForwarding;
this.allowForwarded = allowForwarded;
this.enableForwardedHost = enableForwardedHost;
this.forwardedHostHeader = forwardedHostHeader;
}

public static ForwardingProxyOptions from(HttpConfiguration httpConfiguration) {
final boolean proxyAddressForwarding = httpConfiguration.proxyAddressForwarding
.orElse(httpConfiguration.proxy.proxyAddressForwarding);
final boolean allowForwarded = httpConfiguration.allowForwarded
.orElse(httpConfiguration.proxy.allowForwarded);

final boolean enableForwardedHost = httpConfiguration.proxy.enableForwardedHost;
final AsciiString forwardedHostHeader = AsciiString.cached(httpConfiguration.proxy.forwardedHostHeader);

return new ForwardingProxyOptions(proxyAddressForwarding, allowForwarded, enableForwardedHost, forwardedHostHeader);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -59,16 +59,22 @@ public class HttpConfiguration {
/**
* If this is true then the address, scheme etc will be set from headers forwarded by the proxy server, such as
* {@code X-Forwarded-For}. This should only be set if you are behind a proxy that sets these headers.
*
* @deprecated use quarkus.http.proxy.proxy-address-forwarding instead.
*/
@Deprecated
@ConfigItem
public boolean proxyAddressForwarding;
public Optional<Boolean> proxyAddressForwarding;

/**
* If this is true and proxy address forwarding is enabled then the standard {@code Forwarded} header will be used,
* rather than the more common but not standard {@code X-Forwarded-For}.
*
* @deprecated use quarkus.http.proxy.allow-forwarded instead.
*/
@Deprecated
@ConfigItem
public boolean allowForwarded;
public Optional<Boolean> allowForwarded;

/**
* If insecure (i.e. http rather than https) requests are allowed. If this is {@code enabled}
Expand Down Expand Up @@ -197,6 +203,8 @@ public class HttpConfiguration {
@ConfigItem
public Map<String, SameSiteCookieConfig> sameSiteCookie;

public ProxyConfig proxy;

public int determinePort(LaunchMode launchMode) {
return launchMode == LaunchMode.TEST ? testPort : port;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package io.quarkus.vertx.http.runtime;

import io.quarkus.runtime.annotations.ConfigGroup;
import io.quarkus.runtime.annotations.ConfigItem;

/**
* Holds configuration related with proxy addressing forward.
*/
@ConfigGroup
public class ProxyConfig {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the existing proxy config should be moved to this class as well (proxyAddressForwarding and allowForwarded).

To do this mark the original ones deprecated, change them to Optional, and then add new ones here. If the old ones are set then log a warning about deprecation and use those values, otherwise use the new config.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

/**
* If this is true then the address, scheme etc will be set from headers forwarded by the proxy server, such as
* {@code X-Forwarded-For}. This should only be set if you are behind a proxy that sets these headers.
*/
@ConfigItem
public boolean proxyAddressForwarding;

/**
* If this is true and proxy address forwarding is enabled then the standard {@code Forwarded} header will be used,
* rather than the more common but not standard {@code X-Forwarded-For}.
*/
@ConfigItem
public boolean allowForwarded;

/**
* Enable override the received request's host through a forwarded host header.
*/
@ConfigItem(defaultValue = "false")
public boolean enableForwardedHost;

/**
* Configure the forwarded host header to be used if override enabled.
*/
@ConfigItem(defaultValue = "X-Forwarded-Host")
public String forwardedHostHeader;

}
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,6 @@ public static void startServerAfterFailedStart() {
ConfigInstantiator.handleObject(buildConfig);
HttpConfiguration config = new HttpConfiguration();
ConfigInstantiator.handleObject(config);

Router router = Router.router(vertx);
if (hotReplacementHandler != null) {
router.route().order(Integer.MIN_VALUE).blockingHandler(hotReplacementHandler);
Expand Down Expand Up @@ -307,12 +306,14 @@ public void handle(Void e) {
root = mainRouter;
}

if (httpConfiguration.proxyAddressForwarding) {
warnIfDeprecatedHttpConfigPropertiesPresent(httpConfiguration);
ForwardingProxyOptions forwardingProxyOptions = ForwardingProxyOptions.from(httpConfiguration);
if (forwardingProxyOptions.proxyAddressForwarding) {
Handler<HttpServerRequest> delegate = root;
root = new Handler<HttpServerRequest>() {
@Override
public void handle(HttpServerRequest event) {
delegate.handle(new ForwardedServerRequestWrapper(event, httpConfiguration.allowForwarded));
delegate.handle(new ForwardedServerRequestWrapper(event, forwardingProxyOptions));
}
};
}
Expand Down Expand Up @@ -378,13 +379,29 @@ public void handle(RoutingContext event) {
rootHandler = root;
}

private void warnIfDeprecatedHttpConfigPropertiesPresent(HttpConfiguration httpConfiguration) {
if (httpConfiguration.proxyAddressForwarding.isPresent()) {
LOGGER.warn(
"`quarkus.http.proxy-address-forwarding` is deprecated and will be removed in a future version - it is "
+ "recommended to switch to `quarkus.http.proxy.proxy-address-forwarding`");
}

if (httpConfiguration.allowForwarded.isPresent()) {
LOGGER.warn(
"`quarkus.http.allow-forwarded` is deprecated and will be removed in a future version - it is "
+ "recommended to switch to `quarkus.http.proxy.allow-forwarded`");
}
}

private static void doServerStart(Vertx vertx, HttpBuildTimeConfig httpBuildTimeConfig,
HttpConfiguration httpConfiguration, LaunchMode launchMode,
Supplier<Integer> eventLoops, String websocketSubProtocols) throws IOException {
// Http server configuration
HttpServerOptions httpServerOptions = createHttpServerOptions(httpConfiguration, launchMode, websocketSubProtocols);
HttpServerOptions domainSocketOptions = createDomainSocketOptions(httpConfiguration, websocketSubProtocols);
HttpServerOptions sslConfig = createSslOptions(httpBuildTimeConfig, httpConfiguration, launchMode);
ForwardingProxyOptions forwardingProxyOptions = ForwardingProxyOptions.from(httpConfiguration);

if (httpConfiguration.insecureRequests != HttpConfiguration.InsecureRequests.ENABLED && sslConfig == null) {
throw new IllegalStateException("Cannot set quarkus.http.redirect-insecure-requests without enabling SSL.");
}
Expand Down