Skip to content

Commit 9d325af

Browse files
Fix java.security.AccessControlException during OpenSearch server shutdown 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>
1 parent dc90634 commit 9d325af

File tree

6 files changed

+62
-12
lines changed

6 files changed

+62
-12
lines changed

modules/transport-netty4/src/main/java/org/opensearch/transport/SharedGroupFactory.java

+3-3
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@
4747
import io.netty.channel.nio.NioEventLoopGroup;
4848
import io.netty.util.concurrent.Future;
4949

50-
import static org.opensearch.common.util.concurrent.OpenSearchExecutors.daemonThreadFactory;
50+
import static org.opensearch.common.util.concurrent.OpenSearchExecutors.privilegedDaemonThreadFactory;
5151

5252
/**
5353
* Creates and returns {@link io.netty.channel.EventLoopGroup} instances. It will return a shared group for
@@ -91,7 +91,7 @@ public synchronized SharedGroup getHttpGroup() {
9191
if (dedicatedHttpGroup == null) {
9292
NioEventLoopGroup eventLoopGroup = new NioEventLoopGroup(
9393
httpWorkerCount,
94-
daemonThreadFactory(settings, HttpServerTransport.HTTP_SERVER_WORKER_THREAD_NAME_PREFIX)
94+
privilegedDaemonThreadFactory(settings, HttpServerTransport.HTTP_SERVER_WORKER_THREAD_NAME_PREFIX)
9595
);
9696
dedicatedHttpGroup = new SharedGroup(new RefCountedGroup(eventLoopGroup));
9797
}
@@ -103,7 +103,7 @@ private SharedGroup getGenericGroup() {
103103
if (genericGroup == null) {
104104
EventLoopGroup eventLoopGroup = new NioEventLoopGroup(
105105
workerCount,
106-
daemonThreadFactory(settings, TcpTransport.TRANSPORT_WORKER_THREAD_NAME_PREFIX)
106+
privilegedDaemonThreadFactory(settings, TcpTransport.TRANSPORT_WORKER_THREAD_NAME_PREFIX)
107107
);
108108
this.genericGroup = new RefCountedGroup(eventLoopGroup);
109109
} else {

modules/transport-netty4/src/main/plugin-metadata/plugin-security.policy

+2-3
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030
* GitHub history for details.
3131
*/
3232

