diff --git a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/BaseKeyStoreCommand.java b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/BaseKeyStoreCommand.java index 75eec8d985b62..493d455e42f6e 100644 --- a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/BaseKeyStoreCommand.java +++ b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/BaseKeyStoreCommand.java @@ -21,16 +21,15 @@ import joptsimple.OptionSet; import joptsimple.OptionSpec; -import org.elasticsearch.cli.EnvironmentAwareCommand; import org.elasticsearch.cli.ExitCodes; +import org.elasticsearch.cli.KeyStoreAwareCommand; import org.elasticsearch.cli.Terminal; import org.elasticsearch.cli.UserException; import org.elasticsearch.env.Environment; import java.nio.file.Path; -import java.util.Arrays; -public abstract class BaseKeyStoreCommand extends EnvironmentAwareCommand { +public abstract class BaseKeyStoreCommand extends KeyStoreAwareCommand { private KeyStoreWrapper keyStore; private SecureString keyStorePassword; @@ -82,30 +81,6 @@ protected SecureString getKeyStorePassword() { return keyStorePassword; } - /** - * Reads the keystore password from the {@link Terminal}, prompting for verification where applicable and returns it as a - * {@link SecureString}. - * - * @param terminal the terminal to use for user inputs - * @param withVerification whether the user should be prompted for password verification - * @return a SecureString with the password the user entered - * @throws UserException If the user is prompted for verification and enters a different password - */ - static SecureString readPassword(Terminal terminal, boolean withVerification) throws UserException { - final char[] passwordArray; - if (withVerification) { - passwordArray = terminal.readSecret("Enter new password for the elasticsearch keystore (empty for no password): "); - char[] passwordVerification = terminal.readSecret("Enter same password again: "); - if (Arrays.equals(passwordArray, passwordVerification) == false) { - throw new UserException(ExitCodes.DATA_ERROR, "Passwords are not equal, exiting."); - } - Arrays.fill(passwordVerification, '\u0000'); - } else { - passwordArray = terminal.readSecret("Enter password for the elasticsearch keystore : "); - } - return new SecureString(passwordArray); - } - /** * This is called after the keystore password has been read from the stdin and the keystore is decrypted and * loaded. The keystore and keystore passwords are available to classes extending {@link BaseKeyStoreCommand} diff --git a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/CreateKeyStoreCommand.java b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/CreateKeyStoreCommand.java index 379af6f3a47f3..c8833650581ea 100644 --- a/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/CreateKeyStoreCommand.java +++ b/distribution/tools/keystore-cli/src/main/java/org/elasticsearch/common/settings/CreateKeyStoreCommand.java @@ -25,8 +25,8 @@ import joptsimple.OptionSet; import joptsimple.OptionSpec; -import org.elasticsearch.cli.EnvironmentAwareCommand; import org.elasticsearch.cli.ExitCodes; +import org.elasticsearch.cli.KeyStoreAwareCommand; import org.elasticsearch.cli.Terminal; import org.elasticsearch.cli.UserException; import org.elasticsearch.env.Environment; @@ -34,7 +34,7 @@ /** * A sub-command for the keystore cli to create a new keystore. */ -class CreateKeyStoreCommand extends EnvironmentAwareCommand { +class CreateKeyStoreCommand extends KeyStoreAwareCommand { private final OptionSpec passwordOption; @@ -46,7 +46,7 @@ class CreateKeyStoreCommand extends EnvironmentAwareCommand { @Override protected void execute(Terminal terminal, OptionSet options, Environment env) throws Exception { try (SecureString password = options.has(passwordOption) ? - BaseKeyStoreCommand.readPassword(terminal, true) : new SecureString(new char[0])) { + readPassword(terminal, true) : new SecureString(new char[0])) { Path keystoreFile = KeyStoreWrapper.keystorePath(env.configFile()); if (Files.exists(keystoreFile)) { if (terminal.promptYesNo("An elasticsearch keystore already exists. Overwrite?", false) == false) { diff --git a/server/src/main/java/org/elasticsearch/cli/KeyStoreAwareCommand.java b/server/src/main/java/org/elasticsearch/cli/KeyStoreAwareCommand.java new file mode 100644 index 0000000000000..cd9740bcd112e --- /dev/null +++ b/server/src/main/java/org/elasticsearch/cli/KeyStoreAwareCommand.java @@ -0,0 +1,81 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.cli; + +import joptsimple.OptionSet; +import org.elasticsearch.common.settings.KeyStoreWrapper; +import org.elasticsearch.common.settings.SecureString; +import org.elasticsearch.env.Environment; + +import javax.crypto.AEADBadTagException; +import java.io.IOException; +import java.security.GeneralSecurityException; +import java.util.Arrays; + +/** + * An {@link org.elasticsearch.cli.EnvironmentAwareCommand} that needs to access the elasticsearch keystore, possibly + * decrypting it if it is password protected. + */ +public abstract class KeyStoreAwareCommand extends EnvironmentAwareCommand { + public KeyStoreAwareCommand(String description) { + super(description); + } + + /** + * Reads the keystore password from the {@link Terminal}, prompting for verification where applicable and returns it as a + * {@link SecureString}. + * + * @param terminal the terminal to use for user inputs + * @param withVerification whether the user should be prompted for password verification + * @return a SecureString with the password the user entered + * @throws UserException If the user is prompted for verification and enters a different password + */ + protected static SecureString readPassword(Terminal terminal, boolean withVerification) throws UserException { + final char[] passwordArray; + if (withVerification) { + passwordArray = terminal.readSecret("Enter new password for the elasticsearch keystore (empty for no password): "); + char[] passwordVerification = terminal.readSecret("Enter same password again: "); + if (Arrays.equals(passwordArray, passwordVerification) == false) { + throw new UserException(ExitCodes.DATA_ERROR, "Passwords are not equal, exiting."); + } + Arrays.fill(passwordVerification, '\u0000'); + } else { + passwordArray = terminal.readSecret("Enter password for the elasticsearch keystore : "); + } + return new SecureString(passwordArray); + } + + /** + * Decrypt the {@code keyStore}, prompting the user to enter the password in the {@link Terminal} if it is password protected + */ + protected static void decryptKeyStore(KeyStoreWrapper keyStore, Terminal terminal) + throws UserException, GeneralSecurityException, IOException { + try (SecureString keystorePassword = keyStore.hasPassword() ? + readPassword(terminal, false) : new SecureString(new char[0])) { + keyStore.decrypt(keystorePassword.getChars()); + } catch (SecurityException e) { + if (e.getCause() instanceof AEADBadTagException) { + throw new UserException(ExitCodes.DATA_ERROR, "Wrong password for elasticsearch.keystore"); + } + } + } + + protected abstract void execute(Terminal terminal, OptionSet options, Environment env) throws Exception; +} diff --git a/test/framework/src/main/java/org/elasticsearch/cli/CommandTestCase.java b/test/framework/src/main/java/org/elasticsearch/cli/CommandTestCase.java index e9c6a2eec9c31..e8a518dffd721 100644 --- a/test/framework/src/main/java/org/elasticsearch/cli/CommandTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/cli/CommandTestCase.java @@ -30,9 +30,6 @@ public abstract class CommandTestCase extends ESTestCase { /** The terminal that execute uses. */ protected final MockTerminal terminal = new MockTerminal(); - /** The last command that was executed. */ - protected Command command; - @Before public void resetTerminal() { terminal.reset(); @@ -43,13 +40,20 @@ public void resetTerminal() { protected abstract Command newCommand(); /** - * Runs the command with the given args. + * Runs a command with the given args. * * Output can be found in {@link #terminal}. - * The command created can be found in {@link #command}. */ public String execute(String... args) throws Exception { - command = newCommand(); + return execute(newCommand(), args); + } + + /** + * Runs the specified command with the given args. + *

+ * Output can be found in {@link #terminal}. + */ + public String execute(Command command, String... args) throws Exception { command.mainWithoutErrorHandling(args, terminal); return terminal.getOutput(); } diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/esnative/tool/SetupPasswordTool.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/esnative/tool/SetupPasswordTool.java index 866f3722e6e76..a9150657646c4 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/esnative/tool/SetupPasswordTool.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/esnative/tool/SetupPasswordTool.java @@ -9,8 +9,8 @@ import joptsimple.OptionSet; import joptsimple.OptionSpec; import org.elasticsearch.ExceptionsHelper; -import org.elasticsearch.cli.EnvironmentAwareCommand; import org.elasticsearch.cli.ExitCodes; +import org.elasticsearch.cli.KeyStoreAwareCommand; import org.elasticsearch.cli.LoggingAwareMultiCommand; import org.elasticsearch.cli.Terminal; import org.elasticsearch.cli.Terminal.Verbosity; @@ -125,7 +125,7 @@ class AutoSetup extends SetupCommand { @Override protected void execute(Terminal terminal, OptionSet options, Environment env) throws Exception { terminal.println(Verbosity.VERBOSE, "Running with configuration path: " + env.configFile()); - setupOptions(options, env); + setupOptions(terminal, options, env); checkElasticKeystorePasswordValid(terminal, env); checkClusterHealth(terminal); @@ -171,7 +171,7 @@ class InteractiveSetup extends SetupCommand { @Override protected void execute(Terminal terminal, OptionSet options, Environment env) throws Exception { terminal.println(Verbosity.VERBOSE, "Running with configuration path: " + env.configFile()); - setupOptions(options, env); + setupOptions(terminal, options, env); checkElasticKeystorePasswordValid(terminal, env); checkClusterHealth(terminal); @@ -221,7 +221,7 @@ private void changedPasswordCallback(Terminal terminal, String user, SecureStrin * An abstract class that provides functionality common to both the auto and * interactive setup modes. */ - private abstract class SetupCommand extends EnvironmentAwareCommand { + private abstract class SetupCommand extends KeyStoreAwareCommand { boolean shouldPrompt; @@ -248,10 +248,9 @@ public void close() { } } - void setupOptions(OptionSet options, Environment env) throws Exception { + void setupOptions(Terminal terminal, OptionSet options, Environment env) throws Exception { keyStoreWrapper = keyStoreFunction.apply(env); - // TODO: We currently do not support keystore passwords - keyStoreWrapper.decrypt(new char[0]); + decryptKeyStore(keyStoreWrapper, terminal); Settings.Builder settingsBuilder = Settings.builder(); settingsBuilder.put(env.settings(), true); diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlMetadataCommand.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlMetadataCommand.java index a60b2204095a5..4ee66406ac1f8 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlMetadataCommand.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlMetadataCommand.java @@ -32,8 +32,8 @@ import org.apache.logging.log4j.Level; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import org.elasticsearch.cli.EnvironmentAwareCommand; import org.elasticsearch.cli.ExitCodes; +import org.elasticsearch.cli.KeyStoreAwareCommand; import org.elasticsearch.cli.SuppressForbidden; import org.elasticsearch.cli.Terminal; import org.elasticsearch.cli.UserException; @@ -68,7 +68,7 @@ /** * CLI tool to generate SAML Metadata for a Service Provider (realm) */ -public class SamlMetadataCommand extends EnvironmentAwareCommand { +public class SamlMetadataCommand extends KeyStoreAwareCommand { static final String METADATA_SCHEMA = "saml-schema-metadata-2.0.xsd"; @@ -414,13 +414,12 @@ private SortedSet sorted(Set strings) { /** * @TODO REALM-SETTINGS[TIM] This can be redone a lot now the realm settings are keyed by type */ - private RealmConfig findRealm(Terminal terminal, OptionSet options, Environment env) throws UserException, IOException, Exception { + private RealmConfig findRealm(Terminal terminal, OptionSet options, Environment env) throws Exception { keyStoreWrapper = keyStoreFunction.apply(env); final Settings settings; if (keyStoreWrapper != null) { - // TODO: We currently do not support keystore passwords - keyStoreWrapper.decrypt(new char[0]); + decryptKeyStore(keyStoreWrapper, terminal); final Settings.Builder settingsBuilder = Settings.builder(); settingsBuilder.put(env.settings(), true); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/tool/SetupPasswordToolTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/tool/SetupPasswordToolTests.java index 12080077ddfab..92495d8622d90 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/tool/SetupPasswordToolTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/esnative/tool/SetupPasswordToolTests.java @@ -35,7 +35,6 @@ import org.elasticsearch.xpack.core.security.user.ElasticUser; import org.elasticsearch.xpack.security.authc.esnative.ReservedRealm; import org.elasticsearch.xpack.security.authc.esnative.tool.HttpResponse.HttpResponseBuilder; -import org.hamcrest.CoreMatchers; import org.hamcrest.Matchers; import org.junit.Before; import org.junit.Rule; @@ -44,6 +43,7 @@ import org.mockito.InOrder; import org.mockito.Mockito; +import javax.crypto.AEADBadTagException; import javax.net.ssl.SSLException; import java.io.IOException; import java.net.HttpURLConnection; @@ -59,9 +59,11 @@ import java.util.Map; import java.util.Set; +import static org.hamcrest.CoreMatchers.containsString; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; @@ -72,8 +74,11 @@ public class SetupPasswordToolTests extends CommandTestCase { private final String pathHomeParameter = "-Epath.home=" + createTempDir(); private SecureString bootstrapPassword; private CommandLineHttpClient httpClient; - private KeyStoreWrapper keyStore; private List usersInSetOrder; + private KeyStoreWrapper passwordProtectedKeystore; + private KeyStoreWrapper keyStore; + private KeyStoreWrapper usedKeyStore; + @Rule public ExpectedException thrown = ExpectedException.none(); @@ -83,19 +88,15 @@ public void setSecretsAndKeyStore() throws Exception { boolean useFallback = randomBoolean(); bootstrapPassword = useFallback ? new SecureString("0xCAFEBABE".toCharArray()) : new SecureString("bootstrap-password".toCharArray()); - this.keyStore = mock(KeyStoreWrapper.class); - this.httpClient = mock(CommandLineHttpClient.class); - - when(keyStore.isLoaded()).thenReturn(true); - if (useFallback) { - when(keyStore.getSettingNames()).thenReturn(new HashSet<>(Arrays.asList(ReservedRealm.BOOTSTRAP_ELASTIC_PASSWORD.getKey(), - KeyStoreWrapper.SEED_SETTING.getKey()))); - when(keyStore.getString(ReservedRealm.BOOTSTRAP_ELASTIC_PASSWORD.getKey())).thenReturn(bootstrapPassword); - } else { - when(keyStore.getSettingNames()).thenReturn(Collections.singleton(KeyStoreWrapper.SEED_SETTING.getKey())); - when(keyStore.getString(KeyStoreWrapper.SEED_SETTING.getKey())).thenReturn(bootstrapPassword); + keyStore = mockKeystore(false, useFallback); + // create a password protected keystore eitherway, so that it can be used for SetupPasswordToolTests#testWrongKeystorePassword + passwordProtectedKeystore = mockKeystore(true, useFallback); + usedKeyStore = randomFrom(keyStore, passwordProtectedKeystore); + if (usedKeyStore.hasPassword()) { + terminal.addSecretInput("keystore-password"); } + this.httpClient = mock(CommandLineHttpClient.class); when(httpClient.getDefaultURL()).thenReturn("http://localhost:9200"); HttpResponse httpResponse = new HttpResponse(HttpURLConnection.HTTP_OK, new HashMap()); @@ -126,35 +127,29 @@ public void setSecretsAndKeyStore() throws Exception { } } + private KeyStoreWrapper mockKeystore(boolean isPasswordProtected, boolean useFallback) throws Exception { + KeyStoreWrapper keyStore = mock(KeyStoreWrapper.class); + when(keyStore.isLoaded()).thenReturn(true); + if (useFallback) { + when(keyStore.getSettingNames()).thenReturn(new HashSet<>(Arrays.asList(ReservedRealm.BOOTSTRAP_ELASTIC_PASSWORD.getKey(), + KeyStoreWrapper.SEED_SETTING.getKey()))); + when(keyStore.getString(ReservedRealm.BOOTSTRAP_ELASTIC_PASSWORD.getKey())).thenReturn(bootstrapPassword); + } else { + when(keyStore.getSettingNames()).thenReturn(Collections.singleton(KeyStoreWrapper.SEED_SETTING.getKey())); + when(keyStore.getString(KeyStoreWrapper.SEED_SETTING.getKey())).thenReturn(bootstrapPassword); + } + if (isPasswordProtected) { + when(keyStore.hasPassword()).thenReturn(true); + doNothing().when(keyStore).decrypt("keystore-password".toCharArray()); + doThrow(new SecurityException("Provided keystore password was incorrect", new AEADBadTagException())) + .when(keyStore).decrypt("wrong-password".toCharArray()); + } + return keyStore; + } + @Override protected Command newCommand() { - return new SetupPasswordTool((e, s) -> httpClient, (e) -> keyStore) { - - @Override - protected AutoSetup newAutoSetup() { - return new AutoSetup() { - @Override - protected Environment createEnv(Map settings) throws UserException { - Settings.Builder builder = Settings.builder(); - settings.forEach((k, v) -> builder.put(k, v)); - return TestEnvironment.newEnvironment(builder.build()); - } - }; - } - - @Override - protected InteractiveSetup newInteractiveSetup() { - return new InteractiveSetup() { - @Override - protected Environment createEnv(Map settings) throws UserException { - Settings.Builder builder = Settings.builder(); - settings.forEach((k, v) -> builder.put(k, v)); - return TestEnvironment.newEnvironment(builder.build()); - } - }; - } - - }; + return getSetupPasswordCommandWithKeyStore(usedKeyStore); } public void testAutoSetup() throws Exception { @@ -165,8 +160,12 @@ public void testAutoSetup() throws Exception { terminal.addTextInput("Y"); execute("auto", pathHomeParameter); } - - verify(keyStore).decrypt(new char[0]); + if (usedKeyStore.hasPassword()) { + // SecureString is already closed (zero-filled) and keystore-password is 17 char long + verify(usedKeyStore).decrypt(new char[17]); + } else { + verify(usedKeyStore).decrypt(new char[0]); + } InOrder inOrder = Mockito.inOrder(httpClient); @@ -401,7 +400,7 @@ public void testInteractiveSetup() throws Exception { ArgumentCaptor> passwordCaptor = ArgumentCaptor.forClass((Class) CheckedSupplier.class); inOrder.verify(httpClient).execute(eq("PUT"), eq(urlWithRoute), eq(ElasticUser.NAME), eq(bootstrapPassword), passwordCaptor.capture(), any(CheckedFunction.class)); - assertThat(passwordCaptor.getValue().get(), CoreMatchers.containsString(user + "-password")); + assertThat(passwordCaptor.getValue().get(), containsString(user + "-password")); } } @@ -409,6 +408,9 @@ public void testInteractivePasswordsFatFingers() throws Exception { URL url = new URL(httpClient.getDefaultURL()); terminal.reset(); + if (usedKeyStore.hasPassword()) { + terminal.addSecretInput("keystore-password"); + } terminal.addTextInput("Y"); for (String user : SetupPasswordTool.USERS) { // fail in strength and match @@ -439,10 +441,25 @@ public void testInteractivePasswordsFatFingers() throws Exception { ArgumentCaptor> passwordCaptor = ArgumentCaptor.forClass((Class) CheckedSupplier.class); inOrder.verify(httpClient).execute(eq("PUT"), eq(urlWithRoute), eq(ElasticUser.NAME), eq(bootstrapPassword), passwordCaptor.capture(), any(CheckedFunction.class)); - assertThat(passwordCaptor.getValue().get(), CoreMatchers.containsString(user + "-password")); + assertThat(passwordCaptor.getValue().get(), containsString(user + "-password")); } } + public void testWrongKeystorePassword() throws Exception { + Command commandWithPasswordProtectedKeystore = getSetupPasswordCommandWithKeyStore(passwordProtectedKeystore); + terminal.reset(); + terminal.addSecretInput("wrong-password"); + final UserException e = expectThrows(UserException.class, () -> { + if (randomBoolean()) { + execute(commandWithPasswordProtectedKeystore, "auto", pathHomeParameter, "-b", "true"); + } else { + terminal.addTextInput("Y"); + execute(commandWithPasswordProtectedKeystore, "auto", pathHomeParameter); + } + }); + assertThat(e.getMessage(), containsString("Wrong password for elasticsearch.keystore")); + } + private String parsePassword(String value) throws IOException { try (XContentParser parser = JsonXContent.jsonXContent .createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, value)) { @@ -481,4 +498,35 @@ private HttpResponse createHttpResponse(final int httpStatus, final String respo builder.withResponseBody(responseJson); return builder.build(); } + + private Command getSetupPasswordCommandWithKeyStore(KeyStoreWrapper keyStore) { + return new SetupPasswordTool((e, s) -> httpClient, (e) -> keyStore) { + + @Override + protected AutoSetup newAutoSetup() { + return new AutoSetup() { + @Override + protected Environment createEnv(Map settings) throws UserException { + Settings.Builder builder = Settings.builder(); + settings.forEach((k, v) -> builder.put(k, v)); + return TestEnvironment.newEnvironment(builder.build()); + } + }; + } + + @Override + protected InteractiveSetup newInteractiveSetup() { + return new InteractiveSetup() { + @Override + protected Environment createEnv(Map settings) throws UserException { + Settings.Builder builder = Settings.builder(); + settings.forEach((k, v) -> builder.put(k, v)); + return TestEnvironment.newEnvironment(builder.build()); + } + }; + } + + }; + + } } diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlMetadataCommandTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlMetadataCommandTests.java index 320eb6e14a6c1..b6eb39ba2c4fa 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlMetadataCommandTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlMetadataCommandTests.java @@ -18,6 +18,7 @@ import org.elasticsearch.xpack.core.security.authc.RealmSettings; import org.elasticsearch.xpack.core.ssl.CertParsingUtils; import org.elasticsearch.xpack.core.ssl.PemUtils; +import org.hamcrest.CoreMatchers; import org.junit.Before; import org.opensaml.saml.common.xml.SAMLConstants; import org.opensaml.saml.saml2.metadata.EntityDescriptor; @@ -33,6 +34,7 @@ import org.opensaml.xmlsec.signature.X509Data; import org.opensaml.xmlsec.signature.support.SignatureValidator; +import javax.crypto.AEADBadTagException; import java.io.OutputStream; import java.nio.file.Files; import java.nio.file.Path; @@ -54,25 +56,35 @@ import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.nullValue; import static org.hamcrest.Matchers.startsWith; +import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; public class SamlMetadataCommandTests extends SamlTestCase { private KeyStoreWrapper keyStore; + private KeyStoreWrapper passwordProtectedKeystore; @Before public void setup() throws Exception { SamlUtils.initialize(logger); this.keyStore = mock(KeyStoreWrapper.class); when(keyStore.isLoaded()).thenReturn(true); + this.passwordProtectedKeystore = mock(KeyStoreWrapper.class); + when(passwordProtectedKeystore.isLoaded()).thenReturn(true); + when(passwordProtectedKeystore.hasPassword()).thenReturn(true); + doNothing().when(passwordProtectedKeystore).decrypt("keystore-password".toCharArray()); + doThrow(new SecurityException("Provided keystore password was incorrect", new AEADBadTagException())) + .when(passwordProtectedKeystore).decrypt("wrong-password".toCharArray()); } public void testDefaultOptions() throws Exception { + final KeyStoreWrapper usedKeyStore = randomFrom(keyStore, passwordProtectedKeystore); final Path certPath = getDataPath("saml.crt"); final Path keyPath = getDataPath("saml.key"); - final SamlMetadataCommand command = new SamlMetadataCommand((e) -> randomFrom(keyStore, null)); + final SamlMetadataCommand command = new SamlMetadataCommand((e) -> usedKeyStore); final OptionSet options = command.getParser().parse(new String[0]); final boolean useSigningCredentials = randomBoolean(); @@ -93,6 +105,9 @@ public void testDefaultOptions() throws Exception { final MockTerminal terminal = new MockTerminal(); + if (usedKeyStore.hasPassword()) { + terminal.addSecretInput("keystore-password"); + } // What is the friendly name for "principal" attribute "urn:oid:0.9.2342.19200300.100.1.1" [default: principal] terminal.addTextInput(""); @@ -147,6 +162,7 @@ public void testDefaultOptions() throws Exception { } public void testFailIfMultipleRealmsExist() throws Exception { + final KeyStoreWrapper usedKeyStore = randomFrom(keyStore, passwordProtectedKeystore); final Settings settings = Settings.builder() .put("path.home", createTempDir()) .put(RealmSettings.PREFIX + "saml.saml_a.type", "saml") @@ -158,11 +174,10 @@ public void testFailIfMultipleRealmsExist() throws Exception { .build(); final Environment env = TestEnvironment.newEnvironment(settings); - final SamlMetadataCommand command = new SamlMetadataCommand((e) -> randomFrom(keyStore, null)); + final SamlMetadataCommand command = new SamlMetadataCommand((e) -> usedKeyStore); final OptionSet options = command.getParser().parse(new String[0]); - final MockTerminal terminal = new MockTerminal(); - + final MockTerminal terminal = getTerminalPossiblyWithPassword(usedKeyStore); final UserException userException = expectThrows(UserException.class, () -> command.buildEntityDescriptor(terminal, options, env)); assertThat(userException.getMessage(), containsString("multiple SAML realms")); assertThat(terminal.getOutput(), containsString("saml_a")); @@ -171,6 +186,7 @@ public void testFailIfMultipleRealmsExist() throws Exception { } public void testSpecifyRealmNameAsParameter() throws Exception { + final KeyStoreWrapper usedKeyStore = randomFrom(keyStore, passwordProtectedKeystore); final Settings settings = Settings.builder() .put("path.home", createTempDir()) .put(RealmSettings.PREFIX + "saml.saml_a.type", "saml") @@ -182,12 +198,12 @@ public void testSpecifyRealmNameAsParameter() throws Exception { .build(); final Environment env = TestEnvironment.newEnvironment(settings); - final SamlMetadataCommand command = new SamlMetadataCommand((e) -> keyStore); + final SamlMetadataCommand command = new SamlMetadataCommand((e) -> usedKeyStore); final OptionSet options = command.getParser().parse(new String[] { "-realm", "saml_b" }); - final MockTerminal terminal = new MockTerminal(); + final MockTerminal terminal = getTerminalPossiblyWithPassword(usedKeyStore); final EntityDescriptor descriptor = command.buildEntityDescriptor(terminal, options, env); assertThat(descriptor, notNullValue()); @@ -202,6 +218,7 @@ public void testSpecifyRealmNameAsParameter() throws Exception { } public void testHandleAttributes() throws Exception { + final KeyStoreWrapper usedKeyStore = randomFrom(keyStore, passwordProtectedKeystore); final Settings settings = Settings.builder() .put("path.home", createTempDir()) .put(RealmSettings.PREFIX + "saml.saml1.type", "saml") @@ -212,14 +229,13 @@ public void testHandleAttributes() throws Exception { .build(); final Environment env = TestEnvironment.newEnvironment(settings); - final SamlMetadataCommand command = new SamlMetadataCommand((e) -> randomFrom(keyStore, null)); + final SamlMetadataCommand command = new SamlMetadataCommand((e) -> usedKeyStore); final OptionSet options = command.getParser().parse(new String[] { "-attribute", "urn:oid:0.9.2342.19200300.100.1.3", "-attribute", "groups" }); - final MockTerminal terminal = new MockTerminal(); - + final MockTerminal terminal = getTerminalPossiblyWithPassword(usedKeyStore); // What is the friendly name for command line attribute "urn:oid:0.9.2342.19200300.100.1.3" [default: none] terminal.addTextInput("mail"); // What is the standard (urn) name for attribute "groups" (required) @@ -256,6 +272,7 @@ public void testHandleAttributes() throws Exception { } public void testHandleAttributesInBatchMode() throws Exception { + final KeyStoreWrapper usedKeyStore = randomFrom(keyStore, passwordProtectedKeystore); final Settings settings = Settings.builder() .put("path.home", createTempDir()) .put(RealmSettings.PREFIX + "saml.saml1.type", "saml") @@ -265,13 +282,13 @@ public void testHandleAttributesInBatchMode() throws Exception { .build(); final Environment env = TestEnvironment.newEnvironment(settings); - final SamlMetadataCommand command = new SamlMetadataCommand((e) -> randomFrom(keyStore, null)); + final SamlMetadataCommand command = new SamlMetadataCommand((e) -> usedKeyStore); final OptionSet options = command.getParser().parse(new String[] { "-attribute", "urn:oid:0.9.2342.19200300.100.1.3", "-batch" }); - final MockTerminal terminal = new MockTerminal(); + final MockTerminal terminal = getTerminalPossiblyWithPassword(usedKeyStore); final EntityDescriptor descriptor = command.buildEntityDescriptor(terminal, options, env); assertThat(descriptor, notNullValue()); @@ -294,10 +311,11 @@ public void testHandleAttributesInBatchMode() throws Exception { public void testSigningMetadataWithPfx() throws Exception { assumeFalse("Can't run in a FIPS JVM, PKCS12 keystores are not usable", inFipsJvm()); + final KeyStoreWrapper usedKeyStore = randomFrom(keyStore, passwordProtectedKeystore); final Path certPath = getDataPath("saml.crt"); final Path keyPath = getDataPath("saml.key"); final Path p12Path = getDataPath("saml.p12"); - final SamlMetadataCommand command = new SamlMetadataCommand((e) -> randomFrom(keyStore, null)); + final SamlMetadataCommand command = new SamlMetadataCommand((e) -> usedKeyStore); final OptionSet options = command.getParser().parse(new String[]{ "-signing-bundle", p12Path.toString() }); @@ -319,8 +337,7 @@ public void testSigningMetadataWithPfx() throws Exception { final Settings settings = settingsBuilder.build(); final Environment env = TestEnvironment.newEnvironment(settings); - final MockTerminal terminal = new MockTerminal(); - + final MockTerminal terminal = getTerminalPossiblyWithPassword(usedKeyStore); // What is the friendly name for "principal" attribute "urn:oid:0.9.2342.19200300.100.1.1" [default: principal] terminal.addTextInput(""); terminal.addSecretInput(""); @@ -354,10 +371,11 @@ public void testSigningMetadataWithPfx() throws Exception { public void testSigningMetadataWithPasswordProtectedPfx() throws Exception { assumeFalse("Can't run in a FIPS JVM, PKCS12 keystores are not usable", inFipsJvm()); + final KeyStoreWrapper usedKeyStore = randomFrom(keyStore, passwordProtectedKeystore); final Path certPath = getDataPath("saml.crt"); final Path keyPath = getDataPath("saml.key"); final Path p12Path = getDataPath("saml_with_password.p12"); - final SamlMetadataCommand command = new SamlMetadataCommand((e) -> randomFrom(keyStore, null)); + final SamlMetadataCommand command = new SamlMetadataCommand((e) -> usedKeyStore); final OptionSet options = command.getParser().parse(new String[]{ "-signing-bundle", p12Path.toString(), "-signing-key-password", "saml" @@ -379,8 +397,7 @@ public void testSigningMetadataWithPasswordProtectedPfx() throws Exception { final Settings settings = settingsBuilder.build(); final Environment env = TestEnvironment.newEnvironment(settings); - final MockTerminal terminal = new MockTerminal(); - + final MockTerminal terminal = getTerminalPossiblyWithPassword(usedKeyStore); final EntityDescriptor descriptor = command.buildEntityDescriptor(terminal, options, env); command.possiblySignDescriptor(terminal, options, descriptor, env); assertThat(descriptor, notNullValue()); @@ -390,10 +407,11 @@ public void testSigningMetadataWithPasswordProtectedPfx() throws Exception { } public void testErrorSigningMetadataWithWrongPassword() throws Exception { + final KeyStoreWrapper usedKeyStore = randomFrom(keyStore, passwordProtectedKeystore); final Path certPath = getDataPath("saml.crt"); final Path keyPath = getDataPath("saml.key"); final Path signingKeyPath = getDataPath("saml_with_password.key"); - final SamlMetadataCommand command = new SamlMetadataCommand((e) -> randomFrom(keyStore, null)); + final SamlMetadataCommand command = new SamlMetadataCommand((e) -> keyStore); final OptionSet options = command.getParser().parse(new String[]{ "-signing-cert", certPath.toString(), "-signing-key", signingKeyPath.toString(), @@ -417,8 +435,7 @@ public void testErrorSigningMetadataWithWrongPassword() throws Exception { final Settings settings = settingsBuilder.build(); final Environment env = TestEnvironment.newEnvironment(settings); - final MockTerminal terminal = new MockTerminal(); - + final MockTerminal terminal = getTerminalPossiblyWithPassword(usedKeyStore); final EntityDescriptor descriptor = command.buildEntityDescriptor(terminal, options, env); final UserException userException = expectThrows(UserException.class, () -> command.possiblySignDescriptor(terminal, options, descriptor, env)); @@ -427,11 +444,12 @@ public void testErrorSigningMetadataWithWrongPassword() throws Exception { } public void testSigningMetadataWithPem() throws Exception { + final KeyStoreWrapper usedKeyStore = randomFrom(keyStore, passwordProtectedKeystore); //Use this keypair for signing the metadata also final Path certPath = getDataPath("saml.crt"); final Path keyPath = getDataPath("saml.key"); - final SamlMetadataCommand command = new SamlMetadataCommand((e) -> randomFrom(keyStore, null)); + final SamlMetadataCommand command = new SamlMetadataCommand((e) -> keyStore); final OptionSet options = command.getParser().parse(new String[]{ "-signing-cert", certPath.toString(), "-signing-key", keyPath.toString() @@ -453,8 +471,7 @@ public void testSigningMetadataWithPem() throws Exception { final Settings settings = settingsBuilder.build(); final Environment env = TestEnvironment.newEnvironment(settings); - final MockTerminal terminal = new MockTerminal(); - + final MockTerminal terminal = getTerminalPossiblyWithPassword(usedKeyStore); final EntityDescriptor descriptor = command.buildEntityDescriptor(terminal, options, env); command.possiblySignDescriptor(terminal, options, descriptor, env); assertThat(descriptor, notNullValue()); @@ -464,13 +481,14 @@ public void testSigningMetadataWithPem() throws Exception { } public void testSigningMetadataWithPasswordProtectedPem() throws Exception { + final KeyStoreWrapper usedKeyStore = randomFrom(keyStore, passwordProtectedKeystore); //Use same keypair for signing the metadata final Path signingKeyPath = getDataPath("saml_with_password.key"); final Path certPath = getDataPath("saml.crt"); final Path keyPath = getDataPath("saml.key"); - final SamlMetadataCommand command = new SamlMetadataCommand((e) -> keyStore); + final SamlMetadataCommand command = new SamlMetadataCommand((e) -> usedKeyStore); final OptionSet options = command.getParser().parse(new String[]{ "-signing-cert", certPath.toString(), "-signing-key", signingKeyPath.toString(), @@ -494,8 +512,7 @@ public void testSigningMetadataWithPasswordProtectedPem() throws Exception { final Settings settings = settingsBuilder.build(); final Environment env = TestEnvironment.newEnvironment(settings); - final MockTerminal terminal = new MockTerminal(); - + final MockTerminal terminal = getTerminalPossiblyWithPassword(usedKeyStore); final EntityDescriptor descriptor = command.buildEntityDescriptor(terminal, options, env); command.possiblySignDescriptor(terminal, options, descriptor, env); assertThat(descriptor, notNullValue()); @@ -505,13 +522,14 @@ public void testSigningMetadataWithPasswordProtectedPem() throws Exception { } public void testSigningMetadataWithPasswordProtectedPemInTerminal() throws Exception { + final KeyStoreWrapper usedKeyStore = randomFrom(keyStore, passwordProtectedKeystore); //Use same keypair for signing the metadata final Path signingKeyPath = getDataPath("saml_with_password.key"); final Path certPath = getDataPath("saml.crt"); final Path keyPath = getDataPath("saml.key"); - final SamlMetadataCommand command = new SamlMetadataCommand((e) -> randomFrom(keyStore, null)); + final SamlMetadataCommand command = new SamlMetadataCommand((e) -> usedKeyStore); final OptionSet options = command.getParser().parse(new String[]{ "-signing-cert", certPath.toString(), "-signing-key", signingKeyPath.toString() @@ -534,8 +552,7 @@ public void testSigningMetadataWithPasswordProtectedPemInTerminal() throws Excep final Settings settings = settingsBuilder.build(); final Environment env = TestEnvironment.newEnvironment(settings); - final MockTerminal terminal = new MockTerminal(); - + final MockTerminal terminal = getTerminalPossiblyWithPassword(usedKeyStore); terminal.addSecretInput("saml"); final EntityDescriptor descriptor = command.buildEntityDescriptor(terminal, options, env); @@ -547,6 +564,7 @@ public void testSigningMetadataWithPasswordProtectedPemInTerminal() throws Excep } public void testDefaultOptionsWithSigningAndMultipleEncryptionKeys() throws Exception { + final KeyStoreWrapper usedKeyStore = randomFrom(keyStore, passwordProtectedKeystore); final Path dir = createTempDir(); final Path ksEncryptionFile = dir.resolve("saml-encryption.p12"); @@ -578,7 +596,7 @@ public void testDefaultOptionsWithSigningAndMultipleEncryptionKeys() throws Exce secureSettings.setString(RealmSettings.PREFIX + "saml.my_saml.encryption.keystore.secure_password", "ks-password"); secureSettings.setString(RealmSettings.PREFIX + "saml.my_saml.encryption.keystore.secure_key_password", "key-password"); - final SamlMetadataCommand command = new SamlMetadataCommand((e) -> keyStore); + final SamlMetadataCommand command = new SamlMetadataCommand((e) -> usedKeyStore); final OptionSet options = command.getParser().parse(new String[0]); final boolean useSigningCredentials = randomBoolean(); @@ -603,8 +621,7 @@ public void testDefaultOptionsWithSigningAndMultipleEncryptionKeys() throws Exce final Settings settings = settingsBuilder.build(); final Environment env = TestEnvironment.newEnvironment(settings); - final MockTerminal terminal = new MockTerminal(); - + final MockTerminal terminal = getTerminalPossiblyWithPassword(usedKeyStore); // What is the friendly name for "principal" attribute // "urn:oid:0.9.2342.19200300.100.1.1" [default: principal] terminal.addTextInput(""); @@ -679,6 +696,27 @@ public void testDefaultOptionsWithSigningAndMultipleEncryptionKeys() throws Exce } } + public void testWrongKeystorePassword() { + final Path certPath = getDataPath("saml.crt"); + final Path keyPath = getDataPath("saml.key"); + + final SamlMetadataCommand command = new SamlMetadataCommand((e) -> passwordProtectedKeystore); + final OptionSet options = command.getParser().parse(new String[]{ + "-signing-cert", certPath.toString(), + "-signing-key", keyPath.toString() + }); + final Settings settings = Settings.builder().put("path.home", createTempDir()).build(); + final Environment env = TestEnvironment.newEnvironment(settings); + + final MockTerminal terminal = new MockTerminal(); + terminal.addSecretInput("wrong-password"); + + UserException e = expectThrows(UserException.class, () -> { + command.buildEntityDescriptor(terminal, options, env); + }); + assertThat(e.getMessage(), CoreMatchers.containsString("Wrong password for elasticsearch.keystore")); + } + private String getAliasName(final Tuple certKeyPair) { // Keys are pre-generated with the same name, so add the serial no to the alias so that keystore entries won't be overwritten return certKeyPair.v1().getSubjectX500Principal().getName().toLowerCase(Locale.US) + "-"+ @@ -700,4 +738,12 @@ private boolean validateSignature(Signature signature) { return false; } } + + private MockTerminal getTerminalPossiblyWithPassword(KeyStoreWrapper keyStore) { + final MockTerminal terminal = new MockTerminal(); + if (keyStore.hasPassword()) { + terminal.addSecretInput("keystore-password"); + } + return terminal; + } }