Hi,

I'm checking in this patch to fix PR 23916. The problem in that bug was that we would never use the DomainCombiner specified in the AccessControlContext passed to the 'doPrivileged' methods, but would instead always set the DomainCombiner to our private, default implementation. This breaks the Subject.doAs methods, because they depend on the SubjectDomainCombiner being preserved.

This also adds a necessary permission check to an AccessControlContext constructor, because without it, untrusted code could pass their own specially-constructed DomainCombiner to 'doPrivileged,' and subvert proper access control checks.

Committed,

2005-09-25  Casey Marshall  <[EMAIL PROTECTED]>

    Fixes PR classpath/23916. Fix suggested by Santiago Gala
    <[EMAIL PROTECTED]>.
    * java/security/AccessControlContext.java
    (<init>): update javadoc; check SecurityPermission
    "createAccessControlContext" if a security manager is set.
    (getProtectionDomains): new method.
    * vm/reference/java/security/VMAccessController.java
    (DEBUG): set to 'gnu.classpath.Configuration.DEBUG.'
    (pushContext, popContext): add debug statement.
    (getContext): debug output changes; include the DomainCombiner
    specified in the AccessControlContext, if any.

Index: java/security/AccessControlContext.java
===================================================================
RCS file: /cvsroot/classpath/classpath/java/security/AccessControlContext.java,v
retrieving revision 1.11
diff -u -B -b -r1.11 AccessControlContext.java
--- java/security/AccessControlContext.java     2 Jul 2005 20:32:40 -0000       
1.11
+++ java/security/AccessControlContext.java     25 Sep 2005 23:44:46 -0000
@@ -77,14 +77,23 @@
 
   /**
    * Construct a new AccessControlContext with the specified
-   * ProtectionDomains and DomainCombiner
+   * [EMAIL PROTECTED] ProtectionDomain}s and [EMAIL PROTECTED] 
DomainCombiner}.
    *
+   * <p>Code calling this constructor must have a [EMAIL PROTECTED]
+   * SecurityPermission} of <i>createAccessControlContext</i>.</p>
+   *
+   * @throws SecurityException If the caller does not have permission
+   * to create an access control context.
    * @since 1.3
    */
   public AccessControlContext(AccessControlContext acc,
                              DomainCombiner combiner)
   {
-    // XXX check permission to call this.
+    SecurityManager sm = System.getSecurityManager ();
+    if (sm != null)
+      {
+        sm.checkPermission (new SecurityPermission 
("createAccessControlContext"));
+      }
     AccessControlContext acc2 = AccessController.getContext();
     protectionDomains = combiner.combine (acc2.protectionDomains,
                                           acc.protectionDomains);
@@ -172,5 +181,10 @@
       h ^= protectionDomains[i].hashCode();
 
     return h;
+  }
+
+  ProtectionDomain[] getProtectionDomains ()
+  {
+    return protectionDomains;
   }
 }
Index: vm/reference/java/security/VMAccessController.java
===================================================================
RCS file: 
/cvsroot/classpath/classpath/vm/reference/java/security/VMAccessController.java,v
retrieving revision 1.6
diff -u -B -b -r1.6 VMAccessController.java
--- vm/reference/java/security/VMAccessController.java  2 Jul 2005 20:33:08 
-0000       1.6
+++ vm/reference/java/security/VMAccessController.java  25 Sep 2005 23:44:46 
-0000
@@ -76,7 +76,7 @@
     DEFAULT_CONTEXT = new AccessControlContext(domain);
   }
 
-  private static final boolean DEBUG = false;
+  private static final boolean DEBUG = gnu.classpath.Configuration.DEBUG;
   private static void debug(String msg)
   {
     System.err.print(">>> VMAccessController: ");
@@ -108,6 +108,8 @@
     LinkedList stack = (LinkedList) contexts.get();
     if (stack == null)
       {
+         if (DEBUG)
+           debug("no stack... creating ");
         stack = new LinkedList();
         contexts.set(stack);
       }
@@ -134,6 +136,10 @@
         if (stack.isEmpty())
           contexts.set(null);
       }
+    else if (DEBUG)
+      {
+        debug("no stack during pop?????");
+      }
   }
 
   /**
@@ -166,7 +172,7 @@
     String[] methods = (String[]) stack[1];
 
     if (DEBUG)
-      debug(">>> got trace of length " + classes.length);
+      debug("got trace of length " + classes.length);
 
     HashSet domains = new HashSet();
     HashSet seenDomains = new HashSet();
@@ -185,8 +191,9 @@
 
         if (DEBUG)
           {
-            debug(">>> checking " + clazz + "." + method);
-            debug(">>> loader = " + clazz.getClassLoader());
+            debug("checking " + clazz + "." + method);
+            // subject to getClassLoader RuntimePermission
+            debug("loader = " + clazz.getClassLoader());
           }
 
         // If the previous frame was a call to doPrivileged, then this is
@@ -198,13 +205,15 @@
             && method.equals ("doPrivileged"))
           {
             // If there was a call to doPrivileged with a supplied context,
-            // return that context.
+            // return that context. If using JAAS doAs*, it should be 
+           // a context with a SubjectDomainCombiner
             LinkedList l = (LinkedList) contexts.get();
             if (l != null)
               context = (AccessControlContext) l.getFirst();
             privileged = 1;
           }
 
+        // subject to getProtectionDomain RuntimePermission
         ProtectionDomain domain = clazz.getProtectionDomain();
 
         if (domain == null)
@@ -225,14 +234,25 @@
     ProtectionDomain[] result = (ProtectionDomain[])
       domains.toArray(new ProtectionDomain[domains.size()]);
 
-    // Intersect the derived protection domain with the context supplied
-    // to doPrivileged.
     if (context != null)
-      context = new AccessControlContext(result, context,
-                                         IntersectingDomainCombiner.SINGLETON);
+      {
+        DomainCombiner dc = context.getDomainCombiner ();
+        // If the supplied context had no explicit DomainCombiner, use
+        // our private version, which computes the intersection of the
+        // context's domains with the derived set.
+        if (dc == null)
+          context = new AccessControlContext
+            (IntersectingDomainCombiner.SINGLETON.combine
+             (result, context.getProtectionDomains ()));
+        // Use the supplied DomainCombiner. This should be secure,
+        // because only trusted code may create an
+        // AccessControlContext with a custom DomainCombiner.
+        else
+          context = new AccessControlContext (result, context, dc);
+      }
     // No context was supplied. Return the derived one.
     else
-      context = new AccessControlContext(result);
+      context = new AccessControlContext (result);
 
     inGetContext.set(Boolean.FALSE);
     return context;
_______________________________________________
Classpath-patches mailing list
[email protected]
http://lists.gnu.org/mailman/listinfo/classpath-patches

Reply via email to