33-
grant codeBase "${codebase.netty-common}" {
33+
grant {
3434
// for reading the system-wide configuration for the backlog of established sockets
3535
permission java.io.FilePermission "/proc/sys/net/core/somaxconn", "read";
3636

@@ -39,9 +39,8 @@ grant codeBase "${codebase.netty-common}" {
3939

4040
// Netty sets custom classloader for some of its internal threads
4141
permission java.lang.RuntimePermission "*", "setContextClassLoader";
42-
};
42+
permission java.lang.RuntimePermission "getClassLoader";
4343

44-
grant codeBase "${codebase.netty-transport}" {
4544
// Netty NioEventLoop wants to change this, because of https://bugs.openjdk.java.net/browse/JDK-6427854
4645
// the bug says it only happened rarely, and that its fixed, but apparently it still happens rarely!
4746
permission java.util.PropertyPermission "sun.nio.ch.bugLevel", "write";

plugins/transport-reactor-netty4/src/main/java/org/opensearch/transport/reactor/SharedGroupFactory.java

+3-3
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
import io.netty.channel.nio.NioEventLoopGroup;
3030
import io.netty.util.concurrent.Future;
3131

32-
import static org.opensearch.common.util.concurrent.OpenSearchExecutors.daemonThreadFactory;
32+
import static org.opensearch.common.util.concurrent.OpenSearchExecutors.privilegedDaemonThreadFactory;
3333

3434
/**
3535
* Creates and returns {@link io.netty.channel.EventLoopGroup} instances. It will return a shared group for
@@ -89,7 +89,7 @@ public synchronized SharedGroup getHttpGroup() {
8989
if (dedicatedHttpGroup == null) {
9090
NioEventLoopGroup eventLoopGroup = new NioEventLoopGroup(
9191
httpWorkerCount,
92-
daemonThreadFactory(settings, HttpServerTransport.HTTP_SERVER_WORKER_THREAD_NAME_PREFIX)
92+
privilegedDaemonThreadFactory(settings, HttpServerTransport.HTTP_SERVER_WORKER_THREAD_NAME_PREFIX)
9393
);
9494
dedicatedHttpGroup = new SharedGroup(new RefCountedGroup(eventLoopGroup));
9595
}
@@ -101,7 +101,7 @@ private SharedGroup getGenericGroup() {
101101
if (genericGroup == null) {
102102
EventLoopGroup eventLoopGroup = new NioEventLoopGroup(
103103
workerCount,
104-
daemonThreadFactory(settings, TcpTransport.TRANSPORT_WORKER_THREAD_NAME_PREFIX)
104+
privilegedDaemonThreadFactory(settings, TcpTransport.TRANSPORT_WORKER_THREAD_NAME_PREFIX)
105105
);
106106
this.genericGroup = new RefCountedGroup(eventLoopGroup);
107107
} else {

plugins/transport-reactor-netty4/src/main/plugin-metadata/plugin-security.policy

+2-3
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* compatible open source license.
77
*/
88

9-
grant codeBase "${codebase.netty-common}" {
9+
grant {
1010
// for reading the system-wide configuration for the backlog of established sockets
1111
permission java.io.FilePermission "/proc/sys/net/core/somaxconn", "read";
1212

@@ -15,9 +15,8 @@ grant codeBase "${codebase.netty-common}" {
1515

1616
// Netty sets custom classloader for some of its internal threads
1717
permission java.lang.RuntimePermission "*", "setContextClassLoader";
18-
};
18+
permission java.lang.RuntimePermission "getClassLoader";
1919

20-
grant codeBase "${codebase.netty-transport}" {
2120
// Netty NioEventLoop wants to change this, because of https://bugs.openjdk.java.net/browse/JDK-6427854
2221
// the bug says it only happened rarely, and that its fixed, but apparently it still happens rarely!
2322
permission java.util.PropertyPermission "sun.nio.ch.bugLevel", "write";

server/src/main/java/org/opensearch/common/util/concurrent/OpenSearchExecutors.java

+51
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,8 @@
4343
import org.opensearch.threadpool.RunnableTaskExecutionListener;
4444
import org.opensearch.threadpool.TaskAwareRunnable;
4545

46+
import java.security.AccessController;
47+
import java.security.PrivilegedAction;
4648
import java.util.List;
4749
import java.util.Optional;
4850
import java.util.concurrent.AbstractExecutorService;
@@ -384,6 +386,19 @@ public static ThreadFactory daemonThreadFactory(String namePrefix) {
384386
return new OpenSearchThreadFactory(namePrefix);
385387
}
386388

389+
public static ThreadFactory privilegedDaemonThreadFactory(Settings settings, String namePrefix) {
390+
return privilegedDaemonThreadFactory(threadName(settings, namePrefix));
391+
}
392+
393+
public static ThreadFactory privilegedDaemonThreadFactory(String nodeName, String namePrefix) {
394+
assert nodeName != null && false == nodeName.isEmpty();
395+
return privilegedDaemonThreadFactory(threadName(nodeName, namePrefix));
396+
}
397+
398+
public static ThreadFactory privilegedDaemonThreadFactory(String namePrefix) {
399+
return new PrivilegedOpenSearchThreadFactory(namePrefix);
400+
}
401+
387402
/**
388403
* A thread factory
389404
*
@@ -411,6 +426,42 @@ public Thread newThread(Runnable r) {
411426

412427
}
413428

429+
/**
430+
* A thread factory
431+
*
432+
* @opensearch.internal
433+
*/
434+
static class PrivilegedOpenSearchThreadFactory implements ThreadFactory {
435+
436+
final ThreadGroup group;
437+
final AtomicInteger threadNumber = new AtomicInteger(1);
438+
final String namePrefix;
439+
440+
@SuppressWarnings("removal")
441+
PrivilegedOpenSearchThreadFactory(String namePrefix) {
442+
this.namePrefix = namePrefix;
443+
SecurityManager s = System.getSecurityManager();
444+
group = (s != null) ? s.getThreadGroup() : Thread.currentThread().getThreadGroup();
445+
}
446+
447+
@Override
448+
public Thread newThread(Runnable r) {
449+
final Thread t = new Thread(group, new Runnable() {
450+
@SuppressWarnings({ "deprecation", "removal" })
451+
@Override
452+
public void run() {
453+
AccessController.doPrivileged((PrivilegedAction<Void>) () -> {
454+
r.run();
455+
return null;
456+
});
457+
}
458+
}, namePrefix + "[T#" + threadNumber.getAndIncrement() + "]", 0);
459+
t.setDaemon(true);
460+
return t;
461+
}
462+
463+
}
464+
414465
/**
415466
* Cannot instantiate.
416467
*/

server/src/main/resources/org/opensearch/bootstrap/security.policy

+1
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ grant codeBase "${codebase.opensearch-secure-sm}" {
4646
grant codeBase "${codebase.opensearch}" {
4747
// needed for loading plugins which may expect the context class loader to be set
4848
permission java.lang.RuntimePermission "setContextClassLoader";
49+
permission java.lang.RuntimePermission "getClassLoader";
4950
// needed for SPI class loading
5051
permission java.lang.RuntimePermission "accessDeclaredMembers";
5152
permission org.opensearch.secure_sm.ThreadContextPermission "markAsSystemContext";

0 commit comments

Comments
 (0)