Skip to content

Commit

Permalink
Stricter validation of HTTP/1 headers (CVE-2019-18802). (#72)
Browse files Browse the repository at this point in the history
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
  • Loading branch information
yanavlasov authored Dec 10, 2019
1 parent cc371ae commit 6a6ac1c
Show file tree
Hide file tree
Showing 13 changed files with 330 additions and 4 deletions.
223 changes: 223 additions & 0 deletions bazel/foreign_cc/nghttp2.patch
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,226 @@ index 35c77d1d..47bd63f5 100644
endif()
# AC_TYPE_UINT8_T
# AC_TYPE_UINT16_T
diff --git a/doc/Makefile.am b/doc/Makefile.am
index c17d93382..4d73cef50 100644
--- a/doc/Makefile.am
+++ b/doc/Makefile.am
@@ -27,6 +27,7 @@ APIDOCS= \
macros.rst \
enums.rst \
types.rst \
+ nghttp2_check_authority.rst \
nghttp2_check_header_name.rst \
nghttp2_check_header_value.rst \
nghttp2_hd_deflate_bound.rst \
diff --git a/lib/includes/nghttp2/nghttp2.h b/lib/includes/nghttp2/nghttp2.h
index 313fb23da..e3aeb9fed 100644
--- a/lib/includes/nghttp2/nghttp2.h
+++ b/lib/includes/nghttp2/nghttp2.h
@@ -4769,6 +4769,19 @@ NGHTTP2_EXTERN int nghttp2_check_header_name(const uint8_t *name, size_t len);
*/
NGHTTP2_EXTERN int nghttp2_check_header_value(const uint8_t *value, size_t len);

+/**
+ * @function
+ *
+ * Returns nonzero if the |value| which is supposed to the value of
+ * :authority or host header field is valid according to
+ * https://tools.ietf.org/html/rfc3986#section-3.2
+ *
+ * |value| is valid if it merely consists of the allowed characters.
+ * In particular, it does not check whether |value| follows the syntax
+ * of authority.
+ */
+NGHTTP2_EXTERN int nghttp2_check_authority(const uint8_t *value, size_t len);
+
/* HPACK API */

struct nghttp2_hd_deflater;
diff --git a/lib/nghttp2_helper.c b/lib/nghttp2_helper.c
index 81a8a0cf9..91136a619 100644
--- a/lib/nghttp2_helper.c
+++ b/lib/nghttp2_helper.c
@@ -505,6 +505,84 @@ int nghttp2_check_header_value(const uint8_t *value, size_t len) {
return 1;
}

+/* Generated by genauthroitychartbl.py */
+static char VALID_AUTHORITY_CHARS[] = {
+ 0 /* NUL */, 0 /* SOH */, 0 /* STX */, 0 /* ETX */,
+ 0 /* EOT */, 0 /* ENQ */, 0 /* ACK */, 0 /* BEL */,
+ 0 /* BS */, 0 /* HT */, 0 /* LF */, 0 /* VT */,
+ 0 /* FF */, 0 /* CR */, 0 /* SO */, 0 /* SI */,
+ 0 /* DLE */, 0 /* DC1 */, 0 /* DC2 */, 0 /* DC3 */,
+ 0 /* DC4 */, 0 /* NAK */, 0 /* SYN */, 0 /* ETB */,
+ 0 /* CAN */, 0 /* EM */, 0 /* SUB */, 0 /* ESC */,
+ 0 /* FS */, 0 /* GS */, 0 /* RS */, 0 /* US */,
+ 0 /* SPC */, 1 /* ! */, 0 /* " */, 0 /* # */,
+ 1 /* $ */, 1 /* % */, 1 /* & */, 1 /* ' */,
+ 1 /* ( */, 1 /* ) */, 1 /* * */, 1 /* + */,
+ 1 /* , */, 1 /* - */, 1 /* . */, 0 /* / */,
+ 1 /* 0 */, 1 /* 1 */, 1 /* 2 */, 1 /* 3 */,
+ 1 /* 4 */, 1 /* 5 */, 1 /* 6 */, 1 /* 7 */,
+ 1 /* 8 */, 1 /* 9 */, 1 /* : */, 1 /* ; */,
+ 0 /* < */, 1 /* = */, 0 /* > */, 0 /* ? */,
+ 1 /* @ */, 1 /* A */, 1 /* B */, 1 /* C */,
+ 1 /* D */, 1 /* E */, 1 /* F */, 1 /* G */,
+ 1 /* H */, 1 /* I */, 1 /* J */, 1 /* K */,
+ 1 /* L */, 1 /* M */, 1 /* N */, 1 /* O */,
+ 1 /* P */, 1 /* Q */, 1 /* R */, 1 /* S */,
+ 1 /* T */, 1 /* U */, 1 /* V */, 1 /* W */,
+ 1 /* X */, 1 /* Y */, 1 /* Z */, 1 /* [ */,
+ 0 /* \ */, 1 /* ] */, 0 /* ^ */, 1 /* _ */,
+ 0 /* ` */, 1 /* a */, 1 /* b */, 1 /* c */,
+ 1 /* d */, 1 /* e */, 1 /* f */, 1 /* g */,
+ 1 /* h */, 1 /* i */, 1 /* j */, 1 /* k */,
+ 1 /* l */, 1 /* m */, 1 /* n */, 1 /* o */,
+ 1 /* p */, 1 /* q */, 1 /* r */, 1 /* s */,
+ 1 /* t */, 1 /* u */, 1 /* v */, 1 /* w */,
+ 1 /* x */, 1 /* y */, 1 /* z */, 0 /* { */,
+ 0 /* | */, 0 /* } */, 1 /* ~ */, 0 /* DEL */,
+ 0 /* 0x80 */, 0 /* 0x81 */, 0 /* 0x82 */, 0 /* 0x83 */,
+ 0 /* 0x84 */, 0 /* 0x85 */, 0 /* 0x86 */, 0 /* 0x87 */,
+ 0 /* 0x88 */, 0 /* 0x89 */, 0 /* 0x8a */, 0 /* 0x8b */,
+ 0 /* 0x8c */, 0 /* 0x8d */, 0 /* 0x8e */, 0 /* 0x8f */,
+ 0 /* 0x90 */, 0 /* 0x91 */, 0 /* 0x92 */, 0 /* 0x93 */,
+ 0 /* 0x94 */, 0 /* 0x95 */, 0 /* 0x96 */, 0 /* 0x97 */,
+ 0 /* 0x98 */, 0 /* 0x99 */, 0 /* 0x9a */, 0 /* 0x9b */,
+ 0 /* 0x9c */, 0 /* 0x9d */, 0 /* 0x9e */, 0 /* 0x9f */,
+ 0 /* 0xa0 */, 0 /* 0xa1 */, 0 /* 0xa2 */, 0 /* 0xa3 */,
+ 0 /* 0xa4 */, 0 /* 0xa5 */, 0 /* 0xa6 */, 0 /* 0xa7 */,
+ 0 /* 0xa8 */, 0 /* 0xa9 */, 0 /* 0xaa */, 0 /* 0xab */,
+ 0 /* 0xac */, 0 /* 0xad */, 0 /* 0xae */, 0 /* 0xaf */,
+ 0 /* 0xb0 */, 0 /* 0xb1 */, 0 /* 0xb2 */, 0 /* 0xb3 */,
+ 0 /* 0xb4 */, 0 /* 0xb5 */, 0 /* 0xb6 */, 0 /* 0xb7 */,
+ 0 /* 0xb8 */, 0 /* 0xb9 */, 0 /* 0xba */, 0 /* 0xbb */,
+ 0 /* 0xbc */, 0 /* 0xbd */, 0 /* 0xbe */, 0 /* 0xbf */,
+ 0 /* 0xc0 */, 0 /* 0xc1 */, 0 /* 0xc2 */, 0 /* 0xc3 */,
+ 0 /* 0xc4 */, 0 /* 0xc5 */, 0 /* 0xc6 */, 0 /* 0xc7 */,
+ 0 /* 0xc8 */, 0 /* 0xc9 */, 0 /* 0xca */, 0 /* 0xcb */,
+ 0 /* 0xcc */, 0 /* 0xcd */, 0 /* 0xce */, 0 /* 0xcf */,
+ 0 /* 0xd0 */, 0 /* 0xd1 */, 0 /* 0xd2 */, 0 /* 0xd3 */,
+ 0 /* 0xd4 */, 0 /* 0xd5 */, 0 /* 0xd6 */, 0 /* 0xd7 */,
+ 0 /* 0xd8 */, 0 /* 0xd9 */, 0 /* 0xda */, 0 /* 0xdb */,
+ 0 /* 0xdc */, 0 /* 0xdd */, 0 /* 0xde */, 0 /* 0xdf */,
+ 0 /* 0xe0 */, 0 /* 0xe1 */, 0 /* 0xe2 */, 0 /* 0xe3 */,
+ 0 /* 0xe4 */, 0 /* 0xe5 */, 0 /* 0xe6 */, 0 /* 0xe7 */,
+ 0 /* 0xe8 */, 0 /* 0xe9 */, 0 /* 0xea */, 0 /* 0xeb */,
+ 0 /* 0xec */, 0 /* 0xed */, 0 /* 0xee */, 0 /* 0xef */,
+ 0 /* 0xf0 */, 0 /* 0xf1 */, 0 /* 0xf2 */, 0 /* 0xf3 */,
+ 0 /* 0xf4 */, 0 /* 0xf5 */, 0 /* 0xf6 */, 0 /* 0xf7 */,
+ 0 /* 0xf8 */, 0 /* 0xf9 */, 0 /* 0xfa */, 0 /* 0xfb */,
+ 0 /* 0xfc */, 0 /* 0xfd */, 0 /* 0xfe */, 0 /* 0xff */
+};
+
+int nghttp2_check_authority(const uint8_t *value, size_t len) {
+ const uint8_t *last;
+ for (last = value + len; value != last; ++value) {
+ if (!VALID_AUTHORITY_CHARS[*value]) {
+ return 0;
+ }
+ }
+ return 1;
+}
+
uint8_t *nghttp2_cpymem(uint8_t *dest, const void *src, size_t len) {
if (len == 0) {
return dest;
diff --git a/lib/nghttp2_http.c b/lib/nghttp2_http.c
index 8d9902998..62f57b6ae 100644
--- a/lib/nghttp2_http.c
+++ b/lib/nghttp2_http.c
@@ -305,84 +305,6 @@ static int http_response_on_header(nghttp2_stream *stream, nghttp2_hd_nv *nv,
return 0;
}

-/* Generated by genauthroitychartbl.py */
-static char VALID_AUTHORITY_CHARS[] = {
- 0 /* NUL */, 0 /* SOH */, 0 /* STX */, 0 /* ETX */,
- 0 /* EOT */, 0 /* ENQ */, 0 /* ACK */, 0 /* BEL */,
- 0 /* BS */, 0 /* HT */, 0 /* LF */, 0 /* VT */,
- 0 /* FF */, 0 /* CR */, 0 /* SO */, 0 /* SI */,
- 0 /* DLE */, 0 /* DC1 */, 0 /* DC2 */, 0 /* DC3 */,
- 0 /* DC4 */, 0 /* NAK */, 0 /* SYN */, 0 /* ETB */,
- 0 /* CAN */, 0 /* EM */, 0 /* SUB */, 0 /* ESC */,
- 0 /* FS */, 0 /* GS */, 0 /* RS */, 0 /* US */,
- 0 /* SPC */, 1 /* ! */, 0 /* " */, 0 /* # */,
- 1 /* $ */, 1 /* % */, 1 /* & */, 1 /* ' */,
- 1 /* ( */, 1 /* ) */, 1 /* * */, 1 /* + */,
- 1 /* , */, 1 /* - */, 1 /* . */, 0 /* / */,
- 1 /* 0 */, 1 /* 1 */, 1 /* 2 */, 1 /* 3 */,
- 1 /* 4 */, 1 /* 5 */, 1 /* 6 */, 1 /* 7 */,
- 1 /* 8 */, 1 /* 9 */, 1 /* : */, 1 /* ; */,
- 0 /* < */, 1 /* = */, 0 /* > */, 0 /* ? */,
- 1 /* @ */, 1 /* A */, 1 /* B */, 1 /* C */,
- 1 /* D */, 1 /* E */, 1 /* F */, 1 /* G */,
- 1 /* H */, 1 /* I */, 1 /* J */, 1 /* K */,
- 1 /* L */, 1 /* M */, 1 /* N */, 1 /* O */,
- 1 /* P */, 1 /* Q */, 1 /* R */, 1 /* S */,
- 1 /* T */, 1 /* U */, 1 /* V */, 1 /* W */,
- 1 /* X */, 1 /* Y */, 1 /* Z */, 1 /* [ */,
- 0 /* \ */, 1 /* ] */, 0 /* ^ */, 1 /* _ */,
- 0 /* ` */, 1 /* a */, 1 /* b */, 1 /* c */,
- 1 /* d */, 1 /* e */, 1 /* f */, 1 /* g */,
- 1 /* h */, 1 /* i */, 1 /* j */, 1 /* k */,
- 1 /* l */, 1 /* m */, 1 /* n */, 1 /* o */,
- 1 /* p */, 1 /* q */, 1 /* r */, 1 /* s */,
- 1 /* t */, 1 /* u */, 1 /* v */, 1 /* w */,
- 1 /* x */, 1 /* y */, 1 /* z */, 0 /* { */,
- 0 /* | */, 0 /* } */, 1 /* ~ */, 0 /* DEL */,
- 0 /* 0x80 */, 0 /* 0x81 */, 0 /* 0x82 */, 0 /* 0x83 */,
- 0 /* 0x84 */, 0 /* 0x85 */, 0 /* 0x86 */, 0 /* 0x87 */,
- 0 /* 0x88 */, 0 /* 0x89 */, 0 /* 0x8a */, 0 /* 0x8b */,
- 0 /* 0x8c */, 0 /* 0x8d */, 0 /* 0x8e */, 0 /* 0x8f */,
- 0 /* 0x90 */, 0 /* 0x91 */, 0 /* 0x92 */, 0 /* 0x93 */,
- 0 /* 0x94 */, 0 /* 0x95 */, 0 /* 0x96 */, 0 /* 0x97 */,
- 0 /* 0x98 */, 0 /* 0x99 */, 0 /* 0x9a */, 0 /* 0x9b */,
- 0 /* 0x9c */, 0 /* 0x9d */, 0 /* 0x9e */, 0 /* 0x9f */,
- 0 /* 0xa0 */, 0 /* 0xa1 */, 0 /* 0xa2 */, 0 /* 0xa3 */,
- 0 /* 0xa4 */, 0 /* 0xa5 */, 0 /* 0xa6 */, 0 /* 0xa7 */,
- 0 /* 0xa8 */, 0 /* 0xa9 */, 0 /* 0xaa */, 0 /* 0xab */,
- 0 /* 0xac */, 0 /* 0xad */, 0 /* 0xae */, 0 /* 0xaf */,
- 0 /* 0xb0 */, 0 /* 0xb1 */, 0 /* 0xb2 */, 0 /* 0xb3 */,
- 0 /* 0xb4 */, 0 /* 0xb5 */, 0 /* 0xb6 */, 0 /* 0xb7 */,
- 0 /* 0xb8 */, 0 /* 0xb9 */, 0 /* 0xba */, 0 /* 0xbb */,
- 0 /* 0xbc */, 0 /* 0xbd */, 0 /* 0xbe */, 0 /* 0xbf */,
- 0 /* 0xc0 */, 0 /* 0xc1 */, 0 /* 0xc2 */, 0 /* 0xc3 */,
- 0 /* 0xc4 */, 0 /* 0xc5 */, 0 /* 0xc6 */, 0 /* 0xc7 */,
- 0 /* 0xc8 */, 0 /* 0xc9 */, 0 /* 0xca */, 0 /* 0xcb */,
- 0 /* 0xcc */, 0 /* 0xcd */, 0 /* 0xce */, 0 /* 0xcf */,
- 0 /* 0xd0 */, 0 /* 0xd1 */, 0 /* 0xd2 */, 0 /* 0xd3 */,
- 0 /* 0xd4 */, 0 /* 0xd5 */, 0 /* 0xd6 */, 0 /* 0xd7 */,
- 0 /* 0xd8 */, 0 /* 0xd9 */, 0 /* 0xda */, 0 /* 0xdb */,
- 0 /* 0xdc */, 0 /* 0xdd */, 0 /* 0xde */, 0 /* 0xdf */,
- 0 /* 0xe0 */, 0 /* 0xe1 */, 0 /* 0xe2 */, 0 /* 0xe3 */,
- 0 /* 0xe4 */, 0 /* 0xe5 */, 0 /* 0xe6 */, 0 /* 0xe7 */,
- 0 /* 0xe8 */, 0 /* 0xe9 */, 0 /* 0xea */, 0 /* 0xeb */,
- 0 /* 0xec */, 0 /* 0xed */, 0 /* 0xee */, 0 /* 0xef */,
- 0 /* 0xf0 */, 0 /* 0xf1 */, 0 /* 0xf2 */, 0 /* 0xf3 */,
- 0 /* 0xf4 */, 0 /* 0xf5 */, 0 /* 0xf6 */, 0 /* 0xf7 */,
- 0 /* 0xf8 */, 0 /* 0xf9 */, 0 /* 0xfa */, 0 /* 0xfb */,
- 0 /* 0xfc */, 0 /* 0xfd */, 0 /* 0xfe */, 0 /* 0xff */
-};
-
-static int check_authority(const uint8_t *value, size_t len) {
- const uint8_t *last;
- for (last = value + len; value != last; ++value) {
- if (!VALID_AUTHORITY_CHARS[*value]) {
- return 0;
- }
- }
- return 1;
-}
-
static int check_scheme(const uint8_t *value, size_t len) {
const uint8_t *last;
if (len == 0) {
@@ -440,7 +362,7 @@ int nghttp2_http_on_header(nghttp2_session *session, nghttp2_stream *stream,

if (nv->token == NGHTTP2_TOKEN__AUTHORITY ||
nv->token == NGHTTP2_TOKEN_HOST) {
- rv = check_authority(nv->value->base, nv->value->len);
+ rv = nghttp2_check_authority(nv->value->base, nv->value->len);
} else if (nv->token == NGHTTP2_TOKEN__SCHEME) {
rv = check_scheme(nv->value->base, nv->value->len);
} else {
2 changes: 2 additions & 0 deletions docs/root/intro/version_history.rst
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,11 @@ Version history
* http: :ref:`AUTO <envoy_api_enum_value_config.filter.network.http_connection_manager.v2.HttpConnectionManager.CodecType.AUTO>` codec protocol inference now requires the H2 magic bytes to be the first bytes transmitted by a downstream client.
* http: remove h2c upgrade headers for HTTP/1 as h2c upgrades are currently not supported.
* http: absolute URL support is now on by default. The prior behavior can be reinstated by setting :ref:`allow_absolute_url <envoy_api_field_core.Http1ProtocolOptions.allow_absolute_url>` to false.
* http: added strict authority checking. This can be reversed temporarily by setting the runtime feature `envoy.reloadable_features.strict_authority_validation` to false.
* http: support :ref:`host rewrite <envoy_api_msg_config.filter.http.dynamic_forward_proxy.v2alpha.PerRouteConfig>` in the dynamic forward proxy.
* http: support :ref:`disabling the filter per route <envoy_api_msg_config.filter.http.grpc_http1_reverse_bridge.v2alpha1.FilterConfigPerRoute>` in the grpc http1 reverse bridge filter.
* http: added the ability to :ref:`configure max connection duration <envoy_api_field_core.HttpProtocolOptions.max_connection_duration>` for downstream connections.
* http: trim LWS at the end of header keys, for correct HTTP/1.1 header parsing.
* listeners: added :ref:`continue_on_listener_filters_timeout <envoy_api_field_Listener.continue_on_listener_filters_timeout>` to configure whether a listener will still create a connection when listener filters time out.
* listeners: added :ref:`HTTP inspector listener filter <config_listener_filters_http_inspector>`.
* listeners: added :ref:`connection balancer <envoy_api_field_Listener.connection_balance_config>`
Expand Down
2 changes: 2 additions & 0 deletions include/envoy/stream_info/stream_info.h
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,8 @@ struct ResponseCodeDetailValues {
// indicates that original "success" headers may have been sent downstream
// despite the subsequent failure.
const std::string LateUpstreamReset = "upstream_reset_after_response_started";
// The request was rejected due to invalid characters in the HOST/:authority header.
const std::string InvalidAuthority = "invalid_authority";
};

using ResponseCodeDetails = ConstSingleton<ResponseCodeDetailValues>;
Expand Down
9 changes: 9 additions & 0 deletions source/common/http/conn_manager_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -779,6 +779,15 @@ void ConnectionManagerImpl::ActiveStream::decodeHeaders(HeaderMapPtr&& headers,
}
}

// Make sure the host is valid.
if (Runtime::runtimeFeatureEnabled("envoy.reloadable_features.strict_authority_validation") &&
!HeaderUtility::authorityIsValid(request_headers_->Host()->value().getStringView())) {
sendLocalReply(Grpc::Common::hasGrpcContentType(*request_headers_), Code::BadRequest, "",
nullptr, is_head_request_, absl::nullopt,
StreamInfo::ResponseCodeDetails::get().InvalidAuthority);
return;
}

// Currently we only support relative paths at the application layer. We expect the codec to have
// broken the path into pieces if applicable. NOTE: Currently the HTTP/1.1 codec only does this
// when the allow_absolute_url flag is enabled on the HCM.
Expand Down
5 changes: 5 additions & 0 deletions source/common/http/header_utility.cc
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,11 @@ bool HeaderUtility::headerIsValid(const absl::string_view header_value) {
header_value.size()) != 0);
}

bool HeaderUtility::authorityIsValid(const absl::string_view header_value) {
return (nghttp2_check_authority(reinterpret_cast<const uint8_t*>(header_value.data()),
header_value.size()) != 0);
}

void HeaderUtility::addHeaders(HeaderMap& headers, const HeaderMap& headers_to_add) {
headers_to_add.iterate(
[](const HeaderEntry& header, void* context) -> HeaderMap::Iterate {
Expand Down
6 changes: 6 additions & 0 deletions source/common/http/header_utility.h
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,12 @@ class HeaderUtility {
*/
static bool headerIsValid(const absl::string_view header_value);

/**
* Validates that the characters in the authority are valid.
* @return bool true if the header values are valid, false otherwise.
*/
static bool authorityIsValid(const absl::string_view authority_value);

/**
* Add headers from one HeaderMap to another
* @param headers target where headers will be added
Expand Down
6 changes: 4 additions & 2 deletions source/common/http/http1/codec_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,9 @@ void ConnectionImpl::onHeaderValue(const char* data, size_t length) {
return;
}

const absl::string_view header_value = absl::string_view(data, length);
// Work around a bug in http_parser where trailing whitespace is not trimmed
// as the spec requires: https://tools.ietf.org/html/rfc7230#section-3.2.4
const absl::string_view header_value = StringUtil::trim(absl::string_view(data, length));

if (strict_header_validation_) {
if (!Http::HeaderUtility::headerIsValid(header_value)) {
Expand All @@ -483,7 +485,7 @@ void ConnectionImpl::onHeaderValue(const char* data, size_t length) {
}

header_parsing_state_ = HeaderParsingState::Value;
current_header_value_.append(data, length);
current_header_value_.append(header_value.data(), header_value.length());

// Verify that the cached value in byte size exists.
ASSERT(current_header_map_->byteSize().has_value());
Expand Down
2 changes: 2 additions & 0 deletions source/common/runtime/runtime_features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ constexpr const char* runtime_features[] = {
"envoy.reloadable_features.buffer_filter_populate_content_length",
"envoy.reloadable_features.trusted_forwarded_proto",
"envoy.reloadable_features.outlier_detection_support_for_grpc_status",
"envoy.reloadable_features.strict_header_validation",
"envoy.reloadable_features.strict_authority_validation",
};

// This is a list of configuration fields which are disallowed by default in Envoy
Expand Down
7 changes: 7 additions & 0 deletions test/common/http/header_utility_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -464,6 +464,13 @@ TEST(HeaderIsValidTest, ValidHeaderValuesAreAccepted) {
EXPECT_TRUE(HeaderUtility::headerIsValid("Some Other Value"));
}

TEST(HeaderIsValidTest, AuthIsValid) {
EXPECT_TRUE(HeaderUtility::authorityIsValid("strangebutlegal$-%&'"));
EXPECT_FALSE(HeaderUtility::authorityIsValid("illegal{}"));
// Full checks are done by Http2CodecImplTest.CheckAuthority, cross checking
// against nghttp2 compliance.
}

TEST(HeaderAddTest, HeaderAdd) {
TestHeaderMapImpl headers{{"myheader1", "123value"}};
TestHeaderMapImpl headers_to_add{{"myheader2", "456value"}};
Expand Down
39 changes: 39 additions & 0 deletions test/common/http/http1/codec_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,25 @@ class Http1ServerConnectionImplTest : public testing::Test {
void testRequestHeadersExceedLimit(std::string header_string);
void testRequestHeadersAccepted(std::string header_string);

// Send the request, and validate the received request headers.
// Then send a response just to clean up.
void sendAndValidateRequestAndSendResponse(absl::string_view raw_request,
const TestHeaderMapImpl& expected_request_headers) {
NiceMock<Http::MockStreamDecoder> decoder;
Http::StreamEncoder* response_encoder = nullptr;
EXPECT_CALL(callbacks_, newStream(_, _))
.Times(1)
.WillOnce(Invoke([&](Http::StreamEncoder& encoder, bool) -> Http::StreamDecoder& {
response_encoder = &encoder;
return decoder;
}));
EXPECT_CALL(decoder, decodeHeaders_(HeaderMapEqual(&expected_request_headers), true)).Times(1);
Buffer::OwnedImpl buffer(raw_request);
codec_->dispatch(buffer);
EXPECT_EQ(0U, buffer.length());
response_encoder->encodeHeaders(TestHeaderMapImpl{{":status", "200"}}, true);
}

protected:
uint32_t max_request_headers_kb_{Http::DEFAULT_MAX_REQUEST_HEADERS_KB};
uint32_t max_request_headers_count_{Http::DEFAULT_MAX_HEADERS_COUNT};
Expand Down Expand Up @@ -167,6 +186,23 @@ TEST_F(Http1ServerConnectionImplTest, EmptyHeader) {
EXPECT_EQ(0U, buffer.length());
}

TEST_F(Http1ServerConnectionImplTest, HostWithLWS) {
initialize();

TestHeaderMapImpl expected_headers{{":authority", "host"}, {":path", "/"}, {":method", "GET"}};

// Regression test spaces before and after the host header value.
sendAndValidateRequestAndSendResponse("GET / HTTP/1.1\r\nHost: host \r\n\r\n", expected_headers);

// Regression test tabs before and after the host header value.
sendAndValidateRequestAndSendResponse("GET / HTTP/1.1\r\nHost: host \r\n\r\n",
expected_headers);

// Regression test mixed spaces and tabs before and after the host header value.
sendAndValidateRequestAndSendResponse(
"GET / HTTP/1.1\r\nHost: host \r\n\r\n", expected_headers);
}

TEST_F(Http1ServerConnectionImplTest, Http10) {
initialize();

Expand Down Expand Up @@ -449,6 +485,9 @@ TEST_F(Http1ServerConnectionImplTest, HeaderInvalidCharsRejection) {
// Regression test for http-parser allowing embedded NULs in header values,
// verify we reject them.
TEST_F(Http1ServerConnectionImplTest, HeaderEmbeddedNulRejection) {
TestScopedRuntime scoped_runtime;
Runtime::LoaderSingleton::getExisting()->mergeValues(
{{"envoy.reloadable_features.strict_header_validation", "false"}});
initialize();

InSequence sequence;
Expand Down
1 change: 1 addition & 0 deletions test/common/http/http2/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ envoy_cc_test(
"//source/common/event:dispatcher_lib",
"//source/common/http:exception_lib",
"//source/common/http:header_map_lib",
"//source/common/http:header_utility_lib",
"//source/common/http/http2:codec_lib",
"//source/common/stats:stats_lib",
"//test/common/http:common_lib",
Expand Down
Loading

0 comments on commit 6a6ac1c

Please sign in to comment.