Issue #5247 - Improve testing of ForwardedRequestCustomizer

+ Merge ProxyPass tests from CheckReverseProxyHeadersTest into
  ForwardedRequestCustomizerTest
+ Deleted CheckReverseProxyHeadersTest.java
+ Add more tests for ForcedHost configuration
+ Updated ForwardedRequestCustomizer to conform to expectations

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
This commit is contained in:
Joakim Erdfelt 2020-09-11 11:11:09 -05:00
parent 4280303046
commit ac42cbd143
No known key found for this signature in database
GPG Key ID: 2D0E1FB8FE4B68B4
3 changed files with 230 additions and 265 deletions

View File

@ -83,19 +83,19 @@ import static java.lang.invoke.MethodType.methodType;
* </tr>
* <tr>
* <td>2</td>
* <td><code>X-Forwarded-Host</code> Header</td>
* <td>Required</td>
* <td>Optional</td>
* <td>left-most value</td>
* </tr>
* <tr>
* <td>3</td>
* <td><code>X-Forwarded-Port</code> Header</td>
* <td>n/a</td>
* <td>Required</td>
* <td>left-most value (only if {@link #getForwardedPortAsAuthority()} is true)</td>
* </tr>
* <tr>
* <td>3</td>
* <td><code>X-Forwarded-Host</code> Header</td>
* <td>Required</td>
* <td>Implied</td>
* <td>left-most value</td>
* </tr>
* <tr>
* <td>4</td>
* <td><code>X-Forwarded-Server</code> Header</td>
* <td>Required</td>
@ -460,26 +460,80 @@ public class ForwardedRequestCustomizer implements Customizer
}
}
String proto = "http";
// Is secure status configured from headers?
if (forwarded.isSecure())
{
// set default to https
proto = config.getSecureScheme();
}
// Set Scheme from configured protocol
if (forwarded._proto != null)
{
request.setScheme(forwarded._proto);
if (forwarded._proto.equalsIgnoreCase(config.getSecureScheme()))
request.setSecure(true);
proto = forwarded._proto;
request.setScheme(proto);
}
if (forwarded.hasAuthority())
// Set authority
String host = null;
int port = -1;
// Use authority from headers, if configured.
if (forwarded._authority != null)
{
httpFields.put(new HostPortHttpField(forwarded._authority._host, forwarded._authority._port));
request.setAuthority(forwarded._authority._host, forwarded._authority._port);
host = forwarded._authority._host;
port = forwarded._authority._port;
}
// Fall back to request metadata if needed.
HttpURI requestURI = request.getMetaData().getURI();
if (host == null)
{
host = requestURI.getHost();
}
if (port == MutableHostPort.UNSET) // is unset by headers
{
port = requestURI.getPort();
}
if (port == MutableHostPort.IMPLIED) // is implied
{
// get Implied port (from protocol / scheme) and HttpConfiguration
int defaultPort = 80;
port = proto.equalsIgnoreCase(config.getSecureScheme()) ? getSecurePort(config) : defaultPort;
}
// Update authority if different from metadata
if (!host.equalsIgnoreCase(requestURI.getHost()) ||
port != requestURI.getPort())
{
httpFields.put(new HostPortHttpField(host, port));
request.setAuthority(host, port);
}
// Set secure status
if (forwarded.isSecure() ||
proto.equalsIgnoreCase(config.getSecureScheme()) ||
port == getSecurePort(config))
{
request.setSecure(true);
request.setScheme(proto);
}
// Set Remote Address
if (forwarded.hasFor())
{
int port = forwarded._for._port > 0 ? forwarded._for._port : request.getRemotePort();
request.setRemoteAddr(InetSocketAddress.createUnresolved(forwarded._for._host, port));
int forPort = forwarded._for._port > 0 ? forwarded._for._port : request.getRemotePort();
request.setRemoteAddr(InetSocketAddress.createUnresolved(forwarded._for._host, forPort));
}
}
protected static int getSecurePort(HttpConfiguration config)
{
return config.getSecurePort() > 0 ? config.getSecurePort() : 443;
}
protected void onError(HttpField field, Throwable t)
{
throw new BadMessageException("Bad header value for " + field.getName(), t);
@ -577,9 +631,12 @@ public class ForwardedRequestCustomizer implements Customizer
private static class MutableHostPort
{
public static final int UNSET = -1;
public static final int IMPLIED = 0;
String _host;
int _hostPriority = -1;
int _port = -1;
int _port = UNSET;
int _portPriority = -1;
public void setHost(String host, int priority)
@ -593,7 +650,7 @@ public class ForwardedRequestCustomizer implements Customizer
public void setPort(int port, int priority)
{
if (port > 0 && priority > _portPriority)
if (priority > _portPriority)
{
_port = port;
_portPriority = priority;
@ -602,22 +659,26 @@ public class ForwardedRequestCustomizer implements Customizer
public void setHostPort(HostPort hostPort, int priority)
{
if (_host == null || priority > _hostPriority)
if (priority > _hostPriority)
{
_host = hostPort.getHost();
_hostPriority = priority;
}
// Is this an authoritative port?
if (priority == FORWARDED_PRIORITY)
{
// Trust port (even if 0/unset)
_port = hostPort.getPort();
_portPriority = priority;
}
else
{
setPort(hostPort.getPort(), priority);
}
int port = hostPort.getPort();
// Is port supplied?
if (port > 0 && priority > _portPriority)
{
_port = hostPort.getPort();
_portPriority = priority;
}
// Since we are Host:Port pair, the port could be unspecified
// Meaning it's implied.
// Make sure that we switch the tracked port from unset to implied
else if (_port == UNSET)
{
// set port to implied (with no priority)
_port = IMPLIED;
}
}
@ -633,16 +694,14 @@ public class ForwardedRequestCustomizer implements Customizer
}
}
private static final int MAX_PRIORITY = 999;
private static final int FORCED_PRIORITY = 999;
private static final int FORWARDED_PRIORITY = 8;
private static final int XFORWARDED_HOST_PRIORITY = 7;
private static final int XFORWARDED_FOR_PRIORITY = 6;
private static final int XFORWARDED_PORT_PRIORITY = 5;
private static final int XFORWARDED_SERVER_PRIORITY = 4;
// HostPort seen in Request metadata
private static final int REQUEST_PRIORITY = 3;
private static final int XFORWARDED_PROTO_PRIORITY = 2;
private static final int XPROXIED_HTTPS_PRIORITY = 1;
private static final int XFORWARDED_PROTO_PRIORITY = 3;
private static final int XPROXIED_HTTPS_PRIORITY = 2;
private class Forwarded extends QuotedCSVParser
{
@ -653,6 +712,7 @@ public class ForwardedRequestCustomizer implements Customizer
MutableHostPort _for;
String _proto;
int _protoPriority = -1;
Boolean _secure;
public Forwarded(Request request, HttpConfiguration config)
{
@ -661,21 +721,14 @@ public class ForwardedRequestCustomizer implements Customizer
_config = config;
if (_forcedHost != null)
{
getAuthority().setHostPort(_forcedHost.getHostPort(), MAX_PRIORITY);
}
else
{
HttpURI requestURI = request.getMetaData().getURI();
if (requestURI.getHost() != null)
{
getAuthority().setHostPort(new HostPort(requestURI.getHost(), requestURI.getPort()), REQUEST_PRIORITY);
}
getAuthority().setHost(_forcedHost.getHostPort().getHost(), FORCED_PRIORITY);
getAuthority().setPort(_forcedHost.getHostPort().getPort(), FORCED_PRIORITY);
}
}
public boolean hasAuthority()
public boolean isSecure()
{
return _authority != null && _authority._host != null;
return (_secure != null && _secure);
}
public boolean hasFor()
@ -707,8 +760,7 @@ public class ForwardedRequestCustomizer implements Customizer
_request.setAttribute("javax.servlet.request.cipher_suite", field.getValue());
if (isSslIsSecure())
{
_request.setSecure(true);
_request.setScheme(_config.getSecureScheme());
_secure = true;
}
}
@ -718,8 +770,7 @@ public class ForwardedRequestCustomizer implements Customizer
_request.setAttribute("javax.servlet.request.ssl_session_id", field.getValue());
if (isSslIsSecure())
{
_request.setSecure(true);
_request.setScheme(_config.getSecureScheme());
_secure = true;
}
}
@ -732,7 +783,8 @@ public class ForwardedRequestCustomizer implements Customizer
@SuppressWarnings("unused")
public void handleForwardedFor(HttpField field)
{
getFor().setHostPort(new HostPort(getLeftMost(field.getValue())), XFORWARDED_FOR_PRIORITY);
HostPort hostField = new HostPort(getLeftMost(field.getValue()));
getFor().setHostPort(hostField, XFORWARDED_FOR_PRIORITY);
}
@SuppressWarnings("unused")
@ -762,8 +814,9 @@ public class ForwardedRequestCustomizer implements Customizer
{
if ("on".equalsIgnoreCase(field.getValue()) || "true".equalsIgnoreCase(field.getValue()))
{
_secure = true;
updateProto(HttpScheme.HTTPS.asString(), XPROXIED_HTTPS_PRIORITY);
updatePort(443, XPROXIED_HTTPS_PRIORITY);
updatePort(getSecurePort(_config), XPROXIED_HTTPS_PRIORITY);
}
}
@ -783,39 +836,41 @@ public class ForwardedRequestCustomizer implements Customizer
switch (name)
{
case "by":
{
if (!getProxyAsAuthority())
break;
if (value.startsWith("_") || "unknown".equals(value))
break;
getAuthority().setHostPort(new HostPort(value), FORWARDED_PRIORITY);
HostPort hostField = new HostPort(value);
getAuthority().setHost(hostField.getHost(), FORWARDED_PRIORITY);
getAuthority().setPort(hostField.getPort(), FORWARDED_PRIORITY);
break;
}
case "for":
{
if (value.startsWith("_") || "unknown".equals(value))
break;
getFor().setHostPort(new HostPort(value), FORWARDED_PRIORITY);
HostPort hostField = new HostPort(value);
getFor().setHost(hostField.getHost(), FORWARDED_PRIORITY);
getFor().setPort(hostField.getPort(), FORWARDED_PRIORITY);
break;
}
case "host":
{
if (value.startsWith("_") || "unknown".equals(value))
break;
getAuthority().setHostPort(new HostPort(value), FORWARDED_PRIORITY);
HostPort hostField = new HostPort(value);
getAuthority().setHost(hostField.getHost(), FORWARDED_PRIORITY);
getAuthority().setPort(hostField.getPort(), FORWARDED_PRIORITY);
break;
}
case "proto":
updateProto(value, FORWARDED_PRIORITY);
getAuthority().setPort(getPortForProto(value), FORWARDED_PRIORITY);
break;
}
}
}
private int getPortForProto(String proto)
{
if ("http".equalsIgnoreCase(proto))
return 80;
if ("https".equalsIgnoreCase(proto))
return 443;
return -1;
}
private void updateAuthority(String value, int priority)
{
HostPort hostField = new HostPort(value);
@ -840,6 +895,11 @@ public class ForwardedRequestCustomizer implements Customizer
{
_proto = proto;
_protoPriority = priority;
if (_proto.equalsIgnoreCase(_config.getSecureScheme()))
{
_secure = true;
}
}
}
}

