Browse Source

Add "simple match" support for reindex-from-remote whitelist

This allows you to whitelist `localhost:*` or `127.0.10.*:9200`.
It explicitly checks for patterns like `*` in the whitelist and
refuses to start if the whitelist would match everything. Beyond
that the user is on their own designing a secure whitelist.
Nik Everett 9 years ago
parent
commit
acf7c7430b

+ 1 - 1
docs/build.gradle

@@ -178,7 +178,7 @@ integTest {
     configFile 'userdict_ja.txt'
     configFile 'KeywordTokenizer.rbbi'
     // Whitelist reindexing from the local node so we can test it.
-    setting 'reindex.remote.whitelist', 'myself'
+    setting 'reindex.remote.whitelist', '127.0.0.1:*'
   }
 }
 

+ 2 - 2
docs/reference/docs/reindex.asciidoc

@@ -410,8 +410,8 @@ basic auth or the password will be sent in plain text.
 Remote hosts have to be explicitly whitelisted in elasticsearch.yaml using the
 `reindex.remote.whitelist` property. It can be set to a comma delimited list
 of allowed remote `host` and `port` combinations (e.g.
-`otherhost:9200, another:9200`). Scheme is ignored by the whitelist - only host
-and port are used.
+`otherhost:9200, another:9200, 127.0.10.*:9200, localhost:*`). Scheme is
+ignored by the whitelist - only host and port are used.
 
 This feature should work with remote clusters of any version of Elasticsearch
 you are likely to find. This should allow you to upgrade from any version of

+ 2 - 2
modules/reindex/build.gradle

@@ -26,13 +26,13 @@ esplugin {
 integTest {
   cluster {
     // Whitelist reindexing from the local node so we can test it.
-    setting 'reindex.remote.whitelist', 'myself'
+    setting 'reindex.remote.whitelist', '127.0.0.1:*'
   }
 }
 
 run {
   // Whitelist reindexing from the local node so we can test it.
-  setting 'reindex.remote.whitelist', 'myself'
+  setting 'reindex.remote.whitelist', '127.0.0.1:*'
 }
 
 dependencies {

+ 32 - 14
modules/reindex/src/main/java/org/elasticsearch/index/reindex/TransportReindexAction.java

@@ -28,6 +28,11 @@ import org.apache.http.impl.client.BasicCredentialsProvider;
 import org.apache.http.impl.nio.reactor.IOReactorConfig;
 import org.apache.http.message.BasicHeader;
 import org.apache.logging.log4j.Logger;
+import org.apache.lucene.util.automaton.Automata;
+import org.apache.lucene.util.automaton.Automaton;
+import org.apache.lucene.util.automaton.CharacterRunAutomaton;
+import org.apache.lucene.util.automaton.MinimizationOperations;
+import org.apache.lucene.util.automaton.Operations;
 import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.action.ActionRequestValidationException;
 import org.elasticsearch.action.bulk.BackoffPolicy;
@@ -44,8 +49,10 @@ import org.elasticsearch.cluster.ClusterState;
 import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
 import org.elasticsearch.cluster.service.ClusterService;
 import org.elasticsearch.common.Nullable;
+import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.common.lucene.uid.Versions;
+import org.elasticsearch.common.regex.Regex;
 import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.settings.Setting.Property;
 import org.elasticsearch.common.settings.Settings;
@@ -65,11 +72,9 @@ import org.elasticsearch.threadpool.ThreadPool;
 import org.elasticsearch.transport.TransportService;
 
 import java.util.ArrayList;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
-import java.util.Set;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.function.BiFunction;
 import java.util.function.Function;
@@ -87,7 +92,7 @@ public class TransportReindexAction extends HandledTransportAction<ReindexReques
     private final ScriptService scriptService;
     private final AutoCreateIndex autoCreateIndex;
     private final Client client;
-    private final Set<String> remoteWhitelist;
+    private final CharacterRunAutomaton remoteWhitelist;
     private final HttpServer httpServer;
 
     @Inject
@@ -100,7 +105,7 @@ public class TransportReindexAction extends HandledTransportAction<ReindexReques
         this.scriptService = scriptService;
         this.autoCreateIndex = autoCreateIndex;
         this.client = client;
-        remoteWhitelist = new HashSet<>(REMOTE_CLUSTER_WHITELIST.get(settings));
+        remoteWhitelist = buildRemoteWhitelist(REMOTE_CLUSTER_WHITELIST.get(settings));
         this.httpServer = httpServer;
     }
 
@@ -128,22 +133,35 @@ public class TransportReindexAction extends HandledTransportAction<ReindexReques
         checkRemoteWhitelist(remoteWhitelist, remoteInfo, publishAddress);
     }
 
