From 39711da49076665836a028d630da379f644050b3 Mon Sep 17 00:00:00 2001 From: Benedikt Labrenz Date: Thu, 22 Aug 2024 12:14:45 +0200 Subject: [PATCH 01/11] add allowed proxy host configuration --- deploy/helm/nifi-operator/crds/crds.yaml | 14 ++++++++++ rust/crd/src/lib.rs | 33 ++++++++++++++++++++++++ rust/operator-binary/src/controller.rs | 22 ++++++++++++---- 3 files changed, 64 insertions(+), 5 deletions(-) diff --git a/deploy/helm/nifi-operator/crds/crds.yaml b/deploy/helm/nifi-operator/crds/crds.yaml index 09b88a02..f3f7708c 100644 --- a/deploy/helm/nifi-operator/crds/crds.yaml +++ b/deploy/helm/nifi-operator/crds/crds.yaml @@ -45,6 +45,20 @@ spec: type: object x-kubernetes-preserve-unknown-fields: true type: array + hostHeaderCheck: + default: + additionalAllowedHosts: [] + allowAll: true + properties: + additionalAllowedHosts: + default: [] + items: + type: string + type: array + allowAll: + default: true + type: boolean + type: object listenerClass: default: cluster-internal description: |- diff --git a/rust/crd/src/lib.rs b/rust/crd/src/lib.rs index 16acaa26..56620729 100644 --- a/rust/crd/src/lib.rs +++ b/rust/crd/src/lib.rs @@ -117,6 +117,11 @@ pub struct NifiClusterConfig { // We don't add `#[serde(default)]` here, as we require authentication pub authentication: Vec, + /// Configuration of allowed proxies e.g. load balancers or Kubernetes Ingress. Using a proxy that is not allowed by NiFi results + /// in a failed host header check. + #[serde(default)] + pub host_header_check: HostHeaderCheckConfig, + /// TLS configuration options for the server. #[serde(default)] pub tls: NifiTls, @@ -158,6 +163,34 @@ pub struct NifiClusterConfig { pub listener_class: CurrentlySupportedListenerClasses, } +#[derive(Clone, Debug, Deserialize, JsonSchema, PartialEq, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct HostHeaderCheckConfig { + /// Allow all proxy hosts by turning off host header validation. + #[serde(default = "default_allow_all")] + pub allow_all: bool, + /// List of proxy hosts to add to the default allow list deployed by SDP containing Kubernetes Services utilized by NiFi. + #[serde(default = "default_additional_allowed_hosts")] + pub additional_allowed_hosts: Vec, +} + +impl Default for HostHeaderCheckConfig { + fn default() -> Self { + Self { + allow_all: true, + additional_allowed_hosts: vec![], + } + } +} + +pub fn default_allow_all() -> bool { + true +} + +pub fn default_additional_allowed_hosts() -> Vec { + vec![] +} + // TODO: Temporary solution until listener-operator is finished #[derive(Clone, Debug, Default, Display, Deserialize, Eq, JsonSchema, PartialEq, Serialize)] #[serde(rename_all = "PascalCase")] diff --git a/rust/operator-binary/src/controller.rs b/rust/operator-binary/src/controller.rs index bae0648a..11dad318 100644 --- a/rust/operator-binary/src/controller.rs +++ b/rust/operator-binary/src/controller.rs @@ -1,7 +1,7 @@ //! Ensures that `Pod`s are configured and running for each [`NifiCluster`] use std::{ borrow::Cow, - collections::{BTreeMap, HashMap}, + collections::{BTreeMap, HashMap, HashSet}, ops::Deref, sync::Arc, }; @@ -1364,16 +1364,25 @@ async fn get_proxy_hosts( nifi: &NifiCluster, nifi_service: &Service, ) -> Result { + let host_header_check = nifi.spec.cluster_config.host_header_check.clone(); + + if host_header_check.allow_all == true { + tracing::info!("spec.clusterConfig.hostHeaderCheck.allowAll is set to true. Adding just \"*\" to allowed proxy hosts."); + return Ok("*".to_string()); + } + let node_role_service_fqdn = nifi .node_role_service_fqdn() .context(NoRoleServiceFqdnSnafu)?; let reporting_task_service_name = reporting_task::build_reporting_task_fqdn_service_name(nifi).context(ReportingTaskSnafu)?; - let mut proxy_setting = vec![ + let mut proxy_hosts_set = HashSet::from([ node_role_service_fqdn.clone(), format!("{node_role_service_fqdn}:{HTTPS_PORT}"), format!("{reporting_task_service_name}:{HTTPS_PORT}"), - ]; + ]); + + proxy_hosts_set.extend(host_header_check.additional_allowed_hosts); // In case NodePort is used add them as well if nifi.spec.cluster_config.listener_class @@ -1391,7 +1400,7 @@ async fn get_proxy_hosts( // We need the addresses of all nodes to add these to the NiFi proxy setting // Since there is no real convention about how to label these addresses we will simply // take all published addresses for now to be on the safe side. - proxy_setting.extend( + proxy_hosts_set.extend( cluster_nodes .into_iter() .flat_map(|node| { @@ -1404,7 +1413,10 @@ async fn get_proxy_hosts( ); } - Ok(proxy_setting.join(",")) + let mut proxy_hosts = Vec::from_iter(proxy_hosts_set); + proxy_hosts.sort(); + + Ok(proxy_hosts.join(",")) } pub fn error_policy(_obj: Arc, _error: &Error, _ctx: Arc) -> Action { From a019d5ba8a85840e8c34ef17efec8a3c9c1edd2e Mon Sep 17 00:00:00 2001 From: Benedikt Labrenz Date: Thu, 22 Aug 2024 12:23:12 +0200 Subject: [PATCH 02/11] add link to host header check PR --- rust/crd/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/rust/crd/src/lib.rs b/rust/crd/src/lib.rs index 56620729..718242a0 100644 --- a/rust/crd/src/lib.rs +++ b/rust/crd/src/lib.rs @@ -167,6 +167,7 @@ pub struct NifiClusterConfig { #[serde(rename_all = "camelCase")] pub struct HostHeaderCheckConfig { /// Allow all proxy hosts by turning off host header validation. + /// See #[serde(default = "default_allow_all")] pub allow_all: bool, /// List of proxy hosts to add to the default allow list deployed by SDP containing Kubernetes Services utilized by NiFi. From f048a2aefb04ef293019bab3429694393c8b9efe Mon Sep 17 00:00:00 2001 From: Benedikt Labrenz Date: Mon, 26 Aug 2024 11:40:21 +0200 Subject: [PATCH 03/11] address pr checks --- deploy/helm/nifi-operator/crds/crds.yaml | 3 +++ rust/operator-binary/src/controller.rs | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/deploy/helm/nifi-operator/crds/crds.yaml b/deploy/helm/nifi-operator/crds/crds.yaml index f3f7708c..f940521b 100644 --- a/deploy/helm/nifi-operator/crds/crds.yaml +++ b/deploy/helm/nifi-operator/crds/crds.yaml @@ -49,14 +49,17 @@ spec: default: additionalAllowedHosts: [] allowAll: true + description: Configuration of allowed proxies e.g. load balancers or Kubernetes Ingress. Using a proxy that is not allowed by NiFi results in a failed host header check. properties: additionalAllowedHosts: default: [] + description: List of proxy hosts to add to the default allow list deployed by SDP containing Kubernetes Services utilized by NiFi. items: type: string type: array allowAll: default: true + description: Allow all proxy hosts by turning off host header validation. See type: boolean type: object listenerClass: diff --git a/rust/operator-binary/src/controller.rs b/rust/operator-binary/src/controller.rs index 11dad318..7f366b35 100644 --- a/rust/operator-binary/src/controller.rs +++ b/rust/operator-binary/src/controller.rs @@ -1366,7 +1366,7 @@ async fn get_proxy_hosts( ) -> Result { let host_header_check = nifi.spec.cluster_config.host_header_check.clone(); - if host_header_check.allow_all == true { + if host_header_check.allow_all { tracing::info!("spec.clusterConfig.hostHeaderCheck.allowAll is set to true. Adding just \"*\" to allowed proxy hosts."); return Ok("*".to_string()); } From 0b404686dc1a94d970403088d31b27c454d6b708 Mon Sep 17 00:00:00 2001 From: Benedikt Labrenz Date: Mon, 26 Aug 2024 11:56:16 +0200 Subject: [PATCH 04/11] update log message --- rust/operator-binary/src/controller.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust/operator-binary/src/controller.rs b/rust/operator-binary/src/controller.rs index 7f366b35..8abc2b12 100644 --- a/rust/operator-binary/src/controller.rs +++ b/rust/operator-binary/src/controller.rs @@ -1367,7 +1367,7 @@ async fn get_proxy_hosts( let host_header_check = nifi.spec.cluster_config.host_header_check.clone(); if host_header_check.allow_all { - tracing::info!("spec.clusterConfig.hostHeaderCheck.allowAll is set to true. Adding just \"*\" to allowed proxy hosts."); + tracing::info!("spec.clusterConfig.hostHeaderCheck.allowAll is set to true. All proxy hosts will be allowed."); return Ok("*".to_string()); } From 1592828f55b3c890ab640b5a533b244046c99629 Mon Sep 17 00:00:00 2001 From: Benedikt Labrenz Date: Mon, 26 Aug 2024 17:14:56 +0200 Subject: [PATCH 05/11] add hostHeaderCheck to smoke test --- tests/templates/kuttl/smoke/30-install-nifi.yaml.j2 | 4 ++++ tests/templates/kuttl/smoke/31-assert.yaml.j2 | 6 ++++++ 2 files changed, 10 insertions(+) create mode 100644 tests/templates/kuttl/smoke/31-assert.yaml.j2 diff --git a/tests/templates/kuttl/smoke/30-install-nifi.yaml.j2 b/tests/templates/kuttl/smoke/30-install-nifi.yaml.j2 index 5ef028fb..48430da5 100644 --- a/tests/templates/kuttl/smoke/30-install-nifi.yaml.j2 +++ b/tests/templates/kuttl/smoke/30-install-nifi.yaml.j2 @@ -42,6 +42,10 @@ spec: listenerClass: {{ test_scenario['values']['listener-class'] }} authentication: - authenticationClass: simple-nifi-users + hostHeaderCheck: + allowAll: false + additionalAllowedHosts: + - example.com:1234 sensitiveProperties: keySecret: nifi-sensitive-property-key {% if lookup('env', 'VECTOR_AGGREGATOR') %} diff --git a/tests/templates/kuttl/smoke/31-assert.yaml.j2 b/tests/templates/kuttl/smoke/31-assert.yaml.j2 new file mode 100644 index 00000000..06b1dc78 --- /dev/null +++ b/tests/templates/kuttl/smoke/31-assert.yaml.j2 @@ -0,0 +1,6 @@ +--- +apiVersion: kuttl.dev/v1beta1 +kind: TestAssert +timeout: 30 +commands: +- script: kubectl get cm -n $NAMESPACE test-nifi-node-default -o yaml | grep -- 'nifi.web.proxy.host=.*example.com:1234' | xargs test ! -z From fc33b3eff06f03dabd54020c384c05d424c20cba Mon Sep 17 00:00:00 2001 From: Benedikt Labrenz Date: Tue, 27 Aug 2024 11:59:51 +0200 Subject: [PATCH 06/11] add docs and changelog entry --- CHANGELOG.md | 5 +++++ .../modules/nifi/pages/usage_guide/security.adoc | 16 ++++++++++++++++ rust/crd/src/lib.rs | 10 +++------- 3 files changed, 24 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 384cf11d..2a1e4a53 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ All notable changes to this project will be documented in this file. ## [Unreleased] +### Added + +- Allow configuring proxy host behaviour ([#668]). + ### Changed - Reduce CRD size from `637KB` to `105KB` by accepting arbitrary YAML input instead of the underlying schema for the following fields ([#664]): @@ -17,6 +21,7 @@ All notable changes to this project will be documented in this file. [#664]: https://github.com/stackabletech/nifi-operator/pull/664 [#665]: https://github.com/stackabletech/nifi-operator/pull/665 +[#668]: https://github.com/stackabletech/nifi-operator/pull/668 ## [24.7.0] - 2024-07-24 diff --git a/docs/modules/nifi/pages/usage_guide/security.adoc b/docs/modules/nifi/pages/usage_guide/security.adoc index ec376611..9c6bb662 100644 --- a/docs/modules/nifi/pages/usage_guide/security.adoc +++ b/docs/modules/nifi/pages/usage_guide/security.adoc @@ -167,3 +167,19 @@ sensitiveProperties: keySecret: nifi-sensitive-property-key algorithm: nifiArgon2AesGcm256 ---- + +[#host-header-check] +== Host Header Check +NiFi checks the Host header of incoming requests and rejects them if they are passing through a proxy that is not on an allow-list configured in the `nifi.web.proxy.host` property. + +A https://github.com/stackabletech/docker-images/pull/694[patch] applied during the build of the SDP container image for NiFi allows turning off this check by adding `nifi.web.proxy.host=*` to the properties. The Host header check for NiFi clusters created by the operator is turned off by default but can be turned in the NiFi configuration. In this case the list of allowed hosts will default to Kubernetes Services used by Nifi and can be extended with custom entries. + +[source,yaml] +---- +spec: + clusterConfig: + hostHeaderCheck: + allowAll: false + additionalAllowedHosts: + - example.com:1234 +---- \ No newline at end of file diff --git a/rust/crd/src/lib.rs b/rust/crd/src/lib.rs index 718242a0..e97e3660 100644 --- a/rust/crd/src/lib.rs +++ b/rust/crd/src/lib.rs @@ -171,15 +171,15 @@ pub struct HostHeaderCheckConfig { #[serde(default = "default_allow_all")] pub allow_all: bool, /// List of proxy hosts to add to the default allow list deployed by SDP containing Kubernetes Services utilized by NiFi. - #[serde(default = "default_additional_allowed_hosts")] + #[serde(default)] pub additional_allowed_hosts: Vec, } impl Default for HostHeaderCheckConfig { fn default() -> Self { Self { - allow_all: true, - additional_allowed_hosts: vec![], + allow_all: default_allow_all(), + additional_allowed_hosts: Vec::default(), } } } @@ -188,10 +188,6 @@ pub fn default_allow_all() -> bool { true } -pub fn default_additional_allowed_hosts() -> Vec { - vec![] -} - // TODO: Temporary solution until listener-operator is finished #[derive(Clone, Debug, Default, Display, Deserialize, Eq, JsonSchema, PartialEq, Serialize)] #[serde(rename_all = "PascalCase")] From 8446ca113b4c024c483d18f61c3a6757d02154c5 Mon Sep 17 00:00:00 2001 From: Benedikt Labrenz Date: Tue, 27 Aug 2024 12:11:09 +0200 Subject: [PATCH 07/11] add additional tracing --- rust/operator-binary/src/controller.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/rust/operator-binary/src/controller.rs b/rust/operator-binary/src/controller.rs index 8abc2b12..d99e4ef5 100644 --- a/rust/operator-binary/src/controller.rs +++ b/rust/operator-binary/src/controller.rs @@ -1368,6 +1368,9 @@ async fn get_proxy_hosts( if host_header_check.allow_all { tracing::info!("spec.clusterConfig.hostHeaderCheck.allowAll is set to true. All proxy hosts will be allowed."); + if host_header_check.additional_allowed_hosts.len() > 0 { + tracing::info!("spec.clusterConfig.hostHeaderCheck.additionalAllowedHosts is ignored and only '*' is added to the allow-list.") + } return Ok("*".to_string()); } From c934c8986c15a86f8ae6c6b8f2734810016f7e4a Mon Sep 17 00:00:00 2001 From: Benedikt Labrenz Date: Tue, 27 Aug 2024 12:13:34 +0200 Subject: [PATCH 08/11] fix clippy --- rust/operator-binary/src/controller.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust/operator-binary/src/controller.rs b/rust/operator-binary/src/controller.rs index d99e4ef5..bdd3320d 100644 --- a/rust/operator-binary/src/controller.rs +++ b/rust/operator-binary/src/controller.rs @@ -1368,7 +1368,7 @@ async fn get_proxy_hosts( if host_header_check.allow_all { tracing::info!("spec.clusterConfig.hostHeaderCheck.allowAll is set to true. All proxy hosts will be allowed."); - if host_header_check.additional_allowed_hosts.len() > 0 { + if !host_header_check.additional_allowed_hosts.is_empty() { tracing::info!("spec.clusterConfig.hostHeaderCheck.additionalAllowedHosts is ignored and only '*' is added to the allow-list.") } return Ok("*".to_string()); From d1837c22ffedeeb043e9f9543077b400c68c9ad0 Mon Sep 17 00:00:00 2001 From: Benedikt Labrenz Date: Tue, 27 Aug 2024 12:17:12 +0200 Subject: [PATCH 09/11] fix typo --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2a1e4a53..f6df5180 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ All notable changes to this project will be documented in this file. ### Added -- Allow configuring proxy host behaviour ([#668]). +- Allow configuring proxy host behavior ([#668]). ### Changed From b91debdcec071722aa906b01124519062c681946 Mon Sep 17 00:00:00 2001 From: Benedikt Labrenz Date: Tue, 27 Aug 2024 15:01:32 +0200 Subject: [PATCH 10/11] Update docs/modules/nifi/pages/usage_guide/security.adoc Co-authored-by: Malte Sander --- docs/modules/nifi/pages/usage_guide/security.adoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/modules/nifi/pages/usage_guide/security.adoc b/docs/modules/nifi/pages/usage_guide/security.adoc index 9c6bb662..6bdbaf57 100644 --- a/docs/modules/nifi/pages/usage_guide/security.adoc +++ b/docs/modules/nifi/pages/usage_guide/security.adoc @@ -172,7 +172,7 @@ sensitiveProperties: == Host Header Check NiFi checks the Host header of incoming requests and rejects them if they are passing through a proxy that is not on an allow-list configured in the `nifi.web.proxy.host` property. -A https://github.com/stackabletech/docker-images/pull/694[patch] applied during the build of the SDP container image for NiFi allows turning off this check by adding `nifi.web.proxy.host=*` to the properties. The Host header check for NiFi clusters created by the operator is turned off by default but can be turned in the NiFi configuration. In this case the list of allowed hosts will default to Kubernetes Services used by Nifi and can be extended with custom entries. +A https://github.com/stackabletech/docker-images/pull/694[patch] applied during the build of the SDP container image for NiFi allows turning off this check by adding `nifi.web.proxy.host=*` to the properties. The Host header check for NiFi clusters created by the operator is disabled by default but can be enabled in the NiFi configuration. In this case the list of allowed hosts will default to Kubernetes Services used by NiFi and can be extended with custom entries. [source,yaml] ---- From cb69e1d69e4e20ece2a15d57872552f27ccae708 Mon Sep 17 00:00:00 2001 From: Benedikt Labrenz Date: Tue, 27 Aug 2024 15:01:42 +0200 Subject: [PATCH 11/11] Update docs/modules/nifi/pages/usage_guide/security.adoc Co-authored-by: Malte Sander --- docs/modules/nifi/pages/usage_guide/security.adoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/modules/nifi/pages/usage_guide/security.adoc b/docs/modules/nifi/pages/usage_guide/security.adoc index 6bdbaf57..100a63c3 100644 --- a/docs/modules/nifi/pages/usage_guide/security.adoc +++ b/docs/modules/nifi/pages/usage_guide/security.adoc @@ -170,7 +170,7 @@ sensitiveProperties: [#host-header-check] == Host Header Check -NiFi checks the Host header of incoming requests and rejects them if they are passing through a proxy that is not on an allow-list configured in the `nifi.web.proxy.host` property. +NiFi checks the host header of incoming requests and rejects them if they are passing through a proxy that is not on an allow-list configured in the `nifi.web.proxy.host` property. A https://github.com/stackabletech/docker-images/pull/694[patch] applied during the build of the SDP container image for NiFi allows turning off this check by adding `nifi.web.proxy.host=*` to the properties. The Host header check for NiFi clusters created by the operator is disabled by default but can be enabled in the NiFi configuration. In this case the list of allowed hosts will default to Kubernetes Services used by NiFi and can be extended with custom entries.