Skip to content

Commit

Permalink
[7.7] Fix ReloadSecureSettings API to consume password (elastic#54771) (
Browse files Browse the repository at this point in the history
elastic#55060)

The secure_settings_password was never taken into consideration in
the ReloadSecureSettings API. This commit fixes that and adds
necessary REST layer testing. Doing so, it also:

- Allows TestClusters to have a password protected keystore
so that it can be set for tests.
- Adds a parameter to the run task so that elastisearch can
be run with a password protected keystore from source.
  • Loading branch information
jkakavas authored Apr 13, 2020
1 parent 206120e commit ee49268
Show file tree
Hide file tree
Showing 19 changed files with 393 additions and 38 deletions.
8 changes: 8 additions & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,14 @@ class Run extends DefaultTask {
public void setDataDir(String dataDirStr) {
project.project(':distribution').run.dataDir = dataDirStr
}

@Option(
option = "keystore-password",
description = "Set the elasticsearch keystore password"
)
public void setKeystorePassword(String password) {
project.project(':distribution').run.keystorePassword = password
}
}

task run(type: Run) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,11 @@ public void keystore(String key, FileSupplier valueSupplier) {
nodes.all(each -> each.keystore(key, valueSupplier));
}

@Override
public void keystorePassword(String password) {
nodes.all(each -> each.keystorePassword(password));
}

@Override
public void cliSetup(String binTool, CharSequence... args) {
nodes.all(each -> each.cliSetup(binTool, args));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ public class ElasticsearchNode implements TestClusterConfiguration {
private final Path httpPortsFile;
private final Path esStdoutFile;
private final Path esStderrFile;
private final Path esStdinFile;
private final Path tmpDir;

private int currentDistro = 0;
Expand All @@ -154,6 +155,7 @@ public class ElasticsearchNode implements TestClusterConfiguration {
private String httpPort = "0";
private String transportPort = "0";
private Path confPathData;
private String keystorePassword = "";

ElasticsearchNode(String name, Project project, ReaperService reaper, File workingDirBase, Jdk bwcJdk) {
this.path = project.getPath();
Expand All @@ -170,6 +172,7 @@ public class ElasticsearchNode implements TestClusterConfiguration {
httpPortsFile = confPathLogs.resolve("http.ports");
esStdoutFile = confPathLogs.resolve("es.stdout.log");
esStderrFile = confPathLogs.resolve("es.stderr.log");
esStdinFile = workingDir.resolve("es.stdin");
tmpDir = workingDir.resolve("tmp");
waitConditions.put("ports files", this::checkPortsFilesExistWithDelay);

Expand Down Expand Up @@ -305,6 +308,11 @@ public void keystore(String key, FileSupplier valueSupplier) {
keystoreFiles.put(key, valueSupplier);
}

@Override
public void keystorePassword(String password) {
keystorePassword = password;
}

@Override
public void cliSetup(String binTool, CharSequence... args) {
cliSetup.add(new CliEntry(binTool, args));
Expand Down Expand Up @@ -439,21 +447,25 @@ public synchronized void start() {
}
}

logToProcessStdout("Creating elasticsearch keystore with password set to [" + keystorePassword + "]");
if (keystorePassword.length() > 0) {
runElasticsearchBinScriptWithInput(keystorePassword + "\n" + keystorePassword, "elasticsearch-keystore", "create", "-p");
} else {
runElasticsearchBinScript("elasticsearch-keystore", "create");
}

if (keystoreSettings.isEmpty() == false || keystoreFiles.isEmpty() == false) {
logToProcessStdout("Adding " + keystoreSettings.size() + " keystore settings and " + keystoreFiles.size() + " keystore files");
runElasticsearchBinScript("elasticsearch-keystore", "create");

keystoreSettings.forEach(
(key, value) -> runElasticsearchBinScriptWithInput(value.toString(), "elasticsearch-keystore", "add", "-x", key)
);
keystoreSettings.forEach((key, value) -> runKeystoreCommandWithPassword(keystorePassword, value.toString(), "add", "-x", key));

for (Map.Entry<String, File> entry : keystoreFiles.entrySet()) {
File file = entry.getValue();
requireNonNull(file, "supplied keystoreFile was null when configuring " + this);
if (file.exists() == false) {
throw new TestClustersException("supplied keystore file " + file + " does not exist, require for " + this);
}
runElasticsearchBinScript("elasticsearch-keystore", "add-file", entry.getKey(), file.getAbsolutePath());
runKeystoreCommandWithPassword(keystorePassword, "", "add-file", entry.getKey(), file.getAbsolutePath());
}
}

Expand Down Expand Up @@ -657,6 +669,11 @@ private void runElasticsearchBinScriptWithInput(String input, String tool, CharS
}
}

private void runKeystoreCommandWithPassword(String keystorePassword, String input, CharSequence... args) {
final String actualInput = keystorePassword.length() > 0 ? keystorePassword + "\n" + input : input;
runElasticsearchBinScriptWithInput(actualInput, "elasticsearch-keystore", args);
}

private void runElasticsearchBinScript(String tool, CharSequence... args) {
runElasticsearchBinScriptWithInput("", tool, args);
}
Expand Down Expand Up @@ -746,6 +763,14 @@ private void startElasticsearchProcess() {
processBuilder.redirectError(ProcessBuilder.Redirect.appendTo(esStderrFile.toFile()));
processBuilder.redirectOutput(ProcessBuilder.Redirect.appendTo(esStdoutFile.toFile()));

if (keystorePassword != null && keystorePassword.length() > 0) {
try {
Files.write(esStdinFile, (keystorePassword + "\n").getBytes(StandardCharsets.UTF_8), StandardOpenOption.CREATE);
processBuilder.redirectInput(esStdinFile.toFile());
} catch (IOException e) {
throw new TestClustersException("Failed to set the keystore password for " + this, e);
}
}
LOGGER.info("Running `{}` in `{}` for {} env: {}", command, workingDir, this, environment);
try {
esProcess = processBuilder.start();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ public class RunTask extends DefaultTestClustersTask {

private Path dataDir = null;

private String keystorePassword = "";

@Option(option = "debug-jvm", description = "Enable debugging configuration, to allow attaching a debugger to elasticsearch.")
public void setDebug(boolean enabled) {
this.debug = enabled;
Expand All @@ -43,6 +45,17 @@ public void setDataDir(String dataDirStr) {
dataDir = Paths.get(dataDirStr).toAbsolutePath();
}

@Option(option = "keystore-password", description = "Set the elasticsearch keystore password")
public void setKeystorePassword(String password) {
keystorePassword = password;
}

@Input
@Optional
public String getKeystorePassword() {
return keystorePassword;
}

@Input
@Optional
public String getDataDir() {
Expand Down Expand Up @@ -90,6 +103,9 @@ public void beforeStart() {
node.jvmArgs("-agentlib:jdwp=transport=dt_socket,server=n,suspend=y,address=" + debugPort);
debugPort += 1;
}
if (keystorePassword.length() > 0) {
node.keystorePassword(keystorePassword);
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ public interface TestClusterConfiguration {

void keystore(String key, FileSupplier valueSupplier);

void keystorePassword(String password);

void cliSetup(String binTool, CharSequence... args);

void setting(String key, String value);
Expand Down
1 change: 1 addition & 0 deletions docs/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ testClusters.integTest {
}
setting 'xpack.autoscaling.enabled', 'true'
setting 'xpack.eql.enabled', 'true'
keystorePassword 's3cr3t'
}

// enable regexes in painless so our tests don't complain about example snippets that use them
Expand Down
37 changes: 11 additions & 26 deletions docs/reference/cluster/nodes-reload-secure-settings.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -36,24 +36,33 @@ the node-specific {es} keystore password.
(Optional, string) The names of particular nodes in the cluster to target.
For example, `nodeId1,nodeId2`. For node selection options, see
<<cluster-nodes>>.

NOTE: {es} requires consistent secure settings across the cluster nodes, but
this consistency is not enforced. Hence, reloading specific nodes is not
standard. It is justifiable only when retrying failed reload operations.

[[cluster-nodes-reload-secure-settings-api-request-body]]
==== {api-request-body-title}

`reload_secure_settings`::
`secure_settings_password`::
(Optional, string) The password for the {es} keystore.

[[cluster-nodes-reload-secure-settings-api-example]]
==== {api-examples-title}

The following examples assume a common password for the {es} keystore on every
node of the cluster:

[source,console]
--------------------------------------------------
POST _nodes/reload_secure_settings
{
"secure_settings_password":"s3cr3t"
}
POST _nodes/nodeId1,nodeId2/reload_secure_settings
{
"secure_settings_password":"s3cr3t"
}
--------------------------------------------------
// TEST[setup:node]
// TEST[s/nodeId1,nodeId2/*/]
Expand Down Expand Up @@ -81,27 +90,3 @@ that was thrown during the reload process, if any.
--------------------------------------------------
// TESTRESPONSE[s/"my_cluster"/$body.cluster_name/]
// TESTRESPONSE[s/"pQHNt5rXTTWNvUgOrdynKg"/\$node_name/]

The following example uses a common password for the {es} keystore on every
node of the cluster:

[source,js]
--------------------------------------------------
POST _nodes/reload_secure_settings
{
"reload_secure_settings": "s3cr3t"
}
--------------------------------------------------
// NOTCONSOLE

The following example uses a password for the {es} keystore on the local node:

[source,js]
--------------------------------------------------
POST _nodes/_local/reload_secure_settings
{
"reload_secure_settings": "s3cr3t"
}
--------------------------------------------------
// NOTCONSOLE

2 changes: 1 addition & 1 deletion docs/reference/setup/secure-settings.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ using the `bin/elasticsearch-keystore add` command, call:
----
POST _nodes/reload_secure_settings
{
"reload_secure_settings": "s3cr3t" <1>
"secure_settings_password": "s3cr3t" <1>
}
----
// NOTCONSOLE
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,15 @@
}
]
},
"params":{
"timeout":{
"type":"time",
"description":"Explicit operation timeout"
"params": {
"timeout": {
"type": "time",
"description": "Explicit operation timeout"
}
},
"body": {
"description": "An object containing the password for the elasticsearch keystore",
"required": false
}
}
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,31 @@
---
"node_reload_secure_settings test":
"node_reload_secure_settings test wrong password":
- skip:
version: " - 7.6.99"
reason: "support for reloading password protected keystores was introduced in 7.7.0"

- do:
nodes.reload_secure_settings:
node_id: _local
body:
secure_settings_password: awrongpasswordhere
- set:
nodes._arbitrary_key_: node_id

- is_true: nodes
- is_true: cluster_name
- match: { nodes.$node_id.reload_exception.type: "security_exception" }
- match: { nodes.$node_id.reload_exception.reason: "Provided keystore password was incorrect" }

---
"node_reload_secure_settings test correct(empty) password":

- do:
nodes.reload_secure_settings: {}

- set:
nodes._arbitrary_key_: node_id

- is_true: nodes
- is_true: cluster_name
- is_false: nodes.$node_id.reload_exception
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public RestChannelConsumer prepareRequest(RestRequest request, NodeClient client
.setNodesIds(nodesIds);
request.withContentOrSourceParamParserOrNull(parser -> {
if (parser != null) {
final NodesReloadSecureSettingsRequest nodesRequest = nodesRequestBuilder.request();
final NodesReloadSecureSettingsRequest nodesRequest = PARSER.parse(parser, null);
nodesRequestBuilder.setSecureStorePassword(nodesRequest.getSecureSettingsPassword());
}
});
Expand Down
52 changes: 52 additions & 0 deletions x-pack/qa/password-protected-keystore/build.gradle
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/*
* 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.
*/

/*
* Tests that need to run against an Elasticsearch cluster that
* is using a password protected keystore in its nodes.
*/

apply plugin: 'elasticsearch.testclusters'
apply plugin: 'elasticsearch.standalone-rest-test'
apply plugin: 'elasticsearch.rest-test'
dependencies {
testCompile project(path: xpackModule('core'), configuration: 'default')
}

testClusters.integTest {
testDistribution = 'DEFAULT'
numberOfNodes = 2
keystorePassword 's3cr3t'

setting 'xpack.security.enabled', 'true'
setting 'xpack.security.authc.anonymous.roles', 'anonymous'
setting 'xpack.security.transport.ssl.enabled', 'true'
setting 'xpack.security.transport.ssl.certificate', 'transport.crt'
setting 'xpack.security.transport.ssl.key', 'transport.key'
setting 'xpack.security.transport.ssl.key_passphrase', 'transport-password'
setting 'xpack.security.transport.ssl.certificate_authorities', 'ca.crt'

extraConfigFile 'transport.key', file('src/test/resources/ssl/transport.key')
extraConfigFile 'transport.crt', file('src/test/resources/ssl/transport.crt')
extraConfigFile 'ca.crt', file('src/test/resources/ssl/ca.crt')
extraConfigFile 'roles.yml', file('src/test/resources/roles.yml')

user username: 'admin_user', password: 'admin-password'
user username:'test-user' ,password: 'test-password', role: 'user_role'
}
Loading

0 comments on commit ee49268

Please sign in to comment.