diff --git a/api/docs/BUILD b/api/docs/BUILD index da20fe5e7171..11ef3876b218 100644 --- a/api/docs/BUILD +++ b/api/docs/BUILD @@ -102,6 +102,7 @@ proto_library( "//envoy/type:range", "//envoy/type/matcher:metadata", "//envoy/type/matcher:number", + "//envoy/type/matcher:regex", "//envoy/type/matcher:string", ], ) diff --git a/api/envoy/api/v2/route/BUILD b/api/envoy/api/v2/route/BUILD index 2fec56ae389b..968ab1c67be2 100644 --- a/api/envoy/api/v2/route/BUILD +++ b/api/envoy/api/v2/route/BUILD @@ -10,6 +10,8 @@ api_proto_library_internal( "//envoy/api/v2/core:base", "//envoy/type:percent", "//envoy/type:range", + "//envoy/type/matcher:regex", + "//envoy/type/matcher:string", ], ) @@ -20,5 +22,7 @@ api_go_proto_library( "//envoy/api/v2/core:base_go_proto", "//envoy/type:percent_go_proto", "//envoy/type:range_go_proto", + "//envoy/type/matcher:regex_go_proto", + "//envoy/type/matcher:string_go_proto", ], ) diff --git a/api/envoy/api/v2/route/route.proto b/api/envoy/api/v2/route/route.proto index 93d021ddb216..e0fbaf0fd685 100644 --- a/api/envoy/api/v2/route/route.proto +++ b/api/envoy/api/v2/route/route.proto @@ -9,6 +9,8 @@ option go_package = "route"; option java_generic_services = true; import "envoy/api/v2/core/base.proto"; +import "envoy/type/matcher/regex.proto"; +import "envoy/type/matcher/string.proto"; import "envoy/type/percent.proto"; import "envoy/type/range.proto"; @@ -349,7 +351,25 @@ message RouteMatch { // * The regex */b[io]t* matches the path */bot* // * The regex */b[io]t* does not match the path */bite* // * The regex */b[io]t* does not match the path */bit/bot* - string regex = 3 [(validate.rules).string.max_bytes = 1024]; + // + // .. attention:: + // This field has been deprecated in favor of `safe_regex` as it is not safe for use with + // untrusted input in all cases. + string regex = 3 [(validate.rules).string.max_bytes = 1024, deprecated = true]; + + // If specified, the route is a regular expression rule meaning that the + // regex must match the *:path* header once the query string is removed. The entire path + // (without the query string) must match the regex. The rule will not match if only a + // subsequence of the *:path* header matches the regex. + // + // [#next-major-version: In the v3 API we should redo how path specification works such + // that we utilize StringMatcher, and additionally have consistent options around whether we + // strip query strings, do a case sensitive match, etc. In the interim it will be too disruptive + // to deprecate the existing options. We should even consider whether we want to do away with + // path_specifier entirely and just rely on a set of header matchers which can already match + // on :path, etc. The issue with that is it is unclear how to generically deal with query string + // stripping. This needs more thought.] + type.matcher.RegexMatcher safe_regex = 10 [(validate.rules).message.required = true]; } // Indicates that prefix/path matching should be case insensitive. The default @@ -404,12 +424,24 @@ message CorsPolicy { // Specifies the origins that will be allowed to do CORS requests. // // An origin is allowed if either allow_origin or allow_origin_regex match. - repeated string allow_origin = 1; + // + // .. attention:: + // This field has been deprecated in favor of `allow_origin_string_match`. + repeated string allow_origin = 1 [deprecated = true]; // Specifies regex patterns that match allowed origins. // // An origin is allowed if either allow_origin or allow_origin_regex match. - repeated string allow_origin_regex = 8 [(validate.rules).repeated .items.string.max_bytes = 1024]; + // + // .. attention:: + // This field has been deprecated in favor of `allow_origin_string_match` as it is not safe for + // use with untrusted input in all cases. + repeated string allow_origin_regex = 8 + [(validate.rules).repeated .items.string.max_bytes = 1024, deprecated = true]; + + // Specifies string patterns that match allowed origins. An origin is allowed if any of the + // string matchers match. + repeated type.matcher.StringMatcher allow_origin_string_match = 11; // Specifies the content for the *access-control-allow-methods* header. string allow_methods = 2; @@ -1077,18 +1109,28 @@ message VirtualCluster { // * The regex */rides/\d+* matches the path */rides/0* // * The regex */rides/\d+* matches the path */rides/123* // * The regex */rides/\d+* does not match the path */rides/123/456* - string pattern = 1 [(validate.rules).string = {min_bytes: 1, max_bytes: 1024}]; + // + // .. attention:: + // This field has been deprecated in favor of `headers` as it is not safe for use with + // untrusted input in all cases. + string pattern = 1 [(validate.rules).string.max_bytes = 1024, deprecated = true]; + + // Specifies a list of header matchers to use for matching requests. Each specified header must + // match. The pseudo-headers `:path` and `:method` can be used to match the request path and + // method, respectively. + repeated HeaderMatcher headers = 4; - // Specifies the name of the virtual cluster. The virtual cluster name as well + // Specifies the name of the virtual cluster. The virtual cluster name as well // as the virtual host name are used when emitting statistics. The statistics are emitted by the // router filter and are documented :ref:`here `. string name = 2 [(validate.rules).string.min_bytes = 1]; // Optionally specifies the HTTP method to match on. For example GET, PUT, // etc. - // [#comment:TODO(htuch): add (validate.rules).enum.defined_only = true once - // https://github.com/lyft/protoc-gen-validate/issues/42 is resolved.] - core.RequestMethod method = 3; + // + // .. attention:: + // This field has been deprecated in favor of `headers`. + core.RequestMethod method = 3 [deprecated = true]; } // Global rate limiting :ref:`architecture overview `. @@ -1248,6 +1290,7 @@ message RateLimit { // ` header will match, regardless of the header's // value. // +// [#next-major-version: HeaderMatcher should be refactored to use StringMatcher.] message HeaderMatcher { // Specifies the name of the header in the request. string name = 1 [(validate.rules).string.min_bytes = 1]; @@ -1273,7 +1316,16 @@ message HeaderMatcher { // * The regex *\d{3}* matches the value *123* // * The regex *\d{3}* does not match the value *1234* // * The regex *\d{3}* does not match the value *123.456* - string regex_match = 5 [(validate.rules).string.max_bytes = 1024]; + // + // .. attention:: + // This field has been deprecated in favor of `safe_regex_match` as it is not safe for use + // with untrusted input in all cases. + string regex_match = 5 [(validate.rules).string.max_bytes = 1024, deprecated = true]; + + // If specified, this regex string is a regular expression rule which implies the entire request + // header value must match the regex. The rule will not match if only a subsequence of the + // request header value matches the regex. + type.matcher.RegexMatcher safe_regex_match = 11; // If specified, header match will be performed based on range. // The rule will match if the request header value is within this range. @@ -1328,11 +1380,25 @@ message QueryParameterMatcher { // Specifies the value of the key. If the value is absent, a request // that contains the key in its query string will match, whether the // key appears with a value (e.g., "?debug=true") or not (e.g., "?debug") - string value = 3; + // + // ..attention:: + // This field is deprecated. Use an `exact` match inside the `string_match` field. + string value = 3 [deprecated = true]; // Specifies whether the query parameter value is a regular expression. // Defaults to false. The entire query parameter value (i.e., the part to // the right of the equals sign in "key=value") must match the regex. // E.g., the regex "\d+$" will match "123" but not "a123" or "123a". - google.protobuf.BoolValue regex = 4; + // + // ..attention:: + // This field is deprecated. Use a `safe_regex` match inside the `string_match` field. + google.protobuf.BoolValue regex = 4 [deprecated = true]; + + oneof query_parameter_match_specifier { + // Specifies whether a query parameter value should match against a string. + type.matcher.StringMatcher string_match = 5 [(validate.rules).message.required = true]; + + // Specifies whether a query parameter should be present. + bool present_match = 6; + } } diff --git a/api/envoy/type/matcher/BUILD b/api/envoy/type/matcher/BUILD index ec4aa09b6c63..5fe594db4ca2 100644 --- a/api/envoy/type/matcher/BUILD +++ b/api/envoy/type/matcher/BUILD @@ -40,11 +40,17 @@ api_proto_library_internal( name = "string", srcs = ["string.proto"], visibility = ["//visibility:public"], + deps = [ + ":regex", + ], ) api_go_proto_library( name = "string", proto = ":string", + deps = [ + ":regex_go_proto", + ], ) api_proto_library_internal( @@ -65,3 +71,14 @@ api_go_proto_library( ":string_go_proto", ], ) + +api_proto_library_internal( + name = "regex", + srcs = ["regex.proto"], + visibility = ["//visibility:public"], +) + +api_go_proto_library( + name = "regex", + proto = ":regex", +) diff --git a/api/envoy/type/matcher/regex.proto b/api/envoy/type/matcher/regex.proto new file mode 100644 index 000000000000..b3b7194441eb --- /dev/null +++ b/api/envoy/type/matcher/regex.proto @@ -0,0 +1,37 @@ +syntax = "proto3"; + +package envoy.type.matcher; + +option java_outer_classname = "StringProto"; +option java_multiple_files = true; +option java_package = "io.envoyproxy.envoy.type.matcher"; +option go_package = "matcher"; + +import "google/protobuf/wrappers.proto"; +import "validate/validate.proto"; + +// [#protodoc-title: RegexMatcher] + +// A regex matcher designed for safety when used with untrusted input. +message RegexMatcher { + // Google's `RE2 `_ regex engine. The regex string must adhere to + // the documented `syntax `_. The engine is designed + // to complete execution in linear time as well as limit the amount of memory used. + message GoogleRE2 { + // This field controls the RE2 "program size" which is a rough estimate of how complex a + // compiled regex is to evaluate. A regex that has a program size greater than the configured + // value will fail to compile. In this case, the configured max program size can be increased + // or the regex can be simplified. If not specified, the default is 100. + google.protobuf.UInt32Value max_program_size = 1; + } + + oneof engine_type { + option (validate.required) = true; + + // Google's RE2 regex engine. + GoogleRE2 google_re2 = 1 [(validate.rules).message.required = true]; + } + + // The regex match string. The string must be supported by the configured engine. + string regex = 2 [(validate.rules).string.min_bytes = 1]; +} diff --git a/api/envoy/type/matcher/string.proto b/api/envoy/type/matcher/string.proto index 55f2171af53e..35628b7ccb10 100644 --- a/api/envoy/type/matcher/string.proto +++ b/api/envoy/type/matcher/string.proto @@ -7,6 +7,8 @@ option java_multiple_files = true; option java_package = "io.envoyproxy.envoy.type.matcher"; option go_package = "matcher"; +import "envoy/type/matcher/regex.proto"; + import "validate/validate.proto"; // [#protodoc-title: StringMatcher] @@ -48,7 +50,14 @@ message StringMatcher { // * The regex *\d{3}* matches the value *123* // * The regex *\d{3}* does not match the value *1234* // * The regex *\d{3}* does not match the value *123.456* - string regex = 4 [(validate.rules).string.max_bytes = 1024]; + // + // .. attention:: + // This field has been deprecated in favor of `safe_regex` as it is not safe for use with + // untrusted input in all cases. + string regex = 4 [(validate.rules).string.max_bytes = 1024, deprecated = true]; + + // The input string must match the regular expression specified here. + RegexMatcher safe_regex = 5 [(validate.rules).message.required = true]; } } diff --git a/bazel/repository_locations.bzl b/bazel/repository_locations.bzl index ebb3e8569064..7c2c5ad23e62 100644 --- a/bazel/repository_locations.bzl +++ b/bazel/repository_locations.bzl @@ -253,8 +253,8 @@ REPOSITORY_LOCATIONS = dict( urls = ["https://github.com/google/cel-cpp/archive/d9d02b20ab85da2444dbdd03410bac6822141364.tar.gz"], ), com_googlesource_code_re2 = dict( - sha256 = "f31db9cd224d018a7e4fe88ef84aaa874b0b3ed91d4d98ee5a1531101d3fdc64", - strip_prefix = "re2-87e2ad45e7b18738e1551474f7ee5886ff572059", - urls = ["https://github.com/google/re2/archive/87e2ad45e7b18738e1551474f7ee5886ff572059.tar.gz"], + sha256 = "38bc0426ee15b5ed67957017fd18201965df0721327be13f60496f2b356e3e01", + strip_prefix = "re2-2019-08-01", + urls = ["https://github.com/google/re2/archive/2019-08-01.tar.gz"], ), ) diff --git a/docs/build.sh b/docs/build.sh index b147712dd537..7b89527bb24c 100755 --- a/docs/build.sh +++ b/docs/build.sh @@ -156,6 +156,7 @@ PROTO_RST=" /envoy/type/matcher/metadata/envoy/type/matcher/metadata.proto.rst /envoy/type/matcher/value/envoy/type/matcher/value.proto.rst /envoy/type/matcher/number/envoy/type/matcher/number.proto.rst + /envoy/type/matcher/regex/envoy/type/matcher/regex.proto.rst /envoy/type/matcher/string/envoy/type/matcher/string.proto.rst " diff --git a/docs/root/api-v2/types/types.rst b/docs/root/api-v2/types/types.rst index 4a6ba6194c62..a5a84c33a13d 100644 --- a/docs/root/api-v2/types/types.rst +++ b/docs/root/api-v2/types/types.rst @@ -10,5 +10,6 @@ Types ../type/range.proto ../type/matcher/metadata.proto ../type/matcher/number.proto + ../type/matcher/regex.proto ../type/matcher/string.proto ../type/matcher/value.proto diff --git a/docs/root/intro/deprecated.rst b/docs/root/intro/deprecated.rst index 6ca0b034b759..fb7832739383 100644 --- a/docs/root/intro/deprecated.rst +++ b/docs/root/intro/deprecated.rst @@ -12,8 +12,25 @@ Deprecated items below are listed in chronological order. Version 1.12.0 (pending) ======================== -* The ORIGINAL_DST_LB :ref:`load balancing policy ` is deprecated, use CLUSTER_PROVIDED policy instead when configuring an :ref:`original destination cluster `. -* The :option:`--allow-unknown-fields` command-line option, use :option:`--allow-unknown-static-fields` instead. +* The ORIGINAL_DST_LB :ref:`load balancing policy ` is + deprecated, use CLUSTER_PROVIDED policy instead when configuring an :ref:`original destination + cluster `. +* The `regex` field in :ref:`StringMatcher ` has been + deprecated in favor of the `safe_regex` field. +* The `regex` field in :ref:`RouteMatch ` has been + deprecated in favor of the `safe_regex` field. +* The `allow_origin` and `allow_origin_regex` fields in :ref:`CorsPolicy + ` have been deprecated in favor of the + `allow_origin_string_match` field. +* The `pattern` and `method` fields in :ref:`VirtualCluster ` + have been deprecated in favor of the `headers` field. +* The `regex_match` field in :ref:`HeaderMatcher ` has been + deprecated in favor of the `safe_regex_match` field. +* The `value` and `regex` fields in :ref:`QueryParameterMatcher + ` has been deprecated in favor of the `string_match` + and `present_match` fields. +* The :option:`--allow-unknown-fields` command-line option, + use :option:`--allow-unknown-static-fields` instead. Version 1.11.0 (July 11, 2019) ============================== diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index 609554b4d374..caf541e796d5 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -7,8 +7,8 @@ Version history * admin: added ability to configure listener :ref:`socket options `. * admin: added config dump support for Secret Discovery Service :ref:`SecretConfigDump `. * api: added ::ref:`set_node_on_first_message_only ` option to omit the node identifier from the subsequent discovery requests on the same stream. -* config: enforcing that terminal filters (e.g. HttpConnectionManager for L4, router for L7) be the last in their respective filter chains. * buffer filter: the buffer filter populates content-length header if not present, behavior can be disabled using the runtime feature `envoy.reloadable_features.buffer_filter_populate_content_length`. +* config: enforcing that terminal filters (e.g. HttpConnectionManager for L4, router for L7) be the last in their respective filter chains. * config: added access log :ref:`extension filter`. * config: added support for :option:`--reject-unknown-dynamic-fields`, providing independent control over whether unknown fields are rejected in static and dynamic configuration. By default, unknown @@ -29,11 +29,15 @@ Version history * http: added the ability to :ref:`merge adjacent slashes` in the path. * listeners: added :ref:`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 `. -* performance: stats symbol table implementation (disabled by default; to test it, add "--use-fake-symbol-table 0" to the command-line arguments when starting Envoy). -* redis: added :ref:`read_policy ` to allow reading from redis replicas for Redis Cluster deployments. -* rbac: added support for DNS SAN as :ref:`principal_name `. * lua: extended `httpCall()` and `respond()` APIs to accept headers with entry values that can be a string or table of strings. * performance: new buffer implementation enabled by default (to disable add "--use-libevent-buffers 1" to the command-line arguments when starting Envoy). +* performance: stats symbol table implementation (disabled by default; to test it, add "--use-fake-symbol-table 0" to the command-line arguments when starting Envoy). +* rbac: added support for DNS SAN as :ref:`principal_name `. +* redis: added :ref:`read_policy ` to allow reading from redis replicas for Redis Cluster deployments. +* regex: introduce new :ref:`RegexMatcher ` type that + provides a safe regex implementation for untrusted user input. This type is now used in all + configuration that processes user provided input. See :ref:`deprecated configuration details + ` for more information. * rbac: added conditions to the policy, see :ref:`condition `. * router: added :ref:`rq_retry_skipped_request_not_complete ` counter stat to router stats. * router check tool: add coverage reporting & enforcement. diff --git a/include/envoy/common/BUILD b/include/envoy/common/BUILD index 4e04008c25d0..105f8b4374b7 100644 --- a/include/envoy/common/BUILD +++ b/include/envoy/common/BUILD @@ -29,6 +29,19 @@ envoy_cc_library( hdrs = ["time.h"], ) +envoy_cc_library( + name = "matchers_interface", + hdrs = ["matchers.h"], +) + +envoy_cc_library( + name = "regex_interface", + hdrs = ["regex.h"], + deps = [ + ":matchers_interface", + ], +) + envoy_cc_library( name = "token_bucket_interface", hdrs = ["token_bucket.h"], diff --git a/include/envoy/common/matchers.h b/include/envoy/common/matchers.h new file mode 100644 index 000000000000..4a79d00b97d6 --- /dev/null +++ b/include/envoy/common/matchers.h @@ -0,0 +1,28 @@ +#pragma once + +#include + +#include "envoy/common/pure.h" + +#include "absl/strings/string_view.h" + +namespace Envoy { +namespace Matchers { + +/** + * Generic string matching interface. + */ +class StringMatcher { +public: + virtual ~StringMatcher() = default; + + /** + * Return whether a passed string value matches. + */ + virtual bool match(const absl::string_view value) const PURE; +}; + +using StringMatcherPtr = std::unique_ptr; + +} // namespace Matchers +} // namespace Envoy diff --git a/include/envoy/common/regex.h b/include/envoy/common/regex.h new file mode 100644 index 000000000000..f4cdc1699ef7 --- /dev/null +++ b/include/envoy/common/regex.h @@ -0,0 +1,21 @@ +#pragma once + +#include + +#include "envoy/common/matchers.h" + +namespace Envoy { +namespace Regex { + +/** + * A compiled regex expression matcher which uses an abstract regex engine. + * + * NOTE: Currently this is the same as StringMatcher, however has been split out as in the future + * we are likely to add other methods such as returning captures, etc. + */ +class CompiledMatcher : public Matchers::StringMatcher {}; + +using CompiledMatcherPtr = std::unique_ptr; + +} // namespace Regex +} // namespace Envoy diff --git a/include/envoy/router/BUILD b/include/envoy/router/BUILD index 3ebe1a1b70e3..d62054044670 100644 --- a/include/envoy/router/BUILD +++ b/include/envoy/router/BUILD @@ -48,6 +48,7 @@ envoy_cc_library( external_deps = ["abseil_optional"], deps = [ "//include/envoy/access_log:access_log_interface", + "//include/envoy/common:matchers_interface", "//include/envoy/config:typed_metadata_interface", "//include/envoy/http:codec_interface", "//include/envoy/http:codes_interface", diff --git a/include/envoy/router/router.h b/include/envoy/router/router.h index a14864b4b287..5b793ec8a841 100644 --- a/include/envoy/router/router.h +++ b/include/envoy/router/router.h @@ -10,6 +10,7 @@ #include "envoy/access_log/access_log.h" #include "envoy/api/v2/core/base.pb.h" +#include "envoy/common/matchers.h" #include "envoy/config/typed_metadata.h" #include "envoy/http/codec.h" #include "envoy/http/codes.h" @@ -101,14 +102,9 @@ class CorsPolicy { virtual ~CorsPolicy() = default; /** - * @return std::list& access-control-allow-origin values. + * @return std::vector& access-control-allow-origin matchers. */ - virtual const std::list& allowOrigins() const PURE; - - /* - * @return std::list& regexes that match allowed origins. - */ - virtual const std::list& allowOriginRegexes() const PURE; + virtual const std::vector& allowOrigins() const PURE; /** * @return std::string access-control-allow-methods value. diff --git a/source/common/access_log/access_log_formatter.cc b/source/common/access_log/access_log_formatter.cc index 899ae27ff8d1..295c544aa76e 100644 --- a/source/common/access_log/access_log_formatter.cc +++ b/source/common/access_log/access_log_formatter.cc @@ -1,6 +1,7 @@ #include "common/access_log/access_log_formatter.h" #include +#include #include #include diff --git a/source/common/access_log/access_log_impl.cc b/source/common/access_log/access_log_impl.cc index 498c3e18176d..837842f621ae 100644 --- a/source/common/access_log/access_log_impl.cc +++ b/source/common/access_log/access_log_impl.cc @@ -193,13 +193,12 @@ bool NotHealthCheckFilter::evaluate(const StreamInfo::StreamInfo& info, const Ht return !info.healthCheck(); } -HeaderFilter::HeaderFilter(const envoy::config::filter::accesslog::v2::HeaderFilter& config) { - header_data_.push_back(Http::HeaderUtility::HeaderData(config.header())); -} +HeaderFilter::HeaderFilter(const envoy::config::filter::accesslog::v2::HeaderFilter& config) + : header_data_(std::make_unique(config.header())) {} bool HeaderFilter::evaluate(const StreamInfo::StreamInfo&, const Http::HeaderMap& request_headers, const Http::HeaderMap&, const Http::HeaderMap&) { - return Http::HeaderUtility::matchHeaders(request_headers, header_data_); + return Http::HeaderUtility::matchHeaders(request_headers, *header_data_); } ResponseFlagFilter::ResponseFlagFilter( diff --git a/source/common/access_log/access_log_impl.h b/source/common/access_log/access_log_impl.h index 7e3aa51b3329..345ea5a4938c 100644 --- a/source/common/access_log/access_log_impl.h +++ b/source/common/access_log/access_log_impl.h @@ -178,7 +178,7 @@ class HeaderFilter : public Filter { const Http::HeaderMap& response_trailers) override; private: - std::vector header_data_; + const Http::HeaderUtility::HeaderDataPtr header_data_; }; /** diff --git a/source/common/common/BUILD b/source/common/common/BUILD index 6ace52dbb343..fbe3714e6374 100644 --- a/source/common/common/BUILD +++ b/source/common/common/BUILD @@ -165,6 +165,8 @@ envoy_cc_library( external_deps = ["abseil_optional"], deps = [ ":utility_lib", + "//include/envoy/common:matchers_interface", + "//source/common/common:regex_lib", "//source/common/config:metadata_lib", "//source/common/protobuf", "@envoy_api//envoy/type/matcher:metadata_cc", @@ -174,6 +176,19 @@ envoy_cc_library( ], ) +envoy_cc_library( + name = "regex_lib", + srcs = ["regex.cc"], + hdrs = ["regex.h"], + deps = [ + ":assert_lib", + "//include/envoy/common:regex_interface", + "//source/common/protobuf:utility_lib", + "@com_googlesource_code_re2//:re2", + "@envoy_api//envoy/type/matcher:regex_cc", + ], +) + envoy_cc_library( name = "non_copyable", hdrs = ["non_copyable.h"], diff --git a/source/common/common/matchers.cc b/source/common/common/matchers.cc index 2fb5f4b918b5..4c648ebe8eb6 100644 --- a/source/common/common/matchers.cc +++ b/source/common/common/matchers.cc @@ -2,6 +2,7 @@ #include "envoy/api/v2/core/base.pb.h" +#include "common/common/regex.h" #include "common/config/metadata.h" #include "absl/strings/match.h" @@ -16,7 +17,7 @@ ValueMatcherConstSharedPtr ValueMatcher::create(const envoy::type::matcher::Valu case envoy::type::matcher::ValueMatcher::kDoubleMatch: return std::make_shared(v.double_match()); case envoy::type::matcher::ValueMatcher::kStringMatch: - return std::make_shared(v.string_match()); + return std::make_shared(v.string_match()); case envoy::type::matcher::ValueMatcher::kBoolMatch: return std::make_shared(v.bool_match()); case envoy::type::matcher::ValueMatcher::kPresentMatch: @@ -56,7 +57,16 @@ bool DoubleMatcher::match(const ProtobufWkt::Value& value) const { }; } -bool StringMatcher::match(const ProtobufWkt::Value& value) const { +StringMatcherImpl::StringMatcherImpl(const envoy::type::matcher::StringMatcher& matcher) + : matcher_(matcher) { + if (matcher.match_pattern_case() == envoy::type::matcher::StringMatcher::kRegex) { + regex_ = Regex::Utility::parseStdRegexAsCompiledMatcher(matcher_.regex()); + } else if (matcher.match_pattern_case() == envoy::type::matcher::StringMatcher::kSafeRegex) { + regex_ = Regex::Utility::parseRegex(matcher_.safe_regex()); + } +} + +bool StringMatcherImpl::match(const ProtobufWkt::Value& value) const { if (value.kind_case() != ProtobufWkt::Value::kStringValue) { return false; } @@ -64,7 +74,7 @@ bool StringMatcher::match(const ProtobufWkt::Value& value) const { return match(value.string_value()); } -bool StringMatcher::match(const absl::string_view value) const { +bool StringMatcherImpl::match(const absl::string_view value) const { switch (matcher_.match_pattern_case()) { case envoy::type::matcher::StringMatcher::kExact: return matcher_.exact() == value; @@ -73,7 +83,8 @@ bool StringMatcher::match(const absl::string_view value) const { case envoy::type::matcher::StringMatcher::kSuffix: return absl::EndsWith(value, matcher_.suffix()); case envoy::type::matcher::StringMatcher::kRegex: - return std::regex_match(value.begin(), value.end(), regex_); + case envoy::type::matcher::StringMatcher::kSafeRegex: + return regex_->match(value); default: NOT_REACHED_GCOVR_EXCL_LINE; } diff --git a/source/common/common/matchers.h b/source/common/common/matchers.h index 8305a3ee4b3f..bb89941f3d0e 100644 --- a/source/common/common/matchers.h +++ b/source/common/common/matchers.h @@ -3,6 +3,8 @@ #include #include "envoy/api/v2/core/base.pb.h" +#include "envoy/common/matchers.h" +#include "envoy/common/regex.h" #include "envoy/type/matcher/metadata.pb.h" #include "envoy/type/matcher/number.pb.h" #include "envoy/type/matcher/string.pb.h" @@ -70,21 +72,16 @@ class DoubleMatcher : public ValueMatcher { const envoy::type::matcher::DoubleMatcher matcher_; }; -class StringMatcher : public ValueMatcher { +class StringMatcherImpl : public ValueMatcher, public StringMatcher { public: - StringMatcher(const envoy::type::matcher::StringMatcher& matcher) : matcher_(matcher) { - if (matcher.match_pattern_case() == envoy::type::matcher::StringMatcher::kRegex) { - regex_ = RegexUtil::parseRegex(matcher_.regex()); - } - } - - bool match(const absl::string_view value) const; + explicit StringMatcherImpl(const envoy::type::matcher::StringMatcher& matcher); + bool match(const absl::string_view value) const override; bool match(const ProtobufWkt::Value& value) const override; private: const envoy::type::matcher::StringMatcher matcher_; - std::regex regex_; + Regex::CompiledMatcherPtr regex_; }; class LowerCaseStringMatcher : public ValueMatcher { @@ -100,9 +97,11 @@ class LowerCaseStringMatcher : public ValueMatcher { envoy::type::matcher::StringMatcher toLowerCase(const envoy::type::matcher::StringMatcher& matcher); - const StringMatcher matcher_; + const StringMatcherImpl matcher_; }; +using LowerCaseStringMatcherPtr = std::unique_ptr; + class ListMatcher : public ValueMatcher { public: ListMatcher(const envoy::type::matcher::ListMatcher& matcher); diff --git a/source/common/common/regex.cc b/source/common/common/regex.cc new file mode 100644 index 000000000000..b78beef7ce3a --- /dev/null +++ b/source/common/common/regex.cc @@ -0,0 +1,78 @@ +#include "common/common/regex.h" + +#include "envoy/common/exception.h" + +#include "common/common/assert.h" +#include "common/common/fmt.h" +#include "common/protobuf/utility.h" + +#include "re2/re2.h" + +namespace Envoy { +namespace Regex { +namespace { + +class CompiledStdMatcher : public CompiledMatcher { +public: + CompiledStdMatcher(std::regex&& regex) : regex_(std::move(regex)) {} + + // CompiledMatcher + bool match(absl::string_view value) const override { + return std::regex_match(value.begin(), value.end(), regex_); + } + +private: + const std::regex regex_; +}; + +class CompiledGoogleReMatcher : public CompiledMatcher { +public: + CompiledGoogleReMatcher(const envoy::type::matcher::RegexMatcher& config) + : regex_(config.regex(), re2::RE2::Quiet) { + if (!regex_.ok()) { + throw EnvoyException(regex_.error()); + } + + const uint32_t max_program_size = + PROTOBUF_GET_WRAPPED_OR_DEFAULT(config.google_re2(), max_program_size, 100); + if (static_cast(regex_.ProgramSize()) > max_program_size) { + throw EnvoyException(fmt::format("regex '{}' RE2 program size of {} > max program size of " + "{}. Increase configured max program size if necessary.", + config.regex(), regex_.ProgramSize(), max_program_size)); + } + } + + // CompiledMatcher + bool match(absl::string_view value) const override { + return re2::RE2::FullMatch(re2::StringPiece(value.data(), value.size()), regex_); + } + +private: + const re2::RE2 regex_; +}; + +} // namespace + +CompiledMatcherPtr Utility::parseRegex(const envoy::type::matcher::RegexMatcher& matcher) { + // Google Re is the only currently supported engine. + ASSERT(matcher.has_google_re2()); + return std::make_unique(matcher); +} + +CompiledMatcherPtr Utility::parseStdRegexAsCompiledMatcher(const std::string& regex, + std::regex::flag_type flags) { + return std::make_unique(parseStdRegex(regex, flags)); +} + +std::regex Utility::parseStdRegex(const std::string& regex, std::regex::flag_type flags) { + // TODO(zuercher): In the future, PGV (https://github.com/lyft/protoc-gen-validate) annotations + // may allow us to remove this in favor of direct validation of regular expressions. + try { + return std::regex(regex, flags); + } catch (const std::regex_error& e) { + throw EnvoyException(fmt::format("Invalid regex '{}': {}", regex, e.what())); + } +} + +} // namespace Regex +} // namespace Envoy diff --git a/source/common/common/regex.h b/source/common/common/regex.h new file mode 100644 index 000000000000..a88c4739addb --- /dev/null +++ b/source/common/common/regex.h @@ -0,0 +1,44 @@ +#pragma once + +#include +#include + +#include "envoy/common/regex.h" +#include "envoy/type/matcher/regex.pb.h" + +namespace Envoy { +namespace Regex { + +/** + * Utilities for constructing regular expressions. + */ +class Utility { +public: + /** + * Constructs a std::regex, converting any std::regex_error exception into an EnvoyException. + * @param regex std::string containing the regular expression to parse. + * @param flags std::regex::flag_type containing parser flags. Defaults to std::regex::optimize. + * @return std::regex constructed from regex and flags. + * @throw EnvoyException if the regex string is invalid. + */ + static std::regex parseStdRegex(const std::string& regex, + std::regex::flag_type flags = std::regex::optimize); + + /** + * Construct an std::regex compiled regex matcher. + * + * TODO(mattklein123): In general this is only currently used in deprecated code paths and can be + * removed once all of those code paths are removed. + */ + static CompiledMatcherPtr + parseStdRegexAsCompiledMatcher(const std::string& regex, + std::regex::flag_type flags = std::regex::optimize); + + /** + * Construct a compiled regex matcher from a match config. + */ + static CompiledMatcherPtr parseRegex(const envoy::type::matcher::RegexMatcher& matcher); +}; + +} // namespace Regex +} // namespace Envoy diff --git a/source/common/common/utility.cc b/source/common/common/utility.cc index ed6483795814..3809da40e6d1 100644 --- a/source/common/common/utility.cc +++ b/source/common/common/utility.cc @@ -5,6 +5,7 @@ #include #include #include +#include #include #include "envoy/common/exception.h" @@ -499,16 +500,6 @@ uint32_t Primes::findPrimeLargerThan(uint32_t x) { return x; } -std::regex RegexUtil::parseRegex(const std::string& regex, std::regex::flag_type flags) { - // TODO(zuercher): In the future, PGV (https://github.com/lyft/protoc-gen-validate) annotations - // may allow us to remove this in favor of direct validation of regular expressions. - try { - return std::regex(regex, flags); - } catch (const std::regex_error& e) { - throw EnvoyException(fmt::format("Invalid regex '{}': {}", regex, e.what())); - } -} - // https://en.wikipedia.org/wiki/Algorithms_for_calculating_variance#Online_algorithm void WelfordStandardDeviation::update(double newValue) { ++count_; diff --git a/source/common/common/utility.h b/source/common/common/utility.h index 6ff000af46dc..48c09be97548 100644 --- a/source/common/common/utility.h +++ b/source/common/common/utility.h @@ -2,7 +2,6 @@ #include #include -#include #include #include #include @@ -388,22 +387,6 @@ class Primes { static uint32_t findPrimeLargerThan(uint32_t x); }; -/** - * Utilities for constructing regular expressions. - */ -class RegexUtil { -public: - /* - * Constructs a std::regex, converting any std::regex_error exception into an EnvoyException. - * @param regex std::string containing the regular expression to parse. - * @param flags std::regex::flag_type containing parser flags. Defaults to std::regex::optimize. - * @return std::regex constructed from regex and flags. - * @throw EnvoyException if the regex string is invalid. - */ - static std::regex parseRegex(const std::string& regex, - std::regex::flag_type flags = std::regex::optimize); -}; - /** * Utilities for working with weighted clusters. */ diff --git a/source/common/http/BUILD b/source/common/http/BUILD index 1dcf5045fa26..a41fdf2bb925 100644 --- a/source/common/http/BUILD +++ b/source/common/http/BUILD @@ -177,6 +177,7 @@ envoy_cc_library( "//source/common/common:empty_string", "//source/common/common:enum_to_int", "//source/common/common:linked_object", + "//source/common/common:regex_lib", "//source/common/common:scope_tracker", "//source/common/common:utility_lib", "//source/common/http/http1:codec_lib", diff --git a/source/common/http/header_utility.cc b/source/common/http/header_utility.cc index b6af8ab75f00..2c2f79ca7265 100644 --- a/source/common/http/header_utility.cc +++ b/source/common/http/header_utility.cc @@ -1,5 +1,6 @@ #include "common/http/header_utility.h" +#include "common/common/regex.h" #include "common/common/utility.h" #include "common/config/rds_json.h" #include "common/http/header_map_impl.h" @@ -32,7 +33,11 @@ HeaderUtility::HeaderData::HeaderData(const envoy::api::v2::route::HeaderMatcher break; case envoy::api::v2::route::HeaderMatcher::kRegexMatch: header_match_type_ = HeaderMatchType::Regex; - regex_pattern_ = RegexUtil::parseRegex(config.regex_match()); + regex_ = Regex::Utility::parseStdRegexAsCompiledMatcher(config.regex_match()); + break; + case envoy::api::v2::route::HeaderMatcher::kSafeRegexMatch: + header_match_type_ = HeaderMatchType::Regex; + regex_ = Regex::Utility::parseRegex(config.safe_regex_match()); break; case envoy::api::v2::route::HeaderMatcher::kRangeMatch: header_match_type_ = HeaderMatchType::Range; @@ -82,11 +87,11 @@ void HeaderUtility::getAllOfHeader(const Http::HeaderMap& headers, absl::string_ } bool HeaderUtility::matchHeaders(const Http::HeaderMap& request_headers, - const std::vector& config_headers) { + const std::vector& config_headers) { // No headers to match is considered a match. if (!config_headers.empty()) { - for (const HeaderData& cfg_header_data : config_headers) { - if (!matchHeaders(request_headers, cfg_header_data)) { + for (const HeaderDataPtr& cfg_header_data : config_headers) { + if (!matchHeaders(request_headers, *cfg_header_data)) { return false; } } @@ -110,7 +115,7 @@ bool HeaderUtility::matchHeaders(const Http::HeaderMap& request_headers, match = header_data.value_.empty() || header_view == header_data.value_; break; case HeaderMatchType::Regex: - match = std::regex_match(header_view.begin(), header_view.end(), header_data.regex_pattern_); + match = header_data.regex_->match(header_view); break; case HeaderMatchType::Range: { int64_t header_value = 0; diff --git a/source/common/http/header_utility.h b/source/common/http/header_utility.h index 7d3e13853059..a4f8c75639bd 100644 --- a/source/common/http/header_utility.h +++ b/source/common/http/header_utility.h @@ -1,13 +1,15 @@ #pragma once -#include #include #include "envoy/api/v2/route/route.pb.h" +#include "envoy/common/regex.h" #include "envoy/http/header_map.h" #include "envoy/json/json_object.h" #include "envoy/type/range.pb.h" +#include "common/protobuf/protobuf.h" + namespace Envoy { namespace Http { @@ -18,7 +20,8 @@ class HeaderUtility { public: enum class HeaderMatchType { Value, Regex, Range, Present, Prefix, Suffix }; - /* Get all instances of the header key specified, and return the values in the vector provided. + /** + * Get all instances of the header key specified, and return the values in the vector provided. * * This should not be used for inline headers, as it turns a constant time lookup into O(n). * @@ -26,7 +29,7 @@ class HeaderUtility { * @param key the header key to return values for * @param out the vector to return values in */ - static void getAllOfHeader(const Http::HeaderMap& headers, absl::string_view key, + static void getAllOfHeader(const HeaderMap& headers, absl::string_view key, std::vector& out); // A HeaderData specifies one of exact value or regex or range element @@ -36,14 +39,28 @@ class HeaderUtility { HeaderData(const envoy::api::v2::route::HeaderMatcher& config); HeaderData(const Json::Object& config); - const Http::LowerCaseString name_; + const LowerCaseString name_; HeaderMatchType header_match_type_; std::string value_; - std::regex regex_pattern_; + Regex::CompiledMatcherPtr regex_; envoy::type::Int64Range range_; const bool invert_match_; }; + using HeaderDataPtr = std::unique_ptr; + + /** + * Build a vector of HeaderData given input config. + */ + static std::vector buildHeaderDataVector( + const Protobuf::RepeatedPtrField& header_matchers) { + std::vector ret; + for (const auto& header_match : header_matchers) { + ret.emplace_back(std::make_unique(header_match)); + } + return ret; + } + /** * See if the headers specified in the config are present in a request. * @param request_headers supplies the headers from the request. @@ -51,10 +68,10 @@ class HeaderUtility { * @return bool true if all the headers (and values) in the config_headers are found in the * request_headers. If no config_headers are specified, returns true. */ - static bool matchHeaders(const Http::HeaderMap& request_headers, - const std::vector& config_headers); + static bool matchHeaders(const HeaderMap& request_headers, + const std::vector& config_headers); - static bool matchHeaders(const Http::HeaderMap& request_headers, const HeaderData& config_header); + static bool matchHeaders(const HeaderMap& request_headers, const HeaderData& config_header); /** * Validates that a header value is valid, according to RFC 7230, section 3.2. @@ -68,7 +85,7 @@ class HeaderUtility { * @param headers target where headers will be added * @param headers_to_add supplies the headers to be added */ - static void addHeaders(Http::HeaderMap& headers, const Http::HeaderMap& headers_to_add); + static void addHeaders(HeaderMap& headers, const HeaderMap& headers_to_add); }; } // namespace Http } // namespace Envoy diff --git a/source/common/router/config_impl.cc b/source/common/router/config_impl.cc index 408689e95d80..7b9898ab7ecc 100644 --- a/source/common/router/config_impl.cc +++ b/source/common/router/config_impl.cc @@ -5,7 +5,6 @@ #include #include #include -#include #include #include @@ -20,6 +19,7 @@ #include "common/common/fmt.h" #include "common/common/hash.h" #include "common/common/logger.h" +#include "common/common/regex.h" #include "common/common/utility.h" #include "common/config/metadata.h" #include "common/config/rds_json.h" @@ -153,10 +153,17 @@ CorsPolicyImpl::CorsPolicyImpl(const envoy::api::v2::route::CorsPolicy& config, max_age_(config.max_age()), legacy_enabled_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, enabled, true)) { for (const auto& origin : config.allow_origin()) { - allow_origin_.push_back(origin); + envoy::type::matcher::StringMatcher matcher_config; + matcher_config.set_exact(origin); + allow_origins_.push_back(std::make_unique(matcher_config)); } for (const auto& regex : config.allow_origin_regex()) { - allow_origin_regex_.push_back(RegexUtil::parseRegex(regex)); + envoy::type::matcher::StringMatcher matcher_config; + matcher_config.set_regex(regex); + allow_origins_.push_back(std::make_unique(matcher_config)); + } + for (const auto& string_match : config.allow_origin_string_match()) { + allow_origins_.push_back(std::make_unique(string_match)); } if (config.has_allow_credentials()) { allow_credentials_ = PROTOBUF_GET_WRAPPED_REQUIRED(config, allow_credentials); @@ -392,6 +399,7 @@ RouteEntryImplBase::RouteEntryImplBase(const VirtualHostImpl& vhost, factory_context.messageValidationVisitor())), rate_limit_policy_(route.route().rate_limits()), shadow_policy_(route.route()), priority_(ConfigUtility::parsePriority(route.route().priority())), + config_headers_(Http::HeaderUtility::buildHeaderDataVector(route.match().headers())), total_cluster_weight_( PROTOBUF_GET_WRAPPED_OR_DEFAULT(route.route().weighted_clusters(), total_weight, 100UL)), request_headers_parser_(HeaderParser::configure(route.request_headers_to_add(), @@ -440,12 +448,9 @@ RouteEntryImplBase::RouteEntryImplBase(const VirtualHostImpl& vhost, } } - for (const auto& header_map : route.match().headers()) { - config_headers_.push_back(header_map); - } - for (const auto& query_parameter : route.match().query_parameters()) { - config_query_parameters_.push_back(query_parameter); + config_query_parameters_.push_back( + std::make_unique(query_parameter)); } if (!route.route().hash_policy().empty()) { @@ -557,7 +562,7 @@ RouteEntryImplBase::loadRuntimeData(const envoy::api::v2::route::RouteMatch& rou } void RouteEntryImplBase::finalizePathHeader(Http::HeaderMap& headers, - const std::string& matched_path, + absl::string_view matched_path, bool insert_envoy_original_path) const { const auto& rewrite = getPathRewrite(); if (rewrite.empty()) { @@ -898,32 +903,36 @@ RouteConstSharedPtr PathRouteEntryImpl::matches(const Http::HeaderMap& headers, RegexRouteEntryImpl::RegexRouteEntryImpl(const VirtualHostImpl& vhost, const envoy::api::v2::route::Route& route, Server::Configuration::FactoryContext& factory_context) - : RouteEntryImplBase(vhost, route, factory_context), - regex_(RegexUtil::parseRegex(route.match().regex())), regex_str_(route.match().regex()) {} + : RouteEntryImplBase(vhost, route, factory_context) { + if (route.match().path_specifier_case() == envoy::api::v2::route::RouteMatch::kRegex) { + regex_ = Regex::Utility::parseStdRegexAsCompiledMatcher(route.match().regex()); + regex_str_ = route.match().regex(); + } else { + ASSERT(route.match().path_specifier_case() == envoy::api::v2::route::RouteMatch::kSafeRegex); + regex_ = Regex::Utility::parseRegex(route.match().safe_regex()); + regex_str_ = route.match().safe_regex().regex(); + } +} -void RegexRouteEntryImpl::rewritePathHeader(Http::HeaderMap& headers, - bool insert_envoy_original_path) const { +absl::string_view RegexRouteEntryImpl::pathOnly(const Http::HeaderMap& headers) const { const Http::HeaderString& path = headers.Path()->value(); const absl::string_view query_string = Http::Utility::findQueryStringStart(path); const size_t path_string_length = path.size() - query_string.length(); + return path.getStringView().substr(0, path_string_length); +} + +void RegexRouteEntryImpl::rewritePathHeader(Http::HeaderMap& headers, + bool insert_envoy_original_path) const { // TODO(yuval-k): This ASSERT can happen if the path was changed by a filter without clearing the // route cache. We should consider if ASSERT-ing is the desired behavior in this case. - - const absl::string_view path_view = path.getStringView(); - ASSERT(std::regex_match(path_view.begin(), path_view.begin() + path_string_length, regex_)); - const std::string matched_path(path_view.begin(), path_view.begin() + path_string_length); - - finalizePathHeader(headers, matched_path, insert_envoy_original_path); + ASSERT(regex_->match(pathOnly(headers))); + finalizePathHeader(headers, pathOnly(headers), insert_envoy_original_path); } RouteConstSharedPtr RegexRouteEntryImpl::matches(const Http::HeaderMap& headers, uint64_t random_value) const { if (RouteEntryImplBase::matchRoute(headers, random_value)) { - const Http::HeaderString& path = headers.Path()->value(); - const absl::string_view query_string = Http::Utility::findQueryStringStart(path); - if (std::regex_match(path.getStringView().begin(), - path.getStringView().begin() + (path.size() - query_string.length()), - regex_)) { + if (regex_->match(pathOnly(headers))) { return clusterEntry(headers, random_value); } } @@ -969,19 +978,22 @@ VirtualHostImpl::VirtualHostImpl(const envoy::api::v2::route::VirtualHost& virtu } for (const auto& route : virtual_host.routes()) { - const bool has_prefix = - route.match().path_specifier_case() == envoy::api::v2::route::RouteMatch::kPrefix; - const bool has_path = - route.match().path_specifier_case() == envoy::api::v2::route::RouteMatch::kPath; - const bool has_regex = - route.match().path_specifier_case() == envoy::api::v2::route::RouteMatch::kRegex; - if (has_prefix) { + switch (route.match().path_specifier_case()) { + case envoy::api::v2::route::RouteMatch::kPrefix: { routes_.emplace_back(new PrefixRouteEntryImpl(*this, route, factory_context)); - } else if (has_path) { + break; + } + case envoy::api::v2::route::RouteMatch::kPath: { routes_.emplace_back(new PathRouteEntryImpl(*this, route, factory_context)); - } else { - ASSERT(has_regex); + break; + } + case envoy::api::v2::route::RouteMatch::kRegex: + case envoy::api::v2::route::RouteMatch::kSafeRegex: { routes_.emplace_back(new RegexRouteEntryImpl(*this, route, factory_context)); + break; + } + case envoy::api::v2::route::RouteMatch::PATH_SPECIFIER_NOT_SET: + NOT_REACHED_GCOVR_EXCL_LINE; } if (validate_clusters) { @@ -1006,10 +1018,27 @@ VirtualHostImpl::VirtualHostImpl(const envoy::api::v2::route::VirtualHost& virtu VirtualHostImpl::VirtualClusterEntry::VirtualClusterEntry( const envoy::api::v2::route::VirtualCluster& virtual_cluster, Stats::StatNamePool& pool) - : pattern_(RegexUtil::parseRegex(virtual_cluster.pattern())), - stat_name_(pool.add(virtual_cluster.name())) { + : stat_name_(pool.add(virtual_cluster.name())) { + if (virtual_cluster.pattern().empty() == virtual_cluster.headers().empty()) { + throw EnvoyException("virtual clusters must define either 'pattern' or 'headers'"); + } + + if (!virtual_cluster.pattern().empty()) { + envoy::api::v2::route::HeaderMatcher matcher_config; + matcher_config.set_name(Http::Headers::get().Path.get()); + matcher_config.set_regex_match(virtual_cluster.pattern()); + headers_.push_back(std::make_unique(matcher_config)); + } else { + ASSERT(!virtual_cluster.headers().empty()); + headers_ = Http::HeaderUtility::buildHeaderDataVector(virtual_cluster.headers()); + } + if (virtual_cluster.method() != envoy::api::v2::core::RequestMethod::METHOD_UNSPECIFIED) { - method_ = envoy::api::v2::core::RequestMethod_Name(virtual_cluster.method()); + envoy::api::v2::route::HeaderMatcher matcher_config; + matcher_config.set_name(Http::Headers::get().Method.get()); + matcher_config.set_exact_match( + envoy::api::v2::core::RequestMethod_Name(virtual_cluster.method())); + headers_.push_back(std::make_unique(matcher_config)); } } @@ -1154,11 +1183,7 @@ const std::shared_ptr VirtualHostImpl::SSL_REDIRECT_ROUT const VirtualCluster* VirtualHostImpl::virtualClusterFromEntries(const Http::HeaderMap& headers) const { for (const VirtualClusterEntry& entry : virtual_clusters_) { - bool method_matches = - !entry.method_ || headers.Method()->value().getStringView() == entry.method_.value(); - - absl::string_view path_view = headers.Path()->value().getStringView(); - if (method_matches && std::regex_match(path_view.begin(), path_view.end(), entry.pattern_)) { + if (Http::HeaderUtility::matchHeaders(headers, entry.headers_)) { return &entry; } } diff --git a/source/common/router/config_impl.h b/source/common/router/config_impl.h index 202ffe6bea6d..39a51ba67899 100644 --- a/source/common/router/config_impl.h +++ b/source/common/router/config_impl.h @@ -104,8 +104,9 @@ class CorsPolicyImpl : public CorsPolicy { CorsPolicyImpl(const envoy::api::v2::route::CorsPolicy& config, Runtime::Loader& loader); // Router::CorsPolicy - const std::list& allowOrigins() const override { return allow_origin_; }; - const std::list& allowOriginRegexes() const override { return allow_origin_regex_; } + const std::vector& allowOrigins() const override { + return allow_origins_; + }; const std::string& allowMethods() const override { return allow_methods_; }; const std::string& allowHeaders() const override { return allow_headers_; }; const std::string& exposeHeaders() const override { return expose_headers_; }; @@ -131,8 +132,7 @@ class CorsPolicyImpl : public CorsPolicy { private: const envoy::api::v2::route::CorsPolicy config_; Runtime::Loader& loader_; - std::list allow_origin_; - std::list allow_origin_regex_; + std::vector allow_origins_; const std::string allow_methods_; const std::string allow_headers_; const std::string expose_headers_; @@ -182,9 +182,8 @@ class VirtualHostImpl : public VirtualHost { // Router::VirtualCluster Stats::StatName statName() const override { return stat_name_; } - const std::regex pattern_; - absl::optional method_; const Stats::StatName stat_name_; + std::vector headers_; }; class CatchAllVirtualCluster : public VirtualCluster { @@ -480,7 +479,7 @@ class RouteEntryImplBase : public RouteEntry, return (isRedirect()) ? prefix_rewrite_redirect_ : prefix_rewrite_; } - void finalizePathHeader(Http::HeaderMap& headers, const std::string& matched_path, + void finalizePathHeader(Http::HeaderMap& headers, absl::string_view matched_path, bool insert_envoy_original_path) const; private: @@ -670,8 +669,8 @@ class RouteEntryImplBase : public RouteEntry, const RateLimitPolicyImpl rate_limit_policy_; const ShadowPolicyImpl shadow_policy_; const Upstream::ResourcePriority priority_; - std::vector config_headers_; - std::vector config_query_parameters_; + std::vector config_headers_; + std::vector config_query_parameters_; std::vector weighted_clusters_; UpgradeMap upgrade_map_; @@ -760,8 +759,10 @@ class RegexRouteEntryImpl : public RouteEntryImplBase { void rewritePathHeader(Http::HeaderMap& headers, bool insert_envoy_original_path) const override; private: - const std::regex regex_; - const std::string regex_str_; + absl::string_view pathOnly(const Http::HeaderMap& headers) const; + + Regex::CompiledMatcherPtr regex_; + std::string regex_str_; }; /** diff --git a/source/common/router/config_utility.cc b/source/common/router/config_utility.cc index c48dd794d5f4..09dc85db59cc 100644 --- a/source/common/router/config_utility.cc +++ b/source/common/router/config_utility.cc @@ -1,25 +1,59 @@ #include "common/router/config_utility.h" -#include #include #include #include "common/common/assert.h" +#include "common/common/regex.h" namespace Envoy { namespace Router { +namespace { + +absl::optional +maybeCreateStringMatcher(const envoy::api::v2::route::QueryParameterMatcher& config) { + switch (config.query_parameter_match_specifier_case()) { + case envoy::api::v2::route::QueryParameterMatcher::kStringMatch: { + return Matchers::StringMatcherImpl(config.string_match()); + } + case envoy::api::v2::route::QueryParameterMatcher::kPresentMatch: { + return absl::nullopt; + } + case envoy::api::v2::route::QueryParameterMatcher::QUERY_PARAMETER_MATCH_SPECIFIER_NOT_SET: { + if (config.value().empty()) { + // Present match. + return absl::nullopt; + } + + envoy::type::matcher::StringMatcher matcher_config; + if (PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, regex, false)) { + matcher_config.set_regex(config.value()); + } else { + matcher_config.set_exact(config.value()); + } + return Matchers::StringMatcherImpl(matcher_config); + } + } + + NOT_REACHED_GCOVR_EXCL_LINE; // Needed for gcc +} + +} // namespace + +ConfigUtility::QueryParameterMatcher::QueryParameterMatcher( + const envoy::api::v2::route::QueryParameterMatcher& config) + : name_(config.name()), matcher_(maybeCreateStringMatcher(config)) {} bool ConfigUtility::QueryParameterMatcher::matches( const Http::Utility::QueryParams& request_query_params) const { auto query_param = request_query_params.find(name_); if (query_param == request_query_params.end()) { return false; - } else if (is_regex_) { - return std::regex_match(query_param->second, regex_pattern_); - } else if (value_.length() == 0) { + } else if (!matcher_.has_value()) { + // Present match. return true; } else { - return (value_ == query_param->second); + return matcher_.value().match(query_param->second); } } @@ -37,9 +71,9 @@ ConfigUtility::parsePriority(const envoy::api::v2::core::RoutingPriority& priori bool ConfigUtility::matchQueryParams( const Http::Utility::QueryParams& query_params, - const std::vector& config_query_params) { + const std::vector& config_query_params) { for (const auto& config_query_param : config_query_params) { - if (!config_query_param.matches(query_params)) { + if (!config_query_param->matches(query_params)) { return false; } } diff --git a/source/common/router/config_utility.h b/source/common/router/config_utility.h index 4a753964059f..030e20ca4cd1 100644 --- a/source/common/router/config_utility.h +++ b/source/common/router/config_utility.h @@ -1,7 +1,6 @@ #pragma once #include -#include #include #include @@ -11,6 +10,7 @@ #include "envoy/upstream/resource_manager.h" #include "common/common/empty_string.h" +#include "common/common/matchers.h" #include "common/common/utility.h" #include "common/config/rds_json.h" #include "common/http/headers.h" @@ -32,10 +32,7 @@ class ConfigUtility { // equivalent of the QueryParameterMatcher proto in the RDS v2 API. class QueryParameterMatcher { public: - QueryParameterMatcher(const envoy::api::v2::route::QueryParameterMatcher& config) - : name_(config.name()), value_(config.value()), - is_regex_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, regex, false)), - regex_pattern_(is_regex_ ? RegexUtil::parseRegex(value_) : std::regex()) {} + QueryParameterMatcher(const envoy::api::v2::route::QueryParameterMatcher& config); /** * Check if the query parameters for a request contain a match for this @@ -47,11 +44,11 @@ class ConfigUtility { private: const std::string name_; - const std::string value_; - const bool is_regex_; - const std::regex regex_pattern_; + const absl::optional matcher_; }; + using QueryParameterMatcherPtr = std::unique_ptr; + /** * @return the resource priority parsed from proto. */ @@ -66,7 +63,7 @@ class ConfigUtility { * query_params */ static bool matchQueryParams(const Http::Utility::QueryParams& query_params, - const std::vector& config_query_params); + const std::vector& config_query_params); /** * Returns the redirect HTTP Status Code enum parsed from proto. diff --git a/source/common/router/router_ratelimit.cc b/source/common/router/router_ratelimit.cc index f8827836367f..28519851ea83 100644 --- a/source/common/router/router_ratelimit.cc +++ b/source/common/router/router_ratelimit.cc @@ -67,11 +67,8 @@ bool GenericKeyAction::populateDescriptor(const Router::RouteEntry&, HeaderValueMatchAction::HeaderValueMatchAction( const envoy::api::v2::route::RateLimit::Action::HeaderValueMatch& action) : descriptor_value_(action.descriptor_value()), - expect_match_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(action, expect_match, true)) { - for (const auto& header_matcher : action.headers()) { - action_headers_.push_back(header_matcher); - } -} + expect_match_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(action, expect_match, true)), + action_headers_(Http::HeaderUtility::buildHeaderDataVector(action.headers())) {} bool HeaderValueMatchAction::populateDescriptor(const Router::RouteEntry&, RateLimit::Descriptor& descriptor, diff --git a/source/common/router/router_ratelimit.h b/source/common/router/router_ratelimit.h index e15f493ddbfa..7f3a368ffbca 100644 --- a/source/common/router/router_ratelimit.h +++ b/source/common/router/router_ratelimit.h @@ -97,7 +97,7 @@ class HeaderValueMatchAction : public RateLimitAction { private: const std::string descriptor_value_; const bool expect_match_; - std::vector action_headers_; + const std::vector action_headers_; }; /* diff --git a/source/common/stats/BUILD b/source/common/stats/BUILD index 7bc419766cdc..ed5efa052b47 100644 --- a/source/common/stats/BUILD +++ b/source/common/stats/BUILD @@ -180,6 +180,7 @@ envoy_cc_library( deps = [ "//include/envoy/stats:stats_interface", "//source/common/common:perf_annotation_lib", + "//source/common/common:regex_lib", ], ) diff --git a/source/common/stats/stats_matcher_impl.cc b/source/common/stats/stats_matcher_impl.cc index e4f14872ae4d..bd73c46adf5c 100644 --- a/source/common/stats/stats_matcher_impl.cc +++ b/source/common/stats/stats_matcher_impl.cc @@ -19,14 +19,14 @@ StatsMatcherImpl::StatsMatcherImpl(const envoy::config::metrics::v2::StatsConfig case envoy::config::metrics::v2::StatsMatcher::kInclusionList: // If we have an inclusion list, we are being default-exclusive. for (const auto& stats_matcher : config.stats_matcher().inclusion_list().patterns()) { - matchers_.push_back(Matchers::StringMatcher(stats_matcher)); + matchers_.push_back(Matchers::StringMatcherImpl(stats_matcher)); } is_inclusive_ = false; break; case envoy::config::metrics::v2::StatsMatcher::kExclusionList: // If we have an exclusion list, we are being default-inclusive. for (const auto& stats_matcher : config.stats_matcher().exclusion_list().patterns()) { - matchers_.push_back(Matchers::StringMatcher(stats_matcher)); + matchers_.push_back(Matchers::StringMatcherImpl(stats_matcher)); } FALLTHRU; default: @@ -48,7 +48,7 @@ bool StatsMatcherImpl::rejects(const std::string& name) const { // This is an XNOR, which can be evaluated by checking for equality. return (is_inclusive_ == std::any_of(matchers_.begin(), matchers_.end(), - [&name](auto matcher) { return matcher.match(name); })); + [&name](auto& matcher) { return matcher.match(name); })); } } // namespace Stats diff --git a/source/common/stats/stats_matcher_impl.h b/source/common/stats/stats_matcher_impl.h index 3712a466d3f6..7fe65e7fc49e 100644 --- a/source/common/stats/stats_matcher_impl.h +++ b/source/common/stats/stats_matcher_impl.h @@ -33,7 +33,7 @@ class StatsMatcherImpl : public StatsMatcher { // StatsMatcherImpl::rejects() for much more detail. bool is_inclusive_{true}; - std::vector matchers_; + std::vector matchers_; }; } // namespace Stats diff --git a/source/common/stats/tag_extractor_impl.cc b/source/common/stats/tag_extractor_impl.cc index 7bb02e6f53e1..a56398895c25 100644 --- a/source/common/stats/tag_extractor_impl.cc +++ b/source/common/stats/tag_extractor_impl.cc @@ -5,8 +5,9 @@ #include "envoy/common/exception.h" +#include "common/common/fmt.h" #include "common/common/perf_annotation.h" -#include "common/common/utility.h" +#include "common/common/regex.h" #include "absl/strings/ascii.h" #include "absl/strings/match.h" @@ -25,7 +26,7 @@ bool regexStartsWithDot(absl::string_view regex) { TagExtractorImpl::TagExtractorImpl(const std::string& name, const std::string& regex, const std::string& substr) : name_(name), prefix_(std::string(extractRegexPrefix(regex))), substr_(substr), - regex_(RegexUtil::parseRegex(regex)) {} + regex_(Regex::Utility::parseStdRegex(regex)) {} std::string TagExtractorImpl::extractRegexPrefix(absl::string_view regex) { std::string prefix; diff --git a/source/extensions/common/tap/tap_matcher.cc b/source/extensions/common/tap/tap_matcher.cc index 1b0d53bfa614..66511e2f8856 100644 --- a/source/extensions/common/tap/tap_matcher.cc +++ b/source/extensions/common/tap/tap_matcher.cc @@ -111,11 +111,8 @@ void NotMatcher::updateLocalStatus(MatchStatusVector& statuses, HttpHeaderMatcherBase::HttpHeaderMatcherBase( const envoy::service::tap::v2alpha::HttpHeadersMatch& config, const std::vector& matchers) - : SimpleMatcher(matchers) { - for (const auto& header_match : config.headers()) { - headers_to_match_.emplace_back(header_match); - } -} + : SimpleMatcher(matchers), + headers_to_match_(Http::HeaderUtility::buildHeaderDataVector(config.headers())) {} void HttpHeaderMatcherBase::matchHeaders(const Http::HeaderMap& headers, MatchStatusVector& statuses) const { diff --git a/source/extensions/common/tap/tap_matcher.h b/source/extensions/common/tap/tap_matcher.h index b80ff24256db..5f8e89c38bea 100644 --- a/source/extensions/common/tap/tap_matcher.h +++ b/source/extensions/common/tap/tap_matcher.h @@ -239,7 +239,7 @@ class HttpHeaderMatcherBase : public SimpleMatcher { protected: void matchHeaders(const Http::HeaderMap& headers, MatchStatusVector& statuses) const; - std::vector headers_to_match_; + const std::vector headers_to_match_; }; /** diff --git a/source/extensions/filters/common/ext_authz/ext_authz_http_impl.cc b/source/extensions/filters/common/ext_authz/ext_authz_http_impl.cc index aff72f7a533f..b40a927fdbfb 100644 --- a/source/extensions/filters/common/ext_authz/ext_authz_http_impl.cc +++ b/source/extensions/filters/common/ext_authz/ext_authz_http_impl.cc @@ -50,18 +50,28 @@ struct SuccessResponse { const MatcherSharedPtr& matchers_; ResponsePtr response_; }; + +std::vector +createLowerCaseMatchers(const envoy::type::matcher::ListStringMatcher& list) { + std::vector matchers; + for (const auto& matcher : list.patterns()) { + matchers.push_back(std::make_unique(matcher)); + } + return matchers; +} + } // namespace // Matchers -HeaderKeyMatcher::HeaderKeyMatcher(std::vector&& list) +HeaderKeyMatcher::HeaderKeyMatcher(std::vector&& list) : matchers_(std::move(list)) {} bool HeaderKeyMatcher::matches(absl::string_view key) const { return std::any_of(matchers_.begin(), matchers_.end(), - [&key](auto matcher) { return matcher.match(key); }); + [&key](auto& matcher) { return matcher->match(key); }); } -NotHeaderKeyMatcher::NotHeaderKeyMatcher(std::vector&& list) +NotHeaderKeyMatcher::NotHeaderKeyMatcher(std::vector&& list) : matcher_(std::move(list)) {} bool NotHeaderKeyMatcher::matches(absl::string_view key) const { return !matcher_.matches(key); } @@ -86,12 +96,11 @@ ClientConfig::toRequestMatchers(const envoy::type::matcher::ListStringMatcher& l {Http::Headers::get().Authorization, Http::Headers::get().Method, Http::Headers::get().Path, Http::Headers::get().Host}}; - std::vector matchers{list.patterns().begin(), - list.patterns().end()}; + std::vector matchers(createLowerCaseMatchers(list)); for (const auto& key : keys) { envoy::type::matcher::StringMatcher matcher; matcher.set_exact(key.get()); - matchers.push_back(matcher); + matchers.push_back(std::make_unique(matcher)); } return std::make_shared(std::move(matchers)); @@ -99,15 +108,14 @@ ClientConfig::toRequestMatchers(const envoy::type::matcher::ListStringMatcher& l MatcherSharedPtr ClientConfig::toClientMatchers(const envoy::type::matcher::ListStringMatcher& list) { - std::vector matchers{list.patterns().begin(), - list.patterns().end()}; + std::vector matchers(createLowerCaseMatchers(list)); // If list is empty, all authorization response headers, except Host, should be added to // the client response. if (matchers.empty()) { envoy::type::matcher::StringMatcher matcher; matcher.set_exact(Http::Headers::get().Host.get()); - matchers.push_back(matcher); + matchers.push_back(std::make_unique(matcher)); return std::make_shared(std::move(matchers)); } @@ -121,7 +129,7 @@ ClientConfig::toClientMatchers(const envoy::type::matcher::ListStringMatcher& li for (const auto& key : keys) { envoy::type::matcher::StringMatcher matcher; matcher.set_exact(key.get()); - matchers.push_back(matcher); + matchers.push_back(std::make_unique(matcher)); } return std::make_shared(std::move(matchers)); @@ -129,9 +137,7 @@ ClientConfig::toClientMatchers(const envoy::type::matcher::ListStringMatcher& li MatcherSharedPtr ClientConfig::toUpstreamMatchers(const envoy::type::matcher::ListStringMatcher& list) { - std::vector matchers{list.patterns().begin(), - list.patterns().end()}; - return std::make_unique(std::move(matchers)); + return std::make_unique(createLowerCaseMatchers(list)); } Http::LowerCaseStrPairVector ClientConfig::toHeadersAdd( diff --git a/source/extensions/filters/common/ext_authz/ext_authz_http_impl.h b/source/extensions/filters/common/ext_authz/ext_authz_http_impl.h index a26d5e4532f4..6d4c57128a84 100644 --- a/source/extensions/filters/common/ext_authz/ext_authz_http_impl.h +++ b/source/extensions/filters/common/ext_authz/ext_authz_http_impl.h @@ -34,17 +34,17 @@ class Matcher { class HeaderKeyMatcher : public Matcher { public: - HeaderKeyMatcher(std::vector&& list); + HeaderKeyMatcher(std::vector&& list); bool matches(absl::string_view key) const override; private: - const std::vector matchers_; + const std::vector matchers_; }; class NotHeaderKeyMatcher : public Matcher { public: - NotHeaderKeyMatcher(std::vector&& list); + NotHeaderKeyMatcher(std::vector&& list); bool matches(absl::string_view key) const override; diff --git a/source/extensions/filters/common/rbac/matchers.h b/source/extensions/filters/common/rbac/matchers.h index 98f369d49e20..8b2ef2bf1076 100644 --- a/source/extensions/filters/common/rbac/matchers.h +++ b/source/extensions/filters/common/rbac/matchers.h @@ -166,14 +166,14 @@ class AuthenticatedMatcher : public Matcher { public: AuthenticatedMatcher(const envoy::config::rbac::v2::Principal_Authenticated& auth) : matcher_(auth.has_principal_name() - ? absl::make_optional(auth.principal_name()) + ? absl::make_optional(auth.principal_name()) : absl::nullopt) {} bool matches(const Network::Connection& connection, const Envoy::Http::HeaderMap& headers, const StreamInfo::StreamInfo&) const override; private: - const absl::optional matcher_; + const absl::optional matcher_; }; /** @@ -217,10 +217,10 @@ class MetadataMatcher : public Matcher { * Perform a match against the request server from the client's connection * request. This is typically TLS SNI. */ -class RequestedServerNameMatcher : public Matcher, Envoy::Matchers::StringMatcher { +class RequestedServerNameMatcher : public Matcher, Envoy::Matchers::StringMatcherImpl { public: RequestedServerNameMatcher(const envoy::type::matcher::StringMatcher& requested_server_name) - : Envoy::Matchers::StringMatcher(requested_server_name) {} + : Envoy::Matchers::StringMatcherImpl(requested_server_name) {} bool matches(const Network::Connection& connection, const Envoy::Http::HeaderMap& headers, const StreamInfo::StreamInfo&) const override; diff --git a/source/extensions/filters/http/cors/cors_filter.cc b/source/extensions/filters/http/cors/cors_filter.cc index 997beb5385fc..2bd26c8b8438 100644 --- a/source/extensions/filters/http/cors/cors_filter.cc +++ b/source/extensions/filters/http/cors/cors_filter.cc @@ -115,35 +115,19 @@ void CorsFilter::setDecoderFilterCallbacks(Http::StreamDecoderFilterCallbacks& c } bool CorsFilter::isOriginAllowed(const Http::HeaderString& origin) { - return isOriginAllowedString(origin) || isOriginAllowedRegex(origin); -} - -bool CorsFilter::isOriginAllowedString(const Http::HeaderString& origin) { - if (allowOrigins() == nullptr) { - return false; - } - for (const auto& o : *allowOrigins()) { - if (o == "*" || origin == o.c_str()) { - return true; - } - } - return false; -} - -bool CorsFilter::isOriginAllowedRegex(const Http::HeaderString& origin) { - if (allowOriginRegexes() == nullptr) { + const auto allow_origins = allowOrigins(); + if (allow_origins == nullptr) { return false; } - for (const auto& regex : *allowOriginRegexes()) { - const absl::string_view origin_view = origin.getStringView(); - if (std::regex_match(origin_view.begin(), origin_view.end(), regex)) { + for (const auto& allow_origin : *allow_origins) { + if (allow_origin->match("*") || allow_origin->match(origin.getStringView())) { return true; } } return false; } -const std::list* CorsFilter::allowOrigins() { +const std::vector* CorsFilter::allowOrigins() { for (const auto policy : policies_) { if (policy && !policy->allowOrigins().empty()) { return &policy->allowOrigins(); @@ -152,15 +136,6 @@ const std::list* CorsFilter::allowOrigins() { return nullptr; } -const std::list* CorsFilter::allowOriginRegexes() { - for (const auto policy : policies_) { - if (policy && !policy->allowOriginRegexes().empty()) { - return &policy->allowOriginRegexes(); - } - } - return nullptr; -} - const std::string& CorsFilter::allowMethods() { for (const auto policy : policies_) { if (policy && !policy->allowMethods().empty()) { diff --git a/source/extensions/filters/http/cors/cors_filter.h b/source/extensions/filters/http/cors/cors_filter.h index b70b382a2177..51b13efeeb28 100644 --- a/source/extensions/filters/http/cors/cors_filter.h +++ b/source/extensions/filters/http/cors/cors_filter.h @@ -82,8 +82,7 @@ class CorsFilter : public Http::StreamFilter { private: friend class CorsFilterTest; - const std::list* allowOrigins(); - const std::list* allowOriginRegexes(); + const std::vector* allowOrigins(); const std::string& allowMethods(); const std::string& allowHeaders(); const std::string& exposeHeaders(); @@ -92,8 +91,6 @@ class CorsFilter : public Http::StreamFilter { bool shadowEnabled(); bool enabled(); bool isOriginAllowed(const Http::HeaderString& origin); - bool isOriginAllowedString(const Http::HeaderString& origin); - bool isOriginAllowedRegex(const Http::HeaderString& origin); Http::StreamDecoderFilterCallbacks* decoder_callbacks_{}; Http::StreamEncoderFilterCallbacks* encoder_callbacks_{}; diff --git a/source/extensions/filters/http/csrf/csrf_filter.cc b/source/extensions/filters/http/csrf/csrf_filter.cc index 85edec966ce3..ee8db9074c1a 100644 --- a/source/extensions/filters/http/csrf/csrf_filter.cc +++ b/source/extensions/filters/http/csrf/csrf_filter.cc @@ -59,10 +59,9 @@ static CsrfStats generateStats(const std::string& prefix, Stats::Scope& scope) { return CsrfStats{ALL_CSRF_STATS(POOL_COUNTER_PREFIX(scope, final_prefix))}; } -static const CsrfPolicy -generatePolicy(const envoy::config::filter::http::csrf::v2::CsrfPolicy& policy, - Runtime::Loader& runtime) { - return CsrfPolicy(policy, runtime); +static CsrfPolicyPtr generatePolicy(const envoy::config::filter::http::csrf::v2::CsrfPolicy& policy, + Runtime::Loader& runtime) { + return std::make_unique(policy, runtime); } } // namespace @@ -128,7 +127,7 @@ bool CsrfFilter::isValid(const absl::string_view source_origin, Http::HeaderMap& } for (const auto& additional_origin : policy_->additional_origins()) { - if (additional_origin.match(source_origin)) { + if (additional_origin->match(source_origin)) { return true; } } diff --git a/source/extensions/filters/http/csrf/csrf_filter.h b/source/extensions/filters/http/csrf/csrf_filter.h index 57def213db6e..0a36058f73e7 100644 --- a/source/extensions/filters/http/csrf/csrf_filter.h +++ b/source/extensions/filters/http/csrf/csrf_filter.h @@ -17,12 +17,10 @@ namespace Csrf { /** * All CSRF filter stats. @see stats_macros.h */ -// clang-format off -#define ALL_CSRF_STATS(COUNTER) \ - COUNTER(missing_source_origin)\ - COUNTER(request_invalid) \ - COUNTER(request_valid) \ -// clang-format on +#define ALL_CSRF_STATS(COUNTER) \ + COUNTER(missing_source_origin) \ + COUNTER(request_invalid) \ + COUNTER(request_valid) /** * Struct definition for CSRF stats. @see stats_macros.h @@ -37,9 +35,11 @@ struct CsrfStats { class CsrfPolicy : public Router::RouteSpecificFilterConfig { public: CsrfPolicy(const envoy::config::filter::http::csrf::v2::CsrfPolicy& policy, - Runtime::Loader& runtime) : policy_(policy), runtime_(runtime) { + Runtime::Loader& runtime) + : policy_(policy), runtime_(runtime) { for (const auto& additional_origin : policy.additional_origins()) { - additional_origins_.emplace_back(Matchers::StringMatcher(additional_origin)); + additional_origins_.emplace_back( + std::make_unique(additional_origin)); } } @@ -58,14 +58,16 @@ class CsrfPolicy : public Router::RouteSpecificFilterConfig { shadow_enabled.default_value()); } - const std::vector& additional_origins() const { return additional_origins_; }; + const std::vector& additional_origins() const { + return additional_origins_; + }; private: const envoy::config::filter::http::csrf::v2::CsrfPolicy policy_; - std::vector additional_origins_; + std::vector additional_origins_; Runtime::Loader& runtime_; - }; +using CsrfPolicyPtr = std::unique_ptr; /** * Configuration for the CSRF filter. @@ -73,15 +75,14 @@ class CsrfPolicy : public Router::RouteSpecificFilterConfig { class CsrfFilterConfig { public: CsrfFilterConfig(const envoy::config::filter::http::csrf::v2::CsrfPolicy& policy, - const std::string& stats_prefix, Stats::Scope& scope, - Runtime::Loader& runtime); + const std::string& stats_prefix, Stats::Scope& scope, Runtime::Loader& runtime); CsrfStats& stats() { return stats_; } - const CsrfPolicy* policy() { return &policy_; } + const CsrfPolicy* policy() { return policy_.get(); } private: CsrfStats stats_; - const CsrfPolicy policy_; + const CsrfPolicyPtr policy_; }; using CsrfFilterConfigSharedPtr = std::shared_ptr; diff --git a/source/extensions/filters/http/fault/fault_filter.cc b/source/extensions/filters/http/fault/fault_filter.cc index 05dccd35c891..0649366a7e5c 100644 --- a/source/extensions/filters/http/fault/fault_filter.cc +++ b/source/extensions/filters/http/fault/fault_filter.cc @@ -33,7 +33,8 @@ struct RcDetailsValues { using RcDetails = ConstSingleton; FaultSettings::FaultSettings(const envoy::config::filter::http::fault::v2::HTTPFault& fault) - : delay_percent_runtime_(PROTOBUF_GET_STRING_OR_DEFAULT(fault, delay_percent_runtime, + : fault_filter_headers_(Http::HeaderUtility::buildHeaderDataVector(fault.headers())), + delay_percent_runtime_(PROTOBUF_GET_STRING_OR_DEFAULT(fault, delay_percent_runtime, RuntimeKeys::get().DelayPercentKey)), abort_percent_runtime_(PROTOBUF_GET_STRING_OR_DEFAULT(fault, abort_percent_runtime, RuntimeKeys::get().AbortPercentKey)), @@ -57,10 +58,6 @@ FaultSettings::FaultSettings(const envoy::config::filter::http::fault::v2::HTTPF std::make_unique(fault.delay()); } - for (const Http::HeaderUtility::HeaderData& header_map : fault.headers()) { - fault_filter_headers_.push_back(header_map); - } - upstream_cluster_ = fault.upstream_cluster(); for (const auto& node : fault.downstream_nodes()) { diff --git a/source/extensions/filters/http/fault/fault_filter.h b/source/extensions/filters/http/fault/fault_filter.h index c4ae48fdcc58..be6ef435420b 100644 --- a/source/extensions/filters/http/fault/fault_filter.h +++ b/source/extensions/filters/http/fault/fault_filter.h @@ -48,7 +48,7 @@ class FaultSettings : public Router::RouteSpecificFilterConfig { public: FaultSettings(const envoy::config::filter::http::fault::v2::HTTPFault& fault); - const std::vector& filterHeaders() const { + const std::vector& filterHeaders() const { return fault_filter_headers_; } envoy::type::FractionalPercent abortPercentage() const { return abort_percentage_; } @@ -88,7 +88,7 @@ class FaultSettings : public Router::RouteSpecificFilterConfig { uint64_t http_status_{}; // HTTP or gRPC return codes Filters::Common::Fault::FaultDelayConfigPtr request_delay_config_; std::string upstream_cluster_; // restrict faults to specific upstream cluster - std::vector fault_filter_headers_; + const std::vector fault_filter_headers_; absl::flat_hash_set downstream_nodes_{}; // Inject failures for specific downstream absl::optional max_active_faults_; Filters::Common::Fault::FaultRateLimitConfigPtr response_rate_limit_; diff --git a/source/extensions/filters/http/health_check/config.cc b/source/extensions/filters/http/health_check/config.cc index 5db283e07272..6db2fa33efc5 100644 --- a/source/extensions/filters/http/health_check/config.cc +++ b/source/extensions/filters/http/health_check/config.cc @@ -21,12 +21,8 @@ Http::FilterFactoryCb HealthCheckFilterConfig::createFilterFactoryFromProtoTyped const bool pass_through_mode = proto_config.pass_through_mode().value(); const int64_t cache_time_ms = PROTOBUF_GET_MS_OR_DEFAULT(proto_config, cache_time, 0); - auto header_match_data = std::make_shared>(); - - for (const envoy::api::v2::route::HeaderMatcher& matcher : proto_config.headers()) { - Http::HeaderUtility::HeaderData single_header_match(matcher); - header_match_data->push_back(std::move(single_header_match)); - } + auto header_match_data = std::make_shared>(); + *header_match_data = Http::HeaderUtility::buildHeaderDataVector(proto_config.headers()); if (!pass_through_mode && cache_time_ms) { throw EnvoyException("cache_time_ms must not be set when path_through_mode is disabled"); diff --git a/source/extensions/filters/http/health_check/health_check.h b/source/extensions/filters/http/health_check/health_check.h index 5515e52e0435..c000e4136618 100644 --- a/source/extensions/filters/http/health_check/health_check.h +++ b/source/extensions/filters/http/health_check/health_check.h @@ -53,7 +53,7 @@ using ClusterMinHealthyPercentages = std::map; using ClusterMinHealthyPercentagesConstSharedPtr = std::shared_ptr; -using HeaderDataVectorSharedPtr = std::shared_ptr>; +using HeaderDataVectorSharedPtr = std::shared_ptr>; /** * Health check responder filter. diff --git a/source/extensions/filters/http/jwt_authn/matcher.cc b/source/extensions/filters/http/jwt_authn/matcher.cc index 53c61c2c5867..75753cdb42e1 100644 --- a/source/extensions/filters/http/jwt_authn/matcher.cc +++ b/source/extensions/filters/http/jwt_authn/matcher.cc @@ -1,6 +1,7 @@ #include "extensions/filters/http/jwt_authn/matcher.h" #include "common/common/logger.h" +#include "common/common/regex.h" #include "common/router/config_impl.h" #include "absl/strings/match.h" @@ -21,14 +22,11 @@ namespace { class BaseMatcherImpl : public Matcher, public Logger::Loggable { public: BaseMatcherImpl(const RequirementRule& rule) - : case_sensitive_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(rule.match(), case_sensitive, true)) { - - for (const auto& header_map : rule.match().headers()) { - config_headers_.push_back(header_map); - } - + : case_sensitive_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(rule.match(), case_sensitive, true)), + config_headers_(Http::HeaderUtility::buildHeaderDataVector(rule.match().headers())) { for (const auto& query_parameter : rule.match().query_parameters()) { - config_query_parameters_.push_back(query_parameter); + config_query_parameters_.push_back( + std::make_unique(query_parameter)); } } @@ -50,8 +48,8 @@ class BaseMatcherImpl : public Matcher, public Logger::Loggable const bool case_sensitive_; private: - std::vector config_headers_; - std::vector config_query_parameters_; + std::vector config_headers_; + std::vector config_query_parameters_; }; /** @@ -108,12 +106,20 @@ class PathMatcherImpl : public BaseMatcherImpl { /** * Perform a match against any path with a regex rule. + * TODO(mattklein123): This code needs dedup with RegexRouteEntryImpl. */ class RegexMatcherImpl : public BaseMatcherImpl { public: - RegexMatcherImpl(const RequirementRule& rule) - : BaseMatcherImpl(rule), regex_(RegexUtil::parseRegex(rule.match().regex())), - regex_str_(rule.match().regex()) {} + RegexMatcherImpl(const RequirementRule& rule) : BaseMatcherImpl(rule) { + if (rule.match().path_specifier_case() == envoy::api::v2::route::RouteMatch::kRegex) { + regex_ = Regex::Utility::parseStdRegexAsCompiledMatcher(rule.match().regex()); + regex_str_ = rule.match().regex(); + } else { + ASSERT(rule.match().path_specifier_case() == envoy::api::v2::route::RouteMatch::kSafeRegex); + regex_ = Regex::Utility::parseRegex(rule.match().safe_regex()); + regex_str_ = rule.match().safe_regex().regex(); + } + } bool matches(const Http::HeaderMap& headers) const override { if (BaseMatcherImpl::matchRoute(headers)) { @@ -121,7 +127,7 @@ class RegexMatcherImpl : public BaseMatcherImpl { const absl::string_view query_string = Http::Utility::findQueryStringStart(path); absl::string_view path_view = path.getStringView(); path_view.remove_suffix(query_string.length()); - if (std::regex_match(path_view.begin(), path_view.end(), regex_)) { + if (regex_->match(path_view)) { ENVOY_LOG(debug, "Regex requirement '{}' matched.", regex_str_); return true; } @@ -130,10 +136,9 @@ class RegexMatcherImpl : public BaseMatcherImpl { } private: - // regex object - const std::regex regex_; + Regex::CompiledMatcherPtr regex_; // raw regex string, for logging. - const std::string regex_str_; + std::string regex_str_; }; } // namespace @@ -145,6 +150,7 @@ MatcherConstPtr Matcher::create(const RequirementRule& rule) { case RouteMatch::PathSpecifierCase::kPath: return std::make_unique(rule); case RouteMatch::PathSpecifierCase::kRegex: + case RouteMatch::PathSpecifierCase::kSafeRegex: return std::make_unique(rule); // path specifier is required. case RouteMatch::PathSpecifierCase::PATH_SPECIFIER_NOT_SET: diff --git a/source/extensions/filters/http/squash/squash_filter.h b/source/extensions/filters/http/squash/squash_filter.h index 0e35c3dd5245..08f63aa4b291 100644 --- a/source/extensions/filters/http/squash/squash_filter.h +++ b/source/extensions/filters/http/squash/squash_filter.h @@ -1,5 +1,7 @@ #pragma once +#include + #include "envoy/config/filter/http/squash/v2/squash.pb.h" #include "envoy/http/async_client.h" #include "envoy/http/filter.h" diff --git a/source/extensions/filters/network/dubbo_proxy/router/route_matcher.cc b/source/extensions/filters/network/dubbo_proxy/router/route_matcher.cc index 81eac5f0f396..35850cfee78b 100644 --- a/source/extensions/filters/network/dubbo_proxy/router/route_matcher.cc +++ b/source/extensions/filters/network/dubbo_proxy/router/route_matcher.cc @@ -15,11 +15,8 @@ namespace Router { RouteEntryImplBase::RouteEntryImplBase( const envoy::config::filter::network::dubbo_proxy::v2alpha1::Route& route) - : cluster_name_(route.route().cluster()) { - for (const auto& header_map : route.match().headers()) { - config_headers_.emplace_back(header_map); - } - + : cluster_name_(route.route().cluster()), + config_headers_(Http::HeaderUtility::buildHeaderDataVector(route.match().headers())) { if (route.route().cluster_specifier_case() == envoy::config::filter::network::dubbo_proxy::v2alpha1::RouteAction::kWeightedClusters) { total_cluster_weight_ = 0UL; diff --git a/source/extensions/filters/network/dubbo_proxy/router/route_matcher.h b/source/extensions/filters/network/dubbo_proxy/router/route_matcher.h index caa8b9ca3112..9ce491f0aec5 100644 --- a/source/extensions/filters/network/dubbo_proxy/router/route_matcher.h +++ b/source/extensions/filters/network/dubbo_proxy/router/route_matcher.h @@ -77,7 +77,7 @@ class RouteEntryImplBase : public RouteEntry, uint64_t total_cluster_weight_; const std::string cluster_name_; - std::vector config_headers_; + const std::vector config_headers_; std::vector weighted_clusters_; // TODO(gengleilei) Implement it. @@ -123,7 +123,7 @@ class MethodRouteEntryImpl : public RouteEntryImplBase { uint64_t random_value) const override; private: - const Matchers::StringMatcher method_name_; + const Matchers::StringMatcherImpl method_name_; std::shared_ptr parameter_route_; }; diff --git a/source/extensions/filters/network/thrift_proxy/router/router_impl.cc b/source/extensions/filters/network/thrift_proxy/router/router_impl.cc index 1fc44b5f78ae..81a9b015ee65 100644 --- a/source/extensions/filters/network/thrift_proxy/router/router_impl.cc +++ b/source/extensions/filters/network/thrift_proxy/router/router_impl.cc @@ -22,11 +22,9 @@ namespace Router { RouteEntryImplBase::RouteEntryImplBase( const envoy::config::filter::network::thrift_proxy::v2alpha1::Route& route) - : cluster_name_(route.route().cluster()), rate_limit_policy_(route.route().rate_limits()) { - for (const auto& header_map : route.match().headers()) { - config_headers_.push_back(header_map); - } - + : cluster_name_(route.route().cluster()), + config_headers_(Http::HeaderUtility::buildHeaderDataVector(route.match().headers())), + rate_limit_policy_(route.route().rate_limits()) { if (route.route().has_metadata_match()) { const auto filter_it = route.route().metadata_match().filter_metadata().find( Envoy::Config::MetadataFilters::get().ENVOY_LB); diff --git a/source/extensions/filters/network/thrift_proxy/router/router_impl.h b/source/extensions/filters/network/thrift_proxy/router/router_impl.h index de8bc9e0d470..e6db1bef68f0 100644 --- a/source/extensions/filters/network/thrift_proxy/router/router_impl.h +++ b/source/extensions/filters/network/thrift_proxy/router/router_impl.h @@ -83,7 +83,7 @@ class RouteEntryImplBase : public RouteEntry, using WeightedClusterEntrySharedPtr = std::shared_ptr; const std::string cluster_name_; - std::vector config_headers_; + const std::vector config_headers_; std::vector weighted_clusters_; uint64_t total_cluster_weight_; Envoy::Router::MetadataMatchCriteriaConstPtr metadata_match_criteria_; diff --git a/source/extensions/filters/network/thrift_proxy/router/router_ratelimit_impl.cc b/source/extensions/filters/network/thrift_proxy/router/router_ratelimit_impl.cc index c1f3f99f7cb2..a9b2b1196f16 100644 --- a/source/extensions/filters/network/thrift_proxy/router/router_ratelimit_impl.cc +++ b/source/extensions/filters/network/thrift_proxy/router/router_ratelimit_impl.cc @@ -70,11 +70,8 @@ bool GenericKeyAction::populateDescriptor(const RouteEntry&, RateLimit::Descript HeaderValueMatchAction::HeaderValueMatchAction( const envoy::api::v2::route::RateLimit::Action::HeaderValueMatch& action) : descriptor_value_(action.descriptor_value()), - expect_match_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(action, expect_match, true)) { - for (const auto& header_matcher : action.headers()) { - action_headers_.push_back(header_matcher); - } -} + expect_match_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(action, expect_match, true)), + action_headers_(Http::HeaderUtility::buildHeaderDataVector(action.headers())) {} bool HeaderValueMatchAction::populateDescriptor(const RouteEntry&, RateLimit::Descriptor& descriptor, diff --git a/source/extensions/filters/network/thrift_proxy/router/router_ratelimit_impl.h b/source/extensions/filters/network/thrift_proxy/router/router_ratelimit_impl.h index 4423e0633881..976456ec2069 100644 --- a/source/extensions/filters/network/thrift_proxy/router/router_ratelimit_impl.h +++ b/source/extensions/filters/network/thrift_proxy/router/router_ratelimit_impl.h @@ -104,7 +104,7 @@ class HeaderValueMatchAction : public RateLimitAction { private: const std::string descriptor_value_; const bool expect_match_; - std::vector action_headers_; + const std::vector action_headers_; }; /* diff --git a/test/common/common/BUILD b/test/common/common/BUILD index 4714f1ff5c72..dd9b768e0ad5 100644 --- a/test/common/common/BUILD +++ b/test/common/common/BUILD @@ -143,6 +143,15 @@ envoy_cc_test( ], ) +envoy_cc_test( + name = "regex_test", + srcs = ["regex_test.cc"], + deps = [ + "//source/common/common:regex_lib", + "//test/test_common:utility_lib", + ], +) + envoy_cc_test( name = "perf_annotation_test", srcs = ["perf_annotation_test.cc"], diff --git a/test/common/common/matchers_test.cc b/test/common/common/matchers_test.cc index d5b509a89082..3c0a64f4ca05 100644 --- a/test/common/common/matchers_test.cc +++ b/test/common/common/matchers_test.cc @@ -257,6 +257,15 @@ TEST(MetadataTest, MatchDoubleListValue) { metadataValue.Clear(); } +TEST(StringMatcher, SafeRegexValue) { + envoy::type::matcher::StringMatcher matcher; + matcher.mutable_safe_regex()->mutable_google_re2(); + matcher.mutable_safe_regex()->set_regex("foo.*"); + EXPECT_TRUE(Matchers::StringMatcherImpl(matcher).match("foo")); + EXPECT_TRUE(Matchers::StringMatcherImpl(matcher).match("foobar")); + EXPECT_FALSE(Matchers::StringMatcherImpl(matcher).match("bar")); +} + TEST(LowerCaseStringMatcher, MatchExactValue) { envoy::type::matcher::StringMatcher matcher; matcher.set_exact("Foo"); diff --git a/test/common/common/regex_test.cc b/test/common/common/regex_test.cc new file mode 100644 index 000000000000..b12454d7cdb0 --- /dev/null +++ b/test/common/common/regex_test.cc @@ -0,0 +1,61 @@ +#include "envoy/common/exception.h" + +#include "common/common/regex.h" + +#include "test/test_common/utility.h" + +#include "gtest/gtest.h" + +namespace Envoy { +namespace Regex { +namespace { + +TEST(Utility, ParseStdRegex) { + EXPECT_THROW_WITH_REGEX(Utility::parseStdRegex("(+invalid)"), EnvoyException, + "Invalid regex '\\(\\+invalid\\)': .+"); + + { + std::regex regex = Utility::parseStdRegex("x*"); + EXPECT_NE(0, regex.flags() & std::regex::optimize); + } + + { + std::regex regex = Utility::parseStdRegex("x*", std::regex::icase); + EXPECT_NE(0, regex.flags() & std::regex::icase); + EXPECT_EQ(0, regex.flags() & std::regex::optimize); + } +} + +TEST(Utility, ParseRegex) { + { + envoy::type::matcher::RegexMatcher matcher; + matcher.mutable_google_re2(); + matcher.set_regex("(+invalid)"); + EXPECT_THROW_WITH_MESSAGE(Utility::parseRegex(matcher), EnvoyException, + "no argument for repetition operator: +"); + } + + // Regression test for https://github.com/envoyproxy/envoy/issues/7728 + { + envoy::type::matcher::RegexMatcher matcher; + matcher.mutable_google_re2(); + matcher.set_regex("/asdf/.*"); + const auto compiled_matcher = Utility::parseRegex(matcher); + const std::string long_string = "/asdf/" + std::string(50 * 1024, 'a'); + EXPECT_TRUE(compiled_matcher->match(long_string)); + } + + // Verify max program size. + { + envoy::type::matcher::RegexMatcher matcher; + matcher.mutable_google_re2()->mutable_max_program_size()->set_value(1); + matcher.set_regex("/asdf/.*"); + EXPECT_THROW_WITH_MESSAGE(Utility::parseRegex(matcher), EnvoyException, + "regex '/asdf/.*' RE2 program size of 24 > max program size of 1. " + "Increase configured max program size if necessary."); + } +} + +} // namespace +} // namespace Regex +} // namespace Envoy diff --git a/test/common/common/utility_test.cc b/test/common/common/utility_test.cc index 3ad89010ac49..750973e79d7f 100644 --- a/test/common/common/utility_test.cc +++ b/test/common/common/utility_test.cc @@ -401,22 +401,6 @@ TEST(Primes, findPrimeLargerThan) { EXPECT_EQ(10007, Primes::findPrimeLargerThan(9991)); } -TEST(RegexUtil, parseRegex) { - EXPECT_THROW_WITH_REGEX(RegexUtil::parseRegex("(+invalid)"), EnvoyException, - "Invalid regex '\\(\\+invalid\\)': .+"); - - { - std::regex regex = RegexUtil::parseRegex("x*"); - EXPECT_NE(0, regex.flags() & std::regex::optimize); - } - - { - std::regex regex = RegexUtil::parseRegex("x*", std::regex::icase); - EXPECT_NE(0, regex.flags() & std::regex::icase); - EXPECT_EQ(0, regex.flags() & std::regex::optimize); - } -} - class WeightedClusterEntry { public: WeightedClusterEntry(const std::string name, const uint64_t weight) diff --git a/test/common/http/header_utility_test.cc b/test/common/http/header_utility_test.cc index 97f6b95b34b6..c61443feec2f 100644 --- a/test/common/http/header_utility_test.cc +++ b/test/common/http/header_utility_test.cc @@ -175,8 +175,9 @@ name: match-header regex_match: (a|b) )EOF"; - std::vector header_data; - header_data.push_back(HeaderUtility::HeaderData(parseHeaderMatcherFromYaml(yaml))); + std::vector header_data; + header_data.push_back( + std::make_unique(parseHeaderMatcherFromYaml(yaml))); EXPECT_FALSE(HeaderUtility::matchHeaders(headers, header_data)); headers.addCopy("match-header", "a"); @@ -202,9 +203,11 @@ name: match-header-A name: match-header-B )EOF"; - std::vector header_data; - header_data.push_back(HeaderUtility::HeaderData(parseHeaderMatcherFromYaml(yamlA))); - header_data.push_back(HeaderUtility::HeaderData(parseHeaderMatcherFromYaml(yamlB))); + std::vector header_data; + header_data.push_back( + std::make_unique(parseHeaderMatcherFromYaml(yamlA))); + header_data.push_back( + std::make_unique(parseHeaderMatcherFromYaml(yamlB))); EXPECT_TRUE(HeaderUtility::matchHeaders(matching_headers_1, header_data)); EXPECT_TRUE(HeaderUtility::matchHeaders(matching_headers_2, header_data)); EXPECT_FALSE(HeaderUtility::matchHeaders(unmatching_headers_1, header_data)); @@ -220,8 +223,9 @@ TEST(MatchHeadersTest, HeaderPresence) { name: match-header )EOF"; - std::vector header_data; - header_data.push_back(HeaderUtility::HeaderData(parseHeaderMatcherFromYaml(yaml))); + std::vector header_data; + header_data.push_back( + std::make_unique(parseHeaderMatcherFromYaml(yaml))); EXPECT_TRUE(HeaderUtility::matchHeaders(matching_headers, header_data)); EXPECT_FALSE(HeaderUtility::matchHeaders(unmatching_headers, header_data)); } @@ -235,8 +239,9 @@ name: match-header exact_match: match-value )EOF"; - std::vector header_data; - header_data.push_back(HeaderUtility::HeaderData(parseHeaderMatcherFromYaml(yaml))); + std::vector header_data; + header_data.push_back( + std::make_unique(parseHeaderMatcherFromYaml(yaml))); EXPECT_TRUE(HeaderUtility::matchHeaders(matching_headers, header_data)); EXPECT_FALSE(HeaderUtility::matchHeaders(unmatching_headers, header_data)); } @@ -252,8 +257,9 @@ exact_match: match-value invert_match: true )EOF"; - std::vector header_data; - header_data.push_back(HeaderUtility::HeaderData(parseHeaderMatcherFromYaml(yaml))); + std::vector header_data; + header_data.push_back( + std::make_unique(parseHeaderMatcherFromYaml(yaml))); EXPECT_TRUE(HeaderUtility::matchHeaders(matching_headers, header_data)); EXPECT_FALSE(HeaderUtility::matchHeaders(unmatching_headers, header_data)); } @@ -266,8 +272,26 @@ name: match-header regex_match: \d{3} )EOF"; - std::vector header_data; - header_data.push_back(HeaderUtility::HeaderData(parseHeaderMatcherFromYaml(yaml))); + std::vector header_data; + header_data.push_back( + std::make_unique(parseHeaderMatcherFromYaml(yaml))); + EXPECT_TRUE(HeaderUtility::matchHeaders(matching_headers, header_data)); + EXPECT_FALSE(HeaderUtility::matchHeaders(unmatching_headers, header_data)); +} + +TEST(MatchHeadersTest, HeaderSafeRegexMatch) { + TestHeaderMapImpl matching_headers{{"match-header", "123"}}; + TestHeaderMapImpl unmatching_headers{{"match-header", "1234"}, {"match-header", "123.456"}}; + const std::string yaml = R"EOF( +name: match-header +safe_regex_match: + google_re2: {} + regex: \d{3} + )EOF"; + + std::vector header_data; + header_data.push_back( + std::make_unique(parseHeaderMatcherFromYaml(yaml))); EXPECT_TRUE(HeaderUtility::matchHeaders(matching_headers, header_data)); EXPECT_FALSE(HeaderUtility::matchHeaders(unmatching_headers, header_data)); } @@ -282,8 +306,9 @@ regex_match: \d{3} invert_match: true )EOF"; - std::vector header_data; - header_data.push_back(HeaderUtility::HeaderData(parseHeaderMatcherFromYaml(yaml))); + std::vector header_data; + header_data.push_back( + std::make_unique(parseHeaderMatcherFromYaml(yaml))); EXPECT_TRUE(HeaderUtility::matchHeaders(matching_headers, header_data)); EXPECT_FALSE(HeaderUtility::matchHeaders(unmatching_headers, header_data)); } @@ -301,8 +326,9 @@ name: match-header end: 0 )EOF"; - std::vector header_data; - header_data.push_back(HeaderUtility::HeaderData(parseHeaderMatcherFromYaml(yaml))); + std::vector header_data; + header_data.push_back( + std::make_unique(parseHeaderMatcherFromYaml(yaml))); EXPECT_TRUE(HeaderUtility::matchHeaders(matching_headers, header_data)); EXPECT_FALSE(HeaderUtility::matchHeaders(unmatching_headers, header_data)); } @@ -322,8 +348,9 @@ name: match-header invert_match: true )EOF"; - std::vector header_data; - header_data.push_back(HeaderUtility::HeaderData(parseHeaderMatcherFromYaml(yaml))); + std::vector header_data; + header_data.push_back( + std::make_unique(parseHeaderMatcherFromYaml(yaml))); EXPECT_TRUE(HeaderUtility::matchHeaders(matching_headers, header_data)); EXPECT_FALSE(HeaderUtility::matchHeaders(unmatching_headers, header_data)); } @@ -338,8 +365,9 @@ name: match-header present_match: true )EOF"; - std::vector header_data; - header_data.push_back(HeaderUtility::HeaderData(parseHeaderMatcherFromYaml(yaml))); + std::vector header_data; + header_data.push_back( + std::make_unique(parseHeaderMatcherFromYaml(yaml))); EXPECT_TRUE(HeaderUtility::matchHeaders(matching_headers, header_data)); EXPECT_FALSE(HeaderUtility::matchHeaders(unmatching_headers, header_data)); } @@ -355,8 +383,9 @@ present_match: true invert_match: true )EOF"; - std::vector header_data; - header_data.push_back(HeaderUtility::HeaderData(parseHeaderMatcherFromYaml(yaml))); + std::vector header_data; + header_data.push_back( + std::make_unique(parseHeaderMatcherFromYaml(yaml))); EXPECT_TRUE(HeaderUtility::matchHeaders(matching_headers, header_data)); EXPECT_FALSE(HeaderUtility::matchHeaders(unmatching_headers, header_data)); } @@ -370,8 +399,9 @@ name: match-header prefix_match: value )EOF"; - std::vector header_data; - header_data.push_back(HeaderUtility::HeaderData(parseHeaderMatcherFromYaml(yaml))); + std::vector header_data; + header_data.push_back( + std::make_unique(parseHeaderMatcherFromYaml(yaml))); EXPECT_TRUE(HeaderUtility::matchHeaders(matching_headers, header_data)); EXPECT_FALSE(HeaderUtility::matchHeaders(unmatching_headers, header_data)); } @@ -386,8 +416,9 @@ prefix_match: value invert_match: true )EOF"; - std::vector header_data; - header_data.push_back(HeaderUtility::HeaderData(parseHeaderMatcherFromYaml(yaml))); + std::vector header_data; + header_data.push_back( + std::make_unique(parseHeaderMatcherFromYaml(yaml))); EXPECT_TRUE(HeaderUtility::matchHeaders(matching_headers, header_data)); EXPECT_FALSE(HeaderUtility::matchHeaders(unmatching_headers, header_data)); } @@ -401,8 +432,9 @@ name: match-header suffix_match: value )EOF"; - std::vector header_data; - header_data.push_back(HeaderUtility::HeaderData(parseHeaderMatcherFromYaml(yaml))); + std::vector header_data; + header_data.push_back( + std::make_unique(parseHeaderMatcherFromYaml(yaml))); EXPECT_TRUE(HeaderUtility::matchHeaders(matching_headers, header_data)); EXPECT_FALSE(HeaderUtility::matchHeaders(unmatching_headers, header_data)); } @@ -417,8 +449,9 @@ suffix_match: value invert_match: true )EOF"; - std::vector header_data; - header_data.push_back(HeaderUtility::HeaderData(parseHeaderMatcherFromYaml(yaml))); + std::vector header_data; + header_data.push_back( + std::make_unique(parseHeaderMatcherFromYaml(yaml))); EXPECT_TRUE(HeaderUtility::matchHeaders(matching_headers, header_data)); EXPECT_FALSE(HeaderUtility::matchHeaders(unmatching_headers, header_data)); } diff --git a/test/common/router/config_impl_test.cc b/test/common/router/config_impl_test.cc index ca9073d4b712..ebed75c45251 100644 --- a/test/common/router/config_impl_test.cc +++ b/test/common/router/config_impl_test.cc @@ -175,7 +175,9 @@ TEST_F(RouteMatcherTest, TestRoutes) { route: cluster: clock - match: - regex: "/baa+" + safe_regex: + google_re2: {} + regex: "/baa+" route: cluster: sheep - match: @@ -295,8 +297,13 @@ TEST_F(RouteMatcherTest, TestRoutes) { - pattern: "^/users/\\d+$" method: PUT name: update_user - - pattern: "^/users/\\d+/location$" - method: POST + - headers: + - name: ":path" + safe_regex_match: + google_re2: {} + regex: "^/users/\\d+/location$" + - name: ":method" + exact_match: POST name: ulu )EOF"; @@ -643,6 +650,26 @@ TEST_F(RouteMatcherTest, TestRoutesWithInvalidRegex) { EnvoyException, "Invalid regex '\\^/\\(\\+invalid\\)':"); } +// Virtual cluster that contains neither pattern nor regex. This must be checked while pattern is +// deprecated. +TEST_F(RouteMatcherTest, TestRoutesWithInvalidVirtualCluster) { + const std::string invalid_virtual_cluster = R"EOF( +virtual_hosts: + - name: regex + domains: ["*"] + routes: + - match: { prefix: "/" } + route: { cluster: "regex" } + virtual_clusters: + - name: "invalid" + )EOF"; + + EXPECT_THROW_WITH_REGEX(TestConfigImpl(parseRouteConfigurationFromV2Yaml(invalid_virtual_cluster), + factory_context_, true), + EnvoyException, + "virtual clusters must define either 'pattern' or 'headers'"); +} + // Validates behavior of request_headers_to_add at router, vhost, and route levels. TEST_F(RouteMatcherTest, TestAddRemoveRequestHeaders) { const std::string yaml = R"EOF( @@ -1325,6 +1352,21 @@ TEST_F(RouteMatcherTest, QueryParamMatchedRouting) { - name: debug route: cluster: local_service_with_valueless_query_parameter + - match: + prefix: "/" + query_parameters: + - name: debug2 + present_match: true + route: + cluster: local_service_with_present_match_query_parameter + - match: + prefix: "/" + query_parameters: + - name: debug3 + string_match: + exact: foo + route: + cluster: local_service_with_string_match_query_parameter - match: prefix: "/" route: @@ -1364,6 +1406,18 @@ TEST_F(RouteMatcherTest, QueryParamMatchedRouting) { config.route(headers, 0)->routeEntry()->clusterName()); } + { + Http::TestHeaderMapImpl headers = genHeaders("example.com", "/?debug2", "GET"); + EXPECT_EQ("local_service_with_present_match_query_parameter", + config.route(headers, 0)->routeEntry()->clusterName()); + } + + { + Http::TestHeaderMapImpl headers = genHeaders("example.com", "/?debug3=foo", "GET"); + EXPECT_EQ("local_service_with_string_match_query_parameter", + config.route(headers, 0)->routeEntry()->clusterName()); + } + { Http::TestHeaderMapImpl headers = genHeaders("example.com", "/?debug=2", "GET"); EXPECT_EQ("local_service_with_valueless_query_parameter", @@ -4217,6 +4271,12 @@ TEST_F(RoutePropertyTest, TestVHostCorsConfig) { domains: ["*"] cors: allow_origin: ["test-origin"] + allow_origin_regex: + - .*\.envoyproxy\.io + allow_origin_string_match: + - safe_regex: + google_re2: {} + regex: .*\.envoyproxy\.io allow_methods: "test-methods" allow_headers: "test-headers" expose_headers: "test-expose-headers" @@ -4258,7 +4318,7 @@ TEST_F(RoutePropertyTest, TestVHostCorsConfig) { EXPECT_EQ(cors_policy->enabled(), false); EXPECT_EQ(cors_policy->shadowEnabled(), true); - EXPECT_THAT(cors_policy->allowOrigins(), ElementsAreArray({"test-origin"})); + EXPECT_EQ(3, cors_policy->allowOrigins().size()); EXPECT_EQ(cors_policy->allowMethods(), "test-methods"); EXPECT_EQ(cors_policy->allowHeaders(), "test-headers"); EXPECT_EQ(cors_policy->exposeHeaders(), "test-expose-headers"); @@ -4311,7 +4371,7 @@ TEST_F(RoutePropertyTest, TestRouteCorsConfig) { EXPECT_EQ(cors_policy->enabled(), false); EXPECT_EQ(cors_policy->shadowEnabled(), true); - EXPECT_THAT(cors_policy->allowOrigins(), ElementsAreArray({"test-origin"})); + EXPECT_EQ(1, cors_policy->allowOrigins().size()); EXPECT_EQ(cors_policy->allowMethods(), "test-methods"); EXPECT_EQ(cors_policy->allowHeaders(), "test-headers"); EXPECT_EQ(cors_policy->exposeHeaders(), "test-expose-headers"); @@ -4350,7 +4410,7 @@ TEST_F(RoutePropertyTest, TestVHostCorsLegacyConfig) { EXPECT_EQ(cors_policy->enabled(), true); EXPECT_EQ(cors_policy->shadowEnabled(), false); - EXPECT_THAT(cors_policy->allowOrigins(), ElementsAreArray({"test-origin"})); + EXPECT_EQ(1, cors_policy->allowOrigins().size()); EXPECT_EQ(cors_policy->allowMethods(), "test-methods"); EXPECT_EQ(cors_policy->allowHeaders(), "test-headers"); EXPECT_EQ(cors_policy->exposeHeaders(), "test-expose-headers"); @@ -4386,7 +4446,7 @@ TEST_F(RoutePropertyTest, TestRouteCorsLegacyConfig) { EXPECT_EQ(cors_policy->enabled(), true); EXPECT_EQ(cors_policy->shadowEnabled(), false); - EXPECT_THAT(cors_policy->allowOrigins(), ElementsAreArray({"test-origin"})); + EXPECT_EQ(1, cors_policy->allowOrigins().size()); EXPECT_EQ(cors_policy->allowMethods(), "test-methods"); EXPECT_EQ(cors_policy->allowHeaders(), "test-headers"); EXPECT_EQ(cors_policy->exposeHeaders(), "test-expose-headers"); diff --git a/test/extensions/filters/http/cors/cors_filter_integration_test.cc b/test/extensions/filters/http/cors/cors_filter_integration_test.cc index 6c78c1892313..b313d6162497 100644 --- a/test/extensions/filters/http/cors/cors_filter_integration_test.cc +++ b/test/extensions/filters/http/cors/cors_filter_integration_test.cc @@ -54,6 +54,8 @@ class CorsFilterIntegrationTest : public testing::TestWithParamadd_routes(); route->mutable_match()->set_prefix("/cors-credentials-allowed"); route->mutable_route()->set_cluster("cluster_0"); @@ -67,7 +69,10 @@ class CorsFilterIntegrationTest : public testing::TestWithParammutable_match()->set_prefix("/cors-allow-origin-regex"); route->mutable_route()->set_cluster("cluster_0"); auto* cors = route->mutable_route()->mutable_cors(); - cors->add_allow_origin_regex(".*\\.envoyproxy\\.io"); + auto* safe_regex = + cors->mutable_allow_origin_string_match()->Add()->mutable_safe_regex(); + safe_regex->mutable_google_re2(); + safe_regex->set_regex(".*\\.envoyproxy\\.io"); } { @@ -115,7 +120,7 @@ INSTANTIATE_TEST_SUITE_P(IpVersions, CorsFilterIntegrationTest, testing::ValuesIn(TestEnvironment::getIpVersionsForTest()), TestUtility::ipTestParamsToString); -TEST_P(CorsFilterIntegrationTest, TestVHostConfigSuccess) { +TEST_P(CorsFilterIntegrationTest, DEPRECATED_FEATURE_TEST(TestVHostConfigSuccess)) { testPreflight( Http::TestHeaderMapImpl{ {":method", "OPTIONS"}, @@ -135,7 +140,7 @@ TEST_P(CorsFilterIntegrationTest, TestVHostConfigSuccess) { }); } -TEST_P(CorsFilterIntegrationTest, TestRouteConfigSuccess) { +TEST_P(CorsFilterIntegrationTest, DEPRECATED_FEATURE_TEST(TestRouteConfigSuccess)) { testPreflight( Http::TestHeaderMapImpl{ {":method", "OPTIONS"}, @@ -156,7 +161,7 @@ TEST_P(CorsFilterIntegrationTest, TestRouteConfigSuccess) { }); } -TEST_P(CorsFilterIntegrationTest, TestRouteConfigBadOrigin) { +TEST_P(CorsFilterIntegrationTest, DEPRECATED_FEATURE_TEST(TestRouteConfigBadOrigin)) { testNormalRequest( Http::TestHeaderMapImpl{ {":method", "OPTIONS"}, @@ -173,7 +178,7 @@ TEST_P(CorsFilterIntegrationTest, TestRouteConfigBadOrigin) { }); } -TEST_P(CorsFilterIntegrationTest, TestCorsDisabled) { +TEST_P(CorsFilterIntegrationTest, DEPRECATED_FEATURE_TEST(TestCorsDisabled)) { testNormalRequest( Http::TestHeaderMapImpl{ {":method", "OPTIONS"}, @@ -190,7 +195,7 @@ TEST_P(CorsFilterIntegrationTest, TestCorsDisabled) { }); } -TEST_P(CorsFilterIntegrationTest, TestEncodeHeaders) { +TEST_P(CorsFilterIntegrationTest, DEPRECATED_FEATURE_TEST(TestEncodeHeaders)) { testNormalRequest( Http::TestHeaderMapImpl{ {":method", "GET"}, @@ -207,7 +212,7 @@ TEST_P(CorsFilterIntegrationTest, TestEncodeHeaders) { }); } -TEST_P(CorsFilterIntegrationTest, TestEncodeHeadersCredentialsAllowed) { +TEST_P(CorsFilterIntegrationTest, DEPRECATED_FEATURE_TEST(TestEncodeHeadersCredentialsAllowed)) { testNormalRequest( Http::TestHeaderMapImpl{ {":method", "GET"}, @@ -225,7 +230,7 @@ TEST_P(CorsFilterIntegrationTest, TestEncodeHeadersCredentialsAllowed) { }); } -TEST_P(CorsFilterIntegrationTest, TestAllowedOriginRegex) { +TEST_P(CorsFilterIntegrationTest, DEPRECATED_FEATURE_TEST(TestAllowedOriginRegex)) { testNormalRequest( Http::TestHeaderMapImpl{ {":method", "GET"}, @@ -243,7 +248,7 @@ TEST_P(CorsFilterIntegrationTest, TestAllowedOriginRegex) { }); } -TEST_P(CorsFilterIntegrationTest, TestExposeHeaders) { +TEST_P(CorsFilterIntegrationTest, DEPRECATED_FEATURE_TEST(TestExposeHeaders)) { testNormalRequest( Http::TestHeaderMapImpl{ {":method", "GET"}, diff --git a/test/extensions/filters/http/cors/cors_filter_test.cc b/test/extensions/filters/http/cors/cors_filter_test.cc index d56cb8ac6a4a..863bef99a8ef 100644 --- a/test/extensions/filters/http/cors/cors_filter_test.cc +++ b/test/extensions/filters/http/cors/cors_filter_test.cc @@ -1,3 +1,4 @@ +#include "common/common/matchers.h" #include "common/http/header_map_impl.h" #include "extensions/filters/http/cors/cors_filter.h" @@ -23,6 +24,21 @@ namespace Envoy { namespace Extensions { namespace HttpFilters { namespace Cors { +namespace { + +Matchers::StringMatcherPtr makeExactStringMatcher(const std::string& exact_match) { + envoy::type::matcher::StringMatcher config; + config.set_exact(exact_match); + return std::make_unique(config); +} + +Matchers::StringMatcherPtr makeStdRegexStringMatcher(const std::string& regex) { + envoy::type::matcher::StringMatcher config; + config.set_regex(regex); + return std::make_unique(config); +} + +} // namespace class CorsFilterTest : public testing::Test { public: @@ -30,7 +46,7 @@ class CorsFilterTest : public testing::Test { cors_policy_ = std::make_unique(); cors_policy_->enabled_ = true; cors_policy_->shadow_enabled_ = false; - cors_policy_->allow_origin_.emplace_back("*"); + cors_policy_->allow_origins_.emplace_back(makeExactStringMatcher("*")); cors_policy_->allow_methods_ = "GET"; cors_policy_->allow_headers_ = "content-type"; cors_policy_->expose_headers_ = "content-type"; @@ -299,8 +315,8 @@ TEST_F(CorsFilterTest, OptionsRequestNotMatchingOrigin) { Http::TestHeaderMapImpl request_headers{ {":method", "OPTIONS"}, {"origin", "test-host"}, {"access-control-request-method", "GET"}}; - cors_policy_->allow_origin_.clear(); - cors_policy_->allow_origin_.emplace_back("localhost"); + cors_policy_->allow_origins_.clear(); + cors_policy_->allow_origins_.emplace_back(makeExactStringMatcher("localhost")); EXPECT_CALL(decoder_callbacks_, encodeHeaders_(_, false)).Times(0); EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_.decodeHeaders(request_headers, false)); @@ -319,7 +335,7 @@ TEST_F(CorsFilterTest, OptionsRequestEmptyOriginList) { Http::TestHeaderMapImpl request_headers{ {":method", "OPTIONS"}, {"origin", "test-host"}, {"access-control-request-method", "GET"}}; - cors_policy_->allow_origin_.clear(); + cors_policy_->allow_origins_.clear(); EXPECT_CALL(decoder_callbacks_, encodeHeaders_(_, false)).Times(0); EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_.decodeHeaders(request_headers, false)); @@ -339,8 +355,8 @@ TEST_F(CorsFilterTest, ValidOptionsRequestWithAllowCredentialsTrue) { {":method", "OPTIONS"}, {"origin", "localhost"}, {"access-control-request-method", "GET"}}; cors_policy_->allow_credentials_ = true; - cors_policy_->allow_origin_.clear(); - cors_policy_->allow_origin_.emplace_back("localhost"); + cors_policy_->allow_origins_.clear(); + cors_policy_->allow_origins_.emplace_back(makeExactStringMatcher("localhost")); Http::TestHeaderMapImpl response_headers{ {":status", "200"}, @@ -485,8 +501,8 @@ TEST_F(CorsFilterTest, EncodeWithAllowCredentialsFalse) { TEST_F(CorsFilterTest, EncodeWithNonMatchingOrigin) { Http::TestHeaderMapImpl request_headers{{"origin", "test-host"}}; - cors_policy_->allow_origin_.clear(); - cors_policy_->allow_origin_.emplace_back("localhost"); + cors_policy_->allow_origins_.clear(); + cors_policy_->allow_origins_.emplace_back(makeExactStringMatcher("localhost")); EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_.decodeHeaders(request_headers, false)); EXPECT_EQ(Http::FilterDataStatus::Continue, filter_.decodeData(data_, false)); @@ -647,8 +663,8 @@ TEST_F(CorsFilterTest, OptionsRequestMatchingOriginByRegex) { {"access-control-max-age", "0"}, }; - cors_policy_->allow_origin_.clear(); - cors_policy_->allow_origin_regex_.emplace_back(std::regex(".*")); + cors_policy_->allow_origins_.clear(); + cors_policy_->allow_origins_.emplace_back(makeStdRegexStringMatcher(".*")); EXPECT_CALL(decoder_callbacks_, encodeHeaders_(HeaderMapEqualRef(&response_headers), true)); @@ -670,8 +686,8 @@ TEST_F(CorsFilterTest, OptionsRequestNotMatchingOriginByRegex) { {"origin", "www.envoyproxy.com"}, {"access-control-request-method", "GET"}}; - cors_policy_->allow_origin_.clear(); - cors_policy_->allow_origin_regex_.emplace_back(std::regex(".*.envoyproxy.io")); + cors_policy_->allow_origins_.clear(); + cors_policy_->allow_origins_.emplace_back(makeStdRegexStringMatcher(".*.envoyproxy.io")); EXPECT_CALL(decoder_callbacks_, encodeHeaders_(_, false)).Times(0); EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_.decodeHeaders(request_headers, false)); diff --git a/test/extensions/filters/http/health_check/health_check_test.cc b/test/extensions/filters/http/health_check/health_check_test.cc index 34df211019b7..a6cfb78104ea 100644 --- a/test/extensions/filters/http/health_check/health_check_test.cc +++ b/test/extensions/filters/http/health_check/health_check_test.cc @@ -47,11 +47,11 @@ class HealthCheckFilterTest : public testing::Test { void prepareFilter( bool pass_through, ClusterMinHealthyPercentagesConstSharedPtr cluster_min_healthy_percentages = nullptr) { - header_data_ = std::make_shared>(); + header_data_ = std::make_shared>(); envoy::api::v2::route::HeaderMatcher matcher; matcher.set_name(":path"); matcher.set_exact_match("/healthcheck"); - header_data_->emplace_back(matcher); + header_data_->emplace_back(std::make_unique(matcher)); filter_ = std::make_unique(context_, pass_through, cache_manager_, header_data_, cluster_min_healthy_percentages); filter_->setDecoderFilterCallbacks(callbacks_); diff --git a/test/extensions/filters/http/jwt_authn/matcher_test.cc b/test/extensions/filters/http/jwt_authn/matcher_test.cc index 0ca3e6035089..360ac000f462 100644 --- a/test/extensions/filters/http/jwt_authn/matcher_test.cc +++ b/test/extensions/filters/http/jwt_authn/matcher_test.cc @@ -55,6 +55,28 @@ TEST_F(MatcherTest, TestMatchRegex) { EXPECT_FALSE(matcher->matches(headers)); } +TEST_F(MatcherTest, TestMatchSafeRegex) { + const char config[] = R"( +match: + safe_regex: + google_re2: {} + regex: "/[^c][au]t")"; + + RequirementRule rule; + TestUtility::loadFromYaml(config, rule); + MatcherConstPtr matcher = Matcher::create(rule); + auto headers = TestHeaderMapImpl{{":path", "/but"}}; + EXPECT_TRUE(matcher->matches(headers)); + headers = TestHeaderMapImpl{{":path", "/mat?ok=bye"}}; + EXPECT_TRUE(matcher->matches(headers)); + headers = TestHeaderMapImpl{{":path", "/maut"}}; + EXPECT_FALSE(matcher->matches(headers)); + headers = TestHeaderMapImpl{{":path", "/cut"}}; + EXPECT_FALSE(matcher->matches(headers)); + headers = TestHeaderMapImpl{{":path", "/mut/"}}; + EXPECT_FALSE(matcher->matches(headers)); +} + TEST_F(MatcherTest, TestMatchPath) { const char config[] = R"(match: path: "/match" diff --git a/test/extensions/filters/network/thrift_proxy/integration_test.cc b/test/extensions/filters/network/thrift_proxy/integration_test.cc index 0350244d29fb..ec6db8fd0856 100644 --- a/test/extensions/filters/network/thrift_proxy/integration_test.cc +++ b/test/extensions/filters/network/thrift_proxy/integration_test.cc @@ -39,7 +39,9 @@ class ThriftConnManagerIntegrationTest - name: "x-header-1" exact_match: "x-value-1" - name: "x-header-2" - regex_match: "0.[5-9]" + safe_regex_match: + google_re2: {} + regex: "0.[5-9]" - name: "x-header-3" range_match: start: 100 diff --git a/test/integration/http_integration.cc b/test/integration/http_integration.cc index fbd7308fd4a6..62d2eb45ba41 100644 --- a/test/integration/http_integration.cc +++ b/test/integration/http_integration.cc @@ -755,7 +755,7 @@ void HttpIntegrationTest::testEnvoyProxying100Continue(bool continue_before_upst auto* virtual_host = route_config->mutable_virtual_hosts(0); { auto* cors = virtual_host->mutable_cors(); - cors->add_allow_origin("*"); + cors->mutable_allow_origin_string_match()->Add()->set_exact("*"); cors->set_allow_headers("content-type,x-grpc-web"); cors->set_allow_methods("GET,POST"); } diff --git a/test/mocks/router/mocks.h b/test/mocks/router/mocks.h index 75249eeaad47..06e5875c032a 100644 --- a/test/mocks/router/mocks.h +++ b/test/mocks/router/mocks.h @@ -12,7 +12,6 @@ #include "envoy/config/config_provider.h" #include "envoy/config/typed_metadata.h" #include "envoy/event/dispatcher.h" -#include "envoy/json/json_object.h" #include "envoy/local_info/local_info.h" #include "envoy/router/rds.h" #include "envoy/router/route_config_provider_manager.h" @@ -54,8 +53,9 @@ class MockDirectResponseEntry : public DirectResponseEntry { class TestCorsPolicy : public CorsPolicy { public: // Router::CorsPolicy - const std::list& allowOrigins() const override { return allow_origin_; }; - const std::list& allowOriginRegexes() const override { return allow_origin_regex_; }; + const std::vector& allowOrigins() const override { + return allow_origins_; + }; const std::string& allowMethods() const override { return allow_methods_; }; const std::string& allowHeaders() const override { return allow_headers_; }; const std::string& exposeHeaders() const override { return expose_headers_; }; @@ -64,13 +64,12 @@ class TestCorsPolicy : public CorsPolicy { bool enabled() const override { return enabled_; }; bool shadowEnabled() const override { return shadow_enabled_; }; - std::list allow_origin_{}; - std::list allow_origin_regex_{}; - std::string allow_methods_{}; - std::string allow_headers_{}; - std::string expose_headers_{}; + std::vector allow_origins_; + std::string allow_methods_; + std::string allow_headers_; + std::string expose_headers_; std::string max_age_{}; - absl::optional allow_credentials_{}; + absl::optional allow_credentials_; bool enabled_{}; bool shadow_enabled_{}; }; diff --git a/test/test_common/utility.cc b/test/test_common/utility.cc index 88f627143894..8cf850b1a300 100644 --- a/test/test_common/utility.cc +++ b/test/test_common/utility.cc @@ -16,6 +16,7 @@ #include #include #include +#include #include #include #include diff --git a/tools/check_format.py b/tools/check_format.py index 40953acc8077..2adcc22e8c35 100755 --- a/tools/check_format.py +++ b/tools/check_format.py @@ -63,6 +63,16 @@ # Files in these paths can use Protobuf::util::JsonStringToMessage JSON_STRING_TO_MESSAGE_WHITELIST = ("./source/common/protobuf/utility.cc") +# Files in these paths can use std::regex +STD_REGEX_WHITELIST = ("./source/common/common/utility.cc", "./source/common/common/regex.h", + "./source/common/common/regex.cc", + "./source/common/stats/tag_extractor_impl.h", + "./source/common/stats/tag_extractor_impl.cc", + "./source/common/access_log/access_log_formatter.cc", + "./source/extensions/filters/http/squash/squash_filter.h", + "./source/extensions/filters/http/squash/squash_filter.cc", + "./source/server/http/admin.h", "./source/server/http/admin.cc") + CLANG_FORMAT_PATH = os.getenv("CLANG_FORMAT", "clang-format-8") BUILDIFIER_PATH = os.getenv("BUILDIFIER_BIN", "$GOPATH/bin/buildifier") ENVOY_BUILD_FIXER_PATH = os.path.join(os.path.dirname(os.path.abspath(sys.argv[0])), @@ -333,6 +343,10 @@ def whitelistedForStatFromString(file_path): return file_path in STAT_FROM_STRING_WHITELIST +def whitelistedForStdRegex(file_path): + return file_path.startswith("./test") or file_path in STD_REGEX_WHITELIST + + def findSubstringAndReturnError(pattern, file_path, error_message): with open(file_path) as f: text = f.read() @@ -581,6 +595,9 @@ def checkSourceLine(line, file_path, reportError): ('.counter(' in line or '.gauge(' in line or '.histogram(' in line): reportError("Don't lookup stats by name at runtime; use StatName saved during construction") + if not whitelistedForStdRegex(file_path) and "std::regex" in line: + reportError("Don't use std::regex in code that handles untrusted input. Use RegexMatcher") + def checkBuildLine(line, file_path, reportError): if "@bazel_tools" in line and not (isSkylarkFile(file_path) or file_path.startswith("./bazel/")): diff --git a/tools/check_format_test_helper.py b/tools/check_format_test_helper.py index 7309a4bcaff1..c41abbc60890 100755 --- a/tools/check_format_test_helper.py +++ b/tools/check_format_test_helper.py @@ -217,6 +217,8 @@ def checkFileExpectingOK(filename): errors += checkUnfixableError( "histogram_from_string.cc", "Don't lookup stats by name at runtime; use StatName saved during construction") + errors += checkUnfixableError( + "regex.cc", "Don't use std::regex in code that handles untrusted input. Use RegexMatcher") errors += fixFileExpectingFailure( "api/missing_package.proto", diff --git a/tools/protodoc/protodoc.py b/tools/protodoc/protodoc.py index 8835dce7e40a..bf6a869f0797 100755 --- a/tools/protodoc/protodoc.py +++ b/tools/protodoc/protodoc.py @@ -44,6 +44,10 @@ # field. NOT_IMPLEMENTED_HIDE_ANNOTATION = 'not-implemented-hide' +# Comment that allows for easy searching for things that need cleaning up in the next major +# API version. +NEXT_MAJOR_VERSION_ANNOTATION = 'next-major-version' + # Comment. Just used for adding text that will not go into the docs at all. COMMENT_ANNOTATION = 'comment' @@ -58,6 +62,7 @@ NOT_IMPLEMENTED_WARN_ANNOTATION, NOT_IMPLEMENTED_HIDE_ANNOTATION, V2_API_DIFF_ANNOTATION, + NEXT_MAJOR_VERSION_ANNOTATION, COMMENT_ANNOTATION, PROTO_STATUS_ANNOTATION, ]) diff --git a/tools/testdata/check_format/regex.cc b/tools/testdata/check_format/regex.cc new file mode 100644 index 000000000000..53241fae9cba --- /dev/null +++ b/tools/testdata/check_format/regex.cc @@ -0,0 +1,9 @@ +#include + +namespace Envoy { + +struct BadRegex { + std::regex bad_; +} + +} // namespace Envoy