Skip to content

Commit

Permalink
Fix env password hashing for PBKDF2
Browse files Browse the repository at this point in the history
Signed-off-by: Terry Quigley <terry.quigley@sas.com>
  • Loading branch information
terryquigleysas committed Oct 3, 2024
1 parent 39adb47 commit a859cb2
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 10 deletions.
20 changes: 10 additions & 10 deletions src/main/java/org/opensearch/security/support/SecurityUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand All @@ -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));
}
Expand All @@ -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,
Expand All @@ -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;
}
}
}
38 changes: 38 additions & 0 deletions src/test/java/org/opensearch/security/UtilTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -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<String, String> 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();
Expand Down

0 comments on commit a859cb2

Please sign in to comment.