Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Correctly encode user given content, such as passwords #571

Merged
merged 28 commits into from
Jul 10, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
ea145f5
replaced first sed instance
Jun 3, 2024
1e144d1
removed second instance of sed - still needs testing
Jun 3, 2024
b75aed0
ldap test worked
Jun 4, 2024
06852b4
remove outdated comment
Jun 4, 2024
adc870f
simplify and add "
Jun 6, 2024
3435367
make ldap admin password more complicated
Jun 6, 2024
9144aa2
use the feature to load secret directly from file
Jun 6, 2024
6f8f241
Load S3 keys from file instead of from env
Jun 6, 2024
c90850c
Merge remote-tracking branch 'origin/main' into 219-replace-sed-with-…
Jun 10, 2024
ad781cc
Merge branch 'main' into 219-replace-sed-with-env-feature
fhennig Jun 19, 2024
603e24c
Split INTERNAL_INITIAL_CLIENT_PASSWORD_ENV and ESCALATOR_INTERNAL_CLI…
sbernauer Jun 25, 2024
550bd69
fix typo
sbernauer Jun 25, 2024
88fb6dc
fix: Re-create internal secret if needed
sbernauer Jun 25, 2024
755a971
typo
sbernauer Jun 25, 2024
dc7b783
Improve internal secret management
sbernauer Jun 25, 2024
6fcd7dc
typo
sbernauer Jun 25, 2024
28950f2
Merge branch 'main' into 219-replace-sed-with-env-feature
sbernauer Jun 25, 2024
55aff25
typo
sbernauer Jun 25, 2024
5cb553d
revert test changes
sbernauer Jun 25, 2024
46c7eaa
Actually run config-utils
sbernauer Jun 25, 2024
3e6ee68
refactor ldap test
labrenbe Jun 25, 2024
c9b4ec2
Adopt ldap test name
sbernauer Jun 25, 2024
07f055e
Fix test
sbernauer Jun 25, 2024
238ca01
fix tests by adding --skip-tests option
labrenbe Jun 26, 2024
46e1e95
Merge remote-tracking branch 'origin/main' into 219-replace-sed-with-…
sbernauer Jun 26, 2024
36f3169
Revert "fix tests by adding --skip-tests option"
sbernauer Jun 26, 2024
a0d3f6b
Set terminationGracePeriodSeconds on all test containers
sbernauer Jun 26, 2024
058fa41
Merge branch 'main' into 219-replace-sed-with-env-feature
labrenbe Jul 10, 2024
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
36 changes: 11 additions & 25 deletions rust/crd/src/authentication/ldap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use stackable_operator::kube::ResourceExt;
use crate::authentication::ResolvedAuthenticationClasses;
use crate::{
security::{add_cert_to_trust_store_cmd, STACKABLE_TLS_DIR, TLS_STORE_PASSWORD},
ENV_INTERNAL_SECRET, RUNTIME_PROPS, RW_CONFIG_DIRECTORY,
ENV_INTERNAL_SECRET,
};

#[derive(Snafu, Debug)]
Expand All @@ -25,10 +25,8 @@ pub struct DruidLdapSettings {
pub authentication_class_name: String,
}

pub const PLACEHOLDER_INTERNAL_CLIENT_PASSWORD: &str =
"xxx_druid_system_internal_client_password_xxx";
pub const PLACEHOLDER_LDAP_BIND_PASSWORD: &str = "xxx_ldap_bind_password_xxx";
pub const PLACEHOLDER_LDAP_BIND_USER: &str = "xxx_ldap_bind_user_xxx";
pub const ENV_LDAP_BIND_USER: &str = "LDAP_BIND_USER";
pub const ENV_LDAP_BIND_PASSWORD: &str = "LDAP_BIND_PASSWORD";

