From e6d74f638b548b339e519b2831f65dd8593bb319 Mon Sep 17 00:00:00 2001 From: Darshit Chanpura <35282393+DarshitChanpura@users.noreply.github.com> Date: Thu, 14 Dec 2023 17:45:54 -0500 Subject: [PATCH] Removes support for txt file for specifying initial admin password and prevents password from being printed in the logs (#3850) Signed-off-by: Darshit Chanpura Signed-off-by: Prabhas Kurapati --- .../action.yml | 11 ++- .../security/support/ConfigConstants.java | 3 +- .../security/tools/democonfig/Installer.java | 4 +- .../SecuritySettingsConfigurer.java | 76 ++++++++----------- .../tools/democonfig/InstallerTests.java | 8 +- .../SecuritySettingsConfigurerTests.java | 35 ++------- .../util/NoExitSecurityManager.java | 5 +- 7 files changed, 52 insertions(+), 90 deletions(-) diff --git a/.github/actions/start-opensearch-with-one-plugin/action.yml b/.github/actions/start-opensearch-with-one-plugin/action.yml index 8513e64fdf..e8e0f4eb77 100644 --- a/.github/actions/start-opensearch-with-one-plugin/action.yml +++ b/.github/actions/start-opensearch-with-one-plugin/action.yml @@ -71,23 +71,22 @@ runs: 'y' | .\opensearch-${{ inputs.opensearch-version }}-SNAPSHOT\bin\opensearch-plugin.bat install file:$(pwd)\${{ inputs.plugin-name }}.zip shell: pwsh - - name: Write password to opensearch_initial_admin_password txt - run: - echo ${{ inputs.admin-password }} >> ./opensearch-${{ env.OPENSEARCH_VERSION }}-SNAPSHOT/config/opensearch_initial_admin_password.txt - shell: bash - # Run any configuration scripts - name: Run Setup Script for Linux if: ${{ runner.os == 'Linux' && inputs.setup-script-name != '' }} run: | echo "running linux setup" + export OPENSEARCH_INITIAL_ADMIN_PASSWORD=${{ inputs.admin-password }} chmod +x ./${{ inputs.setup-script-name }}.sh ./${{ inputs.setup-script-name }}.sh shell: bash - name: Run Setup Script for Windows if: ${{ runner.os == 'Windows' && inputs.setup-script-name != '' }} - run: .\${{ inputs.setup-script-name }}.bat + run: | + echo "running windows setup" + $env:OPENSEARCH_INITIAL_ADMIN_PASSWORD="${{ inputs.admin-password }}" + .\${{ inputs.setup-script-name }}.bat shell: pwsh # Run OpenSearch diff --git a/src/main/java/org/opensearch/security/support/ConfigConstants.java b/src/main/java/org/opensearch/security/support/ConfigConstants.java index f106466984..f10dedade3 100644 --- a/src/main/java/org/opensearch/security/support/ConfigConstants.java +++ b/src/main/java/org/opensearch/security/support/ConfigConstants.java @@ -334,9 +334,8 @@ public enum RolesMappingResolution { public static final boolean EXTENSIONS_BWC_PLUGIN_MODE_DEFAULT = false; // CS-ENFORCE-SINGLE - // Variables for initial admin password support + // Variable for initial admin password support public static final String OPENSEARCH_INITIAL_ADMIN_PASSWORD = "OPENSEARCH_INITIAL_ADMIN_PASSWORD"; - public static final String OPENSEARCH_INITIAL_ADMIN_PASSWORD_TXT = "opensearch_initial_admin_password.txt"; public static Set getSettingAsSet( final Settings settings, diff --git a/src/main/java/org/opensearch/security/tools/democonfig/Installer.java b/src/main/java/org/opensearch/security/tools/democonfig/Installer.java index 68bc4d7cbf..61acd7e4c9 100644 --- a/src/main/java/org/opensearch/security/tools/democonfig/Installer.java +++ b/src/main/java/org/opensearch/security/tools/democonfig/Installer.java @@ -428,9 +428,7 @@ void finishScriptExecution() { } System.out.println( - "### To access your secured cluster open https://: and log in with admin/" - + SecuritySettingsConfigurer.ADMIN_PASSWORD - + "." + "### To access your secured cluster open https://: and log in with admin/." ); System.out.println("### (Ignore the SSL certificate warning because we installed self-signed demo certificates)"); diff --git a/src/main/java/org/opensearch/security/tools/democonfig/SecuritySettingsConfigurer.java b/src/main/java/org/opensearch/security/tools/democonfig/SecuritySettingsConfigurer.java index 28da25c592..b3644e6c4d 100644 --- a/src/main/java/org/opensearch/security/tools/democonfig/SecuritySettingsConfigurer.java +++ b/src/main/java/org/opensearch/security/tools/democonfig/SecuritySettingsConfigurer.java @@ -28,6 +28,7 @@ import com.fasterxml.jackson.databind.JsonNode; import org.opensearch.common.settings.Settings; +import org.opensearch.core.common.Strings; import org.opensearch.security.DefaultObjectMapper; import org.opensearch.security.dlic.rest.validation.PasswordValidator; import org.opensearch.security.dlic.rest.validation.RequestContentValidator; @@ -91,12 +92,34 @@ public void configureSecuritySettings() { writeSecurityConfigToOpenSearchYML(); } + /** + * Checks if security plugin is already configured. If so, the script execution will exit. + */ + void checkIfSecurityPluginIsAlreadyConfigured() { + // Check if the configuration file contains the 'plugins.security' string + if (installer.OPENSEARCH_CONF_FILE != null && new File(installer.OPENSEARCH_CONF_FILE).exists()) { + try (BufferedReader br = new BufferedReader(new FileReader(installer.OPENSEARCH_CONF_FILE, StandardCharsets.UTF_8))) { + String line; + while ((line = br.readLine()) != null) { + if (line.toLowerCase().contains("plugins.security")) { + System.out.println(installer.OPENSEARCH_CONF_FILE + " seems to be already configured for Security. Quit."); + System.exit(installer.skip_updates); + } + } + } catch (IOException e) { + System.err.println("Error reading configuration file."); + System.exit(-1); + } + } else { + System.err.println("OpenSearch configuration file does not exist. Quit."); + System.exit(-1); + } + } + /** * Replaces the admin password in internal_users.yml with the custom or generated password */ void updateAdminPassword() { - String initialAdminPassword = System.getenv().get(ConfigConstants.OPENSEARCH_INITIAL_ADMIN_PASSWORD); - String ADMIN_PASSWORD_FILE_PATH = installer.OPENSEARCH_CONF_DIR + ConfigConstants.OPENSEARCH_INITIAL_ADMIN_PASSWORD_TXT; String INTERNAL_USERS_FILE_PATH = installer.OPENSEARCH_CONF_DIR + "opensearch-security" + File.separator + "internal_users.yml"; boolean shouldValidatePassword = installer.environment.equals(ExecutionEnvironment.DEMO); try { @@ -107,21 +130,10 @@ void updateAdminPassword() { .build() ); - // Read custom password - if (initialAdminPassword != null && !initialAdminPassword.isEmpty()) { + // Read custom password from environment variable + String initialAdminPassword = System.getenv().get(ConfigConstants.OPENSEARCH_INITIAL_ADMIN_PASSWORD); + if (!Strings.isNullOrEmpty(initialAdminPassword)) { ADMIN_PASSWORD = initialAdminPassword; - } else { - File adminPasswordFile = new File(ADMIN_PASSWORD_FILE_PATH); - if (adminPasswordFile.exists() && adminPasswordFile.length() > 0) { - try (BufferedReader br = new BufferedReader(new FileReader(ADMIN_PASSWORD_FILE_PATH, StandardCharsets.UTF_8))) { - ADMIN_PASSWORD = br.readLine(); - } catch (IOException e) { - System.out.println( - "Error reading admin password from " + ConfigConstants.OPENSEARCH_INITIAL_ADMIN_PASSWORD_TXT + "." - ); - System.exit(-1); - } - } } // If script execution environment is set to demo, validate custom password, else if set to test, skip validation @@ -133,15 +145,13 @@ void updateAdminPassword() { } // if ADMIN_PASSWORD is still an empty string, it implies no custom password was provided. We exit the setup. - if (ADMIN_PASSWORD.isEmpty()) { + if (Strings.isNullOrEmpty(ADMIN_PASSWORD)) { System.out.println("No custom admin password found. Please provide a password."); System.exit(-1); } - // print the password to the logs - System.out.println("\t***************************************************"); - System.out.println("\t\tADMIN PASSWORD SET TO: " + ADMIN_PASSWORD); - System.out.println("\t***************************************************"); + // Print an update to the logs + System.out.println("Admin password set successfully."); writePasswordToInternalUsersFile(ADMIN_PASSWORD, INTERNAL_USERS_FILE_PATH); @@ -188,30 +198,6 @@ void writePasswordToInternalUsersFile(String adminPassword, String internalUsers Files.move(tempFilePath, internalUsersPath, java.nio.file.StandardCopyOption.REPLACE_EXISTING); } - /** - * Checks if security plugin is already configured. If so, the script execution will not continue. - */ - void checkIfSecurityPluginIsAlreadyConfigured() { - // Check if the configuration file contains the 'plugins.security' string - if (installer.OPENSEARCH_CONF_FILE != null && new File(installer.OPENSEARCH_CONF_FILE).exists()) { - try (BufferedReader br = new BufferedReader(new FileReader(installer.OPENSEARCH_CONF_FILE, StandardCharsets.UTF_8))) { - String line; - while ((line = br.readLine()) != null) { - if (line.toLowerCase().contains("plugins.security")) { - System.out.println(installer.OPENSEARCH_CONF_FILE + " seems to be already configured for Security. Quit."); - System.exit(installer.skip_updates); - } - } - } catch (IOException e) { - System.err.println("Error reading configuration file."); - System.exit(-1); - } - } else { - System.err.println("OpenSearch configuration file does not exist. Quit."); - System.exit(-1); - } - } - /** * Update opensearch.yml with security configuration information */ diff --git a/src/test/java/org/opensearch/security/tools/democonfig/InstallerTests.java b/src/test/java/org/opensearch/security/tools/democonfig/InstallerTests.java index 4e64fdea72..06c6edf734 100644 --- a/src/test/java/org/opensearch/security/tools/democonfig/InstallerTests.java +++ b/src/test/java/org/opensearch/security/tools/democonfig/InstallerTests.java @@ -394,9 +394,7 @@ public void testFinishScriptExecution() { + System.lineSeparator() + "### After that you can also use the Security Plugin ConfigurationGUI" + System.lineSeparator() - + "### To access your secured cluster open https://: and log in with admin/" - + SecuritySettingsConfigurer.ADMIN_PASSWORD - + "." + + "### To access your secured cluster open https://: and log in with admin/." + System.lineSeparator() + "### (Ignore the SSL certificate warning because we installed self-signed demo certificates)" + System.lineSeparator(); @@ -454,9 +452,7 @@ public void testFinishScriptExecution_withInitSecurityEnabled() { + System.lineSeparator() + "### To use the Security Plugin ConfigurationGUI" + System.lineSeparator() - + "### To access your secured cluster open https://: and log in with admin/" - + SecuritySettingsConfigurer.ADMIN_PASSWORD - + "." + + "### To access your secured cluster open https://: and log in with admin/." + System.lineSeparator() + "### (Ignore the SSL certificate warning because we installed self-signed demo certificates)" + System.lineSeparator(); diff --git a/src/test/java/org/opensearch/security/tools/democonfig/SecuritySettingsConfigurerTests.java b/src/test/java/org/opensearch/security/tools/democonfig/SecuritySettingsConfigurerTests.java index cb36ba0d6c..948a66996c 100644 --- a/src/test/java/org/opensearch/security/tools/democonfig/SecuritySettingsConfigurerTests.java +++ b/src/test/java/org/opensearch/security/tools/democonfig/SecuritySettingsConfigurerTests.java @@ -13,11 +13,9 @@ // CS-SUPPRESS-SINGLE: RegexpSingleline extension key-word is used in file ext variable import java.io.BufferedReader; -import java.io.BufferedWriter; import java.io.ByteArrayOutputStream; import java.io.File; import java.io.FileReader; -import java.io.FileWriter; import java.io.IOException; import java.io.InputStream; import java.io.PrintStream; @@ -45,7 +43,6 @@ import static org.opensearch.security.tools.democonfig.util.DemoConfigHelperUtil.createDirectory; import static org.opensearch.security.tools.democonfig.util.DemoConfigHelperUtil.createFile; import static org.opensearch.security.tools.democonfig.util.DemoConfigHelperUtil.deleteDirectoryRecursive; -import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; @RunWith(com.carrotsearch.randomizedtesting.RandomizedRunner.class) @@ -79,42 +76,25 @@ public void tearDown() throws NoSuchFieldException, IllegalAccessException { System.setErr(originalErr); System.setIn(originalIn); deleteDirectoryRecursive(installer.OPENSEARCH_CONF_DIR); - unsetEnv(adminPasswordKey); + unsetEnvVariables(); Installer.resetInstance(); } @Test public void testUpdateAdminPasswordWithCustomPassword() throws NoSuchFieldException, IllegalAccessException { - String customPassword = "myStrongPassword123"; // generateStrongPassword(); + String customPassword = "myStrongPassword123"; setEnv(adminPasswordKey, customPassword); securitySettingsConfigurer.updateAdminPassword(); assertThat(customPassword, is(equalTo(SecuritySettingsConfigurer.ADMIN_PASSWORD))); - verifyStdOutContainsString("ADMIN PASSWORD SET TO: " + customPassword); - } - - @Test - public void testUpdateAdminPasswordWithFilePassword() throws IOException { - String customPassword = "myStrongPassword123"; - String initialAdminPasswordTxt = installer.OPENSEARCH_CONF_DIR + adminPasswordKey + ".txt"; - createFile(initialAdminPasswordTxt); - - try (BufferedWriter writer = new BufferedWriter(new FileWriter(initialAdminPasswordTxt, StandardCharsets.UTF_8))) { - writer.write(customPassword); - } catch (IOException e) { - throw new IOException("Unable to update the internal users file with the hashed password."); - } - - securitySettingsConfigurer.updateAdminPassword(); - - assertEquals(customPassword, SecuritySettingsConfigurer.ADMIN_PASSWORD); - verifyStdOutContainsString("ADMIN PASSWORD SET TO: " + customPassword); + verifyStdOutContainsString("Admin password set successfully."); } @Test public void testUpdateAdminPassword_noPasswordSupplied() { + SecuritySettingsConfigurer.ADMIN_PASSWORD = ""; // to ensure 0 flaky-ness try { System.setSecurityManager(new NoExitSecurityManager()); securitySettingsConfigurer.updateAdminPassword(); @@ -150,7 +130,8 @@ public void testUpdateAdminPasswordWithWeakPassword_skipPasswordValidation() thr securitySettingsConfigurer.updateAdminPassword(); assertThat("weakpassword", is(equalTo(SecuritySettingsConfigurer.ADMIN_PASSWORD))); - verifyStdOutContainsString("ADMIN PASSWORD SET TO: weakpassword"); + + verifyStdOutContainsString("Admin password set successfully."); } @Test @@ -298,7 +279,7 @@ public static void setEnv(String key, String value) throws NoSuchFieldException, } @SuppressWarnings("unchecked") - public static void unsetEnv(String key) throws NoSuchFieldException, IllegalAccessException { + public static void unsetEnvVariables() throws NoSuchFieldException, IllegalAccessException { Class[] classes = Collections.class.getDeclaredClasses(); Map env = System.getenv(); for (Class cl : classes) { @@ -307,7 +288,7 @@ public static void unsetEnv(String key) throws NoSuchFieldException, IllegalAcce field.setAccessible(true); Object obj = field.get(env); Map map = (Map) obj; - map.remove(key); + map.clear(); } } } diff --git a/src/test/java/org/opensearch/security/tools/democonfig/util/NoExitSecurityManager.java b/src/test/java/org/opensearch/security/tools/democonfig/util/NoExitSecurityManager.java index 16bb61f169..0602812f5d 100644 --- a/src/test/java/org/opensearch/security/tools/democonfig/util/NoExitSecurityManager.java +++ b/src/test/java/org/opensearch/security/tools/democonfig/util/NoExitSecurityManager.java @@ -11,10 +11,13 @@ package org.opensearch.security.tools.democonfig.util; +/** + * Helper class to allow capturing and testing exit codes and block test execution from exiting mid-way + */ public class NoExitSecurityManager extends SecurityManager { @Override public void checkPermission(java.security.Permission perm) { - // Allow everything except System.exit code 0 &b -1 + // Allow everything except System.exit code 0 & -1 if (perm instanceof java.lang.RuntimePermission && ("exitVM.0".equals(perm.getName()) || "exitVM.-1".equals(perm.getName()))) { StringBuilder sb = new StringBuilder(); sb.append("System.exit(");