From 60802a116693fd86da38114e1715aa41626df9a6 Mon Sep 17 00:00:00 2001 From: Lior Okman Date: Sun, 31 Dec 2023 11:27:08 +0200 Subject: [PATCH 1/3] Add parameters to ClientTrafficPolicy to control slash merging and the expected behavior for path URIs with escaped slashes. Signed-off-by: Lior Okman --- api/v1alpha1/clienttrafficpolicy_types.go | 4 ++ api/v1alpha1/pathsettings_types.go | 54 +++++++++++++++++++ api/v1alpha1/zz_generated.deepcopy.go | 30 +++++++++++ ...y.envoyproxy.io_clienttrafficpolicies.yaml | 21 ++++++++ site/content/en/latest/api/extension_types.md | 27 ++++++++++ 5 files changed, 136 insertions(+) create mode 100644 api/v1alpha1/pathsettings_types.go diff --git a/api/v1alpha1/clienttrafficpolicy_types.go b/api/v1alpha1/clienttrafficpolicy_types.go index 5b1d7d597a1..9ec283ce08d 100644 --- a/api/v1alpha1/clienttrafficpolicy_types.go +++ b/api/v1alpha1/clienttrafficpolicy_types.go @@ -74,6 +74,10 @@ type ClientTrafficPolicySpec struct { // // +optional TLS *TLSSettings `json:"tls,omitempty"` + // Path enables managing how the incoming path set by clients can be normalized. + // + // +optional + Path *PathSettings `json:"path,omitempty"` } // HTTP3Settings provides HTTP/3 configuration on the listener. diff --git a/api/v1alpha1/pathsettings_types.go b/api/v1alpha1/pathsettings_types.go new file mode 100644 index 00000000000..502b2b0cefb --- /dev/null +++ b/api/v1alpha1/pathsettings_types.go @@ -0,0 +1,54 @@ +// Copyright Envoy Gateway Authors +// SPDX-License-Identifier: Apache-2.0 +// The full text of the Apache license is available in the LICENSE file at +// the root of the repo. + +package v1alpha1 + +// PathEscapedSlashAction determines the action for requests that contain %2F, %2f, %5C, or %5c +// sequences in the URI path. +// +kubebuilder:validation:Enum=KeepUnchanged;RejectRequest;UnescapeForward;UnescapeRedirect +type PathEscapedSlashAction string + +const ( + // KeepUnchangedAction keeps escaped slashes as they arrive without changes + KeepUnchangedAction PathEscapedSlashAction = "KeepUnchanged" + // RejectRequestAction rejects client requests containing escaped slashes + // with a 400 status. gRPC requests will be rejected with the INTERNAL (13) + // error code. + // The "httpN.downstream_rq_failed_path_normalization" counter is incremented + // for each rejected request. + RejectRequestAction PathEscapedSlashAction = "RejectRequest" + // UnescapeRedirect unescapes %2F and %5C sequences and redirects to the new path + // if these sequences were present. + // + // Redirect occurs after path normalization and merge slashes transformations if + // they were configured. gRPC requests will be rejected with the INTERNAL (13) + // error code. + // This option minimizes possibility of path confusion exploits by forcing request + // with unescaped slashes to traverse all parties: downstream client, intermediate + // proxies, Envoy and upstream server. + // The “httpN.downstream_rq_redirected_with_normalized_path” counter is incremented + // for each redirected request. + UnescapeRedirect PathEscapedSlashAction = "UnescapeRedirect" + // UnescapeForward unescapes %2F and %5C sequences and forwards the request. + // Note: this option should not be enabled if intermediaries perform path based access + // control as it may lead to path confusion vulnerabilities. + UnescapeForward PathEscapedSlashAction = "UnescapeForward" +) + +// PathSettings provides settings that managing how the incoming path set by clients is handled. +type PathSettings struct { + // EscapedSlashesAction determines how %2f, %2F, %5c, or %5C sequences in the path URI + // should be handled. + // The default is UnescapeRedirect. + // + // +optional + EscapedSlashesAction *PathEscapedSlashAction `json:"escapedSlashesAction,omitempty"` + // DisableMergeSlashes allows disabling the default configuration of merging adjecent + // slashes in the path. + // Note that slash merging is not part of the HTTP spec and is provided for convenience. + // + // +optional + DisableMergeSlashes *bool `json:"disableMergeSlashes,omitempty"` +} diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 5fe230b4351..635a6a4975e 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -341,6 +341,11 @@ func (in *ClientTrafficPolicySpec) DeepCopyInto(out *ClientTrafficPolicySpec) { *out = new(TLSSettings) (*in).DeepCopyInto(*out) } + if in.Path != nil { + in, out := &in.Path, &out.Path + *out = new(PathSettings) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClientTrafficPolicySpec. @@ -1896,6 +1901,31 @@ func (in *OpenTelemetryEnvoyProxyAccessLog) DeepCopy() *OpenTelemetryEnvoyProxyA return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *PathSettings) DeepCopyInto(out *PathSettings) { + *out = *in + if in.EscapedSlashesAction != nil { + in, out := &in.EscapedSlashesAction, &out.EscapedSlashesAction + *out = new(PathEscapedSlashAction) + **out = **in + } + if in.DisableMergeSlashes != nil { + in, out := &in.DisableMergeSlashes, &out.DisableMergeSlashes + *out = new(bool) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PathSettings. +func (in *PathSettings) DeepCopy() *PathSettings { + if in == nil { + return nil + } + out := new(PathSettings) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ProxyAccessLog) DeepCopyInto(out *ProxyAccessLog) { *out = *in diff --git a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_clienttrafficpolicies.yaml b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_clienttrafficpolicies.yaml index 23cee2ea799..95e66797b46 100644 --- a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_clienttrafficpolicies.yaml +++ b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_clienttrafficpolicies.yaml @@ -53,6 +53,27 @@ spec: http3: description: HTTP3 provides HTTP/3 configuration on the listener. type: object + path: + description: Path enables managing how the incoming path set by clients + can be normalized. + properties: + disableMergeSlashes: + description: DisableMergeSlashes allows disabling the default + configuration of merging adjecent slashes in the path. Note + that slash merging is not part of the HTTP spec and is provided + for convenience. + type: boolean + escapedSlashesAction: + description: EscapedSlashesAction determines how %2f, %2F, %5c, + or %5C sequences in the path URI should be handled. The default + is UnescapeRedirect. + enum: + - KeepUnchanged + - RejectRequest + - UnescapeForward + - UnescapeRedirect + type: string + type: object suppressEnvoyHeaders: description: SuppressEnvoyHeaders configures the Envoy Router filter to suppress the "x-envoy-' headers from both requests and responses. diff --git a/site/content/en/latest/api/extension_types.md b/site/content/en/latest/api/extension_types.md index 2c5db9d7370..eb16b883241 100644 --- a/site/content/en/latest/api/extension_types.md +++ b/site/content/en/latest/api/extension_types.md @@ -220,6 +220,7 @@ _Appears in:_ | `enableProxyProtocol` _boolean_ | EnableProxyProtocol interprets the ProxyProtocol header and adds the Client Address into the X-Forwarded-For header. Note Proxy Protocol must be present when this field is set, else the connection is closed. | | `http3` _[HTTP3Settings](#http3settings)_ | HTTP3 provides HTTP/3 configuration on the listener. | | `tls` _[TLSSettings](#tlssettings)_ | TLS settings configure TLS termination settings with the downstream client. | +| `path` _[PathSettings](#pathsettings)_ | Path enables managing how the incoming path set by clients can be normalized. | @@ -1285,6 +1286,32 @@ _Appears in:_ | `resources` _object (keys:string, values:string)_ | Resources is a set of labels that describe the source of a log entry, including envoy node info. It's recommended to follow [semantic conventions](https://opentelemetry.io/docs/reference/specification/resource/semantic_conventions/). | +#### PathEscapedSlashAction + +_Underlying type:_ `string` + +PathEscapedSlashAction determines the action for requests that contain %2F, %2f, %5C, or %5c sequences in the URI path. + +_Appears in:_ +- [PathSettings](#pathsettings) + + + +#### PathSettings + + + +PathSettings provides settings that managing how the incoming path set by clients is handled. + +_Appears in:_ +- [ClientTrafficPolicySpec](#clienttrafficpolicyspec) + +| Field | Description | +| --- | --- | +| `escapedSlashesAction` _[PathEscapedSlashAction](#pathescapedslashaction)_ | EscapedSlashesAction determines how %2f, %2F, %5c, or %5C sequences in the path URI should be handled. The default is UnescapeRedirect. | +| `disableMergeSlashes` _boolean_ | DisableMergeSlashes allows disabling the default configuration of merging adjecent slashes in the path. Note that slash merging is not part of the HTTP spec and is provided for convenience. | + + #### ProviderType _Underlying type:_ `string` From 063bde4230e9548ccb45d597d511a5a4c7572d2b Mon Sep 17 00:00:00 2001 From: Lior Okman Date: Sun, 31 Dec 2023 12:02:13 +0200 Subject: [PATCH 2/3] Make linter succeed. Signed-off-by: Lior Okman --- api/v1alpha1/pathsettings_types.go | 2 +- .../generated/gateway.envoyproxy.io_clienttrafficpolicies.yaml | 2 +- site/content/en/latest/api/extension_types.md | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/api/v1alpha1/pathsettings_types.go b/api/v1alpha1/pathsettings_types.go index 502b2b0cefb..2aee56d7e81 100644 --- a/api/v1alpha1/pathsettings_types.go +++ b/api/v1alpha1/pathsettings_types.go @@ -45,7 +45,7 @@ type PathSettings struct { // // +optional EscapedSlashesAction *PathEscapedSlashAction `json:"escapedSlashesAction,omitempty"` - // DisableMergeSlashes allows disabling the default configuration of merging adjecent + // DisableMergeSlashes allows disabling the default configuration of merging adjacent // slashes in the path. // Note that slash merging is not part of the HTTP spec and is provided for convenience. // diff --git a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_clienttrafficpolicies.yaml b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_clienttrafficpolicies.yaml index 95e66797b46..f92450beb0b 100644 --- a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_clienttrafficpolicies.yaml +++ b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_clienttrafficpolicies.yaml @@ -59,7 +59,7 @@ spec: properties: disableMergeSlashes: description: DisableMergeSlashes allows disabling the default - configuration of merging adjecent slashes in the path. Note + configuration of merging adjacent slashes in the path. Note that slash merging is not part of the HTTP spec and is provided for convenience. type: boolean diff --git a/site/content/en/latest/api/extension_types.md b/site/content/en/latest/api/extension_types.md index eb16b883241..c84f5354c93 100644 --- a/site/content/en/latest/api/extension_types.md +++ b/site/content/en/latest/api/extension_types.md @@ -1309,7 +1309,7 @@ _Appears in:_ | Field | Description | | --- | --- | | `escapedSlashesAction` _[PathEscapedSlashAction](#pathescapedslashaction)_ | EscapedSlashesAction determines how %2f, %2F, %5c, or %5C sequences in the path URI should be handled. The default is UnescapeRedirect. | -| `disableMergeSlashes` _boolean_ | DisableMergeSlashes allows disabling the default configuration of merging adjecent slashes in the path. Note that slash merging is not part of the HTTP spec and is provided for convenience. | +| `disableMergeSlashes` _boolean_ | DisableMergeSlashes allows disabling the default configuration of merging adjacent slashes in the path. Note that slash merging is not part of the HTTP spec and is provided for convenience. | #### ProviderType From 642cff7c31a3f7264277d5a0ec64b73c33ee706e Mon Sep 17 00:00:00 2001 From: Lior Okman Date: Fri, 5 Jan 2024 07:16:12 +0200 Subject: [PATCH 3/3] * Renamed UnescapeRedirect to UnescapeAndRedirect * Renamed UnescapeForward to UnescapeAndForward Signed-off-by: Lior Okman --- api/v1alpha1/pathsettings_types.go | 12 ++++++------ .../gateway.envoyproxy.io_clienttrafficpolicies.yaml | 6 +++--- site/content/en/latest/api/extension_types.md | 2 +- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/api/v1alpha1/pathsettings_types.go b/api/v1alpha1/pathsettings_types.go index 2aee56d7e81..c6963af7c58 100644 --- a/api/v1alpha1/pathsettings_types.go +++ b/api/v1alpha1/pathsettings_types.go @@ -7,7 +7,7 @@ package v1alpha1 // PathEscapedSlashAction determines the action for requests that contain %2F, %2f, %5C, or %5c // sequences in the URI path. -// +kubebuilder:validation:Enum=KeepUnchanged;RejectRequest;UnescapeForward;UnescapeRedirect +// +kubebuilder:validation:Enum=KeepUnchanged;RejectRequest;UnescapeAndForward;UnescapeAndRedirect type PathEscapedSlashAction string const ( @@ -19,7 +19,7 @@ const ( // The "httpN.downstream_rq_failed_path_normalization" counter is incremented // for each rejected request. RejectRequestAction PathEscapedSlashAction = "RejectRequest" - // UnescapeRedirect unescapes %2F and %5C sequences and redirects to the new path + // UnescapeAndRedirect unescapes %2F and %5C sequences and redirects to the new path // if these sequences were present. // // Redirect occurs after path normalization and merge slashes transformations if @@ -30,18 +30,18 @@ const ( // proxies, Envoy and upstream server. // The “httpN.downstream_rq_redirected_with_normalized_path” counter is incremented // for each redirected request. - UnescapeRedirect PathEscapedSlashAction = "UnescapeRedirect" - // UnescapeForward unescapes %2F and %5C sequences and forwards the request. + UnescapeAndRedirect PathEscapedSlashAction = "UnescapeAndRedirect" + // UnescapeAndForward unescapes %2F and %5C sequences and forwards the request. // Note: this option should not be enabled if intermediaries perform path based access // control as it may lead to path confusion vulnerabilities. - UnescapeForward PathEscapedSlashAction = "UnescapeForward" + UnescapeAndForward PathEscapedSlashAction = "UnescapeAndForward" ) // PathSettings provides settings that managing how the incoming path set by clients is handled. type PathSettings struct { // EscapedSlashesAction determines how %2f, %2F, %5c, or %5C sequences in the path URI // should be handled. - // The default is UnescapeRedirect. + // The default is UnescapeAndRedirect. // // +optional EscapedSlashesAction *PathEscapedSlashAction `json:"escapedSlashesAction,omitempty"` diff --git a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_clienttrafficpolicies.yaml b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_clienttrafficpolicies.yaml index f92450beb0b..780fec94db7 100644 --- a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_clienttrafficpolicies.yaml +++ b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_clienttrafficpolicies.yaml @@ -66,12 +66,12 @@ spec: escapedSlashesAction: description: EscapedSlashesAction determines how %2f, %2F, %5c, or %5C sequences in the path URI should be handled. The default - is UnescapeRedirect. + is UnescapeAndRedirect. enum: - KeepUnchanged - RejectRequest - - UnescapeForward - - UnescapeRedirect + - UnescapeAndForward + - UnescapeAndRedirect type: string type: object suppressEnvoyHeaders: diff --git a/site/content/en/latest/api/extension_types.md b/site/content/en/latest/api/extension_types.md index c84f5354c93..d7961bc5f3f 100644 --- a/site/content/en/latest/api/extension_types.md +++ b/site/content/en/latest/api/extension_types.md @@ -1308,7 +1308,7 @@ _Appears in:_ | Field | Description | | --- | --- | -| `escapedSlashesAction` _[PathEscapedSlashAction](#pathescapedslashaction)_ | EscapedSlashesAction determines how %2f, %2F, %5c, or %5C sequences in the path URI should be handled. The default is UnescapeRedirect. | +| `escapedSlashesAction` _[PathEscapedSlashAction](#pathescapedslashaction)_ | EscapedSlashesAction determines how %2f, %2F, %5c, or %5C sequences in the path URI should be handled. The default is UnescapeAndRedirect. | | `disableMergeSlashes` _boolean_ | DisableMergeSlashes allows disabling the default configuration of merging adjacent slashes in the path. Note that slash merging is not part of the HTTP spec and is provided for convenience. |