Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions docs/changelog/138052.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 138052
summary: Include Secure Setting Names and Keystore Modified Time in Reload API Response
area: Security
type: enhancement
issues: []
Copy link
Contributor

Choose a reason for hiding this comment

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

This could reference #112268

Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.nullValue;

Expand Down Expand Up @@ -412,6 +413,10 @@ public void onResponse(NodesReloadSecureSettingsResponse nodesReloadResponse) {
assertThat(nodesMap.size(), equalTo(cluster().size()));
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think it matters but this test is disabled when running in fips mode. Since we're reading properties on the keystore we might want to make sure it works in fips mode too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The current approach seems adequate. I see that ReloadSecureSettingsWithPasswordProtectedKeystoreRestIT has a group of tests with a keystore password.

for (final NodesReloadSecureSettingsResponse.NodeResponse nodeResponse : nodesReloadResponse.getNodes()) {
assertThat(nodeResponse.reloadException(), nullValue());
assertThat(nodeResponse.keystorePath(), notNullValue());
assertThat(nodeResponse.keystoreDigest(), notNullValue());
assertThat(nodeResponse.keystoreLastModifiedTime(), notNullValue());
assertThat(nodeResponse.secureSettingNames(), is(new String[] { "keystore.seed" }));
}
} catch (final AssertionError e) {
reloadSettingsError.set(e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
package org.elasticsearch.action.admin.cluster.node.reload;

import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.TransportVersion;
import org.elasticsearch.action.FailedNodeException;
import org.elasticsearch.action.support.TransportAction;
import org.elasticsearch.action.support.nodes.BaseNodeResponse;
Expand All @@ -23,7 +24,11 @@
import org.elasticsearch.xcontent.XContentBuilder;

import java.io.IOException;
import java.time.Instant;
import java.util.Arrays;
import java.util.List;
import java.util.Objects;
import java.util.stream.Stream;

/**
* The response for the reload secure settings action
Expand Down Expand Up @@ -58,6 +63,22 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
ElasticsearchException.generateThrowableXContent(builder, params, e);
builder.endObject();
}
if (node.secureSettingNames() != null) {
builder.array("secure_setting_names", b -> {
for (String settingName : Stream.of(node.secureSettingNames()).sorted().toList()) {
b.value(settingName);
}
});
}
if (node.keystorePath() != null) {
builder.field("keystore_path", node.keystorePath());
}
if (node.keystoreDigest() != null) {
builder.field("keystore_digest", node.keystoreDigest());
}
if (node.keystoreLastModifiedTime() != null) {
builder.field("keystore_last_modified_time", Instant.ofEpochMilli(node.keystoreLastModifiedTime()));
}
builder.endObject();
}
builder.endObject();
Expand All @@ -71,32 +92,77 @@ public String toString() {

public static class NodeResponse extends BaseNodeResponse {

private Exception reloadException = null;
private static final TransportVersion KEYSTORE_DETAILS = TransportVersion.fromName(
"keystore_details_in_reload_secure_settings_response"
);

private final Exception reloadException;
private final String[] secureSettingNames;
private final String keystorePath;
private final String keystoreDigest;
private final Long keystoreLastModifiedTime;

public NodeResponse(StreamInput in) throws IOException {
super(in);
if (in.readBoolean()) {
reloadException = in.readException();
reloadException = in.readOptionalException();
if (in.getTransportVersion().onOrAfter(KEYSTORE_DETAILS)) {
secureSettingNames = in.readOptionalStringArray();
keystorePath = in.readOptionalString();
keystoreDigest = in.readOptionalString();
keystoreLastModifiedTime = in.readOptionalLong();
} else {
secureSettingNames = null;
keystorePath = null;
keystoreDigest = null;
keystoreLastModifiedTime = null;
}
}

public NodeResponse(DiscoveryNode node, Exception reloadException) {
public NodeResponse(
DiscoveryNode node,
Exception reloadException,
String[] secureSettingNames,
String keystorePath,
String keystoreDigest,
Long keystoreLastModifiedTime
) {
super(node);
this.reloadException = reloadException;
this.secureSettingNames = secureSettingNames;
this.keystorePath = keystorePath;
this.keystoreDigest = keystoreDigest;
this.keystoreLastModifiedTime = keystoreLastModifiedTime;
}

public Exception reloadException() {
return this.reloadException;
}

public String[] secureSettingNames() {
return this.secureSettingNames;
}

public String keystorePath() {
return this.keystorePath;
}

public String keystoreDigest() {
return this.keystoreDigest;
}

public Long keystoreLastModifiedTime() {
return this.keystoreLastModifiedTime;
}

@Override
public void writeTo(StreamOutput out) throws IOException {
super.writeTo(out);
if (reloadException != null) {
out.writeBoolean(true);
out.writeException(reloadException);
} else {
out.writeBoolean(false);
out.writeOptionalException(reloadException);
if (out.getTransportVersion().onOrAfter(KEYSTORE_DETAILS)) {
out.writeOptionalStringArray(secureSettingNames);
out.writeOptionalString(keystorePath);
out.writeOptionalString(keystoreDigest);
out.writeOptionalLong(keystoreLastModifiedTime);
}
}

Expand All @@ -109,12 +175,22 @@ public boolean equals(Object o) {
return false;
}
final NodesReloadSecureSettingsResponse.NodeResponse that = (NodesReloadSecureSettingsResponse.NodeResponse) o;
return reloadException != null ? reloadException.equals(that.reloadException) : that.reloadException == null;
return Objects.equals(reloadException, that.reloadException)
&& Arrays.equals(secureSettingNames, that.secureSettingNames)
&& Objects.equals(keystorePath, that.keystorePath)
&& Objects.equals(keystoreDigest, that.keystoreDigest)
&& Objects.equals(keystoreLastModifiedTime, that.keystoreLastModifiedTime);
}

@Override
public int hashCode() {
return reloadException != null ? reloadException.hashCode() : 0;
return Objects.hash(
reloadException,
Arrays.hashCode(secureSettingNames),
keystorePath,
keystoreDigest,
keystoreLastModifiedTime
);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import org.elasticsearch.cluster.ClusterState;
import org.elasticsearch.cluster.node.DiscoveryNode;
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.hash.MessageDigests;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.settings.KeyStoreWrapper;
import org.elasticsearch.common.settings.Settings;
Expand All @@ -32,6 +33,9 @@
import org.elasticsearch.transport.TransportService;

import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.attribute.BasicFileAttributes;
import java.util.ArrayList;
import java.util.List;

Expand Down Expand Up @@ -113,7 +117,11 @@ protected NodesReloadSecureSettingsResponse.NodeResponse nodeOperation(
if (keystore == null) {
return new NodesReloadSecureSettingsResponse.NodeResponse(
clusterService.localNode(),
new IllegalStateException("Keystore is missing")
new IllegalStateException("Keystore is missing"),
null,
null,
null,
null
);
}
// decrypt the keystore using the password from the request
Expand All @@ -134,9 +142,35 @@ protected NodesReloadSecureSettingsResponse.NodeResponse nodeOperation(
}
});
ExceptionsHelper.rethrowAndSuppress(exceptions);
return new NodesReloadSecureSettingsResponse.NodeResponse(clusterService.localNode(), null);
Path keystorePath = KeyStoreWrapper.keystorePath(environment.configDir());
return new NodesReloadSecureSettingsResponse.NodeResponse(
clusterService.localNode(),
null,
keystore.getSettingNames().toArray(String[]::new),
keystorePath.toString(),
failsafeSha256Digest(keystorePath),
failsafeLastModifiedTime(keystorePath)
);
} catch (final Exception e) {
return new NodesReloadSecureSettingsResponse.NodeResponse(clusterService.localNode(), e);
return new NodesReloadSecureSettingsResponse.NodeResponse(clusterService.localNode(), e, null, null, null, null);
}
}

private static Long failsafeLastModifiedTime(Path path) {
try {
return Files.readAttributes(path, BasicFileAttributes.class).lastModifiedTime().toMillis();
} catch (IOException e) {
logger.warn("Failed to read last modified time of [" + path + "]", e);
return null;
}
}

private static String failsafeSha256Digest(Path path) {
try {
return MessageDigests.toHexString(MessageDigests.sha256().digest(Files.readAllBytes(path)));
} catch (IOException e) {
logger.warn("Failed to compute SHA-256 digest of [" + path + "]", e);
return null;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1196,6 +1196,17 @@ public <T extends Exception> T readException() throws IOException {
return ElasticsearchException.readException(this);
}

/**
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice addition!

* Reads an optional {@link Exception}.
*/
@Nullable
public <T extends Exception> T readOptionalException() throws IOException {
if (readBoolean()) {
return ElasticsearchException.readException(this);
}
return null;
}

/**
* Get the registry of named writeables if this stream has one,
* {@code null} otherwise.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1091,6 +1091,18 @@ public void writeException(Throwable throwable) throws IOException {
ElasticsearchException.writeException(throwable, this);
}

/**
* Write an optional {@link Throwable} to the stream.
*/
public void writeOptionalException(@Nullable Throwable throwable) throws IOException {
if (throwable == null) {
writeBoolean(false);
} else {
writeBoolean(true);
writeException(throwable);
}
}

/**
* Writes a {@link NamedWriteable} to the current stream, by first writing its name and then the object itself
*/
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
9220000
2 changes: 1 addition & 1 deletion server/src/main/resources/transport/upper_bounds/9.3.csv
Original file line number Diff line number Diff line change
@@ -1 +1 @@
search_project_routing,9219000
keystore_details_in_reload_secure_settings_response,9220000
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,20 @@ private <T extends Exception> T serialize(T exception, TransportVersion version)
return in.readException();
}

private <T extends Exception> T serializeOptional(T exception) throws IOException {
return serializeOptional(exception, TransportVersionUtils.randomCompatibleVersion(random()));
}

private <T extends Exception> T serializeOptional(T exception, TransportVersion version) throws IOException {
BytesStreamOutput out = new BytesStreamOutput();
out.setTransportVersion(version);
out.writeOptionalException(exception);

StreamInput in = out.bytes().streamInput();
in.setTransportVersion(version);
return in.readOptionalException();
}

public void testIllegalShardRoutingStateException() throws IOException {
final ShardRouting routing = newShardRouting("test", 0, "xyz", false, ShardRoutingState.STARTED);
final String routingAsString = routing.toString();
Expand Down Expand Up @@ -301,6 +315,14 @@ public void testSearchException() throws IOException {
assertTrue(ex.getCause() instanceof NullPointerException);
}

public void testOptionalSearchException() throws IOException {
SearchException ex = serializeOptional(new SearchException(null, "hello world", new NullPointerException()));
assertNull(ex.shard());
assertEquals(ex.getMessage(), "hello world");
assertTrue(ex.getCause() instanceof NullPointerException);
assertNull(serializeOptional(null));
}

public void testActionNotFoundTransportException() throws IOException {
ActionNotFoundTransportException ex = serialize(new ActionNotFoundTransportException("AACCCTION"));
assertEquals("AACCCTION", ex.action());
Expand Down
Loading