diff --git a/CHANGELOG.md b/CHANGELOG.md index 6adc9199..3c9a98d2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,8 +10,19 @@ All notable changes to this project will be documented in this file. - Support for the client spooling protocol ([#793]). - Helm: Allow Pod `priorityClassName` to be configured ([#798]). +### Fixed + +- Previously we had a bug that could lead to missing certificates ([#796]). + + This could be the case when the Stackable PKI rotated its CA certificate or you specified multiple + CAs in your SecretClass. + Especially the CA rotation could break working clusters at any time. + We now correctly handle multiple certificates for both cases. + See [this GitHub issue](https://github.com/stackabletech/issues/issues/764) for details + [#779]: https://github.com/stackabletech/trino-operator/pull/779 [#793]: https://github.com/stackabletech/trino-operator/pull/793 +[#796]: https://github.com/stackabletech/trino-operator/pull/796 [#798]: https://github.com/stackabletech/trino-operator/pull/798 ## [25.7.0] - 2025-07-23 diff --git a/rust/operator-binary/src/authentication/oidc/mod.rs b/rust/operator-binary/src/authentication/oidc/mod.rs index 7d3615f5..5f2449e6 100644 --- a/rust/operator-binary/src/authentication/oidc/mod.rs +++ b/rust/operator-binary/src/authentication/oidc/mod.rs @@ -184,7 +184,7 @@ impl TrinoOidcAuthentication { oauth2_authentication_config.add_commands( TrinoRole::Coordinator, crate::crd::Container::Prepare, - command::add_cert_to_truststore(&path, STACKABLE_CLIENT_TLS_DIR, "oidc-idp"), + command::add_cert_to_truststore(&path, STACKABLE_CLIENT_TLS_DIR), ); } } diff --git a/rust/operator-binary/src/catalog/commons.rs b/rust/operator-binary/src/catalog/commons.rs index 86cda6c5..fc3f4e26 100644 --- a/rust/operator-binary/src/catalog/commons.rs +++ b/rust/operator-binary/src/catalog/commons.rs @@ -77,7 +77,7 @@ impl ExtendCatalogConfig for s3::v1alpha1::InlineConnectionOrReference { async fn extend_catalog_config( &self, catalog_config: &mut CatalogConfig, - catalog_name: &str, + _catalog_name: &str, catalog_namespace: Option, client: &Client, trino_version: u16, @@ -151,11 +151,7 @@ impl ExtendCatalogConfig for s3::v1alpha1::InlineConnectionOrReference { }) => { if let Some(ca_cert) = s3.tls.tls_ca_cert_mount_path() { catalog_config.init_container_extra_start_commands.extend( - command::add_cert_to_truststore( - &ca_cert, - STACKABLE_CLIENT_TLS_DIR, - &format!("{catalog_name}-ca-cert"), - ), + command::add_cert_to_truststore(&ca_cert, STACKABLE_CLIENT_TLS_DIR), ); } } diff --git a/rust/operator-binary/src/command.rs b/rust/operator-binary/src/command.rs index 29c5769a..e14cac17 100644 --- a/rust/operator-binary/src/command.rs +++ b/rust/operator-binary/src/command.rs @@ -16,7 +16,7 @@ use crate::{ RW_CONFIG_DIR_NAME, SPOOLING_MANAGER_PROPERTIES, STACKABLE_CLIENT_TLS_DIR, STACKABLE_INTERNAL_TLS_DIR, STACKABLE_MOUNT_INTERNAL_TLS_DIR, STACKABLE_MOUNT_SERVER_TLS_DIR, STACKABLE_SERVER_TLS_DIR, STACKABLE_TLS_STORE_PASSWORD, - SYSTEM_TRUST_STORE, SYSTEM_TRUST_STORE_PASSWORD, TrinoRole, v1alpha1, + TrinoRole, v1alpha1, }, }; @@ -45,38 +45,25 @@ pub fn container_prepare_args( )); } + // Create truststore that will be used when talking to external tools like S3 + // It will be populated from the system truststore so that connections against public services like AWS S3 are still possible + // FIXME: *Technically* we should only add the system truststore in case any webPki usage is detected, whether that's in + // S3, LDAP, OIDC, FTE or whatnot. + args.push(format!("cert-tools generate-pkcs12-truststore --pem /etc/pki/ca-trust/extracted/pem/tls-ca-bundle.pem --out {STACKABLE_CLIENT_TLS_DIR}/truststore.p12 --out-password {STACKABLE_TLS_STORE_PASSWORD}")); + if trino.tls_enabled() { - args.extend(import_truststore( - STACKABLE_MOUNT_SERVER_TLS_DIR, - STACKABLE_SERVER_TLS_DIR, - )); - args.extend(import_keystore( - STACKABLE_MOUNT_SERVER_TLS_DIR, - STACKABLE_SERVER_TLS_DIR, - )); + args.push(format!("cp {STACKABLE_MOUNT_SERVER_TLS_DIR}/truststore.p12 {STACKABLE_SERVER_TLS_DIR}/truststore.p12")); + args.push(format!("cp {STACKABLE_MOUNT_SERVER_TLS_DIR}/keystore.p12 {STACKABLE_SERVER_TLS_DIR}/keystore.p12")); } if trino.get_internal_tls().is_some() { - args.extend(import_truststore( - STACKABLE_MOUNT_INTERNAL_TLS_DIR, - STACKABLE_INTERNAL_TLS_DIR, - )); - args.extend(import_keystore( - STACKABLE_MOUNT_INTERNAL_TLS_DIR, - STACKABLE_INTERNAL_TLS_DIR, - )); + args.push(format!("cp {STACKABLE_MOUNT_INTERNAL_TLS_DIR}/truststore.p12 {STACKABLE_INTERNAL_TLS_DIR}/truststore.p12")); + args.push(format!("cp {STACKABLE_MOUNT_INTERNAL_TLS_DIR}/keystore.p12 {STACKABLE_INTERNAL_TLS_DIR}/keystore.p12")); if trino.tls_enabled() { - args.extend(import_truststore( - STACKABLE_MOUNT_SERVER_TLS_DIR, - STACKABLE_INTERNAL_TLS_DIR, - )) + args.push(format!("cert-tools generate-pkcs12-truststore --pkcs12 {STACKABLE_MOUNT_SERVER_TLS_DIR}/truststore.p12:{STACKABLE_TLS_STORE_PASSWORD} --pkcs12 {STACKABLE_INTERNAL_TLS_DIR}/truststore.p12:{STACKABLE_TLS_STORE_PASSWORD} --out {STACKABLE_INTERNAL_TLS_DIR}/truststore.p12 --out-password {STACKABLE_TLS_STORE_PASSWORD}")); } } - // Create truststore that will be used when talking to external tools like S3 - // It will be populated from the system truststore so that connections against public services like AWS S3 are still possible - args.extend(import_system_truststore(STACKABLE_CLIENT_TLS_DIR)); - // Add the commands that are needed to set up the catalogs catalogs.iter().for_each(|catalog| { args.extend_from_slice(&catalog.init_container_extra_start_commands); @@ -159,77 +146,11 @@ wait_for_termination $! args } -/// Adds a CA file from `cert_file` into a truststore named `truststore.p12` in `destination_directory` -/// under the alias `alias_name`. -pub fn add_cert_to_truststore( - cert_file: &str, - destination_directory: &str, - alias_name: &str, -) -> Vec { - vec![ - format!( - "echo Adding cert from {cert_file} to truststore {destination_directory}/truststore.p12" - ), - format!( - "keytool -importcert -file {cert_file} -keystore {destination_directory}/truststore.p12 -storetype pkcs12 -noprompt -alias {alias_name} -storepass {STACKABLE_TLS_STORE_PASSWORD}" - ), - ] -} - -/// Generates the shell script to import a secret operator provided keystore without password -/// into a new keystore with password in a writeable empty dir -/// -/// # Arguments -/// - `source_directory`: The directory of the source keystore. Should usually be a secret operator volume mount. -/// - `destination_directory`: The directory of the destination keystore. Should usually be an empty dir. -fn import_keystore(source_directory: &str, destination_directory: &str) -> Vec { - vec![ - // The source directory is a secret-op mount and we do not want to write / add anything in there - // Therefore we import all the contents to a keystore in "writeable" empty dirs. - // Keytool is only barking if a password is not set for the destination keystore (which we set) - // and do provide an empty password for the source keystore coming from the secret-operator. - // Using no password will result in a warning. - format!( - "echo Importing {source_directory}/keystore.p12 to {destination_directory}/keystore.p12" - ), - format!( - "keytool -importkeystore -srckeystore {source_directory}/keystore.p12 -srcstoretype PKCS12 -srcstorepass \"\" -destkeystore {destination_directory}/keystore.p12 -deststoretype PKCS12 -deststorepass {STACKABLE_TLS_STORE_PASSWORD} -noprompt" - ), - ] -} - -/// Generates the shell script to import a secret operator provided truststore without password -/// into a new truststore with password in a writeable empty dir -/// -/// # Arguments -/// - `source_directory`: The directory of the source truststore. Should usually be a secret operator volume mount. -/// - `destination_directory`: The directory of the destination truststore. Should usually be an empty dir. -fn import_truststore(source_directory: &str, destination_directory: &str) -> Vec { - vec![ - // The source directory is a secret-op mount and we do not want to write / add anything in there - // Therefore we import all the contents to a truststore in "writeable" empty dirs. - // Keytool is only barking if a password is not set for the destination truststore (which we set) - // and do provide an empty password for the source truststore coming from the secret-operator. - // Using no password will result in a warning. - // All secret-op generated truststores have one entry with alias "1". We generate a UUID for - // the destination truststore to avoid conflicts when importing multiple secret-op generated - // truststores. We do not use the UUID rust crate since this will continuously change the STS... and - // leads to never-ending reconciles. - format!( - "echo Importing {source_directory}/truststore.p12 to {destination_directory}/truststore.p12" - ), - format!( - "keytool -importkeystore -srckeystore {source_directory}/truststore.p12 -srcstoretype PKCS12 -srcstorepass \"\" -srcalias 1 -destkeystore {destination_directory}/truststore.p12 -deststoretype PKCS12 -deststorepass {STACKABLE_TLS_STORE_PASSWORD} -destalias $(cat /proc/sys/kernel/random/uuid) -noprompt" - ), - ] -} - -/// Import the system truststore to a truststore named `truststore.p12` in `destination_directory`. -fn import_system_truststore(destination_directory: &str) -> Vec { - vec![ - format!("echo Importing {SYSTEM_TRUST_STORE} to {destination_directory}/truststore.p12"), - format!( - "keytool -importkeystore -srckeystore {SYSTEM_TRUST_STORE} -srcstoretype jks -srcstorepass {SYSTEM_TRUST_STORE_PASSWORD} -destkeystore {destination_directory}/truststore.p12 -deststoretype pkcs12 -deststorepass {STACKABLE_TLS_STORE_PASSWORD} -noprompt" - ), - ] +/// Adds a PEM file to configured PKCS12 truststore (using the [`STACKABLE_TLS_STORE_PASSWORD`] +/// password) +pub fn add_cert_to_truststore(cert_file: &str, destination_directory: &str) -> Vec { + let truststore = format!("{destination_directory}/truststore.p12"); + vec![format!( + "cert-tools generate-pkcs12-truststore --pkcs12 {truststore}:{STACKABLE_TLS_STORE_PASSWORD} --pem {cert_file} --out {truststore} --out-password {STACKABLE_TLS_STORE_PASSWORD}" + )] } diff --git a/rust/operator-binary/src/config/s3.rs b/rust/operator-binary/src/config/s3.rs index dedcbcd6..a403ce83 100644 --- a/rust/operator-binary/src/config/s3.rs +++ b/rust/operator-binary/src/config/s3.rs @@ -108,11 +108,7 @@ impl ResolvedS3Config { }) => { if let Some(ca_cert) = s3_connection.tls.tls_ca_cert_mount_path() { resolved_config.init_container_extra_start_commands.extend( - command::add_cert_to_truststore( - &ca_cert, - STACKABLE_CLIENT_TLS_DIR, - "resolved-s3-ca-cert", - ), + command::add_cert_to_truststore(&ca_cert, STACKABLE_CLIENT_TLS_DIR), ); } } diff --git a/rust/operator-binary/src/controller.rs b/rust/operator-binary/src/controller.rs index d07a817b..474114ba 100644 --- a/rust/operator-binary/src/controller.rs +++ b/rust/operator-binary/src/controller.rs @@ -87,7 +87,8 @@ use crate::{ JVM_SECURITY_PROPERTIES, LOG_PROPERTIES, MAX_TRINO_LOG_FILES_SIZE, METRICS_PORT, METRICS_PORT_NAME, NODE_PROPERTIES, RW_CONFIG_DIR_NAME, SPOOLING_MANAGER_PROPERTIES, STACKABLE_CLIENT_TLS_DIR, STACKABLE_INTERNAL_TLS_DIR, STACKABLE_MOUNT_INTERNAL_TLS_DIR, - STACKABLE_MOUNT_SERVER_TLS_DIR, STACKABLE_SERVER_TLS_DIR, TrinoRole, + STACKABLE_MOUNT_SERVER_TLS_DIR, STACKABLE_SERVER_TLS_DIR, STACKABLE_TLS_STORE_PASSWORD, + TrinoRole, authentication::resolve_authentication_classes, catalog, discovery::{TrinoDiscovery, TrinoDiscoveryProtocol, TrinoPodRef}, @@ -1689,6 +1690,7 @@ fn create_tls_volume( secret_volume_source_builder .with_pod_scope() .with_format(SecretFormat::TlsPkcs12) + .with_tls_pkcs12_password(STACKABLE_TLS_STORE_PASSWORD) .with_auto_tls_cert_lifetime(*requested_secret_lifetime); if let Some(listener_scope) = &listener_scope { diff --git a/rust/operator-binary/src/crd/mod.rs b/rust/operator-binary/src/crd/mod.rs index e3cd0aa9..238f28ff 100644 --- a/rust/operator-binary/src/crd/mod.rs +++ b/rust/operator-binary/src/crd/mod.rs @@ -101,10 +101,8 @@ pub const STACKABLE_INTERNAL_TLS_DIR: &str = "/stackable/internal_tls"; pub const STACKABLE_LOG_DIR: &str = "/stackable/log"; pub const STACKABLE_MOUNT_SERVER_TLS_DIR: &str = "/stackable/mount_server_tls"; pub const STACKABLE_MOUNT_INTERNAL_TLS_DIR: &str = "/stackable/mount_internal_tls"; -pub const SYSTEM_TRUST_STORE: &str = "/etc/pki/java/cacerts"; // store pws pub const STACKABLE_TLS_STORE_PASSWORD: &str = "changeit"; -pub const SYSTEM_TRUST_STORE_PASSWORD: &str = "changeit"; // secret vars pub const ENV_INTERNAL_SECRET: &str = "INTERNAL_SECRET"; pub const ENV_SPOOLING_SECRET: &str = "SPOOLING_SECRET";