Skip to content

Commit 5d78783

Browse files
tevans78tevans78
authored
Merge pull request #22426 from loriadi/PH46816-validation
PH46816: Header validation
2 parents d5cb72e + d33cd90 commit 5d78783

File tree

12 files changed

+818
-37
lines changed

12 files changed

+818
-37
lines changed

‎dev/com.ibm.ws.transport.http.remoteip_fat/fat/src/com/ibm/ws/transport/http/HttpXForwardedAndForwardedHeaderTests.java‎

Lines changed: 111 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*******************************************************************************
2-
* Copyright (c) 2018 IBM Corporation and others.
2+
* Copyright (c) 2018, 2022 IBM Corporation and others.
33
* All rights reserved. This program and the accompanying materials
44
* are made available under the terms of the Eclipse Public License v1.0
55
* which accompanies this distribution, and is available at
@@ -1847,6 +1847,113 @@ public void testTrustedForwardedWithDefaultRemoteIpRefConfig() throws Exception
18471847
assertTrue("Response does not contain expected isSecure", response.contains("isSecure: false"));
18481848
}
18491849

1850+
/**
1851+
* Test x-forwarded header with an invalid X-Forwarded-Proto header.
1852+
*
1853+
* The remote client address should not be verified.
1854+
*
1855+
* @throws Exception
1856+
*
1857+
* In this test, the x-forwarded-proto header is invalid, use the other
1858+
* x-forwarded headers but discard the proto
1859+
*/
1860+
@Test
1861+
@Mode(TestMode.FULL)
1862+
public void testTrustedXForwardedIPv4WithInvalidProto() throws Exception {
1863+
String variation = "defaultRemoteIP";
1864+
String testName = "testTrustedXForwardedIPv4WithInvalidProto";
1865+
String servletName = "EndpointInformationServlet";
1866+
startServer(variation);
1867+
1868+
// Note that the proto contains invalid characters.
1869+
List<BasicHeader> headerList = new ArrayList<BasicHeader>();
1870+
headerList.addAll(Arrays.asList(new BasicHeader("X-Forwarded-For", "192.100.0.299,192.168.0.101,192.168.2.322,172.31.255.188"),
1871+
new BasicHeader("X-Forwarded-Host", "clienttest.com"),
1872+
new BasicHeader("X-Forwarded-Proto", "https://evil.com/#")));
1873+
1874+
HttpResponse httpResponse = execute(APP_NAME, servletName, headerList, testName);
1875+
String response = getResponseAsString(httpResponse);
1876+
1877+
Log.info(ME, testName, "Response: " + response);
1878+
1879+
assertTrue("Response does not contain Endpoint Information Servlet Test message", response.contains("Endpoint Information Servlet Test"));
1880+
assertTrue("Response does contain the expected Remote Address", response.contains("Remote Address: 192.100.0.299"));
1881+
assertTrue("Response does not contain valid Scheme", !response.contains("Scheme: https://evil.com/#"));
1882+
assertTrue("Response does not contain valid Scheme", response.contains("Scheme: http"));
1883+
}
1884+
1885+
/**
1886+
* Test Forward header with an invalid proto parameter.
1887+
*
1888+
* The remote client address should not be verified.
1889+
*
1890+
* In this test, one of the parms of the forwarded header is invalid - proto.
1891+
* Throw out all the parms for the whole header.
1892+
*
1893+
* @throws Exception
1894+
*/
1895+
@Test
1896+
@Mode(TestMode.FULL)
1897+
public void testTrustedForwardedIPv4WithInvalidProto() throws Exception {
1898+
String variation = "defaultRemoteIP";
1899+
String testName = "testTrustedForwardedIPv4WithInvalidProto";
1900+
String servletName = "EndpointInformationServlet";
1901+
startServer(variation);
1902+
1903+
List<BasicHeader> headerList = new ArrayList<BasicHeader>();
1904+
// Note that the proto contains invalid characters.
1905+
headerList.addAll(Arrays
1906+
.asList(new BasicHeader("Forwarded", "for=192.100.0.299,for=192.168.0.101,for=192.168.2.322,for=172.31.255.188;"
1907+
+ "host=clienttest.com;"
1908+
+ "proto=https://evil.com/#")));
1909+
1910+
HttpResponse httpResponse = execute(APP_NAME, servletName, headerList, testName);
1911+
String response = getResponseAsString(httpResponse);
1912+
1913+
Log.info(ME, testName, "Response: " + response);
1914+
1915+
assertTrue("Response does not contain Endpoint Information Servlet Test message", response.contains("Endpoint Information Servlet Test"));
1916+
assertTrue("Response does contain the expected Remote Address", response.contains("Remote Address: 127.0.0.1"));
1917+
assertTrue("Response does not contain expected Remote Host", response.contains("Remote Host: localhost"));
1918+
assertTrue("Response does not contain valid Scheme", !response.contains("Scheme: https://evil.com/#"));
1919+
assertTrue("Response does not contain valid Scheme", response.contains("Scheme: http"));
1920+
}
1921+
1922+
/**
1923+
* Test Forward header with an invalid host parameter.
1924+
* The only invalid host would be an ipv6 address with mismatched [] or
1925+
* a [ bracket not in the first position.
1926+
*
1927+
* The remote client address should not be verified.
1928+
*
1929+
* @throws Exception
1930+
*/
1931+
@Test
1932+
@Mode(TestMode.FULL)
1933+
public void testTrustedForwardedIPv4WithInvalidHost() throws Exception {
1934+
String variation = "defaultRemoteIP";
1935+
String testName = "testTrustedForwardedIPv4WithInvalidHost";
1936+
String servletName = "EndpointInformationServlet";
1937+
startServer(variation);
1938+
1939+
List<BasicHeader> headerList = new ArrayList<BasicHeader>();
1940+
// Note that the host contains invalid characters.
1941+
headerList.addAll(Arrays
1942+
.asList(new BasicHeader("Forwarded", "for=192.100.0.299,for=192.168.0.101,for=192.168.2.322,for=172.31.255.188;"
1943+
+ "host=a[a];"
1944+
+ "proto=https")));
1945+
1946+
HttpResponse httpResponse = execute(APP_NAME, servletName, headerList, testName);
1947+
String response = getResponseAsString(httpResponse);
1948+
1949+
Log.info(ME, testName, "Response: " + response);
1950+
1951+
assertTrue("Response does not contain Endpoint Information Servlet Test message", response.contains("Endpoint Information Servlet Test"));
1952+
assertTrue("Response does contain the expected Remote Address", response.contains("Remote Address: 127.0.0.1"));
1953+
assertTrue("Response does not contain valid Remote Host", !response.contains("Remote Host: []"));
1954+
assertTrue("Response does not contain valid Scheme", response.contains("Scheme: http"));
1955+
}
1956+
18501957
/**
18511958
* Private method to start a server.
18521959
*
@@ -1865,10 +1972,10 @@ private void startServer(String variation) throws Exception {
18651972
/**
18661973
* Private method to execute/drive an HTTP request and obtain an HTTP response
18671974
*
1868-
* @param app The app name or context root
1869-
* @param path The specific path to drive the request
1975+
* @param app The app name or context root
1976+
* @param path The specific path to drive the request
18701977
* @param headerList A list of headers to be added in the request
1871-
* @param variation The name of the server for logging purposes
1978+
* @param variation The name of the server for logging purposes
18721979
* @return The HTTP response for the request
18731980
* @throws Exception
18741981
*/

