From 199662ee3f25542c3019f3d8a5efb72f1a75e269 Mon Sep 17 00:00:00 2001 From: Terry Quigley <77437788+terryquigleysas@users.noreply.github.com> Date: Fri, 4 Oct 2024 16:41:23 +0100 Subject: [PATCH] Fix env var password hashing for PBKDF2 (#4773) Signed-off-by: Terry Quigley --- .../security/support/SecurityUtils.java | 20 +++++----- .../security/tools/SecurityAdmin.java | 4 +- .../org/opensearch/security/UtilTests.java | 38 +++++++++++++++++++ 3 files changed, 50 insertions(+), 12 deletions(-) diff --git a/src/main/java/org/opensearch/security/support/SecurityUtils.java b/src/main/java/org/opensearch/security/support/SecurityUtils.java index 1df5f23637..4c278e3b37 100644 --- a/src/main/java/org/opensearch/security/support/SecurityUtils.java +++ b/src/main/java/org/opensearch/security/support/SecurityUtils.java @@ -96,16 +96,16 @@ public static String replaceEnvVars(String in, Settings settings) { return in; } - return replaceEnvVarsBC(replaceEnvVarsNonBC(replaceEnvVarsBase64(in))); + return replaceEnvVarsBC(replaceEnvVarsNonBC(replaceEnvVarsBase64(in, settings), settings), settings); } - private static String replaceEnvVarsNonBC(String in) { + private static String replaceEnvVarsNonBC(String in, Settings settings) { // ${env.MY_ENV_VAR} // ${env.MY_ENV_VAR:-default} Matcher matcher = ENV_PATTERN.matcher(in); StringBuffer sb = new StringBuffer(); while (matcher.find()) { - final String replacement = resolveEnvVar(matcher.group(1), matcher.group(2), false); + final String replacement = resolveEnvVar(matcher.group(1), matcher.group(2), false, settings); if (replacement != null) { matcher.appendReplacement(sb, Matcher.quoteReplacement(replacement)); } @@ -114,13 +114,13 @@ private static String replaceEnvVarsNonBC(String in) { return sb.toString(); } - private static String replaceEnvVarsBC(String in) { + private static String replaceEnvVarsBC(String in, Settings settings) { // ${envbc.MY_ENV_VAR} // ${envbc.MY_ENV_VAR:-default} Matcher matcher = ENVBC_PATTERN.matcher(in); StringBuffer sb = new StringBuffer(); while (matcher.find()) { - final String replacement = resolveEnvVar(matcher.group(1), matcher.group(2), true); + final String replacement = resolveEnvVar(matcher.group(1), matcher.group(2), true, settings); if (replacement != null) { matcher.appendReplacement(sb, Matcher.quoteReplacement(replacement)); } @@ -129,13 +129,13 @@ private static String replaceEnvVarsBC(String in) { return sb.toString(); } - private static String replaceEnvVarsBase64(String in) { + private static String replaceEnvVarsBase64(String in, Settings settings) { // ${envbc.MY_ENV_VAR} // ${envbc.MY_ENV_VAR:-default} Matcher matcher = ENVBASE64_PATTERN.matcher(in); StringBuffer sb = new StringBuffer(); while (matcher.find()) { - final String replacement = resolveEnvVar(matcher.group(1), matcher.group(2), false); + final String replacement = resolveEnvVar(matcher.group(1), matcher.group(2), false, settings); if (replacement != null) { matcher.appendReplacement( sb, @@ -149,16 +149,16 @@ private static String replaceEnvVarsBase64(String in) { // ${env.MY_ENV_VAR} // ${env.MY_ENV_VAR:-default} - private static String resolveEnvVar(String envVarName, String mode, boolean bc) { + private static String resolveEnvVar(String envVarName, String mode, boolean bc, Settings settings) { final String envVarValue = System.getenv(envVarName); if (envVarValue == null || envVarValue.isEmpty()) { if (mode != null && mode.startsWith(":-") && mode.length() > 2) { - return bc ? Hasher.hash(mode.substring(2).toCharArray()) : mode.substring(2); + return bc ? Hasher.hash(mode.substring(2).toCharArray(), settings) : mode.substring(2); } else { return null; } } else { - return bc ? Hasher.hash(envVarValue.toCharArray()) : envVarValue; + return bc ? Hasher.hash(envVarValue.toCharArray(), settings) : envVarValue; } } } diff --git a/src/main/java/org/opensearch/security/tools/SecurityAdmin.java b/src/main/java/org/opensearch/security/tools/SecurityAdmin.java index 6080224c36..df9b004d15 100644 --- a/src/main/java/org/opensearch/security/tools/SecurityAdmin.java +++ b/src/main/java/org/opensearch/security/tools/SecurityAdmin.java @@ -204,7 +204,7 @@ public static int execute(final String[] args) throws Exception { .longOpt("truststore-type") .hasArg() .argName("type") - .desc("JKS or PKCS12, if not given we use the file extension to dectect the type") + .desc("JKS or PKCS12, if not given we use the file extension to detect the type") .build() ); options.addOption( @@ -212,7 +212,7 @@ public static int execute(final String[] args) throws Exception { .longOpt("keystore-type") .hasArg() .argName("type") - .desc("JKS or PKCS12, if not given we use the file extension to dectect the type") + .desc("JKS or PKCS12, if not given we use the file extension to detect the type") .build() ); // CS-ENFORCE-SINGLE diff --git a/src/test/java/org/opensearch/security/UtilTests.java b/src/test/java/org/opensearch/security/UtilTests.java index 2445b560df..fcfd26576f 100644 --- a/src/test/java/org/opensearch/security/UtilTests.java +++ b/src/test/java/org/opensearch/security/UtilTests.java @@ -155,6 +155,44 @@ public void testEnvReplace() { assertTrue(checked); } + @Test + public void testEnvReplacePBKDF2() { + Settings settings = Settings.builder().put(ConfigConstants.SECURITY_PASSWORD_HASHING_ALGORITHM, ConfigConstants.PBKDF2).build(); + final PasswordHasher passwordHasherPBKDF2 = PasswordHasherFactory.createPasswordHasher(settings); + assertThat(SecurityUtils.replaceEnvVars("abv${env.MYENV}xyz", settings), is("abv${env.MYENV}xyz")); + assertThat(SecurityUtils.replaceEnvVars("abv${envbc.MYENV}xyz", settings), is("abv${envbc.MYENV}xyz")); + assertThat(SecurityUtils.replaceEnvVars("abv${env.MYENV:-tTt}xyz", settings), is("abvtTtxyz")); + assertTrue(passwordHasherPBKDF2.check("tTt".toCharArray(), SecurityUtils.replaceEnvVars("${envbc.MYENV:-tTt}", settings))); + assertThat(SecurityUtils.replaceEnvVars("abv${env.MYENV:-tTt}xyz${env.MYENV:-xxx}", settings), is("abvtTtxyzxxx")); + assertTrue(SecurityUtils.replaceEnvVars("abv${env.MYENV:-tTt}xyz${envbc.MYENV:-xxx}", settings).startsWith("abvtTtxyz$3$")); + assertThat(SecurityUtils.replaceEnvVars("abv${env.MYENV:tTt}xyz", settings), is("abv${env.MYENV:tTt}xyz")); + assertThat(SecurityUtils.replaceEnvVars("abv${env.MYENV-tTt}xyz", settings), is("abv${env.MYENV-tTt}xyz")); + + Map env = System.getenv(); + assertTrue(env.size() > 0); + + boolean checked = false; + + for (String k : env.keySet()) { + String val = System.getenv().get(k); + if (val == null || val.isEmpty()) { + continue; + } + assertThat(SecurityUtils.replaceEnvVars("abv${env." + k + "}xyz", settings), is("abv" + val + "xyz")); + assertThat(SecurityUtils.replaceEnvVars("abv${" + k + "}xyz", settings), is("abv${" + k + "}xyz")); + assertThat(SecurityUtils.replaceEnvVars("abv${env." + k + ":-k182765ggh}xyz", settings), is("abv" + val + "xyz")); + assertThat( + SecurityUtils.replaceEnvVars("abv${env." + k + "}xyzabv${env." + k + "}xyz", settings), + is("abv" + val + "xyzabv" + val + "xyz") + ); + assertThat(SecurityUtils.replaceEnvVars("abv${env." + k + ":-k182765ggh}xyz", settings), is("abv" + val + "xyz")); + assertTrue(passwordHasherPBKDF2.check(val.toCharArray(), SecurityUtils.replaceEnvVars("${envbc." + k + "}", settings))); + checked = true; + } + + assertTrue(checked); + } + @Test public void testNoEnvReplace() { Settings settings = Settings.builder().put(ConfigConstants.SECURITY_DISABLE_ENVVAR_REPLACEMENT, true).build();