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.

Sign up for GitHub

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

Allow less strict host header parsing to handle non-RFC2396 compliant host headers #1284

Merged
merged 1 commit into from
Aug 19, 2022
Merged

Allow less strict host header parsing to handle non-RFC2396 compliant host headers #1284

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
79 changes: 66 additions & 13 deletions zuul-core/src/main/java/com/netflix/zuul/message/http/HttpRequestMessageImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import com.netflix.config.CachedDynamicBooleanProperty;
import com.netflix.config.CachedDynamicIntProperty;
import com.netflix.config.DynamicStringProperty;
import com.netflix.util.Pair;
import com.netflix.zuul.context.CommonContextKeys;
import com.netflix.zuul.context.SessionContext;
import com.netflix.zuul.filters.ZuulFilter;
Expand Down Expand Up @@ -53,6 +54,10 @@ public class HttpRequestMessageImpl implements HttpRequestMessage
{
private static final Logger LOG = LoggerFactory.getLogger(HttpRequestMessageImpl.class);

private static final CachedDynamicBooleanProperty STRICT_HOST_HEADER_VALIDATION = new CachedDynamicBooleanProperty(
"zuul.HttpRequestMessage.host.header.strict.validation", true
);

private static final CachedDynamicIntProperty MAX_BODY_SIZE_PROP = new CachedDynamicIntProperty(
"zuul.HttpRequestMessage.body.max.size", 15 * 1000 * 1024
);
Expand Down Expand Up @@ -493,13 +498,9 @@ static String getOriginalHost(Headers headers, String serverName) throws URISynt
if (xForwardedHost != null) {
return xForwardedHost;
}
String host = headers.getFirst(HttpHeaderNames.HOST);
if (host != null) {
URI uri = new URI(/* scheme= */ null, host, /* path= */ null, /* query= */ null, /* fragment= */ null);
if (uri.getHost() == null) {
throw new URISyntaxException(host, "Bad host name");
}
return uri.getHost();
Pair<String, Integer> host = parseHostHeader(headers);
if (host.first() != null) {
return host.first();
}
return serverName;
}
Expand Down Expand Up @@ -542,14 +543,17 @@ static int getOriginalPort(SessionContext context, Headers headers, int serverPo
if (portStr != null && !portStr.isEmpty()) {
return Integer.parseInt(portStr);
}
// Check if port was specified on a Host header.
String host = headers.getFirst(HttpHeaderNames.HOST);
if (host != null) {
URI uri = new URI(/* scheme= */ null, host, /* path= */ null, /* query= */ null, /* fragment= */ null);
if (uri.getPort() != -1) {
return uri.getPort();

try {
// Check if port was specified on a Host header.
Pair<String, Integer> host = parseHostHeader(headers);
if (host.second() != -1) {
return host.second();
}
} catch (URISyntaxException e) {
LOG.debug("Invalid host header, falling back to serverPort", e);
}

return serverPort;
}

Expand All @@ -563,6 +567,55 @@ public Optional<Integer> getClientDestinationPort() {
}
}

/**
* Attempt to parse the Host header from the collection of headers
* and return the hostname and port components.
*
* @return Hostname and Port pair.
* Hostname may be null. Port may be -1 when no valid port is found in the host header.
*/
private static Pair<String, Integer> parseHostHeader(Headers headers) throws URISyntaxException {
String host = headers.getFirst(HttpHeaderNames.HOST);
if (host == null) {
return new Pair<>(null, -1);
}

try {
// attempt to use default URI parsing - this can fail when not strictly following RFC2396,
// for example, having underscores in host names will fail parsing
URI uri = new URI(/* scheme= */ null, host, /* path= */ null, /* query= */ null, /* fragment= */ null);
if (uri.getHost() != null) {
return new Pair<>(uri.getHost(), uri.getPort());
}
} catch (URISyntaxException e) {
LOG.debug("URI parsing failed", e);
}

if (STRICT_HOST_HEADER_VALIDATION.get()) {
throw new URISyntaxException(host, "Invalid host");
}

// fallback to using a colon split
// valid IPv6 addresses would have been handled already so any colon is safely assumed a port separator
Copy link
Contributor

Choose a reason for hiding this comment

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

What if it's an invalid IPv6 address?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is a fallback below that detects more than 1 colon (e.g. a non-bracketed IPv6 address) which will throw an exception

String[] components = host.split(":");
if (components.length > 2) {
// handle case with unbracketed IPv6 addresses
return new Pair<>(null, -1);
}

String parsedHost = components[0];
int parsedPort = -1;
if (components.length > 1) {
try {
parsedPort = Integer.parseInt(components[1]);
} catch (NumberFormatException e) {
// ignore failing to parse port numbers and fallback to default port
Copy link
Contributor

Choose a reason for hiding this comment

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

Debug logging?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added 🪨

LOG.debug("Parsing of host port component failed", e);
}
}
return new Pair<>(parsedHost, parsedPort);
}

/**
* Attempt to reconstruct the full URI that the client used.
*
Expand Down
92 changes: 92 additions & 0 deletions zuul-core/src/test/java/com/netflix/zuul/message/http/HttpRequestMessageImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import static org.mockito.Mockito.when;

import com.google.common.net.InetAddresses;
import com.netflix.config.ConfigurationManager;
import com.netflix.zuul.context.CommonContextKeys;
import com.netflix.zuul.context.SessionContext;
import com.netflix.zuul.message.Headers;
Expand All @@ -36,6 +37,9 @@
import java.net.SocketAddress;
import java.net.URISyntaxException;
import java.util.Optional;

import org.apache.commons.configuration.AbstractConfiguration;
import org.junit.After;
import org.junit.Assert;
import org.junit.Test;
import org.junit.runner.RunWith;
Expand All @@ -45,6 +49,12 @@
public class HttpRequestMessageImplTest {

HttpRequestMessageImpl request;
private final AbstractConfiguration config = ConfigurationManager.getConfigInstance();

@After
public void resetConfig() {
config.clearProperty("zuul.HttpRequestMessage.host.header.strict.validation");
Copy link
Contributor

Choose a reason for hiding this comment

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

nice trick!

}

@Test
public void testOriginalRequestInfo() {
Expand Down Expand Up @@ -260,6 +270,40 @@ public void testGetOriginalHost() {
Assert.assertEquals("blah.netflix.com", request.getOriginalHost());
}

@Test
public void testGetOriginalHost_handlesNonRFC2396Hostnames() {
config.setProperty("zuul.HttpRequestMessage.host.header.strict.validation", false);

HttpQueryParams queryParams = new HttpQueryParams();
Headers headers = new Headers();
headers.add("Host", "my_underscore_endpoint.netflix.com");
request = new HttpRequestMessageImpl(new SessionContext(), "HTTP/1.1", "POST", "/some/where", queryParams,
headers,
"192.168.0.2", "https", 7002, "localhost");
Assert.assertEquals("my_underscore_endpoint.netflix.com", request.getOriginalHost());

headers = new Headers();
headers.add("Host", "my_underscore_endpoint.netflix.com:8080");
request = new HttpRequestMessageImpl(new SessionContext(), "HTTP/1.1", "POST", "/some/where", queryParams,
headers,
"192.168.0.2", "https", 7002, "localhost");
Assert.assertEquals("my_underscore_endpoint.netflix.com", request.getOriginalHost());

headers = new Headers();
headers.add("Host", "my_underscore_endpoint^including~more-chars.netflix.com");
request = new HttpRequestMessageImpl(new SessionContext(), "HTTP/1.1", "POST", "/some/where", queryParams,
headers,
"192.168.0.2", "https", 7002, "localhost");
Assert.assertEquals("my_underscore_endpoint^including~more-chars.netflix.com", request.getOriginalHost());

headers = new Headers();
headers.add("Host", "hostname%5Ewith-url-encoded.netflix.com");
request = new HttpRequestMessageImpl(new SessionContext(), "HTTP/1.1", "POST", "/some/where", queryParams,
headers,
"192.168.0.2", "https", 7002, "localhost");
Assert.assertEquals("hostname%5Ewith-url-encoded.netflix.com", request.getOriginalHost());
}

@Test
public void getOriginalHost_failsOnUnbracketedIpv6Address() {
HttpQueryParams queryParams = new HttpQueryParams();
Expand All @@ -272,6 +316,20 @@ public void getOriginalHost_failsOnUnbracketedIpv6Address() {
assertThrows(URISyntaxException.class, () -> HttpRequestMessageImpl.getOriginalHost(headers, "server"));
}

@Test
public void getOriginalHost_fallsBackOnUnbracketedIpv6Address_WithNonStrictValidation() {
config.setProperty("zuul.HttpRequestMessage.host.header.strict.validation", false);

HttpQueryParams queryParams = new HttpQueryParams();
Headers headers = new Headers();
headers.add("Host", "ba::dd");
request = new HttpRequestMessageImpl(new SessionContext(), "HTTP/1.1", "POST", "/some/where", queryParams,
headers,
"192.168.0.2", "https", 7002, "server");

Assert.assertEquals("server", request.getOriginalHost());
}

@Test
public void testGetOriginalPort() {
HttpQueryParams queryParams = new HttpQueryParams();
Expand Down Expand Up @@ -331,6 +389,40 @@ public void testGetOriginalPort() {
headers,
"192.168.0.2", "https", 7002, "localhost");
Assert.assertEquals(7005, request.getOriginalPort());

headers = new Headers();
headers.add("Host", "host_with_underscores.netflix.com:8080");
request = new HttpRequestMessageImpl(new SessionContext(), "HTTP/1.1", "POST", "/some/where", queryParams,
headers,
"192.168.0.2", "https", 7002, "localhost");
Assert.assertEquals("should fallback to server port", 7002, request.getOriginalPort());
}

@Test
public void testGetOriginalPort_NonStrictValidation() {
config.setProperty("zuul.HttpRequestMessage.host.header.strict.validation", false);

HttpQueryParams queryParams = new HttpQueryParams();
Headers headers = new Headers();
headers.add("Host", "host_with_underscores.netflix.com:8080");
request = new HttpRequestMessageImpl(new SessionContext(), "HTTP/1.1", "POST", "/some/where", queryParams,
headers,
"192.168.0.2", "https", 7002, "localhost");
Assert.assertEquals(8080, request.getOriginalPort());

headers = new Headers();
headers.add("Host", "host-with-carrots^1.0.0.netflix.com:8080");
request = new HttpRequestMessageImpl(new SessionContext(), "HTTP/1.1", "POST", "/some/where", queryParams,
headers,
"192.168.0.2", "https", 7002, "localhost");
Assert.assertEquals(8080, request.getOriginalPort());

headers = new Headers();
headers.add("Host", "host-with-carrots-no-port^1.0.0.netflix.com");
request = new HttpRequestMessageImpl(new SessionContext(), "HTTP/1.1", "POST", "/some/where", queryParams,
headers,
"192.168.0.2", "https", 7002, "localhost");
Assert.assertEquals(7002, request.getOriginalPort());
}

@Test
Expand Down