From 9d325af6bfbf6dfe3fd49b96eb029325b8e9a70b Mon Sep 17 00:00:00 2001 From: "opensearch-trigger-bot[bot]" <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> Date: Wed, 29 Jan 2025 15:46:51 -0500 Subject: [PATCH] Fix java.security.AccessControlException during OpenSearch server shutdown cycle (#17183) (#17189) (cherry picked from commit b9900ee5c36dd73925babfa4d29c8d085f48b962) Signed-off-by: Andriy Redko Signed-off-by: github-actions[bot] Co-authored-by: github-actions[bot] --- .../transport/SharedGroupFactory.java | 6 +-- .../plugin-metadata/plugin-security.policy | 5 +- .../transport/reactor/SharedGroupFactory.java | 6 +-- .../plugin-metadata/plugin-security.policy | 5 +- .../util/concurrent/OpenSearchExecutors.java | 51 +++++++++++++++++++ .../org/opensearch/bootstrap/security.policy | 1 + 6 files changed, 62 insertions(+), 12 deletions(-) diff --git a/modules/transport-netty4/src/main/java/org/opensearch/transport/SharedGroupFactory.java b/modules/transport-netty4/src/main/java/org/opensearch/transport/SharedGroupFactory.java index 454293442572c..00c7f666d2b35 100644 --- a/modules/transport-netty4/src/main/java/org/opensearch/transport/SharedGroupFactory.java +++ b/modules/transport-netty4/src/main/java/org/opensearch/transport/SharedGroupFactory.java @@ -47,7 +47,7 @@ import io.netty.channel.nio.NioEventLoopGroup; import io.netty.util.concurrent.Future; -import static org.opensearch.common.util.concurrent.OpenSearchExecutors.daemonThreadFactory; +import static org.opensearch.common.util.concurrent.OpenSearchExecutors.privilegedDaemonThreadFactory; /** * Creates and returns {@link io.netty.channel.EventLoopGroup} instances. It will return a shared group for @@ -91,7 +91,7 @@ public synchronized SharedGroup getHttpGroup() { if (dedicatedHttpGroup == null) { NioEventLoopGroup eventLoopGroup = new NioEventLoopGroup( httpWorkerCount, - daemonThreadFactory(settings, HttpServerTransport.HTTP_SERVER_WORKER_THREAD_NAME_PREFIX) + privilegedDaemonThreadFactory(settings, HttpServerTransport.HTTP_SERVER_WORKER_THREAD_NAME_PREFIX) ); dedicatedHttpGroup = new SharedGroup(new RefCountedGroup(eventLoopGroup)); } @@ -103,7 +103,7 @@ private SharedGroup getGenericGroup() { if (genericGroup == null) { EventLoopGroup eventLoopGroup = new NioEventLoopGroup( workerCount, - daemonThreadFactory(settings, TcpTransport.TRANSPORT_WORKER_THREAD_NAME_PREFIX) + privilegedDaemonThreadFactory(settings, TcpTransport.TRANSPORT_WORKER_THREAD_NAME_PREFIX) ); this.genericGroup = new RefCountedGroup(eventLoopGroup); } else { diff --git a/modules/transport-netty4/src/main/plugin-metadata/plugin-security.policy b/modules/transport-netty4/src/main/plugin-metadata/plugin-security.policy index c8eee6bb577d0..62cac9cda2a3e 100644 --- a/modules/transport-netty4/src/main/plugin-metadata/plugin-security.policy +++ b/modules/transport-netty4/src/main/plugin-metadata/plugin-security.policy @@ -30,7 +30,7 @@ * GitHub history for details. */ -grant codeBase "${codebase.netty-common}" { +grant { // for reading the system-wide configuration for the backlog of established sockets permission java.io.FilePermission "/proc/sys/net/core/somaxconn", "read"; @@ -39,9 +39,8 @@ grant codeBase "${codebase.netty-common}" { // Netty sets custom classloader for some of its internal threads permission java.lang.RuntimePermission "*", "setContextClassLoader"; -}; + permission java.lang.RuntimePermission "getClassLoader"; -grant codeBase "${codebase.netty-transport}" { // Netty NioEventLoop wants to change this, because of https://bugs.openjdk.java.net/browse/JDK-6427854 // the bug says it only happened rarely, and that its fixed, but apparently it still happens rarely! permission java.util.PropertyPermission "sun.nio.ch.bugLevel", "write"; diff --git a/plugins/transport-reactor-netty4/src/main/java/org/opensearch/transport/reactor/SharedGroupFactory.java b/plugins/transport-reactor-netty4/src/main/java/org/opensearch/transport/reactor/SharedGroupFactory.java index ab7de33c8e673..7df888fefce32 100644 --- a/plugins/transport-reactor-netty4/src/main/java/org/opensearch/transport/reactor/SharedGroupFactory.java +++ b/plugins/transport-reactor-netty4/src/main/java/org/opensearch/transport/reactor/SharedGroupFactory.java @@ -29,7 +29,7 @@ import io.netty.channel.nio.NioEventLoopGroup; import io.netty.util.concurrent.Future; -import static org.opensearch.common.util.concurrent.OpenSearchExecutors.daemonThreadFactory; +import static org.opensearch.common.util.concurrent.OpenSearchExecutors.privilegedDaemonThreadFactory; /** * Creates and returns {@link io.netty.channel.EventLoopGroup} instances. It will return a shared group for @@ -89,7 +89,7 @@ public synchronized SharedGroup getHttpGroup() { if (dedicatedHttpGroup == null) { NioEventLoopGroup eventLoopGroup = new NioEventLoopGroup( httpWorkerCount, - daemonThreadFactory(settings, HttpServerTransport.HTTP_SERVER_WORKER_THREAD_NAME_PREFIX) + privilegedDaemonThreadFactory(settings, HttpServerTransport.HTTP_SERVER_WORKER_THREAD_NAME_PREFIX) ); dedicatedHttpGroup = new SharedGroup(new RefCountedGroup(eventLoopGroup)); } @@ -101,7 +101,7 @@ private SharedGroup getGenericGroup() { if (genericGroup == null) { EventLoopGroup eventLoopGroup = new NioEventLoopGroup( workerCount, - daemonThreadFactory(settings, TcpTransport.TRANSPORT_WORKER_THREAD_NAME_PREFIX) + privilegedDaemonThreadFactory(settings, TcpTransport.TRANSPORT_WORKER_THREAD_NAME_PREFIX) ); this.genericGroup = new RefCountedGroup(eventLoopGroup); } else { diff --git a/plugins/transport-reactor-netty4/src/main/plugin-metadata/plugin-security.policy b/plugins/transport-reactor-netty4/src/main/plugin-metadata/plugin-security.policy index 4f2dcde995338..2b589d7518988 100644 --- a/plugins/transport-reactor-netty4/src/main/plugin-metadata/plugin-security.policy +++ b/plugins/transport-reactor-netty4/src/main/plugin-metadata/plugin-security.policy @@ -6,7 +6,7 @@ * compatible open source license. */ -grant codeBase "${codebase.netty-common}" { +grant { // for reading the system-wide configuration for the backlog of established sockets permission java.io.FilePermission "/proc/sys/net/core/somaxconn", "read"; @@ -15,9 +15,8 @@ grant codeBase "${codebase.netty-common}" { // Netty sets custom classloader for some of its internal threads permission java.lang.RuntimePermission "*", "setContextClassLoader"; -}; + permission java.lang.RuntimePermission "getClassLoader"; -grant codeBase "${codebase.netty-transport}" { // Netty NioEventLoop wants to change this, because of https://bugs.openjdk.java.net/browse/JDK-6427854 // the bug says it only happened rarely, and that its fixed, but apparently it still happens rarely! permission java.util.PropertyPermission "sun.nio.ch.bugLevel", "write"; diff --git a/server/src/main/java/org/opensearch/common/util/concurrent/OpenSearchExecutors.java b/server/src/main/java/org/opensearch/common/util/concurrent/OpenSearchExecutors.java index c3f33885c93ae..5afdf46ca43ff 100644 --- a/server/src/main/java/org/opensearch/common/util/concurrent/OpenSearchExecutors.java +++ b/server/src/main/java/org/opensearch/common/util/concurrent/OpenSearchExecutors.java @@ -43,6 +43,8 @@ import org.opensearch.threadpool.RunnableTaskExecutionListener; import org.opensearch.threadpool.TaskAwareRunnable; +import java.security.AccessController; +import java.security.PrivilegedAction; import java.util.List; import java.util.Optional; import java.util.concurrent.AbstractExecutorService; @@ -384,6 +386,19 @@ public static ThreadFactory daemonThreadFactory(String namePrefix) { return new OpenSearchThreadFactory(namePrefix); } + public static ThreadFactory privilegedDaemonThreadFactory(Settings settings, String namePrefix) { + return privilegedDaemonThreadFactory(threadName(settings, namePrefix)); + } + + public static ThreadFactory privilegedDaemonThreadFactory(String nodeName, String namePrefix) { + assert nodeName != null && false == nodeName.isEmpty(); + return privilegedDaemonThreadFactory(threadName(nodeName, namePrefix)); + } + + public static ThreadFactory privilegedDaemonThreadFactory(String namePrefix) { + return new PrivilegedOpenSearchThreadFactory(namePrefix); + } + /** * A thread factory * @@ -411,6 +426,42 @@ public Thread newThread(Runnable r) { } + /** + * A thread factory + * + * @opensearch.internal + */ + static class PrivilegedOpenSearchThreadFactory implements ThreadFactory { + + final ThreadGroup group; + final AtomicInteger threadNumber = new AtomicInteger(1); + final String namePrefix; + + @SuppressWarnings("removal") + PrivilegedOpenSearchThreadFactory(String namePrefix) { + this.namePrefix = namePrefix; + SecurityManager s = System.getSecurityManager(); + group = (s != null) ? s.getThreadGroup() : Thread.currentThread().getThreadGroup(); + } + + @Override + public Thread newThread(Runnable r) { + final Thread t = new Thread(group, new Runnable() { + @SuppressWarnings({ "deprecation", "removal" }) + @Override + public void run() { + AccessController.doPrivileged((PrivilegedAction) () -> { + r.run(); + return null; + }); + } + }, namePrefix + "[T#" + threadNumber.getAndIncrement() + "]", 0); + t.setDaemon(true); + return t; + } + + } + /** * Cannot instantiate. */ diff --git a/server/src/main/resources/org/opensearch/bootstrap/security.policy b/server/src/main/resources/org/opensearch/bootstrap/security.policy index 22e445f7d9022..a6d6014b26bfb 100644 --- a/server/src/main/resources/org/opensearch/bootstrap/security.policy +++ b/server/src/main/resources/org/opensearch/bootstrap/security.policy @@ -46,6 +46,7 @@ grant codeBase "${codebase.opensearch-secure-sm}" { grant codeBase "${codebase.opensearch}" { // needed for loading plugins which may expect the context class loader to be set permission java.lang.RuntimePermission "setContextClassLoader"; + permission java.lang.RuntimePermission "getClassLoader"; // needed for SPI class loading permission java.lang.RuntimePermission "accessDeclaredMembers"; permission org.opensearch.secure_sm.ThreadContextPermission "markAsSystemContext";