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

Handle pwd protected keystores in all CLI tools #45289

Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,16 @@

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;

/**
* A sub-command for the keystore cli to create a new keystore.
*/
class CreateKeyStoreCommand extends EnvironmentAwareCommand {
class CreateKeyStoreCommand extends KeyStoreAwareCommand {

private final OptionSpec<Void> passwordOption;

Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
@@ -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() ?
new SecureString(terminal.readSecret("Enter the password for elasticsearch.keystore: ")) : new SecureString(new char[0])) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there are a reason this doesn't call readPassword(terminal, false) ?

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;
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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;

Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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";

Expand Down Expand Up @@ -414,13 +414,12 @@ private SortedSet<String> sorted(Set<String> 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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -74,6 +76,7 @@ public class SetupPasswordToolTests extends CommandTestCase {
private CommandLineHttpClient httpClient;
private KeyStoreWrapper keyStore;
private List<String> usersInSetOrder;
private boolean passwordProtectedKeystore = false;
@Rule
public ExpectedException thrown = ExpectedException.none();

Expand All @@ -95,6 +98,14 @@ public void setSecretsAndKeyStore() throws Exception {
when(keyStore.getSettingNames()).thenReturn(Collections.singleton(KeyStoreWrapper.SEED_SETTING.getKey()));
when(keyStore.getString(KeyStoreWrapper.SEED_SETTING.getKey())).thenReturn(bootstrapPassword);
}
if (randomBoolean()) {
passwordProtectedKeystore = true;
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());
terminal.addSecretInput("keystore-password");
}

when(httpClient.getDefaultURL()).thenReturn("http://localhost:9200");

Expand Down Expand Up @@ -165,8 +176,12 @@ public void testAutoSetup() throws Exception {
terminal.addTextInput("Y");
execute("auto", pathHomeParameter);
}

verify(keyStore).decrypt(new char[0]);
if (passwordProtectedKeystore) {
// SecureString is already closed (zero-filled) and keystore-password is 17 char long
verify(keyStore).decrypt(new char[17]);
} else {
verify(keyStore).decrypt(new char[0]);
}

InOrder inOrder = Mockito.inOrder(httpClient);

Expand Down Expand Up @@ -401,14 +416,17 @@ public void testInteractiveSetup() throws Exception {
ArgumentCaptor<CheckedSupplier<String, Exception>> 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 testInteractivePasswordsFatFingers() throws Exception {
URL url = new URL(httpClient.getDefaultURL());

terminal.reset();
if (passwordProtectedKeystore) {
terminal.addSecretInput("keystore-password");
}
terminal.addTextInput("Y");
for (String user : SetupPasswordTool.USERS) {
// fail in strength and match
Expand Down Expand Up @@ -439,10 +457,25 @@ public void testInteractivePasswordsFatFingers() throws Exception {
ArgumentCaptor<CheckedSupplier<String, Exception>> 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 {
assumeFalse("Can't test this when the keystore isn't password protected", passwordProtectedKeystore == false);
terminal.reset();
terminal.addSecretInput("wrong-password");
final UserException e = expectThrows(UserException.class, () -> {
if (randomBoolean()) {
execute("auto", pathHomeParameter, "-b", "true");
} else {
terminal.addTextInput("Y");
execute("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)) {
Expand Down
Loading