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
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,12 @@ All notable changes to this project will be documented in this file.

## [Unreleased]

### Changed

- Make internal shared secrets stable across reconcile operations. This ensures that Trino pods have the same secrets at any time. ([#873]).

[#873]: https://github.com/stackabletech/trino-operator/pull/873

## [26.3.0] - 2026-03-16

## [26.3.0-rc1] - 2026-03-16
Expand Down
29 changes: 25 additions & 4 deletions rust/operator-binary/src/controller.rs
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,9 @@ pub enum Error {
"client spooling protocol is not supported for Trino version {product_version}"
))]
ClientSpoolingProtocolTrinoVersion { product_version: String },

#[snafu(display("failed to generate random bytes"))]
GenerateRandomBytes { source: openssl::error::ErrorStack },
}

type Result<T, E = Error> = std::result::Result<T, E>;
Expand Down Expand Up @@ -1533,7 +1536,10 @@ async fn create_random_secret(
client: &Client,
) -> Result<()> {
let mut internal_secret = BTreeMap::new();
internal_secret.insert(secret_key.to_string(), get_random_base64(secret_byte_size));
internal_secret.insert(
secret_key.to_string(),
get_random_base64(secret_name.as_bytes(), secret_byte_size)?,
);

let secret = Secret {
immutable: Some(true),
Expand Down Expand Up @@ -1578,10 +1584,17 @@ fn shared_spooling_secret_name(trino: &v1alpha1::TrinoCluster) -> String {

// TODO: Maybe switch to something non-openssl.
// See https://github.com/stackabletech/airflow-operator/pull/686#discussion_r2348354468 (which is currently under discussion)
fn get_random_base64(byte_size: usize) -> String {
fn get_random_base64(seed: &[u8], byte_size: usize) -> Result<String> {
let mut buf: Vec<u8> = vec![0; byte_size];
openssl::rand::rand_bytes(&mut buf).unwrap();
openssl::base64::encode_block(&buf)
openssl::pkcs5::pbkdf2_hmac(
seed,
b"",
1,
openssl::hash::MessageDigest::sha256(),
&mut buf,
)
Comment on lines +1589 to +1595
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already chatted in person, but I think this is problematic from a security perspective:
If a user knows the name of the stacklet, it can easily calculate the shared secret, in which case it is not secret any more ^^
Or another example: The TrinoClusters "trino" in dev, qa and prod namespaces share the same shared secret

.context(GenerateRandomBytesSnafu)?;
Ok(openssl::base64::encode_block(&buf))
}

fn container_ports(trino: &v1alpha1::TrinoCluster) -> Vec<ContainerPort> {
Expand Down Expand Up @@ -1878,6 +1891,14 @@ mod tests {
crd::v1alpha1::TrinoCluster,
};

#[test]
fn test_get_random_base64_is_stable() {
let seed = b"my-trino-cluster-internal-secret";
let result1 = get_random_base64(seed, 32).expect("failed to generate random base64");
let result2 = get_random_base64(seed, 32).expect("failed to generate random base64");
assert_eq!(result1, result2);
}

#[tokio::test]
async fn test_config_overrides() {
let trino_yaml = r#"
Expand Down
Loading