Browse Source

Fix SecureSM to allow innocuous threads and threadgroups for parallel streams (#117277) (#117292)

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.

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Ryan Ernst 10 months ago
parent
commit
519057d917

+ 15 - 3
libs/secure-sm/src/main/java/org/elasticsearch/secure_sm/SecureSM.java

@@ -157,7 +157,9 @@ public class SecureSM extends SecurityManager {
     // 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) {
@@ -184,11 +186,21 @@ public class SecureSM extends SecurityManager {
     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();
@@ -196,7 +208,7 @@ public class SecureSM extends SecurityManager {
 
         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);
         }
     }

+ 11 - 0
libs/secure-sm/src/test/java/org/elasticsearch/secure_sm/SecureSMTests.java

@@ -14,7 +14,10 @@ import junit.framework.TestCase;
 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 {
@@ -128,4 +131,12 @@ public class SecureSMTests extends TestCase {
         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());
+    }
 }