From 5f25f5b389de60cb8dc36d3c24d8ad64843e4daa Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Tue, 14 Feb 2023 07:43:19 +1100 Subject: [PATCH] Fix/jetty 10.0.x/uri host mismatch alt (#9343) * Introduce HttpCompliance.MISMATCHED_AUTHORITY * Update HttpCompliance.RFC2616 Signed-off-by: Joakim Erdfelt * Update NcsaRequestLogTest.testAbsolute Signed-off-by: Joakim Erdfelt * Use RFC2616 mode in RFC2616 tests Signed-off-by: Joakim Erdfelt * Alternative fix for mismatched host headers This PR fixes the miss-matched host header issue in the Request.setMetaData method. This requires no change to the HttpParser. A more comprehensive fix can be considered for jetty-12. Signed-off-by: gregw * Alternative fix for mismatched host headers Updates from review Signed-off-by: gregw --------- Signed-off-by: Joakim Erdfelt Signed-off-by: gregw Co-authored-by: Joakim Erdfelt --- .../eclipse/jetty/http/HttpCompliance.java | 15 +++++++++-- .../eclipse/jetty/http/HttpParserTest.java | 1 - .../org/eclipse/jetty/server/Request.java | 27 ++++++++++++++++--- .../ForwardedRequestCustomizerTest.java | 8 ++++-- .../jetty/server/PartialRFC2616Test.java | 11 +++++++- .../org/eclipse/jetty/server/RequestTest.java | 9 +++++++ .../server/handler/NcsaRequestLogTest.java | 2 +- .../src/test/resources/RFC2616Base.xml | 6 +++++ 8 files changed, 69 insertions(+), 10 deletions(-) diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpCompliance.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpCompliance.java index 8e0a47d6924..1b9a20b9846 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpCompliance.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpCompliance.java @@ -117,7 +117,14 @@ public final class HttpCompliance implements ComplianceViolation.Mode * should reject a request if the Host headers contains an invalid / unsafe authority. * A deployment may include this violation to allow unsafe host headesr on a received request. */ - UNSAFE_HOST_HEADER("https://www.rfc-editor.org/rfc/rfc7230#section-2.7.1", "Invalid Authority"); + UNSAFE_HOST_HEADER("https://www.rfc-editor.org/rfc/rfc7230#section-2.7.1", "Invalid Authority"), + + /** + * Since RFC 7230: Section 5.4, the HTTP protocol + * must reject a request if the target URI has an authority that is different than a provided Host header. + * A deployment may include this violation to allow different values on the target URI and the Host header on a received request. + */ + MISMATCHED_AUTHORITY("https://www.rfc-editor.org/rfc/rfc7230#section-5.4", "Mismatched Authority"); private final String url; private final String description; @@ -162,7 +169,11 @@ public final class HttpCompliance implements ComplianceViolation.Mode * The HttpCompliance mode that supports RFC 7230 * with only the violations that differ from {@link #RFC7230}. */ - public static final HttpCompliance RFC2616 = new HttpCompliance("RFC2616", of(Violation.HTTP_0_9, Violation.MULTILINE_FIELD_VALUE)); + public static final HttpCompliance RFC2616 = new HttpCompliance("RFC2616", of( + Violation.HTTP_0_9, + Violation.MULTILINE_FIELD_VALUE, + Violation.MISMATCHED_AUTHORITY + )); /** * A legacy HttpCompliance mode that allows all violations except case-insensitive methods. diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java index c2bad0b9cdc..aa42d6da7df 100644 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java @@ -42,7 +42,6 @@ import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.is; -import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.nullValue; import static org.hamcrest.Matchers.startsWith; import static org.junit.jupiter.api.Assertions.assertEquals; diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java b/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java index 00575980873..7531450a12a 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java @@ -66,6 +66,7 @@ import javax.servlet.http.WebConnection; import org.eclipse.jetty.http.BadMessageException; import org.eclipse.jetty.http.ComplianceViolation; import org.eclipse.jetty.http.HostPortHttpField; +import org.eclipse.jetty.http.HttpCompliance; import org.eclipse.jetty.http.HttpCookie; import org.eclipse.jetty.http.HttpCookie.SetCookieHttpField; import org.eclipse.jetty.http.HttpField; @@ -97,6 +98,8 @@ import org.eclipse.jetty.util.UrlEncoded; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import static org.eclipse.jetty.http.HttpCompliance.Violation.MISMATCHED_AUTHORITY; + /** * Jetty Request. *

@@ -1740,9 +1743,28 @@ public class Request implements HttpServletRequest throw new BadMessageException(badMessage); } + HttpField host = getHttpFields().getField(HttpHeader.HOST); if (uri.isAbsolute() && uri.hasAuthority() && uri.getPath() != null) { _uri = uri; + if (host instanceof HostPortHttpField && !((HostPortHttpField)host).getHostPort().toString().equals(uri.getAuthority())) + { + HttpChannel httpChannel = getHttpChannel(); + HttpConfiguration httpConfiguration = httpChannel.getHttpConfiguration(); + if (httpConfiguration != null) + { + HttpCompliance httpCompliance = httpConfiguration.getHttpCompliance(); + if (httpCompliance.allows(MISMATCHED_AUTHORITY)) + { + if (httpChannel instanceof ComplianceViolation.Listener) + ((ComplianceViolation.Listener)httpChannel).onComplianceViolation(httpCompliance, MISMATCHED_AUTHORITY, _uri.toString()); + } + else + { + throw new BadMessageException(400, "Mismatched Authority"); + } + } + } } else { @@ -1756,10 +1778,9 @@ public class Request implements HttpServletRequest if (!uri.hasAuthority()) { - HttpField field = getHttpFields().getField(HttpHeader.HOST); - if (field instanceof HostPortHttpField) + if (host instanceof HostPortHttpField) { - HostPortHttpField authority = (HostPortHttpField)field; + HostPortHttpField authority = (HostPortHttpField)host; builder.host(authority.getHost()).port(authority.getPort()); } diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/ForwardedRequestCustomizerTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/ForwardedRequestCustomizerTest.java index ac7939601bd..b4755d38f89 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/ForwardedRequestCustomizerTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/ForwardedRequestCustomizerTest.java @@ -23,6 +23,7 @@ import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.eclipse.jetty.http.HttpCompliance; import org.eclipse.jetty.http.HttpTester; import org.eclipse.jetty.server.handler.AbstractHandler; import org.junit.jupiter.api.AfterEach; @@ -38,7 +39,6 @@ import static org.junit.jupiter.api.Assertions.assertTrue; public class ForwardedRequestCustomizerTest { private Server server; - private RequestHandler handler; private LocalConnector connector; private LocalConnector connectorAlt; private LocalConnector connectorConfigured; @@ -68,6 +68,8 @@ public class ForwardedRequestCustomizerTest // Default behavior Connector HttpConnectionFactory http = new HttpConnectionFactory(); + HttpCompliance mismatchedAuthorityHttpCompliance = HttpCompliance.RFC7230.with("Mismatched_Authority", HttpCompliance.Violation.MISMATCHED_AUTHORITY); + http.getHttpConfiguration().setHttpCompliance(mismatchedAuthorityHttpCompliance); http.getHttpConfiguration().setSecurePort(443); customizer = new ForwardedRequestCustomizer(); http.getHttpConfiguration().addCustomizer(customizer); @@ -77,6 +79,7 @@ public class ForwardedRequestCustomizerTest // Alternate behavior Connector HttpConnectionFactory httpAlt = new HttpConnectionFactory(); httpAlt.getHttpConfiguration().setSecurePort(8443); + httpAlt.getHttpConfiguration().setHttpCompliance(mismatchedAuthorityHttpCompliance); customizerAlt = new ForwardedRequestCustomizer(); httpAlt.getHttpConfiguration().addCustomizer(customizerAlt); connectorAlt = new LocalConnector(server, httpAlt); @@ -97,9 +100,10 @@ public class ForwardedRequestCustomizerTest http.getHttpConfiguration().addCustomizer(customizerConfigured); connectorConfigured = new LocalConnector(server, http); + connectorConfigured.getConnectionFactory(HttpConnectionFactory.class).getHttpConfiguration().setHttpCompliance(mismatchedAuthorityHttpCompliance); server.addConnector(connectorConfigured); - handler = new RequestHandler(); + RequestHandler handler = new RequestHandler(); server.setHandler(handler); handler.requestTester = (request, response) -> diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/PartialRFC2616Test.java b/jetty-server/src/test/java/org/eclipse/jetty/server/PartialRFC2616Test.java index 286eee66050..31f689fcbb1 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/PartialRFC2616Test.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/PartialRFC2616Test.java @@ -17,6 +17,7 @@ import java.util.Date; import java.util.List; import java.util.concurrent.TimeUnit; +import org.eclipse.jetty.http.HttpCompliance; import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.http.HttpParser; @@ -47,6 +48,7 @@ public class PartialRFC2616Test { server = new Server(); connector = new LocalConnector(server); + connector.getConnectionFactory(HttpConfiguration.ConnectionFactory.class).getHttpConfiguration().setHttpCompliance(HttpCompliance.RFC2616); connector.setIdleTimeout(10000); server.addConnector(connector); @@ -403,12 +405,19 @@ public class PartialRFC2616Test } + /** + * @throws Exception + * @see RFC 2616 - Section 5.2 - The Resource Identified by a Request + */ @Test public void test521() throws Exception { // Default Host int offset = 0; - String response = connector.getResponse("GET http://VirtualHost:8888/path/R1 HTTP/1.1\n" + "Host: wronghost\n" + "Connection: close\n" + "\n"); + String response = connector.getResponse( + "GET http://VirtualHost:8888/path/R1 HTTP/1.1\n" + + "Host: wronghost\n" + + "Connection: close\n" + "\n"); offset = checkContains(response, offset, "HTTP/1.1 200", "Virtual host") + 1; offset = checkContains(response, offset, "Virtual Dump", "Virtual host") + 1; offset = checkContains(response, offset, "pathInfo=/path/R1", "Virtual host") + 1; diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java index 1c5b6ea942e..ccea038e573 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java @@ -969,6 +969,15 @@ public class RequestTest "Connection: close\n" + "\n"); i = 0; + assertThat(response, containsString("400 Bad Request")); + + results.clear(); + response = _connector.getResponse( + "GET http://myhost:8888/ HTTP/1.1\n" + + "Host: myhost:8888\n" + + "Connection: close\n" + + "\n"); + i = 0; assertThat(response, containsString("200 OK")); assertEquals("http://myhost:8888/", results.get(i++)); assertEquals("0.0.0.0", results.get(i++)); diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/handler/NcsaRequestLogTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/handler/NcsaRequestLogTest.java index e64fcc9129b..f2d0edb9b46 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/handler/NcsaRequestLogTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/handler/NcsaRequestLogTest.java @@ -190,7 +190,7 @@ public class NcsaRequestLogTest _connector.getResponse( "GET http://hostname:8888/foo?name=value HTTP/1.1\n" + - "Host: servername\n" + + "Host: hostname:8888\n" + "\n"); String log = _entries.poll(5, TimeUnit.SECONDS); assertThat(log, containsString("GET http://hostname:8888/foo?name=value")); diff --git a/tests/test-integration/src/test/resources/RFC2616Base.xml b/tests/test-integration/src/test/resources/RFC2616Base.xml index aad3e838d43..449b0174946 100644 --- a/tests/test-integration/src/test/resources/RFC2616Base.xml +++ b/tests/test-integration/src/test/resources/RFC2616Base.xml @@ -22,6 +22,12 @@ false 1024 + + + RFC2616 + + +