From 9cb247287fa30f713a3e836dd44cab67644aff81 Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Tue, 12 Jul 2016 17:41:21 +0200 Subject: [PATCH] consolidate security code in on place an allow test based on the jar dependency to opt out of netty internal property setting assertion --- .../index/reindex/RetryTests.java | 19 +++++- .../http/netty/NettyHttpServerTransport.java | 33 ++++------ .../elasticsearch/transport/NettyPlugin.java | 30 +++++++++ .../transport/netty/NettyTransport.java | 61 ++++++++----------- .../smoketest/ESSmokeClientTestCase.java | 17 ++++++ 5 files changed, 100 insertions(+), 60 deletions(-) diff --git a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/RetryTests.java b/modules/reindex/src/test/java/org/elasticsearch/index/reindex/RetryTests.java index 7db6d510ef5..51d951bbb5e 100644 --- a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/RetryTests.java +++ b/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> 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 ASSERT_NETTY_BUGLEVEL = Setting.boolSetting("netty.assert.buglevel", true, + Setting.Property.NodeScope); + + @Override + public List> 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); } diff --git a/modules/transport-netty/src/main/java/org/elasticsearch/http/netty/NettyHttpServerTransport.java b/modules/transport-netty/src/main/java/org/elasticsearch/http/netty/NettyHttpServerTransport.java index 4e9996dbb70..628b823f04f 100644 --- a/modules/transport-netty/src/main/java/org/elasticsearch/http/netty/NettyHttpServerTransport.java +++ b/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,23 +281,17 @@ 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) () -> { - if (blockingServer) { - serverBootstrap = new ServerBootstrap(new OioServerSocketChannelFactory( - Executors.newCachedThreadPool(daemonThreadFactory(settings, "http_server_boss")), - Executors.newCachedThreadPool(daemonThreadFactory(settings, "http_server_worker")) - )); - } else { - serverBootstrap = new ServerBootstrap(new NioServerSocketChannelFactory( - Executors.newCachedThreadPool(daemonThreadFactory(settings, "http_server_boss")), - 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?"; + if (blockingServer) { + serverBootstrap = new ServerBootstrap(new OioServerSocketChannelFactory( + Executors.newCachedThreadPool(daemonThreadFactory(settings, "http_server_boss")), + Executors.newCachedThreadPool(daemonThreadFactory(settings, "http_server_worker")) + )); + } else { + serverBootstrap = new ServerBootstrap(new NioServerSocketChannelFactory( + Executors.newCachedThreadPool(daemonThreadFactory(settings, "http_server_boss")), + Executors.newCachedThreadPool(daemonThreadFactory(settings, "http_server_worker")), + workerCount)); + } serverBootstrap.setPipelineFactory(configureServerChannelPipelineFactory()); serverBootstrap.setOption("child.tcpNoDelay", tcpNoDelay); diff --git a/modules/transport-netty/src/main/java/org/elasticsearch/transport/NettyPlugin.java b/modules/transport-netty/src/main/java/org/elasticsearch/transport/NettyPlugin.java index ed9ab598bed..5a9a8af009e 100644 --- a/modules/transport-netty/src/main/java/org/elasticsearch/transport/NettyPlugin.java +++ b/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) () -> { + 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> getSettings() { return Arrays.asList( diff --git a/modules/transport-netty/src/main/java/org/elasticsearch/transport/netty/NettyTransport.java b/modules/transport-netty/src/main/java/org/elasticsearch/transport/netty/NettyTransport.java index 5b2ccc07c3b..d55f90cf34f 100644 --- a/modules/transport-netty/src/main/java/org/elasticsearch/transport/netty/NettyTransport.java +++ b/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,24 +183,19 @@ public class NettyTransport extends TcpTransport { private ClientBootstrap createClientBootstrap() { // this doPrivileged is for SelectorUtil.java that tries to set "sun.nio.ch.bugLevel" - AccessController.doPrivileged((PrivilegedAction) () -> { - if (blockingClient) { - clientBootstrap = new ClientBootstrap(new OioClientSocketChannelFactory( - Executors.newCachedThreadPool(daemonThreadFactory(settings, TRANSPORT_CLIENT_WORKER_THREAD_NAME_PREFIX)))); - } else { - int bossCount = NETTY_BOSS_COUNT.get(settings); - clientBootstrap = new ClientBootstrap( - new NioClientSocketChannelFactory( - Executors.newCachedThreadPool(daemonThreadFactory(settings, TRANSPORT_CLIENT_BOSS_THREAD_NAME_PREFIX)), - bossCount, - new NioWorkerPool(Executors.newCachedThreadPool( - 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?"; + if (blockingClient) { + clientBootstrap = new ClientBootstrap(new OioClientSocketChannelFactory( + Executors.newCachedThreadPool(daemonThreadFactory(settings, TRANSPORT_CLIENT_WORKER_THREAD_NAME_PREFIX)))); + } else { + int bossCount = NETTY_BOSS_COUNT.get(settings); + clientBootstrap = new ClientBootstrap( + new NioClientSocketChannelFactory( + Executors.newCachedThreadPool(daemonThreadFactory(settings, TRANSPORT_CLIENT_BOSS_THREAD_NAME_PREFIX)), + bossCount, + new NioWorkerPool(Executors.newCachedThreadPool( + daemonThreadFactory(settings, TRANSPORT_CLIENT_WORKER_THREAD_NAME_PREFIX)), workerCount), + new HashedWheelTimer(daemonThreadFactory(settings, "transport_client_timer")))); + } clientBootstrap.setPipelineFactory(configureClientChannelPipelineFactory()); clientBootstrap.setOption("connectTimeoutMillis", connectTimeout.millis()); @@ -288,22 +281,18 @@ public class NettyTransport extends TcpTransport { 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" - ServerBootstrap serverBootstrap = AccessController.doPrivileged((PrivilegedAction) () -> { - if (blockingServer) { - return new ServerBootstrap(new OioServerSocketChannelFactory( - Executors.newCachedThreadPool(bossFactory), - Executors.newCachedThreadPool(workerFactory) - )); - } else { - return 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?"; + final ServerBootstrap serverBootstrap; + if (blockingServer) { + serverBootstrap = new ServerBootstrap(new OioServerSocketChannelFactory( + Executors.newCachedThreadPool(bossFactory), + Executors.newCachedThreadPool(workerFactory) + )); + } else { + serverBootstrap = new ServerBootstrap(new NioServerSocketChannelFactory( + Executors.newCachedThreadPool(bossFactory), + Executors.newCachedThreadPool(workerFactory), + workerCount)); + } serverBootstrap.setPipelineFactory(configureServerChannelPipelineFactory(name, settings)); if (!"default".equals(tcpNoDelay)) { serverBootstrap.setOption("child.tcpNoDelay", Booleans.parseBoolean(tcpNoDelay, null)); diff --git a/qa/smoke-test-client/src/test/java/org/elasticsearch/smoketest/ESSmokeClientTestCase.java b/qa/smoke-test-client/src/test/java/org/elasticsearch/smoketest/ESSmokeClientTestCase.java index d33e60c3928..7b897284823 100644 --- a/qa/smoke-test-client/src/test/java/org/elasticsearch/smoketest/ESSmokeClientTestCase.java +++ b/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 ASSERT_NETTY_BUGLEVEL = Setting.boolSetting("netty.assert.buglevel", true, + Setting.Property.NodeScope); + @Override + public List> 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);