Backport of [NET-11043] crd: support request normalization and header…

… match options to prevent L7 intentions bypass into release/1.1.x (#4395) backport of commit 2d3d34e Co-authored-by: Michael Zalimeni <[email protected]>
hashicorp · Oct 18, 2024 · 40259a8 · 40259a8
1 parent f4c1885
commit 40259a8
Show file tree

Hide file tree

Showing 15 changed files with 442 additions and 57 deletions.
diff --git a/.changelog/4385.txt b/.changelog/4385.txt
@@ -0,0 +1,6 @@
+```release-note:security
+crd: Add `http.incoming.requestNormalization` to the Mesh CRD to support configuring service traffic request normalization.
+```
+```release-note:security
+crd: Add `contains` and `ignoreCase` to the Intentions CRD to support configuring L7 Header intentions resilient to variable casing and multiple header values.
+```
diff --git a/acceptance/go.mod b/acceptance/go.mod
@@ -8,7 +8,7 @@ require (
 	github.com/google/uuid v1.3.0
 	github.com/gruntwork-io/terratest v0.31.2
 	github.com/hashicorp/consul-k8s/control-plane v0.0.0-20221117191905-0b1cc2b631e3
-	github.com/hashicorp/consul/api v1.21.3
+	github.com/hashicorp/consul/api v1.21.4
 	github.com/hashicorp/consul/sdk v0.16.0
 	github.com/hashicorp/go-uuid v1.0.3
 	github.com/hashicorp/go-version v1.6.0

diff --git a/acceptance/go.sum b/acceptance/go.sum
@@ -307,8 +307,8 @@ github.com/gruntwork-io/terratest v0.31.2 h1:xvYHA80MUq5kx670dM18HInewOrrQrAN+Xb
 github.com/gruntwork-io/terratest v0.31.2/go.mod h1:EEgJie28gX/4AD71IFqgMj6e99KP5mi81hEtzmDjxTo=
 github.com/hashicorp/consul-k8s/control-plane v0.0.0-20221117191905-0b1cc2b631e3 h1:4wROIZB8Y4cN/wPILChc2zQ/q00z1VyJitdgyLbITdU=
 github.com/hashicorp/consul-k8s/control-plane v0.0.0-20221117191905-0b1cc2b631e3/go.mod h1:j9Db/whkzvNC+KP2GftY0HxxleLm9swxXjlu3tYaOAw=
-github.com/hashicorp/consul/api v1.21.3 h1:hHfaZwkrxbU6TjnlmxtSrvH0mXGep8aCXBrJ9nNhJug=
-github.com/hashicorp/consul/api v1.21.3/go.mod h1:Kms78llSVzB8FtrLan+V+TSJTnfHkRGlYbhazvGzJUs=
+github.com/hashicorp/consul/api v1.21.4 h1:5g1nmbwIQ5eMZg8qhiayP/m6+WRiTv9935eFnf+zNaA=
+github.com/hashicorp/consul/api v1.21.4/go.mod h1:Kms78llSVzB8FtrLan+V+TSJTnfHkRGlYbhazvGzJUs=
 github.com/hashicorp/consul/sdk v0.16.0 h1:SE9m0W6DEfgIVCJX7xU+iv/hUl4m/nxqMTnCdMxDpJ8=
 github.com/hashicorp/consul/sdk v0.16.0/go.mod h1:7pxqqhqoaPqnBnzXD1StKed62LqJeClzVsUEy85Zr0A=
 github.com/hashicorp/errwrap v1.0.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4=

diff --git a/charts/consul/templates/crd-meshes.yaml b/charts/consul/templates/crd-meshes.yaml
@@ -62,10 +62,55 @@ spec:
               http:
                 description: HTTP defines the HTTP configuration for the service mesh.
                 properties:
+                  incoming:
+                    description: Incoming configures settings for incoming HTTP traffic
+                      to mesh proxies.
+                    properties:
+                      requestNormalization:
+                        description: |-
+                          RequestNormalizationMeshConfig contains options pertaining to the
+                          normalization of HTTP requests processed by mesh proxies.
+                        properties:
+                          headersWithUnderscoresAction:
+                            description: |-
+                              HeadersWithUnderscoresAction sets the value of the \`headers_with_underscores_action\` option in the Envoy
+                              listener's \`HttpConnectionManager\` under \`common_http_protocol_options\`. The default value of this option is
+                              empty, which is equivalent to \`ALLOW\`. Refer to the Envoy documentation for more information on available
+                              options.
+                            type: string
+                          insecureDisablePathNormalization:
+                            description: |-
+                              InsecureDisablePathNormalization sets the value of the \`normalize_path\` option in the Envoy listener's
+                              `HttpConnectionManager`. The default value is \`false\`. When set to \`true\` in Consul, \`normalize_path\` is
+                              set to \`false\` for the Envoy proxy. This parameter disables the normalization of request URL paths according to
+                              RFC 3986, conversion of \`\\\` to \`/\`, and decoding non-reserved %-encoded characters. When using L7 intentions
+                              with path match rules, we recommend enabling path normalization in order to avoid match rule circumvention with
+                              non-normalized path values.
+                            type: boolean
+                          mergeSlashes:
+                            description: |-
+                              MergeSlashes sets the value of the \`merge_slashes\` option in the Envoy listener's \`HttpConnectionManager\`.
+                              The default value is \`false\`. This option controls the normalization of request URL paths by merging
+                              consecutive \`/\` characters. This normalization is not part of RFC 3986. When using L7 intentions with path
+                              match rules, we recommend enabling this setting to avoid match rule circumvention through non-normalized path
+                              values, unless legitimate service traffic depends on allowing for repeat \`/\` characters, or upstream services
+                              are configured to differentiate between single and multiple slashes.
+                            type: boolean
+                          pathWithEscapedSlashesAction:
+                            description: |-
+                              PathWithEscapedSlashesAction sets the value of the \`path_with_escaped_slashes_action\` option in the Envoy
+                              listener's \`HttpConnectionManager\`. The default value of this option is empty, which is equivalent to
+                              \`IMPLEMENTATION_SPECIFIC_DEFAULT\`. This parameter controls the action taken in response to request URL paths
+                              with escaped slashes in the path. When using L7 intentions with path match rules, we recommend enabling this
+                              setting to avoid match rule circumvention through non-normalized path values, unless legitimate service traffic
+                              depends on allowing for escaped \`/\` or \`\\\` characters, or upstream services are configured to differentiate
+                              between escaped and unescaped slashes. Refer to the Envoy documentation for more information on available
+                              options.
+                            type: string
+                        type: object
+                    type: object
                   sanitizeXForwardedClientCert:
                     type: boolean
-                required:
-                - sanitizeXForwardedClientCert
                 type: object
               peering:
                 description: Peering defines the peering configuration for the service

diff --git a/charts/consul/templates/crd-serviceintentions.yaml b/charts/consul/templates/crd-serviceintentions.yaml
@@ -134,10 +134,19 @@ spec:
                                   If more than one is configured all must match for the overall match to apply.
                                 items:
                                   properties:
+                                    contains:
+                                      description: Contains matches if the header
+                                        with the given name contains this value.
+                                      type: string
                                     exact:
                                       description: Exact matches if the header with
                                         the given name is this value.
                                       type: string
+                                    ignoreCase:
+                                      description: IgnoreCase ignores the case of
+                                        the header value when matching with exact,
+                                        prefix, suffix, or contains.
+                                      type: boolean
                                     invert:
                                       description: Invert inverts the logic of the
                                         match.

diff --git a/cli/go.mod b/cli/go.mod
@@ -96,7 +96,7 @@ require (
 	github.com/gorilla/websocket v1.5.0 // indirect
 	github.com/gosuri/uitable v0.0.4 // indirect
 	github.com/gregjones/httpcache v0.0.0-20180305231024-9cad4c3443a7 // indirect
-	github.com/hashicorp/consul/api v1.21.3 // indirect
+	github.com/hashicorp/consul/api v1.21.4 // indirect
 	github.com/hashicorp/consul/envoyextensions v0.2.2 // indirect
 	github.com/hashicorp/errwrap v1.1.0 // indirect
 	github.com/hashicorp/go-cleanhttp v0.5.2 // indirect

diff --git a/cli/go.sum b/cli/go.sum
@@ -285,8 +285,8 @@ github.com/gosuri/uitable v0.0.4 h1:IG2xLKRvErL3uhY6e1BylFzG+aJiwQviDDTfOKeKTpY=
 github.com/gosuri/uitable v0.0.4/go.mod h1:tKR86bXuXPZazfOTG1FIzvjIdXzd0mo4Vtn16vt0PJo=
 github.com/gregjones/httpcache v0.0.0-20180305231024-9cad4c3443a7 h1:pdN6V1QBWetyv/0+wjACpqVH+eVULgEjkurDLq3goeM=
 github.com/gregjones/httpcache v0.0.0-20180305231024-9cad4c3443a7/go.mod h1:FecbI9+v66THATjSRHfNgh1IVFe/9kFxbXtjV0ctIMA=
-github.com/hashicorp/consul/api v1.21.3 h1:hHfaZwkrxbU6TjnlmxtSrvH0mXGep8aCXBrJ9nNhJug=
-github.com/hashicorp/consul/api v1.21.3/go.mod h1:Kms78llSVzB8FtrLan+V+TSJTnfHkRGlYbhazvGzJUs=
+github.com/hashicorp/consul/api v1.21.4 h1:5g1nmbwIQ5eMZg8qhiayP/m6+WRiTv9935eFnf+zNaA=
+github.com/hashicorp/consul/api v1.21.4/go.mod h1:Kms78llSVzB8FtrLan+V+TSJTnfHkRGlYbhazvGzJUs=
 github.com/hashicorp/consul/envoyextensions v0.2.2 h1:AriKSqZEXkfRemBuWMkU9b9ZtvGIaBhg+4ZAYtbovNU=
 github.com/hashicorp/consul/envoyextensions v0.2.2/go.mod h1:b8muOXsWk/d+spHj7fc0mJMzFd5JVRPPueOsej4fSkk=
 github.com/hashicorp/consul/sdk v0.13.1 h1:EygWVWWMczTzXGpO93awkHFzfUka6hLYJ0qhETd+6lY=

diff --git a/control-plane/api/v1alpha1/mesh_types.go b/control-plane/api/v1alpha1/mesh_types.go
@@ -84,7 +84,18 @@ type MeshTLSConfig struct {
 }
 
 type MeshHTTPConfig struct {
-	SanitizeXForwardedClientCert bool `json:"sanitizeXForwardedClientCert"`
+	SanitizeXForwardedClientCert bool `json:"sanitizeXForwardedClientCert,omitempty"`
+	// Incoming configures settings for incoming HTTP traffic to mesh proxies.
+	Incoming *MeshDirectionalHTTPConfig `json:"incoming,omitempty"`
+	// There is not currently an outgoing MeshDirectionalHTTPConfig, as
+	// the only required config for either direction at present is inbound
+	// request normalization.
+}
+
+// MeshDirectionalHTTPConfig holds mesh configuration specific to HTTP
+// requests for a given traffic direction.
+type MeshDirectionalHTTPConfig struct {
+	RequestNormalization *RequestNormalizationMeshConfig `json:"requestNormalization,omitempty"`
 }
 
 type PeeringMeshConfig struct {
@@ -114,6 +125,61 @@ type MeshDirectionalTLSConfig struct {
 	CipherSuites []string `json:"cipherSuites,omitempty"`
 }
 
+// RequestNormalizationMeshConfig contains options pertaining to the
+// normalization of HTTP requests processed by mesh proxies.
+type RequestNormalizationMeshConfig struct {
+	// InsecureDisablePathNormalization sets the value of the \`normalize_path\` option in the Envoy listener's
+	// `HttpConnectionManager`. The default value is \`false\`. When set to \`true\` in Consul, \`normalize_path\` is
+	// set to \`false\` for the Envoy proxy. This parameter disables the normalization of request URL paths according to
+	// RFC 3986, conversion of \`\\\` to \`/\`, and decoding non-reserved %-encoded characters. When using L7 intentions
+	// with path match rules, we recommend enabling path normalization in order to avoid match rule circumvention with
+	// non-normalized path values.
+	InsecureDisablePathNormalization bool `json:"insecureDisablePathNormalization,omitempty"`
+	// MergeSlashes sets the value of the \`merge_slashes\` option in the Envoy listener's \`HttpConnectionManager\`.
+	// The default value is \`false\`. This option controls the normalization of request URL paths by merging
+	// consecutive \`/\` characters. This normalization is not part of RFC 3986. When using L7 intentions with path
+	// match rules, we recommend enabling this setting to avoid match rule circumvention through non-normalized path
+	// values, unless legitimate service traffic depends on allowing for repeat \`/\` characters, or upstream services
+	// are configured to differentiate between single and multiple slashes.
+	MergeSlashes bool `json:"mergeSlashes,omitempty"`
+	// PathWithEscapedSlashesAction sets the value of the \`path_with_escaped_slashes_action\` option in the Envoy
+	// listener's \`HttpConnectionManager\`. The default value of this option is empty, which is equivalent to
+	// \`IMPLEMENTATION_SPECIFIC_DEFAULT\`. This parameter controls the action taken in response to request URL paths
+	// with escaped slashes in the path. When using L7 intentions with path match rules, we recommend enabling this
+	// setting to avoid match rule circumvention through non-normalized path values, unless legitimate service traffic
+	// depends on allowing for escaped \`/\` or \`\\\` characters, or upstream services are configured to differentiate
+	// between escaped and unescaped slashes. Refer to the Envoy documentation for more information on available
+	// options.
+	PathWithEscapedSlashesAction string `json:"pathWithEscapedSlashesAction,omitempty"`
+	// HeadersWithUnderscoresAction sets the value of the \`headers_with_underscores_action\` option in the Envoy
+	// listener's \`HttpConnectionManager\` under \`common_http_protocol_options\`. The default value of this option is
+	// empty, which is equivalent to \`ALLOW\`. Refer to the Envoy documentation for more information on available
+	// options.
+	HeadersWithUnderscoresAction string `json:"headersWithUnderscoresAction,omitempty"`
+}
+
+// PathWithEscapedSlashesAction is an enum that defines the action to take when
+// a request path contains escaped slashes. It mirrors exactly the set of options
+// in Envoy's UriPathNormalizationOptions.PathWithEscapedSlashesAction enum.
+// See github.com/envoyproxy/go-control-plane envoy_http_v3.HttpConnectionManager_PathWithEscapedSlashesAction.
+const (
+	PathWithEscapedSlashesActionDefault             = "IMPLEMENTATION_SPECIFIC_DEFAULT"
+	PathWithEscapedSlashesActionKeep                = "KEEP_UNCHANGED"
+	PathWithEscapedSlashesActionReject              = "REJECT_REQUEST"
+	PathWithEscapedSlashesActionUnescapeAndRedirect = "UNESCAPE_AND_REDIRECT"
+	PathWithEscapedSlashesActionUnescapeAndForward  = "UNESCAPE_AND_FORWARD"
+)
+
+// HeadersWithUnderscoresAction is an enum that defines the action to take when
+// a request contains headers with underscores. It mirrors exactly the set of
+// options in Envoy's HttpProtocolOptions.HeadersWithUnderscoresAction enum.
+// See github.com/envoyproxy/go-control-plane envoy_core_v3.HttpProtocolOptions_HeadersWithUnderscoresAction.
+const (
+	HeadersWithUnderscoresActionAllow         = "ALLOW"
+	HeadersWithUnderscoresActionRejectRequest = "REJECT_REQUEST"
+	HeadersWithUnderscoresActionDropHeader    = "DROP_HEADER"
+)
+
 func (in *TransparentProxyMeshConfig) toConsul() capi.TransparentProxyMeshConfig {
 	return capi.TransparentProxyMeshConfig{MeshDestinationsOnly: in.MeshDestinationsOnly}
 }
@@ -223,6 +289,11 @@ func (in *Mesh) Validate(consulMeta common.ConsulMeta) error {
 
 	errs = append(errs, in.Spec.TLS.validate(path.Child("tls"))...)
 	errs = append(errs, in.Spec.Peering.validate(path.Child("peering"), consulMeta.PartitionsEnabled, consulMeta.Partition)...)
+	if in.Spec.HTTP != nil &&
+		in.Spec.HTTP.Incoming != nil &&
+		in.Spec.HTTP.Incoming.RequestNormalization != nil {
+		errs = append(errs, in.Spec.HTTP.Incoming.RequestNormalization.validate(path.Child("http", "incoming", "requestNormalization"))...)
+	}
 
 	if len(errs) > 0 {
 		return apierrors.NewInvalid(
@@ -248,6 +319,28 @@ func (in *MeshHTTPConfig) toConsul() *capi.MeshHTTPConfig {
 	}
 	return &capi.MeshHTTPConfig{
 		SanitizeXForwardedClientCert: in.SanitizeXForwardedClientCert,
+		Incoming:                     in.Incoming.toConsul(),
+	}
+}
+
+func (in *MeshDirectionalHTTPConfig) toConsul() *capi.MeshDirectionalHTTPConfig {
+	if in == nil {
+		return nil
+	}
+	return &capi.MeshDirectionalHTTPConfig{
+		RequestNormalization: in.RequestNormalization.toConsul(),
+	}
+}
+
+func (in *RequestNormalizationMeshConfig) toConsul() *capi.RequestNormalizationMeshConfig {
+	if in == nil {
+		return nil
+	}
+	return &capi.RequestNormalizationMeshConfig{
+		InsecureDisablePathNormalization: in.InsecureDisablePathNormalization,
+		MergeSlashes:                     in.MergeSlashes,
+		PathWithEscapedSlashesAction:     in.PathWithEscapedSlashesAction,
+		HeadersWithUnderscoresAction:     in.HeadersWithUnderscoresAction,
 	}
 }
 
@@ -312,6 +405,36 @@ func (in *PeeringMeshConfig) validate(path *field.Path, partitionsEnabled bool,
 	return errs
 }
 
+func (in *RequestNormalizationMeshConfig) validate(path *field.Path) field.ErrorList {
+	if in == nil {
+		return nil
+	}
+
+	var errs field.ErrorList
+	pathWithEscapedSlashesActions := []string{
+		PathWithEscapedSlashesActionDefault,
+		PathWithEscapedSlashesActionKeep,
+		PathWithEscapedSlashesActionReject,
+		PathWithEscapedSlashesActionUnescapeAndRedirect,
+		PathWithEscapedSlashesActionUnescapeAndForward,
+		"",
+	}
+	headersWithUnderscoresActions := []string{
+		HeadersWithUnderscoresActionAllow,
+		HeadersWithUnderscoresActionRejectRequest,
+		HeadersWithUnderscoresActionDropHeader,
+		"",
+	}
+
+	if !sliceContains(pathWithEscapedSlashesActions, in.PathWithEscapedSlashesAction) {
+		errs = append(errs, field.Invalid(path.Child("pathWithEscapedSlashesAction"), in.PathWithEscapedSlashesAction, notInSliceMessage(pathWithEscapedSlashesActions)))
+	}
+	if !sliceContains(headersWithUnderscoresActions, in.HeadersWithUnderscoresAction) {
+		errs = append(errs, field.Invalid(path.Child("headersWithUnderscoresAction"), in.HeadersWithUnderscoresAction, notInSliceMessage(headersWithUnderscoresActions)))
+	}
+	return errs
+}
+
 // DefaultNamespaceFields has no behaviour here as meshes have no namespace specific fields.
 func (in *Mesh) DefaultNamespaceFields(_ common.ConsulMeta) {
 }