Skip to content
Open
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
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,12 @@ All notable changes to this project will be documented in this file.

### Fixed

- Redact the user-provided PKCS#12 keystore password in operator logs. ([#706]).
- Redact the user-provided PKCS#12 keystore password in operator logs ([#706]).
- Refuse to overwrite pre-existing ConfigMaps or Secrets that are not owned by the reconciling TrustStore ([#707]).

[#693]: https://github.com/stackabletech/secret-operator/pull/693
[#706]: https://github.com/stackabletech/secret-operator/pull/706
[#707]: https://github.com/stackabletech/secret-operator/pull/707

## [26.3.0] - 2026-03-16

Expand Down
188 changes: 188 additions & 0 deletions rust/operator-binary/src/truststore_controller.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use stackable_operator::{
k8s_openapi::{
ByteString,
api::core::v1::{ConfigMap, Secret},
apimachinery::pkg::apis::meta::v1::OwnerReference,
},
kube::{
Resource,
Expand Down Expand Up @@ -198,6 +199,12 @@ pub enum Error {
#[snafu(display("TrustStore has no associated Namespace"))]
NoTrustStoreNamespace,

#[snafu(display("TrustStore has no associated name"))]
NoTrustStoreName,

#[snafu(display("TrustStore has no associated UID"))]
NoTrustStoreUid,

#[snafu(display("failed to convert trust data into desired format"))]
FormatData {
source: format::IntoFilesError,
Expand All @@ -220,6 +227,21 @@ pub enum Error {
source: stackable_operator::client::Error,
secret: ObjectRef<Secret>,
},

#[snafu(display(
"failed to look up pre-existing target {object} before applying the TrustStore"
))]
GetExistingTarget {
source: stackable_operator::client::Error,
object: ObjectRef<stackable_operator::kube::api::DynamicObject>,
},

#[snafu(display(
"refusing to overwrite pre-existing {object} that is not owned by this TrustStore"
))]
RefuseToOverwriteForeignTarget {
object: ObjectRef<stackable_operator::kube::api::DynamicObject>,
},
}

type Result<T, E = Error> = std::result::Result<T, E>;
Expand All @@ -235,12 +257,63 @@ impl ReconcilerError for Error {
Error::InitBackend { secret_class, .. } => Some(secret_class.clone().erase()),
Error::BackendGetTrustData { source } => source.secondary_object(),
Error::NoTrustStoreNamespace => None,
Error::NoTrustStoreName => None,
Error::NoTrustStoreUid => None,
Error::FormatData { secret_class, .. } => Some(secret_class.clone().erase()),
Error::BuildOwnerReference { .. } => None,
Error::ApplyTrustStoreConfigMap { config_map, .. } => Some(config_map.clone().erase()),
Error::ApplyTrustStoreSecret { secret, .. } => Some(secret.clone().erase()),
Error::GetExistingTarget { object, .. } => Some(object.clone()),
Error::RefuseToOverwriteForeignTarget { object } => Some(object.clone()),
}
}
}

/// Returns `true` if `existing_owners` contain a controller `OwnerReference` that points to the
/// TrustStore with the given UID. Used to ensure we only overwrite output ConfigMaps and Secrets
/// that we previously created ourselves; refusing otherwise prevents the TrustStore primitive
/// from being abused to clobber foreign same-named objects (e.g. `kube-root-ca.crt`) via the
/// operator's elevated cluster-wide write permissions.
fn is_owned_by_truststore(existing_owners: &[OwnerReference], truststore_uid: &str) -> bool {
let truststore_kind = <v1alpha1::TrustStore as Resource>::kind(&());
existing_owners.iter().any(|owner| {
owner.controller == Some(true)
&& owner.kind == truststore_kind
&& owner.api_version.starts_with("secrets.stackable.tech/")
&& owner.uid == truststore_uid
})
}

/// Looks up a pre-existing object with the same name/namespace as the TrustStore output and fails
/// if it exists but is not controlled by this TrustStore. See [`is_owned_by_truststore`] for the
/// security rationale.
async fn ensure_existing_target_is_not_foreign<K>(
client: &stackable_operator::client::Client,
name: &str,
namespace: &str,
truststore_uid: &str,
object_ref: ObjectRef<stackable_operator::kube::api::DynamicObject>,
) -> Result<()>
where
K: stackable_operator::kube::Resource<DynamicType = ()>
+ stackable_operator::client::GetApi<Namespace = str>
+ Clone
+ std::fmt::Debug
+ serde::de::DeserializeOwned,
{
let existing = client
.get_opt::<K>(name, namespace)
.await
.with_context(|_| GetExistingTargetSnafu {
object: object_ref.clone(),
})?;
if let Some(existing) = existing {
let owners = existing.meta().owner_references.as_deref().unwrap_or(&[]);
if !is_owned_by_truststore(owners, truststore_uid) {
return RefuseToOverwriteForeignTargetSnafu { object: object_ref }.fail();
}
}
Ok(())
}

