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

Introduce HttpCompliance.MISMATCHED_AUTHORITY #9312

Closed
wants to merge 6 commits into from
Closed

Introduce HttpCompliance.MISMATCHED_AUTHORITY #9312

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
73b52b2
Introduce HttpCompliance.MISMATCHED_AUTHORITY
joakime Feb 3, 2023
7c266fc
Update ForwardedRequestCustomizerTest
joakime Feb 3, 2023
9e39c4f
Expand RequestTest
joakime Feb 3, 2023
bfc73b1
Update HttpCompliance.RFC2616
joakime Feb 3, 2023
a663b40
Update NcsaRequestLogTest.testAbsolute
joakime Feb 3, 2023
8c243e5
Use RFC2616 mode in RFC2616 tests
joakime Feb 3, 2023
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
15 changes: 13 additions & 2 deletions jetty-http/src/main/java/org/eclipse/jetty/http/HttpCompliance.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,14 @@ public enum Violation implements ComplianceViolation
* 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 <a href="https://www.rfc-editor.org/rfc/rfc7230#section-5.4">RFC 7230: Section 5.4</a>, 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;
Expand Down Expand Up @@ -162,7 +169,11 @@ public String getDescription()
* The HttpCompliance mode that supports <a href="https://tools.ietf.org/html/rfc2616">RFC 7230</a>
* 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.
Expand Down
31 changes: 21 additions & 10 deletions jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import static org.eclipse.jetty.http.HttpCompliance.Violation.CASE_SENSITIVE_FIELD_NAME;
import static org.eclipse.jetty.http.HttpCompliance.Violation.DUPLICATE_HOST_HEADERS;
import static org.eclipse.jetty.http.HttpCompliance.Violation.HTTP_0_9;
import static org.eclipse.jetty.http.HttpCompliance.Violation.MISMATCHED_AUTHORITY;
import static org.eclipse.jetty.http.HttpCompliance.Violation.MULTIPLE_CONTENT_LENGTHS;
import static org.eclipse.jetty.http.HttpCompliance.Violation.NO_COLON_AFTER_FIELD_NAME;
import static org.eclipse.jetty.http.HttpCompliance.Violation.TRANSFER_ENCODING_WITH_CONTENT_LENGTH;
Expand Down Expand Up @@ -1037,22 +1038,32 @@ else if (_endOfContent == EndOfContent.CHUNKED_CONTENT)
LOG.warn("Encountered multiple `Host` headers. Previous `Host` header already seen as `{}`, new `Host` header has appeared as `{}`", _parsedHost, _valueString);
checkViolation(DUPLICATE_HOST_HEADERS);
}
if (!MISMATCHED_AUTHORITY.isAllowedBy(_complianceMode))
{
HttpURI httpURI = HttpURI.build().uri(_method == null ? null : _method.toString(), _uri.toString());
Copy link
Contributor

Choose a reason for hiding this comment

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

We really don't want to go to the effort here build a HttpURI here, only to throw it away and then in fact we have already started building the exact same HttpURI in org.eclipse.jetty.server.HttpChannelOverHttp#startRequest.

Option A) is to change the RequestHandler signature to

    public interface RequestHandler extends HttpHandler
    {
        /**
         * This is the method called by parser when the HTTP request line is parsed
         *
         * @param method The method
         * @param uri The raw bytes of the URI.
         * @param version the http version in use
         * @deprecated use {@link #startRequest(String, HttpURI, HttpVersion)}
         */
        @Deprecated
        default void startRequest(String method, String uri, HttpVersion version)
        {}
        
        /**
         * This is the method called by parser when the HTTP request line is parsed
         *
         * @param method The method
         * @param uri The raw bytes of the URI.
         * @param version the http version in use
         */
        default void startRequest(String method, HttpURI uri, HttpVersion version)
        {
            startRequest(method, uri.toString(), version);
        }
    }

We can then have a recycled HttpURI.Mutable in the parser and build the URI there, so it is available.

Option B) is to move this check to HttpChannelOverHttp

I think option A is OK. Let me do some experiments and I may have a diff for you....

Copy link
Contributor

Choose a reason for hiding this comment

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

@joakime I have created branch jetty-10-9312-HttpParser-HttpURI that has this updated signature.
The only issue is that the field is a Mutable, and we take it as an Immutable as we call startRequest... and that immutable is not available when you need to do this check. So perhaps we need yet another field to remember the immutable?