‎dev/com.ibm.ws.transport.http/src/com/ibm/ws/genericbnf/internal/BNFHeadersImpl.java‎

Lines changed: 101 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*******************************************************************************
2-
* Copyright (c) 2004, 2019 IBM Corporation and others.
2+
* Copyright (c) 2004, 2022 IBM Corporation and others.
33
* All rights reserved. This program and the accompanying materials
44
* are made available under the terms of the Eclipse Public License v1.0
55
* which accompanies this distribution, and is available at
@@ -264,8 +264,8 @@ public BNFHeadersImpl() {
264264
* options.
265265
*
266266
* @param useDirect -- use direct ByteBuffers or indirect
267-
* @param outSize -- size of buffers to use while marshalling headers
268-
* @param inSize -- size of buffers to use while parsing headers
267+
* @param outSize -- size of buffers to use while marshalling headers
268+
* @param inSize -- size of buffers to use while parsing headers
269269
* @param cacheSize -- byte cache size of optimized parsing
270270
*/
271271
protected void init(boolean useDirect, int outSize, int inSize, int cacheSize) {
@@ -3133,8 +3133,8 @@ protected WsByteBuffer[] putBytes(byte[] data, WsByteBuffer[] inBuffers) {
31333133
* (extended) when the cache was flushed.
31343134
*
31353135
* @param data
3136-
* @param offset (into data to start at)
3137-
* @param length (to copy from the offset into data)
3136+
* @param offset (into data to start at)
3137+
* @param length (to copy from the offset into data)
31383138
* @param inBuffers
31393139
* @return WsByteBuffer[]
31403140
*/
@@ -3609,7 +3609,7 @@ private boolean parseCRLFs(WsByteBuffer buff) throws MalformedMessageException {
36093609
* Returns either the TOKEN_RC_DELIM or the TOKEN_RC_MOREDATA return codes.
36103610
*
36113611
* @param buff
3612-
* @param log - whether to debug log contents or not
3612+
* @param log - whether to debug log contents or not
36133613
* @return TokenCodes
36143614
* @throws MalformedMessageException
36153615
*/
@@ -3875,7 +3875,7 @@ private boolean parseHeaderValueNonExtract(WsByteBuffer buff) throws MalformedMe
38753875
* @param buff
38763876
* @param bDelimiter
38773877
* @param bApproveCRLF
3878-
* @param log - control how much of the token to debug log
3878+
* @param log - control how much of the token to debug log
38793879
* @return TokenCodes
38803880
* @throws MalformedMessageException
38813881
*/
@@ -3965,10 +3965,10 @@ protected void setHeaderValue() throws MalformedMessageException {
39653965
/**
39663966
* Sets the temporary parse token from the input buffer.
39673967
*
3968-
* @param buff The current WsByteBuffer being parsed
3968+
* @param buff The current WsByteBuffer being parsed
39693969
* @param start The start position of the token
39703970
* @param delim Did we stop on the delimiter or not?
3971-
* @param log Whether to log the contents or not
3971+
* @param log Whether to log the contents or not
39723972
*/
39733973
private void saveParsedToken(WsByteBuffer buff, int start, boolean delim, int log) {
39743974

@@ -4288,12 +4288,18 @@ private void processXForwardedHeader(HeaderElement header) {
42884288
processXForwardedAddressExtract(header.getDebugValue(), forwardedByList);
42894289
} else if (X_FORWARDED_PROTO.equalsIgnoreCase(header.getName())) {
42904290
forwardedProto = header.getDebugValue();
4291+
if (!validateProto(forwardedProto)) {
4292+
forwardedProto = null;
4293+
if (TraceComponent.isAnyTracingEnabled() && tc.isEntryEnabled()) {
4294+
Tr.debug(tc, "X-Forwarded-Proto value is invalid: " + header.getDebugValue());
4295+
}
4296+
}
4297+
42914298
} else if (X_FORWARDED_PORT.equalsIgnoreCase(header.getName())) {
42924299
forwardedPort = header.getDebugValue();
42934300
} else if (X_FORWARDED_HOST.equalsIgnoreCase(header.getName())) {
42944301
forwardedHost = header.getDebugValue();
42954302
}
4296-
42974303
if (TraceComponent.isAnyTracingEnabled() && tc.isEntryEnabled()) {
42984304
Tr.exit(tc, "processXForwardedHeader");
42994305
}
@@ -4358,8 +4364,30 @@ private void processSpecForwardedHeader(HeaderElement header) {
43584364

43594365
} else if (node.toLowerCase().startsWith(PROTO)) {
43604366
forwardedProto = nodeExtract;
4367+
boolean validProto = validateProto(forwardedProto);
4368+
if (!validProto) {
4369+
if (TraceComponent.isAnyTracingEnabled() && tc.isEntryEnabled()) {
4370+
Tr.debug(tc, "Forwarded header proto value was malformed: " + forwardedProto);
4371+
}
4372+
processForwardedErrorState();
4373+
if (TraceComponent.isAnyTracingEnabled() && tc.isEntryEnabled()) {
4374+
Tr.exit(tc, "processSpecForwardedHeader");
4375+
}
4376+
return;
4377+
}
43614378
} else if (node.toLowerCase().startsWith(HOST)) {
43624379
forwardedHost = nodeExtract;
4380+
forwardedHost = validateForwardedHost(forwardedHost);
4381+
if (forwardedHost == null) {
4382+
if (TraceComponent.isAnyTracingEnabled() && tc.isEntryEnabled()) {
4383+
Tr.debug(tc, "Forwarded header host value was malformed: " + nodeExtract);
4384+
}
4385+
processForwardedErrorState();
4386+
if (TraceComponent.isAnyTracingEnabled() && tc.isEntryEnabled()) {
4387+
Tr.exit(tc, "processSpecForwardedHeader");
4388+
}
4389+
return;
4390+
}
43634391
}
43644392
//Unrecognized parameter
43654393
else {
@@ -4497,6 +4525,69 @@ private void processForwardedAddressExtract(String nodeExtract, ListType type) {
44974525
list.add(nodeName);
44984526
}
44994527

4528+
/*
4529+
* A valid proto may start with an alpha followed by any number of chars that are
4530+
* - alpha
4531+
* - numeric
4532+
* - "+" or "-" or "."
4533+
*/
4534+
4535+
private boolean validateProto(String forwardedProto) {
4536+
if (TraceComponent.isAnyTracingEnabled() && tc.isEntryEnabled()) {
4537+
Tr.entry(tc, "validateProto");
4538+
}
4539+
char[] a = forwardedProto.toCharArray();
4540+
boolean valid = true;
4541+
char c = a[0];
4542+
valid = ((c >= 'a') && (c <= 'z')) ||
4543+
((c >= 'A') && (c <= 'Z'));
4544+
if (valid) {
4545+
4546+
for (int i = 1; i < a.length; i++) {
4547+
c = a[i];
4548+
valid = ((c >= 'a') && (c <= 'z')) ||
4549+
((c >= 'A') && (c <= 'Z')) ||
4550+
((c >= '0') && (c <= '9')) ||
4551+
(c == '+') || (c == '-') || (c == '.');
4552+
if (!valid) {
4553+
break;
4554+
}
4555+
}
4556+
4557+
}
4558+
if (TraceComponent.isAnyTracingEnabled() && tc.isEntryEnabled()) {
4559+
Tr.debug(tc, "ValidateProto value is valid: " + valid);
4560+
Tr.exit(tc, "validateProto");
4561+
}
4562+
return valid;
4563+
4564+
}
4565+
4566+
/*
4567+
* Valid hostname can be a bracketed ipv6 address, or
4568+
* any other string.
4569+
* Validate the ipv6 address has opening and closing brackets.
4570+
*/
4571+
4572+
private String validateForwardedHost(String forwardedHost) {
4573+
int openBracket = forwardedHost.indexOf("[");
4574+
int closedBracket = forwardedHost.indexOf("]");
4575+
String nodename = forwardedHost;
4576+
4577+
if (openBracket > -1) {
4578+
//This is an IPv6address
4579+
//The nodename is enclosed in "[ ]", get it now
4580+
4581+
//If the first character isn't the open bracket or if close bracket
4582+
//is missing, this is a badly formed header
4583+
if (openBracket != 0 || !(closedBracket > -1)) {
4584+
nodename = null;
4585+
}
4586+
}
4587+
return nodename;
4588+
4589+
}
4590+
45004591
private void processForwardedErrorState() {
45014592
this.forwardedByList = null;
45024593
this.forwardedForList = null;

‎dev/com.ibm.ws.transport.http/src/com/ibm/ws/http/dispatcher/internal/channel/HttpDispatcherLink.java‎

Lines changed: 44 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -920,16 +920,56 @@ public String getRemoteHostAddress() {
920920
if (remoteAddr == null) {
921921
remoteAddr = getTrustedHeader(HttpHeaderKeys.HDR_$WSRA.getName());
922922
if (remoteAddr != null) {
923-
if (TraceComponent.isAnyTracingEnabled() && tc.isDebugEnabled())
924-
Tr.debug(tc, "getRemoteHostAddress isTrusted --> true, addr --> " + remoteAddr);
925-
} else {
926-
remoteAddr = contextRemoteHostAddress();
923+
if (!validateRemoteAddress(remoteAddr)) {
924+
if (TraceComponent.isAnyTracingEnabled() && tc.isDebugEnabled())
925+
Tr.debug(tc, "getRemoteHostAddress isTrusted --> true, invalid addr --> " + remoteAddr);
926+
remoteAddr = null;
927+
}
927928
}
928929
}
930+
if (remoteAddr == null) {
931+
remoteAddr = contextRemoteHostAddress();
932+
}
929933

930934
return remoteAddr;
931935
}
932936

937+
/*
938+
* Validate the remote address
939+
*/
940+
boolean validateRemoteAddress(String address) {
941+
//The node identifier is defined by the ABNF syntax as
942+
// node = nodename [ ":" node-port ]
943+
// nodename = IPv4address / "[" IPv6address "]" /
944+
// "unknown" / obfnode
945+
//As such, to make it equivalent to the de-facto headers, remove the quotations
946+
//and possible port
947+
String extract = address.replaceAll("\"", "").trim();
948+
String nodeName = null;
949+
950+
//obfnodes are only allowed to contain ALPHA / DIGIT / "." / "_" / "-"
951+
//so if the token contains "[", it is an IPv6 address
952+
int openBracket = extract.indexOf("[");
953+
int closedBracket = extract.indexOf("]");
954+
955+
if (openBracket > -1) {
956+
//This is an IPv6address
957+
//The nodename is enclosed in "[ ]", get it now
958+
959+
//If the first character isn't the open bracket or if a close bracket
960+
//is not provided, this is a badly formed header
961+
if (openBracket != 0 || !(closedBracket > -1)) {
962+
//badly formated header
963+
if (TraceComponent.isAnyTracingEnabled() && tc.isEntryEnabled()) {
964+
Tr.debug(tc, "$WSRA IPv6 address was malformed: " + address);
965+
}
966+
return false;
967+
}
968+
969+
}
970+
return true;
971+
}
972+
933973
/**
934974
* @return the remote host address of the inbound connection
935975
*/

‎dev/com.ibm.ws.webcontainer.servlet.3.1_fat/.classpath‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,5 +34,6 @@
3434
<classpathentry kind="src" path="test-applications/multiNBReadApp.war/src"/>
3535
<classpathentry kind="con" path="aQute.bnd.classpath.container"/>
3636
<classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER/org.eclipse.jdt.internal.debug.ui.launcher.StandardVMType/JavaSE-1.8"/>
37+
<classpathentry kind="src" path="src"/>
3738
<classpathentry kind="output" path="bin"/>
3839
</classpath>

0 commit comments

Comments
 (0)