struct Ctx {
Expand Down Expand Up @@ -311,6 +384,18 @@ async fn reconcile(
.context(BuildOwnerReferenceSnafu)?
.build();

let truststore_name = truststore
Comment thread
sweb marked this conversation as resolved.
.metadata
.name
.as_deref()
.context(NoTrustStoreNameSnafu)?;
let truststore_namespace = selector.namespace.as_str();
let truststore_uid = truststore
.metadata
.uid
.as_deref()
.context(NoTrustStoreUidSnafu)?;

match truststore.spec.target_kind {
v1alpha1::TrustStoreOutputType::ConfigMap => {
let trust_cm = ConfigMap {
Expand All @@ -319,6 +404,14 @@ async fn reconcile(
binary_data: Some(binary_data),
..Default::default()
};
ensure_existing_target_is_not_foreign::<ConfigMap>(
&ctx.client,
truststore_name,
truststore_namespace,
truststore_uid,
ObjectRef::from_obj(&trust_cm).erase(),
)
.await?;
ctx.client
.apply_patch(CONTROLLER_NAME, &trust_cm, &trust_cm)
.await
Expand All @@ -333,6 +426,14 @@ async fn reconcile(
data: Some(binary_data),
..Default::default()
};
ensure_existing_target_is_not_foreign::<Secret>(
&ctx.client,
truststore_name,
truststore_namespace,
truststore_uid,
ObjectRef::from_obj(&trust_secret).erase(),
)
.await?;
ctx.client
.apply_patch(CONTROLLER_NAME, &trust_secret, &trust_secret)
.await
Expand All @@ -352,3 +453,90 @@ fn error_policy(
) -> controller::Action {
controller::Action::requeue(Duration::from_secs(5))
}

#[cfg(test)]
mod tests {
use super::*;

fn owner_ref(
api_version: &str,
kind: &str,
uid: &str,
controller: Option<bool>,
) -> OwnerReference {
OwnerReference {
api_version: api_version.to_string(),
kind: kind.to_string(),
name: "some-name".to_string(),
uid: uid.to_string(),
controller,
block_owner_deletion: None,
}
}

#[test]
fn empty_owner_refs_are_rejected() {
assert!(!is_owned_by_truststore(&[], "my-uid"));
}

#[test]
fn matching_controller_owner_ref_is_accepted() {
let owners = [owner_ref(
"secrets.stackable.tech/v1alpha1",
"TrustStore",
"my-uid",
Some(true),
)];
assert!(is_owned_by_truststore(&owners, "my-uid"));
}

#[test]
fn non_controller_owner_ref_is_rejected() {
let owners = [owner_ref(
"secrets.stackable.tech/v1alpha1",
"TrustStore",
"my-uid",
Some(false),
)];
assert!(!is_owned_by_truststore(&owners, "my-uid"));
}

#[test]
fn different_uid_is_rejected() {
let owners = [owner_ref(
"secrets.stackable.tech/v1alpha1",
"TrustStore",
"other-uid",
Some(true),
)];
assert!(!is_owned_by_truststore(&owners, "my-uid"));
}

#[test]
fn different_kind_is_rejected() {
let owners = [owner_ref("v1", "ConfigMap", "my-uid", Some(true))];
assert!(!is_owned_by_truststore(&owners, "my-uid"));
}

#[test]
fn foreign_api_group_is_rejected() {
let owners = [owner_ref(
"evil.example.com/v1",
"TrustStore",
"my-uid",
Some(true),
)];
assert!(!is_owned_by_truststore(&owners, "my-uid"));
}

#[test]
fn unrelated_controller_owner_ref_is_rejected() {
let owners = [owner_ref(
"apps/v1",
"Deployment",
"some-deployment-uid",
Some(true),
)];
assert!(!is_owned_by_truststore(&owners, "my-uid"));
}
}
Loading