View File

@ -1,200 +0,0 @@
//
// ========================================================================
// Copyright (c) 1995-2020 Mort Bay Consulting Pty Ltd and others.
// ------------------------------------------------------------------------
// All rights reserved. This program and the accompanying materials
// are made available under the terms of the Eclipse Public License v1.0
// and Apache License v2.0 which accompanies this distribution.
//
// The Eclipse Public License is available at
// http://www.eclipse.org/legal/epl-v10.html
//
// The Apache License v2.0 is available at
// http://www.opensource.org/licenses/apache2.0.php
//
// You may elect to redistribute this code under either of these licenses.
// ========================================================================
//
package org.eclipse.jetty.server;
import java.io.IOException;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import org.eclipse.jetty.server.handler.AbstractHandler;
import org.junit.jupiter.api.Test;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
/**
*
*/
public class CheckReverseProxyHeadersTest
{
@Test
public void testCheckReverseProxyHeaders() throws Exception
{
// Classic ProxyPass from example.com:80 to localhost:8080
testRequest("Host: localhost:8080\n" +
"X-Forwarded-For: 10.20.30.40\n" +
"X-Forwarded-Host: example.com", request ->
{
assertEquals("example.com", request.getServerName());
assertEquals(80, request.getServerPort());
assertEquals("10.20.30.40", request.getRemoteAddr());
assertEquals("10.20.30.40", request.getRemoteHost());
assertEquals("example.com", request.getHeader("Host"));
assertEquals("http", request.getScheme());
assertFalse(request.isSecure());
});
// IPv6 ProxyPass from example.com:80 to localhost:8080
testRequest("Host: localhost:8080\n" +
"X-Forwarded-For: 10.20.30.40\n" +
"X-Forwarded-Host: [::1]", request ->
{
assertEquals("[::1]", request.getServerName());
assertEquals(80, request.getServerPort());
assertEquals("10.20.30.40", request.getRemoteAddr());
assertEquals("10.20.30.40", request.getRemoteHost());
assertEquals("[::1]", request.getHeader("Host"));
assertEquals("http", request.getScheme());
assertFalse(request.isSecure());
});
// IPv6 ProxyPass from example.com:80 to localhost:8080
testRequest("Host: localhost:8080\n" +
"X-Forwarded-For: 10.20.30.40\n" +
"X-Forwarded-Host: [::1]:8888", request ->
{
assertEquals("[::1]", request.getServerName());
assertEquals(8888, request.getServerPort());
assertEquals("10.20.30.40", request.getRemoteAddr());
assertEquals("10.20.30.40", request.getRemoteHost());
assertEquals("[::1]:8888", request.getHeader("Host"));
assertEquals("http", request.getScheme());
assertFalse(request.isSecure());
});
// ProxyPass from example.com:81 to localhost:8080
testRequest("Host: localhost:8080\n" +
"X-Forwarded-For: 10.20.30.40\n" +
"X-Forwarded-Host: example.com:81\n" +
"X-Forwarded-Server: example.com\n" +
"X-Forwarded-Proto: https", request ->
{
assertEquals("example.com", request.getServerName());
assertEquals(81, request.getServerPort());
assertEquals("10.20.30.40", request.getRemoteAddr());
assertEquals("10.20.30.40", request.getRemoteHost());
assertEquals("example.com:81", request.getHeader("Host"));
assertEquals("https", request.getScheme());
assertTrue(request.isSecure());
});
// Multiple ProxyPass from example.com:80 to rp.example.com:82 to localhost:8080
testRequest("Host: localhost:8080\n" +
"X-Forwarded-For: 10.20.30.40, 10.0.0.1\n" +
"X-Forwarded-Host: example.com, rp.example.com:82\n" +
"X-Forwarded-Server: example.com, rp.example.com\n" +
"X-Forwarded-Proto: https, http", request ->
{
assertEquals("example.com", request.getServerName());
assertEquals(443, request.getServerPort());
assertEquals("10.20.30.40", request.getRemoteAddr());
assertEquals("10.20.30.40", request.getRemoteHost());
assertEquals("example.com", request.getHeader("Host"));
assertEquals("https", request.getScheme());
assertTrue(request.isSecure());
});
}
private void testRequest(String headers, RequestValidator requestValidator) throws Exception
{
Server server = new Server();
// Activate reverse proxy headers checking
HttpConnectionFactory http = new HttpConnectionFactory();
http.getHttpConfiguration().addCustomizer(new ForwardedRequestCustomizer());
LocalConnector connector = new LocalConnector(server, http);
server.setConnectors(new Connector[]{connector});
ValidationHandler validationHandler = new ValidationHandler(requestValidator);
server.setHandler(validationHandler);
try
{
server.start();
connector.getResponse("GET / HTTP/1.1\r\n" + "Connection: close\r\n" + headers + "\r\n\r\n");
Error error = validationHandler.getError();
if (error != null)
{
throw error;
}
}
finally
{
server.stop();
}
}
/**
* Interface for validate a wrapped request.
*/
private static interface RequestValidator
{
/**
* Validate the current request.
*
* @param request the request.
*/
void validate(HttpServletRequest request);
}
/**
* Handler for validation.
*/
private static class ValidationHandler extends AbstractHandler
{
private final RequestValidator _requestValidator;
private Error _error;
private ValidationHandler(RequestValidator requestValidator)
{
_requestValidator = requestValidator;
}
/**
* Retrieve the validation error.
*
* @return the validation error or <code>null</code> if there was no error.
*/
public Error getError()
{
return _error;
}
@Override
public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
{
try
{
_requestValidator.validate(request);
}
catch (Error e)
{
_error = e;
}
catch (Throwable e)
{
_error = new Error(e);
}
}
}
}

