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.

Sign up for GitHub

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

Jump to bottom

Handle pwd protected keystores in all CLI tools #45289

Merged
merged 9 commits into from
Aug 16, 2019
Merged

Handle pwd protected keystores in all CLI tools #45289

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
36dc4b8
Handle pwd protected keystores in all CLI tools
jkakavas Aug 7, 2019
39fa224
Merge remote-tracking branch 'origin/feature-pwd-protected-keystore' …
jkakavas Aug 7, 2019
4e2270f
add missing test and fix long lines
jkakavas Aug 7, 2019
e4cb7cc
onlyh test wrong password when keystore is pwd protected
jkakavas Aug 7, 2019
1b0262f
Address feedback to limit code duplication
jkakavas Aug 8, 2019
6471272
Address feedback
jkakavas Aug 12, 2019
3b07ef9
Address feedback
jkakavas Aug 14, 2019
f4da1fe
address feedback
jkakavas Aug 16, 2019
b1798f9
Merge remote-tracking branch 'origin/feature-pwd-protected-keystore' …
jkakavas Aug 16, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 15 additions & 5 deletions ...src/main/java/org/elasticsearch/xpack/security/authc/esnative/tool/SetupPasswordTool.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import org.elasticsearch.xpack.security.authc.esnative.ReservedRealm;
import org.elasticsearch.xpack.security.authc.esnative.tool.HttpResponse.HttpResponseBuilder;

import javax.crypto.AEADBadTagException;
import javax.net.ssl.SSLException;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
Expand Down Expand Up @@ -125,7 +126,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 +172,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 @@ -248,10 +249,19 @@ 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]);
try (SecureString keystorePassword = keyStoreWrapper.hasPassword() ?
Copy link
Member

Choose a reason for hiding this comment

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

Since we don't need to write the keystore back out in these tools, can we have a utility method somewhere to read the keystore, instead of copying this code to each tool?

new SecureString(terminal.readSecret("Enter the password for elasticsearch.keystore: ")) : new SecureString(new char[0])) {
keyStoreWrapper.decrypt(keystorePassword.getChars());
} catch (SecurityException e) {
if (e.getCause() instanceof AEADBadTagException) {
terminal.println("");
terminal.println("Failed to decrypt elasticsearch.keystore with the provided password.");
terminal.println("");
throw new UserException(ExitCodes.DATA_ERROR, "Wrong password for elasticsearch.keystore");
}
}

Settings.Builder settingsBuilder = Settings.builder();
settingsBuilder.put(env.settings(), true);
Expand Down
18 changes: 15 additions & 3 deletions ...curity/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlMetadataCommand.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
import org.elasticsearch.common.io.PathUtils;
import org.elasticsearch.common.logging.Loggers;
import org.elasticsearch.common.settings.KeyStoreWrapper;
import org.elasticsearch.common.settings.SecureString;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.LocaleUtils;
import org.elasticsearch.common.util.concurrent.ThreadContext;
Expand All @@ -65,6 +66,8 @@
import org.w3c.dom.Element;
import org.xml.sax.SAXException;

import javax.crypto.AEADBadTagException;

/**
* CLI tool to generate SAML Metadata for a Service Provider (realm)
*/
Expand Down Expand Up @@ -414,13 +417,22 @@ 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]);
try (SecureString keystorePassword = keyStoreWrapper.hasPassword() ?
new SecureString(terminal.readSecret("Enter the password for elasticsearch.keystore: ")) : new SecureString(new char[0])) {
keyStoreWrapper.decrypt(keystorePassword.getChars());
} catch (SecurityException e) {
if (e.getCause() instanceof AEADBadTagException) {
terminal.println("");
terminal.println("Failed to decrypt elasticsearch.keystore with the provided password.");
terminal.println("");
throw new UserException(ExitCodes.DATA_ERROR, "Wrong password for elasticsearch.keystore");
}
}

final Settings.Builder settingsBuilder = Settings.builder();
settingsBuilder.put(env.settings(), true);
Expand Down
43 changes: 38 additions & 5 deletions ...est/java/org/elasticsearch/xpack/security/authc/esnative/tool/SetupPasswordToolTests.java
View file
Open in desktop
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