Skip to content

Fix java.security.AccessControlException during OpenSearch server shutdown cycle #17183

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 29, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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));
}
Expand All @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand All @@ -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";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -89,7 +89,7 @@
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)

Check warning on line 92 in plugins/transport-reactor-netty4/src/main/java/org/opensearch/transport/reactor/SharedGroupFactory.java

View check run for this annotation

Codecov / codecov/patch

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

Added line #L92 was not covered by tests
);
dedicatedHttpGroup = new SharedGroup(new RefCountedGroup(eventLoopGroup));
}
Expand All @@ -101,7 +101,7 @@
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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand All @@ -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";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -382,6 +384,19 @@
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));

Check warning on line 393 in server/src/main/java/org/opensearch/common/util/concurrent/OpenSearchExecutors.java

View check run for this annotation

Codecov / codecov/patch

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

Added line #L393 was not covered by tests
}

public static ThreadFactory privilegedDaemonThreadFactory(String namePrefix) {
return new PrivilegedOpenSearchThreadFactory(namePrefix);
}

/**
* A thread factory
*
Expand Down Expand Up @@ -409,6 +424,42 @@

}

/**
* 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.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
Loading