Skip to content

Commit 98ad0eb

Browse files
authored
Reload secure settings with password (#43197)
If a password is not set, we assume an empty string to be compatible with previous behavior. Only allow the reload to be broadcast to other nodes if TLS is enabled for the transport layer.
1 parent af512f9 commit 98ad0eb

File tree

11 files changed

+466
-116
lines changed

11 files changed

+466
-116
lines changed

build.gradle

+2-2
Original file line numberDiff line numberDiff line change
@@ -160,8 +160,8 @@ task verifyVersions {
160160
* after the backport of the backcompat code is complete.
161161
*/
162162

163-
boolean bwc_tests_enabled = true
164-
final String bwc_tests_disabled_issue = "" /* place a PR link here when committing bwc changes */
163+
boolean bwc_tests_enabled = false
164+
final String bwc_tests_disabled_issue = "https://github.com/elastic/elasticsearch/pull/43197"
165165
if (bwc_tests_enabled == false) {
166166
if (bwc_tests_disabled_issue.isEmpty()) {
167167
throw new GradleException("bwc_tests_disabled_issue must be set when bwc_tests_enabled == false")

server/src/main/java/org/elasticsearch/action/admin/cluster/node/reload/NodesReloadSecureSettingsRequest.java

+77-4
Original file line numberDiff line numberDiff line change
@@ -19,22 +19,95 @@
1919

2020
package org.elasticsearch.action.admin.cluster.node.reload;
2121

22+
import org.elasticsearch.Version;
2223
import org.elasticsearch.action.support.nodes.BaseNodesRequest;
24+
import org.elasticsearch.common.CharArrays;
25+
import org.elasticsearch.common.Nullable;
26+
import org.elasticsearch.common.bytes.BytesArray;
27+
import org.elasticsearch.common.bytes.BytesReference;
28+
import org.elasticsearch.common.io.stream.StreamInput;
29+
import org.elasticsearch.common.io.stream.StreamOutput;
30+
import org.elasticsearch.common.settings.SecureString;
31+
32+
import java.io.IOException;
33+
import java.util.Arrays;
2334

2435
/**
25-
* Request for a reload secure settings action.
36+
* Request for a reload secure settings action
2637
*/
2738
public class NodesReloadSecureSettingsRequest extends BaseNodesRequest<NodesReloadSecureSettingsRequest> {
2839

40+
/**
41+
* The password is used to re-read and decrypt the contents
42+
* of the node's keystore (backing the implementation of
43+
* {@code SecureSettings}).
44+
*/
45+
@Nullable
46+
private SecureString secureSettingsPassword;
47+
2948
public NodesReloadSecureSettingsRequest() {
3049
}
3150

3251
/**
33-
* Reload secure settings only on certain nodes, based on the nodes IDs specified. If none are passed, secure settings will be reloaded
34-
* on all the nodes.
52+
* Reload secure settings only on certain nodes, based on the nodes ids
53+
* specified. If none are passed, secure settings will be reloaded on all the
54+
* nodes.
3555
*/
36-
public NodesReloadSecureSettingsRequest(final String... nodesIds) {
56+
public NodesReloadSecureSettingsRequest(String... nodesIds) {
3757
super(nodesIds);
3858
}
3959

60+
@Nullable
61+
public SecureString getSecureSettingsPassword() {
62+
return secureSettingsPassword;
63+
}
64+
65+
public void setSecureStorePassword(SecureString secureStorePassword) {
66+
this.secureSettingsPassword = secureStorePassword;
67+
}
68+
69+
public void closePassword() {
70+
if (this.secureSettingsPassword != null) {
71+
this.secureSettingsPassword.close();
72+
}
73+
}
74+
75+
boolean hasPassword() {
76+
return this.secureSettingsPassword != null && this.secureSettingsPassword.length() > 0;
77+
}
78+
79+
@Override
80+
public void readFrom(StreamInput in) throws IOException {
81+
super.readFrom(in);
82+
if (in.getVersion().onOrAfter(Version.V_7_4_0)) {
83+
final BytesReference bytesRef = in.readOptionalBytesReference();
84+
if (bytesRef != null) {
85+
byte[] bytes = BytesReference.toBytes(bytesRef);
86+
try {
87+
this.secureSettingsPassword = new SecureString(CharArrays.utf8BytesToChars(bytes));
88+
} finally {
89+
Arrays.fill(bytes, (byte) 0);
90+
}
91+
} else {
92+
this.secureSettingsPassword = null;
93+
}
94+
}
95+
}
96+
97+
@Override
98+
public void writeTo(StreamOutput out) throws IOException {
99+
super.writeTo(out);
100+
if (out.getVersion().onOrAfter(Version.V_7_4_0)) {
101+
if (this.secureSettingsPassword == null) {
102+
out.writeOptionalBytesReference(null);
103+
} else {
104+
final byte[] passwordBytes = CharArrays.toUtf8Bytes(this.secureSettingsPassword.getChars());
105+
try {
106+
out.writeOptionalBytesReference(new BytesArray(passwordBytes));
107+
} finally {
108+
Arrays.fill(passwordBytes, (byte) 0);
109+
}
110+
}
111+
}
112+
}
40113
}

server/src/main/java/org/elasticsearch/action/admin/cluster/node/reload/NodesReloadSecureSettingsRequestBuilder.java

+6
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
import org.elasticsearch.action.support.nodes.NodesOperationRequestBuilder;
2323
import org.elasticsearch.client.ElasticsearchClient;
24+
import org.elasticsearch.common.settings.SecureString;
2425

2526
/**
2627
* Builder for the reload secure settings nodes request
@@ -32,4 +33,9 @@ public NodesReloadSecureSettingsRequestBuilder(ElasticsearchClient client, Nodes
3233
super(client, action, new NodesReloadSecureSettingsRequest());
3334
}
3435

36+
public NodesReloadSecureSettingsRequestBuilder setSecureStorePassword(SecureString secureStorePassword) {
37+
request.setSecureStorePassword(secureStorePassword);
38+
return this;
39+
}
40+
3541
}

server/src/main/java/org/elasticsearch/action/admin/cluster/node/reload/TransportNodesReloadSecureSettingsAction.java

+47-1
Original file line numberDiff line numberDiff line change
@@ -21,16 +21,20 @@
2121

2222
import org.apache.logging.log4j.message.ParameterizedMessage;
2323
import org.apache.logging.log4j.util.Supplier;
24+
import org.elasticsearch.ElasticsearchException;
2425
import org.elasticsearch.ExceptionsHelper;
26+
import org.elasticsearch.action.ActionListener;
2527
import org.elasticsearch.action.FailedNodeException;
2628
import org.elasticsearch.action.support.ActionFilters;
2729
import org.elasticsearch.action.support.nodes.BaseNodeRequest;
2830
import org.elasticsearch.action.support.nodes.TransportNodesAction;
31+
import org.elasticsearch.cluster.node.DiscoveryNode;
2932
import org.elasticsearch.cluster.service.ClusterService;
3033
import org.elasticsearch.common.inject.Inject;
3134
import org.elasticsearch.common.io.stream.StreamInput;
3235
import org.elasticsearch.common.io.stream.StreamOutput;
3336
import org.elasticsearch.common.settings.KeyStoreWrapper;
37+
import org.elasticsearch.common.settings.SecureString;
3438
import org.elasticsearch.common.settings.Settings;
3539
import org.elasticsearch.env.Environment;
3640
import org.elasticsearch.plugins.PluginsService;
@@ -78,15 +82,39 @@ protected NodesReloadSecureSettingsResponse.NodeResponse newNodeResponse() {
7882
return new NodesReloadSecureSettingsResponse.NodeResponse();
7983
}
8084

85+
@Override
86+
protected void doExecute(Task task, NodesReloadSecureSettingsRequest request,
87+
ActionListener<NodesReloadSecureSettingsResponse> listener) {
88+
if (request.hasPassword() && isNodeLocal(request) == false && isNodeTransportTLSEnabled() == false) {
89+
request.closePassword();
90+
listener.onFailure(
91+
new ElasticsearchException("Secure settings cannot be updated cluster wide when TLS for the transport layer" +
92+
" is not enabled. Enable TLS or use the API with a `_local` filter on each node."));
93+
} else {
94+
super.doExecute(task, request, ActionListener.wrap(response -> {
95+
request.closePassword();
96+
listener.onResponse(response);
97+
}, e -> {
98+
request.closePassword();
99+
listener.onFailure(e);
100+
}));
101+
}
102+
}
103+
81104
@Override
82105
protected NodesReloadSecureSettingsResponse.NodeResponse nodeOperation(NodeRequest nodeReloadRequest, Task task) {
106+
final NodesReloadSecureSettingsRequest request = nodeReloadRequest.request;
107+
// We default to using an empty string as the keystore password so that we mimic pre 7.3 API behavior
108+
final SecureString secureSettingsPassword = request.hasPassword() ? request.getSecureSettingsPassword() :
109+
new SecureString(new char[0]);
83110
try (KeyStoreWrapper keystore = KeyStoreWrapper.load(environment.configFile())) {
84111
// reread keystore from config file
85112
if (keystore == null) {
86113
return new NodesReloadSecureSettingsResponse.NodeResponse(clusterService.localNode(),
87114
new IllegalStateException("Keystore is missing"));
88115
}
89-
keystore.decrypt(new char[0]);
116+
// decrypt the keystore using the password from the request
117+
keystore.decrypt(secureSettingsPassword.getChars());
90118
// add the keystore to the original node settings object
91119
final Settings settingsWithKeystore = Settings.builder()
92120
.put(environment.settings(), false)
@@ -107,6 +135,8 @@ protected NodesReloadSecureSettingsResponse.NodeResponse nodeOperation(NodeReque
107135
return new NodesReloadSecureSettingsResponse.NodeResponse(clusterService.localNode(), null);
108136
} catch (final Exception e) {
109137
return new NodesReloadSecureSettingsResponse.NodeResponse(clusterService.localNode(), e);
138+
} finally {
139+
secureSettingsPassword.close();
110140
}
111141
}
112142

@@ -134,4 +164,20 @@ public void writeTo(StreamOutput out) throws IOException {
134164
request.writeTo(out);
135165
}
136166
}
167+
168+
/**
169+
* Returns true if the node is configured for TLS on the transport layer
170+
*/
171+
private boolean isNodeTransportTLSEnabled() {
172+
return transportService.isTransportSecure();
173+
}
174+
175+
private boolean isNodeLocal(NodesReloadSecureSettingsRequest request) {
176+
if (null == request.concreteNodes()) {
177+
resolveRequest(request, clusterService.state());
178+
assert request.concreteNodes() != null;
179+
}
180+
final DiscoveryNode[] nodes = request.concreteNodes();
181+
return nodes.length == 1 && nodes[0].getId().equals(clusterService.localNode().getId());
182+
}
137183
}

server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestReloadSecureSettingsAction.java

+27-11
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,11 @@
2323
import org.elasticsearch.action.admin.cluster.node.reload.NodesReloadSecureSettingsRequestBuilder;
2424
import org.elasticsearch.action.admin.cluster.node.reload.NodesReloadSecureSettingsResponse;
2525
import org.elasticsearch.client.node.NodeClient;
26+
import org.elasticsearch.common.ParseField;
2627
import org.elasticsearch.common.Strings;
28+
import org.elasticsearch.common.settings.SecureString;
2729
import org.elasticsearch.common.settings.Settings;
30+
import org.elasticsearch.common.xcontent.ObjectParser;
2831
import org.elasticsearch.common.xcontent.XContentBuilder;
2932
import org.elasticsearch.rest.BaseRestHandler;
3033
import org.elasticsearch.rest.BytesRestResponse;
@@ -41,6 +44,14 @@
4144

4245
public final class RestReloadSecureSettingsAction extends BaseRestHandler {
4346

47+
static final ObjectParser<NodesReloadSecureSettingsRequest, String> PARSER =
48+
new ObjectParser<>("reload_secure_settings", NodesReloadSecureSettingsRequest::new);
49+
50+
static {
51+
PARSER.declareString((request, value) -> request.setSecureStorePassword(new SecureString(value.toCharArray())),
52+
new ParseField("secure_settings_password"));
53+
}
54+
4455
public RestReloadSecureSettingsAction(Settings settings, RestController controller) {
4556
super(settings);
4657
controller.registerHandler(POST, "/_nodes/reload_secure_settings", this);
@@ -56,23 +67,28 @@ public String getName() {
5667
public RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException {
5768
final String[] nodesIds = Strings.splitStringByCommaToArray(request.param("nodeId"));
5869
final NodesReloadSecureSettingsRequestBuilder nodesRequestBuilder = client.admin()
59-
.cluster()
60-
.prepareReloadSecureSettings()
61-
.setTimeout(request.param("timeout"))
62-
.setNodesIds(nodesIds);
63-
final NodesReloadSecureSettingsRequest nodesRequest = nodesRequestBuilder.request();
70+
.cluster()
71+
.prepareReloadSecureSettings()
72+
.setTimeout(request.param("timeout"))
73+
.setNodesIds(nodesIds);
74+
request.withContentOrSourceParamParserOrNull(parser -> {
75+
if (parser != null) {
76+
final NodesReloadSecureSettingsRequest nodesRequest = PARSER.parse(parser, null);
77+
nodesRequestBuilder.setSecureStorePassword(nodesRequest.getSecureSettingsPassword());
78+
}
79+
});
80+
6481
return channel -> nodesRequestBuilder
6582
.execute(new RestBuilderListener<NodesReloadSecureSettingsResponse>(channel) {
6683
@Override
6784
public RestResponse buildResponse(NodesReloadSecureSettingsResponse response, XContentBuilder builder)
68-
throws Exception {
85+
throws Exception {
6986
builder.startObject();
70-
{
71-
RestActions.buildNodesHeader(builder, channel.request(), response);
72-
builder.field("cluster_name", response.getClusterName().value());
73-
response.toXContent(builder, channel.request());
74-
}
87+
RestActions.buildNodesHeader(builder, channel.request(), response);
88+
builder.field("cluster_name", response.getClusterName().value());
89+
response.toXContent(builder, channel.request());
7590
builder.endObject();
91+
nodesRequestBuilder.request().closePassword();
7692
return new BytesRestResponse(RestStatus.OK, builder);
7793
}
7894
});

server/src/main/java/org/elasticsearch/transport/Transport.java

+4
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,10 @@ public interface Transport extends LifecycleComponent {
5353

5454
void setMessageListener(TransportMessageListener listener);
5555

56+
default boolean isSecure() {
57+
return false;
58+
}
59+
5660
/**
5761
* The address the transport is bound on.
5862
*/

server/src/main/java/org/elasticsearch/transport/TransportService.java

+4
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,10 @@ public TransportStats stats() {
301301
return transport.getStats();
302302
}
303303

304+
public boolean isTransportSecure() {
305+
return transport.isSecure();
306+
}
307+
304308
public BoundTransportAddress boundAddress() {
305309
return transport.boundAddress();
306310
}

0 commit comments

Comments
 (0)