Skip to content

Commit 7a8a66d

Browse files
authored
[7.x] Fix ReloadSecureSettings API to consume password (#54771) (#55059)
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.
1 parent 8627999 commit 7a8a66d

File tree

19 files changed

+393
-38
lines changed

19 files changed

+393
-38
lines changed

build.gradle

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -408,6 +408,14 @@ class Run extends DefaultTask {
408408
public void setDataDir(String dataDirStr) {
409409
project.project(':distribution').run.dataDir = dataDirStr
410410
}
411+
412+
@Option(
413+
option = "keystore-password",
414+
description = "Set the elasticsearch keystore password"
415+
)
416+
public void setKeystorePassword(String password) {
417+
project.project(':distribution').run.keystorePassword = password
418+
}
411419
}
412420

413421
task run(type: Run) {

buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchCluster.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,11 @@ public void keystore(String key, FileSupplier valueSupplier) {
174174
nodes.all(each -> each.keystore(key, valueSupplier));
175175
}
176176

177+
@Override
178+
public void keystorePassword(String password) {
179+
nodes.all(each -> each.keystorePassword(password));
180+
}
181+
177182
@Override
178183
public void cliSetup(String binTool, CharSequence... args) {
179184
nodes.all(each -> each.cliSetup(binTool, args));

buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,7 @@ public class ElasticsearchNode implements TestClusterConfiguration {
143143
private final Path httpPortsFile;
144144
private final Path esStdoutFile;
145145
private final Path esStderrFile;
146+
private final Path esStdinFile;
146147
private final Path tmpDir;
147148

148149
private int currentDistro = 0;
@@ -154,6 +155,7 @@ public class ElasticsearchNode implements TestClusterConfiguration {
154155
private String httpPort = "0";
155156
private String transportPort = "0";
156157
private Path confPathData;
158+
private String keystorePassword = "";
157159

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

@@ -305,6 +308,11 @@ public void keystore(String key, FileSupplier valueSupplier) {
305308
keystoreFiles.put(key, valueSupplier);
306309
}
307310

311+
@Override
312+
public void keystorePassword(String password) {
313+
keystorePassword = password;
314+
}
315+
308316
@Override
309317
public void cliSetup(String binTool, CharSequence... args) {
310318
cliSetup.add(new CliEntry(binTool, args));
@@ -439,21 +447,25 @@ public synchronized void start() {
439447
}
440448
}
441449

450+
logToProcessStdout("Creating elasticsearch keystore with password set to [" + keystorePassword + "]");
451+
if (keystorePassword.length() > 0) {
452+
runElasticsearchBinScriptWithInput(keystorePassword + "\n" + keystorePassword, "elasticsearch-keystore", "create", "-p");
453+
} else {
454+
runElasticsearchBinScript("elasticsearch-keystore", "create");
455+
}
456+
442457
if (keystoreSettings.isEmpty() == false || keystoreFiles.isEmpty() == false) {
443458
logToProcessStdout("Adding " + keystoreSettings.size() + " keystore settings and " + keystoreFiles.size() + " keystore files");
444-
runElasticsearchBinScript("elasticsearch-keystore", "create");
445459

446-
keystoreSettings.forEach(
447-
(key, value) -> runElasticsearchBinScriptWithInput(value.toString(), "elasticsearch-keystore", "add", "-x", key)
448-
);
460+
keystoreSettings.forEach((key, value) -> runKeystoreCommandWithPassword(keystorePassword, value.toString(), "add", "-x", key));
449461

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

@@ -657,6 +669,11 @@ private void runElasticsearchBinScriptWithInput(String input, String tool, CharS
657669
}
658670
}
659671

672+
private void runKeystoreCommandWithPassword(String keystorePassword, String input, CharSequence... args) {
673+
final String actualInput = keystorePassword.length() > 0 ? keystorePassword + "\n" + input : input;
674+
runElasticsearchBinScriptWithInput(actualInput, "elasticsearch-keystore", args);
675+
}
676+
660677
private void runElasticsearchBinScript(String tool, CharSequence... args) {
661678
runElasticsearchBinScriptWithInput("", tool, args);
662679
}
@@ -746,6 +763,14 @@ private void startElasticsearchProcess() {
746763
processBuilder.redirectError(ProcessBuilder.Redirect.appendTo(esStderrFile.toFile()));
747764
processBuilder.redirectOutput(ProcessBuilder.Redirect.appendTo(esStdoutFile.toFile()));
748765

766+
if (keystorePassword != null && keystorePassword.length() > 0) {
767+
try {
768+
Files.write(esStdinFile, (keystorePassword + "\n").getBytes(StandardCharsets.UTF_8), StandardOpenOption.CREATE);
769+
processBuilder.redirectInput(esStdinFile.toFile());
770+
} catch (IOException e) {
771+
throw new TestClustersException("Failed to set the keystore password for " + this, e);
772+
}
773+
}
749774
LOGGER.info("Running `{}` in `{}` for {} env: {}", command, workingDir, this, environment);
750775
try {
751776
esProcess = processBuilder.start();

buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/RunTask.java

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ public class RunTask extends DefaultTestClustersTask {
2828

2929
private Path dataDir = null;
3030

31+
private String keystorePassword = "";
32+
3133
@Option(option = "debug-jvm", description = "Enable debugging configuration, to allow attaching a debugger to elasticsearch.")
3234
public void setDebug(boolean enabled) {
3335
this.debug = enabled;
@@ -43,6 +45,17 @@ public void setDataDir(String dataDirStr) {
4345
dataDir = Paths.get(dataDirStr).toAbsolutePath();
4446
}
4547

48+
@Option(option = "keystore-password", description = "Set the elasticsearch keystore password")
49+
public void setKeystorePassword(String password) {
50+
keystorePassword = password;
51+
}
52+
53+
@Input
54+
@Optional
55+
public String getKeystorePassword() {
56+
return keystorePassword;
57+
}
58+
4659
@Input
4760
@Optional
4861
public String getDataDir() {
@@ -90,6 +103,9 @@ public void beforeStart() {
90103
node.jvmArgs("-agentlib:jdwp=transport=dt_socket,server=n,suspend=y,address=" + debugPort);
91104
debugPort += 1;
92105
}
106+
if (keystorePassword.length() > 0) {
107+
node.keystorePassword(keystorePassword);
108+
}
93109
}
94110
}
95111
}

buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/TestClusterConfiguration.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,8 @@ public interface TestClusterConfiguration {
5757

5858
void keystore(String key, FileSupplier valueSupplier);
5959

60+
void keystorePassword(String password);
61+
6062
void cliSetup(String binTool, CharSequence... args);
6163

6264
void setting(String key, String value);

docs/build.gradle

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ testClusters.integTest {
5858
}
5959
setting 'xpack.autoscaling.enabled', 'true'
6060
setting 'xpack.eql.enabled', 'true'
61+
keystorePassword 's3cr3t'
6162
}
6263

6364
// enable regexes in painless so our tests don't complain about example snippets that use them

docs/reference/cluster/nodes-reload-secure-settings.asciidoc

Lines changed: 11 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -36,24 +36,33 @@ the node-specific {es} keystore password.
3636
(Optional, string) The names of particular nodes in the cluster to target.
3737
For example, `nodeId1,nodeId2`. For node selection options, see
3838
<<cluster-nodes>>.
39-
39+
4040
NOTE: {es} requires consistent secure settings across the cluster nodes, but
4141
this consistency is not enforced. Hence, reloading specific nodes is not
4242
standard. It is justifiable only when retrying failed reload operations.
4343

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

47-
`reload_secure_settings`::
47+
`secure_settings_password`::
4848
(Optional, string) The password for the {es} keystore.
4949

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

53+
The following examples assume a common password for the {es} keystore on every
54+
node of the cluster:
55+
5356
[source,console]
5457
--------------------------------------------------
5558
POST _nodes/reload_secure_settings
59+
{
60+
"secure_settings_password":"s3cr3t"
61+
}
5662
POST _nodes/nodeId1,nodeId2/reload_secure_settings
63+
{
64+
"secure_settings_password":"s3cr3t"
65+
}
5766
--------------------------------------------------
5867
// TEST[setup:node]
5968
// TEST[s/nodeId1,nodeId2/*/]
@@ -81,27 +90,3 @@ that was thrown during the reload process, if any.
8190
--------------------------------------------------
8291
// TESTRESPONSE[s/"my_cluster"/$body.cluster_name/]
8392
// TESTRESPONSE[s/"pQHNt5rXTTWNvUgOrdynKg"/\$node_name/]
84-
85-
The following example uses a common password for the {es} keystore on every
86-
node of the cluster:
87-
88-
[source,js]
89-
--------------------------------------------------
90-
POST _nodes/reload_secure_settings
91-
{
92-
"reload_secure_settings": "s3cr3t"
93-
}
94-
--------------------------------------------------
95-
// NOTCONSOLE
96-
97-
The following example uses a password for the {es} keystore on the local node:
98-
99-
[source,js]
100-
--------------------------------------------------
101-
POST _nodes/_local/reload_secure_settings
102-
{
103-
"reload_secure_settings": "s3cr3t"
104-
}
105-
--------------------------------------------------
106-
// NOTCONSOLE
107-

docs/reference/setup/secure-settings.asciidoc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ using the `bin/elasticsearch-keystore add` command, call:
3535
----
3636
POST _nodes/reload_secure_settings
3737
{
38-
"reload_secure_settings": "s3cr3t" <1>
38+
"secure_settings_password": "s3cr3t" <1>
3939
}
4040
----
4141
// NOTCONSOLE

rest-api-spec/src/main/resources/rest-api-spec/api/nodes.reload_secure_settings.json

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,15 @@
2727
}
2828
]
2929
},
30-
"params":{
31-
"timeout":{
32-
"type":"time",
33-
"description":"Explicit operation timeout"
30+
"params": {
31+
"timeout": {
32+
"type": "time",
33+
"description": "Explicit operation timeout"
3434
}
35+
},
36+
"body": {
37+
"description": "An object containing the password for the elasticsearch keystore",
38+
"required": false
3539
}
3640
}
3741
}
Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,31 @@
11
---
2-
"node_reload_secure_settings test":
2+
"node_reload_secure_settings test wrong password":
3+
- skip:
4+
version: " - 7.99.99"
5+
reason: "support for reloading password protected keystores was introduced in 7.7.0"
6+
7+
- do:
8+
nodes.reload_secure_settings:
9+
node_id: _local
10+
body:
11+
secure_settings_password: awrongpasswordhere
12+
- set:
13+
nodes._arbitrary_key_: node_id
14+
15+
- is_true: nodes
16+
- is_true: cluster_name
17+
- match: { nodes.$node_id.reload_exception.type: "security_exception" }
18+
- match: { nodes.$node_id.reload_exception.reason: "Provided keystore password was incorrect" }
19+
20+
---
21+
"node_reload_secure_settings test correct(empty) password":
322

423
- do:
524
nodes.reload_secure_settings: {}
625

26+
- set:
27+
nodes._arbitrary_key_: node_id
28+
729
- is_true: nodes
830
- is_true: cluster_name
31+
- is_false: nodes.$node_id.reload_exception

0 commit comments

Comments
 (0)