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

Jump to bottom

Configurable Unsafe Host Header Behaviors #9283

Merged
merged 9 commits into from
Feb 3, 2023
Merged

Configurable Unsafe Host Header Behaviors #9283

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
3129163
Adding HttpCompliance.DUPLICATE_HOST_HEADERS
joakime Feb 1, 2023
e400ce8
Adding HttpCompliance.UNSAFE_HOST_HEADER
joakime Feb 1, 2023
849a097
Adding warning logging for bad Host / authority situations
joakime Dec 22, 2022
2ac0652
Fixing checkstyle violation
joakime Feb 1, 2023
7907d89
Fixing HttpParserTest.testDuplicateHostAllow
joakime Feb 1, 2023
98a7268
Updates from review
joakime Feb 1, 2023
de72f2f
Reworked HostPort.unsafe() handling and test cases per review
joakime Feb 2, 2023
58c2573
Clean up implementation
joakime Feb 2, 2023
1215dba
Add more comments
joakime Feb 2, 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
16 changes: 15 additions & 1 deletion 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 @@ -103,7 +103,21 @@ public enum Violation implements ComplianceViolation
* line of a single token with neither a colon nor value following, to be interpreted as a field name with no value.
* A deployment may include this violation to allow such fields to be in a received request.
*/
NO_COLON_AFTER_FIELD_NAME("https://tools.ietf.org/html/rfc7230#section-3.2", "Fields must have a Colon");
NO_COLON_AFTER_FIELD_NAME("https://tools.ietf.org/html/rfc7230#section-3.2", "Fields must have a Colon"),

/**
* Since <a href="https://www.rfc-editor.org/rfc/rfc7230#section-5.4">RFC 7230: Section 5.4</a>, the HTTP protocol
* says that a Server must reject a request duplicate host headers.
* A deployment may include this violation to allow duplicate host headers on a received request.
*/
DUPLICATE_HOST_HEADERS("https://www.rfc-editor.org/rfc/rfc7230#section-5.4", "Duplicate Host Header"),