if (httpURI.isAbsolute())
{
if (!httpURI.getAuthority().equalsIgnoreCase(_valueString))
{
if (LOG.isWarnEnabled())
LOG.warn("Mismatched Authority: request-line authority is [{}], Host header is [{}]", httpURI.getHost(), _valueString);
throw new BadMessageException(MISMATCHED_AUTHORITY.getDescription());
}
}
}
_parsedHost = _valueString;
if (!(_field instanceof HostPortHttpField) && _valueString != null && !_valueString.isEmpty())
{
HostPort hostPort;
if (UNSAFE_HOST_HEADER.isAllowedBy(_complianceMode))
{
_field = new HostPortHttpField(_header,
CASE_SENSITIVE_FIELD_NAME.isAllowedBy(_complianceMode) ? _headerString : _header.asString(),
HostPort.unsafe(_valueString));
}
hostPort = HostPort.unsafe(_valueString);
else
{
_field = new HostPortHttpField(_header,
CASE_SENSITIVE_FIELD_NAME.isAllowedBy(_complianceMode) ? _headerString : _header.asString(),
_valueString);
}
hostPort = new HostPort(_valueString);

_field = new HostPortHttpField(_header,
CASE_SENSITIVE_FIELD_NAME.isAllowedBy(_complianceMode) ? _headerString : _header.asString(),
hostPort);

addToFieldCache = _fieldCache.isEnabled();
}
break;
Expand Down
35 changes: 35 additions & 0 deletions jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1964,6 +1964,41 @@ public void testUriHost10()
assertEquals(0, _port);
}

@Test
public void testUriHost11MismatchDeny()
{
ByteBuffer buffer = BufferUtil.toBuffer(
"GET http://host/ HTTP/1.1\r\n" +
"Host: foo\r\n" + // different authority
"Connection: close\r\n" +
"\r\n");

HttpParser.RequestHandler handler = new Handler();
HttpParser parser = new HttpParser(handler);
parser.parseNext(buffer);
assertEquals("Mismatched Authority", _bad);
assertEquals("http://host/", _uriOrStatus);
assertEquals(0, _port);
}

@Test
public void testUriHost11MismatchAllow()
{
ByteBuffer buffer = BufferUtil.toBuffer(
"GET http://host/ HTTP/1.1\r\n" +
"Host: foo\r\n" + // different authority
"Connection: close\r\n" +
"\r\n");

HttpParser.RequestHandler handler = new Handler();
HttpCompliance httpCompliance = HttpCompliance.from("RFC7230,MISMATCHED_AUTHORITY");
HttpParser parser = new HttpParser(handler, httpCompliance);
parser.parseNext(buffer);
assertNull(_bad);
assertEquals("http://host/", _uriOrStatus);
assertEquals(0, _port);
}

