Skip to content

Commit

Permalink
header: New HeaderMatcher and StringMatcher type - Contains (#12623)
Browse files Browse the repository at this point in the history
For matching values in the header that might be somewhere in the middle of the header, the present option is to use Regex in the form .Search-Pattern.. This can cause catastrophic backtracking as described in #7728
As a solution, I have introduced another header match type called contains which is based on absl::StrContains().

Risk Level: Low
Testing: Unit tests are included and manual testing was performed.

Fixes #12590

Signed-off-by: Shivanshu Goswami <shigoswami@ebay.com>
  • Loading branch information
Shivanshu Goswami authored Aug 21, 2020
1 parent 60de12b commit e322daa
Show file tree
Hide file tree
Showing 15 changed files with 193 additions and 9 deletions.
11 changes: 10 additions & 1 deletion api/envoy/config/route/v3/route_components.proto
Original file line number Diff line number Diff line change
Expand Up @@ -1687,7 +1687,7 @@ message RateLimit {
// value.
//
// [#next-major-version: HeaderMatcher should be refactored to use StringMatcher.]
// [#next-free-field: 12]
// [#next-free-field: 13]
message HeaderMatcher {
option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.route.HeaderMatcher";

Expand Down Expand Up @@ -1741,6 +1741,15 @@ message HeaderMatcher {
//
// * The suffix *abcd* matches the value *xyzabcd*, but not for *xyzbcd*.
string suffix_match = 10 [(validate.rules).string = {min_bytes: 1}];

// If specified, header match will be performed based on whether the header value contains
// the given value or not.
// Note: empty contains match is not allowed, please use present_match instead.
//
// Examples:
//
// * The value *abcd* matches the value *xyzabcdpqr*, but not for *xyzbcdpqr*.
string contains_match = 12 [(validate.rules).string = {min_bytes: 1}];
}

// If specified, the match result will be inverted before checking. Defaults to false.
Expand Down
11 changes: 10 additions & 1 deletion api/envoy/config/route/v4alpha/route_components.proto

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 9 additions & 1 deletion api/envoy/type/matcher/v3/string.proto
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE;
// [#protodoc-title: String matcher]

// Specifies the way to match a string.
// [#next-free-field: 7]
// [#next-free-field: 8]
message StringMatcher {
option (udpa.annotations.versioning).previous_message_type = "envoy.type.matcher.StringMatcher";

Expand Down Expand Up @@ -53,6 +53,14 @@ message StringMatcher {

// The input string must match the regular expression specified here.
RegexMatcher safe_regex = 5 [(validate.rules).message = {required: true}];

// The input string must have the substring specified here.
// Note: empty contains match is not allowed, please use regex instead.
//
// Examples:
//
// * *abc* matches the value *xyz.abc.def*
string contains = 7 [(validate.rules).string = {min_bytes: 1}];
}

// If true, indicates the exact/prefix/suffix matching should be case insensitive. This has no
Expand Down
10 changes: 9 additions & 1 deletion api/envoy/type/matcher/v4alpha/string.proto

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions docs/root/version_history/current.rst
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ Minor Behavior Changes
* build: the debug information will be generated separately to reduce target size and reduce compilation time when build in compilation mode `dbg` and `opt`. Users will need to build dwp file to debug with gdb.
* compressor: always insert `Vary` headers for compressible resources even if it's decided not to compress a response due to incompatible `Accept-Encoding` value. The `Vary` header needs to be inserted to let a caching proxy in front of Envoy know that the requested resource still can be served with compression applied.
* decompressor: headers-only requests were incorrectly not advertising accept-encoding when configured to do so. This is now fixed.
* http: added :ref:`contains <envoy_api_msg_type.matcher.StringMatcher>` a new string matcher type which matches if the value of the string has the substring mentioned in contains matcher.
* http: added :ref:`contains <envoy_api_msg_route.HeaderMatcher>` a new header matcher type which matches if the value of the header has the substring mentioned in contains matcher.
* http: added :ref:`headers_to_add <envoy_v3_api_field_extensions.filters.network.http_connection_manager.v3.ResponseMapper.headers_to_add>` to :ref:`local reply mapper <config_http_conn_man_local_reply>` to allow its users to add/append/override response HTTP headers to local replies.
* http: added HCM level configuration of :ref:`error handling on invalid messaging <envoy_v3_api_field_extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.stream_error_on_invalid_http_message>` which substantially changes Envoy's behavior when encountering invalid HTTP/1.1 defaulting to closing the connection instead of allowing reuse. This can temporarily be reverted by setting `envoy.reloadable_features.hcm_stream_error_on_invalid_message` to false, or permanently reverted by setting the :ref:`HCM option <envoy_v3_api_field_extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.stream_error_on_invalid_http_message>` to true to restore prior HTTP/1.1 beavior and setting the *new* HTTP/2 configuration :ref:`override_stream_error_on_invalid_http_message <envoy_v3_api_field_config.core.v3.Http2ProtocolOptions.override_stream_error_on_invalid_http_message>` to false to retain prior HTTP/2 behavior.
* http: changed Envoy to send error headers and body when possible. This behavior may be temporarily reverted by setting `envoy.reloadable_features.allow_response_for_timeout` to false.
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 9 additions & 1 deletion generated_api_shadow/envoy/type/matcher/v3/string.proto

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 9 additions & 1 deletion generated_api_shadow/envoy/type/matcher/v4alpha/string.proto

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions source/common/common/matchers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,12 @@ StringMatcherImpl::StringMatcherImpl(const envoy::type::matcher::v3::StringMatch
throw EnvoyException("ignore_case has no effect for safe_regex.");
}
regex_ = Regex::Utility::parseRegex(matcher_.safe_regex());
} else if (matcher.match_pattern_case() ==
envoy::type::matcher::v3::StringMatcher::MatchPatternCase::kContains) {
if (matcher_.ignore_case()) {
// Cache the lowercase conversion of the Contains matcher for future use
lowercase_contains_match_ = absl::AsciiStrToLower(matcher_.contains());
}
}
}

Expand All @@ -100,6 +106,10 @@ bool StringMatcherImpl::match(const absl::string_view value) const {
case envoy::type::matcher::v3::StringMatcher::MatchPatternCase::kSuffix:
return matcher_.ignore_case() ? absl::EndsWithIgnoreCase(value, matcher_.suffix())
: absl::EndsWith(value, matcher_.suffix());
case envoy::type::matcher::v3::StringMatcher::MatchPatternCase::kContains:
return matcher_.ignore_case()
? absl::StrContains(absl::AsciiStrToLower(value), lowercase_contains_match_)
: absl::StrContains(value, matcher_.contains());
case envoy::type::matcher::v3::StringMatcher::MatchPatternCase::kHiddenEnvoyDeprecatedRegex:
FALLTHRU;
case envoy::type::matcher::v3::StringMatcher::MatchPatternCase::kSafeRegex:
Expand Down
1 change: 1 addition & 0 deletions source/common/common/matchers.h
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ class StringMatcherImpl : public ValueMatcher, public StringMatcher {
private:
const envoy::type::matcher::v3::StringMatcher matcher_;
Regex::CompiledMatcherPtr regex_;
std::string lowercase_contains_match_;
};

class ListMatcher : public ValueMatcher {
Expand Down
7 changes: 7 additions & 0 deletions source/common/http/header_utility.cc
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,10 @@ HeaderUtility::HeaderData::HeaderData(const envoy::config::route::v3::HeaderMatc
header_match_type_ = HeaderMatchType::Suffix;
value_ = config.suffix_match();
break;
case envoy::config::route::v3::HeaderMatcher::HeaderMatchSpecifierCase::kContainsMatch:
header_match_type_ = HeaderMatchType::Contains;
value_ = config.contains_match();
break;
case envoy::config::route::v3::HeaderMatcher::HeaderMatchSpecifierCase::
HEADER_MATCH_SPECIFIER_NOT_SET:
FALLTHRU;
Expand Down Expand Up @@ -132,6 +136,9 @@ bool HeaderUtility::matchHeaders(const HeaderMap& request_headers, const HeaderD
case HeaderMatchType::Suffix:
match = absl::EndsWith(header_view, header_data.value_);
break;
case HeaderMatchType::Contains:
match = absl::StrContains(header_view, header_data.value_);
break;
default:
NOT_REACHED_GCOVR_EXCL_LINE;
}
Expand Down
2 changes: 1 addition & 1 deletion source/common/http/header_utility.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ namespace Http {
*/
class HeaderUtility {
public:
enum class HeaderMatchType { Value, Regex, Range, Present, Prefix, Suffix };
enum class HeaderMatchType { Value, Regex, Range, Present, Prefix, Suffix, Contains };

/**
* Get all instances of the header key specified, and return the values in the vector provided.
Expand Down
49 changes: 49 additions & 0 deletions test/common/common/matchers_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,25 @@ TEST(MetadataTest, MatchStringSuffixValue) {
EXPECT_TRUE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata));
}

TEST(MetadataTest, MatchStringContainsValue) {
envoy::config::core::v3::Metadata metadata;
Envoy::Config::Metadata::mutableMetadataValue(metadata, "envoy.filter.a", "label")
.set_string_value("test");
Envoy::Config::Metadata::mutableMetadataValue(metadata, "envoy.filter.b", "label")
.set_string_value("abcprodef");

envoy::type::matcher::v3::MetadataMatcher matcher;
matcher.set_filter("envoy.filter.b");
matcher.add_path()->set_key("label");

matcher.mutable_value()->mutable_string_match()->set_exact("test");
EXPECT_FALSE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata));
matcher.mutable_value()->mutable_string_match()->set_contains("pride");
EXPECT_FALSE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata));
matcher.mutable_value()->mutable_string_match()->set_contains("prod");
EXPECT_TRUE(Envoy::Matchers::MetadataMatcher(matcher).match(metadata));
}

TEST(MetadataTest, MatchBoolValue) {
envoy::config::core::v3::Metadata metadata;
Envoy::Config::Metadata::mutableMetadataValue(metadata, "envoy.filter.a", "label")
Expand Down Expand Up @@ -306,6 +325,22 @@ TEST(StringMatcher, SuffixMatchIgnoreCase) {
EXPECT_FALSE(Matchers::StringMatcherImpl(matcher).match("other"));
}

TEST(StringMatcher, ContainsMatchIgnoreCase) {
envoy::type::matcher::v3::StringMatcher matcher;
matcher.set_contains("contained-str");
EXPECT_TRUE(Matchers::StringMatcherImpl(matcher).match("abc-contained-str-def"));
EXPECT_TRUE(Matchers::StringMatcherImpl(matcher).match("contained-str"));
EXPECT_FALSE(Matchers::StringMatcherImpl(matcher).match("ABC-Contained-Str-DEF"));
EXPECT_FALSE(Matchers::StringMatcherImpl(matcher).match("abc-container-int-def"));
EXPECT_FALSE(Matchers::StringMatcherImpl(matcher).match("other"));

matcher.set_ignore_case(true);
EXPECT_TRUE(Matchers::StringMatcherImpl(matcher).match("abc-contained-str-def"));
EXPECT_TRUE(Matchers::StringMatcherImpl(matcher).match("abc-cOnTaInEd-str-def"));
EXPECT_FALSE(Matchers::StringMatcherImpl(matcher).match("abc-ContAineR-str-def"));
EXPECT_FALSE(Matchers::StringMatcherImpl(matcher).match("other"));
}

TEST(StringMatcher, SafeRegexValue) {
envoy::type::matcher::v3::StringMatcher matcher;
matcher.mutable_safe_regex()->mutable_google_re2();
Expand Down Expand Up @@ -402,6 +437,20 @@ TEST(PathMatcher, MatchSuffixPath) {
EXPECT_FALSE(Matchers::PathMatcher(matcher).match("/suffiz#suffix"));
}

TEST(PathMatcher, MatchContainsPath) {
envoy::type::matcher::v3::PathMatcher matcher;
matcher.mutable_path()->set_contains("contains");

EXPECT_TRUE(Matchers::PathMatcher(matcher).match("/contains"));
EXPECT_TRUE(Matchers::PathMatcher(matcher).match("/abc-contains"));
EXPECT_TRUE(Matchers::PathMatcher(matcher).match("/contains-abc"));
EXPECT_TRUE(Matchers::PathMatcher(matcher).match("/abc-contains-def"));
EXPECT_TRUE(Matchers::PathMatcher(matcher).match("/abc-contains-def?param=val"));
EXPECT_TRUE(Matchers::PathMatcher(matcher).match("/abc-contains-def#fragment"));
EXPECT_FALSE(Matchers::PathMatcher(matcher).match("/abc-def#containsfragment?param=contains"));
EXPECT_FALSE(Matchers::PathMatcher(matcher).match("/abc-curtains-def"));
}

TEST(PathMatcher, MatchRegexPath) {
envoy::type::matcher::v3::StringMatcher matcher;
matcher.mutable_safe_regex()->mutable_google_re2();
Expand Down
47 changes: 47 additions & 0 deletions test/common/http/header_utility_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,20 @@ suffix_match: value
EXPECT_EQ("value", header_data.value_);
}

TEST(HeaderDataConstructorTest, ContainsMatchSpecifier) {
const std::string yaml = R"EOF(
name: test-header
contains_match: somevalueinside
)EOF";

HeaderUtility::HeaderData header_data =
HeaderUtility::HeaderData(parseHeaderMatcherFromYaml(yaml));

EXPECT_EQ("test-header", header_data.name_.get());
EXPECT_EQ(HeaderUtility::HeaderMatchType::Contains, header_data.header_match_type_);
EXPECT_EQ("somevalueinside", header_data.value_);
}

TEST(HeaderDataConstructorTest, InvertMatchSpecifier) {
const std::string yaml = R"EOF(
name: test-header
Expand Down Expand Up @@ -499,6 +513,39 @@ invert_match: true
EXPECT_FALSE(HeaderUtility::matchHeaders(unmatching_headers, header_data));
}

TEST(MatchHeadersTest, HeaderContainsMatch) {
TestRequestHeaderMapImpl matching_headers{{"match-header", "123onevalue456"}};
TestRequestHeaderMapImpl unmatching_headers{{"match-header", "123anothervalue456"}};

const std::string yaml = R"EOF(
name: match-header
contains_match: onevalue
)EOF";

std::vector<HeaderUtility::HeaderDataPtr> header_data;
header_data.push_back(
std::make_unique<HeaderUtility::HeaderData>(parseHeaderMatcherFromYaml(yaml)));
EXPECT_TRUE(HeaderUtility::matchHeaders(matching_headers, header_data));
EXPECT_FALSE(HeaderUtility::matchHeaders(unmatching_headers, header_data));
}

TEST(MatchHeadersTest, HeaderContainsInverseMatch) {
TestRequestHeaderMapImpl matching_headers{{"match-header", "123onevalue456"}};
TestRequestHeaderMapImpl unmatching_headers{{"match-header", "123anothervalue456"}};

const std::string yaml = R"EOF(
name: match-header
contains_match: onevalue
invert_match: true
)EOF";

std::vector<HeaderUtility::HeaderDataPtr> header_data;
header_data.push_back(
std::make_unique<HeaderUtility::HeaderData>(parseHeaderMatcherFromYaml(yaml)));
EXPECT_TRUE(HeaderUtility::matchHeaders(unmatching_headers, header_data));
EXPECT_FALSE(HeaderUtility::matchHeaders(matching_headers, header_data));
}

TEST(HeaderIsValidTest, InvalidHeaderValuesAreRejected) {
// ASCII values 1-31 are control characters (with the exception of ASCII
// values 9, 10, and 13 which are a horizontal tab, line feed, and carriage
Expand Down

0 comments on commit e322daa

Please sign in to comment.