Prevent cluster internal ClusterState.Custom impls to leak to a cli…

…ent (#26232)

Today a `ClusterState.Custom` can be fetched by a transport client and
leaks to the user even if the classes are private etc since the serialized
bytes can be reconstructed. This change adds an option to customs to mark
them as private such that our clusterstate action will never leak it.

core/src/main/java/org/elasticsearch/action/admin/cluster/state/ClusterStateRequestBuilder.java

@@ -68,6 +68,15 @@ public ClusterStateRequestBuilder setNodes(boolean filter) {
         return this;
     }
 
+    /**
+     * Should the cluster state result include the {@link org.elasticsearch.cluster.ClusterState.Custom}. Defaults
+     * to <tt>true</tt>.
+     */
+    public ClusterStateRequestBuilder setCustoms(boolean filter) {
+        request.customs(filter);
+        return this;
+    }
+
     /**
      * Should the cluster state result include the {@link org.elasticsearch.cluster.routing.RoutingTable}. Defaults
      * to <tt>true</tt>.

core/src/main/java/org/elasticsearch/action/admin/cluster/state/TransportClusterStateAction.java

@@ -126,7 +126,11 @@ protected void masterOperation(final ClusterStateRequest request, final ClusterS
             builder.metaData(mdBuilder);
         }
         if (request.customs()) {
-            builder.customs(currentState.customs());
+            for (ObjectObjectCursor<String, ClusterState.Custom> custom : currentState.customs()) {
+                if (custom.value.isPrivate() == false) {
+                    builder.putCustom(custom.key, custom.value);
+                }
+            }
         }
         listener.onResponse(new ClusterStateResponse(currentState.getClusterName(), builder.build(),
                                                         serializeFullClusterState(currentState, Version.CURRENT).length()));

core/src/main/java/org/elasticsearch/cluster/ClusterState.java

@@ -92,6 +92,14 @@ public class ClusterState implements ToXContentFragment, Diffable<ClusterState>
     public static final ClusterState EMPTY_STATE = builder(ClusterName.CLUSTER_NAME_SETTING.getDefault(Settings.EMPTY)).build();
 
     public interface Custom extends NamedDiffable<Custom>, ToXContent {
+
+        /**
+         * Returns <code>true</code> iff this {@link Custom} is private to the cluster and should never be send to a client.
+         * The default is <code>false</code>;
+         */
+        default boolean isPrivate() {
+            return false;
+        }
     }
 
     private static final NamedDiffableValueSerializer<Custom> CUSTOM_VALUE_SERIALIZER = new NamedDiffableValueSerializer<>(Custom.class);

core/src/test/java/org/elasticsearch/cluster/SimpleClusterStateIT.java

@@ -28,19 +28,32 @@
 import org.elasticsearch.cluster.metadata.MappingMetaData;
 import org.elasticsearch.cluster.metadata.MetaData;
 import org.elasticsearch.cluster.routing.RoutingTable;
+import org.elasticsearch.cluster.service.ClusterService;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.UUIDs;
 import org.elasticsearch.common.collect.ImmutableOpenMap;
+import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
+import org.elasticsearch.common.io.stream.StreamInput;
+import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.unit.ByteSizeValue;
+import org.elasticsearch.common.xcontent.ToXContent;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentFactory;
 import org.elasticsearch.index.IndexNotFoundException;
 import org.elasticsearch.index.mapper.MapperService;
+import org.elasticsearch.plugins.ClusterPlugin;
+import org.elasticsearch.plugins.Plugin;
 import org.elasticsearch.test.ESIntegTestCase;
 import org.elasticsearch.test.hamcrest.CollectionAssertions;
 import org.junit.Before;
 
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Collection;
 import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.function.Supplier;
 
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertIndexTemplateExists;
@@ -54,6 +67,11 @@
  */
 public class SimpleClusterStateIT extends ESIntegTestCase {
 
+    @Override
+    protected Collection<Class<? extends Plugin>> nodePlugins() {
+        return Collections.singletonList(PrivateCustomPlugin.class);
+    }
+
     @Before
     public void indexData() throws Exception {
         index("foo", "bar", "1", XContentFactory.jsonBuilder().startObject().field("foo", "foo").endObject());
@@ -258,4 +276,67 @@ public void testIndicesIgnoreUnavailableFalse() throws Exception {
             assertThat(e.getMessage(), is("no such index"));
         }
     }
+
+    public void testPrivateCustomsAreExcluded() {
+        ClusterStateResponse clusterStateResponse = client().admin().cluster().prepareState().setCustoms(true).get();
+        assertFalse(clusterStateResponse.getState().customs().containsKey("test"));
+        // just to make sure there is something
+        assertTrue(clusterStateResponse.getState().customs().containsKey(SnapshotDeletionsInProgress.TYPE));
+        ClusterState state = internalCluster().getInstance(ClusterService.class).state();
+        assertTrue(state.customs().containsKey("test"));
+    }
+
+    private static class TestCustom extends AbstractNamedDiffable<ClusterState.Custom> implements ClusterState.Custom {
+
+        private final int value;
+
+        TestCustom(int value) {
+            this.value = value;
+        }
+
+        TestCustom(StreamInput in) throws IOException {
+            this.value = in.readInt();
+        }
+        @Override
+        public String getWriteableName() {
+            return "test";
+        }
+
+        @Override
+        public void writeTo(StreamOutput out) throws IOException {
+            out.writeInt(value);
+        }
+
+        @Override
+        public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
+            return builder;
+        }
+
+        static NamedDiff<ClusterState.Custom> readDiffFrom(StreamInput in) throws IOException {
+            return readDiffFrom(ClusterState.Custom.class, "test", in);
+        }
+
+        @Override
+        public boolean isPrivate() {
+            return true;
+        }
+    }
+
+    public static class PrivateCustomPlugin extends Plugin implements ClusterPlugin {
+
+        public PrivateCustomPlugin() {}
+
+        @Override
+        public Map<String, Supplier<ClusterState.Custom>> getInitialClusterStateCustomSupplier() {
+            return Collections.singletonMap("test", () -> new TestCustom(1));
+        }
+
+        @Override
+        public List<NamedWriteableRegistry.Entry> getNamedWriteables() {
+            List<NamedWriteableRegistry.Entry> entries = new ArrayList<>();
+            entries.add(new NamedWriteableRegistry.Entry(ClusterState.Custom.class, "test", TestCustom::new));
+            entries.add(new NamedWriteableRegistry.Entry(NamedDiff.class, "test", TestCustom::readDiffFrom));
+            return entries;
+        }
+    }
 }