/**
* Since <a href="https://www.rfc-editor.org/rfc/rfc7230#section-2.7.1">RFC 7230</a>, the HTTP protocol
* 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");

private final String url;
private final String description;
Expand Down
37 changes: 28 additions & 9 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 @@ -24,6 +24,7 @@

import org.eclipse.jetty.http.HttpTokens.EndOfContent;
import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.HostPort;
import org.eclipse.jetty.util.Index;
import org.eclipse.jetty.util.StringUtil;
import org.eclipse.jetty.util.Utf8StringBuilder;
Expand All @@ -33,10 +34,12 @@
import static org.eclipse.jetty.http.HttpCompliance.RFC7230;
import static org.eclipse.jetty.http.HttpCompliance.Violation;
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.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;
import static org.eclipse.jetty.http.HttpCompliance.Violation.UNSAFE_HOST_HEADER;
import static org.eclipse.jetty.http.HttpCompliance.Violation.WHITESPACE_AFTER_FIELD_NAME;

/**
Expand Down Expand Up @@ -226,7 +229,7 @@ public enum State
private String _valueString;
private int _responseStatus;
private int _headerBytes;
private boolean _host;
private String _parsedHost;
private boolean _headerComplete;
private volatile State _state = State.START;
private volatile FieldState _fieldState = FieldState.FIELD;
Expand Down Expand Up @@ -1028,14 +1031,28 @@ else if (_endOfContent == EndOfContent.CHUNKED_CONTENT)
break;

case HOST:
if (_host)
throw new BadMessageException(HttpStatus.BAD_REQUEST_400, "Bad Host: multiple headers");
_host = true;
if (_parsedHost != null)
{
if (LOG.isWarnEnabled())
LOG.warn("Encountered multiple `Host` headers. Previous `Host` header already seen as `{}`, new `Host` header has appeared as `{}`", _parsedHost, _valueString);
checkViolation(DUPLICATE_HOST_HEADERS);
}
_parsedHost = _valueString;
if (!(_field instanceof HostPortHttpField) && _valueString != null && !_valueString.isEmpty())
{
_field = new HostPortHttpField(_header,
CASE_SENSITIVE_FIELD_NAME.isAllowedBy(_complianceMode) ? _headerString : _header.asString(),
_valueString);
HostPort hostPort;
if (UNSAFE_HOST_HEADER.isAllowedBy(_complianceMode))
{
_field = new HostPortHttpField(_header,
CASE_SENSITIVE_FIELD_NAME.isAllowedBy(_complianceMode) ? _headerString : _header.asString(),
HostPort.unsafe(_valueString));
}
else
{
_field = new HostPortHttpField(_header,
CASE_SENSITIVE_FIELD_NAME.isAllowedBy(_complianceMode) ? _headerString : _header.asString(),
_valueString);
}
addToFieldCache = _fieldCache.isEnabled();
}
break;
Expand Down Expand Up @@ -1072,6 +1089,8 @@ else if (_endOfContent == EndOfContent.CHUNKED_CONTENT)
_fieldCache.add(_field);
}
}
if (LOG.isDebugEnabled())
LOG.debug("parsedHeader({}) header={}, headerString=[{}], valueString=[{}]", _field, _header, _headerString, _valueString);
_handler.parsedHeader(_field != null ? _field : new HttpField(_header, _headerString, _valueString));
}

Expand Down Expand Up @@ -1183,7 +1202,7 @@ protected boolean parseFields(ByteBuffer buffer)
}

// Was there a required host header?
if (!_host && _version == HttpVersion.HTTP_1_1 && _requestHandler != null)
if (_parsedHost == null && _version == HttpVersion.HTTP_1_1 && _requestHandler != null)
{
throw new BadMessageException(HttpStatus.BAD_REQUEST_400, "No Host");
}
Expand Down Expand Up @@ -1888,7 +1907,7 @@ public void reset()
_responseStatus = 0;
_contentChunk = null;
_headerBytes = 0;
_host = false;
_parsedHost = null;
_headerComplete = false;
}

Expand Down
122 changes: 111 additions & 11 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 @@ -18,6 +18,7 @@
import java.util.ArrayList;
import java.util.List;
import java.util.Locale;
import java.util.stream.Stream;

import org.eclipse.jetty.http.HttpParser.State;
import org.eclipse.jetty.logging.StacklessLogging;
Expand All @@ -28,6 +29,8 @@
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
import org.junit.jupiter.params.provider.ValueSource;

import static org.eclipse.jetty.http.HttpCompliance.Violation.CASE_INSENSITIVE_METHOD;
Expand All @@ -39,6 +42,7 @@
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;
Expand Down Expand Up @@ -2041,17 +2045,95 @@ public void testHostPort()
assertEquals(8888, _port);
}

public static Stream<String> badHostHeaderSource()
{
return List.of(
":80", // no host, port only
"host:", // no port
"127.0.0.1:", // no port
"[0::0::0::0::1", // no IP literal ending bracket
"0::0::0::0::1]", // no IP literal starting bracket
"[0::0::0::0::1]:", // no port
"[0::0::0::1]", // not valid to Java (InetAddress, InetSocketAddress, or URI) : "Expected hex digits or IPv4 address"
"[0::0::0::1]:80", // not valid to Java (InetAddress, InetSocketAddress, or URI) : "Expected hex digits or IPv4 address"
"0:1:2:3:4:5:6", // not valid to Java (InetAddress, InetSocketAddress, or URI) : "IPv6 address too short"
"host:xxx", // invalid port
"127.0.0.1:xxx", // host + invalid port
"[0::0::0::0::1]:xxx", // ipv6 + invalid port
"host:-80", // host + invalid port
"127.0.0.1:-80", // ipv4 + invalid port
"[0::0::0::0::1]:-80", // ipv6 + invalid port
"127.0.0.1:65536", // ipv4 + port value too high
"a b c d", // whitespace in reg-name
"a\to\tz", // tabs in reg-name
"hosta, hostb, hostc", // space sin reg-name
"[ab:cd:ef:gh:ij:kl:mn]", // invalid ipv6 address
// Examples of bad Host header values (usually client bugs that shouldn't allow them)
"Group - Machine", // spaces
"<calculated when request is sent>",
"[link](https://example.org/)",
"example.org/zed", // has slash
// common hacking attempts, seen as values on the `Host:` request header
"| ping 127.0.0.1 -n 10",
"%uf%80%ff%xx%uffff",
"[${jndi${:-:}ldap${:-:}]", // log4j hacking
"[${jndi:ldap://example.org:59377/nessus}]", // log4j hacking
"${ip}", // variation of log4j hack
"' *; host xyz.hacking.pro; '",
"'/**/OR/**/1/**/=/**/1",
"AND (SELECT 1 FROM(SELECT COUNT(*),CONCAT('x',(SELECT (ELT(1=1,1))),'x',FLOOR(RAND(0)*2))x FROM INFORMATION_SCHEMA.CHARACTER_SETS GROUP BY x)a)"
).stream();
}

