Skip to content

Commit 47559a0

Browse files
sbernaueradwk67
andauthored
fix: Prevent missing certificates (#753)
* fix: Prevent missing certificates * changelog * Apply suggestions from code review Co-authored-by: Andrew Kenworthy <[email protected]> --------- Co-authored-by: Andrew Kenworthy <[email protected]>
1 parent 57ea52e commit 47559a0

File tree

6 files changed

+40
-100
lines changed

6 files changed

+40
-100
lines changed

CHANGELOG.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,18 @@ All notable changes to this project will be documented in this file.
88

99
- Helm: Allow Pod `priorityClassName` to be configured ([#752]).
1010

11+
### Fixed
12+
13+
- Previously we had a bug that could lead to missing certificates ([#753]).
14+
15+
This could be the case when the Stackable PKI rotated its CA certificate or you specified multiple
16+
CAs in your SecretClass.
17+
Especially the CA rotation could break working clusters at any time.
18+
We now correctly handle multiple certificates for both cases.
19+
See [this GitHub issue](https://github.com/stackabletech/issues/issues/764) for details
20+
1121
[#752]: https://github.com/stackabletech/druid-operator/pull/752
22+
[#753]: https://github.com/stackabletech/druid-operator/pull/753
1223

1324
## [25.7.0] - 2025-07-23
1425

rust/operator-binary/src/authentication/ldap.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,15 +96,13 @@ pub fn generate_runtime_properties_config(
9696
}
9797

9898
pub fn prepare_container_commands(
99-
auth_class_name: &String,
10099
provider: &ldap::v1alpha1::AuthenticationProvider,
101100
command: &mut Vec<String>,
102101
) {
103102
if let Some(tls_ca_cert_mount_path) = provider.tls.tls_ca_cert_mount_path() {
104103
command.push(add_cert_to_trust_store_cmd(
105104
&tls_ca_cert_mount_path,
106105
STACKABLE_TLS_DIR,
107-
&format!("ldap-{}", auth_class_name),
108106
TLS_STORE_PASSWORD,
109107
))
110108
}

rust/operator-binary/src/authentication/mod.rs

Lines changed: 6 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -54,11 +54,9 @@ pub enum Error {
5454
pub enum DruidAuthenticationConfig {
5555
Tls {},
5656
Ldap {
57-
auth_class_name: String,
5857
provider: authentication::ldap::v1alpha1::AuthenticationProvider,
5958
},
6059
Oidc {
61-
auth_class_name: String,
6260
provider: authentication::oidc::v1alpha1::AuthenticationProvider,
6361
oidc: authentication::oidc::v1alpha1::ClientAuthenticationOptions,
6462
},
@@ -75,19 +73,10 @@ impl DruidAuthenticationConfig {
7573
None => Ok(None),
7674
Some(auth_class_resolved) => match &auth_class_resolved {
7775
AuthenticationClassResolved::Tls { .. } => Ok(Some(Self::Tls {})),
78-
AuthenticationClassResolved::Ldap {
79-
auth_class_name,
80-
provider,
81-
} => Ok(Some(Self::Ldap {
82-
auth_class_name: auth_class_name.to_string(),
76+
AuthenticationClassResolved::Ldap { provider, .. } => Ok(Some(Self::Ldap {
8377
provider: provider.clone(),
8478
})),
85-
AuthenticationClassResolved::Oidc {
86-
auth_class_name,
87-
provider,
88-
oidc,
89-
} => Ok(Some(Self::Oidc {
90-
auth_class_name: auth_class_name.to_string(),
79+
AuthenticationClassResolved::Oidc { provider, oidc, .. } => Ok(Some(Self::Oidc {
9180
provider: provider.clone(),
9281
oidc: oidc.clone(),
9382
})),
@@ -133,25 +122,16 @@ impl DruidAuthenticationConfig {
133122

134123
pub fn main_container_commands(&self) -> Vec<String> {
135124
let mut command = vec![];
136-
if let DruidAuthenticationConfig::Oidc {
137-
auth_class_name,
138-
provider,
139-
..
140-
} = self
141-
{
142-
oidc::main_container_commands(auth_class_name, provider, &mut command)
125+
if let DruidAuthenticationConfig::Oidc { provider, .. } = self {
126+
oidc::main_container_commands(provider, &mut command)
143127
}
144128
command
145129
}
146130

147131
pub fn prepare_container_commands(&self) -> Vec<String> {
148132
let mut command = vec![];
149-
if let DruidAuthenticationConfig::Ldap {
150-
auth_class_name,
151-
provider,
152-
} = self
153-
{
154-
ldap::prepare_container_commands(auth_class_name, provider, &mut command)
133+
if let DruidAuthenticationConfig::Ldap { provider } = self {
134+
ldap::prepare_container_commands(provider, &mut command)
155135
}
156136
command
157137
}

rust/operator-binary/src/authentication/oidc.rs

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -107,15 +107,11 @@ pub fn generate_runtime_properties_config(
107107
}
108108

109109
pub fn main_container_commands(
110-
auth_class_name: &String,
111110
provider: &oidc::v1alpha1::AuthenticationProvider,
112111
command: &mut Vec<String>,
113112
) {
114113
if let Some(tls_ca_cert_mount_path) = provider.tls.tls_ca_cert_mount_path() {
115-
command.push(add_cert_to_jvm_trust_store_cmd(
116-
&tls_ca_cert_mount_path,
117-
&format!("oidc-{}", auth_class_name),
118-
))
114+
command.push(add_cert_to_jvm_trust_store_cmd(&tls_ca_cert_mount_path))
119115
}
120116
}
121117

rust/operator-binary/src/crd/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use std::collections::{BTreeMap, HashMap, HashSet};
22

33
use indoc::formatdoc;
44
use product_config::types::PropertyNameKind;
5+
use security::add_cert_to_jvm_trust_store_cmd;
56
use serde::{Deserialize, Serialize};
67
use snafu::{OptionExt, ResultExt, Snafu};
78
use stackable_operator::{
@@ -996,8 +997,7 @@ impl DruidRole {
996997

997998
if let Some(s3) = s3 {
998999
if let Some(ca_cert_file) = s3.tls.tls_ca_cert_mount_path() {
999-
// The alias can not clash, as we only support a single S3Connection
1000-
commands.push(format!("keytool -importcert -file {ca_cert_file} -alias stackable-s3-ca-cert -keystore {STACKABLE_TRUST_STORE} -storepass {STACKABLE_TRUST_STORE_PASSWORD} -noprompt"));
1000+
commands.push(add_cert_to_jvm_trust_store_cmd(&ca_cert_file));
10011001
}
10021002
}
10031003

rust/operator-binary/src/crd/security.rs

Lines changed: 20 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ use stackable_operator::{
2222
};
2323

2424
use crate::crd::{
25-
DruidRole, STACKABLE_TRUST_STORE, STACKABLE_TRUST_STORE_PASSWORD,
25+
DruidRole, STACKABLE_TRUST_STORE_PASSWORD,
2626
authentication::{self, AuthenticationClassesResolved},
2727
v1alpha1,
2828
};
@@ -81,15 +81,16 @@ const SERVER_HTTPS_CERT_ALIAS: &str = "druid.server.https.certAlias";
8181
const SERVER_HTTPS_VALIDATE_HOST_NAMES: &str = "druid.server.https.validateHostnames";
8282
const SERVER_HTTPS_KEY_MANAGER_PASSWORD: &str = "druid.server.https.keyManagerPassword";
8383
const SERVER_HTTPS_REQUIRE_CLIENT_CERTIFICATE: &str = "druid.server.https.requireClientCertificate";
84-
const TLS_ALIAS_NAME: &str = "tls";
84+
/// The alias of the certificate in the keystore used for TLS stuff.
85+
/// All secret-op generated keystores have one entry with the alias "1".
86+
/// (side node: I think technically they don't have an alias and the JVm counts them, but not sure)
87+
const TLS_ALIAS_NAME: &str = "1";
8588
pub const AUTH_TRUST_STORE_PATH: &str = "druid.auth.basic.ssl.trustStorePath";
8689
pub const AUTH_TRUST_STORE_TYPE: &str = "druid.auth.basic.ssl.trustStoreType";
8790
pub const AUTH_TRUST_STORE_PASSWORD: &str = "druid.auth.basic.ssl.trustStorePassword";
8891
// Misc TLS
8992
pub const TLS_STORE_PASSWORD: &str = "changeit";
9093
pub const TLS_STORE_TYPE: &str = "pkcs12";
91-
const SYSTEM_TRUST_STORE: &str = "/etc/pki/java/cacerts";
92-
const SYSTEM_TRUST_STORE_PASSWORD: &str = "changeit";
9394

9495
// directories
9596
const STACKABLE_MOUNT_TLS_DIR: &str = "/stackable/mount_tls";
@@ -432,12 +433,13 @@ impl DruidTlsSecurity {
432433
}
433434

434435
vec![
435-
// Copy system truststore to empty dir and convert to PKCS12
436-
import_system_truststore(STACKABLE_TLS_DIR),
437-
// Import secret-op truststore to copied system trust store
438-
import_truststore(STACKABLE_MOUNT_TLS_DIR, STACKABLE_TLS_DIR),
439-
// Import / Copy secret-op keystore to empty dir and set required alias
440-
import_keystore(STACKABLE_MOUNT_TLS_DIR, STACKABLE_TLS_DIR),
436+
// FIXME: *Technically* we should only add the system truststore in case any webPki usage is detected,
437+
// wether that's in S3, LDAP, OIDC, FTE or whatnot.
438+
format!(
439+
"cert-tools generate-pkcs12-truststore --pkcs12 '{STACKABLE_MOUNT_TLS_DIR}/truststore.p12:{TLS_STORE_PASSWORD}' --pem /etc/pki/ca-trust/extracted/pem/tls-ca-bundle.pem --out {STACKABLE_TLS_DIR}/truststore.p12 --out-password '{TLS_STORE_PASSWORD}'"
440+
),
441+
// We can copy the keystore as is.
442+
format!("cp {STACKABLE_MOUNT_TLS_DIR}/keystore.p12 {STACKABLE_TLS_DIR}/keystore.p12"),
441443
]
442444
}
443445

@@ -468,66 +470,19 @@ impl DruidTlsSecurity {
468470
}
469471
}
470472

471-
/// Generate a script to add a CA to a truststore
473+
/// Generate a bash command to add a CA to a truststore
472474
pub fn add_cert_to_trust_store_cmd(
473-
cert: &str,
474-
trust_store_directory: &str,
475-
alias_name: &str,
475+
cert_file: &str,
476+
destination_directory: &str,
476477
store_password: &str,
477478
) -> String {
479+
let truststore = format!("{destination_directory}/truststore.p12");
478480
format!(
479-
"keytool -importcert -file {cert} -keystore {trust_store_directory}/truststore.p12 -storetype pkcs12 -alias {alias_name} -storepass {store_password} -noprompt"
481+
"cert-tools generate-pkcs12-truststore --pkcs12 {truststore}:{store_password} --pem {cert_file} --out {truststore} --out-password {store_password}"
480482
)
481483
}
482484

483-
pub fn add_cert_to_jvm_trust_store_cmd(cert: &str, alias_name: &str) -> String {
484-
format!(
485-
"keytool -importcert -file {cert} -keystore {STACKABLE_TRUST_STORE} -storetype pkcs12 -alias {alias_name} -storepass {STACKABLE_TRUST_STORE_PASSWORD} -noprompt"
486-
)
487-
}
488-
489-
/// Import the system truststore to a truststore named `truststore.p12` in `destination_directory`.
490-
fn import_system_truststore(destination_directory: &str) -> String {
491-
let dest_truststore_path = format!("{destination_directory}/truststore.p12");
492-
format!(
493-
"keytool -importkeystore -srckeystore {SYSTEM_TRUST_STORE} -srcstoretype jks -srcstorepass {SYSTEM_TRUST_STORE_PASSWORD} -destkeystore {dest_truststore_path} -deststoretype pkcs12 -deststorepass {TLS_STORE_PASSWORD} -noprompt"
494-
)
495-
}
496-
497-
/// Generates the shell script to import a secret operator provided truststore without password
498-
/// into a new truststore with password in a writeable empty dir
499-
///
500-
/// # Arguments
501-
/// - `source_directory`: The directory of the source truststore. Should usually be a secret
502-
/// operator volume mount.
503-
/// - `destination_directory`: The directory of the destination truststore. Should usually be an
504-
/// empty dir.
505-
fn import_truststore(source_directory: &str, destination_directory: &str) -> String {
506-
let source_truststore_path = format!("{source_directory}/truststore.p12");
507-
let dest_truststore_path = format!("{destination_directory}/truststore.p12");
508-
// The source directory is a secret-op mount and we do not want to write / add anything in there
509-
// Therefore we import all the contents to a truststore in "writeable" empty dirs.
510-
// Keytool is only barking if a password is not set for the destination truststore (which we set)
511-
// and do provide an empty password for the source truststore coming from the secret-operator.
512-
// Using no password will result in a warning.
513-
// All secret-op generated truststores have one entry with alias "1". We generate a UUID for
514-
// the destination truststore to avoid conflicts when importing multiple secret-op generated
515-
// truststores. We do not use the UUID rust crate since this will continuously change the STS... and
516-
// leads to never-ending reconciles.
517-
format!(
518-
"keytool -importkeystore -srckeystore {source_truststore_path} -srcstoretype PKCS12 -srcstorepass {TLS_STORE_PASSWORD} -srcalias 1 -destkeystore {dest_truststore_path} -deststoretype PKCS12 -deststorepass {TLS_STORE_PASSWORD} -destalias $(cat /proc/sys/kernel/random/uuid) -noprompt"
519-
)
520-
}
521-
522-
/// Generate a script to import a mounted keystore to an empty dir and set an alias
523-
fn import_keystore(source_directory: &str, destination_directory: &str) -> String {
524-
let source_keystore_path = format!("{source_directory}/keystore.p12");
525-
let dest_keystore_path = format!("{destination_directory}/keystore.p12");
526-
// The source directory is a secret-op mount and we do not want to write / add anything in there
527-
// Therefore we import all the contents to a keystore in "writeable" empty dirs.
528-
// Using no password will result in a warning.
529-
// All secret-op generated keystores have one entry with alias "1".
530-
format!(
531-
"keytool -importkeystore -srckeystore {source_keystore_path} -srcstoretype PKCS12 -srcstorepass {TLS_STORE_PASSWORD} -srcalias 1 -destkeystore {dest_keystore_path} -deststoretype PKCS12 -deststorepass {TLS_STORE_PASSWORD} -destalias {TLS_ALIAS_NAME} -noprompt"
532-
)
485+
/// Generate a bash command to add a CA to the truststore that is passed to the JVM
486+
pub fn add_cert_to_jvm_trust_store_cmd(cert_file: &str) -> String {
487+
add_cert_to_trust_store_cmd(cert_file, "/stackable", STACKABLE_TRUST_STORE_PASSWORD)
533488
}

0 commit comments

Comments
 (0)