@Test
public void testNoHost()
{
Expand Down
96 changes: 48 additions & 48 deletions jetty-server/src/test/java/org/eclipse/jetty/server/ForwardedRequestCustomizerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,24 +155,24 @@ public static Stream<Arguments> cases()
new Request("HTTP/1.0 - No Host header, with X-Forwarded-Host")
.headers(
"GET /example HTTP/1.0",
"X-Forwarded-Host: alt.example.net:7070"
"X-Forwarded-Host: alt.example.org:7070"
),
new Expectations()
.scheme("http").serverName("alt.example.net").serverPort(7070)
.scheme("http").serverName("alt.example.org").serverPort(7070)
.secure(false)
.requestURL("http://alt.example.net:7070/example")
.requestURL("http://alt.example.org:7070/example")
),
Arguments.of(
new Request("HTTP/1.0 - Empty Host header, with X-Forwarded-Host")
.headers(
"GET /example HTTP/1.0",
"Host:",
"X-Forwarded-Host: alt.example.net:7070"
"X-Forwarded-Host: alt.example.org:7070"
),
new Expectations()
.scheme("http").serverName("alt.example.net").serverPort(7070)
.scheme("http").serverName("alt.example.org").serverPort(7070)
.secure(false)
.requestURL("http://alt.example.net:7070/example")
.requestURL("http://alt.example.org:7070/example")
),
// Host IPv4
Arguments.of(
Expand All @@ -199,7 +199,7 @@ public static Stream<Arguments> cases()
Arguments.of(new Request("IPv4 in Request Line")
.headers(
"GET https://1.2.3.4:2222/ HTTP/1.1",
"Host: wrong"
"Host: 1.2.3.4:2222"
),
new Expectations()
.scheme("https").serverName("1.2.3.4").serverPort(2222)
Expand All @@ -209,7 +209,7 @@ public static Stream<Arguments> cases()
Arguments.of(new Request("IPv6 in Request Line")
.headers(
"GET http://[::1]:2222/ HTTP/1.1",
"Host: wrong"
"Host: [::1]:2222"
),
new Expectations()
.scheme("http").serverName("[::1]").serverPort(2222)
Expand Down Expand Up @@ -749,25 +749,25 @@ public static Stream<Arguments> cases()
new Request("RFC7239 - mixed with HTTP/1.0 - No Host header")
.headers(
"GET /example HTTP/1.0",
"Forwarded: for=1.1.1.1:6060,proto=http;host=alt.example.net:7070"
"Forwarded: for=1.1.1.1:6060,proto=http;host=alt.example.org:7070"
),
new Expectations()
.scheme("http").serverName("alt.example.net").serverPort(7070)
.scheme("http").serverName("alt.example.org").serverPort(7070)
.secure(false)
.requestURL("http://alt.example.net:7070/example")
.requestURL("http://alt.example.org:7070/example")
.remoteAddr("1.1.1.1").remotePort(6060)
),
Arguments.of(
new Request("RFC7239 - mixed with HTTP/1.0 - Empty Host header")
.headers(
"GET /example HTTP/1.0",
"Host:",
"Forwarded: for=1.1.1.1:6060,proto=http;host=alt.example.net:7070"
"Forwarded: for=1.1.1.1:6060,proto=http;host=alt.example.org:7070"
),
new Expectations()
.scheme("http").serverName("alt.example.net").serverPort(7070)
.scheme("http").serverName("alt.example.org").serverPort(7070)
.secure(false)
.requestURL("http://alt.example.net:7070/example")
.requestURL("http://alt.example.org:7070/example")
.remoteAddr("1.1.1.1").remotePort(6060)
),
// =================================================================
Expand Down Expand Up @@ -890,15 +890,15 @@ public static Stream<Arguments> cases()
Arguments.of(new Request("https initial authority, X-Forwarded-Proto on http, Proxy-Ssl-Id exists (setSslIsSecure==false)")
.configureCustomizer((customizer) -> customizer.setSslIsSecure(false))
.headers(
"GET https://alt.example.net/foo HTTP/1.1",
"Host: myhost",
"GET https://alt.example.org/foo HTTP/1.1",
"Host: alt.example.org",
"X-Forwarded-Proto: http",
"Proxy-Ssl-Id: Wibble"
),
new Expectations()
.scheme("http").serverName("alt.example.net").serverPort(80)
.scheme("http").serverName("alt.example.org").serverPort(80)
.secure(false)
.requestURL("http://alt.example.net/foo")
.requestURL("http://alt.example.org/foo")
.remoteAddr("0.0.0.0").remotePort(0)
.sslSession("Wibble")
),
Expand All @@ -920,63 +920,63 @@ public static Stream<Arguments> cases()
Arguments.of(new Request("Https initial authority, X-Proxied-Https off, Proxy-Ssl-Id exists (setSslIsSecure==true)")
.configureCustomizer((customizer) -> customizer.setSslIsSecure(true))
.headers(
"GET https://alt.example.net/foo HTTP/1.1",
"Host: myhost",
"GET https://alt.example.org/foo HTTP/1.1",
"Host: alt.example.org",
"X-Proxied-Https: off", // this wins for scheme and secure
"Proxy-Ssl-Id: Wibble"
),
new Expectations()
.scheme("http").serverName("alt.example.net").serverPort(80)
.scheme("http").serverName("alt.example.org").serverPort(80)
.secure(false)
.requestURL("http://alt.example.net/foo")
.requestURL("http://alt.example.org/foo")
.remoteAddr("0.0.0.0").remotePort(0)
.sslSession("Wibble")
),
Arguments.of(new Request("Https initial authority, X-Proxied-Https off, Proxy-Ssl-Id exists (setSslIsSecure==true) (alt order)")
.configureCustomizer((customizer) -> customizer.setSslIsSecure(true))
.headers(
"GET https://alt.example.net/foo HTTP/1.1",
"Host: myhost",
"GET https://alt.example.org/foo HTTP/1.1",
"Host: alt.example.org",
"Proxy-Ssl-Id: Wibble",
"X-Proxied-Https: off" // this wins for scheme and secure
),
new Expectations()
.scheme("http").serverName("alt.example.net").serverPort(80)
.scheme("http").serverName("alt.example.org").serverPort(80)
.secure(false)
.requestURL("http://alt.example.net/foo")
.requestURL("http://alt.example.org/foo")
.remoteAddr("0.0.0.0").remotePort(0)
.sslSession("Wibble")
),
Arguments.of(new Request("Http initial authority, X-Proxied-Https off, Proxy-Ssl-Id exists (setSslIsSecure==false)")
.configureCustomizer((customizer) -> customizer.setSslIsSecure(false))
.headers(
"GET https://alt.example.net/foo HTTP/1.1",
"Host: myhost",
"GET https://alt.example.org/foo HTTP/1.1",
"Host: alt.example.org",
"X-Proxied-Https: off",
"Proxy-Ssl-Id: Wibble",
"Proxy-auth-cert: 0123456789abcdef"
),
new Expectations()
.scheme("http").serverName("alt.example.net").serverPort(80)
.scheme("http").serverName("alt.example.org").serverPort(80)
.secure(false)
.requestURL("http://alt.example.net/foo")
.requestURL("http://alt.example.org/foo")
.remoteAddr("0.0.0.0").remotePort(0)
.sslSession("Wibble")
.sslCertificate("0123456789abcdef")
),
Arguments.of(new Request("Http initial authority, X-Proxied-Https off, Proxy-Ssl-Id exists (setSslIsSecure==false) (alt)")
.configureCustomizer((customizer) -> customizer.setSslIsSecure(false))
.headers(
"GET https://alt.example.net/foo HTTP/1.1",
"Host: myhost",
"GET https://alt.example.org/foo HTTP/1.1",
"Host: alt.example.org",
"Proxy-Ssl-Id: Wibble",
"Proxy-auth-cert: 0123456789abcdef",
"X-Proxied-Https: off"
),
new Expectations()
.scheme("http").serverName("alt.example.net").serverPort(80)
.scheme("http").serverName("alt.example.org").serverPort(80)
.secure(false)
.requestURL("http://alt.example.net/foo")
.requestURL("http://alt.example.org/foo")
.remoteAddr("0.0.0.0").remotePort(0)
.sslSession("Wibble")
.sslCertificate("0123456789abcdef")
Expand Down Expand Up @@ -1026,38 +1026,38 @@ public static Stream<Arguments> nonStandardPortCases()
Arguments.of(new Request("RFC7239 with https and h2")
.headers(
"GET /test/forwarded.jsp HTTP/1.1",
"Host: web.example.net",
"Forwarded: for=192.168.2.6;host=web.example.net;proto=https;proto-version=h2"
"Host: web.example.org",
"Forwarded: for=192.168.2.6;host=web.example.org;proto=https;proto-version=h2"
),
new Expectations()
.scheme("https").serverName("web.example.net").serverPort(443)
.requestURL("https://web.example.net/test/forwarded.jsp")
.scheme("https").serverName("web.example.org").serverPort(443)
.requestURL("https://web.example.org/test/forwarded.jsp")
.remoteAddr("192.168.2.6").remotePort(0)
),
// RFC7239 Tests with https and proxy provided port
Arguments.of(new Request("RFC7239 with proxy provided port on https and h2")
.headers(
"GET /test/forwarded.jsp HTTP/1.1",
"Host: web.example.net:9443",
"Forwarded: for=192.168.2.6;host=web.example.net:9443;proto=https;proto-version=h2"
"Host: web.example.org:9443",
"Forwarded: for=192.168.2.6;host=web.example.org:9443;proto=https;proto-version=h2"
),
new Expectations()
.scheme("https").serverName("web.example.net").serverPort(9443)
.requestURL("https://web.example.net:9443/test/forwarded.jsp")
.scheme("https").serverName("web.example.org").serverPort(9443)
.requestURL("https://web.example.org:9443/test/forwarded.jsp")
.remoteAddr("192.168.2.6").remotePort(0)
),
// RFC7239 Tests with https, no port in Host, but proxy provided port
Arguments.of(new Request("RFC7239 with client provided host and different proxy provided port on https and h2")
.headers(
"GET /test/forwarded.jsp HTTP/1.1",
"Host: web.example.net",
"Forwarded: for=192.168.2.6;host=new.example.net:7443;proto=https;proto-version=h2"
// Client: https://web.example.net/test/forwarded.jsp
// Proxy Requests: https://new.example.net/test/forwarded.jsp
"Host: web.example.org",
"Forwarded: for=192.168.2.6;host=new.example.org:7443;proto=https;proto-version=h2"
// Client: https://web.example.org/test/forwarded.jsp
// Proxy Requests: https://new.example.org/test/forwarded.jsp
),
new Expectations()
.scheme("https").serverName("new.example.net").serverPort(7443)
.requestURL("https://new.example.net:7443/test/forwarded.jsp")
.scheme("https").serverName("new.example.org").serverPort(7443)
.requestURL("https://new.example.org:7443/test/forwarded.jsp")
.remoteAddr("192.168.2.6").remotePort(0)
)
);
Expand Down
Loading