Fix java.security.AccessControlException during OpenSearch server shu…

…tdown cycle (#17183) (#17189) (cherry picked from commit b9900ee) Signed-off-by: Andriy Redko <andriy.redko@aiven.io> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
opensearch-project · Jan 29, 2025 · 9d325af · 9d325af
1 parent dc90634
commit 9d325af
Show file tree

Hide file tree

Showing 6 changed files with 62 additions and 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
@@ -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
@@ -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/...ort-reactor-netty4/src/main/java/org/opensearch/transport/reactor/SharedGroupFactory.java b/...ort-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
@@ -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
@@ -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<Void>) () -> {
+                        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
@@ -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";