Skip to content

Commit 0a103eb

Browse files
authored
fix: Refuse to overwrite foreign objects from TrustStore reconciler (#707)
* fix: Refuse to overwrite foreign objects from TrustStore reconciler * fix: address review feedback
1 parent 80ba763 commit 0a103eb

2 files changed

Lines changed: 191 additions & 1 deletion

File tree

CHANGELOG.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,12 @@ All notable changes to this project will be documented in this file.
1515

1616
### Fixed
1717

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

2021
[#693]: https://github.com/stackabletech/secret-operator/pull/693
2122
[#706]: https://github.com/stackabletech/secret-operator/pull/706
23+
[#707]: https://github.com/stackabletech/secret-operator/pull/707
2224

2325
## [26.3.0] - 2026-03-16
2426

rust/operator-binary/src/truststore_controller.rs

Lines changed: 188 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ use stackable_operator::{
88
k8s_openapi::{
99
ByteString,
1010
api::core::v1::{ConfigMap, Secret},
11+
apimachinery::pkg::apis::meta::v1::OwnerReference,
1112
},
1213
kube::{
1314
Resource,
@@ -198,6 +199,12 @@ pub enum Error {
198199
#[snafu(display("TrustStore has no associated Namespace"))]
199200
NoTrustStoreNamespace,
200201

202+
#[snafu(display("TrustStore has no associated name"))]
203+
NoTrustStoreName,
204+
205+
#[snafu(display("TrustStore has no associated UID"))]
206+
NoTrustStoreUid,
207+
201208
#[snafu(display("failed to convert trust data into desired format"))]
202209
FormatData {
203210
source: format::IntoFilesError,
@@ -220,6 +227,21 @@ pub enum Error {
220227
source: stackable_operator::client::Error,
221228
secret: ObjectRef<Secret>,
222229
},
230+
231+
#[snafu(display(
232+
"failed to look up pre-existing target {object} before applying the TrustStore"
233+
))]
234+
GetExistingTarget {
235+
source: stackable_operator::client::Error,
236+
object: ObjectRef<stackable_operator::kube::api::DynamicObject>,
237+
},
238+
239+
#[snafu(display(
240+
"refusing to overwrite pre-existing {object} that is not owned by this TrustStore"
241+
))]
242+
RefuseToOverwriteForeignTarget {
243+
object: ObjectRef<stackable_operator::kube::api::DynamicObject>,
244+
},
223245
}
224246

225247
type Result<T, E = Error> = std::result::Result<T, E>;
@@ -235,12 +257,63 @@ impl ReconcilerError for Error {
235257
Error::InitBackend { secret_class, .. } => Some(secret_class.clone().erase()),
236258
Error::BackendGetTrustData { source } => source.secondary_object(),
237259
Error::NoTrustStoreNamespace => None,
260+
Error::NoTrustStoreName => None,
261+
Error::NoTrustStoreUid => None,
238262
Error::FormatData { secret_class, .. } => Some(secret_class.clone().erase()),
239263
Error::BuildOwnerReference { .. } => None,
240264
Error::ApplyTrustStoreConfigMap { config_map, .. } => Some(config_map.clone().erase()),
241265
Error::ApplyTrustStoreSecret { secret, .. } => Some(secret.clone().erase()),
266+
Error::GetExistingTarget { object, .. } => Some(object.clone()),
267+
Error::RefuseToOverwriteForeignTarget { object } => Some(object.clone()),
268+
}
269+
}
270+
}
271+
272+
/// Returns `true` if `existing_owners` contain a controller `OwnerReference` that points to the
273+
/// TrustStore with the given UID. Used to ensure we only overwrite output ConfigMaps and Secrets
274+
/// that we previously created ourselves; refusing otherwise prevents the TrustStore primitive
275+
/// from being abused to clobber foreign same-named objects (e.g. `kube-root-ca.crt`) via the
276+
/// operator's elevated cluster-wide write permissions.
277+
fn is_owned_by_truststore(existing_owners: &[OwnerReference], truststore_uid: &str) -> bool {
278+
let truststore_kind = <v1alpha1::TrustStore as Resource>::kind(&());
279+
existing_owners.iter().any(|owner| {
280+
owner.controller == Some(true)
281+
&& owner.kind == truststore_kind
282+
&& owner.api_version.starts_with("secrets.stackable.tech/")
283+
&& owner.uid == truststore_uid
284+
})
285+
}
286+
287+
/// Looks up a pre-existing object with the same name/namespace as the TrustStore output and fails
288+
/// if it exists but is not controlled by this TrustStore. See [`is_owned_by_truststore`] for the
289+
/// security rationale.
290+
async fn ensure_existing_target_is_not_foreign<K>(
291+
client: &stackable_operator::client::Client,
292+
name: &str,
293+
namespace: &str,
294+
truststore_uid: &str,
295+
object_ref: ObjectRef<stackable_operator::kube::api::DynamicObject>,
296+
) -> Result<()>
297+
where
298+
K: stackable_operator::kube::Resource<DynamicType = ()>
299+
+ stackable_operator::client::GetApi<Namespace = str>
300+
+ Clone
301+
+ std::fmt::Debug
302+
+ serde::de::DeserializeOwned,
303+
{
304+
let existing = client
305+
.get_opt::<K>(name, namespace)
306+
.await
307+
.with_context(|_| GetExistingTargetSnafu {
308+
object: object_ref.clone(),
309+
})?;
310+
if let Some(existing) = existing {
311+
let owners = existing.meta().owner_references.as_deref().unwrap_or(&[]);
312+
if !is_owned_by_truststore(owners, truststore_uid) {
313+
return RefuseToOverwriteForeignTargetSnafu { object: object_ref }.fail();
242314
}
243315
}
316+
Ok(())
244317
}
245318