impl DruidLdapSettings {
pub fn new_from(
Expand Down Expand Up @@ -63,7 +61,7 @@ impl DruidLdapSettings {

config.insert(
format!("{PREFIX}.initialInternalClientPassword"),
Some(PLACEHOLDER_INTERNAL_CLIENT_PASSWORD.to_string()),
Some(format!("${{env:{ENV_INTERNAL_SECRET}}}").to_string()),
);
config.insert(
format!("{PREFIX}.authorizerName"),
Expand Down Expand Up @@ -100,11 +98,11 @@ impl DruidLdapSettings {
if self.ldap.bind_credentials_mount_paths().is_some() {
config.insert(
format!("{PREFIX}.credentialsValidator.bindUser"),
Some(PLACEHOLDER_LDAP_BIND_USER.to_string()), // NOTE: this placeholder will be replaced from a mounted secret operator volume on container startup
Some(format!("${{env:{ENV_LDAP_BIND_USER}}}").to_string()),
);
config.insert(
format!("{PREFIX}.credentialsValidator.bindPassword"),
Some(PLACEHOLDER_LDAP_BIND_PASSWORD.to_string()), // NOTE: this placeholder will be replaced from a mounted secret operator volume on container startup
Some(format!("${{env:{ENV_LDAP_BIND_PASSWORD}}}").to_string()),
);
}

Expand Down Expand Up @@ -139,7 +137,7 @@ impl DruidLdapSettings {
);
config.insert(
"druid.escalator.internalClientPassword".to_string(),
Some(PLACEHOLDER_INTERNAL_CLIENT_PASSWORD.to_string()),
Some(format!("${{env:{ENV_INTERNAL_SECRET}}}").to_string()),
);
config.insert(
"druid.escalator.authorizerName".to_string(),
Expand Down Expand Up @@ -183,27 +181,15 @@ impl DruidLdapSettings {
pub fn main_container_commands(&self) -> Vec<String> {
let mut commands = Vec::new();

let runtime_properties_file: String = format!("{RW_CONFIG_DIRECTORY}/{RUNTIME_PROPS}");
let internal_client_password = format!("$(echo ${ENV_INTERNAL_SECRET})");

commands
.push(format!("echo \"Replacing LDAP placeholders with their proper values in {runtime_properties_file}\""));
commands.push(format!(
r#"sed "s|{PLACEHOLDER_INTERNAL_CLIENT_PASSWORD}|{internal_client_password}|g" -i {runtime_properties_file}"# // using another delimiter (|) here because of base64 string
));

if let Some((ldap_bind_user_path, ldap_bind_password_path)) =
self.ldap.bind_credentials_mount_paths()
{
let ldap_bind_user = format!("$(cat {ldap_bind_user_path})");
let ldap_bind_password = format!("$(cat {ldap_bind_password_path})");

commands.push(format!(
r#"sed "s/{PLACEHOLDER_LDAP_BIND_USER}/{ldap_bind_user}/g" -i {runtime_properties_file}"#
));
"export {ENV_LDAP_BIND_USER}=\"$(cat {ldap_bind_user_path})\""
sbernauer marked this conversation as resolved.
Show resolved Hide resolved
));
commands.push(format!(
r#"sed "s/{PLACEHOLDER_LDAP_BIND_PASSWORD}/{ldap_bind_password}/g" -i {runtime_properties_file}"#
));
"export {ENV_LDAP_BIND_PASSWORD}=\"$(cat {ldap_bind_password_path})\""
));
}

commands
Expand Down
18 changes: 3 additions & 15 deletions rust/crd/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,9 +136,7 @@ pub const SC_VOLUME_NAME: &str = "segment-cache";

pub const ENV_INTERNAL_SECRET: &str = "INTERNAL_SECRET";

// DB credentials
pub const DB_USERNAME_PLACEHOLDER: &str = "xxx_db_username_xxx";
pub const DB_PASSWORD_PLACEHOLDER: &str = "xxx_db_password_xxx";
// DB credentials - both of these are read from an env var by Druid with the ${env:...} syntax
pub const DB_USERNAME_ENV: &str = "DB_USERNAME_ENV";
pub const DB_PASSWORD_ENV: &str = "DB_PASSWORD_ENV";

Expand Down Expand Up @@ -518,7 +516,6 @@ impl DruidRole {
pub fn main_container_prepare_commands(
&self,
s3_connection: Option<&S3ConnectionSpec>,
credentials_secret: Option<&String>,
) -> Vec<String> {
let mut commands = vec![];

Expand Down Expand Up @@ -560,15 +557,6 @@ impl DruidRole {
rw_conf = RW_CONFIG_DIRECTORY,
));

// db credentials
if credentials_secret.is_some() {
commands.extend([
format!("echo replacing {DB_USERNAME_PLACEHOLDER} and {DB_PASSWORD_PLACEHOLDER} with secret values."),
format!("sed -i \"s|{DB_USERNAME_PLACEHOLDER}|${DB_USERNAME_ENV}|g\" {RW_CONFIG_DIRECTORY}/{RUNTIME_PROPS}"),
format!("sed -i \"s|{DB_PASSWORD_PLACEHOLDER}|${DB_PASSWORD_ENV}|g\" {RW_CONFIG_DIRECTORY}/{RUNTIME_PROPS}"),
]);
}

commands
}

Expand Down Expand Up @@ -631,11 +619,11 @@ impl DruidCluster {
if mds.credentials_secret.is_some() {
result.insert(
METADATA_STORAGE_USER.to_string(),
Some(DB_USERNAME_PLACEHOLDER.into()),
Some(format!("${{env:{DB_USERNAME_ENV}}}")),
);
result.insert(
METADATA_STORAGE_PASSWORD.to_string(),
Some(DB_PASSWORD_PLACEHOLDER.into()),
Some(format!("${{env:{DB_PASSWORD_ENV}}}")),
);
}

Expand Down
3 changes: 1 addition & 2 deletions rust/operator-binary/src/druid_controller.rs
Original file line number Diff line number Diff line change
Expand Up @@ -915,8 +915,7 @@ fn build_rolegroup_statefulset(
.metadata_storage_database
.credentials_secret
.as_ref();
let mut main_container_commands =
role.main_container_prepare_commands(s3_conn, credentials_secret);
let mut main_container_commands = role.main_container_prepare_commands(s3_conn);
let mut prepare_container_commands = vec![];
if let Some(ContainerLogConfig {
choice: Some(ContainerLogConfigChoice::Automatic(log_config)),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@
apiVersion: kuttl.dev/v1beta1
kind: TestAssert
commands:
- script: kubectl exec -n $NAMESPACE openldap-0 -- ldapsearch -H ldap://localhost:1389 -D cn=admin,dc=example,dc=org -w admin -b ou=users,dc=example,dc=org > /dev/null
- script: kubectl exec -n $NAMESPACE openldap-0 -- bash -c LDAPTLS_CACERT=/tls/ca.crt ldapsearch -Z -H ldaps://localhost:1636 -D cn=admin,dc=example,dc=org -w admin -b ou=users,dc=example,dc=org > /dev/null
- script: kubectl exec -n $NAMESPACE openldap-0 -- ldapsearch -H ldap://localhost:1389 -D cn=admin,dc=example,dc=org -w "admin-pw withSpace$%\" &&} §" -b ou=users,dc=example,dc=org > /dev/null
- script: kubectl exec -n $NAMESPACE openldap-0 -- bash -c LDAPTLS_CACERT=/tls/ca.crt ldapsearch -Z -H ldaps://localhost:1636 -D cn=admin,dc=example,dc=org -w "admin-pw withSpace$%\" &&} §" -b ou=users,dc=example,dc=org > /dev/null
Original file line number Diff line number Diff line change
Expand Up @@ -46,4 +46,4 @@ metadata:
secrets.stackable.tech/class: druid-ldap-secret
stringData:
user: cn=admin,dc=example,dc=org
password: admin
password: "admin-pw withSpace$%\" &&} §"
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ spec:
- name: LDAP_ADMIN_USERNAME
value: admin
- name: LDAP_ADMIN_PASSWORD
value: admin
value: "admin-pw withSpace$%\" &&} §" # use spaces and special characters to make sure it doesn't break anything
- name: LDAP_ENABLE_TLS
value: \"yes\"
- name: LDAP_TLS_CERT_FILE
Expand Down