You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2023/01/12 15:34:37 UTC

[tomcat] branch main updated: Another batch of SecurityManager related clean-up

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

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


The following commit(s) were added to refs/heads/main by this push:
     new c43ac6a405 Another batch of SecurityManager related clean-up
c43ac6a405 is described below

commit c43ac6a405af65363c4a80636a980f3615a0ee72
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Thu Jan 12 15:34:30 2023 +0000

    Another batch of SecurityManager related clean-up
---
 .../apache/catalina/servlets/DefaultServlet.java   |  27 +---
 .../catalina/session/PersistentManagerBase.java    | 151 +--------------------
 .../apache/catalina/session/StandardManager.java   |  93 -------------
 .../apache/catalina/session/StandardSession.java   |  25 +---
 .../catalina/startup/ClassLoaderFactory.java       |  28 ++--
 5 files changed, 19 insertions(+), 305 deletions(-)

diff --git a/java/org/apache/catalina/servlets/DefaultServlet.java b/java/org/apache/catalina/servlets/DefaultServlet.java
index e859438324..7a647b0804 100644
--- a/java/org/apache/catalina/servlets/DefaultServlet.java
+++ b/java/org/apache/catalina/servlets/DefaultServlet.java
@@ -35,7 +35,6 @@ import java.io.StringWriter;
 import java.io.UnsupportedEncodingException;
 import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
-import java.security.AccessController;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Comparator;
@@ -84,8 +83,6 @@ import org.apache.tomcat.util.http.parser.EntityTag;
 import org.apache.tomcat.util.http.parser.Ranges;
 import org.apache.tomcat.util.res.StringManager;
 import org.apache.tomcat.util.security.Escape;
-import org.apache.tomcat.util.security.PrivilegedGetTccl;
-import org.apache.tomcat.util.security.PrivilegedSetTccl;
 import org.w3c.dom.Document;
 import org.xml.sax.InputSource;
 import org.xml.sax.SAXException;