246319
struct Ctx {
@@ -311,6 +384,18 @@ async fn reconcile(
311384
.context(BuildOwnerReferenceSnafu)?
312385
.build();
313386

387+
let truststore_name = truststore
388+
.metadata
389+
.name
390+
.as_deref()
391+
.context(NoTrustStoreNameSnafu)?;
392+
let truststore_namespace = selector.namespace.as_str();
393+
let truststore_uid = truststore
394+
.metadata
395+
.uid
396+
.as_deref()
397+
.context(NoTrustStoreUidSnafu)?;
398+
314399
match truststore.spec.target_kind {
315400
v1alpha1::TrustStoreOutputType::ConfigMap => {
316401
let trust_cm = ConfigMap {
@@ -319,6 +404,14 @@ async fn reconcile(
319404
binary_data: Some(binary_data),
320405
..Default::default()
321406
};
407+
ensure_existing_target_is_not_foreign::<ConfigMap>(
408+
&ctx.client,
409+
truststore_name,
410+
truststore_namespace,
411+
truststore_uid,
412+
ObjectRef::from_obj(&trust_cm).erase(),
413+
)
414+
.await?;
322415
ctx.client
323416
.apply_patch(CONTROLLER_NAME, &trust_cm, &trust_cm)
324417
.await
@@ -333,6 +426,14 @@ async fn reconcile(
333426
data: Some(binary_data),
334427
..Default::default()
335428
};
429+
ensure_existing_target_is_not_foreign::<Secret>(
430+
&ctx.client,
431+
truststore_name,
432+
truststore_namespace,
433+
truststore_uid,
434+
ObjectRef::from_obj(&trust_secret).erase(),
435+
)
436+
.await?;
336437
ctx.client
337438
.apply_patch(CONTROLLER_NAME, &trust_secret, &trust_secret)
338439
.await
@@ -352,3 +453,90 @@ fn error_policy(
352453
) -> controller::Action {
353454
controller::Action::requeue(Duration::from_secs(5))
354455
}
456+
457+
#[cfg(test)]
458+
mod tests {
459+
use super::*;
460+
461+
fn owner_ref(
462+
api_version: &str,
463+
kind: &str,
464+
uid: &str,
465+
controller: Option<bool>,
466+
) -> OwnerReference {
467+
OwnerReference {
468+
api_version: api_version.to_string(),
469+
kind: kind.to_string(),
470+
name: "some-name".to_string(),
471+
uid: uid.to_string(),
472+
controller,
473+
block_owner_deletion: None,
474+
}
475+
}
476+
477+
#[test]
478+
fn empty_owner_refs_are_rejected() {
479+
assert!(!is_owned_by_truststore(&[], "my-uid"));
480+
}
481+
482+
#[test]
483+
fn matching_controller_owner_ref_is_accepted() {
484+
let owners = [owner_ref(
485+
"secrets.stackable.tech/v1alpha1",
486+
"TrustStore",
487+
"my-uid",
488+
Some(true),
489+
)];
490+
assert!(is_owned_by_truststore(&owners, "my-uid"));
491+
}
492+
493+
#[test]
494+
fn non_controller_owner_ref_is_rejected() {
495+
let owners = [owner_ref(
496+
"secrets.stackable.tech/v1alpha1",
497+
"TrustStore",
498+
"my-uid",
499+
Some(false),
500+
)];
501+
assert!(!is_owned_by_truststore(&owners, "my-uid"));
502+
}
503+
504+
#[test]
505+
fn different_uid_is_rejected() {
506+
let owners = [owner_ref(
507+
"secrets.stackable.tech/v1alpha1",
508+
"TrustStore",
509+
"other-uid",
510+
Some(true),
511+
)];
512+
assert!(!is_owned_by_truststore(&owners, "my-uid"));
513+
}
514+
515+
#[test]
516+
fn different_kind_is_rejected() {
517+
let owners = [owner_ref("v1", "ConfigMap", "my-uid", Some(true))];
518+
assert!(!is_owned_by_truststore(&owners, "my-uid"));
519+
}
520+
521+
#[test]
522+
fn foreign_api_group_is_rejected() {
523+
let owners = [owner_ref(
524+
"evil.example.com/v1",
525+
"TrustStore",
526+
"my-uid",
527+
Some(true),
528+
)];
529+
assert!(!is_owned_by_truststore(&owners, "my-uid"));
530+
}
531+
532+
#[test]
533+
fn unrelated_controller_owner_ref_is_rejected() {
534+
let owners = [owner_ref(
535+
"apps/v1",
536+
"Deployment",
537+
"some-deployment-uid",
538+
Some(true),
539+
)];
540+
assert!(!is_owned_by_truststore(&owners, "my-uid"));
541+
}
542+
}

0 commit comments

Comments
 (0)