consolidate security code in on place an allow test based on the jar dependency to opt out of netty internal property setting assertion

modules/reindex/src/test/java/org/elasticsearch/index/reindex/RetryTests.java

@@ -28,6 +28,7 @@ import org.elasticsearch.action.bulk.BulkResponse;
 import org.elasticsearch.action.bulk.Retry;
 import org.elasticsearch.common.bytes.BytesArray;
 import org.elasticsearch.common.network.NetworkModule;
+import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.transport.TransportAddress;
 import org.elasticsearch.common.util.concurrent.EsRejectedExecutionException;
@@ -41,6 +42,7 @@ import org.junit.Before;
 
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.List;
 import java.util.concurrent.CyclicBarrier;
 
@@ -60,7 +62,19 @@ public class RetryTests extends ESSingleNodeTestCase {
 
     @Override
     protected Collection<Class<? extends Plugin>> getPlugins() {
-        return pluginList(ReindexPlugin.class, NettyPlugin.class); // we need netty here to http communication
+        return pluginList(ReindexPlugin.class, NettyPlugin.class, BogusPlugin.class); // we need netty here to http communication
+    }
+
+    public static final class BogusPlugin extends Plugin {
+        // se NettyUtils.... this runs without the permission from the netty module so it will fail since reindex can't set the property
+        // to make it still work we disable that check but need to register the setting first
+        private static final Setting<Boolean> ASSERT_NETTY_BUGLEVEL = Setting.boolSetting("netty.assert.buglevel", true,
+            Setting.Property.NodeScope);
+
+        @Override
+        public List<Setting<?>> getSettings() {
+            return Collections.singletonList(ASSERT_NETTY_BUGLEVEL);
+        }
     }
 
     /**
@@ -70,6 +84,7 @@ public class RetryTests extends ESSingleNodeTestCase {
     protected Settings nodeSettings() {
         Settings.Builder settings = Settings.builder().put(super.nodeSettings());
         // Use pools of size 1 so we can block them
+        settings.put("netty.assert.buglevel", false);
         settings.put("thread_pool.bulk.size", 1);
         settings.put("thread_pool.search.size", 1);
         // Use queues of size 1 because size 0 is broken and because search requests need the queue to function
@@ -189,7 +204,7 @@ public class RetryTests extends ESSingleNodeTestCase {
                 barrier.await();
                 logger.info("Blocked the [{}] executor", name);
                 barrier.await();
-                logger.info("Ublocking the [{}] executor", name);
+                logger.info("Unblocking the [{}] executor", name);
             } catch (Exception e) {
                 throw new RuntimeException(e);
             }

modules/transport-netty/src/main/java/org/elasticsearch/http/netty/NettyHttpServerTransport.java

@@ -78,8 +78,6 @@ import org.jboss.netty.handler.timeout.ReadTimeoutException;
 import java.io.IOException;
 import java.net.InetAddress;
 import java.net.InetSocketAddress;
-import java.security.AccessController;
-import java.security.PrivilegedAction;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
@@ -112,9 +110,6 @@ import static org.elasticsearch.http.HttpTransportSettings.SETTING_PIPELINING;
 import static org.elasticsearch.http.HttpTransportSettings.SETTING_PIPELINING_MAX_EVENTS;
 import static org.elasticsearch.http.netty.cors.CorsHandler.ANY_ORIGIN;
 
-/**
- *
- */
 public class NettyHttpServerTransport extends AbstractLifecycleComponent implements HttpServerTransport {
 
     static {
@@ -286,8 +281,6 @@ public class NettyHttpServerTransport extends AbstractLifecycleComponent impleme
     @Override
     protected void doStart() {
         this.serverOpenChannels = new OpenChannelsHandler(logger);
-        // this doPrivileged is for SelectorUtil.java that tries to set "sun.nio.ch.bugLevel"
-        AccessController.doPrivileged((PrivilegedAction<Object>) () -> {
         if (blockingServer) {
             serverBootstrap = new ServerBootstrap(new OioServerSocketChannelFactory(
                 Executors.newCachedThreadPool(daemonThreadFactory(settings, "http_server_boss")),
@@ -299,10 +292,6 @@ public class NettyHttpServerTransport extends AbstractLifecycleComponent impleme
                 Executors.newCachedThreadPool(daemonThreadFactory(settings, "http_server_worker")),
                 workerCount));
         }
-                return null;
-            });
-        assert System.getProperty("sun.nio.ch.bugLevel") != null :
-            "sun.nio.ch.bugLevel is null somebody pulls in SelectorUtil without doing stuff in a doPrivileged block?";
         serverBootstrap.setPipelineFactory(configureServerChannelPipelineFactory());
 
         serverBootstrap.setOption("child.tcpNoDelay", tcpNoDelay);

modules/transport-netty/src/main/java/org/elasticsearch/transport/NettyPlugin.java

@@ -19,20 +19,50 @@
 
 package org.elasticsearch.transport;
 
+import org.elasticsearch.SpecialPermission;
 import org.elasticsearch.common.network.NetworkModule;
 import org.elasticsearch.common.settings.Setting;
+import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.http.netty.NettyHttpServerTransport;
 import org.elasticsearch.plugins.Plugin;
 import org.elasticsearch.transport.netty.NettyTransport;
 
+import java.security.AccessController;
+import java.security.PrivilegedAction;
 import java.util.Arrays;
 import java.util.List;
 
 public class NettyPlugin extends Plugin {
 
+
     public static final String NETTY_TRANSPORT_NAME = "netty";
     public static final String NETTY_HTTP_TRANSPORT_NAME = "netty";
 
+    public NettyPlugin(Settings settings) {
+        SecurityManager sm = System.getSecurityManager();
+        if (sm != null) {
+            sm.checkPermission(new SpecialPermission());
+        }
+        AccessController.doPrivileged((PrivilegedAction<Void>) () -> {
+            try {
+                Class.forName("org.jboss.netty.channel.socket.nio.SelectorUtil");
+            } catch (ClassNotFoundException e) {
+                throw new AssertionError(e); // we don't do anything with this
+            }
+            return null;
+        });
+        /*
+         * Asserts that sun.nio.ch.bugLevel has been set to a non-null value. This assertion will fail if the corresponding code
+         * is not executed in a doPrivileged block. This can be disabled via `netty.assert.buglevel` setting which isn't registered
+         * by default but test can do so if they depend on the jar instead of the module.
+         */
+        //TODO Once we have no jar level dependency we can get rid of this.
+        if (settings.getAsBoolean("netty.assert.buglevel", true)) {
+            assert System.getProperty("sun.nio.ch.bugLevel") != null :
+                "sun.nio.ch.bugLevel is null somebody pulls in SelectorUtil without doing stuff in a doPrivileged block?";
+        }
+    }
+
     @Override
     public List<Setting<?>> getSettings() {
         return Arrays.asList(

modules/transport-netty/src/main/java/org/elasticsearch/transport/netty/NettyTransport.java

@@ -65,8 +65,6 @@ import org.jboss.netty.util.HashedWheelTimer;
 
 import java.io.IOException;
 import java.net.InetSocketAddress;
-import java.security.AccessController;
-import java.security.PrivilegedAction;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Iterator;
@@ -185,7 +183,6 @@ public class NettyTransport extends TcpTransport<Channel> {
 
     private ClientBootstrap createClientBootstrap() {
         // this doPrivileged is for SelectorUtil.java that tries to set "sun.nio.ch.bugLevel"
-        AccessController.doPrivileged((PrivilegedAction<Object>) () -> {
         if (blockingClient) {
             clientBootstrap = new ClientBootstrap(new OioClientSocketChannelFactory(
                 Executors.newCachedThreadPool(daemonThreadFactory(settings, TRANSPORT_CLIENT_WORKER_THREAD_NAME_PREFIX))));
@@ -199,10 +196,6 @@ public class NettyTransport extends TcpTransport<Channel> {
                         daemonThreadFactory(settings, TRANSPORT_CLIENT_WORKER_THREAD_NAME_PREFIX)), workerCount),
                     new HashedWheelTimer(daemonThreadFactory(settings, "transport_client_timer"))));
         }
-            return null;
-        });
-        assert System.getProperty("sun.nio.ch.bugLevel") != null :
-            "sun.nio.ch.bugLevel is null somebody pulls in SelectorUtil without doing stuff in a doPrivileged block?";
         clientBootstrap.setPipelineFactory(configureClientChannelPipelineFactory());
         clientBootstrap.setOption("connectTimeoutMillis", connectTimeout.millis());
 
@@ -288,22 +281,18 @@ public class NettyTransport extends TcpTransport<Channel> {
 
         final ThreadFactory bossFactory = daemonThreadFactory(this.settings, HTTP_SERVER_BOSS_THREAD_NAME_PREFIX, name);
         final ThreadFactory workerFactory = daemonThreadFactory(this.settings, HTTP_SERVER_WORKER_THREAD_NAME_PREFIX, name);
-        // this doPrivileged is for SelectorUtil.java that tries to set "sun.nio.ch.bugLevel"
+        final ServerBootstrap serverBootstrap;
-        ServerBootstrap serverBootstrap = AccessController.doPrivileged((PrivilegedAction<ServerBootstrap>) () -> {
         if (blockingServer) {
-                return new ServerBootstrap(new OioServerSocketChannelFactory(
+            serverBootstrap = new ServerBootstrap(new OioServerSocketChannelFactory(
                 Executors.newCachedThreadPool(bossFactory),
                 Executors.newCachedThreadPool(workerFactory)
             ));
         } else {
-                return new ServerBootstrap(new NioServerSocketChannelFactory(
+            serverBootstrap = new ServerBootstrap(new NioServerSocketChannelFactory(
                 Executors.newCachedThreadPool(bossFactory),
                 Executors.newCachedThreadPool(workerFactory),
                 workerCount));
         }
-        });
-        assert System.getProperty("sun.nio.ch.bugLevel") != null :
-            "sun.nio.ch.bugLevel is null somebody pulls in SelectorUtil without doing stuff in a doPrivileged block?";
         serverBootstrap.setPipelineFactory(configureServerChannelPipelineFactory(name, settings));
         if (!"default".equals(tcpNoDelay)) {
             serverBootstrap.setOption("child.tcpNoDelay", Booleans.parseBoolean(tcpNoDelay, null));

qa/smoke-test-client/src/test/java/org/elasticsearch/smoketest/ESSmokeClientTestCase.java

@@ -26,12 +26,14 @@ import org.elasticsearch.client.transport.TransportClient;
 import org.elasticsearch.common.logging.ESLogger;
 import org.elasticsearch.common.logging.ESLoggerFactory;
 import org.elasticsearch.common.network.NetworkModule;
+import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.transport.InetSocketTransportAddress;
 import org.elasticsearch.common.transport.TransportAddress;
 import org.elasticsearch.env.Environment;
 import org.elasticsearch.node.Node;
 import org.elasticsearch.node.internal.InternalSettingsPreparer;
+import org.elasticsearch.plugins.Plugin;
 import org.elasticsearch.test.ESIntegTestCase;
 import org.elasticsearch.transport.MockTcpTransportPlugin;
 import org.elasticsearch.transport.NettyPlugin;
@@ -45,6 +47,8 @@ import java.net.InetAddress;
 import java.net.InetSocketAddress;
 import java.net.URL;
 import java.nio.file.Path;
+import java.util.Collections;
+import java.util.List;
 import java.util.Locale;
 import java.util.concurrent.atomic.AtomicInteger;
 
@@ -76,6 +80,17 @@ public abstract class ESSmokeClientTestCase extends LuceneTestCase {
     private static String clusterAddresses;
     protected String index;
 
+    private static final class BogusPlugin extends Plugin {
+        // se NettyUtils.... this runs without the permission from the netty module so it will fail since reindex can't set the property
+        // to make it still work we disable that check but need to register the setting first
+        private static final Setting<Boolean> ASSERT_NETTY_BUGLEVEL = Setting.boolSetting("netty.assert.buglevel", true,
+            Setting.Property.NodeScope);
+        @Override
+        public List<Setting<?>> getSettings() {
+            return Collections.singletonList(ASSERT_NETTY_BUGLEVEL);
+        }
+    }
+
     private static Client startClient(Path tempDir, TransportAddress... transportAddresses) {
         TransportClient.Builder transportClientBuilder = TransportClient.builder();
         Settings.Builder builder = Settings.builder()
@@ -86,6 +101,8 @@ public abstract class ESSmokeClientTestCase extends LuceneTestCase {
         if (random().nextBoolean()) {
             builder.put(NetworkModule.TRANSPORT_TYPE_KEY, NettyPlugin.NETTY_TRANSPORT_NAME);
             transportClientBuilder.addPlugin(NettyPlugin.class);
+            transportClientBuilder.addPlugin(BogusPlugin.class);
+            builder.put("netty.assert.buglevel", false); // see BogusPlugin
         } else {
             builder.put(NetworkModule.TRANSPORT_TYPE_KEY, MockTcpTransportPlugin.MOCK_TCP_TRANSPORT_NAME);
             transportClientBuilder.addPlugin(MockTcpTransportPlugin.class);