@@ -1713,22 +1710,9 @@ public class DefaultServlet extends HttpServlet {
 
         // Prevent possible memory leak. Ensure Transformer and
         // TransformerFactory are not loaded from the web application.
-        ClassLoader original;
-        if (Globals.IS_SECURITY_ENABLED) {
-            PrivilegedGetTccl pa = new PrivilegedGetTccl();
-            original = AccessController.doPrivileged(pa);
-        } else {
-            original = Thread.currentThread().getContextClassLoader();
-        }
+        ClassLoader original = Thread.currentThread().getContextClassLoader();
         try {
-            if (Globals.IS_SECURITY_ENABLED) {
-                PrivilegedSetTccl pa =
-                        new PrivilegedSetTccl(DefaultServlet.class.getClassLoader());
-                AccessController.doPrivileged(pa);
-            } else {
-                Thread.currentThread().setContextClassLoader(
-                        DefaultServlet.class.getClassLoader());
-            }
+            Thread.currentThread().setContextClassLoader(DefaultServlet.class.getClassLoader());
 
             TransformerFactory tFactory = TransformerFactory.newInstance();
             Source xmlSource = new StreamSource(new StringReader(sb.toString()));
@@ -1743,12 +1727,7 @@ public class DefaultServlet extends HttpServlet {
         } catch (TransformerException e) {
             throw new ServletException(sm.getString("defaultServlet.xslError"), e);
         } finally {
-            if (Globals.IS_SECURITY_ENABLED) {
-                PrivilegedSetTccl pa = new PrivilegedSetTccl(original);
-                AccessController.doPrivileged(pa);
-            } else {
-                Thread.currentThread().setContextClassLoader(original);
-            }
+            Thread.currentThread().setContextClassLoader(original);
         }
     }
 
diff --git a/java/org/apache/catalina/session/PersistentManagerBase.java b/java/org/apache/catalina/session/PersistentManagerBase.java
index 2c5b01deb9..61f753d6f6 100644
--- a/java/org/apache/catalina/session/PersistentManagerBase.java
+++ b/java/org/apache/catalina/session/PersistentManagerBase.java
@@ -17,9 +17,6 @@
 package org.apache.catalina.session;
 
 import java.io.IOException;
-import java.security.AccessController;
-import java.security.PrivilegedActionException;
-import java.security.PrivilegedExceptionAction;
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -32,7 +29,6 @@ import org.apache.catalina.LifecycleState;
 import org.apache.catalina.Session;
 import org.apache.catalina.Store;
 import org.apache.catalina.StoreManager;
-import org.apache.catalina.security.SecurityUtil;
 import org.apache.juli.logging.Log;
 import org.apache.juli.logging.LogFactory;
 /**
@@ -52,81 +48,6 @@ public abstract class PersistentManagerBase extends ManagerBase
 
     private final Log log = LogFactory.getLog(PersistentManagerBase.class); // must not be static
 
-    // ---------------------------------------------------- Security Classes
-
-    private class PrivilegedStoreClear
-        implements PrivilegedExceptionAction<Void> {
-
-        PrivilegedStoreClear() {
-            // NOOP
-        }
-
-        @Override
-        public Void run() throws Exception{
-           store.clear();
-           return null;
-        }
-    }
-
-    private class PrivilegedStoreRemove
-        implements PrivilegedExceptionAction<Void> {
-
-        private String id;
-
-        PrivilegedStoreRemove(String id) {
-            this.id = id;
-        }
-
-        @Override
-        public Void run() throws Exception{
-           store.remove(id);
-           return null;
-        }
-    }
-
-    private class PrivilegedStoreLoad
-        implements PrivilegedExceptionAction<Session> {
-
-        private String id;
-
-        PrivilegedStoreLoad(String id) {
-            this.id = id;
-        }
-
-        @Override
-        public Session run() throws Exception{
-           return store.load(id);
-        }
-    }
-
-    private class PrivilegedStoreSave
-        implements PrivilegedExceptionAction<Void> {
-
-        private Session session;
-
-        PrivilegedStoreSave(Session session) {
-            this.session = session;
-        }
-
-        @Override
-        public Void run() throws Exception{
-           store.save(session);
-           return null;
-        }
-    }
-
-    private class PrivilegedStoreKeys
-        implements PrivilegedExceptionAction<String[]> {
-
-        PrivilegedStoreKeys() {
-            // NOOP
-        }
-
-        @Override
-        public String[] run() throws Exception{
-           return store.keys();
-        }
-    }
 
     // ----------------------------------------------------- Instance Variables
 
@@ -404,15 +325,7 @@ public abstract class PersistentManagerBase extends ManagerBase
         }
 
         try {
-            if (SecurityUtil.isPackageProtectionEnabled()) {
-                try {
-                    AccessController.doPrivileged(new PrivilegedStoreClear());
-                } catch (PrivilegedActionException e) {
-                    log.error(sm.getString("persistentManager.storeClearError"), e.getException());
-                }
-            } else {
-                store.clear();
-            }
+            store.clear();
         } catch (IOException e) {
             log.error(sm.getString("persistentManager.storeClearError"), e);
         }
@@ -536,17 +449,7 @@ public abstract class PersistentManagerBase extends ManagerBase
 
         String[] ids = null;
         try {
-            if (SecurityUtil.isPackageProtectionEnabled()) {
-                try {
-                    ids = AccessController.doPrivileged(new PrivilegedStoreKeys());
-                } catch (PrivilegedActionException e) {
-                    log.error(sm.getString("persistentManager.storeLoadKeysError"),
-                            e.getException());
-                    return;
-                }
-            } else {
-                ids = store.keys();
-            }
+            ids = store.keys();
         } catch (IOException e) {
             log.error(sm.getString("persistentManager.storeLoadKeysError"), e);
             return;
@@ -596,15 +499,7 @@ public abstract class PersistentManagerBase extends ManagerBase
      */
     protected void removeSession(String id){
         try {
-            if (SecurityUtil.isPackageProtectionEnabled()) {
-                try {
-                    AccessController.doPrivileged(new PrivilegedStoreRemove(id));
-                } catch (PrivilegedActionException e) {
-                    log.error(sm.getString("persistentManager.removeError"), e.getException());
-                }
-            } else {
-                store.remove(id);
-            }
+            store.remove(id);
         } catch (IOException e) {
             log.error(sm.getString("persistentManager.removeError"), e);
         }
@@ -768,11 +663,7 @@ public abstract class PersistentManagerBase extends ManagerBase
 
     private Session loadSessionFromStore(String id) throws IOException {
         try {
-            if (SecurityUtil.isPackageProtectionEnabled()){
-                return securedStoreLoad(id);
-            } else {
-                 return store.load(id);
-            }
+             return store.load(id);
         } catch (ClassNotFoundException e) {
             String msg = sm.getString(
                     "persistentManager.deserializeError", id);
@@ -782,25 +673,6 @@ public abstract class PersistentManagerBase extends ManagerBase
     }
 
 
-    private Session securedStoreLoad(String id) throws IOException, ClassNotFoundException {
-        try {
-            return AccessController.doPrivileged(
-                    new PrivilegedStoreLoad(id));
-        } catch (PrivilegedActionException ex) {
-            Exception e = ex.getException();
-            log.error(sm.getString(
-                    "persistentManager.swapInException", id),
-                    e);
-            if (e instanceof IOException){
-                throw (IOException)e;
-            } else if (e instanceof ClassNotFoundException) {
-                throw (ClassNotFoundException)e;
-            }
-        }
-        return null;
-    }
-
-
     /**
      * Remove the session from the Manager's list of active
      * sessions and write it out to the Store. If the session
@@ -838,20 +710,7 @@ public abstract class PersistentManagerBase extends ManagerBase
         }
 
         try {
-            if (SecurityUtil.isPackageProtectionEnabled()){
-                try{
-                    AccessController.doPrivileged(new PrivilegedStoreSave(session));
-                }catch(PrivilegedActionException ex){
-                    Exception exception = ex.getException();
-                    if (exception instanceof IOException) {
-                        throw (IOException) exception;
-                    }
-                    log.error(sm.getString("persistentManager.serializeError",
-                            session.getIdInternal(), exception));
-                }
-            } else {
-                 store.save(session);
-            }
+             store.save(session);
         } catch (IOException e) {
             log.error(sm.getString("persistentManager.serializeError", session.getIdInternal(), e));
             throw e;
diff --git a/java/org/apache/catalina/session/StandardManager.java b/java/org/apache/catalina/session/StandardManager.java
index ad3273c2dc..0201215305 100644
--- a/java/org/apache/catalina/session/StandardManager.java
+++ b/java/org/apache/catalina/session/StandardManager.java
@@ -25,9 +25,6 @@ import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.ObjectInputStream;
 import java.io.ObjectOutputStream;
-import java.security.AccessController;
-import java.security.PrivilegedActionException;
-import java.security.PrivilegedExceptionAction;
 import java.util.ArrayList;
 import java.util.List;
 
@@ -38,7 +35,6 @@ import org.apache.catalina.LifecycleException;
 import org.apache.catalina.LifecycleState;
 import org.apache.catalina.Loader;
 import org.apache.catalina.Session;
-import org.apache.catalina.security.SecurityUtil;
 import org.apache.catalina.util.CustomObjectInputStream;
 import org.apache.juli.logging.Log;
 import org.apache.juli.logging.LogFactory;
@@ -60,38 +56,6 @@ public class StandardManager extends ManagerBase {
 
     private final Log log = LogFactory.getLog(StandardManager.class); // must not be static
 
-    // ---------------------------------------------------- Security Classes
-
-    private class PrivilegedDoLoad
-        implements PrivilegedExceptionAction<Void> {
-
-        PrivilegedDoLoad() {
-            // NOOP
-        }
-
-        @Override
-        public Void run() throws Exception{
-           doLoad();
-           return null;
-        }
-    }
-
-    private class PrivilegedDoUnload
-        implements PrivilegedExceptionAction<Void> {
-
-        PrivilegedDoUnload() {
-            // NOOP
-        }
-
-        @Override
-        public Void run() throws Exception{
-            doUnload();
-            return null;
-        }
-
-    }
-
-
     // ----------------------------------------------------- Instance Variables
 
     /**
@@ -144,36 +108,6 @@ public class StandardManager extends ManagerBase {
 
     @Override
     public void load() throws ClassNotFoundException, IOException {
-        if (SecurityUtil.isPackageProtectionEnabled()){
-            try{
-                AccessController.doPrivileged( new PrivilegedDoLoad() );
-            } catch (PrivilegedActionException ex){
-                Exception exception = ex.getException();
-                if (exception instanceof ClassNotFoundException) {
-                    throw (ClassNotFoundException)exception;
-                } else if (exception instanceof IOException) {
-                    throw (IOException)exception;
-                }
-                if (log.isDebugEnabled()) {
-                    log.debug("Unreported exception in load() ", exception);
-                }
-            }
-        } else {
-            doLoad();
-        }
-    }
-
-
-    /**
-     * Load any currently active sessions that were previously unloaded
-     * to the appropriate persistence mechanism, if any.  If persistence is not
-     * supported, this method returns without doing anything.
-     *
-     * @exception ClassNotFoundException if a serialized class cannot be
-     *  found during the reload
-     * @exception IOException if an input/output error occurs
-     */
-    protected void doLoad() throws ClassNotFoundException, IOException {
         if (log.isDebugEnabled()) {
             log.debug("Start: Loading persisted sessions");
         }
@@ -252,33 +186,6 @@ public class StandardManager extends ManagerBase {
 
     @Override
     public void unload() throws IOException {
-        if (SecurityUtil.isPackageProtectionEnabled()) {
-            try {
-                AccessController.doPrivileged(new PrivilegedDoUnload());
-            } catch (PrivilegedActionException ex){
-                Exception exception = ex.getException();
-                if (exception instanceof IOException) {
-                    throw (IOException)exception;
-                }
-                if (log.isDebugEnabled()) {
-                    log.debug("Unreported exception in unLoad()", exception);
-                }
-            }
-        } else {
-            doUnload();
-        }
-    }
-
-
-    /**
-     * Save any currently active sessions in the appropriate persistence
-     * mechanism, if any.  If persistence is not supported, this method
-     * returns without doing anything.
-     *
-     * @exception IOException if an input/output error occurs
-     */
-    protected void doUnload() throws IOException {
-
         if (log.isDebugEnabled()) {
             log.debug(sm.getString("standardManager.unloading.debug"));
         }
diff --git a/java/org/apache/catalina/session/StandardSession.java b/java/org/apache/catalina/session/StandardSession.java
index b94a0ed4bd..bb02064506 100644
--- a/java/org/apache/catalina/session/StandardSession.java
+++ b/java/org/apache/catalina/session/StandardSession.java
@@ -24,9 +24,7 @@ import java.io.ObjectOutputStream;
 import java.io.ObjectStreamException;
 import java.io.Serializable;
 import java.io.WriteAbortedException;
-import java.security.AccessController;
 import java.security.Principal;
-import java.security.PrivilegedAction;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Enumeration;
@@ -57,7 +55,6 @@ import org.apache.catalina.SessionEvent;
 import org.apache.catalina.SessionListener;
 import org.apache.catalina.TomcatPrincipal;
 import org.apache.catalina.authenticator.SavedRequest;
-import org.apache.catalina.security.SecurityUtil;
 import org.apache.tomcat.util.ExceptionUtils;
 import org.apache.tomcat.util.res.StringManager;
 
@@ -609,11 +606,7 @@ public class StandardSession implements HttpSession, Session, Serializable {
     @Override
     public HttpSession getSession() {
         if (facade == null) {
-            if (SecurityUtil.isPackageProtectionEnabled()) {
-                facade = AccessController.doPrivileged(new PrivilegedNewSessionFacade(this));
-            } else {
-                facade = new StandardSessionFacade(this);
-            }
+            facade = new StandardSessionFacade(this);
         }
         return facade;
     }
@@ -1787,20 +1780,4 @@ public class StandardSession implements HttpSession, Session, Serializable {
             }
         }
     }
-
-
-    private static class PrivilegedNewSessionFacade implements
-            PrivilegedAction<StandardSessionFacade> {
-
-        private final HttpSession session;
-
-        public PrivilegedNewSessionFacade(HttpSession session) {
-            this.session = session;
-        }
-
-        @Override
-        public StandardSessionFacade run(){
-            return new StandardSessionFacade(session);
-        }
-    }
 }
\ No newline at end of file
diff --git a/java/org/apache/catalina/startup/ClassLoaderFactory.java b/java/org/apache/catalina/startup/ClassLoaderFactory.java
index 394e8a0606..b005335463 100644
--- a/java/org/apache/catalina/startup/ClassLoaderFactory.java
+++ b/java/org/apache/catalina/startup/ClassLoaderFactory.java
@@ -23,8 +23,6 @@ import java.net.URI;
 import java.net.URISyntaxException;
 import java.net.URL;
 import java.net.URLClassLoader;
-import java.security.AccessController;
-import java.security.PrivilegedAction;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Locale;
@@ -128,14 +126,11 @@ public final class ClassLoaderFactory {
 
         // Construct the class loader itself
         final URL[] array = set.toArray(new URL[0]);
-        return AccessController.doPrivileged(
-                (PrivilegedAction<URLClassLoader>) () -> {
-                    if (parent == null) {
-                        return new URLClassLoader(array);
-                    } else {
-                        return new URLClassLoader(array, parent);
-                    }
-                });
+        if (parent == null) {
+            return new URLClassLoader(array);
+        } else {
+            return new URLClassLoader(array, parent);
+        }
     }
 
 
@@ -236,14 +231,11 @@ public final class ClassLoaderFactory {
             }
         }
 
-        return AccessController.doPrivileged(
-                (PrivilegedAction<URLClassLoader>) () -> {
-                    if (parent == null) {
-                        return new URLClassLoader(array);
-                    } else {
-                        return new URLClassLoader(array, parent);
-                    }
-                });
+        if (parent == null) {
+            return new URLClassLoader(array);
+        } else {
+            return new URLClassLoader(array, parent);
+        }
     }
 
     private static boolean validateFile(File file,


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