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