Skip to content

Commit

Permalink
Deprecate disabling HTTP header validation (#13609)
Browse files Browse the repository at this point in the history 
Motivation:
HTTP request/response splitting is a serious vulnerability, which is
mitigated by enabling HTTP header validation. We already do header
validation by default, but we have many constructors in our HTTP codec
that permit turning it off. We should discourage all our down-stream
users from turning header validation off.

Modification:
- Deprecate all constructors in our HTTP codec, that allow header
validation to be turned off.
- Offer alternative constructors to all the newly deprecated ones, where
header validation is enabled.
- Trailer validation in `DefaultLastHttpContent` is now enabled by
default in the `DefaultLastHttpContent(ByteBuf, HttpHeaders)`
constructor (Behavioral change!)
- Add better constructors, and more guidance in the javadocs, for
`DefaultHttpHeaders` for those who want to use custom header validators.

Result:
We hopefully encourage integrators to validate headers. And we hopefully
get fewer issues reported about disabled header validation when people
run security scanners on their code.
  • Loading branch information
@chrisvest
chrisvest committed Nov 28, 2023
1 parent 7dba4fc commit 84a13a9
Show file tree
Hide file tree
Showing 27 changed files with 1,423 additions and 245 deletions.
40 changes: 39 additions & 1 deletion codec-http/src/main/java/io/netty/handler/codec/http/CombinedHttpHeaders.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
package io.netty.handler.codec.http;

import io.netty.handler.codec.DefaultHeaders;
import io.netty.handler.codec.DefaultHeaders.NameValidator;
import io.netty.handler.codec.DefaultHeaders.ValueValidator;
import io.netty.handler.codec.Headers;
import io.netty.handler.codec.ValueConverter;
import io.netty.util.HashingStrategy;
Expand All @@ -28,6 +30,7 @@

import static io.netty.handler.codec.http.HttpHeaderNames.SET_COOKIE;
import static io.netty.util.AsciiString.CASE_INSENSITIVE_HASHER;
import static io.netty.util.internal.ObjectUtil.checkNotNull;
import static io.netty.util.internal.StringUtil.COMMA;
import static io.netty.util.internal.StringUtil.unescapeCsvFields;

Expand All @@ -37,11 +40,38 @@
* Please refer to section <a href="https://tools.ietf.org/html/rfc7230#section-3.2.2">RFC 7230, 3.2.2</a>.
*/
public class CombinedHttpHeaders extends DefaultHttpHeaders {
/**
* Create a combined HTTP header object, with optional validation.
*
* @param validate Should Netty validate header values to ensure they aren't malicious.
* @deprecated Prefer instead to configuring a {@link HttpHeadersFactory}
* by calling {@link DefaultHttpHeadersFactory#withCombiningHeaders(boolean) withCombiningHeaders(true)}
* on {@link DefaultHttpHeadersFactory#headersFactory()}.
*/
@Deprecated
public CombinedHttpHeaders(boolean validate) {
super(new CombinedHttpHeadersImpl(CASE_INSENSITIVE_HASHER, valueConverter(), nameValidator(validate),
valueValidator(validate)));
}

CombinedHttpHeaders(NameValidator<CharSequence> nameValidator, ValueValidator<CharSequence> valueValidator) {
super(new CombinedHttpHeadersImpl(
CASE_INSENSITIVE_HASHER,
valueConverter(),
checkNotNull(nameValidator, "nameValidator"),
checkNotNull(valueValidator, "valueValidator")));
}

CombinedHttpHeaders(
NameValidator<CharSequence> nameValidator, ValueValidator<CharSequence> valueValidator, int sizeHint) {
super(new CombinedHttpHeadersImpl(
CASE_INSENSITIVE_HASHER,
valueConverter(),
checkNotNull(nameValidator, "nameValidator"),
checkNotNull(valueValidator, "valueValidator"),
sizeHint));
}

@Override
public boolean containsValue(CharSequence name, CharSequence value, boolean ignoreCase) {
return super.containsValue(name, StringUtil.trimOws(value), ignoreCase);
Expand Down Expand Up @@ -91,7 +121,15 @@ public CharSequence escape(CharSequence name, CharSequence value) {
ValueConverter<CharSequence> valueConverter,
NameValidator<CharSequence> nameValidator,
ValueValidator<CharSequence> valueValidator) {
super(nameHashingStrategy, valueConverter, nameValidator, 16, valueValidator);
this(nameHashingStrategy, valueConverter, nameValidator, valueValidator, 16);
}

CombinedHttpHeadersImpl(HashingStrategy<CharSequence> nameHashingStrategy,
ValueConverter<CharSequence> valueConverter,
NameValidator<CharSequence> nameValidator,
ValueValidator<CharSequence> valueValidator,
int sizeHint) {
super(nameHashingStrategy, valueConverter, nameValidator, sizeHint, valueValidator);
}

@Override
Expand Down
50 changes: 44 additions & 6 deletions codec-http/src/main/java/io/netty/handler/codec/http/DefaultFullHttpRequest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@
import io.netty.buffer.ByteBufUtil;
import io.netty.buffer.Unpooled;
import io.netty.util.IllegalReferenceCountException;

import static io.netty.handler.codec.http.DefaultHttpHeadersFactory.headersFactory;
import static io.netty.handler.codec.http.DefaultHttpHeadersFactory.trailersFactory;
import static io.netty.util.internal.ObjectUtil.checkNotNull;

/**
Expand All @@ -33,25 +36,60 @@ public class DefaultFullHttpRequest extends DefaultHttpRequest implements FullHt
*/
private int hash;

/**
* Create a full HTTP response with the given HTTP version, method, and URI.
*/
public DefaultFullHttpRequest(HttpVersion httpVersion, HttpMethod method, String uri) {
this(httpVersion, method, uri, Unpooled.buffer(0));
this(httpVersion, method, uri, Unpooled.buffer(0), headersFactory(), trailersFactory());
}

/**
* Create a full HTTP response with the given HTTP version, method, URI, and contents.
*/
public DefaultFullHttpRequest(HttpVersion httpVersion, HttpMethod method, String uri, ByteBuf content) {
this(httpVersion, method, uri, content, true);
this(httpVersion, method, uri, content, headersFactory(), trailersFactory());
}

/**
* Create a full HTTP response with the given HTTP version, method, URI, and optional validation.
* @deprecated Use the {@link #DefaultFullHttpRequest(HttpVersion, HttpMethod, String, ByteBuf,
* HttpHeadersFactory, HttpHeadersFactory)} constructor instead.
*/
@Deprecated
public DefaultFullHttpRequest(HttpVersion httpVersion, HttpMethod method, String uri, boolean validateHeaders) {
this(httpVersion, method, uri, Unpooled.buffer(0), validateHeaders);
this(httpVersion, method, uri, Unpooled.buffer(0),
headersFactory().withValidation(validateHeaders),
trailersFactory().withValidation(validateHeaders));
}

/**
* Create a full HTTP response with the given HTTP version, method, URI, contents, and optional validation.
* @deprecated Use the {@link #DefaultFullHttpRequest(HttpVersion, HttpMethod, String, ByteBuf,
* HttpHeadersFactory, HttpHeadersFactory)} constructor instead.
*/
@Deprecated
public DefaultFullHttpRequest(HttpVersion httpVersion, HttpMethod method, String uri,
ByteBuf content, boolean validateHeaders) {
super(httpVersion, method, uri, validateHeaders);
this.content = checkNotNull(content, "content");
trailingHeader = new DefaultHttpHeaders(validateHeaders);
this(httpVersion, method, uri, content,
headersFactory().withValidation(validateHeaders),
trailersFactory().withValidation(validateHeaders));
}

/**
* Create a full HTTP response with the given HTTP version, method, URI, contents,
* and factories for creating headers and trailers.
* <p>
* The recommended default header factory is {@link DefaultHttpHeadersFactory#headersFactory()},
* and the recommended default trailer factory is {@link DefaultHttpHeadersFactory#trailersFactory()}.
*/
public DefaultFullHttpRequest(HttpVersion httpVersion, HttpMethod method, String uri,
ByteBuf content, HttpHeadersFactory headersFactory, HttpHeadersFactory trailersFactory) {
this(httpVersion, method, uri, content, headersFactory.newHeaders(), trailersFactory.newHeaders());
}

/**
* Create a full HTTP response with the given HTTP version, method, URI, contents, and header and trailer objects.
*/
public DefaultFullHttpRequest(HttpVersion httpVersion, HttpMethod method, String uri,
ByteBuf content, HttpHeaders headers, HttpHeaders trailingHeader) {
super(httpVersion, method, uri, headers);
Expand Down
76 changes: 67 additions & 9 deletions codec-http/src/main/java/io/netty/handler/codec/http/DefaultFullHttpResponse.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
import io.netty.buffer.Unpooled;
import io.netty.util.IllegalReferenceCountException;

import static io.netty.handler.codec.http.DefaultHttpHeadersFactory.headersFactory;
import static io.netty.handler.codec.http.DefaultHttpHeadersFactory.trailersFactory;
import static io.netty.util.internal.ObjectUtil.checkNotNull;

/**
Expand All @@ -35,36 +37,92 @@ public class DefaultFullHttpResponse extends DefaultHttpResponse implements Full
*/
private int hash;

/**
* Create an empty HTTP response with the given HTTP version and status.
*/
public DefaultFullHttpResponse(HttpVersion version, HttpResponseStatus status) {
this(version, status, Unpooled.buffer(0));
this(version, status, Unpooled.buffer(0), headersFactory(), trailersFactory());
}

/**
* Create an HTTP response with the given HTTP version, status, and contents.
*/
public DefaultFullHttpResponse(HttpVersion version, HttpResponseStatus status, ByteBuf content) {
this(version, status, content, true);
this(version, status, content, headersFactory(), trailersFactory());
}

/**
* Create an empty HTTP response with the given HTTP version, status, and optional header validation.
*
* @deprecated Prefer the {@link #DefaultFullHttpResponse(HttpVersion, HttpResponseStatus, ByteBuf,
* HttpHeadersFactory, HttpHeadersFactory)} constructor instead.
*/
@Deprecated
public DefaultFullHttpResponse(HttpVersion version, HttpResponseStatus status, boolean validateHeaders) {
this(version, status, Unpooled.buffer(0), validateHeaders, false);
this(version, status, Unpooled.buffer(0),
headersFactory().withValidation(validateHeaders),
trailersFactory().withValidation(validateHeaders));
}

/**
* Create an empty HTTP response with the given HTTP version, status, optional header validation,
* and optional header combining.
*
* @deprecated Prefer the {@link #DefaultFullHttpResponse(HttpVersion, HttpResponseStatus, ByteBuf,
* HttpHeadersFactory, HttpHeadersFactory)} constructor instead.
*/
@Deprecated
public DefaultFullHttpResponse(HttpVersion version, HttpResponseStatus status, boolean validateHeaders,
boolean singleFieldHeaders) {
this(version, status, Unpooled.buffer(0), validateHeaders, singleFieldHeaders);
this(version, status, Unpooled.buffer(0),
headersFactory().withValidation(validateHeaders).withCombiningHeaders(singleFieldHeaders),
trailersFactory().withValidation(validateHeaders).withCombiningHeaders(singleFieldHeaders));
}

/**
* Create an HTTP response with the given HTTP version, status, contents, and optional header validation.
*
* @deprecated Prefer the {@link #DefaultFullHttpResponse(HttpVersion, HttpResponseStatus, ByteBuf,
* HttpHeadersFactory, HttpHeadersFactory)} constructor instead.
*/
@Deprecated
public DefaultFullHttpResponse(HttpVersion version, HttpResponseStatus status,
ByteBuf content, boolean validateHeaders) {
this(version, status, content, validateHeaders, false);
this(version, status, content,
headersFactory().withValidation(validateHeaders),
trailersFactory().withValidation(validateHeaders));
}

/**
* Create an HTTP response with the given HTTP version, status, contents, optional header validation,
* and optional header combining.
*
* @deprecated Prefer the {@link #DefaultFullHttpResponse(HttpVersion, HttpResponseStatus, ByteBuf,
* HttpHeadersFactory, HttpHeadersFactory)} constructor instead.
*/
@Deprecated
public DefaultFullHttpResponse(HttpVersion version, HttpResponseStatus status,
ByteBuf content, boolean validateHeaders, boolean singleFieldHeaders) {
super(version, status, validateHeaders, singleFieldHeaders);
this.content = checkNotNull(content, "content");
this.trailingHeaders = singleFieldHeaders ? new CombinedHttpHeaders(validateHeaders)
: new DefaultHttpHeaders(validateHeaders);
this(version, status, content,
headersFactory().withValidation(validateHeaders).withCombiningHeaders(singleFieldHeaders),
trailersFactory().withValidation(validateHeaders).withCombiningHeaders(singleFieldHeaders));
}

/**
* Create an HTTP response with the given HTTP version, status, contents,
* and with headers and trailers created by the given header factories.
* <p>
* The recommended header factory is {@link DefaultHttpHeadersFactory#headersFactory()},
* and the recommended trailer factory is {@link DefaultHttpHeadersFactory#trailersFactory()}.
*/
public DefaultFullHttpResponse(HttpVersion version, HttpResponseStatus status, ByteBuf content,
HttpHeadersFactory headersFactory, HttpHeadersFactory trailersFactory) {
this(version, status, content, headersFactory.newHeaders(), trailersFactory.newHeaders());
}

/**
* Create an HTTP response with the given HTTP version, status, contents, headers and trailers.
*/
public DefaultFullHttpResponse(HttpVersion version, HttpResponseStatus status,
ByteBuf content, HttpHeaders headers, HttpHeaders trailingHeaders) {
super(version, status, headers);
Expand Down

0 comments on commit 84a13a9

Please sign in to comment.