Add more unit tests for SecurityManager/AccessController issues

git-svn-id: https://svn.apache.org/repos/asf/jakarta/commons/proper/logging/trunk@423653 13f79535-47bb-0310-9956-ffa450edef68

src/test/org/apache/commons/logging/security/MockSecurityManager.java

@@ -18,68 +18,118 @@ package org.apache.commons.logging.security;
 
 import java.io.FilePermission;
 import java.security.Permission;
-import java.util.PropertyPermission;
+import java.security.Permissions;
 
 
 /**
  * Custom implementation of a security manager, so we can control the
  * security environment for tests in this package.
- * <p>
- * Note that we don't want to refuse permission to any junit method; otherwise
- * any call to an assert will not be able to output its data!  
  */
 public class MockSecurityManager extends SecurityManager {
-    public void checkPermission(Permission p) throws SecurityException {
-        // System.out.println("\n\ntesting permission:" + p.getClass() + ":"+ p);
 
-        // allow read-only access to files, as this is needed to load classes!
+    private Permissions permissions = new Permissions();
-        if (p instanceof FilePermission) {
+    private static final Permission setSecurityManagerPerm =
-            FilePermission fp = (FilePermission) p;
+        new RuntimePermission("setSecurityManager");
-            if (fp.getActions().equals("read")) {
+    
-                return;
+    private int untrustedCodeCount = 0;
-            }
+
+    public MockSecurityManager() {
+        permissions.add(setSecurityManagerPerm);
+    }
+
+    /**
+     * Define the set of permissions to be granted to classes in the o.a.c.l package,
+     * but NOT to unit-test classes in o.a.c.l.security package.
+     */
+    public void addPermission(Permission p) {
+        permissions.add(p);
+    }
+
+    /**
+     * This returns the number of times that a check of a permission failed
+     * due to stack-walking tracing up into untrusted code. Any non-zero
+     * value indicates a bug in JCL, ie a situation where code was not
+     * correctly wrapped in an AccessController block. The result of such a
+     * bug is that signing JCL is not sufficient to allow JCL to perform
+     * the operation; the caller would need to be signed too. 
+     */
+    public int getUntrustedCodeCount() {
+        return untrustedCodeCount;
+    }
+
+    public void checkPermission(Permission p) throws SecurityException {
+        if (setSecurityManagerPerm.implies(p)) {
+            // ok, allow this; we don't want to block any calls to setSecurityManager
+            // otherwise this custom security manager cannot be reset to the original.
+            // System.out.println("setSecurityManager: granted");
+            return;
         }
 
+        // Allow read-only access to files, as this is needed to load classes!
+        // Ideally, we would limit this to just .class and .jar files.
+        if (p instanceof FilePermission) {
+          FilePermission fp = (FilePermission) p;
+          if (fp.getActions().equals("read")) {
+            // System.out.println("Permit read of files");
+            return;
+          }
+        }
+
+        System.out.println("\n\ntesting permission:" + p.getClass() + ":"+ p);
+
         Exception e = new Exception();
         e.fillInStackTrace();
         StackTraceElement[] stack = e.getStackTrace();
         
-        boolean isControlled = false;
+        // scan the call stack from most recent to oldest.
         // start at 1 to skip the entry in the stack for this method
         for(int i=1; i<stack.length; ++i) {
-            String mname = stack[i].getMethodName();
-            if (mname.equals("setSecurityManager")) {
-                // ok, allow this; we don't want to block any calls to setSecurityManager
-                // otherwise this custom security manager cannot be reset to the original
-                // one...
-                // System.out.println("Allow setSecurityManager");
-                return;
-            }
-
             String cname = stack[i].getClassName();
-            //System.out.println("" + i + ":" + stack[i].getClassName() + 
+            System.out.println("" + i + ":" + stack[i].getClassName() + 
-            //  "." + stack[i].getMethodName());
+              "." + stack[i].getMethodName());
-            if (cname.startsWith("org.apache.commons.logging")) {
+
-                isControlled = true;
+            if (cname.equals("java.security.AccessController")) {
-                break;
+                // Presumably method name equals "doPrivileged"
+                //
+                // The previous iteration of this loop verified that the
+                // PrivilegedAction.run method associated with this
+                // doPrivileged method call had the right permissions,
+                // so we just return here. Effectively, the method invoking
+                // doPrivileged asserted that it checked the input params
+                // and found them safe, and that code is trusted, so we
+                // don't need to check the trust level of code higher in
+                // the call stack.
+                System.out.println("Access controller found: returning");
+                return;
+            } else if (cname.startsWith("java.") 
+                || cname.startsWith("javax.") 
+                || cname.startsWith("junit.") 
+                || cname.startsWith("org.apache.tools.ant.")
+                || cname.startsWith("sun.")) {
+                // Code in these packages is trusted if the caller is trusted.
+                //
+                // TODO: maybe check class is loaded via system loader or similar rather
+                // than checking name? Trusted domains may be different in alternative
+                // jvms..
+            } else if (cname.startsWith("org.apache.commons.logging.security")) {
+                // this is the unit test code; treat this like an untrusted client
+                // app that is using JCL
+                ++untrustedCodeCount;
+                System.out.println("Untrusted code [testcase] found");
+                throw new SecurityException("Untrusted code [testcase] found");
+            } else if (cname.startsWith("org.apache.commons.logging.")) {
+                if (permissions.implies(p)) {
+                    // Code here is trusted if the caller is trusted
+                    System.out.println("Permission in allowed set for JCL class");
+                } else {
+                    System.out.println("Permission refused:" + p.getClass() + ":" + p);
+                    throw new SecurityException("Permission refused:" + p.getClass() + ":" + p);
+                }
+            } else {
+                // we found some code that is not trusted to perform this operation.
+                System.out.println("Unexpected code: permission refused:" + p.getClass() + ":" + p);
+                throw new SecurityException("Unexpected code: permission refused:" + p.getClass() + ":" + p);
             }
         }
-        
-        if (!isControlled) {
-            // we have scanned the entire stack, and found no logging classes, so
-            // this must have been called from junit
-            // System.out.println("Not relevant to test; returning success");
-            return;
-        }
-        
-        if (p instanceof PropertyPermission) {
-            // emulate an applet environment where system properties are not accessable
-            throw new SecurityException(
-               "Permission refused to access property:" 
-                    + ((PropertyPermission)p).getName());
-        }
-
-        // emulate an environment where *everything* is refused
-        throw new SecurityException("Permission refused:" + p.getClass() + ":" + p);
     }
 }

src/test/org/apache/commons/logging/security/SecurityTestCaseAllowed.java

@@ -18,26 +18,37 @@ package org.apache.commons.logging.security;
 
 import java.io.PrintWriter;
 import java.io.StringWriter;
+import java.lang.reflect.Field;
 import java.lang.reflect.Method;
+import java.security.AllPermission;
+import java.util.Hashtable;
 
 import junit.framework.Test;
 import junit.framework.TestCase;
 
 import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
 import org.apache.commons.logging.PathableClassLoader;
 import org.apache.commons.logging.PathableTestSuite;
 
 /**
- * Tests for logging with a SecurityManager that forbids access to nearly everything.
+ * Tests for logging with a security policy that allows JCL access to everything.
  * <p>
- * Performing tests with security permissions disabled is tricky, as building error
+ * This class has only one unit test, as we are (in part) checking behaviour in
- * messages on failure requires certain security permissions. If the security manager
+ * the static block of the LogFactory class. As that class cannot be unloaded after
- * blocks these, then the test can fail without the error messages being output.
+ * being loaded into a classloader, the only workaround is to use the 
+ * PathableClassLoader approach to ensure each test is run in its own
+ * classloader, and use a separate testcase class for each test.
  */
-public class SecurityTestCase extends TestCase
+public class SecurityTestCaseAllowed extends TestCase
 {
     private SecurityManager oldSecMgr;
 
+    // Dummy special hashtable, so we can tell JCL to use this instead of
+    // the standard one.
+    public static class CustomHashtable extends Hashtable {
+    }
+
     /**
      * Return the tests included in this test suite.
      */
@@ -48,7 +59,7 @@ public class SecurityTestCase extends TestCase
         parent.addLogicalLib("testclasses");
 
         Class testClass = parent.loadClass(
-            "org.apache.commons.logging.security.SecurityTestCase");
+            "org.apache.commons.logging.security.SecurityTestCaseAllowed");
         return new PathableTestSuite(testClass, parent);
     }
 
@@ -63,8 +74,16 @@ public class SecurityTestCase extends TestCase
         System.setSecurityManager(oldSecMgr);
     }
 
-    public void testSimple() {
+    /**
-        SecurityManager mySecurityManager = new MockSecurityManager();
+     * Test what happens when JCL is run with all permissions enabled. Custom
+     * overrides should take effect.
+     */
+    public void testAllAllowed() {
+        System.setProperty(
+                LogFactory.HASHTABLE_IMPLEMENTATION_PROPERTY,
+                CustomHashtable.class.getName());
+        MockSecurityManager mySecurityManager = new MockSecurityManager();
+        mySecurityManager.addPermission(new AllPermission());
         System.setSecurityManager(mySecurityManager);
 
         try {
@@ -72,9 +91,20 @@ public class SecurityTestCase extends TestCase
             // initialiser for the LogFactory class is executed.
             Class c = this.getClass().getClassLoader().loadClass(
                     "org.apache.commons.logging.LogFactory");
-            Method m = c.getMethod("getInstance", new Class[] {Class.class});
+            Method m = c.getMethod("getLog", new Class[] {Class.class});
             Log log = (Log) m.invoke(null, new Object[] {this.getClass()});
             log.info("testing");
+            
+            // check that the default map implementation was loaded, as JCL was
+            // forbidden from reading the HASHTABLE_IMPLEMENTATION_PROPERTY property.
+            System.setSecurityManager(null);
+            Field factoryField = c.getDeclaredField("factories");
+            factoryField.setAccessible(true);
+            Object factoryTable = factoryField.get(null); 
+            assertNotNull(factoryTable);
+            assertEquals(CustomHashtable.class.getName(), factoryTable.getClass().getName());
+            
+            assertEquals(0, mySecurityManager.getUntrustedCodeCount());
         } catch(Throwable t) {
             // Restore original security manager so output can be generated; the
             // PrintWriter constructor tries to read the line.separator

src/test/org/apache/commons/logging/security/SecurityTestCaseForbidden.java

@@ -0,0 +1,127 @@
+/*
+ * Copyright 2006 The Apache Software Foundation.
+ * 
+ * Licensed 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.commons.logging.security;
+
+import java.io.PrintWriter;
+import java.io.StringWriter;
+import java.lang.reflect.Field;
+import java.lang.reflect.Method;
+import java.util.Hashtable;
+
+import junit.framework.Test;
+import junit.framework.TestCase;
+
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.apache.commons.logging.PathableClassLoader;
+import org.apache.commons.logging.PathableTestSuite;
+
+/**
+ * Tests for logging with a security policy that forbids JCL access to anything.
+ * <p>
+ * Performing tests with security permissions disabled is tricky, as building error
+ * messages on failure requires certain security permissions. If the security manager
+ * blocks these, then the test can fail without the error messages being output.
+ * <p>
+ * This class has only one unit test, as we are (in part) checking behaviour in
+ * the static block of the LogFactory class. As that class cannot be unloaded after
+ * being loaded into a classloader, the only workaround is to use the 
+ * PathableClassLoader approach to ensure each test is run in its own
+ * classloader, and use a separate testcase class for each test.
+ */
+public class SecurityTestCaseForbidden extends TestCase
+{
+    private SecurityManager oldSecMgr;
+
+    // Dummy special hashtable, so we can tell JCL to use this instead of
+    // the standard one.
+    public static class CustomHashtable extends Hashtable {
+    }
+
+    /**
+     * Return the tests included in this test suite.
+     */
+    public static Test suite() throws Exception {
+        PathableClassLoader parent = new PathableClassLoader(null);
+        parent.useSystemLoader("junit.");
+        parent.addLogicalLib("commons-logging");
+        parent.addLogicalLib("testclasses");
+
+        Class testClass = parent.loadClass(
+            "org.apache.commons.logging.security.SecurityTestCaseForbidden");
+        return new PathableTestSuite(testClass, parent);
+    }
+
+    public void setUp() {
+        // save security manager so it can be restored in tearDown
+        oldSecMgr = System.getSecurityManager();
+    }
+    
+    public void tearDown() {
+        // Restore, so other tests don't get stuffed up if a test
+        // sets a custom security manager.
+        System.setSecurityManager(oldSecMgr);
+    }
+
+    /**
+     * Test what happens when JCL is run with absolutely no security
+     * priveleges at all, including reading system properties. Everything
+     * should fall back to the built-in defaults.
+     */
+    public void testAllForbidden() {
+        System.setProperty(
+                LogFactory.HASHTABLE_IMPLEMENTATION_PROPERTY,
+                CustomHashtable.class.getName());
+        MockSecurityManager mySecurityManager = new MockSecurityManager();
+        System.setSecurityManager(mySecurityManager);
+
+        try {
+            // Use reflection so that we can control exactly when the static
+            // initialiser for the LogFactory class is executed.
+            Class c = this.getClass().getClassLoader().loadClass(
+                    "org.apache.commons.logging.LogFactory");
+            Method m = c.getMethod("getLog", new Class[] {Class.class});
+            Log log = (Log) m.invoke(null, new Object[] {this.getClass()});
+            log.info("testing");
+            
+            // check that the default map implementation was loaded, as JCL was
+            // forbidden from reading the HASHTABLE_IMPLEMENTATION_PROPERTY property.
+            //
+            // The default is either the java Hashtable class (java < 1.2) or the
+            // JCL WeakHashtable (java >= 1.3).
+            System.setSecurityManager(oldSecMgr);
+            Field factoryField = c.getDeclaredField("factories");
+            factoryField.setAccessible(true);
+            Object factoryTable = factoryField.get(null); 
+            assertNotNull(factoryTable);
+            String ftClassName = factoryTable.getClass().getName();
+            assertTrue("Custom hashtable unexpectedly used", 
+                    !CustomHashtable.class.getName().equals(ftClassName));
+
+            assertEquals(0, mySecurityManager.getUntrustedCodeCount());
+        } catch(Throwable t) {
+            // Restore original security manager so output can be generated; the
+            // PrintWriter constructor tries to read the line.separator
+            // system property.
+            System.setSecurityManager(oldSecMgr);
+            StringWriter sw = new StringWriter();
+            PrintWriter pw = new PrintWriter(sw);
+            t.printStackTrace(pw);
+            fail("Unexpected exception:" + t.getMessage() + ":" + sw.toString());
+        }
+    }
+}