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

feat: Allow configuring NiFi proxy host behaviour #668

Merged
merged 13 commits into from
Aug 28, 2024
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ All notable changes to this project will be documented in this file.

## [Unreleased]

### Added

- Allow configuring proxy host behavior ([#668]).

### Changed

- Reduce CRD size from `637KB` to `105KB` by accepting arbitrary YAML input instead of the underlying schema for the following fields ([#664]):
Expand All @@ -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

Expand Down
17 changes: 17 additions & 0 deletions deploy/helm/nifi-operator/crds/crds.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,23 @@ spec:
type: object
x-kubernetes-preserve-unknown-fields: true
type: array
hostHeaderCheck:
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 <https://github.com/stackabletech/docker-images/pull/694>
type: boolean
type: object
listenerClass:
default: cluster-internal
description: |-
Expand Down
16 changes: 16 additions & 0 deletions docs/modules/nifi/pages/usage_guide/security.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -234,3 +234,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 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]
----
spec:
clusterConfig:
hostHeaderCheck:
allowAll: false
additionalAllowedHosts:
- example.com:1234
----
30 changes: 30 additions & 0 deletions rust/crd/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,11 @@ pub struct NifiClusterConfig {
// We don't add `#[serde(default)]` here, as we require authentication
pub authentication: Vec<ClientAuthenticationDetails>,

/// 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,
Expand Down Expand Up @@ -159,6 +164,31 @@ 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.
/// See <https://github.com/stackabletech/docker-images/pull/694>
#[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)]
pub additional_allowed_hosts: Vec<String>,
}

impl Default for HostHeaderCheckConfig {
fn default() -> Self {
Self {
allow_all: default_allow_all(),
additional_allowed_hosts: Vec::default(),
}
}
}

pub fn default_allow_all() -> bool {
true
}

// TODO: Temporary solution until listener-operator is finished
#[derive(Clone, Debug, Default, Display, Deserialize, Eq, JsonSchema, PartialEq, Serialize)]
#[serde(rename_all = "PascalCase")]
Expand Down
25 changes: 20 additions & 5 deletions rust/operator-binary/src/controller.rs
Original file line number Diff line number Diff line change
@@ -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,
};
Expand Down Expand Up @@ -1380,16 +1380,28 @@ async fn get_proxy_hosts(
nifi: &NifiCluster,
nifi_service: &Service,
) -> Result<String> {
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. All proxy hosts will be allowed.");
maltesander marked this conversation as resolved.
Show resolved Hide resolved
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());
}

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
Expand All @@ -1407,7 +1419,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| {
Expand All @@ -1420,7 +1432,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<NifiCluster>, _error: &Error, _ctx: Arc<Ctx>) -> Action {
Expand Down
4 changes: 4 additions & 0 deletions tests/templates/kuttl/smoke/30-install-nifi.yaml.j2
Original file line number Diff line number Diff line change
Expand Up @@ -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') %}
Expand Down
6 changes: 6 additions & 0 deletions tests/templates/kuttl/smoke/31-assert.yaml.j2
Original file line number Diff line number Diff line change
@@ -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
Loading