Nodes reload request store password as SecureString

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

@@ -19,11 +19,18 @@
 
 package org.elasticsearch.action.admin.cluster.node.reload;
 
+
 import org.elasticsearch.action.ActionRequestValidationException;
 import org.elasticsearch.action.support.nodes.BaseNodesRequest;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
+import org.elasticsearch.common.settings.SecureString;
+
 import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.nio.CharBuffer;
+import java.nio.charset.StandardCharsets;
+import java.util.Arrays;
 
 import static org.elasticsearch.action.ValidateActions.addValidationError;
 
@@ -38,7 +45,7 @@ public class NodesReloadSecureSettingsRequest extends BaseNodesRequest<NodesRelo
      * of the node's keystore (backing the implementation of
      * {@code SecureSettings}).
      */
-    private String secureSettingsPassword;
+    private SecureString secureSettingsPassword;
 
     public NodesReloadSecureSettingsRequest() {
     }
@@ -62,24 +69,92 @@ public ActionRequestValidationException validate() {
         return validationException;
     }
 
-    public String secureSettingsPassword() {
+    public SecureString secureSettingsPassword() {
         return secureSettingsPassword;
     }
 
-    public NodesReloadSecureSettingsRequest secureStorePassword(String secureStorePassword) {
+    public NodesReloadSecureSettingsRequest secureStorePassword(SecureString secureStorePassword) {
         this.secureSettingsPassword = secureStorePassword;
         return this;
     }
 
     @Override
     public void readFrom(StreamInput in) throws IOException {
         super.readFrom(in);
-        secureSettingsPassword = in.readString();
+        final byte[] passwordBytes = in.readByteArray();
+        try {
+            this.secureSettingsPassword = new SecureString(utf8BytesToChars(passwordBytes));
+        } finally {
+            Arrays.fill(passwordBytes, (byte) 0);
+        }
     }
 
     @Override
     public void writeTo(StreamOutput out) throws IOException {
         super.writeTo(out);
-        out.writeString(secureSettingsPassword);
+        final byte[] passwordBytes = charsToUtf8Bytes(this.secureSettingsPassword.getChars());
+        try {
+            out.writeByteArray(passwordBytes);
+        } finally {
+            Arrays.fill(passwordBytes, (byte) 0);
+        }
+    }
+
+    /**
+     * Encodes the provided char[] to a UTF-8 byte[]. This is done while avoiding
+     * conversions to String. The provided char[] is not modified by this method, so
+     * the caller needs to take care of clearing the value if it is sensitive.
+     */
+    private static byte[] charsToUtf8Bytes(char[] chars) {
+        final CharBuffer charBuffer = CharBuffer.wrap(chars);
+        final ByteBuffer byteBuffer = StandardCharsets.UTF_8.encode(charBuffer);
+        final byte[] bytes;
+        if (byteBuffer.hasArray()) {
+            // there is no guarantee that the byte buffers backing array is the right size
+            // so we need to make a copy
+            bytes = Arrays.copyOfRange(byteBuffer.array(), byteBuffer.position(), byteBuffer.limit());
+            Arrays.fill(byteBuffer.array(), (byte) 0); // clear sensitive data
+        } else {
+            final int length = byteBuffer.limit() - byteBuffer.position();
+            bytes = new byte[length];
+            byteBuffer.get(bytes);
+            // if the buffer is not read only we can reset and fill with 0's
+            if (byteBuffer.isReadOnly() == false) {
+                byteBuffer.clear(); // reset
+                for (int i = 0; i < byteBuffer.limit(); i++) {
+                    byteBuffer.put((byte) 0);
+                }
+            }
+        }
+        return bytes;
+    }
+
+    /**
+     * Decodes the provided byte[] to a UTF-8 char[]. This is done while avoiding
+     * conversions to String. The provided byte[] is not modified by this method, so
+     * the caller needs to take care of clearing the value if it is sensitive.
+     */
+    public static char[] utf8BytesToChars(byte[] utf8Bytes) {
+        final ByteBuffer byteBuffer = ByteBuffer.wrap(utf8Bytes);
+        final CharBuffer charBuffer = StandardCharsets.UTF_8.decode(byteBuffer);
+        final char[] chars;
+        if (charBuffer.hasArray()) {
+            // there is no guarantee that the char buffers backing array is the right size
+            // so we need to make a copy
+            chars = Arrays.copyOfRange(charBuffer.array(), charBuffer.position(), charBuffer.limit());
+            Arrays.fill(charBuffer.array(), (char) 0); // clear sensitive data
+        } else {
+            final int length = charBuffer.limit() - charBuffer.position();
+            chars = new char[length];
+            charBuffer.get(chars);
+            // if the buffer is not read only we can reset and fill with 0's
+            if (charBuffer.isReadOnly() == false) {
+                charBuffer.clear(); // reset
+                for (int i = 0; i < charBuffer.limit(); i++) {
+                    charBuffer.put((char) 0);
+                }
+            }
+        }
+        return chars;
     }
 }

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

@@ -19,22 +19,66 @@
 
 package org.elasticsearch.action.admin.cluster.node.reload;
 
+import org.elasticsearch.ElasticsearchParseException;
 import org.elasticsearch.action.support.nodes.NodesOperationRequestBuilder;
 import org.elasticsearch.client.ElasticsearchClient;
+import org.elasticsearch.common.bytes.BytesReference;
+import org.elasticsearch.common.settings.SecureString;
+import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
+import org.elasticsearch.common.xcontent.NamedXContentRegistry;
+import org.elasticsearch.common.xcontent.XContentParser;
+import org.elasticsearch.common.xcontent.XContentType;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.util.Objects;
 
 /**
  * Builder for the reload secure settings nodes request
  */
 public class NodesReloadSecureSettingsRequestBuilder extends NodesOperationRequestBuilder<NodesReloadSecureSettingsRequest,
         NodesReloadSecureSettingsResponse, NodesReloadSecureSettingsRequestBuilder> {
 
+    public static final String SECURE_SETTINGS_PASSWORD_FIELD_NAME = "secure_settings_password";
+
     public NodesReloadSecureSettingsRequestBuilder(ElasticsearchClient client, NodesReloadSecureSettingsAction action) {
         super(client, action, new NodesReloadSecureSettingsRequest());
     }
 
-    public NodesReloadSecureSettingsRequestBuilder setSecureStorePassword(String secureStorePassword) {
+    public NodesReloadSecureSettingsRequestBuilder setSecureStorePassword(SecureString secureStorePassword) {
         request.secureStorePassword(secureStorePassword);
         return this;
     }
 
+    public NodesReloadSecureSettingsRequestBuilder source(BytesReference source, XContentType xContentType) throws IOException {
+        Objects.requireNonNull(xContentType);
+        // EMPTY is ok here because we never call namedObject
+        try (final InputStream stream = source.streamInput();
+                final XContentParser parser = xContentType.xContent().createParser(NamedXContentRegistry.EMPTY,
+                        LoggingDeprecationHandler.INSTANCE, stream)) {
+            XContentParser.Token token;
+            token = parser.nextToken();
+            if (token != XContentParser.Token.START_OBJECT) {
+                throw new ElasticsearchParseException("expected an object, but found token [{}]", token);
+            }
+            token = parser.nextToken();
+            if (token != XContentParser.Token.FIELD_NAME || false == SECURE_SETTINGS_PASSWORD_FIELD_NAME.equals(parser.currentName())) {
+                throw new ElasticsearchParseException("expected a field named [{}], but found [{}]", SECURE_SETTINGS_PASSWORD_FIELD_NAME,
+                        token);
+            }
+            token = parser.nextToken();
+            if (token != XContentParser.Token.VALUE_STRING) {
+                throw new ElasticsearchParseException("expected field [{}] to be of type string, but found [{}] instead",
+                        SECURE_SETTINGS_PASSWORD_FIELD_NAME, token);
+            }
+            final String password = parser.text();
+            setSecureStorePassword(new SecureString(password.toCharArray()));
+            token = parser.nextToken();
+            if (token != XContentParser.Token.END_OBJECT) {
+                throw new ElasticsearchParseException("expected end of object, but found token [{}]", token);
+            }
+        }
+        return this;
+    }
+
 }

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

@@ -32,6 +32,7 @@
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.settings.KeyStoreWrapper;
+import org.elasticsearch.common.settings.SecureString;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.env.Environment;
 import org.elasticsearch.plugins.PluginsService;
@@ -84,15 +85,15 @@ protected NodesReloadSecureSettingsResponse.NodeResponse newNodeResponse() {
     protected NodesReloadSecureSettingsResponse.NodeResponse nodeOperation(NodeRequest nodeReloadRequest) {
         final NodesReloadSecureSettingsRequest request = nodeReloadRequest.request;
         KeyStoreWrapper keystore = null;
-        try {
+        try (final SecureString secureSettingsPassword = request.secureSettingsPassword()) {
             // reread keystore from config file
             keystore = KeyStoreWrapper.load(environment.configFile());
             if (keystore == null) {
                 return new NodesReloadSecureSettingsResponse.NodeResponse(clusterService.localNode(),
                         new IllegalStateException("Keystore is missing"));
             }
             // decrypt the keystore using the password from the request
-            keystore.decrypt(request.secureSettingsPassword().toCharArray());
+            keystore.decrypt(secureSettingsPassword.getChars());
             // add the keystore to the original node settings object
             final Settings settingsWithKeystore = Settings.builder()
                     .put(environment.settings(), false)

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

@@ -19,13 +19,21 @@
 
 package org.elasticsearch.rest.action.admin.cluster;
 
+import org.elasticsearch.action.admin.cluster.node.reload.NodesReloadSecureSettingsRequest;
+import org.elasticsearch.action.admin.cluster.node.reload.NodesReloadSecureSettingsRequestBuilder;
+import org.elasticsearch.action.admin.cluster.node.reload.NodesReloadSecureSettingsResponse;
 import org.elasticsearch.client.node.NodeClient;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.rest.BaseRestHandler;
+import org.elasticsearch.rest.BytesRestResponse;
 import org.elasticsearch.rest.RestController;
 import org.elasticsearch.rest.RestRequest;
-import org.elasticsearch.rest.action.RestActions.NodesResponseRestListener;
+import org.elasticsearch.rest.RestResponse;
+import org.elasticsearch.rest.RestStatus;
+import org.elasticsearch.rest.action.RestActions;
+import org.elasticsearch.rest.action.RestBuilderListener;
 
 import java.io.IOException;
 
@@ -47,13 +55,28 @@ public String getName() {
     @Override
     public RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException {
         final String[] nodesIds = Strings.splitStringByCommaToArray(request.param("nodeId"));
-        return channel -> client.admin()
+        final NodesReloadSecureSettingsRequestBuilder nodesRequestBuilder = client.admin()
                 .cluster()
                 .prepareReloadSecureSettings()
                 .setTimeout(request.param("timeout"))
-                .setNodesIds(nodesIds)
-                .setSecureStorePassword(request.param("secure_settings_password", ""))
-                .execute(new NodesResponseRestListener<>(channel));
+                .source(request.requiredContent(), request.getXContentType())
+                .setNodesIds(nodesIds);
+        final NodesReloadSecureSettingsRequest nodesRequest = nodesRequestBuilder.request();
+        return channel -> nodesRequestBuilder
+                .execute(new RestBuilderListener<NodesReloadSecureSettingsResponse>(channel) {
+                    @Override
+                    public RestResponse buildResponse(NodesReloadSecureSettingsResponse response, XContentBuilder builder)
+                            throws Exception {
+                        builder.startObject();
+                        RestActions.buildNodesHeader(builder, channel.request(), response);
+                        builder.field("cluster_name", response.getClusterName().value());
+                        response.toXContent(builder, channel.request());
+                        builder.endObject();
+                        // clear password for the original request
+                        nodesRequest.secureSettingsPassword().close();
+                        return new BytesRestResponse(RestStatus.OK, builder);
+                    }
+                });
     }
 
     @Override