Author: markt
Date: Tue Jan 30 13:59:11 2018
New Revision: 1822644
URL: http://svn.apache.org/viewvc?rev=1822644&view=rev
Log:
Enable strict host/port validation for all connectors.
Modified:
tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java
tomcat/trunk/java/org/apache/coyote/ajp/AjpProcessor.java
tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java
tomcat/trunk/java/org/apache/coyote/http11/LocalStrings.properties
tomcat/trunk/java/org/apache/coyote/http2/LocalStrings.properties
tomcat/trunk/java/org/apache/coyote/http2/Stream.java
tomcat/trunk/java/org/apache/tomcat/util/http/parser/Host.java
tomcat/trunk/webapps/docs/changelog.xml
Modified: tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java?rev=1822644&r1=1822643&r2=1822644&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java (original)
+++ tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java Tue Jan 30
13:59:11 2018
@@ -27,6 +27,8 @@ import javax.servlet.RequestDispatcher;
import org.apache.tomcat.util.ExceptionUtils;
import org.apache.tomcat.util.buf.ByteChunk;
+import org.apache.tomcat.util.buf.MessageBytes;
+import org.apache.tomcat.util.http.parser.Host;
import org.apache.tomcat.util.net.AbstractEndpoint.Handler.SocketState;
import org.apache.tomcat.util.net.DispatchType;
import org.apache.tomcat.util.net.SSLSupport;
@@ -42,6 +44,9 @@ public abstract class AbstractProcessor
private static final StringManager sm =
StringManager.getManager(AbstractProcessor.class);
+ // Used to avoid useless B2C conversion on the host name.
+ private char[] hostNameC = new char[0];
+
protected final Adapter adapter;
protected final AsyncStateMachine asyncStateMachine;
private volatile long asyncTimeout = -1;
@@ -244,6 +249,68 @@ public abstract class AbstractProcessor
}
+ protected void parseHost(MessageBytes valueMB) {
+ if (valueMB == null || valueMB.isNull()) {
+ populateHost();
+ return;
+ }
+
+ ByteChunk valueBC = valueMB.getByteChunk();
+ byte[] valueB = valueBC.getBytes();
+ int valueL = valueBC.getLength();
+ int valueS = valueBC.getStart();
+ if (hostNameC.length < valueL) {
+ hostNameC = new char[valueL];
+ }
+
+ try {
+ // Validates the host name
+ int colonPos = Host.parse(valueMB);
+
+ // Extract the port information first, if any
+ if (colonPos != -1) {
+ int port = 0;
+ for (int i = colonPos + 1; i < valueL; i++) {
+ char c = (char) valueB[i + valueS];
+ if (c < '0' || c > '9') {
+ response.setStatus(400);
+ setErrorState(ErrorState.CLOSE_CLEAN, null);
+ return;
+ }
+ port = port * 10 + c - '0';
+ }
+ request.setServerPort(port);
+
+ // Only need to copy the host name up to the :
+ valueL = colonPos;
+ }
+
+ // Extract the host name
+ for (int i = 0; i < valueL; i++) {
+ hostNameC[i] = (char) valueB[i + valueS];
+ }
+ request.serverName().setChars(hostNameC, 0, valueL);
+
+ } catch (IllegalArgumentException e) {
+ // IllegalArgumentException indicates that the host name is invalid
+ response.setStatus(400);
+ setErrorState(ErrorState.CLOSE_CLEAN, null);
+ }
+ }
+
+
+ /**
+ * Called when a host name is not present in the request (e.g. HTTP/1.0).
+ * It populates the server name and port with appropriate information. The
+ * source is expected to vary by protocol.
+ * <p>
+ * The default implementation is a NO-OP.
+ */
+ protected void populateHost() {
+ // NO-OP
+ }
+
+
@Override
public final void action(ActionCode actionCode, Object param) {
switch (actionCode) {
Modified: tomcat/trunk/java/org/apache/coyote/ajp/AjpProcessor.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/ajp/AjpProcessor.java?rev=1822644&r1=1822643&r2=1822644&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/ajp/AjpProcessor.java (original)
+++ tomcat/trunk/java/org/apache/coyote/ajp/AjpProcessor.java Tue Jan 30
13:59:11 2018
@@ -39,7 +39,6 @@ import org.apache.juli.logging.Log;
import org.apache.juli.logging.LogFactory;
import org.apache.tomcat.util.ExceptionUtils;
import org.apache.tomcat.util.buf.ByteChunk;
-import org.apache.tomcat.util.buf.HexUtils;
import org.apache.tomcat.util.buf.MessageBytes;
import org.apache.tomcat.util.http.MimeHeaders;
import org.apache.tomcat.util.net.AbstractEndpoint.Handler.SocketState;
@@ -175,12 +174,6 @@ public class AjpProcessor extends Abstra
/**
- * Host name (used to avoid useless B2C conversion on the host name).
- */
- private char[] hostNameC = new char[0];
-
-
- /**
* Temp message bytes used for processing.
*/
private final MessageBytes tmpMB = MessageBytes.newInstance();
@@ -866,69 +859,20 @@ public class AjpProcessor extends Abstra
/**
- * Parse host.
+ * {@inheritDoc}
+ * <p>
+ * This implementation populates the server name and port from the local
+ * name and port provided by the AJP message.
*/
- private void parseHost(MessageBytes valueMB) {
-
- if (valueMB == null || valueMB.isNull()) {
- // No host information (HTTP/1.0)
- // Ensure the local port field is populated and then use it.
- request.action(ActionCode.REQ_LOCALPORT_ATTRIBUTE, request);
- request.setServerPort(request.getLocalPort());
- try {
- request.serverName().duplicate(request.localName());
- } catch (IOException e) {
- response.setStatus(400);
- setErrorState(ErrorState.CLOSE_CLEAN, e);
- }
- return;
- }
-
- ByteChunk valueBC = valueMB.getByteChunk();
- byte[] valueB = valueBC.getBytes();
- int valueL = valueBC.getLength();
- int valueS = valueBC.getStart();
- int colonPos = -1;
- if (hostNameC.length < valueL) {
- hostNameC = new char[valueL];
- }
-
- boolean ipv6 = (valueB[valueS] == '[');
- boolean bracketClosed = false;
- for (int i = 0; i < valueL; i++) {
- char b = (char) valueB[i + valueS];
- hostNameC[i] = b;
- if (b == ']') {
- bracketClosed = true;
- } else if (b == ':') {
- if (!ipv6 || bracketClosed) {
- colonPos = i;
- break;
- }
- }
- }
-
- if (colonPos < 0) {
- request.serverName().setChars(hostNameC, 0, valueL);
- } else {
-
- request.serverName().setChars(hostNameC, 0, colonPos);
-
- int port = 0;
- int mult = 1;
- for (int i = valueL - 1; i > colonPos; i--) {
- int charValue = HexUtils.getDec(valueB[i + valueS]);
- if (charValue == -1) {
- // Invalid character
- // 400 - Bad request
- response.setStatus(400);
- setErrorState(ErrorState.CLOSE_CLEAN, null);
- break;
- }
- port = port + (charValue * mult);
- mult = 10 * mult;
- }
- request.setServerPort(port);
+ @Override
+ protected void populateHost() {
+ // No host information (HTTP/1.0)
+ request.setServerPort(request.getLocalPort());
+ try {
+ request.serverName().duplicate(request.localName());
+ } catch (IOException e) {
+ response.setStatus(400);
+ setErrorState(ErrorState.CLOSE_CLEAN, e);
}
}
Modified: tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java?rev=1822644&r1=1822643&r2=1822644&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java Tue Jan 30
13:59:11 2018
@@ -48,11 +48,9 @@ import org.apache.juli.logging.LogFactor
import org.apache.tomcat.util.ExceptionUtils;
import org.apache.tomcat.util.buf.Ascii;
import org.apache.tomcat.util.buf.ByteChunk;
-import org.apache.tomcat.util.buf.HexUtils;
import org.apache.tomcat.util.buf.MessageBytes;
import org.apache.tomcat.util.http.FastHttpDateFormat;
import org.apache.tomcat.util.http.MimeHeaders;
-import org.apache.tomcat.util.http.parser.Host;
import org.apache.tomcat.util.log.UserDataHelper;
import org.apache.tomcat.util.net.AbstractEndpoint.Handler.SocketState;
import org.apache.tomcat.util.net.SSLSupport;
@@ -133,12 +131,6 @@ public class Http11Processor extends Abs
/**
- * Host name (used to avoid useless B2C conversion on the host name).
- */
- private char[] hostNameC = new char[0];
-
-
- /**
* Instance of the new protocol to use after the HTTP connection has been
* upgraded.
*/
@@ -973,94 +965,22 @@ public class Http11Processor extends Abs
}
}
+
/**
- * Parse host.
+ * {@inheritDoc}
+ * <p>
+ * This implementation provides the server name from the default host and
+ * the server port from the local port.
*/
- private void parseHost(MessageBytes valueMB) {
-
- if (valueMB == null || valueMB.isNull()) {
- // No host information (HTTP/1.0)
- // Ensure the local port field is populated and then use it.
- request.action(ActionCode.REQ_LOCALPORT_ATTRIBUTE, request);
- request.setServerPort(request.getLocalPort());
-
- // request.serverName() will be set to the default host name by the
- // mapper
- return;
- }
-
- ByteChunk valueBC = valueMB.getByteChunk();
- byte[] valueB = valueBC.getBytes();
- int valueL = valueBC.getLength();
- int valueS = valueBC.getStart();
- int colonPos = -1;
- if (hostNameC.length < valueL) {
- hostNameC = new char[valueL];
- }
-
- // TODO
- // To minimise breakage to existing systems, just report any errors. In
- // a future release this will switch to returning a 400 response.
- // Depending on user feedback, the 400 response may be made optional.
- try {
- Host.parse(valueMB);
- } catch (IOException | IllegalArgumentException e) {
- // IOException should never happen
- // IllegalArgumentException indicates that the host name is invalid
- UserDataHelper.Mode logMode = userDataHelper.getNextMode();
- if (logMode != null) {
- String message = sm.getString("http11processor.host.parse",
- valueMB.toString(), e.getMessage());
- switch (logMode) {
- case INFO_THEN_DEBUG:
- message += sm.getString("http11processor.fallToDebug");
- //$FALL-THROUGH$
- case INFO:
- log.info(message, e);
- break;
- case DEBUG:
- log.debug(message, e);
- }
- }
- }
-
- boolean ipv6 = (valueB[valueS] == '[');
- boolean bracketClosed = false;
- for (int i = 0; i < valueL; i++) {
- char b = (char) valueB[i + valueS];
- hostNameC[i] = b;
- if (b == ']') {
- bracketClosed = true;
- } else if (b == ':') {
- if (!ipv6 || bracketClosed) {
- colonPos = i;
- break;
- }
- }
- }
-
- if (colonPos < 0) {
- request.serverName().setChars(hostNameC, 0, valueL);
- } else {
- request.serverName().setChars(hostNameC, 0, colonPos);
-
- int port = 0;
- int mult = 1;
- for (int i = valueL - 1; i > colonPos; i--) {
- int charValue = HexUtils.getDec(valueB[i + valueS]);
- if (charValue == -1 || charValue > 9) {
- // Invalid character
- // 400 - Bad request
- response.setStatus(400);
- setErrorState(ErrorState.CLOSE_CLEAN, null);
- break;
- }
- port = port + (charValue * mult);
- mult = 10 * mult;
- }
- request.setServerPort(port);
- }
+ @Override
+ protected void populateHost() {
+ // No host information (HTTP/1.0)
+ // Ensure the local port field is populated before using it.
+ request.action(ActionCode.REQ_LOCALPORT_ATTRIBUTE, request);
+ request.setServerPort(request.getLocalPort());
+ // request.serverName() will be set to the default host name by the
+ // mapper
}
Modified: tomcat/trunk/java/org/apache/coyote/http11/LocalStrings.properties
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/LocalStrings.properties?rev=1822644&r1=1822643&r2=1822644&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/LocalStrings.properties
(original)
+++ tomcat/trunk/java/org/apache/coyote/http11/LocalStrings.properties Tue Jan
30 13:59:11 2018
@@ -19,7 +19,6 @@ abstractHttp11Protocol.httpUpgradeConfig
http11processor.fallToDebug=\n Note: further occurrences of HTTP request
parsing errors will be logged at DEBUG level.
http11processor.header.parse=Error parsing HTTP request header
-http11processor.host.parse=The host header [{0}] failed validation with the
error [{1}]. Processing of the request will continue but Tomcat will reject
these requests with a 400 response in a future release.
http11processor.neverused=This method should never be used
http11processor.request.inconsistentHosts=The host specified in the request
line is not consistent with the host header
http11processor.request.multipleHosts=The request contained multiple host
headers
Modified: tomcat/trunk/java/org/apache/coyote/http2/LocalStrings.properties
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http2/LocalStrings.properties?rev=1822644&r1=1822643&r2=1822644&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http2/LocalStrings.properties (original)
+++ tomcat/trunk/java/org/apache/coyote/http2/LocalStrings.properties Tue Jan
30 13:59:11 2018
@@ -79,6 +79,7 @@ stream.header.connection=Connection [{0}
stream.header.contentLength=Connection [{0}], Stream [{1}], The content length
header value [{2}] does not agree with the size of the data received [{3}]
stream.header.debug=Connection [{0}], Stream [{1}], HTTP header [{2}], Value
[{3}]
stream.header.duplicate=Connection [{0}], Stream [{1}], received multiple
[{3}] headers
+stream.header.invalid=Connection [{0}], Stream [{1}], The header [{2}]
contained invalid date [{3}]
stream.header.noPath=Connection [{0}], Stream [{1}], The [:path] pseudo header
was empty
stream.header.required=Connection [{0}], Stream [{1}], One or more required
headers was missing
stream.header.te=Connection [{0}], Stream [{1}], HTTP header [te] is not
permitted to have the value [{2}] in an HTTP/2 request
Modified: tomcat/trunk/java/org/apache/coyote/http2/Stream.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http2/Stream.java?rev=1822644&r1=1822643&r2=1822644&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http2/Stream.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http2/Stream.java Tue Jan 30 13:59:11
2018
@@ -41,6 +41,7 @@ import org.apache.juli.logging.LogFactor
import org.apache.tomcat.util.buf.ByteChunk;
import org.apache.tomcat.util.buf.MessageBytes;
import org.apache.tomcat.util.http.MimeHeaders;
+import org.apache.tomcat.util.http.parser.Host;
import org.apache.tomcat.util.net.ApplicationBufferHandler;
import org.apache.tomcat.util.res.StringManager;
@@ -325,7 +326,14 @@ class Stream extends AbstractStream impl
}
case ":authority": {
if (coyoteRequest.serverName().isNull()) {
- int i = value.lastIndexOf(':');
+ int i;
+ try {
+ i = Host.parse(value);
+ } catch (IllegalArgumentException iae) {
+ // Host value invalid
+ throw new
HpackException(sm.getString("stream.header.invalid",
+ getConnectionId(), getIdentifier(), ":authority",
value));
+ }
if (i > -1) {
coyoteRequest.serverName().setString(value.substring(0,
i));
coyoteRequest.setServerPort(Integer.parseInt(value.substring(i + 1)));
Modified: tomcat/trunk/java/org/apache/tomcat/util/http/parser/Host.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/http/parser/Host.java?rev=1822644&r1=1822643&r2=1822644&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/http/parser/Host.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/http/parser/Host.java Tue Jan 30
13:59:11 2018
@@ -35,10 +35,8 @@ public class Host {
*
* @throws IllegalArgumentException If the host header value is not
* specification compliant
- *
- * @throws IOException If a problem occurs reading the data from the input
*/
- public static int parse(MessageBytes mb) throws IOException {
+ public static int parse(MessageBytes mb) {
return parse(new MessageBytesReader(mb));
}
@@ -53,27 +51,30 @@ public class Host {
*
* @throws IllegalArgumentException If the host header value is not
* specification compliant
- *
- * @throws IOException If a problem occurs reading the data from the input
*/
- public static int parse(String string) throws IOException {
+ public static int parse(String string) {
return parse(new StringReader(string));
}
- private static int parse(Reader reader) throws IOException {
- reader.mark(1);
- int first = reader.read();
- reader.reset();
- if (HttpParser.isAlpha(first)) {
- return HttpParser.readHostDomainName(reader);
- } else if (HttpParser.isNumeric(first)) {
- return HttpParser.readHostIPv4(reader, false);
- } else if ('[' == first) {
- return HttpParser.readHostIPv6(reader);
- } else {
- // Invalid
- throw new IllegalArgumentException();
+ private static int parse(Reader reader) {
+ try {
+ reader.mark(1);
+ int first = reader.read();
+ reader.reset();
+ if (HttpParser.isAlpha(first)) {
+ return HttpParser.readHostDomainName(reader);
+ } else if (HttpParser.isNumeric(first)) {
+ return HttpParser.readHostIPv4(reader, false);
+ } else if ('[' == first) {
+ return HttpParser.readHostIPv6(reader);
+ } else {
+ // Invalid
+ throw new IllegalArgumentException();
+ }
+ } catch (IOException ioe) {
+ // Should never happen
+ throw new IllegalArgumentException(ioe);
}
}
Modified: tomcat/trunk/webapps/docs/changelog.xml
URL:
http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1822644&r1=1822643&r2=1822644&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Tue Jan 30 13:59:11 2018
@@ -125,6 +125,11 @@
local port rather than the connector configuration since the configured
value maybe zero. (markt)
</fix>
+ <add>
+ Enable strict validation of the provided host name and port for all
+ connectors. Requests with invalid host names and/or ports will be
+ rejected with a 400 response. (markt)
+ </add>
</changelog>
</subsection>
<subsection name="Jasper">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
