Skip to content

Commit 16e9433

Browse files
authored
Fix ReloadSecureSettings API to consume password (#54771)
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 33dc417 commit 16e9433

File tree

19 files changed

+390
-38
lines changed

19 files changed

+390
-38
lines changed

build.gradle

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -405,6 +405,14 @@ class Run extends DefaultTask {
405405
public void setDataDir(String dataDirStr) {
406406
project.project(':distribution').run.dataDir = dataDirStr
407407
}
408+
409+
@Option(
410+
option = "keystore-password",
411+
description = "Set the elasticsearch keystore password"
412+
)
413+
public void setKeystorePassword(String password) {
414+
project.project(':distribution').run.keystorePassword = password
415+
}
408416
}
409417

410418
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
@@ -189,6 +189,11 @@ public void keystore(String key, FileSupplier valueSupplier) {
189189
nodes.all(each -> each.keystore(key, valueSupplier));
190190
}
191191

192+
@Override
193+
public void keystorePassword(String password) {
194+
nodes.all(each -> each.keystorePassword(password));
195+
}
196+
192197
@Override
193198
public void cliSetup(String binTool, CharSequence... args) {
194199
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
@@ -145,6 +145,7 @@ public class ElasticsearchNode implements TestClusterConfiguration {
145145
private final Path httpPortsFile;
146146
private final Path esStdoutFile;
147147
private final Path esStderrFile;
148+
private final Path esStdinFile;
148149
private final Path tmpDir;
149150

150151
private int currentDistro = 0;
@@ -156,6 +157,7 @@ public class ElasticsearchNode implements TestClusterConfiguration {
156157
private String httpPort = "0";
157158
private String transportPort = "0";
158159
private Path confPathData;
160+
private String keystorePassword = "";
159161

160162
ElasticsearchNode(String path, String name, Project project, ReaperService reaper, File workingDirBase) {
161163
this.path = path;
@@ -171,6 +173,7 @@ public class ElasticsearchNode implements TestClusterConfiguration {
171173
httpPortsFile = confPathLogs.resolve("http.ports");
172174
esStdoutFile = confPathLogs.resolve("es.stdout.log");
173175
esStderrFile = confPathLogs.resolve("es.stderr.log");
176+
esStdinFile = workingDir.resolve("es.stdin");
174177
tmpDir = workingDir.resolve("tmp");
175178
waitConditions.put("ports files", this::checkPortsFilesExistWithDelay);
176179

@@ -327,6 +330,11 @@ public void keystore(String key, FileSupplier valueSupplier) {
327330
keystoreFiles.put(key, valueSupplier);
328331
}
329332

333+
@Override
334+
public void keystorePassword(String password) {
335+
keystorePassword = password;
336+
}
337+
330338
@Override
331339
public void cliSetup(String binTool, CharSequence... args) {
332340
cliSetup.add(new CliEntry(binTool, args));
@@ -453,21 +461,25 @@ public synchronized void start() {
453461
logToProcessStdout("installed plugins");
454462
}
455463

464+
logToProcessStdout("Creating elasticsearch keystore with password set to [" + keystorePassword + "]");
465+
if (keystorePassword.length() > 0) {
466+
runElasticsearchBinScriptWithInput(keystorePassword + "\n" + keystorePassword, "elasticsearch-keystore", "create", "-p");
467+
} else {
468+
runElasticsearchBinScript("elasticsearch-keystore", "create");
469+
}
470+
456471
if (keystoreSettings.isEmpty() == false || keystoreFiles.isEmpty() == false) {
457472
logToProcessStdout("Adding " + keystoreSettings.size() + " keystore settings and " + keystoreFiles.size() + " keystore files");
458-
runElasticsearchBinScript("elasticsearch-keystore", "create");
459473

460-
keystoreSettings.forEach(
461-
(key, value) -> runElasticsearchBinScriptWithInput(value.toString(), "elasticsearch-keystore", "add", "-x", key)
462-
);
474+
keystoreSettings.forEach((key, value) -> runKeystoreCommandWithPassword(keystorePassword, value.toString(), "add", "-x", key));
463475

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

@@ -670,6 +682,11 @@ private void runElasticsearchBinScriptWithInput(String input, String tool, CharS
670682
}
671683
}
672684

685+
private void runKeystoreCommandWithPassword(String keystorePassword, String input, CharSequence... args) {
686+
final String actualInput = keystorePassword.length() > 0 ? keystorePassword + "\n" + input : input;
687+
runElasticsearchBinScriptWithInput(actualInput, "elasticsearch-keystore", args);
688+
}
689+
673690
private void runElasticsearchBinScript(String tool, CharSequence... args) {
674691
runElasticsearchBinScriptWithInput("", tool, args);
675692
}
@@ -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
@@ -67,6 +67,8 @@ public interface TestClusterConfiguration {
6767

6868
void keystore(String key, FileSupplier valueSupplier);
6969

70+
void keystorePassword(String password);
71+
7072
void cliSetup(String binTool, CharSequence... args);
7173

7274
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: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,28 @@
11
---
2-
"node_reload_secure_settings test":
2+
"node_reload_secure_settings test wrong password":
3+
4+
- do:
5+
nodes.reload_secure_settings:
6+
node_id: _local
7+
body:
8+
secure_settings_password: awrongpasswordhere
9+
- set:
10+
nodes._arbitrary_key_: node_id
11+
12+
- is_true: nodes
13+
- is_true: cluster_name
14+
- match: { nodes.$node_id.reload_exception.type: "security_exception" }
15+
- match: { nodes.$node_id.reload_exception.reason: "Provided keystore password was incorrect" }
16+
17+
---
18+
"node_reload_secure_settings test correct(empty) password":
319

420
- do:
521
nodes.reload_secure_settings: {}
622

23+
- set:
24+
nodes._arbitrary_key_: node_id
25+
726
- is_true: nodes
827
- is_true: cluster_name
28+
- is_false: nodes.$node_id.reload_exception

0 commit comments

Comments
 (0)