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

[NET-11043] crd: support request normalization and header match options to prevent L7 intentions bypass #4385

Merged
merged 1 commit into from
Oct 17, 2024
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
6 changes: 6 additions & 0 deletions .changelog/4385.txt
Original file line number Diff line number Diff line change
@@ -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.
```
2 changes: 1 addition & 1 deletion acceptance/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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-20240821160356-557f7c37e108
github.com/hashicorp/consul/api v1.29.4
github.com/hashicorp/consul/api v1.30.0
github.com/hashicorp/consul/sdk v0.16.1
github.com/hashicorp/go-multierror v1.1.1
github.com/hashicorp/go-uuid v1.0.3
Expand Down
4 changes: 2 additions & 2 deletions acceptance/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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-20240821160356-557f7c37e108 h1:5jSMtMGeY//hvkAefiomxP1Jqb5MtnKgsnlsZpEwiJE=
github.com/hashicorp/consul-k8s/control-plane v0.0.0-20240821160356-557f7c37e108/go.mod h1:SY22WR9TJmlcK18Et2MAqy+kqAFJzbWFElN89vMTSiM=
github.com/hashicorp/consul/api v1.29.4 h1:P6slzxDLBOxUSj3fWo2o65VuKtbtOXFi7TSSgtXutuE=
github.com/hashicorp/consul/api v1.29.4/go.mod h1:HUlfw+l2Zy68ceJavv2zAyArl2fqhGWnMycyt56sBgg=
github.com/hashicorp/consul/api v1.30.0 h1:ArHVMMILb1nQv8vZSGIwwQd2gtc+oSQZ6CalyiyH2XQ=
github.com/hashicorp/consul/api v1.30.0/go.mod h1:B2uGchvaXVW2JhFoS8nqTxMD5PBykr4ebY4JWHTTeLM=
github.com/hashicorp/consul/proto-public v0.6.2 h1:+DA/3g/IiKlJZb88NBn0ZgXrxJp2NlvCZdEyl+qxvL0=
github.com/hashicorp/consul/proto-public v0.6.2/go.mod h1:cXXbOg74KBNGajC+o8RlA502Esf0R9prcoJgiOX/2Tg=
github.com/hashicorp/consul/sdk v0.16.1 h1:V8TxTnImoPD5cj0U9Spl0TUxcytjcbbJeADFF07KdHg=
Expand Down
49 changes: 47 additions & 2 deletions charts/consul/templates/crd-meshes.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,55 @@ spec:
http:
description: HTTP defines the HTTP configuration for the service mesh.
properties:
incoming:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docs here and below copied from consul changes. Subject to change based on feedback from Docs team.

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
Expand Down
9 changes: 9 additions & 0 deletions charts/consul/templates/crd-serviceintentions.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion cli/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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.29.4 // indirect
github.com/hashicorp/consul/api v1.30.0 // indirect
github.com/hashicorp/consul/envoyextensions v0.7.3 // indirect
github.com/hashicorp/errwrap v1.1.0 // indirect
github.com/hashicorp/go-cleanhttp v0.5.2 // indirect
Expand Down
6 changes: 2 additions & 4 deletions cli/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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.29.4 h1:P6slzxDLBOxUSj3fWo2o65VuKtbtOXFi7TSSgtXutuE=
github.com/hashicorp/consul/api v1.29.4/go.mod h1:HUlfw+l2Zy68ceJavv2zAyArl2fqhGWnMycyt56sBgg=
github.com/hashicorp/consul/api v1.30.0 h1:ArHVMMILb1nQv8vZSGIwwQd2gtc+oSQZ6CalyiyH2XQ=
github.com/hashicorp/consul/api v1.30.0/go.mod h1:B2uGchvaXVW2JhFoS8nqTxMD5PBykr4ebY4JWHTTeLM=
github.com/hashicorp/consul/envoyextensions v0.7.3 h1:5Gn1Hj135NYNRBmB3IdwhkxIHQgEJPjXYPZcA+05rNY=
github.com/hashicorp/consul/envoyextensions v0.7.3/go.mod h1:tya/kHsOBGaeAS9inAfUFJIEJ812c125cQD4MrLTt2s=
github.com/hashicorp/consul/proto-public v0.6.2 h1:+DA/3g/IiKlJZb88NBn0ZgXrxJp2NlvCZdEyl+qxvL0=
github.com/hashicorp/consul/proto-public v0.6.2/go.mod h1:cXXbOg74KBNGajC+o8RlA502Esf0R9prcoJgiOX/2Tg=
github.com/hashicorp/consul/sdk v0.16.1 h1:V8TxTnImoPD5cj0U9Spl0TUxcytjcbbJeADFF07KdHg=
github.com/hashicorp/consul/sdk v0.16.1/go.mod h1:fSXvwxB2hmh1FMZCNl6PwX0Q/1wdWtHJcZ7Ea5tns0s=
github.com/hashicorp/consul/troubleshoot v0.7.1 h1:IQYxC1qsV3jO74VZDyPi283Ufi84/mXSMm53U8dsN2M=
Expand Down
125 changes: 124 additions & 1 deletion control-plane/api/v1alpha1/mesh_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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}
}
Expand Down Expand Up @@ -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(
Expand All @@ -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,
}
}

Expand Down Expand Up @@ -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) {
}
Loading
Loading