diff --git a/CODEOWNERS b/CODEOWNERS index 9a7d0f06b94a..601f9c69755d 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -92,6 +92,7 @@ extensions/filters/common/original_src @snowp @klarose /*/extensions/filters/listener/tls_inspector @piotrsikora @htuch /*/extensions/grpc_credentials/example @wozz @htuch /*/extensions/grpc_credentials/file_based_metadata @wozz @htuch +/*/extensions/internal_redirect @alyssawilk @penguingao /*/extensions/stat_sinks/dog_statsd @taiki45 @jmarantz /*/extensions/stat_sinks/hystrix @trabetti @jmarantz /*/extensions/stat_sinks/metrics_service @ramaraochavali @jmarantz diff --git a/api/BUILD b/api/BUILD index f5bd8c1a8d0b..c701bdcf4833 100644 --- a/api/BUILD +++ b/api/BUILD @@ -222,6 +222,9 @@ proto_library( "//envoy/extensions/filters/network/thrift_proxy/v3:pkg", "//envoy/extensions/filters/network/zookeeper_proxy/v3:pkg", "//envoy/extensions/filters/udp/dns_filter/v3alpha:pkg", + "//envoy/extensions/internal_redirect/allow_listed_routes/v3:pkg", + "//envoy/extensions/internal_redirect/previous_routes/v3:pkg", + "//envoy/extensions/internal_redirect/safe_cross_scheme/v3:pkg", "//envoy/extensions/retry/host/omit_host_metadata/v3:pkg", "//envoy/extensions/retry/priority/previous_priorities/v3:pkg", "//envoy/extensions/transport_sockets/alts/v3:pkg", diff --git a/api/envoy/config/route/v3/route_components.proto b/api/envoy/config/route/v3/route_components.proto index 3f82fbd80fb0..cbf50ad16448 100644 --- a/api/envoy/config/route/v3/route_components.proto +++ b/api/envoy/config/route/v3/route_components.proto @@ -3,6 +3,7 @@ syntax = "proto3"; package envoy.config.route.v3; import "envoy/config/core/v3/base.proto"; +import "envoy/config/core/v3/extension.proto"; import "envoy/config/core/v3/proxy_protocol.proto"; import "envoy/type/matcher/v3/regex.proto"; import "envoy/type/matcher/v3/string.proto"; @@ -536,7 +537,7 @@ message CorsPolicy { core.v3.RuntimeFractionalPercent shadow_enabled = 10; } -// [#next-free-field: 34] +// [#next-free-field: 35] message RouteAction { option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.route.RouteAction"; @@ -549,6 +550,7 @@ message RouteAction { } // Configures :ref:`internal redirect ` behavior. + // [#next-major-version: remove this definition - it's defined in the InternalRedirectPolicy message.] enum InternalRedirectAction { PASS_THROUGH_INTERNAL_REDIRECT = 0; HANDLE_INTERNAL_REDIRECT = 1; @@ -986,7 +988,13 @@ message RouteAction { repeated UpgradeConfig upgrade_configs = 25; - InternalRedirectAction internal_redirect_action = 26; + // If present, Envoy will try to follow an upstream redirect response instead of proxying the + // response back to the downstream. An upstream redirect response is defined + // by :ref:`redirect_response_codes + // `. + InternalRedirectPolicy internal_redirect_policy = 34; + + InternalRedirectAction internal_redirect_action = 26 [deprecated = true]; // An internal redirect is handled, iff the number of previous internal redirects that a // downstream request has encountered is lower than this value, and @@ -1002,7 +1010,7 @@ message RouteAction { // will pass the redirect back to downstream. // // If not specified, at most one redirect will be followed. - google.protobuf.UInt32Value max_internal_redirects = 31; + google.protobuf.UInt32Value max_internal_redirects = 31 [deprecated = true]; // Indicates that the route has a hedge policy. Note that if this is set, // it'll take precedence over the virtual host level hedge policy entirely @@ -1593,3 +1601,30 @@ message QueryParameterMatcher { bool present_match = 6; } } + +// HTTP Internal Redirect :ref:`architecture overview `. +message InternalRedirectPolicy { + // An internal redirect is not handled, unless the number of previous internal redirects that a + // downstream request has encountered is lower than this value. + // In the case where a downstream request is bounced among multiple routes by internal redirect, + // the first route that hits this threshold, or does not set :ref:`internal_redirect_policy + // ` + // will pass the redirect back to downstream. + // + // If not specified, at most one redirect will be followed. + google.protobuf.UInt32Value max_internal_redirects = 1; + + // Defines what upstream response codes are allowed to trigger internal redirect. If unspecified, + // only 302 will be treated as internal redirect. + // Only 301, 302, 303, 307 and 308 are valid values. Any other codes will be ignored. + repeated uint32 redirect_response_codes = 2 [(validate.rules).repeated = {max_items: 5}]; + + // Specifies a list of predicates that are queried when an upstream response is deemed + // to trigger an internal redirect by all other criteria. Any predicate in the list can reject + // the redirect, causing the response to be proxied to downstream. + repeated core.v3.TypedExtensionConfig predicates = 3; + + // Allow internal redirect to follow a target URI with a different scheme than the value of + // x-forwarded-proto. The default is false. + bool allow_cross_scheme_redirect = 4; +} diff --git a/api/envoy/config/route/v4alpha/route_components.proto b/api/envoy/config/route/v4alpha/route_components.proto index 6e1b1f9f5a0a..bd111aaac6a8 100644 --- a/api/envoy/config/route/v4alpha/route_components.proto +++ b/api/envoy/config/route/v4alpha/route_components.proto @@ -3,6 +3,7 @@ syntax = "proto3"; package envoy.config.route.v4alpha; import "envoy/config/core/v4alpha/base.proto"; +import "envoy/config/core/v4alpha/extension.proto"; import "envoy/config/core/v4alpha/proxy_protocol.proto"; import "envoy/type/matcher/v4alpha/regex.proto"; import "envoy/type/matcher/v4alpha/string.proto"; @@ -539,7 +540,7 @@ message CorsPolicy { core.v4alpha.RuntimeFractionalPercent shadow_enabled = 10; } -// [#next-free-field: 34] +// [#next-free-field: 35] message RouteAction { option (udpa.annotations.versioning).previous_message_type = "envoy.config.route.v3.RouteAction"; @@ -552,6 +553,7 @@ message RouteAction { } // Configures :ref:`internal redirect ` behavior. + // [#next-major-version: remove this definition - it's defined in the InternalRedirectPolicy message.] enum InternalRedirectAction { PASS_THROUGH_INTERNAL_REDIRECT = 0; HANDLE_INTERNAL_REDIRECT = 1; @@ -750,9 +752,9 @@ message RouteAction { ConnectConfig connect_config = 3; } - reserved 12, 18, 19, 16, 22, 21, 10; + reserved 12, 18, 19, 16, 22, 21, 10, 26, 31; - reserved "request_mirror_policy"; + reserved "request_mirror_policy", "internal_redirect_action", "max_internal_redirects"; oneof cluster_specifier { option (validate.required) = true; @@ -992,23 +994,11 @@ message RouteAction { repeated UpgradeConfig upgrade_configs = 25; - InternalRedirectAction internal_redirect_action = 26; - - // An internal redirect is handled, iff the number of previous internal redirects that a - // downstream request has encountered is lower than this value, and - // :ref:`internal_redirect_action ` - // is set to :ref:`HANDLE_INTERNAL_REDIRECT - // ` - // In the case where a downstream request is bounced among multiple routes by internal redirect, - // the first route that hits this threshold, or has - // :ref:`internal_redirect_action ` - // set to - // :ref:`PASS_THROUGH_INTERNAL_REDIRECT - // ` - // will pass the redirect back to downstream. - // - // If not specified, at most one redirect will be followed. - google.protobuf.UInt32Value max_internal_redirects = 31; + // If present, Envoy will try to follow an upstream redirect response instead of proxying the + // response back to the downstream. An upstream redirect response is defined + // by :ref:`redirect_response_codes + // `. + InternalRedirectPolicy internal_redirect_policy = 34; // Indicates that the route has a hedge policy. Note that if this is set, // it'll take precedence over the virtual host level hedge policy entirely @@ -1603,3 +1593,33 @@ message QueryParameterMatcher { bool present_match = 6; } } + +// HTTP Internal Redirect :ref:`architecture overview `. +message InternalRedirectPolicy { + option (udpa.annotations.versioning).previous_message_type = + "envoy.config.route.v3.InternalRedirectPolicy"; + + // An internal redirect is not handled, unless the number of previous internal redirects that a + // downstream request has encountered is lower than this value. + // In the case where a downstream request is bounced among multiple routes by internal redirect, + // the first route that hits this threshold, or does not set :ref:`internal_redirect_policy + // ` + // will pass the redirect back to downstream. + // + // If not specified, at most one redirect will be followed. + google.protobuf.UInt32Value max_internal_redirects = 1; + + // Defines what upstream response codes are allowed to trigger internal redirect. If unspecified, + // only 302 will be treated as internal redirect. + // Only 301, 302, 303, 307 and 308 are valid values. Any other codes will be ignored. + repeated uint32 redirect_response_codes = 2 [(validate.rules).repeated = {max_items: 5}]; + + // Specifies a list of predicates that are queried when an upstream response is deemed + // to trigger an internal redirect by all other criteria. Any predicate in the list can reject + // the redirect, causing the response to be proxied to downstream. + repeated core.v4alpha.TypedExtensionConfig predicates = 3; + + // Allow internal redirect to follow a target URI with a different scheme than the value of + // x-forwarded-proto. The default is false. + bool allow_cross_scheme_redirect = 4; +} diff --git a/api/envoy/extensions/internal_redirect/allow_listed_routes/v3/BUILD b/api/envoy/extensions/internal_redirect/allow_listed_routes/v3/BUILD new file mode 100644 index 000000000000..ef3541ebcb1d --- /dev/null +++ b/api/envoy/extensions/internal_redirect/allow_listed_routes/v3/BUILD @@ -0,0 +1,9 @@ +# DO NOT EDIT. This file is generated by tools/proto_sync.py. + +load("@envoy_api//bazel:api_build_system.bzl", "api_proto_package") + +licenses(["notice"]) # Apache 2 + +api_proto_package( + deps = ["@com_github_cncf_udpa//udpa/annotations:pkg"], +) diff --git a/api/envoy/extensions/internal_redirect/allow_listed_routes/v3/allow_listed_routes_config.proto b/api/envoy/extensions/internal_redirect/allow_listed_routes/v3/allow_listed_routes_config.proto new file mode 100644 index 000000000000..a6da5b0f5d9b --- /dev/null +++ b/api/envoy/extensions/internal_redirect/allow_listed_routes/v3/allow_listed_routes_config.proto @@ -0,0 +1,24 @@ +syntax = "proto3"; + +package envoy.extensions.internal_redirect.allow_listed_routes.v3; + +import "udpa/annotations/status.proto"; +import "udpa/annotations/versioning.proto"; +import "validate/validate.proto"; + +option java_package = "io.envoyproxy.envoy.extensions.internal_redirect.allow_listed_routes.v3"; +option java_outer_classname = "AllowListedRoutesConfigProto"; +option java_multiple_files = true; +option (udpa.annotations.file_status).package_version_status = ACTIVE; + +// [#protodoc-title: Allow listed routes internal redirect predicate] + +// An internal redirect predicate that accepts only explicitly allowed target routes. +// [#extension: envoy.internal_redirect_predicates.allow_listed_routes] +message AllowListedRoutesConfig { + // The list of routes that's allowed as redirect target by this predicate, + // identified by the route's :ref:`name `. + // Empty route names are not allowed. + repeated string allowed_route_names = 1 + [(validate.rules).repeated = {items {string {min_len: 1}}}]; +} diff --git a/api/envoy/extensions/internal_redirect/previous_routes/v3/BUILD b/api/envoy/extensions/internal_redirect/previous_routes/v3/BUILD new file mode 100644 index 000000000000..ef3541ebcb1d --- /dev/null +++ b/api/envoy/extensions/internal_redirect/previous_routes/v3/BUILD @@ -0,0 +1,9 @@ +# DO NOT EDIT. This file is generated by tools/proto_sync.py. + +load("@envoy_api//bazel:api_build_system.bzl", "api_proto_package") + +licenses(["notice"]) # Apache 2 + +api_proto_package( + deps = ["@com_github_cncf_udpa//udpa/annotations:pkg"], +) diff --git a/api/envoy/extensions/internal_redirect/previous_routes/v3/previous_routes_config.proto b/api/envoy/extensions/internal_redirect/previous_routes/v3/previous_routes_config.proto new file mode 100644 index 000000000000..6cc5fba871ea --- /dev/null +++ b/api/envoy/extensions/internal_redirect/previous_routes/v3/previous_routes_config.proto @@ -0,0 +1,19 @@ +syntax = "proto3"; + +package envoy.extensions.internal_redirect.previous_routes.v3; + +import "udpa/annotations/status.proto"; +import "udpa/annotations/versioning.proto"; + +option java_package = "io.envoyproxy.envoy.extensions.internal_redirect.previous_routes.v3"; +option java_outer_classname = "PreviousRoutesConfigProto"; +option java_multiple_files = true; +option (udpa.annotations.file_status).package_version_status = ACTIVE; + +// [#protodoc-title: Previous routes internal redirect predicate] + +// An internal redirect predicate that rejects redirect targets that are pointing +// to a route that has been followed by a previous redirect from the current route. +// [#extension: envoy.internal_redirect_predicates.previous_routes] +message PreviousRoutesConfig { +} diff --git a/api/envoy/extensions/internal_redirect/safe_cross_scheme/v3/BUILD b/api/envoy/extensions/internal_redirect/safe_cross_scheme/v3/BUILD new file mode 100644 index 000000000000..ef3541ebcb1d --- /dev/null +++ b/api/envoy/extensions/internal_redirect/safe_cross_scheme/v3/BUILD @@ -0,0 +1,9 @@ +# DO NOT EDIT. This file is generated by tools/proto_sync.py. + +load("@envoy_api//bazel:api_build_system.bzl", "api_proto_package") + +licenses(["notice"]) # Apache 2 + +api_proto_package( + deps = ["@com_github_cncf_udpa//udpa/annotations:pkg"], +) diff --git a/api/envoy/extensions/internal_redirect/safe_cross_scheme/v3/safe_cross_scheme_config.proto b/api/envoy/extensions/internal_redirect/safe_cross_scheme/v3/safe_cross_scheme_config.proto new file mode 100644 index 000000000000..54cec2f09bbb --- /dev/null +++ b/api/envoy/extensions/internal_redirect/safe_cross_scheme/v3/safe_cross_scheme_config.proto @@ -0,0 +1,24 @@ +syntax = "proto3"; + +package envoy.extensions.internal_redirect.safe_cross_scheme.v3; + +import "udpa/annotations/status.proto"; +import "udpa/annotations/versioning.proto"; +import "validate/validate.proto"; + +option java_package = "io.envoyproxy.envoy.extensions.internal_redirect.safe_cross_scheme.v3"; +option java_outer_classname = "SafeCrossSchemeConfigProto"; +option java_multiple_files = true; +option (udpa.annotations.file_status).package_version_status = ACTIVE; + +// [#protodoc-title: SafeCrossScheme internal redirect predicate] + +// An internal redirect predicate that checks the scheme between the +// downstream url and the redirect target url and allows a) same scheme +// redirect and b) safe cross scheme redirect, which means if the downstream +// scheme is HTTPS, both HTTPS and HTTP redirect targets are allowed, but if the +// downstream scheme is HTTP, only HTTP redirect targets are allowed. +// [#extension: +// envoy.internal_redirect_predicates.safe_cross_scheme] +message SafeCrossSchemeConfig { +} diff --git a/api/versioning/BUILD b/api/versioning/BUILD index e23f851b221e..d7771fbbd29e 100644 --- a/api/versioning/BUILD +++ b/api/versioning/BUILD @@ -105,6 +105,9 @@ proto_library( "//envoy/extensions/filters/network/thrift_proxy/v3:pkg", "//envoy/extensions/filters/network/zookeeper_proxy/v3:pkg", "//envoy/extensions/filters/udp/dns_filter/v3alpha:pkg", + "//envoy/extensions/internal_redirect/allow_listed_routes/v3:pkg", + "//envoy/extensions/internal_redirect/previous_routes/v3:pkg", + "//envoy/extensions/internal_redirect/safe_cross_scheme/v3:pkg", "//envoy/extensions/retry/host/omit_host_metadata/v3:pkg", "//envoy/extensions/retry/priority/previous_priorities/v3:pkg", "//envoy/extensions/transport_sockets/alts/v3:pkg", diff --git a/docs/root/api-v3/config/config.rst b/docs/root/api-v3/config/config.rst index 663ac872183d..ba7ca7e70f76 100644 --- a/docs/root/api-v3/config/config.rst +++ b/docs/root/api-v3/config/config.rst @@ -17,3 +17,4 @@ Extensions grpc_credential/grpc_credential retry/retry trace/trace + internal_redirect/internal_redirect diff --git a/docs/root/api-v3/config/internal_redirect/internal_redirect.rst b/docs/root/api-v3/config/internal_redirect/internal_redirect.rst new file mode 100644 index 000000000000..5452e8accee7 --- /dev/null +++ b/docs/root/api-v3/config/internal_redirect/internal_redirect.rst @@ -0,0 +1,8 @@ +Internal Redirect Predicates +============================ + +.. toctree:: + :glob: + :maxdepth: 2 + + ../../extensions/internal_redirect/** diff --git a/docs/root/intro/arch_overview/http/http_connection_management.rst b/docs/root/intro/arch_overview/http/http_connection_management.rst index b69c587b17d4..74e8d90b99e8 100644 --- a/docs/root/intro/arch_overview/http/http_connection_management.rst +++ b/docs/root/intro/arch_overview/http/http_connection_management.rst @@ -151,37 +151,60 @@ previously attempted priorities. Internal redirects -------------------------- -Envoy supports handling 302 redirects internally, that is capturing a 302 redirect response, -synthesizing a new request, sending it to the upstream specified by the new route match, and -returning the redirected response as the response to the original request. +Envoy supports handling 3xx redirects internally, that is capturing a configurable 3xx redirect +response, synthesizing a new request, sending it to the upstream specified by the new route match, +and returning the redirected response as the response to the original request. -Internal redirects are configured via the ref:`internal redirect action -` field and -`max internal redirects ` field in -route configuration. When redirect handling is on, any 302 response from upstream is -subject to the redirect being handled by Envoy. +Internal redirects are configured via the :ref:`internal redirect policy +` field in route configuration. +When redirect handling is on, any 3xx response from upstream, that matches +:ref:`redirect_response_codes +` +is subject to the redirect being handled by Envoy. For a redirect to be handled successfully it must pass the following checks: -1. Be a 302 response. -2. Have a *location* header with a valid, fully qualified URL matching the scheme of the original request. +1. Have a response code matching one of :ref:`redirect_response_codes + `, which is + either 302 (by default), or a set of 3xx codes (301, 302, 303, 307, 308). +2. Have a *location* header with a valid, fully qualified URL. 3. The request must have been fully processed by Envoy. 4. The request must not have a body. -5. The number of previously handled internal redirect within a given downstream request does not exceed - `max internal redirects ` of the route - that the request or redirected request is hitting. +5. :ref:`allow_cross_scheme_redirect + ` is true (default to false), + or the scheme of the downstream request and the *location* header are the same. +6. The number of previously handled internal redirect within a given downstream request does not + exceed :ref:`max internal redirects + ` + of the route that the request or redirected request is hitting. +7. All :ref:`predicates ` accept + the target route. Any failure will result in redirect being passed downstream instead. Since a redirected request may be bounced between different routes, any route in the chain of redirects that 1. does not have internal redirect enabled -2. or has a `max internal redirects - ` +2. or has a :ref:`max internal redirects + ` smaller or equal to the redirect chain length when the redirect chain hits it +3. or is disallowed by any of the :ref:`predicates + ` will cause the redirect to be passed downstream. +Two predicates can be used to create a DAG that defines the redirect chain, the :ref:`previous routes +` predicate, and +the :ref:`allow_listed_routes +`. +Specifically, the *allow listed routes* predicate defines edges of individual node in the DAG +and the *previous routes* predicate defines "visited" state of the edges, so that loop can be avoided +if so desired. + +A third predicate :ref:`safe_cross_scheme +` +can be used to prevent HTTP -> HTTPS redirect. + Once the redirect has passed these checks, the request headers which were shipped to the original upstream will be modified by: diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index 8e19981456af..4b0bf0bf5ce9 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -46,6 +46,8 @@ Changes tracing is not forced. * router: allow retries of streaming or incomplete requests. This removes stat `rq_retry_skipped_request_not_complete`. * router: allow retries by default when upstream responds with :ref:`x-envoy-overloaded `. +* router: more fine grained internal redirect configs are added to the :ref`internal_redirect_policy + ` field. * runtime: add new gauge :ref:`deprecated_feature_seen_since_process_start ` that gets reset across hot restarts. * stats: added the option to :ref:`report counters as deltas ` to the metrics service stats sink. * tracing: tracing configuration has been made fully dynamic and every HTTP connection manager @@ -63,3 +65,10 @@ Deprecated * The * :ref:`GoogleRE2.max_program_size` field is now deprecated. Management servers are expected to validate regexp program sizes instead of expecting the client to do it. +* The :ref:`internal_redirect_action ` + field and :ref:`max_internal_redirects ` field + are now deprecated. This changes the implemented default cross scheme redirect behavior. + All cross scheme redirect are disallowed by default. To restore + the previous behavior, set allow_cross_scheme_redirect=true and use + :ref:`safe_cross_scheme`, + in :ref:`predicates `. diff --git a/generated_api_shadow/envoy/config/route/v3/route_components.proto b/generated_api_shadow/envoy/config/route/v3/route_components.proto index 00d4f5e628a7..a72c8a226001 100644 --- a/generated_api_shadow/envoy/config/route/v3/route_components.proto +++ b/generated_api_shadow/envoy/config/route/v3/route_components.proto @@ -3,6 +3,7 @@ syntax = "proto3"; package envoy.config.route.v3; import "envoy/config/core/v3/base.proto"; +import "envoy/config/core/v3/extension.proto"; import "envoy/config/core/v3/proxy_protocol.proto"; import "envoy/type/matcher/v3/regex.proto"; import "envoy/type/matcher/v3/string.proto"; @@ -548,7 +549,7 @@ message CorsPolicy { [deprecated = true, (validate.rules).repeated = {items {string {max_bytes: 1024}}}]; } -// [#next-free-field: 34] +// [#next-free-field: 35] message RouteAction { option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.route.RouteAction"; @@ -561,6 +562,7 @@ message RouteAction { } // Configures :ref:`internal redirect ` behavior. + // [#next-major-version: remove this definition - it's defined in the InternalRedirectPolicy message.] enum InternalRedirectAction { PASS_THROUGH_INTERNAL_REDIRECT = 0; HANDLE_INTERNAL_REDIRECT = 1; @@ -995,7 +997,13 @@ message RouteAction { repeated UpgradeConfig upgrade_configs = 25; - InternalRedirectAction internal_redirect_action = 26; + // If present, Envoy will try to follow an upstream redirect response instead of proxying the + // response back to the downstream. An upstream redirect response is defined + // by :ref:`redirect_response_codes + // `. + InternalRedirectPolicy internal_redirect_policy = 34; + + InternalRedirectAction internal_redirect_action = 26 [deprecated = true]; // An internal redirect is handled, iff the number of previous internal redirects that a // downstream request has encountered is lower than this value, and @@ -1011,7 +1019,7 @@ message RouteAction { // will pass the redirect back to downstream. // // If not specified, at most one redirect will be followed. - google.protobuf.UInt32Value max_internal_redirects = 31; + google.protobuf.UInt32Value max_internal_redirects = 31 [deprecated = true]; // Indicates that the route has a hedge policy. Note that if this is set, // it'll take precedence over the virtual host level hedge policy entirely @@ -1611,3 +1619,30 @@ message QueryParameterMatcher { google.protobuf.BoolValue hidden_envoy_deprecated_regex = 4 [deprecated = true, (envoy.annotations.disallowed_by_default) = true]; } + +// HTTP Internal Redirect :ref:`architecture overview `. +message InternalRedirectPolicy { + // An internal redirect is not handled, unless the number of previous internal redirects that a + // downstream request has encountered is lower than this value. + // In the case where a downstream request is bounced among multiple routes by internal redirect, + // the first route that hits this threshold, or does not set :ref:`internal_redirect_policy + // ` + // will pass the redirect back to downstream. + // + // If not specified, at most one redirect will be followed. + google.protobuf.UInt32Value max_internal_redirects = 1; + + // Defines what upstream response codes are allowed to trigger internal redirect. If unspecified, + // only 302 will be treated as internal redirect. + // Only 301, 302, 303, 307 and 308 are valid values. Any other codes will be ignored. + repeated uint32 redirect_response_codes = 2 [(validate.rules).repeated = {max_items: 5}]; + + // Specifies a list of predicates that are queried when an upstream response is deemed + // to trigger an internal redirect by all other criteria. Any predicate in the list can reject + // the redirect, causing the response to be proxied to downstream. + repeated core.v3.TypedExtensionConfig predicates = 3; + + // Allow internal redirect to follow a target URI with a different scheme than the value of + // x-forwarded-proto. The default is false. + bool allow_cross_scheme_redirect = 4; +} diff --git a/generated_api_shadow/envoy/config/route/v4alpha/route_components.proto b/generated_api_shadow/envoy/config/route/v4alpha/route_components.proto index 6e1b1f9f5a0a..ee42a986ced8 100644 --- a/generated_api_shadow/envoy/config/route/v4alpha/route_components.proto +++ b/generated_api_shadow/envoy/config/route/v4alpha/route_components.proto @@ -3,6 +3,7 @@ syntax = "proto3"; package envoy.config.route.v4alpha; import "envoy/config/core/v4alpha/base.proto"; +import "envoy/config/core/v4alpha/extension.proto"; import "envoy/config/core/v4alpha/proxy_protocol.proto"; import "envoy/type/matcher/v4alpha/regex.proto"; import "envoy/type/matcher/v4alpha/string.proto"; @@ -539,7 +540,7 @@ message CorsPolicy { core.v4alpha.RuntimeFractionalPercent shadow_enabled = 10; } -// [#next-free-field: 34] +// [#next-free-field: 35] message RouteAction { option (udpa.annotations.versioning).previous_message_type = "envoy.config.route.v3.RouteAction"; @@ -552,6 +553,7 @@ message RouteAction { } // Configures :ref:`internal redirect ` behavior. + // [#next-major-version: remove this definition - it's defined in the InternalRedirectPolicy message.] enum InternalRedirectAction { PASS_THROUGH_INTERNAL_REDIRECT = 0; HANDLE_INTERNAL_REDIRECT = 1; @@ -992,7 +994,13 @@ message RouteAction { repeated UpgradeConfig upgrade_configs = 25; - InternalRedirectAction internal_redirect_action = 26; + // If present, Envoy will try to follow an upstream redirect response instead of proxying the + // response back to the downstream. An upstream redirect response is defined + // by :ref:`redirect_response_codes + // `. + InternalRedirectPolicy internal_redirect_policy = 34; + + InternalRedirectAction hidden_envoy_deprecated_internal_redirect_action = 26 [deprecated = true]; // An internal redirect is handled, iff the number of previous internal redirects that a // downstream request has encountered is lower than this value, and @@ -1008,7 +1016,8 @@ message RouteAction { // will pass the redirect back to downstream. // // If not specified, at most one redirect will be followed. - google.protobuf.UInt32Value max_internal_redirects = 31; + google.protobuf.UInt32Value hidden_envoy_deprecated_max_internal_redirects = 31 + [deprecated = true]; // Indicates that the route has a hedge policy. Note that if this is set, // it'll take precedence over the virtual host level hedge policy entirely @@ -1603,3 +1612,33 @@ message QueryParameterMatcher { bool present_match = 6; } } + +// HTTP Internal Redirect :ref:`architecture overview `. +message InternalRedirectPolicy { + option (udpa.annotations.versioning).previous_message_type = + "envoy.config.route.v3.InternalRedirectPolicy"; + + // An internal redirect is not handled, unless the number of previous internal redirects that a + // downstream request has encountered is lower than this value. + // In the case where a downstream request is bounced among multiple routes by internal redirect, + // the first route that hits this threshold, or does not set :ref:`internal_redirect_policy + // ` + // will pass the redirect back to downstream. + // + // If not specified, at most one redirect will be followed. + google.protobuf.UInt32Value max_internal_redirects = 1; + + // Defines what upstream response codes are allowed to trigger internal redirect. If unspecified, + // only 302 will be treated as internal redirect. + // Only 301, 302, 303, 307 and 308 are valid values. Any other codes will be ignored. + repeated uint32 redirect_response_codes = 2 [(validate.rules).repeated = {max_items: 5}]; + + // Specifies a list of predicates that are queried when an upstream response is deemed + // to trigger an internal redirect by all other criteria. Any predicate in the list can reject + // the redirect, causing the response to be proxied to downstream. + repeated core.v4alpha.TypedExtensionConfig predicates = 3; + + // Allow internal redirect to follow a target URI with a different scheme than the value of + // x-forwarded-proto. The default is false. + bool allow_cross_scheme_redirect = 4; +} diff --git a/generated_api_shadow/envoy/extensions/internal_redirect/allow_listed_routes/v3/BUILD b/generated_api_shadow/envoy/extensions/internal_redirect/allow_listed_routes/v3/BUILD new file mode 100644 index 000000000000..ef3541ebcb1d --- /dev/null +++ b/generated_api_shadow/envoy/extensions/internal_redirect/allow_listed_routes/v3/BUILD @@ -0,0 +1,9 @@ +# DO NOT EDIT. This file is generated by tools/proto_sync.py. + +load("@envoy_api//bazel:api_build_system.bzl", "api_proto_package") + +licenses(["notice"]) # Apache 2 + +api_proto_package( + deps = ["@com_github_cncf_udpa//udpa/annotations:pkg"], +) diff --git a/generated_api_shadow/envoy/extensions/internal_redirect/allow_listed_routes/v3/allow_listed_routes_config.proto b/generated_api_shadow/envoy/extensions/internal_redirect/allow_listed_routes/v3/allow_listed_routes_config.proto new file mode 100644 index 000000000000..a6da5b0f5d9b --- /dev/null +++ b/generated_api_shadow/envoy/extensions/internal_redirect/allow_listed_routes/v3/allow_listed_routes_config.proto @@ -0,0 +1,24 @@ +syntax = "proto3"; + +package envoy.extensions.internal_redirect.allow_listed_routes.v3; + +import "udpa/annotations/status.proto"; +import "udpa/annotations/versioning.proto"; +import "validate/validate.proto"; + +option java_package = "io.envoyproxy.envoy.extensions.internal_redirect.allow_listed_routes.v3"; +option java_outer_classname = "AllowListedRoutesConfigProto"; +option java_multiple_files = true; +option (udpa.annotations.file_status).package_version_status = ACTIVE; + +// [#protodoc-title: Allow listed routes internal redirect predicate] + +// An internal redirect predicate that accepts only explicitly allowed target routes. +// [#extension: envoy.internal_redirect_predicates.allow_listed_routes] +message AllowListedRoutesConfig { + // The list of routes that's allowed as redirect target by this predicate, + // identified by the route's :ref:`name `. + // Empty route names are not allowed. + repeated string allowed_route_names = 1 + [(validate.rules).repeated = {items {string {min_len: 1}}}]; +} diff --git a/generated_api_shadow/envoy/extensions/internal_redirect/previous_routes/v3/BUILD b/generated_api_shadow/envoy/extensions/internal_redirect/previous_routes/v3/BUILD new file mode 100644 index 000000000000..ef3541ebcb1d --- /dev/null +++ b/generated_api_shadow/envoy/extensions/internal_redirect/previous_routes/v3/BUILD @@ -0,0 +1,9 @@ +# DO NOT EDIT. This file is generated by tools/proto_sync.py. + +load("@envoy_api//bazel:api_build_system.bzl", "api_proto_package") + +licenses(["notice"]) # Apache 2 + +api_proto_package( + deps = ["@com_github_cncf_udpa//udpa/annotations:pkg"], +) diff --git a/generated_api_shadow/envoy/extensions/internal_redirect/previous_routes/v3/previous_routes_config.proto b/generated_api_shadow/envoy/extensions/internal_redirect/previous_routes/v3/previous_routes_config.proto new file mode 100644 index 000000000000..6cc5fba871ea --- /dev/null +++ b/generated_api_shadow/envoy/extensions/internal_redirect/previous_routes/v3/previous_routes_config.proto @@ -0,0 +1,19 @@ +syntax = "proto3"; + +package envoy.extensions.internal_redirect.previous_routes.v3; + +import "udpa/annotations/status.proto"; +import "udpa/annotations/versioning.proto"; + +option java_package = "io.envoyproxy.envoy.extensions.internal_redirect.previous_routes.v3"; +option java_outer_classname = "PreviousRoutesConfigProto"; +option java_multiple_files = true; +option (udpa.annotations.file_status).package_version_status = ACTIVE; + +// [#protodoc-title: Previous routes internal redirect predicate] + +// An internal redirect predicate that rejects redirect targets that are pointing +// to a route that has been followed by a previous redirect from the current route. +// [#extension: envoy.internal_redirect_predicates.previous_routes] +message PreviousRoutesConfig { +} diff --git a/generated_api_shadow/envoy/extensions/internal_redirect/safe_cross_scheme/v3/BUILD b/generated_api_shadow/envoy/extensions/internal_redirect/safe_cross_scheme/v3/BUILD new file mode 100644 index 000000000000..ef3541ebcb1d --- /dev/null +++ b/generated_api_shadow/envoy/extensions/internal_redirect/safe_cross_scheme/v3/BUILD @@ -0,0 +1,9 @@ +# DO NOT EDIT. This file is generated by tools/proto_sync.py. + +load("@envoy_api//bazel:api_build_system.bzl", "api_proto_package") + +licenses(["notice"]) # Apache 2 + +api_proto_package( + deps = ["@com_github_cncf_udpa//udpa/annotations:pkg"], +) diff --git a/generated_api_shadow/envoy/extensions/internal_redirect/safe_cross_scheme/v3/safe_cross_scheme_config.proto b/generated_api_shadow/envoy/extensions/internal_redirect/safe_cross_scheme/v3/safe_cross_scheme_config.proto new file mode 100644 index 000000000000..54cec2f09bbb --- /dev/null +++ b/generated_api_shadow/envoy/extensions/internal_redirect/safe_cross_scheme/v3/safe_cross_scheme_config.proto @@ -0,0 +1,24 @@ +syntax = "proto3"; + +package envoy.extensions.internal_redirect.safe_cross_scheme.v3; + +import "udpa/annotations/status.proto"; +import "udpa/annotations/versioning.proto"; +import "validate/validate.proto"; + +option java_package = "io.envoyproxy.envoy.extensions.internal_redirect.safe_cross_scheme.v3"; +option java_outer_classname = "SafeCrossSchemeConfigProto"; +option java_multiple_files = true; +option (udpa.annotations.file_status).package_version_status = ACTIVE; + +// [#protodoc-title: SafeCrossScheme internal redirect predicate] + +// An internal redirect predicate that checks the scheme between the +// downstream url and the redirect target url and allows a) same scheme +// redirect and b) safe cross scheme redirect, which means if the downstream +// scheme is HTTPS, both HTTPS and HTTP redirect targets are allowed, but if the +// downstream scheme is HTTP, only HTTP redirect targets are allowed. +// [#extension: +// envoy.internal_redirect_predicates.safe_cross_scheme] +message SafeCrossSchemeConfig { +} diff --git a/include/envoy/router/BUILD b/include/envoy/router/BUILD index b829997d24aa..44aee699e338 100644 --- a/include/envoy/router/BUILD +++ b/include/envoy/router/BUILD @@ -53,6 +53,7 @@ envoy_cc_library( hdrs = ["router.h"], external_deps = ["abseil_optional"], deps = [ + ":internal_redirect_interface", "//include/envoy/access_log:access_log_interface", "//include/envoy/common:matchers_interface", "//include/envoy/config:typed_metadata_interface", @@ -108,3 +109,13 @@ envoy_cc_library( "//include/envoy/stream_info:filter_state_interface", ], ) + +envoy_cc_library( + name = "internal_redirect_interface", + hdrs = ["internal_redirect.h"], + deps = [ + "//include/envoy/config:typed_config_interface", + "//include/envoy/stream_info:filter_state_interface", + "//source/common/common:minimal_logger_lib", + ], +) diff --git a/include/envoy/router/internal_redirect.h b/include/envoy/router/internal_redirect.h new file mode 100644 index 000000000000..95f624255ace --- /dev/null +++ b/include/envoy/router/internal_redirect.h @@ -0,0 +1,66 @@ +#pragma once + +#include "envoy/config/typed_config.h" +#include "envoy/stream_info/filter_state.h" + +#include "common/common/logger.h" + +#include "absl/strings/string_view.h" + +namespace Envoy { +namespace Router { + +/** + * Used to decide if an internal redirect is allowed to be followed based on the target route. + * Subclassing Logger::Loggable so that implementations can log details. + */ +class InternalRedirectPredicate : Logger::Loggable { +public: + virtual ~InternalRedirectPredicate() = default; + + /** + * A FilterState is provided so that predicate implementation can use it to preserve state across + * internal redirects. + * @param filter_state supplies the filter state associated with the current request so that the + * predicates can use it to persist states across filter chains. + * @param target_route_name indicates the route that an internal redirect is targeting. + * @param downstream_is_https indicates the downstream request is using https. + * @param target_is_https indicates the internal redirect target url has https in the url. + * @return whether the route specified by target_route_name is allowed to be followed. Any + * predicate returning false will prevent the redirect from being followed, causing the + * response to be proxied downstream. + */ + virtual bool acceptTargetRoute(StreamInfo::FilterState& filter_state, + absl::string_view target_route_name, bool downstream_is_https, + bool target_is_https) PURE; + + /** + * @return the name of the current predicate. + */ + virtual absl::string_view name() const PURE; +}; + +using InternalRedirectPredicateSharedPtr = std::shared_ptr; + +/** + * Factory for InternalRedirectPredicate. + */ +class InternalRedirectPredicateFactory : public Config::TypedFactory { +public: + ~InternalRedirectPredicateFactory() override = default; + + /** + * @param config contains the proto stored in TypedExtensionConfig.typed_config for the predicate. + * @param current_route_name stores the route name of the route where the predicate is installed. + * @return an InternalRedirectPredicate. The given current_route_name is useful for predicates + * that need to create per-route FilterState. + */ + virtual InternalRedirectPredicateSharedPtr + createInternalRedirectPredicate(const Protobuf::Message& config, + absl::string_view current_route_name) PURE; + + std::string category() const override { return "envoy.internal_redirect_predicates"; } +}; + +} // namespace Router +} // namespace Envoy diff --git a/include/envoy/router/router.h b/include/envoy/router/router.h index 185ba349d185..39be039e7cfd 100644 --- a/include/envoy/router/router.h +++ b/include/envoy/router/router.h @@ -17,6 +17,7 @@ #include "envoy/http/codes.h" #include "envoy/http/hash_policy.h" #include "envoy/http/header_map.h" +#include "envoy/router/internal_redirect.h" #include "envoy/tracing/http_tracer.h" #include "envoy/type/v3/percent.pb.h" #include "envoy/upstream/resource_manager.h" @@ -237,9 +238,42 @@ class RetryPolicy { enum class RetryStatus { No, NoOverflow, NoRetryLimitExceeded, Yes }; /** - * InternalRedirectAction from the route configuration. + * InternalRedirectPolicy from the route configuration. */ -enum class InternalRedirectAction { PassThrough, Handle }; +class InternalRedirectPolicy { +public: + virtual ~InternalRedirectPolicy() = default; + + /** + * @return whether internal redirect is enabled on this route. + */ + virtual bool enabled() const PURE; + + /** + * @param response_code the response code from the upstream. + * @return whether the given response_code should trigger an internal redirect on this route. + */ + virtual bool shouldRedirectForResponseCode(const Http::Code& response_code) const PURE; + + /** + * Creates the target route predicates. This should really be called only once for each upstream + * redirect response. Creating the predicates lazily to avoid wasting CPU cycles on non-redirect + * responses, which should be the most common case. + * @return a vector of newly constructed InternalRedirectPredicate instances. + */ + virtual std::vector predicates() const PURE; + + /** + * @return the maximum number of allowed internal redirects on this route. + */ + virtual uint32_t maxInternalRedirects() const PURE; + + /** + * @return if it is allowed to follow the redirect with a different scheme in + * the target URI than the downstream request. + */ + virtual bool isCrossSchemeRedirectAllowed() const PURE; +}; /** * Wraps retry state for an active routed request. @@ -686,6 +720,13 @@ class RouteEntry : public ResponseEntry { */ virtual const RetryPolicy& retryPolicy() const PURE; + /** + * @return const InternalRedirectPolicy& the internal redirect policy for the route. All routes + * have a internal redirect policy even if it is not enabled, which means redirects are + * simply proxied as normal responses. + */ + virtual const InternalRedirectPolicy& internalRedirectPolicy() const PURE; + /** * @return uint32_t any route cap on bytes which should be buffered for shadowing or retries. * This is an upper bound so does not necessarily reflect the bytes which will be buffered @@ -834,17 +875,6 @@ class RouteEntry : public ResponseEntry { */ virtual const absl::optional& connectConfig() const PURE; - /** - * @returns the internal redirect action which should be taken on this route. - */ - virtual InternalRedirectAction internalRedirectAction() const PURE; - - /** - * @returns the threshold of number of previously handled internal redirects, for this route to - * stop handle internal redirects. - */ - virtual uint32_t maxInternalRedirects() const PURE; - /** * @return std::string& the name of the route. */ diff --git a/source/common/http/async_client_impl.cc b/source/common/http/async_client_impl.cc index 3ef9de94a806..088d93fe43bf 100644 --- a/source/common/http/async_client_impl.cc +++ b/source/common/http/async_client_impl.cc @@ -20,6 +20,7 @@ const std::vector> const AsyncStreamImpl::NullHedgePolicy AsyncStreamImpl::RouteEntryImpl::hedge_policy_; const AsyncStreamImpl::NullRateLimitPolicy AsyncStreamImpl::RouteEntryImpl::rate_limit_policy_; const AsyncStreamImpl::NullRetryPolicy AsyncStreamImpl::RouteEntryImpl::retry_policy_; +const Router::InternalRedirectPolicyImpl AsyncStreamImpl::RouteEntryImpl::internal_redirect_policy_; const std::vector AsyncStreamImpl::RouteEntryImpl::shadow_policies_; const AsyncStreamImpl::NullVirtualHost AsyncStreamImpl::RouteEntryImpl::virtual_host_; const AsyncStreamImpl::NullRateLimitPolicy AsyncStreamImpl::NullVirtualHost::rate_limit_policy_; diff --git a/source/common/http/async_client_impl.h b/source/common/http/async_client_impl.h index beaea6c3ef60..241c7135348c 100644 --- a/source/common/http/async_client_impl.h +++ b/source/common/http/async_client_impl.h @@ -231,6 +231,9 @@ class AsyncStreamImpl : public AsyncClient::Stream, } const Router::RateLimitPolicy& rateLimitPolicy() const override { return rate_limit_policy_; } const Router::RetryPolicy& retryPolicy() const override { return retry_policy_; } + const Router::InternalRedirectPolicy& internalRedirectPolicy() const override { + return internal_redirect_policy_; + } uint32_t retryShadowBufferLimit() const override { return std::numeric_limits::max(); } @@ -279,15 +282,12 @@ class AsyncStreamImpl : public AsyncClient::Stream, bool includeAttemptCountInRequest() const override { return false; } bool includeAttemptCountInResponse() const override { return false; } const Router::RouteEntry::UpgradeMap& upgradeMap() const override { return upgrade_map_; } - Router::InternalRedirectAction internalRedirectAction() const override { - return Router::InternalRedirectAction::PassThrough; - } - uint32_t maxInternalRedirects() const override { return 1; } const std::string& routeName() const override { return route_name_; } std::unique_ptr hash_policy_; static const NullHedgePolicy hedge_policy_; static const NullRateLimitPolicy rate_limit_policy_; static const NullRetryPolicy retry_policy_; + static const Router::InternalRedirectPolicyImpl internal_redirect_policy_; static const std::vector shadow_policies_; static const NullVirtualHost virtual_host_; static const std::multimap opaque_config_; diff --git a/source/common/router/BUILD b/source/common/router/BUILD index 3fb3a37fb915..b194d6e239a6 100644 --- a/source/common/router/BUILD +++ b/source/common/router/BUILD @@ -289,6 +289,7 @@ envoy_cc_library( "//source/common/access_log:access_log_lib", "//source/common/buffer:watermark_buffer_lib", "//source/common/common:assert_lib", + "//source/common/common:cleanup_lib", "//source/common/common:empty_string", "//source/common/common:enum_to_int", "//source/common/common:hash_lib", diff --git a/source/common/router/config_impl.cc b/source/common/router/config_impl.cc index a6c3fd0b34c4..fb8ec0d8449d 100644 --- a/source/common/router/config_impl.cc +++ b/source/common/router/config_impl.cc @@ -45,18 +45,6 @@ namespace Envoy { namespace Router { namespace { -InternalRedirectAction -convertInternalRedirectAction(const envoy::config::route::v3::RouteAction& route) { - switch (route.internal_redirect_action()) { - case envoy::config::route::v3::RouteAction::PASS_THROUGH_INTERNAL_REDIRECT: - return InternalRedirectAction::PassThrough; - case envoy::config::route::v3::RouteAction::HANDLE_INTERNAL_REDIRECT: - return InternalRedirectAction::Handle; - default: - return InternalRedirectAction::PassThrough; - } -} - const std::string DEPRECATED_ROUTER_NAME = "envoy.router"; const absl::string_view getPath(const Http::RequestHeaderMap& headers) { @@ -156,6 +144,52 @@ Upstream::RetryPrioritySharedPtr RetryPolicyImpl::retryPriority() const { *validation_visitor_, num_retries_); } +InternalRedirectPolicyImpl::InternalRedirectPolicyImpl( + const envoy::config::route::v3::InternalRedirectPolicy& policy_config, + ProtobufMessage::ValidationVisitor& validator, absl::string_view current_route_name) + : current_route_name_(current_route_name), + redirect_response_codes_(buildRedirectResponseCodes(policy_config)), + max_internal_redirects_( + PROTOBUF_GET_WRAPPED_OR_DEFAULT(policy_config, max_internal_redirects, 1)), + enabled_(true), allow_cross_scheme_redirect_(policy_config.allow_cross_scheme_redirect()) { + for (const auto& predicate : policy_config.predicates()) { + const std::string type{ + TypeUtil::typeUrlToDescriptorFullName(predicate.typed_config().type_url())}; + auto* factory = + Registry::FactoryRegistry::getFactoryByType(type); + + auto config = factory->createEmptyConfigProto(); + Envoy::Config::Utility::translateOpaqueConfig(predicate.typed_config(), {}, validator, *config); + predicate_factories_.emplace_back(factory, std::move(config)); + } +} + +std::vector InternalRedirectPolicyImpl::predicates() const { + std::vector predicates; + for (const auto& predicate_factory : predicate_factories_) { + predicates.emplace_back(predicate_factory.first->createInternalRedirectPredicate( + *predicate_factory.second, current_route_name_)); + } + return predicates; +} + +absl::flat_hash_set InternalRedirectPolicyImpl::buildRedirectResponseCodes( + const envoy::config::route::v3::InternalRedirectPolicy& policy_config) const { + if (policy_config.redirect_response_codes_size() == 0) { + return absl::flat_hash_set{Http::Code::Found}; + } + absl::flat_hash_set ret; + std::for_each(policy_config.redirect_response_codes().begin(), + policy_config.redirect_response_codes().end(), [&ret](uint32_t response_code) { + const absl::flat_hash_set valid_redirect_response_code = {301, 302, 303, + 307, 308}; + if (valid_redirect_response_code.contains(response_code)) { + ret.insert(static_cast(response_code)); + } + }); + return ret; +} + CorsPolicyImpl::CorsPolicyImpl(const envoy::config::route::v3::CorsPolicy& config, Runtime::Loader& loader) : config_(config), loader_(loader), allow_methods_(config.allow_methods()), @@ -278,6 +312,8 @@ RouteEntryImplBase::RouteEntryImplBase(const VirtualHostImpl& vhost, strip_query_(route.redirect().strip_query()), hedge_policy_(buildHedgePolicy(vhost.hedgePolicy(), route.route())), retry_policy_(buildRetryPolicy(vhost.retryPolicy(), route.route(), validator)), + internal_redirect_policy_( + buildInternalRedirectPolicy(route.route(), validator, route.name())), rate_limit_policy_(route.route().rate_limits()), priority_(ConfigUtility::parsePriority(route.route().priority())), config_headers_(Http::HeaderUtility::buildHeaderDataVector(route.match().headers())), @@ -297,10 +333,7 @@ RouteEntryImplBase::RouteEntryImplBase(const VirtualHostImpl& vhost, per_filter_configs_(route.typed_per_filter_config(), route.hidden_envoy_deprecated_per_filter_config(), factory_context, validator), - route_name_(route.name()), time_source_(factory_context.dispatcher().timeSource()), - internal_redirect_action_(convertInternalRedirectAction(route.route())), - max_internal_redirects_( - PROTOBUF_GET_WRAPPED_OR_DEFAULT(route.route(), max_internal_redirects, 1)) { + route_name_(route.name()), time_source_(factory_context.dispatcher().timeSource()) { if (route.route().has_metadata_match()) { const auto filter_it = route.route().metadata_match().filter_metadata().find( Envoy::Config::MetadataFilters::get().ENVOY_LB); @@ -709,6 +742,28 @@ RetryPolicyImpl RouteEntryImplBase::buildRetryPolicy( return RetryPolicyImpl(); } +InternalRedirectPolicyImpl RouteEntryImplBase::buildInternalRedirectPolicy( + const envoy::config::route::v3::RouteAction& route_config, + ProtobufMessage::ValidationVisitor& validator, absl::string_view current_route_name) const { + if (route_config.has_internal_redirect_policy()) { + return InternalRedirectPolicyImpl(route_config.internal_redirect_policy(), validator, + current_route_name); + } + envoy::config::route::v3::InternalRedirectPolicy policy_config; + switch (route_config.internal_redirect_action()) { + case envoy::config::route::v3::RouteAction::HANDLE_INTERNAL_REDIRECT: + break; + case envoy::config::route::v3::RouteAction::PASS_THROUGH_INTERNAL_REDIRECT: + FALLTHRU; + default: + return InternalRedirectPolicyImpl(); + } + if (route_config.has_max_internal_redirects()) { + *policy_config.mutable_max_internal_redirects() = route_config.max_internal_redirects(); + } + return InternalRedirectPolicyImpl(policy_config, validator, current_route_name); +} + DecoratorConstPtr RouteEntryImplBase::parseDecorator(const envoy::config::route::v3::Route& route) { DecoratorConstPtr ret; if (route.has_decorator()) { diff --git a/source/common/router/config_impl.h b/source/common/router/config_impl.h index d5bec26b4791..31a81abf7e84 100644 --- a/source/common/router/config_impl.h +++ b/source/common/router/config_impl.h @@ -386,6 +386,46 @@ class RouteTracingImpl : public RouteTracing { Tracing::CustomTagMap custom_tags_; }; +/** + * Implementation of InternalRedirectPolicy that reads from the proto + * InternalRedirectPolicy of the RouteAction. + */ +class InternalRedirectPolicyImpl : public InternalRedirectPolicy { +public: + // Constructor that enables internal redirect with policy_config controlling the configurable + // behaviors. + explicit InternalRedirectPolicyImpl( + const envoy::config::route::v3::InternalRedirectPolicy& policy_config, + ProtobufMessage::ValidationVisitor& validator, absl::string_view current_route_name); + // Default constructor that disables internal redirect. + InternalRedirectPolicyImpl() = default; + + bool enabled() const override { return enabled_; } + + bool shouldRedirectForResponseCode(const Http::Code& response_code) const override { + return redirect_response_codes_.contains(response_code); + } + + std::vector predicates() const override; + + uint32_t maxInternalRedirects() const override { return max_internal_redirects_; } + + bool isCrossSchemeRedirectAllowed() const override { return allow_cross_scheme_redirect_; } + +private: + absl::flat_hash_set buildRedirectResponseCodes( + const envoy::config::route::v3::InternalRedirectPolicy& policy_config) const; + + const std::string current_route_name_; + const absl::flat_hash_set redirect_response_codes_; + const uint32_t max_internal_redirects_{1}; + const bool enabled_{false}; + const bool allow_cross_scheme_redirect_{false}; + + std::vector> + predicate_factories_; +}; + /** * Base implementation for all route entries. */ @@ -442,6 +482,9 @@ class RouteEntryImplBase : public RouteEntry, Upstream::ResourcePriority priority() const override { return priority_; } const RateLimitPolicy& rateLimitPolicy() const override { return rate_limit_policy_; } const RetryPolicy& retryPolicy() const override { return retry_policy_; } + const InternalRedirectPolicy& internalRedirectPolicy() const override { + return internal_redirect_policy_; + } uint32_t retryShadowBufferLimit() const override { return retry_shadow_buffer_limit_; } const std::vector& shadowPolicies() const override { return shadow_policies_; } const VirtualCluster* virtualCluster(const Http::HeaderMap& headers) const override { @@ -472,10 +515,6 @@ class RouteEntryImplBase : public RouteEntry, } const absl::optional& connectConfig() const override { return connect_config_; } const UpgradeMap& upgradeMap() const override { return upgrade_map_; } - InternalRedirectAction internalRedirectAction() const override { - return internal_redirect_action_; - } - uint32_t maxInternalRedirects() const override { return max_internal_redirects_; } // Router::DirectResponseEntry std::string newPath(const Http::RequestHeaderMap& headers) const override; @@ -548,6 +587,9 @@ class RouteEntryImplBase : public RouteEntry, Upstream::ResourcePriority priority() const override { return parent_->priority(); } const RateLimitPolicy& rateLimitPolicy() const override { return parent_->rateLimitPolicy(); } const RetryPolicy& retryPolicy() const override { return parent_->retryPolicy(); } + const InternalRedirectPolicy& internalRedirectPolicy() const override { + return parent_->internalRedirectPolicy(); + } uint32_t retryShadowBufferLimit() const override { return parent_->retryShadowBufferLimit(); } const std::vector& shadowPolicies() const override { return parent_->shadowPolicies(); @@ -602,10 +644,6 @@ class RouteEntryImplBase : public RouteEntry, return parent_->connectConfig(); } const UpgradeMap& upgradeMap() const override { return parent_->upgradeMap(); } - InternalRedirectAction internalRedirectAction() const override { - return parent_->internalRedirectAction(); - } - uint32_t maxInternalRedirects() const override { return parent_->maxInternalRedirects(); } // Router::Route const DirectResponseEntry* directResponseEntry() const override { return nullptr; } @@ -694,6 +732,11 @@ class RouteEntryImplBase : public RouteEntry, const envoy::config::route::v3::RouteAction& route_config, ProtobufMessage::ValidationVisitor& validation_visitor) const; + InternalRedirectPolicyImpl + buildInternalRedirectPolicy(const envoy::config::route::v3::RouteAction& route_config, + ProtobufMessage::ValidationVisitor& validator, + absl::string_view current_route_name) const; + // Default timeout is 15s if nothing is specified in the route config. static const uint64_t DEFAULT_ROUTE_TIMEOUT_MS = 15000; @@ -720,6 +763,7 @@ class RouteEntryImplBase : public RouteEntry, const bool strip_query_; const HedgePolicyImpl hedge_policy_; const RetryPolicyImpl retry_policy_; + const InternalRedirectPolicyImpl internal_redirect_policy_; const RateLimitPolicyImpl rate_limit_policy_; std::vector shadow_policies_; const Upstream::ResourcePriority priority_; @@ -749,8 +793,6 @@ class RouteEntryImplBase : public RouteEntry, PerFilterConfigs per_filter_configs_; const std::string route_name_; TimeSource& time_source_; - InternalRedirectAction internal_redirect_action_; - uint32_t max_internal_redirects_{1}; }; /** diff --git a/source/common/router/router.cc b/source/common/router/router.cc index 677244a7f9e2..516f5b403c1e 100644 --- a/source/common/router/router.cc +++ b/source/common/router/router.cc @@ -15,6 +15,7 @@ #include "envoy/upstream/upstream.h" #include "common/common/assert.h" +#include "common/common/cleanup.h" #include "common/common/empty_string.h" #include "common/common/enum_to_int.h" #include "common/common/scope_tracker.h" @@ -59,60 +60,6 @@ bool schemeIsHttp(const Http::RequestHeaderMap& downstream_headers, return false; } -bool convertRequestHeadersForInternalRedirect(Http::RequestHeaderMap& downstream_headers, - StreamInfo::FilterState& filter_state, - uint32_t max_internal_redirects, - const Http::HeaderEntry& internal_redirect, - const Network::Connection& connection) { - // Make sure the redirect response contains a URL to redirect to. - if (internal_redirect.value().getStringView().length() == 0) { - return false; - } - if (!downstream_headers.Path()) { - return false; - } - - Http::Utility::Url absolute_url; - if (!absolute_url.initialize(internal_redirect.value().getStringView(), false)) { - return false; - } - - // Don't allow serving TLS responses over plaintext. - bool scheme_is_http = schemeIsHttp(downstream_headers, connection); - if (scheme_is_http && absolute_url.scheme() == Http::Headers::get().SchemeValues.Https) { - return false; - } - - // Make sure that performing the redirect won't result in exceeding the configured number of - // redirects allowed for this route. - if (!filter_state.hasData(NumInternalRedirectsFilterStateName)) { - filter_state.setData( - NumInternalRedirectsFilterStateName, std::make_shared(0), - StreamInfo::FilterState::StateType::Mutable, StreamInfo::FilterState::LifeSpan::Request); - } - StreamInfo::UInt32Accessor& num_internal_redirect = - filter_state.getDataMutable(NumInternalRedirectsFilterStateName); - - if (num_internal_redirect.value() >= max_internal_redirects) { - return false; - } - num_internal_redirect.increment(); - - // Preserve the original request URL for the second pass. - downstream_headers.setEnvoyOriginalUrl( - absl::StrCat(scheme_is_http ? Http::Headers::get().SchemeValues.Http - : Http::Headers::get().SchemeValues.Https, - "://", downstream_headers.Host()->value().getStringView(), - downstream_headers.Path()->value().getStringView())); - - // Replace the original host, scheme and path. - downstream_headers.setScheme(absolute_url.scheme()); - downstream_headers.setHost(absolute_url.hostAndPort()); - downstream_headers.setPath(absolute_url.pathAndQueryParams()); - - return true; -} - constexpr uint64_t TimeoutPrecisionFactor = 100; Http::ConnectionPool::Instance* @@ -1305,8 +1252,9 @@ void Filter::onUpstreamHeaders(uint64_t response_code, Http::ResponseHeaderMapPt } } - if (static_cast(response_code) == Http::Code::Found && - route_entry_->internalRedirectAction() == InternalRedirectAction::Handle && + if (route_entry_->internalRedirectPolicy().enabled() && + route_entry_->internalRedirectPolicy().shouldRedirectForResponseCode( + static_cast(response_code)) && setupRedirect(*headers, upstream_request)) { return; // If the redirect could not be handled, fail open and let it pass to the @@ -1489,15 +1437,11 @@ bool Filter::setupRedirect(const Http::ResponseHeaderMap& headers, attempting_internal_redirect_with_complete_stream_ = upstream_request.upstreamTiming().last_upstream_rx_byte_received_ && downstream_end_stream_; - const StreamInfo::FilterStateSharedPtr& filter_state = callbacks_->streamInfo().filterState(); - // Redirects are not supported for streaming requests yet. if (downstream_end_stream_ && !callbacks_->decodingBuffer() && // Redirects with body not yet supported. location != nullptr && - convertRequestHeadersForInternalRedirect(*downstream_headers_, *filter_state, - route_entry_->maxInternalRedirects(), *location, - *callbacks_->connection()) && + convertRequestHeadersForInternalRedirect(*downstream_headers_, *location) && callbacks_->recreateStream()) { cluster_->stats().upstream_internal_redirect_succeeded_total_.inc(); return true; @@ -1510,6 +1454,95 @@ bool Filter::setupRedirect(const Http::ResponseHeaderMap& headers, return false; } +bool Filter::convertRequestHeadersForInternalRedirect(Http::RequestHeaderMap& downstream_headers, + const Http::HeaderEntry& internal_redirect) { + if (!downstream_headers.Path()) { + ENVOY_STREAM_LOG(trace, "no path in downstream_headers", *callbacks_); + return false; + } + + // Make sure the redirect response contains a URL to redirect to. + if (internal_redirect.value().getStringView().length() == 0) { + config_.stats_.passthrough_internal_redirect_bad_location_.inc(); + return false; + } + Http::Utility::Url absolute_url; + if (!absolute_url.initialize(internal_redirect.value().getStringView(), false)) { + config_.stats_.passthrough_internal_redirect_bad_location_.inc(); + return false; + } + + const auto& policy = route_entry_->internalRedirectPolicy(); + // Don't allow serving TLS responses over plaintext unless allowed by policy. + const bool scheme_is_http = schemeIsHttp(downstream_headers, *callbacks_->connection()); + const bool target_is_http = absolute_url.scheme() == Http::Headers::get().SchemeValues.Http; + if (!policy.isCrossSchemeRedirectAllowed() && scheme_is_http != target_is_http) { + config_.stats_.passthrough_internal_redirect_unsafe_scheme_.inc(); + return false; + } + + const StreamInfo::FilterStateSharedPtr& filter_state = callbacks_->streamInfo().filterState(); + // Make sure that performing the redirect won't result in exceeding the configured number of + // redirects allowed for this route. + if (!filter_state->hasData(NumInternalRedirectsFilterStateName)) { + filter_state->setData( + NumInternalRedirectsFilterStateName, std::make_shared(0), + StreamInfo::FilterState::StateType::Mutable, StreamInfo::FilterState::LifeSpan::Request); + } + StreamInfo::UInt32Accessor& num_internal_redirect = + filter_state->getDataMutable(NumInternalRedirectsFilterStateName); + + if (num_internal_redirect.value() >= policy.maxInternalRedirects()) { + config_.stats_.passthrough_internal_redirect_too_many_redirects_.inc(); + return false; + } + std::string original_host(downstream_headers.Host()->value().getStringView()); + std::string original_path(downstream_headers.Path()->value().getStringView()); + const bool scheme_is_set = (downstream_headers.Scheme() != nullptr); + Cleanup restore_original_headers( + [&downstream_headers, original_host, original_path, scheme_is_set, scheme_is_http]() { + downstream_headers.setHost(original_host); + downstream_headers.setPath(original_path); + if (scheme_is_set) { + downstream_headers.setScheme(scheme_is_http ? Http::Headers::get().SchemeValues.Http + : Http::Headers::get().SchemeValues.Https); + } + }); + + // Replace the original host, scheme and path. + downstream_headers.setScheme(absolute_url.scheme()); + downstream_headers.setHost(absolute_url.hostAndPort()); + downstream_headers.setPath(absolute_url.pathAndQueryParams()); + + callbacks_->clearRouteCache(); + const auto route = callbacks_->route(); + // Don't allow a redirect to a non existing route. + if (!route) { + config_.stats_.passthrough_internal_redirect_no_route_.inc(); + return false; + } + + const auto& route_name = route->routeEntry()->routeName(); + for (const auto& predicate : policy.predicates()) { + if (!predicate->acceptTargetRoute(*filter_state, route_name, !scheme_is_http, + !target_is_http)) { + config_.stats_.passthrough_internal_redirect_predicate_.inc(); + ENVOY_STREAM_LOG(trace, "rejecting redirect targeting {}, by {} predicate", *callbacks_, + route_name, predicate->name()); + return false; + } + } + + num_internal_redirect.increment(); + restore_original_headers.cancel(); + // Preserve the original request URL for the second pass. + downstream_headers.setEnvoyOriginalUrl(absl::StrCat(scheme_is_http + ? Http::Headers::get().SchemeValues.Http + : Http::Headers::get().SchemeValues.Https, + "://", original_host, original_path)); + return true; +} + void Filter::doRetry() { ENVOY_STREAM_LOG(debug, "performing retry", *callbacks_); diff --git a/source/common/router/router.h b/source/common/router/router.h index 83ba6dee85d3..682aaed92ca6 100644 --- a/source/common/router/router.h +++ b/source/common/router/router.h @@ -41,6 +41,11 @@ namespace Router { */ // clang-format off #define ALL_ROUTER_STATS(COUNTER) \ + COUNTER(passthrough_internal_redirect_bad_location) \ + COUNTER(passthrough_internal_redirect_unsafe_scheme) \ + COUNTER(passthrough_internal_redirect_too_many_redirects) \ + COUNTER(passthrough_internal_redirect_no_route) \ + COUNTER(passthrough_internal_redirect_predicate) \ COUNTER(no_route) \ COUNTER(no_cluster) \ COUNTER(rq_redirect) \ @@ -502,6 +507,8 @@ class Filter : Logger::Loggable, void resetOtherUpstreams(UpstreamRequest& upstream_request); void sendNoHealthyUpstreamResponse(); bool setupRedirect(const Http::ResponseHeaderMap& headers, UpstreamRequest& upstream_request); + bool convertRequestHeadersForInternalRedirect(Http::RequestHeaderMap& downstream_headers, + const Http::HeaderEntry& internal_redirect); void updateOutlierDetection(Upstream::Outlier::Result result, UpstreamRequest& upstream_request, absl::optional code); void doRetry(); diff --git a/source/extensions/extensions_build_config.bzl b/source/extensions/extensions_build_config.bzl index 434a74d949ce..9bd2531dcdb7 100644 --- a/source/extensions/extensions_build_config.bzl +++ b/source/extensions/extensions_build_config.bzl @@ -180,4 +180,11 @@ EXTENSIONS = { # "envoy.filters.http.cache.simple_http_cache": "//source/extensions/filters/http/cache/simple_http_cache:simple_http_cache_lib", + + # + # Internal redirect predicates + # + "envoy.internal_redirect_predicates.allow_listed_routes": "//source/extensions/internal_redirect/allow_listed_routes:config", + "envoy.internal_redirect_predicates.previous_routes": "//source/extensions/internal_redirect/previous_routes:config", + "envoy.internal_redirect_predicates.safe_cross_scheme": "//source/extensions/internal_redirect/safe_cross_scheme:config", } diff --git a/source/extensions/internal_redirect/BUILD b/source/extensions/internal_redirect/BUILD new file mode 100644 index 000000000000..6156949edef6 --- /dev/null +++ b/source/extensions/internal_redirect/BUILD @@ -0,0 +1,17 @@ +licenses(["notice"]) # Apache 2 + +load( + "//bazel:envoy_build_system.bzl", + "envoy_cc_library", + "envoy_package", +) + +envoy_package() + +envoy_cc_library( + name = "well_known_names", + hdrs = ["well_known_names.h"], + deps = [ + "//source/common/singleton:const_singleton", + ], +) diff --git a/source/extensions/internal_redirect/allow_listed_routes/BUILD b/source/extensions/internal_redirect/allow_listed_routes/BUILD new file mode 100644 index 000000000000..02cf2789dc79 --- /dev/null +++ b/source/extensions/internal_redirect/allow_listed_routes/BUILD @@ -0,0 +1,35 @@ +licenses(["notice"]) # Apache 2 + +load( + "//bazel:envoy_build_system.bzl", + "envoy_cc_extension", + "envoy_cc_library", + "envoy_package", +) + +envoy_package() + +envoy_cc_library( + name = "allow_listed_routes_lib", + hdrs = ["allow_listed_routes.h"], + deps = [ + "//include/envoy/router:internal_redirect_interface", + "//include/envoy/stream_info:filter_state_interface", + "//source/extensions/internal_redirect:well_known_names", + "@envoy_api//envoy/extensions/internal_redirect/allow_listed_routes/v3:pkg_cc_proto", + ], +) + +envoy_cc_extension( + name = "config", + srcs = ["config.cc"], + hdrs = ["config.h"], + security_posture = "robust_to_untrusted_downstream_and_upstream", + deps = [ + ":allow_listed_routes_lib", + "//include/envoy/registry", + "//include/envoy/router:internal_redirect_interface", + "//source/extensions/internal_redirect:well_known_names", + "@envoy_api//envoy/extensions/internal_redirect/allow_listed_routes/v3:pkg_cc_proto", + ], +) diff --git a/source/extensions/internal_redirect/allow_listed_routes/allow_listed_routes.h b/source/extensions/internal_redirect/allow_listed_routes/allow_listed_routes.h new file mode 100644 index 000000000000..72d8d605db0f --- /dev/null +++ b/source/extensions/internal_redirect/allow_listed_routes/allow_listed_routes.h @@ -0,0 +1,37 @@ +#pragma once + +#include "envoy/extensions/internal_redirect/allow_listed_routes/v3/allow_listed_routes_config.pb.h" +#include "envoy/router/internal_redirect.h" +#include "envoy/stream_info/filter_state.h" + +#include "extensions/internal_redirect/well_known_names.h" + +#include "absl/container/flat_hash_set.h" +#include "absl/strings/string_view.h" + +namespace Envoy { +namespace Extensions { +namespace InternalRedirect { + +class AllowListedRoutesPredicate : public Router::InternalRedirectPredicate { +public: + explicit AllowListedRoutesPredicate( + const envoy::extensions::internal_redirect::allow_listed_routes::v3::AllowListedRoutesConfig& + config) + : allowed_routes_(config.allowed_route_names().begin(), config.allowed_route_names().end()) {} + + bool acceptTargetRoute(StreamInfo::FilterState&, absl::string_view route_name, bool, + bool) override { + return allowed_routes_.contains(route_name); + } + + absl::string_view name() const override { + return InternalRedirectPredicateValues::get().AllowListedRoutesPredicate; + } + + const absl::flat_hash_set allowed_routes_; +}; + +} // namespace InternalRedirect +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/internal_redirect/allow_listed_routes/config.cc b/source/extensions/internal_redirect/allow_listed_routes/config.cc new file mode 100644 index 000000000000..55c2d5af81ce --- /dev/null +++ b/source/extensions/internal_redirect/allow_listed_routes/config.cc @@ -0,0 +1,14 @@ +#include "extensions/internal_redirect/allow_listed_routes/config.h" + +#include "envoy/registry/registry.h" +#include "envoy/router/internal_redirect.h" + +namespace Envoy { +namespace Extensions { +namespace InternalRedirect { + +REGISTER_FACTORY(AllowListedRoutesPredicateFactory, Router::InternalRedirectPredicateFactory); + +} // namespace InternalRedirect +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/internal_redirect/allow_listed_routes/config.h b/source/extensions/internal_redirect/allow_listed_routes/config.h new file mode 100644 index 000000000000..1a122f4f31b6 --- /dev/null +++ b/source/extensions/internal_redirect/allow_listed_routes/config.h @@ -0,0 +1,40 @@ +#pragma once + +#include "envoy/extensions/internal_redirect/allow_listed_routes/v3/allow_listed_routes_config.pb.h" +#include "envoy/extensions/internal_redirect/allow_listed_routes/v3/allow_listed_routes_config.pb.validate.h" +#include "envoy/router/internal_redirect.h" + +#include "common/protobuf/message_validator_impl.h" +#include "common/protobuf/utility.h" + +#include "extensions/internal_redirect/allow_listed_routes/allow_listed_routes.h" +#include "extensions/internal_redirect/well_known_names.h" + +namespace Envoy { +namespace Extensions { +namespace InternalRedirect { + +class AllowListedRoutesPredicateFactory : public Router::InternalRedirectPredicateFactory { +public: + Router::InternalRedirectPredicateSharedPtr + createInternalRedirectPredicate(const Protobuf::Message& config, absl::string_view) override { + auto allow_listed_routes_config = + MessageUtil::downcastAndValidate( + config, ProtobufMessage::getStrictValidationVisitor()); + return std::make_shared(allow_listed_routes_config); + } + + std::string name() const override { + return InternalRedirectPredicateValues::get().AllowListedRoutesPredicate; + } + + ProtobufTypes::MessagePtr createEmptyConfigProto() override { + return std::make_unique< + envoy::extensions::internal_redirect::allow_listed_routes::v3::AllowListedRoutesConfig>(); + } +}; + +} // namespace InternalRedirect +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/internal_redirect/previous_routes/BUILD b/source/extensions/internal_redirect/previous_routes/BUILD new file mode 100644 index 000000000000..d022a4c6719c --- /dev/null +++ b/source/extensions/internal_redirect/previous_routes/BUILD @@ -0,0 +1,35 @@ +licenses(["notice"]) # Apache 2 + +load( + "//bazel:envoy_build_system.bzl", + "envoy_cc_extension", + "envoy_cc_library", + "envoy_package", +) + +envoy_package() + +envoy_cc_library( + name = "previous_routes_lib", + srcs = ["previous_routes.cc"], + hdrs = ["previous_routes.h"], + deps = [ + "//include/envoy/router:internal_redirect_interface", + "//include/envoy/stream_info:filter_state_interface", + "//source/extensions/internal_redirect:well_known_names", + ], +) + +envoy_cc_extension( + name = "config", + srcs = ["config.cc"], + hdrs = ["config.h"], + security_posture = "robust_to_untrusted_downstream_and_upstream", + deps = [ + ":previous_routes_lib", + "//include/envoy/registry", + "//include/envoy/router:internal_redirect_interface", + "//source/extensions/internal_redirect:well_known_names", + "@envoy_api//envoy/extensions/internal_redirect/previous_routes/v3:pkg_cc_proto", + ], +) diff --git a/source/extensions/internal_redirect/previous_routes/config.cc b/source/extensions/internal_redirect/previous_routes/config.cc new file mode 100644 index 000000000000..d5d4b67c491e --- /dev/null +++ b/source/extensions/internal_redirect/previous_routes/config.cc @@ -0,0 +1,14 @@ +#include "extensions/internal_redirect/previous_routes/config.h" + +#include "envoy/registry/registry.h" +#include "envoy/router/internal_redirect.h" + +namespace Envoy { +namespace Extensions { +namespace InternalRedirect { + +REGISTER_FACTORY(PreviousRoutesPredicateFactory, Router::InternalRedirectPredicateFactory); + +} // namespace InternalRedirect +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/internal_redirect/previous_routes/config.h b/source/extensions/internal_redirect/previous_routes/config.h new file mode 100644 index 000000000000..21ccb3c1646b --- /dev/null +++ b/source/extensions/internal_redirect/previous_routes/config.h @@ -0,0 +1,34 @@ +#pragma once + +#include "envoy/extensions/internal_redirect/previous_routes/v3/previous_routes_config.pb.h" +#include "envoy/extensions/internal_redirect/previous_routes/v3/previous_routes_config.pb.validate.h" +#include "envoy/router/internal_redirect.h" + +#include "extensions/internal_redirect/previous_routes/previous_routes.h" +#include "extensions/internal_redirect/well_known_names.h" + +namespace Envoy { +namespace Extensions { +namespace InternalRedirect { + +class PreviousRoutesPredicateFactory : public Router::InternalRedirectPredicateFactory { +public: + Router::InternalRedirectPredicateSharedPtr + createInternalRedirectPredicate(const Protobuf::Message&, + absl::string_view current_route_name) override { + return std::make_shared(current_route_name); + } + + std::string name() const override { + return InternalRedirectPredicateValues::get().PreviousRoutesPredicate; + } + + ProtobufTypes::MessagePtr createEmptyConfigProto() override { + return std::make_unique< + envoy::extensions::internal_redirect::previous_routes::v3::PreviousRoutesConfig>(); + } +}; + +} // namespace InternalRedirect +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/internal_redirect/previous_routes/previous_routes.cc b/source/extensions/internal_redirect/previous_routes/previous_routes.cc new file mode 100644 index 000000000000..a29187e29d43 --- /dev/null +++ b/source/extensions/internal_redirect/previous_routes/previous_routes.cc @@ -0,0 +1,52 @@ +#include "extensions/internal_redirect/previous_routes/previous_routes.h" + +#include "envoy/router/internal_redirect.h" +#include "envoy/stream_info/filter_state.h" + +#include "absl/container/flat_hash_set.h" + +namespace Envoy { +namespace Extensions { +namespace InternalRedirect { + +namespace { + +constexpr absl::string_view PreviousRoutesPredicateStateNamePrefix = + "envoy.internal_redirect.previous_routes_predicate_state"; + +class PreviousRoutesPredicateState : public StreamInfo::FilterState::Object { +public: + PreviousRoutesPredicateState() = default; + // Disallow copy so that we don't accidentally take a copy of the state + // through FilterState::getDataMutable, which will cause confusing bug that + // states are not updated in the original copy. + PreviousRoutesPredicateState(const PreviousRoutesPredicateState&) = delete; + PreviousRoutesPredicateState& operator=(const PreviousRoutesPredicateState&) = delete; + + bool insertRouteIfNotPresent(absl::string_view route) { + return previous_routes_.insert(std::string(route)).second; + } + +private: + absl::flat_hash_set previous_routes_; +}; + +} // namespace + +bool PreviousRoutesPredicate::acceptTargetRoute(StreamInfo::FilterState& filter_state, + absl::string_view route_name, bool, bool) { + auto filter_state_name = + absl::StrCat(PreviousRoutesPredicateStateNamePrefix, ".", current_route_name_); + if (!filter_state.hasData(filter_state_name)) { + filter_state.setData(filter_state_name, std::make_unique(), + StreamInfo::FilterState::StateType::Mutable, + StreamInfo::FilterState::LifeSpan::Request); + } + auto& predicate_state = + filter_state.getDataMutable(filter_state_name); + return predicate_state.insertRouteIfNotPresent(route_name); +} + +} // namespace InternalRedirect +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/internal_redirect/previous_routes/previous_routes.h b/source/extensions/internal_redirect/previous_routes/previous_routes.h new file mode 100644 index 000000000000..b79f4f8b1754 --- /dev/null +++ b/source/extensions/internal_redirect/previous_routes/previous_routes.h @@ -0,0 +1,32 @@ +#pragma once + +#include "envoy/router/internal_redirect.h" +#include "envoy/stream_info/filter_state.h" + +#include "extensions/internal_redirect/well_known_names.h" + +#include "absl/strings/string_view.h" + +namespace Envoy { +namespace Extensions { +namespace InternalRedirect { + +class PreviousRoutesPredicate : public Router::InternalRedirectPredicate { +public: + explicit PreviousRoutesPredicate(absl::string_view current_route_name) + : current_route_name_(current_route_name) {} + + bool acceptTargetRoute(StreamInfo::FilterState& filter_state, absl::string_view route_name, bool, + bool) override; + + absl::string_view name() const override { + return InternalRedirectPredicateValues::get().PreviousRoutesPredicate; + } + +private: + const std::string current_route_name_; +}; + +} // namespace InternalRedirect +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/internal_redirect/safe_cross_scheme/BUILD b/source/extensions/internal_redirect/safe_cross_scheme/BUILD new file mode 100644 index 000000000000..94293850b53b --- /dev/null +++ b/source/extensions/internal_redirect/safe_cross_scheme/BUILD @@ -0,0 +1,34 @@ +licenses(["notice"]) # Apache 2 + +load( + "//bazel:envoy_build_system.bzl", + "envoy_cc_extension", + "envoy_cc_library", + "envoy_package", +) + +envoy_package() + +envoy_cc_library( + name = "safe_cross_scheme_lib", + hdrs = ["safe_cross_scheme.h"], + deps = [ + "//include/envoy/router:internal_redirect_interface", + "//include/envoy/stream_info:filter_state_interface", + "//source/extensions/internal_redirect:well_known_names", + ], +) + +envoy_cc_extension( + name = "config", + srcs = ["config.cc"], + hdrs = ["config.h"], + security_posture = "robust_to_untrusted_downstream_and_upstream", + deps = [ + ":safe_cross_scheme_lib", + "//include/envoy/registry", + "//include/envoy/router:internal_redirect_interface", + "//source/extensions/internal_redirect:well_known_names", + "@envoy_api//envoy/extensions/internal_redirect/safe_cross_scheme/v3:pkg_cc_proto", + ], +) diff --git a/source/extensions/internal_redirect/safe_cross_scheme/config.cc b/source/extensions/internal_redirect/safe_cross_scheme/config.cc new file mode 100644 index 000000000000..43b7664fd7ff --- /dev/null +++ b/source/extensions/internal_redirect/safe_cross_scheme/config.cc @@ -0,0 +1,14 @@ +#include "extensions/internal_redirect/safe_cross_scheme/config.h" + +#include "envoy/registry/registry.h" +#include "envoy/router/internal_redirect.h" + +namespace Envoy { +namespace Extensions { +namespace InternalRedirect { + +REGISTER_FACTORY(SafeCrossSchemePredicateFactory, Router::InternalRedirectPredicateFactory); + +} // namespace InternalRedirect +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/internal_redirect/safe_cross_scheme/config.h b/source/extensions/internal_redirect/safe_cross_scheme/config.h new file mode 100644 index 000000000000..49a8fdfa8b69 --- /dev/null +++ b/source/extensions/internal_redirect/safe_cross_scheme/config.h @@ -0,0 +1,32 @@ +#pragma once + +#include "envoy/extensions/internal_redirect/safe_cross_scheme/v3/safe_cross_scheme_config.pb.h" +#include "envoy/router/internal_redirect.h" + +#include "extensions/internal_redirect/safe_cross_scheme/safe_cross_scheme.h" +#include "extensions/internal_redirect/well_known_names.h" + +namespace Envoy { +namespace Extensions { +namespace InternalRedirect { + +class SafeCrossSchemePredicateFactory : public Router::InternalRedirectPredicateFactory { +public: + Router::InternalRedirectPredicateSharedPtr + createInternalRedirectPredicate(const Protobuf::Message&, absl::string_view) override { + return std::make_shared(); + } + + std::string name() const override { + return InternalRedirectPredicateValues::get().SafeCrossSchemePredicate; + } + + ProtobufTypes::MessagePtr createEmptyConfigProto() override { + return std::make_unique< + envoy::extensions::internal_redirect::safe_cross_scheme::v3::SafeCrossSchemeConfig>(); + } +}; + +} // namespace InternalRedirect +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/internal_redirect/safe_cross_scheme/safe_cross_scheme.h b/source/extensions/internal_redirect/safe_cross_scheme/safe_cross_scheme.h new file mode 100644 index 000000000000..fb33e58b6fdd --- /dev/null +++ b/source/extensions/internal_redirect/safe_cross_scheme/safe_cross_scheme.h @@ -0,0 +1,28 @@ +#pragma once + +#include "envoy/router/internal_redirect.h" +#include "envoy/stream_info/filter_state.h" + +#include "extensions/internal_redirect/well_known_names.h" + +#include "absl/strings/string_view.h" + +namespace Envoy { +namespace Extensions { +namespace InternalRedirect { + +class SafeCrossSchemePredicate : public Router::InternalRedirectPredicate { +public: + bool acceptTargetRoute(StreamInfo::FilterState&, absl::string_view, bool downstream_is_https, + bool target_is_https) override { + return downstream_is_https || !target_is_https; + } + + absl::string_view name() const override { + return InternalRedirectPredicateValues::get().SafeCrossSchemePredicate; + } +}; + +} // namespace InternalRedirect +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/internal_redirect/well_known_names.h b/source/extensions/internal_redirect/well_known_names.h new file mode 100644 index 000000000000..003e270329d6 --- /dev/null +++ b/source/extensions/internal_redirect/well_known_names.h @@ -0,0 +1,27 @@ +#pragma once + +#include + +#include "common/singleton/const_singleton.h" + +namespace Envoy { +namespace Extensions { +namespace InternalRedirect { + +/** + * Well-known internal redirect predicate names. + */ +class InternalRedirectPredicatesNameValues { +public: + const std::string AllowListedRoutesPredicate = + "envoy.internal_redirect_predicates.allow_listed_routes"; + const std::string PreviousRoutesPredicate = "envoy.internal_redirect_predicates.previous_routes"; + const std::string SafeCrossSchemePredicate = + "envoy.internal_redirect_predicates.safe_cross_scheme"; +}; + +using InternalRedirectPredicateValues = ConstSingleton; + +} // namespace InternalRedirect +} // namespace Extensions +} // namespace Envoy diff --git a/test/common/http/async_client_impl_test.cc b/test/common/http/async_client_impl_test.cc index 66f8e3f3e94d..e21c496e89a5 100644 --- a/test/common/http/async_client_impl_test.cc +++ b/test/common/http/async_client_impl_test.cc @@ -1400,8 +1400,7 @@ TEST_F(AsyncClientImplUnitTest, RouteImplInitTest) { route_impl_.routeEntry()->typedMetadata().get("bar")); EXPECT_EQ(nullptr, route_impl_.routeEntry()->perFilterConfig("bar")); EXPECT_TRUE(route_impl_.routeEntry()->upgradeMap().empty()); - EXPECT_EQ(Router::InternalRedirectAction::PassThrough, - route_impl_.routeEntry()->internalRedirectAction()); + EXPECT_EQ(false, route_impl_.routeEntry()->internalRedirectPolicy().enabled()); EXPECT_TRUE(route_impl_.routeEntry()->shadowPolicies().empty()); EXPECT_TRUE(route_impl_.routeEntry()->virtualHost().rateLimitPolicy().empty()); EXPECT_EQ(nullptr, route_impl_.routeEntry()->virtualHost().corsPolicy()); diff --git a/test/common/router/config_impl_test.cc b/test/common/router/config_impl_test.cc index dc7f9bc53b4a..a48a9174f9e8 100644 --- a/test/common/router/config_impl_test.cc +++ b/test/common/router/config_impl_test.cc @@ -2739,8 +2739,7 @@ TEST_F(RouteMatcherTest, ClusterHeader) { route->routeEntry()->maxGrpcTimeout(); route->routeEntry()->grpcTimeoutOffset(); route->routeEntry()->upgradeMap(); - route->routeEntry()->internalRedirectAction(); - route->routeEntry()->maxInternalRedirects(); + route->routeEntry()->internalRedirectPolicy(); } } @@ -6886,6 +6885,130 @@ name: RetriableStatusCodes EXPECT_NE(predicates1, predicates2); } +TEST_F(RouteConfigurationV2, InternalRedirctIsDisabledWhenNotSpecifiedInRouteAction) { + const std::string InternalRedirectEnabled = R"EOF( +name: InternalRedirectEnabled +virtual_hosts: + - name: regex + domains: [idle.lyft.com] + routes: + - match: + safe_regex: + google_re2: {} + regex: "/regex" + route: + cluster: some-cluster + )EOF"; + + TestConfigImpl config(parseRouteConfigurationFromV2Yaml(InternalRedirectEnabled), + factory_context_, true); + Http::TestRequestHeaderMapImpl headers = + genRedirectHeaders("idle.lyft.com", "/regex", true, false); + const auto& internal_redirect_policy = + config.route(headers, 0)->routeEntry()->internalRedirectPolicy(); + EXPECT_FALSE(internal_redirect_policy.enabled()); +} + +TEST_F(RouteConfigurationV2, DefaultInternalRedirctPolicyIsSensible) { + const std::string InternalRedirectEnabled = R"EOF( +name: InternalRedirectEnabled +virtual_hosts: + - name: regex + domains: [idle.lyft.com] + routes: + - match: + safe_regex: + google_re2: {} + regex: "/regex" + route: + cluster: some-cluster + internal_redirect_policy: {} + )EOF"; + + TestConfigImpl config(parseRouteConfigurationFromV2Yaml(InternalRedirectEnabled), + factory_context_, true); + Http::TestRequestHeaderMapImpl headers = + genRedirectHeaders("idle.lyft.com", "/regex", true, false); + const auto& internal_redirect_policy = + config.route(headers, 0)->routeEntry()->internalRedirectPolicy(); + EXPECT_TRUE(internal_redirect_policy.enabled()); + EXPECT_TRUE(internal_redirect_policy.shouldRedirectForResponseCode(static_cast(302))); + EXPECT_FALSE( + internal_redirect_policy.shouldRedirectForResponseCode(static_cast(200))); + EXPECT_EQ(1, internal_redirect_policy.maxInternalRedirects()); + EXPECT_TRUE(internal_redirect_policy.predicates().empty()); + EXPECT_FALSE(internal_redirect_policy.isCrossSchemeRedirectAllowed()); +} + +TEST_F(RouteConfigurationV2, InternalRedirctPolicyDropsInvalidRedirectCode) { + const std::string InternalRedirectEnabled = R"EOF( +name: InternalRedirectEnabled +virtual_hosts: + - name: regex + domains: [idle.lyft.com] + routes: + - match: + safe_regex: + google_re2: {} + regex: "/regex" + route: + cluster: some-cluster + internal_redirect_policy: + redirect_response_codes: [301, 302, 303, 304] + )EOF"; + + TestConfigImpl config(parseRouteConfigurationFromV2Yaml(InternalRedirectEnabled), + factory_context_, true); + Http::TestRequestHeaderMapImpl headers = + genRedirectHeaders("idle.lyft.com", "/regex", true, false); + const auto& internal_redirect_policy = + config.route(headers, 0)->routeEntry()->internalRedirectPolicy(); + EXPECT_TRUE(internal_redirect_policy.enabled()); + EXPECT_TRUE(internal_redirect_policy.shouldRedirectForResponseCode(static_cast(301))); + EXPECT_TRUE(internal_redirect_policy.shouldRedirectForResponseCode(static_cast(302))); + EXPECT_TRUE(internal_redirect_policy.shouldRedirectForResponseCode(static_cast(303))); + EXPECT_FALSE( + internal_redirect_policy.shouldRedirectForResponseCode(static_cast(304))); + EXPECT_FALSE( + internal_redirect_policy.shouldRedirectForResponseCode(static_cast(305))); + EXPECT_FALSE( + internal_redirect_policy.shouldRedirectForResponseCode(static_cast(306))); + EXPECT_FALSE( + internal_redirect_policy.shouldRedirectForResponseCode(static_cast(307))); +} + +TEST_F(RouteConfigurationV2, InternalRedirctPolicyDropsInvalidRedirectCodeCauseEmptySet) { + const std::string InternalRedirectEnabled = R"EOF( +name: InternalRedirectEnabled +virtual_hosts: + - name: regex + domains: [idle.lyft.com] + routes: + - match: + safe_regex: + google_re2: {} + regex: "/regex" + route: + cluster: some-cluster + internal_redirect_policy: + redirect_response_codes: [200, 304] + )EOF"; + + TestConfigImpl config(parseRouteConfigurationFromV2Yaml(InternalRedirectEnabled), + factory_context_, true); + Http::TestRequestHeaderMapImpl headers = + genRedirectHeaders("idle.lyft.com", "/regex", true, false); + const auto& internal_redirect_policy = + config.route(headers, 0)->routeEntry()->internalRedirectPolicy(); + EXPECT_TRUE(internal_redirect_policy.enabled()); + EXPECT_FALSE( + internal_redirect_policy.shouldRedirectForResponseCode(static_cast(302))); + EXPECT_FALSE( + internal_redirect_policy.shouldRedirectForResponseCode(static_cast(304))); + EXPECT_FALSE( + internal_redirect_policy.shouldRedirectForResponseCode(static_cast(200))); +} + class PerFilterConfigsTest : public testing::Test, public ConfigImplTestBase { public: PerFilterConfigsTest() diff --git a/test/common/router/router_test.cc b/test/common/router/router_test.cc index 058f7a580d00..ff40ce748fe4 100644 --- a/test/common/router/router_test.cc +++ b/test/common/router/router_test.cc @@ -294,16 +294,18 @@ class RouterTestBase : public testing::Test { router_.decodeHeaders(default_request_headers_, end_stream); } - void enableRedirects() { - ON_CALL(callbacks_.route_->route_entry_, internalRedirectAction()) - .WillByDefault(Return(InternalRedirectAction::Handle)); - ON_CALL(callbacks_, connection()).WillByDefault(Return(&connection_)); - setMaxInternalRedirects(1); - } - - void setMaxInternalRedirects(uint32_t max_internal_redirects) { - ON_CALL(callbacks_.route_->route_entry_, maxInternalRedirects()) + void enableRedirects(uint32_t max_internal_redirects = 1) { + ON_CALL(callbacks_.route_->route_entry_.internal_redirect_policy_, enabled()) + .WillByDefault(Return(true)); + ON_CALL(callbacks_.route_->route_entry_.internal_redirect_policy_, + shouldRedirectForResponseCode(_)) + .WillByDefault(Return(true)); + ON_CALL(callbacks_.route_->route_entry_.internal_redirect_policy_, maxInternalRedirects()) .WillByDefault(Return(max_internal_redirects)); + ON_CALL(callbacks_.route_->route_entry_.internal_redirect_policy_, + isCrossSchemeRedirectAllowed()) + .WillByDefault(Return(false)); + ON_CALL(callbacks_, connection()).WillByDefault(Return(&connection_)); } void setNumPreviousRedirect(uint32_t num_previous_redirects) { @@ -4433,11 +4435,12 @@ TEST_F(RouterTest, RetryRespectsRetryHostPredicate) { } TEST_F(RouterTest, InternalRedirectRejectedWhenReachingMaxInternalRedirect) { - enableRedirects(); - setMaxInternalRedirects(3); + enableRedirects(3); setNumPreviousRedirect(3); sendRequest(); + EXPECT_CALL(callbacks_, recreateStream()).Times(0); + response_decoder_->decodeHeaders(std::move(redirect_headers_), false); Buffer::OwnedImpl data("1234567890"); @@ -4445,6 +4448,8 @@ TEST_F(RouterTest, InternalRedirectRejectedWhenReachingMaxInternalRedirect) { EXPECT_EQ(1U, cm_.thread_local_cluster_.cluster_.info_->stats_store_ .counter("upstream_internal_redirect_failed_total") .value()); + EXPECT_EQ(1UL, + stats_store_.counter("test.passthrough_internal_redirect_too_many_redirects").value()); } TEST_F(RouterTest, InternalRedirectRejectedWithEmptyLocation) { @@ -4452,6 +4457,9 @@ TEST_F(RouterTest, InternalRedirectRejectedWithEmptyLocation) { sendRequest(); redirect_headers_->setLocation(""); + + EXPECT_CALL(callbacks_, recreateStream()).Times(0); + response_decoder_->decodeHeaders(std::move(redirect_headers_), false); Buffer::OwnedImpl data("1234567890"); @@ -4459,6 +4467,7 @@ TEST_F(RouterTest, InternalRedirectRejectedWithEmptyLocation) { EXPECT_EQ(1U, cm_.thread_local_cluster_.cluster_.info_->stats_store_ .counter("upstream_internal_redirect_failed_total") .value()); + EXPECT_EQ(1UL, stats_store_.counter("test.passthrough_internal_redirect_bad_location").value()); } TEST_F(RouterTest, InternalRedirectRejectedWithInvalidLocation) { @@ -4466,6 +4475,9 @@ TEST_F(RouterTest, InternalRedirectRejectedWithInvalidLocation) { sendRequest(); redirect_headers_->setLocation("h"); + + EXPECT_CALL(callbacks_, recreateStream()).Times(0); + response_decoder_->decodeHeaders(std::move(redirect_headers_), false); Buffer::OwnedImpl data("1234567890"); @@ -4473,6 +4485,7 @@ TEST_F(RouterTest, InternalRedirectRejectedWithInvalidLocation) { EXPECT_EQ(1U, cm_.thread_local_cluster_.cluster_.info_->stats_store_ .counter("upstream_internal_redirect_failed_total") .value()); + EXPECT_EQ(1UL, stats_store_.counter("test.passthrough_internal_redirect_bad_location").value()); } TEST_F(RouterTest, InternalRedirectRejectedWithoutCompleteRequest) { @@ -4480,6 +4493,8 @@ TEST_F(RouterTest, InternalRedirectRejectedWithoutCompleteRequest) { sendRequest(false); + EXPECT_CALL(callbacks_, recreateStream()).Times(0); + response_decoder_->decodeHeaders(std::move(redirect_headers_), false); Buffer::OwnedImpl data("1234567890"); @@ -4495,6 +4510,9 @@ TEST_F(RouterTest, InternalRedirectRejectedWithoutLocation) { sendRequest(); redirect_headers_->removeLocation(); + + EXPECT_CALL(callbacks_, recreateStream()).Times(0); + response_decoder_->decodeHeaders(std::move(redirect_headers_), false); Buffer::OwnedImpl data("1234567890"); response_decoder_->decodeData(data, true); @@ -4508,7 +4526,10 @@ TEST_F(RouterTest, InternalRedirectRejectedWithBody) { sendRequest(); - EXPECT_CALL(callbacks_, decodingBuffer()).Times(1); + Buffer::InstancePtr body_data(new Buffer::OwnedImpl("random_fake_data")); + EXPECT_CALL(callbacks_, decodingBuffer()).WillOnce(Return(body_data.get())); + EXPECT_CALL(callbacks_, recreateStream()).Times(0); + response_decoder_->decodeHeaders(std::move(redirect_headers_), false); Buffer::OwnedImpl data("1234567890"); response_decoder_->decodeData(data, true); @@ -4517,26 +4538,61 @@ TEST_F(RouterTest, InternalRedirectRejectedWithBody) { .value()); } -TEST_F(RouterTest, InternalRedirectRejectedWithCrossSchemeRedirect) { +TEST_F(RouterTest, CrossSchemeRedirectRejectedByPolicy) { enableRedirects(); sendRequest(); redirect_headers_->setLocation("https://www.foo.com"); + + EXPECT_CALL(callbacks_, decodingBuffer()).Times(1); + EXPECT_CALL(callbacks_, recreateStream()).Times(0); + response_decoder_->decodeHeaders(std::move(redirect_headers_), true); EXPECT_EQ(1U, cm_.thread_local_cluster_.cluster_.info_->stats_store_ .counter("upstream_internal_redirect_failed_total") .value()); + EXPECT_EQ(1UL, stats_store_.counter("test.passthrough_internal_redirect_unsafe_scheme").value()); } -TEST_F(RouterTest, HttpInternalRedirectSucceeded) { +TEST_F(RouterTest, InternalRedirectRejectedByPredicate) { enableRedirects(); - setMaxInternalRedirects(3); + + sendRequest(); + + redirect_headers_->setLocation("http://www.foo.com/some/path"); + + auto mock_predicate = std::make_shared>(); + + EXPECT_CALL(callbacks_, decodingBuffer()).Times(1); + EXPECT_CALL(callbacks_, clearRouteCache()).Times(1); + EXPECT_CALL(callbacks_.route_->route_entry_.internal_redirect_policy_, predicates()) + .WillOnce(Return(std::vector({mock_predicate}))); + EXPECT_CALL(*mock_predicate, acceptTargetRoute(_, _, _, _)).WillOnce(Return(false)); + ON_CALL(*mock_predicate, name()).WillByDefault(Return("mock_predicate")); + EXPECT_CALL(callbacks_, recreateStream()).Times(0); + + response_decoder_->decodeHeaders(std::move(redirect_headers_), true); + EXPECT_EQ(1U, cm_.thread_local_cluster_.cluster_.info_->stats_store_ + .counter("upstream_internal_redirect_failed_total") + .value()); + EXPECT_EQ(1UL, stats_store_.counter("test.passthrough_internal_redirect_predicate").value()); + + // Make sure the original host/path is preserved. + EXPECT_EQ("host", default_request_headers_.Host()->value().getStringView()); + EXPECT_EQ("/", default_request_headers_.Path()->value().getStringView()); + // Make sure x-envoy-original-url is not set for unsuccessful redirect. + EXPECT_EQ(nullptr, default_request_headers_.EnvoyOriginalUrl()); +} + +TEST_F(RouterTest, HttpInternalRedirectSucceeded) { + enableRedirects(3); setNumPreviousRedirect(2); default_request_headers_.setForwardedProto("http"); sendRequest(); EXPECT_CALL(callbacks_, decodingBuffer()).Times(1); + EXPECT_CALL(callbacks_, clearRouteCache()).Times(1); EXPECT_CALL(callbacks_, recreateStream()).Times(1).WillOnce(Return(true)); response_decoder_->decodeHeaders(std::move(redirect_headers_), false); EXPECT_EQ(1U, cm_.thread_local_cluster_.cluster_.info_->stats_store_ @@ -4553,8 +4609,7 @@ TEST_F(RouterTest, HttpInternalRedirectSucceeded) { TEST_F(RouterTest, HttpsInternalRedirectSucceeded) { auto ssl_connection = std::make_shared(); - enableRedirects(); - setMaxInternalRedirects(3); + enableRedirects(3); setNumPreviousRedirect(1); sendRequest(); @@ -4562,6 +4617,30 @@ TEST_F(RouterTest, HttpsInternalRedirectSucceeded) { redirect_headers_->setLocation("https://www.foo.com"); EXPECT_CALL(connection_, ssl()).Times(1).WillOnce(Return(ssl_connection)); EXPECT_CALL(callbacks_, decodingBuffer()).Times(1); + EXPECT_CALL(callbacks_, clearRouteCache()).Times(1); + EXPECT_CALL(callbacks_, recreateStream()).Times(1).WillOnce(Return(true)); + response_decoder_->decodeHeaders(std::move(redirect_headers_), false); + EXPECT_EQ(1U, cm_.thread_local_cluster_.cluster_.info_->stats_store_ + .counter("upstream_internal_redirect_succeeded_total") + .value()); + + // In production, the HCM recreateStream would have called this. + router_.onDestroy(); +} + +TEST_F(RouterTest, CrossSchemeRedirectAllowedByPolicy) { + auto ssl_connection = std::make_shared(); + enableRedirects(); + + sendRequest(); + + redirect_headers_->setLocation("http://www.foo.com"); + EXPECT_CALL(connection_, ssl()).Times(1).WillOnce(Return(ssl_connection)); + EXPECT_CALL(callbacks_, decodingBuffer()).Times(1); + EXPECT_CALL(callbacks_.route_->route_entry_.internal_redirect_policy_, + isCrossSchemeRedirectAllowed()) + .WillOnce(Return(true)); + EXPECT_CALL(callbacks_, clearRouteCache()).Times(1); EXPECT_CALL(callbacks_, recreateStream()).Times(1).WillOnce(Return(true)); response_decoder_->decodeHeaders(std::move(redirect_headers_), false); EXPECT_EQ(1U, cm_.thread_local_cluster_.cluster_.info_->stats_store_ diff --git a/test/extensions/internal_redirect/previous_routes/BUILD b/test/extensions/internal_redirect/previous_routes/BUILD new file mode 100644 index 000000000000..8425dec9126c --- /dev/null +++ b/test/extensions/internal_redirect/previous_routes/BUILD @@ -0,0 +1,24 @@ +licenses(["notice"]) # Apache 2 + +load( + "//bazel:envoy_build_system.bzl", + "envoy_package", +) +load( + "//test/extensions:extensions_build_system.bzl", + "envoy_extension_cc_test", +) + +envoy_package() + +envoy_extension_cc_test( + name = "config_test", + srcs = ["config_test.cc"], + extension_name = "envoy.internal_redirect_predicates.previous_routes", + deps = [ + "//source/common/stream_info:filter_state_lib", + "//source/extensions/internal_redirect:well_known_names", + "//source/extensions/internal_redirect/previous_routes:config", + "@envoy_api//envoy/extensions/internal_redirect/previous_routes/v3:pkg_cc_proto", + ], +) diff --git a/test/extensions/internal_redirect/previous_routes/config_test.cc b/test/extensions/internal_redirect/previous_routes/config_test.cc new file mode 100644 index 000000000000..1d69320fc2ed --- /dev/null +++ b/test/extensions/internal_redirect/previous_routes/config_test.cc @@ -0,0 +1,83 @@ +#include "envoy/extensions/internal_redirect/previous_routes/v3/previous_routes_config.pb.h" +#include "envoy/registry/registry.h" +#include "envoy/router/internal_redirect.h" + +#include "common/stream_info/filter_state_impl.h" + +#include "extensions/internal_redirect/previous_routes/config.h" +#include "extensions/internal_redirect/well_known_names.h" + +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +using namespace testing; + +namespace Envoy { +namespace Extensions { +namespace InternalRedirect { +namespace { + +class PreviousRoutesTest : public testing::Test { +protected: + PreviousRoutesTest() : filter_state_(StreamInfo::FilterState::LifeSpan::FilterChain) { + factory_ = Registry::FactoryRegistry::getFactory( + InternalRedirectPredicateValues::get().PreviousRoutesPredicate); + config_ = factory_->createEmptyConfigProto(); + } + + StreamInfo::FilterStateImpl filter_state_; + Router::InternalRedirectPredicateFactory* factory_; + ProtobufTypes::MessagePtr config_; +}; + +TEST_F(PreviousRoutesTest, TargetIsOnlyTakenOnce) { + std::string current_route_name = "fake_current_route"; + // Create the predicate for the first time. It should remember nothing in the + // filter state, so it allows the redirect. + { + auto predicate = factory_->createInternalRedirectPredicate(*config_, current_route_name); + ASSERT(predicate); + + EXPECT_TRUE(predicate->acceptTargetRoute(filter_state_, "route_1", false, false)); + // New filter state data is created with route name. + EXPECT_TRUE(filter_state_.hasDataWithName( + "envoy.internal_redirect.previous_routes_predicate_state.fake_current_route")); + } + + // The second predicate should see the previously taken route. + { + auto predicate = factory_->createInternalRedirectPredicate(*config_, current_route_name); + ASSERT(predicate); + + EXPECT_FALSE(predicate->acceptTargetRoute(filter_state_, "route_1", false, false)); + } +} + +TEST_F(PreviousRoutesTest, RoutesAreIndependent) { + // Create the predicate on route_0. + { + auto predicate = factory_->createInternalRedirectPredicate(*config_, "route_0"); + ASSERT(predicate); + + EXPECT_TRUE(predicate->acceptTargetRoute(filter_state_, "route_2", false, false)); + // New filter state data is created with route name. + EXPECT_TRUE(filter_state_.hasDataWithName( + "envoy.internal_redirect.previous_routes_predicate_state.route_0")); + } + + // The predicate created on route_1 should also allow a redirect to route_2 + { + auto predicate = factory_->createInternalRedirectPredicate(*config_, "route_1"); + ASSERT(predicate); + + EXPECT_TRUE(predicate->acceptTargetRoute(filter_state_, "route_2", false, false)); + // New filter state data is created with route name. + EXPECT_TRUE(filter_state_.hasDataWithName( + "envoy.internal_redirect.previous_routes_predicate_state.route_1")); + } +} + +} // namespace +} // namespace InternalRedirect +} // namespace Extensions +} // namespace Envoy diff --git a/test/integration/BUILD b/test/integration/BUILD index d18b58487f89..0d838146ee65 100644 --- a/test/integration/BUILD +++ b/test/integration/BUILD @@ -632,9 +632,15 @@ envoy_cc_test( deps = [ ":http_protocol_integration_lib", "//source/common/http:header_map_lib", + "//source/extensions/internal_redirect/allow_listed_routes:config", + "//source/extensions/internal_redirect/previous_routes:config", + "//source/extensions/internal_redirect/safe_cross_scheme:config", "//test/test_common:utility_lib", "@envoy_api//envoy/config/route/v3:pkg_cc_proto", "@envoy_api//envoy/extensions/filters/network/http_connection_manager/v3:pkg_cc_proto", + "@envoy_api//envoy/extensions/internal_redirect/allow_listed_routes/v3:pkg_cc_proto", + "@envoy_api//envoy/extensions/internal_redirect/previous_routes/v3:pkg_cc_proto", + "@envoy_api//envoy/extensions/internal_redirect/safe_cross_scheme/v3:pkg_cc_proto", ], ) diff --git a/test/integration/redirect_integration_test.cc b/test/integration/redirect_integration_test.cc index 14c4fa5da7e2..02861c34b6c8 100644 --- a/test/integration/redirect_integration_test.cc +++ b/test/integration/redirect_integration_test.cc @@ -1,5 +1,8 @@ #include "envoy/config/route/v3/route_components.pb.h" #include "envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.pb.h" +#include "envoy/extensions/internal_redirect/allow_listed_routes/v3/allow_listed_routes_config.pb.h" +#include "envoy/extensions/internal_redirect/previous_routes/v3/previous_routes_config.pb.h" +#include "envoy/extensions/internal_redirect/safe_cross_scheme/v3/safe_cross_scheme_config.pb.h" #include "test/integration/http_protocol_integration.h" @@ -19,16 +22,17 @@ class RedirectIntegrationTest : public HttpProtocolIntegrationTest { config_helper_.addVirtualHost(pass_through); auto handle = config_helper_.createVirtualHost("handle.internal.redirect"); - handle.mutable_routes(0)->mutable_route()->set_internal_redirect_action( - envoy::config::route::v3::RouteAction::HANDLE_INTERNAL_REDIRECT); + handle.mutable_routes(0)->set_name("redirect"); + handle.mutable_routes(0)->mutable_route()->mutable_internal_redirect_policy(); config_helper_.addVirtualHost(handle); auto handle_max_3_hop = config_helper_.createVirtualHost("handle.internal.redirect.max.three.hop"); - handle_max_3_hop.mutable_routes(0)->mutable_route()->set_internal_redirect_action( - envoy::config::route::v3::RouteAction::HANDLE_INTERNAL_REDIRECT); + handle_max_3_hop.mutable_routes(0)->set_name("max_three_hop"); + handle_max_3_hop.mutable_routes(0)->mutable_route()->mutable_internal_redirect_policy(); handle_max_3_hop.mutable_routes(0) ->mutable_route() + ->mutable_internal_redirect_policy() ->mutable_max_internal_redirects() ->set_value(3); config_helper_.addVirtualHost(handle_max_3_hop); @@ -176,6 +180,9 @@ TEST_P(RedirectIntegrationTest, InternalRedirectWithThreeHopLimit) { EXPECT_EQ( 1, test_server_->counter("cluster.cluster_0.upstream_internal_redirect_failed_total")->value()); + EXPECT_EQ( + 1, test_server_->counter("http.config_test.passthrough_internal_redirect_too_many_redirects") + ->value()); } TEST_P(RedirectIntegrationTest, InternalRedirectToDestinationWithBody) { @@ -220,6 +227,180 @@ TEST_P(RedirectIntegrationTest, InternalRedirectToDestinationWithBody) { ->value()); } +TEST_P(RedirectIntegrationTest, InternalRedirectPreventedByPreviousRoutesPredicate) { + auto handle_prevent_repeated_target = + config_helper_.createVirtualHost("handle.internal.redirect.no.repeated.target"); + auto* internal_redirect_policy = handle_prevent_repeated_target.mutable_routes(0) + ->mutable_route() + ->mutable_internal_redirect_policy(); + internal_redirect_policy->mutable_max_internal_redirects()->set_value(10); + envoy::extensions::internal_redirect::previous_routes::v3::PreviousRoutesConfig + previous_routes_config; + auto* predicate = internal_redirect_policy->add_predicates(); + predicate->set_name("previous_routes"); + predicate->mutable_typed_config()->PackFrom(previous_routes_config); + config_helper_.addVirtualHost(handle_prevent_repeated_target); + + // Validate that header sanitization is only called once. + config_helper_.addConfigModifier( + [](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& + hcm) { hcm.set_via("via_value"); }); + initialize(); + fake_upstreams_[0]->set_allow_unexpected_disconnects(true); + + codec_client_ = makeHttpConnection(lookupPort("http")); + + default_request_headers_.setHost("handle.internal.redirect.no.repeated.target"); + IntegrationStreamDecoderPtr response = + codec_client_->makeHeaderOnlyRequest(default_request_headers_); + + auto first_request = waitForNextStream(); + // Redirect to another route + redirect_response_.setLocation("http://handle.internal.redirect.max.three.hop/random/path"); + first_request->encodeHeaders(redirect_response_, true); + + auto second_request = waitForNextStream(); + // Redirect back to the original route. + redirect_response_.setLocation("http://handle.internal.redirect.no.repeated.target/another/path"); + second_request->encodeHeaders(redirect_response_, true); + + auto third_request = waitForNextStream(); + // Redirect to the same route as the first redirect. This should fail. + redirect_response_.setLocation("http://handle.internal.redirect.max.three.hop/yet/another/path"); + third_request->encodeHeaders(redirect_response_, true); + + response->waitForEndStream(); + ASSERT_TRUE(response->complete()); + EXPECT_EQ("302", response->headers().Status()->value().getStringView()); + EXPECT_EQ("http://handle.internal.redirect.max.three.hop/yet/another/path", + response->headers().Location()->value().getStringView()); + EXPECT_EQ(2, test_server_->counter("cluster.cluster_0.upstream_internal_redirect_succeeded_total") + ->value()); + EXPECT_EQ( + 1, + test_server_->counter("http.config_test.passthrough_internal_redirect_predicate")->value()); +} + +TEST_P(RedirectIntegrationTest, InternalRedirectPreventedByAllowListedRoutesPredicate) { + auto handle_allow_listed_redirect_route = + config_helper_.createVirtualHost("handle.internal.redirect.only.allow.listed.target"); + auto* internal_redirect_policy = handle_allow_listed_redirect_route.mutable_routes(0) + ->mutable_route() + ->mutable_internal_redirect_policy(); + + auto* allow_listed_routes_predicate = internal_redirect_policy->add_predicates(); + allow_listed_routes_predicate->set_name("allow_listed_routes"); + envoy::extensions::internal_redirect::allow_listed_routes::v3::AllowListedRoutesConfig + allow_listed_routes_config; + *allow_listed_routes_config.add_allowed_route_names() = "max_three_hop"; + allow_listed_routes_predicate->mutable_typed_config()->PackFrom(allow_listed_routes_config); + + internal_redirect_policy->mutable_max_internal_redirects()->set_value(10); + + config_helper_.addVirtualHost(handle_allow_listed_redirect_route); + + // Validate that header sanitization is only called once. + config_helper_.addConfigModifier( + [](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& + hcm) { hcm.set_via("via_value"); }); + initialize(); + fake_upstreams_[0]->set_allow_unexpected_disconnects(true); + + codec_client_ = makeHttpConnection(lookupPort("http")); + + default_request_headers_.setHost("handle.internal.redirect.only.allow.listed.target"); + IntegrationStreamDecoderPtr response = + codec_client_->makeHeaderOnlyRequest(default_request_headers_); + + auto first_request = waitForNextStream(); + // Redirect to another route + redirect_response_.setLocation("http://handle.internal.redirect.max.three.hop/random/path"); + first_request->encodeHeaders(redirect_response_, true); + + auto second_request = waitForNextStream(); + // Redirect back to the original route. + redirect_response_.setLocation( + "http://handle.internal.redirect.only.allow.listed.target/another/path"); + second_request->encodeHeaders(redirect_response_, true); + + auto third_request = waitForNextStream(); + // Redirect to the non-allow-listed route. This should fail. + redirect_response_.setLocation("http://handle.internal.redirect/yet/another/path"); + third_request->encodeHeaders(redirect_response_, true); + + response->waitForEndStream(); + ASSERT_TRUE(response->complete()); + EXPECT_EQ("302", response->headers().Status()->value().getStringView()); + EXPECT_EQ("http://handle.internal.redirect/yet/another/path", + response->headers().Location()->value().getStringView()); + EXPECT_EQ(2, test_server_->counter("cluster.cluster_0.upstream_internal_redirect_succeeded_total") + ->value()); + EXPECT_EQ( + 1, + test_server_->counter("http.config_test.passthrough_internal_redirect_predicate")->value()); +} + +TEST_P(RedirectIntegrationTest, InternalRedirectPreventedBySafeCrossSchemePredicate) { + auto handle_safe_cross_scheme_route = config_helper_.createVirtualHost( + "handle.internal.redirect.only.allow.safe.cross.scheme.redirect"); + auto* internal_redirect_policy = handle_safe_cross_scheme_route.mutable_routes(0) + ->mutable_route() + ->mutable_internal_redirect_policy(); + + internal_redirect_policy->set_allow_cross_scheme_redirect(true); + + auto* predicate = internal_redirect_policy->add_predicates(); + predicate->set_name("safe_cross_scheme_predicate"); + envoy::extensions::internal_redirect::safe_cross_scheme::v3::SafeCrossSchemeConfig + predicate_config; + predicate->mutable_typed_config()->PackFrom(predicate_config); + + internal_redirect_policy->mutable_max_internal_redirects()->set_value(10); + + config_helper_.addVirtualHost(handle_safe_cross_scheme_route); + + // Validate that header sanitization is only called once. + config_helper_.addConfigModifier( + [](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& + hcm) { hcm.set_via("via_value"); }); + initialize(); + fake_upstreams_[0]->set_allow_unexpected_disconnects(true); + + codec_client_ = makeHttpConnection(lookupPort("http")); + + default_request_headers_.setHost( + "handle.internal.redirect.only.allow.safe.cross.scheme.redirect"); + IntegrationStreamDecoderPtr response = + codec_client_->makeHeaderOnlyRequest(default_request_headers_); + + auto first_request = waitForNextStream(); + // Redirect to another route + redirect_response_.setLocation("http://handle.internal.redirect.max.three.hop/random/path"); + first_request->encodeHeaders(redirect_response_, true); + + auto second_request = waitForNextStream(); + // Redirect back to the original route. + redirect_response_.setLocation( + "http://handle.internal.redirect.only.allow.safe.cross.scheme.redirect/another/path"); + second_request->encodeHeaders(redirect_response_, true); + + auto third_request = waitForNextStream(); + // Redirect to https target. This should fail. + redirect_response_.setLocation("https://handle.internal.redirect/yet/another/path"); + third_request->encodeHeaders(redirect_response_, true); + + response->waitForEndStream(); + ASSERT_TRUE(response->complete()); + EXPECT_EQ("302", response->headers().Status()->value().getStringView()); + EXPECT_EQ("https://handle.internal.redirect/yet/another/path", + response->headers().Location()->value().getStringView()); + EXPECT_EQ(2, test_server_->counter("cluster.cluster_0.upstream_internal_redirect_succeeded_total") + ->value()); + EXPECT_EQ( + 1, + test_server_->counter("http.config_test.passthrough_internal_redirect_predicate")->value()); +} + TEST_P(RedirectIntegrationTest, InvalidRedirect) { initialize(); diff --git a/test/integration/stats_integration_test.cc b/test/integration/stats_integration_test.cc index ea7fa20fd23b..9221d3d38381 100644 --- a/test/integration/stats_integration_test.cc +++ b/test/integration/stats_integration_test.cc @@ -272,6 +272,7 @@ TEST_P(ClusterMemoryTestRunner, MemoryLargeClusterSizeWithFakeSymbolTable) { // 2020/04/02 10624 43356 44000 Use 100 clusters rather than 1000 to avoid timeouts // 2020/04/07 10661 43349 44000 fix clang tidy on master // 2020/04/23 10531 44169 44600 http: max stream duration upstream support. + // 2020/05/05 10908 44233 44600 router: add InternalRedirectPolicy and predicate // Note: when adjusting this value: EXPECT_MEMORY_EQ is active only in CI // 'release' builds, where we control the platform and tool-chain. So you @@ -285,7 +286,7 @@ TEST_P(ClusterMemoryTestRunner, MemoryLargeClusterSizeWithFakeSymbolTable) { // If you encounter a failure here, please see // https://github.com/envoyproxy/envoy/blob/master/source/docs/stats.md#stats-memory-tests // for details on how to fix. - EXPECT_MEMORY_EQ(m_per_cluster, 44169); + EXPECT_MEMORY_EQ(m_per_cluster, 44233); EXPECT_MEMORY_LE(m_per_cluster, 44600); } @@ -331,6 +332,7 @@ TEST_P(ClusterMemoryTestRunner, MemoryLargeClusterSizeWithRealSymbolTable) { // 2020/04/02 10624 35564 36000 Use 100 clusters rather than 1000 to avoid timeouts // 2020/04/07 10661 35557 36000 fix clang tidy on master // 2020/04/23 10531 36281 36800 http: max stream duration upstream support. + // 2020/05/05 10908 36345 36800 router: add InternalRedirectPolicy and predicate // Note: when adjusting this value: EXPECT_MEMORY_EQ is active only in CI // 'release' builds, where we control the platform and tool-chain. So you @@ -344,7 +346,7 @@ TEST_P(ClusterMemoryTestRunner, MemoryLargeClusterSizeWithRealSymbolTable) { // If you encounter a failure here, please see // https://github.com/envoyproxy/envoy/blob/master/source/docs/stats.md#stats-memory-tests // for details on how to fix. - EXPECT_MEMORY_EQ(m_per_cluster, 36281); + EXPECT_MEMORY_EQ(m_per_cluster, 36345); EXPECT_MEMORY_LE(m_per_cluster, 36800); } diff --git a/test/mocks/router/mocks.cc b/test/mocks/router/mocks.cc index c04efe43e255..2d0a995bf1ca 100644 --- a/test/mocks/router/mocks.cc +++ b/test/mocks/router/mocks.cc @@ -22,6 +22,10 @@ TestRetryPolicy::TestRetryPolicy() { num_retries_ = 1; } TestRetryPolicy::~TestRetryPolicy() = default; +MockInternalRedirectPolicy::MockInternalRedirectPolicy() { + ON_CALL(*this, enabled()).WillByDefault(Return(false)); +} + MockRetryState::MockRetryState() = default; void MockRetryState::expectHeadersRetry() { @@ -85,6 +89,7 @@ MockRouteEntry::MockRouteEntry() { ON_CALL(*this, opaqueConfig()).WillByDefault(ReturnRef(opaque_config_)); ON_CALL(*this, rateLimitPolicy()).WillByDefault(ReturnRef(rate_limit_policy_)); ON_CALL(*this, retryPolicy()).WillByDefault(ReturnRef(retry_policy_)); + ON_CALL(*this, internalRedirectPolicy()).WillByDefault(ReturnRef(internal_redirect_policy_)); ON_CALL(*this, retryShadowBufferLimit()) .WillByDefault(Return(std::numeric_limits::max())); ON_CALL(*this, shadowPolicies()).WillByDefault(ReturnRef(shadow_policies_)); diff --git a/test/mocks/router/mocks.h b/test/mocks/router/mocks.h index f7c719442e27..d221d81b3439 100644 --- a/test/mocks/router/mocks.h +++ b/test/mocks/router/mocks.h @@ -131,6 +131,22 @@ class TestRetryPolicy : public RetryPolicy { absl::optional max_interval_{}; }; +class MockInternalRedirectPolicy : public InternalRedirectPolicy { +public: + MockInternalRedirectPolicy(); + MOCK_METHOD(bool, enabled, (), (const)); + MOCK_METHOD(bool, shouldRedirectForResponseCode, (const Http::Code& response_code), (const)); + MOCK_METHOD(std::vector, predicates, (), (const)); + MOCK_METHOD(uint32_t, maxInternalRedirects, (), (const)); + MOCK_METHOD(bool, isCrossSchemeRedirectAllowed, (), (const)); +}; + +class MockInternalRedirectPredicate : public InternalRedirectPredicate { +public: + MOCK_METHOD(bool, acceptTargetRoute, (StreamInfo::FilterState&, absl::string_view, bool, bool)); + MOCK_METHOD(absl::string_view, name, (), (const)); +}; + class MockRetryState : public RetryState { public: MockRetryState(); @@ -335,6 +351,7 @@ class MockRouteEntry : public RouteEntry { MOCK_METHOD(Upstream::ResourcePriority, priority, (), (const)); MOCK_METHOD(const RateLimitPolicy&, rateLimitPolicy, (), (const)); MOCK_METHOD(const RetryPolicy&, retryPolicy, (), (const)); + MOCK_METHOD(const InternalRedirectPolicy&, internalRedirectPolicy, (), (const)); MOCK_METHOD(uint32_t, retryShadowBufferLimit, (), (const)); MOCK_METHOD(const std::vector&, shadowPolicies, (), (const)); MOCK_METHOD(std::chrono::milliseconds, timeout, (), (const)); @@ -356,8 +373,6 @@ class MockRouteEntry : public RouteEntry { MOCK_METHOD(bool, includeAttemptCountInResponse, (), (const)); MOCK_METHOD(const absl::optional&, connectConfig, (), (const)); MOCK_METHOD(const UpgradeMap&, upgradeMap, (), (const)); - MOCK_METHOD(InternalRedirectAction, internalRedirectAction, (), (const)); - MOCK_METHOD(uint32_t, maxInternalRedirects, (), (const)); MOCK_METHOD(const std::string&, routeName, (), (const)); std::string cluster_name_{"fake_cluster"}; @@ -365,6 +380,7 @@ class MockRouteEntry : public RouteEntry { std::multimap opaque_config_; TestVirtualCluster virtual_cluster_; TestRetryPolicy retry_policy_; + testing::NiceMock internal_redirect_policy_; TestHedgePolicy hedge_policy_; testing::NiceMock rate_limit_policy_; std::vector shadow_policies_; diff --git a/tools/spelling/spelling_dictionary.txt b/tools/spelling/spelling_dictionary.txt index 497e035e2c02..00bbda734aa3 100644 --- a/tools/spelling/spelling_dictionary.txt +++ b/tools/spelling/spelling_dictionary.txt @@ -17,6 +17,7 @@ ASCII ASSERTs AST AWS +Allowlisted BACKTRACE BSON BPF @@ -170,6 +171,7 @@ LHS LLVM LPT LRS +Loggable MB MD MERCHANTABILITY @@ -362,6 +364,7 @@ alloc alloca allocator allowlist +allowlisted alls alphanumerics amongst