@ParameterizedTest
@ValueSource(strings = {
"Host: whatever.com:xxxx",
"Host: myhost:testBadPort",
"Host: a b c d", // whitespace in reg-name
"Host: a\to\tz", // tabs in reg-name
"Host: hosta, hostb, hostc", // spaces in reg-name
"Host: [sd ajklf;d sajklf;d sajfkl;d]", // not a valid IPv6 address
"Host: hosta\nHost: hostb\nHost: hostc" // multi-line
})
public void testBadHost(String hostline)
@MethodSource("badHostHeaderSource")
public void testBadHostReject(String hostline)
{
ByteBuffer buffer = BufferUtil.toBuffer(
"GET / HTTP/1.1\n" +
"Host: " + hostline + "\n" +
"Connection: close\n" +
"\n");

HttpParser.RequestHandler handler = new Handler();
HttpParser parser = new HttpParser(handler);
parser.parseNext(buffer);
assertThat(_bad, startsWith("Bad "));
}

@ParameterizedTest
@MethodSource("badHostHeaderSource")
public void testBadHostAllow(String hostline)
{
ByteBuffer buffer = BufferUtil.toBuffer(
"GET / HTTP/1.1\n" +
"Host: " + hostline + "\n" +
"Connection: close\n" +
"\n");

HttpParser.RequestHandler handler = new Handler();
HttpCompliance httpCompliance = HttpCompliance.from("RFC7230,UNSAFE_HOST_HEADER");
HttpParser parser = new HttpParser(handler, httpCompliance);
parser.parseNext(buffer);
assertNull(_bad);
assertNotNull(_host);
}

public static Stream<Arguments> duplicateHostHeadersSource()
{
return Stream.of(
// different values
Arguments.of("Host: hosta\nHost: hostb\nHost: hostc"),
// same values
Arguments.of("Host: foo\nHost: foo"),
// separated by another header
Arguments.of("Host: bar\nX-Zed: zed\nHost: bar")
);
}

@ParameterizedTest
@MethodSource("duplicateHostHeadersSource")
public void testDuplicateHostReject(String hostline)
{
ByteBuffer buffer = BufferUtil.toBuffer(
"GET / HTTP/1.1\n" +
Expand All @@ -2062,7 +2144,25 @@ public void testBadHost(String hostline)
HttpParser.RequestHandler handler = new Handler();
HttpParser parser = new HttpParser(handler);
parser.parseNext(buffer);
assertThat(_bad, startsWith("Bad"));
assertThat(_bad, startsWith("Duplicate Host Header"));
}

@ParameterizedTest
@MethodSource("duplicateHostHeadersSource")
public void testDuplicateHostAllow(String hostline)
{
ByteBuffer buffer = BufferUtil.toBuffer(
"GET / HTTP/1.1\n" +
hostline + "\n" +
"Connection: close\n" +
"\n");

HttpParser.RequestHandler handler = new Handler();
HttpCompliance httpCompliance = HttpCompliance.from("RFC7230,DUPLICATE_HOST_HEADERS");
HttpParser parser = new HttpParser(handler, httpCompliance);
parser.parseNext(buffer);
assertNull(_bad);
assertNotNull(_host);
}

@ParameterizedTest
Expand Down
Loading