From 5840c1707e264f3efb6e5afef4239f0d03c1f6bb Mon Sep 17 00:00:00 2001 From: Michael Zalimeni Date: Thu, 17 Oct 2024 18:17:25 -0400 Subject: [PATCH] backport of commit 2d3d34e5548ad6f6ee3e7c502bcab6af271ae152 --- .changelog/4385.txt | 6 + acceptance/go.mod | 2 +- acceptance/go.sum | 4 +- charts/consul/templates/crd-meshes.yaml | 49 ++++++- .../templates/crd-serviceintentions.yaml | 9 ++ cli/go.mod | 2 +- cli/go.sum | 6 +- control-plane/api/v1alpha1/mesh_types.go | 125 +++++++++++++++++- control-plane/api/v1alpha1/mesh_types_test.go | 112 ++++++++++++++++ .../api/v1alpha1/serviceintentions_types.go | 28 ++-- .../v1alpha1/serviceintentions_types_test.go | 92 ++++++++----- .../bases/consul.hashicorp.com_meshes.yaml | 49 ++++++- ...onsul.hashicorp.com_serviceintentions.yaml | 9 ++ control-plane/go.mod | 4 +- control-plane/go.sum | 4 +- control-plane/helper/test/test_util.go | 2 +- 16 files changed, 443 insertions(+), 60 deletions(-) create mode 100644 .changelog/4385.txt diff --git a/.changelog/4385.txt b/.changelog/4385.txt new file mode 100644 index 0000000000..2d45c62919 --- /dev/null +++ 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 index 6b5ecf82e2..1265e79ec5 100644 --- a/acceptance/go.mod +++ b/acceptance/go.mod @@ -9,7 +9,7 @@ require ( github.com/google/uuid v1.3.0 github.com/gruntwork-io/terratest v0.46.7 github.com/hashicorp/consul-k8s/control-plane v0.0.0-20240226161840-f3842c41cb2b - github.com/hashicorp/consul/api v1.28.4 + github.com/hashicorp/consul/api v1.28.5 github.com/hashicorp/consul/proto-public v0.6.1 github.com/hashicorp/consul/sdk v0.16.0 github.com/hashicorp/go-multierror v1.1.1 diff --git a/acceptance/go.sum b/acceptance/go.sum index 0a9df640d8..d53d328651 100644 --- a/acceptance/go.sum +++ b/acceptance/go.sum @@ -186,8 +186,8 @@ github.com/gruntwork-io/terratest v0.46.7 h1:oqGPBBO87SEsvBYaA0R5xOq+Lm2Xc5dmFVf github.com/gruntwork-io/terratest v0.46.7/go.mod h1:6gI5MlLeyF+SLwqocA5GBzcTix+XiuxCy1BPwKuT+WM= github.com/hashicorp/consul-k8s/control-plane v0.0.0-20240226161840-f3842c41cb2b h1:AdeWjUb+rxrRryC5ZHaL32oOZuxubOzV2q6oJ97UMT0= github.com/hashicorp/consul-k8s/control-plane v0.0.0-20240226161840-f3842c41cb2b/go.mod h1:TVaSJM7vYM/mtKGpVc/Lch53lrqLI9XAXJgy/gY8v4A= -github.com/hashicorp/consul/api v1.28.4 h1:l/d3utZ7ITzIjd8OXznuSTbBnWFHGP2VE8WdGNFrhjE= -github.com/hashicorp/consul/api v1.28.4/go.mod h1:GE+eY9pliiJiUXGchDpTaMVp27rSksHG+U+5jY+uJR4= +github.com/hashicorp/consul/api v1.28.5 h1:WscnEAY/opiVDpXFEE8bajxtf/ZKypVIYeO1lG0m5nI= +github.com/hashicorp/consul/api v1.28.5/go.mod h1:PbuTRqH1ejk45b1+duKXVQwnx+mS1f8gdna/zbQLNi4= github.com/hashicorp/consul/proto-public v0.6.1 h1:+uzH3olCrksXYWAYHKqK782CtK9scfqH+Unlw3UHhCg= github.com/hashicorp/consul/proto-public v0.6.1/go.mod h1:cXXbOg74KBNGajC+o8RlA502Esf0R9prcoJgiOX/2Tg= github.com/hashicorp/consul/sdk v0.16.0 h1:SE9m0W6DEfgIVCJX7xU+iv/hUl4m/nxqMTnCdMxDpJ8= diff --git a/charts/consul/templates/crd-meshes.yaml b/charts/consul/templates/crd-meshes.yaml index f81e61a2c5..9ecc1f3cbc 100644 --- a/charts/consul/templates/crd-meshes.yaml +++ b/charts/consul/templates/crd-meshes.yaml @@ -66,10 +66,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 index 72159ec187..09daae2f6c 100644 --- a/charts/consul/templates/crd-serviceintentions.yaml +++ b/charts/consul/templates/crd-serviceintentions.yaml @@ -168,10 +168,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 index 8400f9f545..e3fb176697 100644 --- 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.28.4 // indirect + github.com/hashicorp/consul/api v1.28.5 // indirect github.com/hashicorp/consul/envoyextensions v0.6.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 index 3539ad7aa7..913ddea35b 100644 --- a/cli/go.sum +++ b/cli/go.sum @@ -288,12 +288,10 @@ 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.28.4 h1:l/d3utZ7ITzIjd8OXznuSTbBnWFHGP2VE8WdGNFrhjE= -github.com/hashicorp/consul/api v1.28.4/go.mod h1:GE+eY9pliiJiUXGchDpTaMVp27rSksHG+U+5jY+uJR4= +github.com/hashicorp/consul/api v1.28.5 h1:WscnEAY/opiVDpXFEE8bajxtf/ZKypVIYeO1lG0m5nI= +github.com/hashicorp/consul/api v1.28.5/go.mod h1:PbuTRqH1ejk45b1+duKXVQwnx+mS1f8gdna/zbQLNi4= github.com/hashicorp/consul/envoyextensions v0.6.2 h1:ljrE/JMWJxw/AXY//plKp8IxAw9131Y2mX45l0lt/+E= github.com/hashicorp/consul/envoyextensions v0.6.2/go.mod h1:SWG7x/XUFlQje//G3sCAjn5gt4S+oy6t6l+RvfXTq38= -github.com/hashicorp/consul/proto-public v0.6.1 h1:+uzH3olCrksXYWAYHKqK782CtK9scfqH+Unlw3UHhCg= -github.com/hashicorp/consul/proto-public v0.6.1/go.mod h1:cXXbOg74KBNGajC+o8RlA502Esf0R9prcoJgiOX/2Tg= 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/consul/troubleshoot v0.6.5 h1:RFL4FGzI1pSCqjc4E49hXyQDajyswC0zQmSKdpm1q8Q= diff --git a/control-plane/api/v1alpha1/mesh_types.go b/control-plane/api/v1alpha1/mesh_types.go index 4d8a14358b..f70167a912 100644 --- a/control-plane/api/v1alpha1/mesh_types.go +++ b/control-plane/api/v1alpha1/mesh_types.go @@ -87,7 +87,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 { @@ -117,6 +128,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} } @@ -227,6 +293,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( @@ -252,6 +323,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, } } @@ -316,6 +409,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) { } diff --git a/control-plane/api/v1alpha1/mesh_types_test.go b/control-plane/api/v1alpha1/mesh_types_test.go index f5e7ede187..aedff6559d 100644 --- a/control-plane/api/v1alpha1/mesh_types_test.go +++ b/control-plane/api/v1alpha1/mesh_types_test.go @@ -64,6 +64,14 @@ func TestMesh_MatchesConsul(t *testing.T) { }, HTTP: &MeshHTTPConfig{ SanitizeXForwardedClientCert: true, + Incoming: &MeshDirectionalHTTPConfig{ + RequestNormalization: &RequestNormalizationMeshConfig{ + InsecureDisablePathNormalization: true, // note: this is the opposite of the recommended default + MergeSlashes: true, + PathWithEscapedSlashesAction: "REJECT_REQUEST", + HeadersWithUnderscoresAction: "DROP_HEADER", + }, + }, }, Peering: &PeeringMeshConfig{ PeerThroughMeshGateways: true, @@ -90,6 +98,14 @@ func TestMesh_MatchesConsul(t *testing.T) { }, HTTP: &capi.MeshHTTPConfig{ SanitizeXForwardedClientCert: true, + Incoming: &capi.MeshDirectionalHTTPConfig{ + RequestNormalization: &capi.RequestNormalizationMeshConfig{ + InsecureDisablePathNormalization: true, // note: this is the opposite of the recommended default + MergeSlashes: true, + PathWithEscapedSlashesAction: "REJECT_REQUEST", + HeadersWithUnderscoresAction: "DROP_HEADER", + }, + }, }, Peering: &capi.PeeringMeshConfig{ PeerThroughMeshGateways: true, @@ -168,6 +184,14 @@ func TestMesh_ToConsul(t *testing.T) { }, HTTP: &MeshHTTPConfig{ SanitizeXForwardedClientCert: true, + Incoming: &MeshDirectionalHTTPConfig{ + RequestNormalization: &RequestNormalizationMeshConfig{ + InsecureDisablePathNormalization: true, // note: this is the opposite of the recommended default + MergeSlashes: true, + PathWithEscapedSlashesAction: "REJECT_REQUEST", + HeadersWithUnderscoresAction: "DROP_HEADER", + }, + }, }, Peering: &PeeringMeshConfig{ PeerThroughMeshGateways: true, @@ -194,6 +218,14 @@ func TestMesh_ToConsul(t *testing.T) { }, HTTP: &capi.MeshHTTPConfig{ SanitizeXForwardedClientCert: true, + Incoming: &capi.MeshDirectionalHTTPConfig{ + RequestNormalization: &capi.RequestNormalizationMeshConfig{ + InsecureDisablePathNormalization: true, // note: this is the opposite of the recommended default + MergeSlashes: true, + PathWithEscapedSlashesAction: "REJECT_REQUEST", + HeadersWithUnderscoresAction: "DROP_HEADER", + }, + }, }, Peering: &capi.PeeringMeshConfig{ PeerThroughMeshGateways: true, @@ -367,6 +399,76 @@ func TestMesh_Validate(t *testing.T) { }, }, }, + "http.incoming.requestNormalization.pathWithEscapedSlashesAction valid": { + input: &Mesh{ + ObjectMeta: metav1.ObjectMeta{ + Name: "name", + }, + Spec: MeshSpec{ + HTTP: &MeshHTTPConfig{ + Incoming: &MeshDirectionalHTTPConfig{ + RequestNormalization: &RequestNormalizationMeshConfig{ + PathWithEscapedSlashesAction: "UNESCAPE_AND_FORWARD", + }, + }, + }, + }, + }, + }, + "http.incoming.requestNormalization.pathWithEscapedSlashesAction invalid": { + input: &Mesh{ + ObjectMeta: metav1.ObjectMeta{ + Name: "name", + }, + Spec: MeshSpec{ + HTTP: &MeshHTTPConfig{ + Incoming: &MeshDirectionalHTTPConfig{ + RequestNormalization: &RequestNormalizationMeshConfig{ + PathWithEscapedSlashesAction: "foo", + }, + }, + }, + }, + }, + expectedErrMsgs: []string{ + `spec.http.incoming.requestNormalization.pathWithEscapedSlashesAction: Invalid value: "foo": must be one of "IMPLEMENTATION_SPECIFIC_DEFAULT", "KEEP_UNCHANGED", "REJECT_REQUEST", "UNESCAPE_AND_REDIRECT", "UNESCAPE_AND_FORWARD", ""`, + }, + }, + "http.incoming.requestNormalization.headerWithUnderscoresAction valid": { + input: &Mesh{ + ObjectMeta: metav1.ObjectMeta{ + Name: "name", + }, + Spec: MeshSpec{ + HTTP: &MeshHTTPConfig{ + Incoming: &MeshDirectionalHTTPConfig{ + RequestNormalization: &RequestNormalizationMeshConfig{ + HeadersWithUnderscoresAction: "REJECT_REQUEST", + }, + }, + }, + }, + }, + }, + "http.incoming.requestNormalization.headersWithUnderscoresAction invalid": { + input: &Mesh{ + ObjectMeta: metav1.ObjectMeta{ + Name: "name", + }, + Spec: MeshSpec{ + HTTP: &MeshHTTPConfig{ + Incoming: &MeshDirectionalHTTPConfig{ + RequestNormalization: &RequestNormalizationMeshConfig{ + HeadersWithUnderscoresAction: "bar", + }, + }, + }, + }, + }, + expectedErrMsgs: []string{ + `spec.http.incoming.requestNormalization.headersWithUnderscoresAction: Invalid value: "bar": must be one of "ALLOW", "REJECT_REQUEST", "DROP_HEADER", ""`, + }, + }, "multiple errors": { input: &Mesh{ ObjectMeta: metav1.ObjectMeta{ @@ -386,6 +488,14 @@ func TestMesh_Validate(t *testing.T) { Peering: &PeeringMeshConfig{ PeerThroughMeshGateways: true, }, + HTTP: &MeshHTTPConfig{ + Incoming: &MeshDirectionalHTTPConfig{ + RequestNormalization: &RequestNormalizationMeshConfig{ + PathWithEscapedSlashesAction: "foo", + HeadersWithUnderscoresAction: "bar", + }, + }, + }, }, }, consulMeta: common.ConsulMeta{ @@ -398,6 +508,8 @@ func TestMesh_Validate(t *testing.T) { `spec.tls.outgoing.tlsMinVersion: Invalid value: "foo": must be one of "TLS_AUTO", "TLSv1_0", "TLSv1_1", "TLSv1_2", "TLSv1_3", ""`, `spec.tls.outgoing.tlsMaxVersion: Invalid value: "bar": must be one of "TLS_AUTO", "TLSv1_0", "TLSv1_1", "TLSv1_2", "TLSv1_3", ""`, `spec.peering.peerThroughMeshGateways: Forbidden: "peerThroughMeshGateways" is only valid in the "default" partition`, + `spec.http.incoming.requestNormalization.pathWithEscapedSlashesAction: Invalid value: "foo": must be one of "IMPLEMENTATION_SPECIFIC_DEFAULT", "KEEP_UNCHANGED", "REJECT_REQUEST", "UNESCAPE_AND_REDIRECT", "UNESCAPE_AND_FORWARD", ""`, + `spec.http.incoming.requestNormalization.headersWithUnderscoresAction: Invalid value: "bar": must be one of "ALLOW", "REJECT_REQUEST", "DROP_HEADER", ""`, }, }, } diff --git a/control-plane/api/v1alpha1/serviceintentions_types.go b/control-plane/api/v1alpha1/serviceintentions_types.go index d393f72a2d..17d0a9cf2a 100644 --- a/control-plane/api/v1alpha1/serviceintentions_types.go +++ b/control-plane/api/v1alpha1/serviceintentions_types.go @@ -140,8 +140,12 @@ type IntentionHTTPHeaderPermission struct { Prefix string `json:"prefix,omitempty"` // Suffix matches if the header with the given name has this suffix. Suffix string `json:"suffix,omitempty"` + // Contains matches if the header with the given name contains this value. + Contains string `json:"contains,omitempty"` // Regex matches if the header with the given name matches this pattern. Regex string `json:"regex,omitempty"` + // IgnoreCase ignores the case of the header value when matching with exact, prefix, suffix, or contains. + IgnoreCase bool `json:"ignoreCase,omitempty"` // Invert inverts the logic of the match. Invert bool `json:"invert,omitempty"` } @@ -448,13 +452,15 @@ func (in IntentionHTTPHeaderPermissions) toConsul() []capi.IntentionHTTPHeaderPe var headerPermissions []capi.IntentionHTTPHeaderPermission for _, permission := range in { headerPermissions = append(headerPermissions, capi.IntentionHTTPHeaderPermission{ - Name: permission.Name, - Present: permission.Present, - Exact: permission.Exact, - Prefix: permission.Prefix, - Suffix: permission.Suffix, - Regex: permission.Regex, - Invert: permission.Invert, + Name: permission.Name, + Present: permission.Present, + Exact: permission.Exact, + Prefix: permission.Prefix, + Suffix: permission.Suffix, + Contains: permission.Contains, + Regex: permission.Regex, + Invert: permission.Invert, + IgnoreCase: permission.IgnoreCase, }) } @@ -568,10 +574,14 @@ func (in IntentionHTTPHeaderPermissions) validate(path *field.Path) field.ErrorL if permission.Present { hdrParts++ } - hdrParts += numNotEmpty(permission.Exact, permission.Regex, permission.Prefix, permission.Suffix) + hdrParts += numNotEmpty(permission.Exact, permission.Regex, permission.Prefix, permission.Suffix, permission.Contains) if hdrParts > 1 { asJson, _ := json.Marshal(in[i]) - errs = append(errs, field.Invalid(path.Index(i), string(asJson), `at most only one of exact, prefix, suffix, regex, or present may be configured.`)) + errs = append(errs, field.Invalid(path.Index(i), string(asJson), `at most only one of exact, prefix, suffix, contains, regex, or present may be configured.`)) + } + if permission.IgnoreCase && (permission.Present || permission.Regex != "") { + asJson, _ := json.Marshal(in[i]) + errs = append(errs, field.Invalid(path.Index(i), string(asJson), `should set one of exact, prefix, suffix, or contains when using ignoreCase.`)) } } return errs diff --git a/control-plane/api/v1alpha1/serviceintentions_types_test.go b/control-plane/api/v1alpha1/serviceintentions_types_test.go index 8d0a6d907a..fb2ae743b5 100644 --- a/control-plane/api/v1alpha1/serviceintentions_types_test.go +++ b/control-plane/api/v1alpha1/serviceintentions_types_test.go @@ -151,13 +151,15 @@ func TestServiceIntentions_MatchesConsul(t *testing.T) { PathRegex: "/baz", Header: IntentionHTTPHeaderPermissions{ { - Name: "header", - Present: true, - Exact: "exact", - Prefix: "prefix", - Suffix: "suffix", - Regex: "regex", - Invert: true, + Name: "header", + Present: true, + Exact: "exact", + Prefix: "prefix", + Suffix: "suffix", + Contains: "contains", + Regex: "regex", + Invert: true, + IgnoreCase: true, }, }, Methods: []string{ @@ -232,13 +234,15 @@ func TestServiceIntentions_MatchesConsul(t *testing.T) { PathRegex: "/baz", Header: []capi.IntentionHTTPHeaderPermission{ { - Name: "header", - Present: true, - Exact: "exact", - Prefix: "prefix", - Suffix: "suffix", - Regex: "regex", - Invert: true, + Name: "header", + Present: true, + Exact: "exact", + Prefix: "prefix", + Suffix: "suffix", + Contains: "contains", + Regex: "regex", + Invert: true, + IgnoreCase: true, }, }, Methods: []string{ @@ -414,13 +418,15 @@ func TestServiceIntentions_ToConsul(t *testing.T) { PathRegex: "/baz", Header: IntentionHTTPHeaderPermissions{ { - Name: "header", - Present: true, - Exact: "exact", - Prefix: "prefix", - Suffix: "suffix", - Regex: "regex", - Invert: true, + Name: "header", + Present: true, + Exact: "exact", + Prefix: "prefix", + Suffix: "suffix", + Contains: "contains", + Regex: "regex", + Invert: true, + IgnoreCase: true, }, }, Methods: []string{ @@ -500,13 +506,15 @@ func TestServiceIntentions_ToConsul(t *testing.T) { PathRegex: "/baz", Header: []capi.IntentionHTTPHeaderPermission{ { - Name: "header", - Present: true, - Exact: "exact", - Prefix: "prefix", - Suffix: "suffix", - Regex: "regex", - Invert: true, + Name: "header", + Present: true, + Exact: "exact", + Prefix: "prefix", + Suffix: "suffix", + Contains: "contains", + Regex: "regex", + Invert: true, + IgnoreCase: true, }, }, Methods: []string{ @@ -1258,11 +1266,26 @@ func TestServiceIntentions_Validate(t *testing.T) { Suffix: "bar", Regex: "foo", }, + { + Name: "suffix-contains", + Suffix: "bar", + Contains: "foo", + }, { Name: "regex-present", Present: true, Regex: "foobar", }, + { + Name: "present-ignoreCase", + Present: true, + IgnoreCase: true, + }, + { + Name: "regex-ignoreCase", + Regex: "foobar", + IgnoreCase: true, + }, }, }, }, @@ -1273,11 +1296,14 @@ func TestServiceIntentions_Validate(t *testing.T) { }, namespacesEnabled: true, expectedErrMsgs: []string{ - `spec.sources[0].permissions[0].header[0]: Invalid value: "{\"name\":\"exact-present\",\"present\":true,\"exact\":\"foobar\"}": at most only one of exact, prefix, suffix, regex, or present may be configured.`, - `spec.sources[0].permissions[0].header[1]: Invalid value: "{\"name\":\"prefix-exact\",\"exact\":\"foobar\",\"prefix\":\"barfood\"}": at most only one of exact, prefix, suffix, regex, or present may be configured.`, - `spec.sources[0].permissions[0].header[2]: Invalid value: "{\"name\":\"suffix-prefix\",\"prefix\":\"foo\",\"suffix\":\"bar\"}": at most only one of exact, prefix, suffix, regex, or present may be configured.`, - `spec.sources[0].permissions[0].header[3]: Invalid value: "{\"name\":\"suffix-regex\",\"suffix\":\"bar\",\"regex\":\"foo\"}": at most only one of exact, prefix, suffix, regex, or present may be configured.`, - `spec.sources[0].permissions[0].header[4]: Invalid value: "{\"name\":\"regex-present\",\"present\":true,\"regex\":\"foobar\"}": at most only one of exact, prefix, suffix, regex, or present may be configured.`, + `spec.sources[0].permissions[0].header[0]: Invalid value: "{\"name\":\"exact-present\",\"present\":true,\"exact\":\"foobar\"}": at most only one of exact, prefix, suffix, contains, regex, or present may be configured.`, + `spec.sources[0].permissions[0].header[1]: Invalid value: "{\"name\":\"prefix-exact\",\"exact\":\"foobar\",\"prefix\":\"barfood\"}": at most only one of exact, prefix, suffix, contains, regex, or present may be configured.`, + `spec.sources[0].permissions[0].header[2]: Invalid value: "{\"name\":\"suffix-prefix\",\"prefix\":\"foo\",\"suffix\":\"bar\"}": at most only one of exact, prefix, suffix, contains, regex, or present may be configured.`, + `spec.sources[0].permissions[0].header[3]: Invalid value: "{\"name\":\"suffix-regex\",\"suffix\":\"bar\",\"regex\":\"foo\"}": at most only one of exact, prefix, suffix, contains, regex, or present may be configured.`, + `spec.sources[0].permissions[0].header[4]: Invalid value: "{\"name\":\"suffix-contains\",\"suffix\":\"bar\",\"contains\":\"foo\"}": at most only one of exact, prefix, suffix, contains, regex, or present may be configured.`, + `spec.sources[0].permissions[0].header[5]: Invalid value: "{\"name\":\"regex-present\",\"present\":true,\"regex\":\"foobar\"}": at most only one of exact, prefix, suffix, contains, regex, or present may be configured.`, + `spec.sources[0].permissions[0].header[6]: Invalid value: "{\"name\":\"present-ignoreCase\",\"present\":true,\"ignoreCase\":true}": should set one of exact, prefix, suffix, or contains when using ignoreCase.`, + `spec.sources[0].permissions[0].header[7]: Invalid value: "{\"name\":\"regex-ignoreCase\",\"regex\":\"foobar\",\"ignoreCase\":true}": should set one of exact, prefix, suffix, or contains when using ignoreCase.`, }, }, "invalid permissions.action": { diff --git a/control-plane/config/crd/bases/consul.hashicorp.com_meshes.yaml b/control-plane/config/crd/bases/consul.hashicorp.com_meshes.yaml index c5c15b3c5d..51ac810778 100644 --- a/control-plane/config/crd/bases/consul.hashicorp.com_meshes.yaml +++ b/control-plane/config/crd/bases/consul.hashicorp.com_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/control-plane/config/crd/bases/consul.hashicorp.com_serviceintentions.yaml b/control-plane/config/crd/bases/consul.hashicorp.com_serviceintentions.yaml index 957295b18e..04b5c08522 100644 --- a/control-plane/config/crd/bases/consul.hashicorp.com_serviceintentions.yaml +++ b/control-plane/config/crd/bases/consul.hashicorp.com_serviceintentions.yaml @@ -164,10 +164,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/control-plane/go.mod b/control-plane/go.mod index 1cf882091f..e4abb71186 100644 --- a/control-plane/go.mod +++ b/control-plane/go.mod @@ -15,7 +15,7 @@ require ( github.com/hashicorp/consul-k8s/control-plane/cni v0.0.0-20240226161840-f3842c41cb2b github.com/hashicorp/consul-k8s/version v0.0.0 github.com/hashicorp/consul-server-connection-manager v0.1.6 - github.com/hashicorp/consul/api v1.28.4 + github.com/hashicorp/consul/api v1.28.5 github.com/hashicorp/consul/proto-public v0.6.1 github.com/hashicorp/consul/sdk v0.16.0 github.com/hashicorp/go-bexpr v0.1.11 @@ -36,6 +36,7 @@ require ( github.com/stretchr/testify v1.8.4 go.uber.org/zap v1.25.0 golang.org/x/exp v0.0.0-20240823005443-9b4947da3948 + golang.org/x/sync v0.8.0 golang.org/x/text v0.17.0 golang.org/x/time v0.3.0 gomodules.xyz/jsonpatch/v2 v2.4.0 @@ -147,7 +148,6 @@ require ( golang.org/x/mod v0.20.0 // indirect golang.org/x/net v0.28.0 // indirect golang.org/x/oauth2 v0.10.0 // indirect - golang.org/x/sync v0.8.0 // indirect golang.org/x/sys v0.24.0 // indirect golang.org/x/term v0.23.0 // indirect golang.org/x/tools v0.24.0 // indirect diff --git a/control-plane/go.sum b/control-plane/go.sum index 2a99e79160..9f42c18438 100644 --- a/control-plane/go.sum +++ b/control-plane/go.sum @@ -214,8 +214,8 @@ github.com/hashicorp/consul-k8s/control-plane/cni v0.0.0-20240226161840-f3842c41 github.com/hashicorp/consul-k8s/control-plane/cni v0.0.0-20240226161840-f3842c41cb2b/go.mod h1:9NKJHOcgmz/6P2y6MegNIOXhIKE/0ils/mHWd5sZgoU= github.com/hashicorp/consul-server-connection-manager v0.1.6 h1:ktj8Fi+dRXn9hhM+FXsfEJayhzzgTqfH08Ne5M6Fmug= github.com/hashicorp/consul-server-connection-manager v0.1.6/go.mod h1:HngMIv57MT+pqCVeRQMa1eTB5dqnyMm8uxjyv+Hn8cs= -github.com/hashicorp/consul/api v1.28.4 h1:l/d3utZ7ITzIjd8OXznuSTbBnWFHGP2VE8WdGNFrhjE= -github.com/hashicorp/consul/api v1.28.4/go.mod h1:GE+eY9pliiJiUXGchDpTaMVp27rSksHG+U+5jY+uJR4= +github.com/hashicorp/consul/api v1.28.5 h1:WscnEAY/opiVDpXFEE8bajxtf/ZKypVIYeO1lG0m5nI= +github.com/hashicorp/consul/api v1.28.5/go.mod h1:PbuTRqH1ejk45b1+duKXVQwnx+mS1f8gdna/zbQLNi4= github.com/hashicorp/consul/proto-public v0.6.1 h1:+uzH3olCrksXYWAYHKqK782CtK9scfqH+Unlw3UHhCg= github.com/hashicorp/consul/proto-public v0.6.1/go.mod h1:cXXbOg74KBNGajC+o8RlA502Esf0R9prcoJgiOX/2Tg= github.com/hashicorp/consul/sdk v0.16.0 h1:SE9m0W6DEfgIVCJX7xU+iv/hUl4m/nxqMTnCdMxDpJ8= diff --git a/control-plane/helper/test/test_util.go b/control-plane/helper/test/test_util.go index df51927e4c..60e90825a6 100644 --- a/control-plane/helper/test/test_util.go +++ b/control-plane/helper/test/test_util.go @@ -294,7 +294,7 @@ func SetupK8sAuthMethodWithNamespaces(t *testing.T, consulClient *api.Client, se Description: "Kubernetes binding rule", AuthMethod: AuthMethod, BindType: api.BindingRuleBindTypeTemplatedPolicy, - BindName: api.ACLTemplatedPolicyWorkloadIdentityName, + BindName: "", //api.ACLTemplatedPolicyWorkloadIdentityName, TODO: remove w/ v2 code BindVars: &api.ACLTemplatedPolicyVariables{ Name: "${serviceaccount.name}", },