Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[config] Add basic config validators for path rewrite, host rewrite and redirect actions #10367

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 16 additions & 7 deletions api/envoy/api/v2/route/route_components.proto
Original file line number Diff line number Diff line change
Expand Up @@ -786,7 +786,8 @@ message RouteAction {
//
// Having above entries in the config, requests to */prefix* will be stripped to */*, while
// requests to */prefix/etc* will be stripped to */etc*.
string prefix_rewrite = 5;
string prefix_rewrite = 5
[(validate.rules).string = {well_known_regex: HTTP_HEADER_VALUE strict: false}];

// Indicates that during forwarding, portions of the path that match the
// pattern should be rewritten, even allowing the substitution of capture
Expand Down Expand Up @@ -821,7 +822,10 @@ message RouteAction {
oneof host_rewrite_specifier {
// Indicates that during forwarding, the host header will be swapped with
// this value.
string host_rewrite = 6 [(udpa.annotations.field_migrate).rename = "host_rewrite_literal"];
string host_rewrite = 6 [
(validate.rules).string = {well_known_regex: HTTP_HEADER_VALUE strict: false},
(udpa.annotations.field_migrate).rename = "host_rewrite_literal"
];

// Indicates that during forwarding, the host header will be swapped with
// the hostname of the upstream host chosen by the cluster manager. This
Expand All @@ -838,8 +842,10 @@ message RouteAction {
//
// Pay attention to the potential security implications of using this option. Provided header
// must come from trusted source.
string auto_host_rewrite_header = 29
[(udpa.annotations.field_migrate).rename = "host_rewrite_header"];
string auto_host_rewrite_header = 29 [
(validate.rules).string = {well_known_regex: HTTP_HEADER_NAME strict: false},
(udpa.annotations.field_migrate).rename = "host_rewrite_header"
];
}

// Specifies the upstream timeout for the route. If not specified, the default is 15s. This
Expand Down Expand Up @@ -1132,14 +1138,16 @@ message RedirectAction {
}

// The host portion of the URL will be swapped with this value.
string host_redirect = 1;
string host_redirect = 1
[(validate.rules).string = {well_known_regex: HTTP_HEADER_VALUE strict: false}];

// The port value of the URL will be swapped with this value.
uint32 port_redirect = 8;

oneof path_rewrite_specifier {
// The path portion of the URL will be swapped with this value.
string path_redirect = 2;
string path_redirect = 2
[(validate.rules).string = {well_known_regex: HTTP_HEADER_VALUE strict: false}];

// Indicates that during redirection, the matched prefix (or path)
// should be swapped with this value. This option allows redirect URLs be dynamically created
Expand All @@ -1149,7 +1157,8 @@ message RedirectAction {
//
// Pay attention to the use of trailing slashes as mentioned in
// :ref:`RouteAction's prefix_rewrite <envoy_api_field_route.RouteAction.prefix_rewrite>`.
string prefix_rewrite = 5;
string prefix_rewrite = 5
[(validate.rules).string = {well_known_regex: HTTP_HEADER_VALUE strict: false}];
}

// The HTTP status code to use in the redirect response. The default response
Expand Down
18 changes: 12 additions & 6 deletions api/envoy/config/route/v3/route_components.proto
Original file line number Diff line number Diff line change
Expand Up @@ -762,7 +762,8 @@ message RouteAction {
//
// Having above entries in the config, requests to */prefix* will be stripped to */*, while
// requests to */prefix/etc* will be stripped to */etc*.
string prefix_rewrite = 5;
string prefix_rewrite = 5
[(validate.rules).string = {well_known_regex: HTTP_HEADER_VALUE strict: false}];

// Indicates that during forwarding, portions of the path that match the
// pattern should be rewritten, even allowing the substitution of capture
Expand Down Expand Up @@ -797,7 +798,8 @@ message RouteAction {
oneof host_rewrite_specifier {
// Indicates that during forwarding, the host header will be swapped with
// this value.
string host_rewrite_literal = 6;
string host_rewrite_literal = 6
[(validate.rules).string = {well_known_regex: HTTP_HEADER_VALUE strict: false}];

// Indicates that during forwarding, the host header will be swapped with
// the hostname of the upstream host chosen by the cluster manager. This
Expand All @@ -814,7 +816,8 @@ message RouteAction {
//
// Pay attention to the potential security implications of using this option. Provided header
// must come from trusted source.
string host_rewrite_header = 29;
string host_rewrite_header = 29
[(validate.rules).string = {well_known_regex: HTTP_HEADER_NAME strict: false}];
}

// Specifies the upstream timeout for the route. If not specified, the default is 15s. This
Expand Down Expand Up @@ -1119,14 +1122,16 @@ message RedirectAction {
}

// The host portion of the URL will be swapped with this value.
string host_redirect = 1;
string host_redirect = 1
[(validate.rules).string = {well_known_regex: HTTP_HEADER_VALUE strict: false}];

// The port value of the URL will be swapped with this value.
uint32 port_redirect = 8;

oneof path_rewrite_specifier {
// The path portion of the URL will be swapped with this value.
string path_redirect = 2;
string path_redirect = 2
[(validate.rules).string = {well_known_regex: HTTP_HEADER_VALUE strict: false}];

// Indicates that during redirection, the matched prefix (or path)
// should be swapped with this value. This option allows redirect URLs be dynamically created
Expand All @@ -1136,7 +1141,8 @@ message RedirectAction {
//
// Pay attention to the use of trailing slashes as mentioned in
// :ref:`RouteAction's prefix_rewrite <envoy_api_field_config.route.v3.RouteAction.prefix_rewrite>`.
string prefix_rewrite = 5;
string prefix_rewrite = 5
[(validate.rules).string = {well_known_regex: HTTP_HEADER_VALUE strict: false}];
}

// The HTTP status code to use in the redirect response. The default response
Expand Down
23 changes: 16 additions & 7 deletions generated_api_shadow/envoy/api/v2/route/route_components.proto

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

18 changes: 12 additions & 6 deletions generated_api_shadow/envoy/config/route/v3/route_components.proto

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

102 changes: 102 additions & 0 deletions test/common/router/config_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3653,6 +3653,108 @@ TEST_F(RouteMatcherTest, TestDuplicatePrefixWildcardDomainConfig) {
"Only unique values for domains are permitted. Duplicate entry of domain bar.*");
}

TEST_F(RouteMatcherTest, TestInvalidCharactersInPrefixRewrites) {
const std::string yaml = R"EOF(
virtual_hosts:
- name: www
domains: ["*"]
routes:
- match: { prefix: "/foo" }
route:
prefix_rewrite: "/\ndroptable"
cluster: www
)EOF";

EXPECT_THROW_WITH_REGEX(
TestConfigImpl(parseRouteConfigurationFromV2Yaml(yaml), factory_context_, true),
EnvoyException,
"RouteActionValidationError.PrefixRewrite:.*value does not match regex pattern");
}

TEST_F(RouteMatcherTest, TestInvalidCharactersInHostRewrites) {
const std::string yaml = R"EOF(
virtual_hosts:
- name: www
domains: ["*"]
routes:
- match: { prefix: "/foo" }
route:
host_rewrite: "new_host\ndroptable"
cluster: www
)EOF";

EXPECT_THROW_WITH_REGEX(
TestConfigImpl(parseRouteConfigurationFromV2Yaml(yaml), factory_context_, true),
EnvoyException,
"RouteActionValidationError.HostRewriteLiteral:.*value does not match regex pattern");
}

TEST_F(RouteMatcherTest, TestInvalidCharactersInAutoHostRewrites) {
const std::string yaml = R"EOF(
virtual_hosts:
- name: www
domains: ["*"]
routes:
- match: { prefix: "/foo" }
route:
auto_host_rewrite_header: "x-host\ndroptable"
cluster: www
)EOF";

EXPECT_THROW_WITH_REGEX(
TestConfigImpl(parseRouteConfigurationFromV2Yaml(yaml), factory_context_, true),
EnvoyException,
"RouteActionValidationError.HostRewriteHeader:.*value does not match regex pattern");
}

TEST_F(RouteMatcherTest, TestInvalidCharactersInHostRedirect) {
const std::string yaml = R"EOF(
virtual_hosts:
- name: www
domains: ["*"]
routes:
- match: { prefix: "/foo" }
redirect: { host_redirect: "new.host\ndroptable" }
)EOF";

EXPECT_THROW_WITH_REGEX(
TestConfigImpl(parseRouteConfigurationFromV2Yaml(yaml), factory_context_, true),
EnvoyException,
"RedirectActionValidationError.HostRedirect:.*value does not match regex pattern");
}

TEST_F(RouteMatcherTest, TestInvalidCharactersInPathRedirect) {
const std::string yaml = R"EOF(
virtual_hosts:
- name: www
domains: ["*"]
routes:
- match: { prefix: "/foo" }
redirect: { path_redirect: "/new_path\ndroptable" }
)EOF";

EXPECT_THROW_WITH_REGEX(
TestConfigImpl(parseRouteConfigurationFromV2Yaml(yaml), factory_context_, true),
EnvoyException,
"RedirectActionValidationError.PathRedirect:.*value does not match regex pattern");
}

TEST_F(RouteMatcherTest, TestInvalidCharactersInPrefixRewriteRedirect) {
const std::string yaml = R"EOF(
virtual_hosts:
- name: www
domains: ["*"]
routes:
- match: { prefix: "/foo" }
redirect: { prefix_rewrite: "/new/prefix\ndroptable"}
)EOF";

EXPECT_THROW_WITH_REGEX(
TestConfigImpl(parseRouteConfigurationFromV2Yaml(yaml), factory_context_, true),
EnvoyException,
"RedirectActionValidationError.PrefixRewrite:.*value does not match regex pattern");
}

TEST_F(RouteMatcherTest, TestPrefixAndRegexRewrites) {
const std::string yaml = R"EOF(
virtual_hosts:
Expand Down