-    static void checkRemoteWhitelist(Set<String> whitelist, RemoteInfo remoteInfo, TransportAddress publishAddress) {
-        if (remoteInfo == null) return;
+    static void checkRemoteWhitelist(CharacterRunAutomaton whitelist, RemoteInfo remoteInfo, TransportAddress publishAddress) {
+        if (remoteInfo == null) {
+            return;
+        }
         String check = remoteInfo.getHost() + ':' + remoteInfo.getPort();
-        if (whitelist.contains(check)) return;
-        /*
-         * For testing we support the key "myself" to allow connecting to the local node. We can't just change the setting to include the
-         * local node because it is intentionally not a dynamic setting for security purposes. We can't use something like "localhost:9200"
-         * because we don't know up front which port we'll get because the tests bind to port 0. Instead we try to resolve it here, taking
-         * "myself" to mean "my published http address".
-         */
-        if (whitelist.contains("myself") && publishAddress != null && publishAddress.toString().equals(check)) {
+        if (whitelist.run(check)) {
             return;
         }
         throw new IllegalArgumentException('[' + check + "] not whitelisted in " + REMOTE_CLUSTER_WHITELIST.getKey());
     }
 
+    /**
+     * Build the {@link CharacterRunAutomaton} that represents the reindex-from-remote whitelist and make sure that it doesn't whitelist
+     * the world.
+     */
+    static CharacterRunAutomaton buildRemoteWhitelist(List<String> whitelist) {
+        if (whitelist.isEmpty()) {
+            return new CharacterRunAutomaton(Automata.makeEmpty());
+        }
+        Automaton automaton = Regex.simpleMatchToAutomaton(whitelist.toArray(Strings.EMPTY_ARRAY));
+        automaton = MinimizationOperations.minimize(automaton, Operations.DEFAULT_MAX_DETERMINIZED_STATES);
+        if (Operations.isTotal(automaton)) {
+            throw new IllegalArgumentException("Refusing to start because whitelist " + whitelist + " accepts all addresses. "
+                    + "This would allow users to reindex-from-remote any URL they like effectively having Elasticsearch make HTTP GETs "
+                    + "for them.");
+        }
+        return new CharacterRunAutomaton(automaton);
+    }
+
     /**
      * Throws an ActionRequestValidationException if the request tries to index
      * back into the same index or into an index that points to two indexes.

+ 65 - 18
modules/reindex/src/test/java/org/elasticsearch/index/reindex/ReindexFromRemoteWhitelistTests.java

@@ -27,11 +27,14 @@ import org.junit.Before;
 
 import java.net.InetAddress;
 import java.net.UnknownHostException;
-import java.util.HashSet;
-import java.util.Set;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
 
+import static java.util.Collections.emptyList;
 import static java.util.Collections.emptyMap;
-import static java.util.Collections.emptySet;
+import static java.util.Collections.singletonList;
+import static org.elasticsearch.index.reindex.TransportReindexAction.buildRemoteWhitelist;
 import static org.elasticsearch.index.reindex.TransportReindexAction.checkRemoteWhitelist;
 
 /**
@@ -46,45 +49,89 @@ public class ReindexFromRemoteWhitelistTests extends ESTestCase {
     }
 
     public void testLocalRequestWithoutWhitelist() {
-        checkRemoteWhitelist(emptySet(), null, localhostOrNone());
+        checkRemoteWhitelist(buildRemoteWhitelist(emptyList()), null, localhostOrNone());
     }
 
     public void testLocalRequestWithWhitelist() {
-        checkRemoteWhitelist(randomWhitelist(), null, localhostOrNone());
+        checkRemoteWhitelist(buildRemoteWhitelist(randomWhitelist()), null, localhostOrNone());
     }
 
     public void testWhitelistedRemote() {
-        Set<String> whitelist = randomWhitelist();
+        List<String> whitelist = randomWhitelist();
         String[] inList = whitelist.iterator().next().split(":");
         String host = inList[0];
         int port = Integer.valueOf(inList[1]);
-        checkRemoteWhitelist(whitelist, new RemoteInfo(randomAsciiOfLength(5), host, port, new BytesArray("test"), null, null, emptyMap()),
+        checkRemoteWhitelist(buildRemoteWhitelist(whitelist),
+                new RemoteInfo(randomAsciiOfLength(5), host, port, new BytesArray("test"), null, null, emptyMap()),
                 localhostOrNone());
     }
 
-    public void testMyselfInWhitelistRemote() throws UnknownHostException {
-        Set<String> whitelist = randomWhitelist();
-        whitelist.add("myself");
+    public void testWhitelistedByPrefix() {
+        checkRemoteWhitelist(buildRemoteWhitelist(singletonList("*.example.com:9200")),
+                new RemoteInfo(randomAsciiOfLength(5), "es.example.com", 9200, new BytesArray("test"), null, null, emptyMap()),
+                localhostOrNone());
+        checkRemoteWhitelist(buildRemoteWhitelist(singletonList("*.example.com:9200")),
+                new RemoteInfo(randomAsciiOfLength(5), "6e134134a1.us-east-1.aws.example.com", 9200,
+                        new BytesArray("test"), null, null, emptyMap()),
+                localhostOrNone());
+    }
+
+    public void testWhitelistedBySuffix() {
+        checkRemoteWhitelist(buildRemoteWhitelist(singletonList("es.example.com:*")),
+                new RemoteInfo(randomAsciiOfLength(5), "es.example.com", 9200, new BytesArray("test"), null, null, emptyMap()),
+                localhostOrNone());
+    }
+
+    public void testWhitelistedByInfix() {
+        checkRemoteWhitelist(buildRemoteWhitelist(singletonList("es*.example.com:9200")),
+                new RemoteInfo(randomAsciiOfLength(5), "es1.example.com", 9200, new BytesArray("test"), null, null, emptyMap()),
+                localhostOrNone());
+    }
+
+
+    public void testLoopbackInWhitelistRemote() throws UnknownHostException {
+        List<String> whitelist = randomWhitelist();
+        whitelist.add("127.0.0.1:*");
         TransportAddress publishAddress = new TransportAddress(InetAddress.getByAddress(new byte[] {0x7f,0x00,0x00,0x01}), 9200);
-        checkRemoteWhitelist(whitelist,
-                new RemoteInfo(randomAsciiOfLength(5), "127.0.0.1", 9200, new BytesArray("test"), null, null, emptyMap()), publishAddress);
+        checkRemoteWhitelist(buildRemoteWhitelist(whitelist),
+                new RemoteInfo(randomAsciiOfLength(5), "127.0.0.1", 9200, new BytesArray("test"), null, null, emptyMap()),
+                publishAddress);
     }
 
     public void testUnwhitelistedRemote() {
         int port = between(1, Integer.MAX_VALUE);
         RemoteInfo remoteInfo = new RemoteInfo(randomAsciiOfLength(5), "not in list", port, new BytesArray("test"), null, null, emptyMap());
+        List<String> whitelist = randomBoolean() ? randomWhitelist() : emptyList();
         Exception e = expectThrows(IllegalArgumentException.class,
-                () -> checkRemoteWhitelist(randomWhitelist(), remoteInfo, localhostOrNone()));
+                () -> checkRemoteWhitelist(buildRemoteWhitelist(whitelist), remoteInfo, localhostOrNone()));
         assertEquals("[not in list:" + port + "] not whitelisted in reindex.remote.whitelist", e.getMessage());
     }
 
-    private Set<String> randomWhitelist() {
+    public void testRejectMatchAll() {
+        assertMatchesTooMuch(singletonList("*"));
+        assertMatchesTooMuch(singletonList("**"));
+        assertMatchesTooMuch(singletonList("***"));
+        assertMatchesTooMuch(Arrays.asList("realstuff", "*"));
+        assertMatchesTooMuch(Arrays.asList("*", "realstuff"));
+        List<String> random = randomWhitelist();
+        random.add("*");
+        assertMatchesTooMuch(random);
+    }
+
+    private void assertMatchesTooMuch(List<String> whitelist) {
+        Exception e = expectThrows(IllegalArgumentException.class, () -> buildRemoteWhitelist(whitelist));
+        assertEquals("Refusing to start because whitelist " + whitelist + " accepts all addresses. "
+                + "This would allow users to reindex-from-remote any URL they like effectively having Elasticsearch make HTTP GETs "
+                + "for them.", e.getMessage());
+    }
+
+    private List<String> randomWhitelist() {
         int size = between(1, 100);
-        Set<String> set = new HashSet<>(size);
-        while (set.size() < size) {
-            set.add(randomAsciiOfLength(5) + ':' + between(1, Integer.MAX_VALUE));
+        List<String> whitelist = new ArrayList<>(size);
+        for (int i = 0; i < size; i++) {
+            whitelist.add(randomAsciiOfLength(5) + ':' + between(1, Integer.MAX_VALUE));
         }
-        return set;
+        return whitelist;
     }
 
     private TransportAddress localhostOrNone() {

+ 1 - 1
modules/reindex/src/test/java/org/elasticsearch/index/reindex/ReindexFromRemoteWithAuthTests.java

@@ -72,7 +72,7 @@ public class ReindexFromRemoteWithAuthTests extends ESSingleNodeTestCase {
         // Weird incantation required to test with netty
         settings.put(NetworkModule.HTTP_ENABLED.getKey(), true);
         // Whitelist reindexing from the http host we're going to use
-        settings.put(TransportReindexAction.REMOTE_CLUSTER_WHITELIST.getKey(), "myself");
+        settings.put(TransportReindexAction.REMOTE_CLUSTER_WHITELIST.getKey(), "127.0.0.1:*");
         settings.put(NetworkModule.HTTP_TYPE_KEY, Netty4Plugin.NETTY_HTTP_TRANSPORT_NAME);
         return settings.build();
     }

+ 1 - 1
modules/reindex/src/test/java/org/elasticsearch/index/reindex/RetryTests.java

@@ -109,7 +109,7 @@ public class RetryTests extends ESSingleNodeTestCase {
         // Enable http so we can test retries on reindex from remote. In this case the "remote" cluster is just this cluster.
         settings.put(NetworkModule.HTTP_ENABLED.getKey(), true);
         // Whitelist reindexing from the http host we're going to use
-        settings.put(TransportReindexAction.REMOTE_CLUSTER_WHITELIST.getKey(), "myself");
+        settings.put(TransportReindexAction.REMOTE_CLUSTER_WHITELIST.getKey(), "127.0.0.1:*");
         if (useNetty3) {
             settings.put(NetworkModule.HTTP_TYPE_KEY, Netty3Plugin.NETTY_HTTP_TRANSPORT_NAME);
             settings.put(NetworkModule.TRANSPORT_TYPE_KEY, Netty3Plugin.NETTY_TRANSPORT_NAME);