View File

@ -76,6 +76,7 @@ public class ForwardedRequestCustomizerTest
http.getHttpConfiguration().setRequestHeaderSize(512);
http.getHttpConfiguration().setResponseHeaderSize(512);
http.getHttpConfiguration().setOutputBufferSize(2048);
http.getHttpConfiguration().setSecurePort(443);
customizer = new ForwardedRequestCustomizer();
http.getHttpConfiguration().addCustomizer(customizer);
connector = new LocalConnector(server, http);
@ -277,6 +278,83 @@ public class ForwardedRequestCustomizerTest
.requestURL("https://myhost/")
),
// =================================================================
// ProxyPass usages
Arguments.of(new Request("ProxyPass (example.com:80 to localhost:8080)")
.headers(
"GET / HTTP/1.1",
"Host: localhost:8080",
"X-Forwarded-For: 10.20.30.40",
"X-Forwarded-Host: example.com"
),
new Expectations()
.scheme("http").serverName("example.com").serverPort(80)
.remoteAddr("10.20.30.40")
.requestURL("http://example.com/")
),
Arguments.of(new Request("ProxyPass (example.com:81 to localhost:8080)")
.headers(
"GET / HTTP/1.1",
"Host: localhost:8080",
"X-Forwarded-For: 10.20.30.40",
"X-Forwarded-Host: example.com:81",
"X-Forwarded-Server: example.com",
"X-Forwarded-Proto: https"
),
new Expectations()
.scheme("https").serverName("example.com").serverPort(81)
.remoteAddr("10.20.30.40")
.requestURL("https://example.com:81/")
),
Arguments.of(new Request("ProxyPass (example.com:443 to localhost:8443)")
.headers(
"GET / HTTP/1.1",
"Host: localhost:8443",
"X-Forwarded-Host: example.com",
"X-Forwarded-Proto: https"
),
new Expectations()
.scheme("https").serverName("example.com").serverPort(443)
.requestURL("https://example.com/")
),
Arguments.of(new Request("ProxyPass (IPv6 from [::1]:80 to localhost:8080)")
.headers(
"GET / HTTP/1.1",
"Host: localhost:8080",
"X-Forwarded-For: 10.20.30.40",
"X-Forwarded-Host: [::1]"
),
new Expectations()
.scheme("http").serverName("[::1]").serverPort(80)
.remoteAddr("10.20.30.40")
.requestURL("http://[::1]/")
),
Arguments.of(new Request("ProxyPass (IPv6 from [::1]:8888 to localhost:8080)")
.headers(
"GET / HTTP/1.1",
"Host: localhost:8080",
"X-Forwarded-For: 10.20.30.40",
"X-Forwarded-Host: [::1]:8888"
),
new Expectations()
.scheme("http").serverName("[::1]").serverPort(8888)
.remoteAddr("10.20.30.40")
.requestURL("http://[::1]:8888/")
),
Arguments.of(new Request("Multiple ProxyPass (example.com:80 to rp.example.com:82 to localhost:8080)")
.headers(
"GET / HTTP/1.1",
"Host: localhost:8080",
"X-Forwarded-For: 10.20.30.40, 10.0.0.1",
"X-Forwarded-Host: example.com, rp.example.com:82",
"X-Forwarded-Server: example.com, rp.example.com",
"X-Forwarded-Proto: https, http"
),
new Expectations()
.scheme("https").serverName("example.com").serverPort(443)
.remoteAddr("10.20.30.40")
.requestURL("https://example.com/")
),
// =================================================================
// X-Forwarded-* usages
Arguments.of(new Request("X-Forwarded-Proto (old syntax)")
@ -574,7 +652,34 @@ public class ForwardedRequestCustomizerTest
.requestURL("http://example.com/")
.remoteAddr("192.0.2.43").remotePort(0)
),
// =================================================================
// Forced Behavior
Arguments.of(new Request("Forced Host (no port)")
.configureCustomizer((customizer) -> customizer.setForcedHost("always.example.com"))
.headers(
"GET / HTTP/1.1",
"Host: myhost",
"X-Forwarded-For: 11.9.8.7:1111",
"X-Forwarded-Host: example.com:2222"
),
new Expectations()
.scheme("http").serverName("always.example.com").serverPort(80)
.requestURL("http://always.example.com/")
.remoteAddr("11.9.8.7").remotePort(1111)
),
Arguments.of(new Request("Forced Host with port")
.configureCustomizer((customizer) -> customizer.setForcedHost("always.example.com:9090"))
.headers(
"GET / HTTP/1.1",
"Host: myhost",
"X-Forwarded-For: 11.9.8.7:1111",
"X-Forwarded-Host: example.com:2222"
),
new Expectations()
.scheme("http").serverName("always.example.com").serverPort(9090)
.requestURL("http://always.example.com:9090/")
.remoteAddr("11.9.8.7").remotePort(1111)
),
// =================================================================
// Legacy Headers
Arguments.of(new Request("X-Proxied-Https")