This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git

commit 9eb49f93f910df9f5408642a798bf39cbeb10804
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Fri Dec 10 15:24:51 2021 +0000

    Improved fix for BZ 65714
    
    https://bz.apache.org/bugzilla/show_bug.cgi?id=65714
    When running under a SecurityManager, ensure that newly created threads
    inherit an appropriate AccessControlContext
---
 .../catalina/security/SecurityClassLoad.java       |  1 +
 .../tomcat/util/security/LocalStrings.properties   |  3 +
 .../PrivilegedSetAccessControlContext.java         | 67 ++++++++++++++++++++++
 .../tomcat/util/threads/TaskThreadFactory.java     | 12 +++-
 webapps/docs/changelog.xml                         |  4 ++
 5 files changed, 84 insertions(+), 3 deletions(-)

diff --git a/java/org/apache/catalina/security/SecurityClassLoad.java 
b/java/org/apache/catalina/security/SecurityClassLoad.java
index bf86414..9529cb6 100644
--- a/java/org/apache/catalina/security/SecurityClassLoad.java
+++ b/java/org/apache/catalina/security/SecurityClassLoad.java
@@ -200,5 +200,6 @@ public final class SecurityClassLoad {
         // security
         loader.loadClass(basePackage + "util.security.PrivilegedGetTccl");
         loader.loadClass(basePackage + "util.security.PrivilegedSetTccl");
+        loader.loadClass(basePackage + 
"util.security.PrivilegedSetAccessControlContext");
     }
 }
diff --git a/java/org/apache/tomcat/util/security/LocalStrings.properties 
b/java/org/apache/tomcat/util/security/LocalStrings.properties
index 577cd38..39e92df 100644
--- a/java/org/apache/tomcat/util/security/LocalStrings.properties
+++ b/java/org/apache/tomcat/util/security/LocalStrings.properties
@@ -14,3 +14,6 @@
 # limitations under the License.
 
 concurrentMessageDigest.noDigest=Digest algorithm unavailable
+
+privilegedSetAccessControlContext.lookupFailed=Unable to obtain reference to 
field Thread.inheritedAccessControlContext
+privilegedSetAccessControlContext.setFailed=Unable to set field 
Thread.inheritedAccessControlContext
diff --git 
a/java/org/apache/tomcat/util/security/PrivilegedSetAccessControlContext.java 
b/java/org/apache/tomcat/util/security/PrivilegedSetAccessControlContext.java
new file mode 100644
index 0000000..2f53480
--- /dev/null
+++ 
b/java/org/apache/tomcat/util/security/PrivilegedSetAccessControlContext.java
@@ -0,0 +1,67 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.tomcat.util.security;
+
+import java.lang.reflect.Field;
+import java.security.AccessControlContext;
+import java.security.AccessController;
+import java.security.PrivilegedAction;
+
+import org.apache.juli.logging.Log;
+import org.apache.juli.logging.LogFactory;
+import org.apache.tomcat.util.res.StringManager;
+
+public class PrivilegedSetAccessControlContext implements 
PrivilegedAction<Void> {
+
+    private static final Log log = 
LogFactory.getLog(PrivilegedSetAccessControlContext.class);
+    private static final StringManager sm = 
StringManager.getManager(PrivilegedSetAccessControlContext.class);
+
+    private static final AccessControlContext acc;
+    private static final Field field;
+
+    static {
+        acc = AccessController.getContext();
+        Field f = null;
+        try {
+            f = Thread.class.getDeclaredField("inheritedAccessControlContext");
+            f.trySetAccessible();
+        } catch (NoSuchFieldException | SecurityException e) {
+            
log.warn(sm.getString("privilegedSetAccessControlContext.lookupFailed"), e);
+        }
+        field = f;
+    }
+
+    private final Thread t;
+
+
+    public PrivilegedSetAccessControlContext(Thread t) {
+        this.t = t;
+    }
+
+
+    @Override
+    public Void run() {
+        try {
+            if (field != null) {
+                field.set(t,  acc);
+            }
+        } catch (IllegalArgumentException | IllegalAccessException e) {
+            
log.warn(sm.getString("privilegedSetAccessControlContext.setFailed"), e);
+        }
+        return null;
+    }
+}
\ No newline at end of file
diff --git a/java/org/apache/tomcat/util/threads/TaskThreadFactory.java 
b/java/org/apache/tomcat/util/threads/TaskThreadFactory.java
index 5601eda..4320f9b 100644
--- a/java/org/apache/tomcat/util/threads/TaskThreadFactory.java
+++ b/java/org/apache/tomcat/util/threads/TaskThreadFactory.java
@@ -21,6 +21,7 @@ import java.security.PrivilegedAction;
 import java.util.concurrent.ThreadFactory;
 import java.util.concurrent.atomic.AtomicInteger;
 
+import org.apache.tomcat.util.security.PrivilegedSetAccessControlContext;
 import org.apache.tomcat.util.security.PrivilegedSetTccl;
 
 /**
@@ -49,13 +50,18 @@ public class TaskThreadFactory implements ThreadFactory {
         t.setDaemon(daemon);
         t.setPriority(threadPriority);
 
-        // Set the context class loader of newly created threads to be the 
class
-        // loader that loaded this factory. This avoids retaining references to
-        // web application class loaders and similar.
         if (Constants.IS_SECURITY_ENABLED) {
+            // Set the context class loader of newly created threads to be the
+            // class loader that loaded this factory. This avoids retaining
+            // references to web application class loaders and similar.
             PrivilegedAction<Void> pa = new PrivilegedSetTccl(
                     t, getClass().getClassLoader());
             AccessController.doPrivileged(pa);
+
+            // This method may be triggered from an InnocuousThread. Ensure 
that
+            // the thread inherits an appropriate AccessControlContext
+            pa = new PrivilegedSetAccessControlContext(t);
+            AccessController.doPrivileged(pa);
         } else {
             t.setContextClassLoader(getClass().getClassLoader());
         }
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index ad95126..e0168ee 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -116,6 +116,10 @@
         Restore pre-starting of <code>minSpareThreads</code> lost in the fix 
for
         <bug>65454</bug>. (markt)
       </fix>
+      <fix>
+        Revert the previous fix for <bug>65714</bug> and implement a more
+        comprehensive fix. (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Jasper">

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to