Skip to content

Commit c32e2fb

Browse files
committed
Fix java.security.AccessControlException during OpenSearch server shutdown cycle
Signed-off-by: Andriy Redko <[email protected]>
1 parent 7306905 commit c32e2fb

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

Lines changed: 3 additions & 3 deletions
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

Lines changed: 2 additions & 3 deletions
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

Lines changed: 3 additions & 3 deletions
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

Lines changed: 2 additions & 3 deletions
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

Lines changed: 51 additions & 0 deletions
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;
@@ -382,6 +384,19 @@ public static ThreadFactory daemonThreadFactory(String namePrefix) {
382384
return new OpenSearchThreadFactory(namePrefix);
383385
}
384386

387+
public static ThreadFactory privilegedDaemonThreadFactory(Settings settings, String namePrefix) {
388+
return privilegedDaemonThreadFactory(threadName(settings, namePrefix));
389+
}
390+
391+
public static ThreadFactory privilegedDaemonThreadFactory(String nodeName, String namePrefix) {
392+
assert nodeName != null && false == nodeName.isEmpty();
393+
return privilegedDaemonThreadFactory(threadName(nodeName, namePrefix));
394+
}
395+
396+
public static ThreadFactory privilegedDaemonThreadFactory(String namePrefix) {
397+
return new PrivilegedOpenSearchThreadFactory(namePrefix);
398+
}
399+
385400
/**
386401
* A thread factory
387402
*
@@ -409,6 +424,42 @@ public Thread newThread(Runnable r) {
409424

410425
}
411426

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

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

Lines changed: 1 addition & 0 deletions
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)