Skip to content

Commit

Permalink
Fix SecureSM to allow innocuous threads and threadgroups for parallel…
Browse files Browse the repository at this point in the history
… streams (elastic#117277)

When a parallel stream is opened, the jdk uses an internal fork join
pool to do work on processing the stream. This pool is internal to the
jdk, and so it should always be allowed to create threads. This commit
modifies SecureSM to account for this innocuous thread group and
threads.
  • Loading branch information
rjernst authored Nov 21, 2024
1 parent 8e6e087 commit a9451df
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,9 @@ private static void debugThreadGroups(final ThreadGroup caller, final ThreadGrou
// Returns true if the given thread is an instance of the JDK's InnocuousThread.
private static boolean isInnocuousThread(Thread t) {
final Class<?> c = t.getClass();
return c.getModule() == Object.class.getModule() && c.getName().equals("jdk.internal.misc.InnocuousThread");
return c.getModule() == Object.class.getModule()
&& (c.getName().equals("jdk.internal.misc.InnocuousThread")
|| c.getName().equals("java.util.concurrent.ForkJoinWorkerThread$InnocuousForkJoinWorkerThread"));
}

protected void checkThreadAccess(Thread t) {
Expand All @@ -184,19 +186,29 @@ protected void checkThreadAccess(Thread t) {
private static final Permission MODIFY_THREADGROUP_PERMISSION = new RuntimePermission("modifyThreadGroup");
private static final Permission MODIFY_ARBITRARY_THREADGROUP_PERMISSION = new ThreadPermission("modifyArbitraryThreadGroup");

// Returns true if the given thread is an instance of the JDK's InnocuousThread.
private static boolean isInnocuousThreadGroup(ThreadGroup t) {
final Class<?> c = t.getClass();
return c.getModule() == Object.class.getModule() && t.getName().equals("InnocuousForkJoinWorkerThreadGroup");
}

protected void checkThreadGroupAccess(ThreadGroup g) {
Objects.requireNonNull(g);

boolean targetThreadGroupIsInnocuous = isInnocuousThreadGroup(g);

// first, check if we can modify thread groups at all.
checkPermission(MODIFY_THREADGROUP_PERMISSION);
if (targetThreadGroupIsInnocuous == false) {
checkPermission(MODIFY_THREADGROUP_PERMISSION);
}

// check the threadgroup, if its our thread group or an ancestor, its fine.
final ThreadGroup source = Thread.currentThread().getThreadGroup();
final ThreadGroup target = g;

if (source == null) {
return; // we are a dead thread, do nothing
} else if (source.parentOf(target) == false) {
} else if (source.parentOf(target) == false && targetThreadGroupIsInnocuous == false) {
checkPermission(MODIFY_ARBITRARY_THREADGROUP_PERMISSION);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@
import java.security.Permission;
import java.security.Policy;
import java.security.ProtectionDomain;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.stream.Collectors;

/** Simple tests for SecureSM */
public class SecureSMTests extends TestCase {
Expand Down Expand Up @@ -128,4 +131,12 @@ public void run() {
t1.join();
assertTrue(interrupted1.get());
}

public void testParallelStreamThreadGroup() throws Exception {
List<Integer> list = new ArrayList<>();
for (int i = 0; i < 100; ++i) {
list.add(i);
}
list.parallelStream().collect(Collectors.toSet());
}
}

0 comments on commit a9451df

Please sign in to comment.