You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@river.apache.org by pe...@apache.org on 2012/07/01 08:42:53 UTC

svn commit: r1355852 - in /river/jtsk/trunk/src: net/jini/security/policy/ org/apache/river/api/security/

Author: peter_firmstone
Date: Sun Jul  1 06:42:52 2012
New Revision: 1355852

URL: http://svn.apache.org/viewvc?rev=1355852&view=rev
Log:
Refactoring for release, clean up and review new public api, sanity check and remove unnecessary methods.

Added dnsjava name service provider to handle reverse dns lookup and to provide concurrent dns lookups.

Updated reference-collections, these were updated to avoid calling hashCode during initialisation of Timed references and temporary referrers, this helped reduced SocketPermission.hashCode calls that caused reverse lookups and recursive permission checks that cause stack overflow in the CombinerSecurityManager.

Two tests are failling due to a change to ConcurrentPolicyFile, now only privileged domains are returned by getPermissions(CodeSource) and all other instances are diverted to the java.security.Policy superclass which returns an empty PermissionCollection this is to avoid checking permissions twice.

Failing tests:

com/sun/jini/test/impl/start/aggregatepolicyprovider/SubPoliciesTest.td
com/sun/jini/test/impl/start/loadersplitpolicyprovider/LoaderSplitPolicyProviderTest.td

Modified:
    river/jtsk/trunk/src/net/jini/security/policy/DynamicPolicyProvider.java
    river/jtsk/trunk/src/org/apache/river/api/security/ConcurrentPolicyFile.java
    river/jtsk/trunk/src/org/apache/river/api/security/PermissionGrant.java
    river/jtsk/trunk/src/org/apache/river/api/security/PermissionGrantBuilderImp.java
    river/jtsk/trunk/src/org/apache/river/api/security/PolicyPermission.java
    river/jtsk/trunk/src/org/apache/river/api/security/PrincipalGrant.java
    river/jtsk/trunk/src/org/apache/river/api/security/RemotePolicy.java
    river/jtsk/trunk/src/org/apache/river/api/security/RemotePolicyProvider.java
    river/jtsk/trunk/src/org/apache/river/api/security/RevocablePolicy.java

Modified: river/jtsk/trunk/src/net/jini/security/policy/DynamicPolicyProvider.java
URL: http://svn.apache.org/viewvc/river/jtsk/trunk/src/net/jini/security/policy/DynamicPolicyProvider.java?rev=1355852&r1=1355851&r2=1355852&view=diff
==============================================================================
--- river/jtsk/trunk/src/net/jini/security/policy/DynamicPolicyProvider.java (original)
+++ river/jtsk/trunk/src/net/jini/security/policy/DynamicPolicyProvider.java Sun Jul  1 06:42:52 2012
@@ -18,7 +18,6 @@
 
 package net.jini.security.policy;
 
-import java.lang.ref.WeakReference;
 import org.apache.river.api.security.AbstractPolicy;
 import org.apache.river.api.security.ScalableNestedPolicy;
 import org.apache.river.api.security.ConcurrentPolicyFile;
@@ -35,7 +34,6 @@ import java.security.PrivilegedAction;
 import java.security.ProtectionDomain;
 import java.security.Security;
 import java.security.UnresolvedPermission;
-import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Enumeration;
@@ -45,14 +43,12 @@ import java.util.LinkedList;
 import java.util.NavigableSet;
 import java.util.TreeSet;
 import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import net.jini.security.GrantPermission;
 import org.apache.river.api.security.PermissionGrant;
 import org.apache.river.api.security.PermissionGrantBuilder;
 import org.apache.river.api.security.RemotePolicy;
-import org.apache.river.api.security.PolicyPermission;
 import org.apache.river.api.security.RevocablePolicy;
 
 /**
@@ -172,11 +168,6 @@ public class DynamicPolicyProvider exten
     private final boolean loggable;
     // do something about some domain permissions for this domain so we can 
     // avoid dead locks due to bug 4911907
-
-    private final Guard revokePermission;
-    private final Guard protectionDomainPermission;
-    
-    
     private final PermissionCollection policyPermissions;
     
     /**
@@ -230,8 +221,6 @@ public class DynamicPolicyProvider exten
 	}
         dynamicPolicyGrants = Collections.newSetFromMap(new ConcurrentHashMap<PermissionGrant,Boolean>(64));
         loggable = logger.isLoggable(Level.FINEST);
-	revokePermission = new PolicyPermission("REVOKE");
-        protectionDomainPermission = new RuntimePermission("getProtectionDomain");
         if (basePolicy instanceof DynamicPolicy) {
             DynamicPolicy dp = (DynamicPolicy) basePolicy;
             basePolicyIsDynamic = dp.grantSupported();
@@ -264,8 +253,6 @@ public class DynamicPolicyProvider exten
         this.basePolicy = basePolicy;
         dynamicPolicyGrants = Collections.newSetFromMap(new ConcurrentHashMap<PermissionGrant,Boolean>(64));
         loggable = logger.isLoggable(Level.FINEST);
-	revokePermission = new PolicyPermission("REVOKE");
-        protectionDomainPermission = new RuntimePermission("getProtectionDomain");
          if (basePolicy instanceof DynamicPolicy) {
             DynamicPolicy dp = (DynamicPolicy) basePolicy;
             basePolicyIsDynamic = dp.grantSupported();
@@ -351,32 +338,6 @@ Put the policy providers and all referen
 	 * ProtectionDomain.  Static Permissions are irrevocable.
 	 */ 
         return basePolicy.getPermissions(codesource);
-//        NavigableSet<Permission> permissions = new TreeSet<Permission>(comparator);
-//        if (!(basePolicy instanceof ScalableNestedPolicy) || codesource == null) {
-//            PermissionCollection pc = basePolicy.getPermissions(codesource);
-//            Enumeration<Permission> enu = pc.elements();
-//            while (enu.hasMoreElements()){
-//                permissions.add(enu.nextElement());
-//            }
-//        }else{
-//            ProtectionDomain pd = new ProtectionDomain(codesource, null);
-//            Collection<PermissionGrant> grants = ((ScalableNestedPolicy) basePolicy).getPermissionGrants(pd, true);
-//            processGrants(grants, null, true, permissions);
-//        }
-////        if (revokeable == true) return convert(permissions);
-////        Iterator<PermissionGrant> dynamicGrants = dynamicPolicyGrants.iterator();
-////        while (dynamicGrants.hasNext()){
-////            PermissionGrant p = dynamicGrants.next();
-////            if ( p.implies(codesource, null) ){
-////		// Only use the trusted grantCache.
-////		Collection<Permission> perms = p.getPermissions();
-////                Iterator<Permission> it = perms.iterator();
-////                while (it.hasNext()){
-////                    permissions.add(it.next());
-////                }
-////	    }
-////        }
-//	return convert(permissions, 16, 0.75F, 16, 16);
     }
     
     @Override
@@ -388,42 +349,6 @@ Put the policy providers and all referen
         Collection<PermissionGrant> pgc = getPermissionGrants(domain);  
         NavigableSet<Permission> permissions = new TreeSet<Permission>(comparator);
         processGrants(pgc, null, true, permissions);
-//        if (!(basePolicy instanceof ScalableNestedPolicy)) {
-//            permissions = new TreeSet<Permission>(comparator);
-//            PermissionCollection pc = basePolicy.getPermissions(domain);
-//            Enumeration<Permission> enu = pc.elements();
-//            while (enu.hasMoreElements()){
-//                permissions.add(enu.nextElement());
-//            }
-//        }else{
-//            Collection<PermissionGrant> grants = 
-//                    ((ScalableNestedPolicy) basePolicy).getPermissionGrants(domain, true);
-//            permissions = new TreeSet<Permission>(comparator);
-//            processGrants(grants, null, false, permissions);
-//        }
-////	PermissionGrant [] grantsRefCopy = remotePolicyGrants; // Interim updates not seen.
-////	int l = grantsRefCopy.length;
-////	for ( int i = 0; i < l; i++ ){
-////	    if ( grantsRefCopy[i].implies(domain) ){
-////		Collection<Permission> perms = grantsRefCopy[i].getPermissions();
-////		Iterator<Permission> it = perms.iterator();
-////                while (it.hasNext()){
-////                    permissions.add(it.next());
-////                }
-////	    }
-////	}
-//        Iterator<PermissionGrant> dynamicGrants = dynamicPolicyGrants.iterator();
-//        while (dynamicGrants.hasNext()){
-//            PermissionGrant p = dynamicGrants.next();
-//            if ( p.implies(domain) ){
-//		// Only use the trusted grantCache.
-//                Collection<Permission> perms = p.getPermissions();
-//                Iterator<Permission> it = perms.iterator();
-//                while (it.hasNext()){
-//                    permissions.add(it.next());
-//                }
-//	    }
-//        }
         PermissionCollection pc = convert(permissions, 32, 0.75F, 1, 8);
 	expandUmbrella(pc);
         return pc;
@@ -486,24 +411,6 @@ Put the policy providers and all referen
             processGrants(grants, permClass, true, permissions);
             if (permissions.contains(ALL_PERMISSION)) return true;
         }
-//	PermissionGrant[] grantsRefCopy = remotePolicyGrants; // In case the grants volatile reference is updated.       
-////        if (thread.isInterrupted()) return false;
-//	int l = grantsRefCopy.length;
-//	for ( int i = 0; i < l; i++){
-//	    if (grantsRefCopy[i].implies(domain)) {
-//		Collection<Permission> perms = grantsRefCopy[i].getPermissions();
-//		Iterator<Permission> it = perms.iterator();
-//                while (it.hasNext()){
-//                    Permission p = it.next();
-//                    if ( permClass == null){
-//                        permissions.add(p);
-//                    } else if ( permClass.isInstance(permission) || permission instanceof UnresolvedPermission){
-//                        permissions.add(p);
-//                    }
-//                }
-//	    }
-//	}
-//        if (thread.isInterrupted()) return false;
         Iterator<PermissionGrant> grants = dynamicPolicyGrants.iterator();
         while (grants.hasNext()){
             PermissionGrant g = grants.next();
@@ -519,9 +426,7 @@ Put the policy providers and all referen
                     }
                 }
             }
-        }
-//        if (thread.isInterrupted()) return false;
-        
+        }   
         PermissionCollection pc = null;
         if (permClass != null){
             pc =convert(permissions, 4, 0.75F, 1, 2);
@@ -547,7 +452,7 @@ Put the policy providers and all referen
     public void refresh() {
         basePolicy.refresh();
         // Clean up any void dynamic grants.
-        Collection<PermissionGrant> remove = new ArrayList<PermissionGrant>(40);
+        Collection<PermissionGrant> remove = new LinkedList<PermissionGrant>();
 	Iterator<PermissionGrant> i = dynamicPolicyGrants.iterator();
         while (i.hasNext()){
             PermissionGrant p = i.next();
@@ -579,23 +484,23 @@ Put the policy providers and all referen
         if (permissions == null || permissions.length == 0) {return;}
         checkNullElements(permissions);
         // Not delgated to base policy.
-        SecurityManager sm = System.getSecurityManager();
-        if (sm != null){
-            sm.checkPermission(new GrantPermission(permissions));
-        }
+        Guard g = new GrantPermission(permissions);
+        g.checkGuard(null);
         final PermissionGrantBuilder pgb = PermissionGrantBuilder.newBuilder();
         pgb.principals(principals)
             .permissions(permissions)
             .context(PermissionGrantBuilder.CLASSLOADER);
-        AccessController.doPrivileged(
-            new PrivilegedAction(){
-            
-                public Object run() {
-                    pgb.clazz(cl);
-                    return null;
-                }
-                 
-            });
+        if (cl != null){
+            AccessController.doPrivileged(
+                new PrivilegedAction(){
+
+                    public Object run() {
+                        pgb.clazz(cl);
+                        return null;
+                    }
+
+                });
+        }
         PermissionGrant pe = pgb.build();
 	dynamicPolicyGrants.add(pe);
 	if (loggable){
@@ -627,36 +532,6 @@ Put the policy providers and all referen
         return perms;
     }
 
-    public Permission[] revoke(Class cl, Principal[] principals) {
-	revokePermission.checkGuard(null);
-        ClassLoader loader = null;
-        if( cl != null ) {
-            loader = cl.getClassLoader();
-        }
-        // defensive copy array
-        if (principals != null && principals.length > 0) {
-	    principals = principals.clone();
-	    checkNullElements(principals);
-	}
-	HashSet<Permission> removed = new HashSet<Permission>();
-	Iterator<PermissionGrant> grants = dynamicPolicyGrants.iterator();
-	while ( grants.hasNext()){
-            PermissionGrant g = grants.next();
-	    if ( g.implies(loader, principals) ){
-		// Only use the trusted grantCache.
-		removed.addAll(g.getPermissions());
-                grants.remove();
-	    }
-	}
-        
-        SecurityManager sm = System.getSecurityManager();
-        if (sm instanceof CachingSecurityManager) {
-            ((CachingSecurityManager) sm).clearCache();
-        }
-       return removed.toArray(new Permission[removed.size()]);
-    }
-
-    @Override
     public Collection<PermissionGrant> getPermissionGrants(ProtectionDomain domain) {
         Collection<PermissionGrant> grants = null;
         if (basePolicy instanceof ScalableNestedPolicy){
@@ -672,36 +547,10 @@ Put the policy providers and all referen
         return grants;
     }
 
-//    @Override
-//    public Collection<PermissionGrant> getPermissionGrants(boolean recursive) {
-//        Collection<PermissionGrant> grants = null;
-//        if ( recursive ){ 
-//            if (!(basePolicy instanceof ScalableNestedPolicy)){
-//                throw new UnsupportedOperationException
-//                        ("base policy doesn't implement ScalableNestedPolicy");
-//            }
-//            grants = ((ScalableNestedPolicy)basePolicy).getPermissionGrants(recursive);
-//        } else {
-//            grants = new LinkedList<PermissionGrant>();
-//        }
-//        grants.addAll(dynamicPolicyGrants);
-//        return grants;
-//    }
-
-    @Override
-    public boolean revoke(PermissionGrant p) {
-        revokePermission.checkGuard(null);
-        return dynamicPolicyGrants.remove(p);
-    }
-
-    @Override
     public boolean grant(PermissionGrant p) {
         Collection<Permission> perms = p.getPermissions();
         GrantPermission guard = new GrantPermission(perms.toArray(new Permission [perms.size()]));
         guard.checkGuard(null);
-        // Since PermissionGrant receives a ProtectionDomain instance, we
-        // must check if the caller has that permission.
-        protectionDomainPermission.checkGuard(null);
         return dynamicPolicyGrants.add(p);
     }
 

Modified: river/jtsk/trunk/src/org/apache/river/api/security/ConcurrentPolicyFile.java
URL: http://svn.apache.org/viewvc/river/jtsk/trunk/src/org/apache/river/api/security/ConcurrentPolicyFile.java?rev=1355852&r1=1355851&r2=1355852&view=diff
==============================================================================
--- river/jtsk/trunk/src/org/apache/river/api/security/ConcurrentPolicyFile.java (original)
+++ river/jtsk/trunk/src/org/apache/river/api/security/ConcurrentPolicyFile.java Sun Jul  1 06:42:52 2012
@@ -175,6 +175,8 @@ public class ConcurrentPolicyFile extend
      */
     private static final String POLICY_URL_PREFIX = "policy.url."; //$NON-NLS-1$
     
+    private static final Permission ALL_PERMISSION = new AllPermission();
+    
     // Reference must be defensively copied before access, once published, never mutated.
     private volatile PermissionGrant [] grantArray;
     
@@ -292,18 +294,13 @@ public class ConcurrentPolicyFile extend
     }
 
     /**
-     * Returns collection of permissions allowed for the codesource 
-     * according to the policy. 
-     * The evaluation assumes that current principals are undefined.
-     * 
      * This returns a java.security.Permissions collection, which allows
      * ProtectionDomain to optimise for the AllPermission case, which avoids
      * unnecessarily consulting the policy.
      * 
-     * If constructed with the four argument constructor, ProtectionDomain.implies
-     * first consults the Policy, then it's own internal Permissions collection,
-     * unless it has AllPermission, in which case it returns true without
-     * consulting the policy.
+     * This implementation only returns a Permissions that contains
+     * AllPermission for privileged domains, otherwise it 
+     * calls super.getPermissions(cs).
      * 
      * @param cs CodeSource
      * @see CodeSource
@@ -311,28 +308,20 @@ public class ConcurrentPolicyFile extend
     @Override
     public PermissionCollection getPermissions(CodeSource cs) {
         if (cs == null) throw new NullPointerException("CodeSource cannot be null");
-        NavigableSet<Permission> perms = new TreeSet<Permission>(comparator);
-//        PermissionCollection perms = new ConcurrentPermissions();
         // for ProtectionDomain AllPermission optimisation.
         PermissionGrant [] grantRefCopy = grantArray;
         int l = grantRefCopy.length;
         for ( int j =0; j < l; j++ ){
             PermissionGrant ge = grantRefCopy[j];
             if (ge.implies(cs, null)){ // No Principal's
-                if (ge.isPrivileged()){// Don't stuff around finish early if you can.
+                if (ge.isPrivileged()){
                     PermissionCollection pc = new Permissions();
-                    pc.add(new AllPermission());
+                    pc.add(ALL_PERMISSION);
                     return pc;
                 }
-                Collection<Permission> c = ge.getPermissions();
-                Iterator<Permission> i = c.iterator();
-                while (i.hasNext()){
-                    Permission p = i.next();
-                    perms.add(p);
-                }
             }
         }
-        return convert(perms);
+        return super.getPermissions(cs);
     }
     
     @Override
@@ -464,7 +453,7 @@ public class ConcurrentPolicyFile extend
             }
             pgb.permissions(perms.toArray(new Permission[perms.size()]));
             applicable.add(pgb.build());
-        }
+                        }
         return applicable;
     }
     

Modified: river/jtsk/trunk/src/org/apache/river/api/security/PermissionGrant.java
URL: http://svn.apache.org/viewvc/river/jtsk/trunk/src/org/apache/river/api/security/PermissionGrant.java?rev=1355852&r1=1355851&r2=1355852&view=diff
==============================================================================
--- river/jtsk/trunk/src/org/apache/river/api/security/PermissionGrant.java (original)
+++ river/jtsk/trunk/src/org/apache/river/api/security/PermissionGrant.java Sun Jul  1 06:42:52 2012
@@ -18,17 +18,35 @@
 
 package org.apache.river.api.security;
 
+import java.security.AllPermission;
 import java.security.CodeSource;
+import java.security.Guard;
 import java.security.Permission;
 import java.security.Principal;
 import java.security.ProtectionDomain;
 import java.util.Collection;
+import java.security.Policy;
+import java.util.Collections;
+import java.util.Set;
+import java.util.TreeSet;
+import net.jini.security.GrantPermission;
 
 /**
- * PermissionGrant implementations are expected to be immutable,
+ * PermissionGrant implementations are expected to be immutable, non blocking,
  * thread safe and have a good hashCode implementation to perform well in
  * Collections.
  * 
+ * Developers may use decorators to alter the behaviour of existing implementations.
+ * Decorators should use a single transient volatile field to store the result of
+ * an event notification, which must be immutable, state
+ * may not be updated in the policy until {@link Policy#refresh()} is called.
+ * 
+ * PermissionGrant does not implement Serializable for security reasons, 
+ * however classes extending PermissionGrant may implement Serializable,
+ * but are forced to use the Serializable Builder Pattern.  Child classes
+ * cannot contain circular references to themselves if they implement
+ * Serializable. {@link http://wiki.apache.org/river/Serialization}
+ * 
  * You shouldn't pass around PermissionGrant's to just anyone; they can
  * provide an attacker with information about granted Permissions.
  * 
@@ -43,14 +61,76 @@ import java.util.Collection;
  * @author Peter Firmstone
  * @since 2.2.1
  */
-public interface PermissionGrant {
+public abstract class PermissionGrant {
+    /*
+     * I had originally wanted this class to be an interface, however it became
+     * obvious during development that securing an interface would be more
+     * challenging despite the added flexibility it would provide.
+     * 
+     * The interface solution required the policy to have an additional cache
+     * where the set of permissions were stored in a map, in case the 
+     * implementor tried to mutate those permissions.
+     * 
+     * To avoid escallation of privileges, some fields and methods are final.
+     * 
+     * Overriding classes are restricted in how they can implement Serializable, such
+     * that all guards are checked after deserialization is complete so
+     * all ProtectionDomains are on the call stack.
+     * 
+     * Child classes are prevented from modifying the contained immutable Permissions.
+     */
+    private static final Guard PD_GUARD = new RuntimePermission("getProtectionDomain");
+    private static final Guard CL_GUARD = new RuntimePermission("getClassLoader");
+    private final Set<Permission> perms;
+    private final boolean privileged;
+    private final PermissionGrant decorated;
+    
+    public PermissionGrant(){
+        perms = Collections.emptySet();
+        privileged = false;
+        decorated = null;
+    }
+    
+    PermissionGrant( Permission[] perm ){
+        decorated = null;
+        if (perm == null || perm.length == 0) {
+            this.perms = Collections.emptySet();
+            privileged = false;
+        }else{
+            // PermissionComparator is used to avoid broken hashCode and equals
+	    Set<Permission> perms = new TreeSet<Permission>(new PermissionComparator());
+            boolean privileged = false;
+            int l = perm.length;
+            for (int i = 0; i < l; i++){
+                perms.add(perm[i]);
+                if (perm[i] instanceof AllPermission) privileged = true;
+            }
+	    this.perms = Collections.unmodifiableSet(perms);
+            this.privileged = privileged;
+        }
+    }
+    
+    protected PermissionGrant(PermissionGrant decorated){
+        PD_GUARD.checkGuard(null);
+        CL_GUARD.checkGuard(null);
+        this.decorated = decorated;
+        perms = Collections.emptySet();
+        privileged = false;
+    }
+    
+    protected final PermissionGrant decorated(){
+        return decorated;
+    }
     
     /**
      * Optimisation for AllPermission.
      * 
      * @return true - if PermissionGrant contains AllPermission.
      */
-    boolean isPrivileged();
+    public final boolean isPrivileged(){
+        if (decorated() != null) return decorated().isPrivileged();
+        return privileged;
+    }
 
     /**
      * A DynamicPolicy implementation can use a PermissionGrant as a container
@@ -62,7 +142,7 @@ public interface PermissionGrant {
      * @return
      * @see RevokeableDynamicPolicy
      */
-    boolean implies(ProtectionDomain pd);  
+    public abstract boolean implies(ProtectionDomain pd);  
     /**
      * Checks if this PermissionGrant applies to the passed in ClassLoader
      * and Principal's.
@@ -75,33 +155,30 @@ public interface PermissionGrant {
      * If this method returns false, follow up using the ProtectionDomain for a
      * more specific test, which may return true.
      */
-    boolean implies(ClassLoader cl, Principal[] pal);
+    public abstract boolean implies(ClassLoader cl, Principal[] pal);
     /**
      * Checks if this PermissionGrant applies to the passed in CodeSource
      * and Principal's.
      * @param cs
      * @return 
      */
-    boolean implies(CodeSource codeSource, Principal[] pal);
-
-    @Override
-    boolean equals(Object o);
+    public abstract boolean implies(CodeSource codeSource, Principal[] pal);
 
     /**
      * Returns an unmodifiable Collection of permissions defined by this
      * PermissionGrant, which may be empty, but not null.
      * @return
      */
-    Collection<Permission> getPermissions();
-
-    @Override
-    int hashCode();
+    public final Collection<Permission> getPermissions(){
+        if (decorated() != null) return decorated().getPermissions();
+        return perms;
+    }
 
     /**
      * Returns true if this PermissionGrant defines no Permissions, or if
      * a PermissionGrant was made to a ProtectionDomain that no longer exists.
      */
-    public boolean isVoid();
+    public abstract boolean isVoid();
     
     /**
      * Provide a PermissionGrantBuilder, suitable for
@@ -109,6 +186,6 @@ public interface PermissionGrant {
      * 
      * @return
      */
-    public PermissionGrantBuilder getBuilderTemplate();
+    public abstract PermissionGrantBuilder getBuilderTemplate();
 
 }

Modified: river/jtsk/trunk/src/org/apache/river/api/security/PermissionGrantBuilderImp.java
URL: http://svn.apache.org/viewvc/river/jtsk/trunk/src/org/apache/river/api/security/PermissionGrantBuilderImp.java?rev=1355852&r1=1355851&r2=1355852&view=diff
==============================================================================
--- river/jtsk/trunk/src/org/apache/river/api/security/PermissionGrantBuilderImp.java (original)
+++ river/jtsk/trunk/src/org/apache/river/api/security/PermissionGrantBuilderImp.java Sun Jul  1 06:42:52 2012
@@ -228,7 +228,7 @@ class PermissionGrantBuilderImp extends 
 
 
     // This is a singleton so we don't need to implement equals or hashCode.
-    private static class NullPermissionGrant implements PermissionGrant, Serializable {
+    private static class NullPermissionGrant extends PermissionGrant implements Serializable {
         private static final long serialVersionUID = 1L;
 
         public boolean implies(ProtectionDomain pd) {
@@ -243,10 +243,6 @@ class PermissionGrantBuilderImp extends 
             return false;
         }
 
-        public Collection<Permission> getPermissions() {
-            return Collections.emptySet();
-        }
-
         public boolean isVoid() {
             return true;
         }
@@ -263,9 +259,5 @@ class PermissionGrantBuilderImp extends 
             return nullGrant;
         }
         
-        public boolean isPrivileged() {
-            return false;
-        }
-        
     }
 }

Modified: river/jtsk/trunk/src/org/apache/river/api/security/PolicyPermission.java
URL: http://svn.apache.org/viewvc/river/jtsk/trunk/src/org/apache/river/api/security/PolicyPermission.java?rev=1355852&r1=1355851&r2=1355852&view=diff
==============================================================================
--- river/jtsk/trunk/src/org/apache/river/api/security/PolicyPermission.java (original)
+++ river/jtsk/trunk/src/org/apache/river/api/security/PolicyPermission.java Sun Jul  1 06:42:52 2012
@@ -8,15 +8,8 @@ package org.apache.river.api.security;
 import java.security.BasicPermission;
 
 /**
- * <p>A "revoke" or "REVOKE" PolicyPermission is allows updating a 
- * RemotePolicy or remove Dynamically granted permission from a 
- * RevokeableDynamicPolicy.</p>
- * 
- * <p>A "implementPermissionGrant" PolicyPermission allows a class to implement
- * org.apache.river.api.security.PermissionGrant interface and use it to
- * update a RemotePolicy.  This is not a permission to grant lightly, since
- * a poor implementation could destabilise a policy or worse allow the caller
- * to grant AllPermission anyone using mutation.</p>
+ * <p>A "remote" or "REMOTE" PolicyPermission is allows updating a 
+ * RemotePolicy </p>
  * 
  * @author Peter Firmstone
  */

Modified: river/jtsk/trunk/src/org/apache/river/api/security/PrincipalGrant.java
URL: http://svn.apache.org/viewvc/river/jtsk/trunk/src/org/apache/river/api/security/PrincipalGrant.java?rev=1355852&r1=1355851&r2=1355852&view=diff
==============================================================================
--- river/jtsk/trunk/src/org/apache/river/api/security/PrincipalGrant.java (original)
+++ river/jtsk/trunk/src/org/apache/river/api/security/PrincipalGrant.java Sun Jul  1 06:42:52 2012
@@ -46,16 +46,15 @@ import java.util.TreeSet;
  *
  * @author Peter Firmstone.
  */
-class PrincipalGrant implements PermissionGrant, Serializable{
+class PrincipalGrant extends PermissionGrant implements Serializable{
     private static final long serialVersionUID = 1L;
     // Null object pattern for CodeSource.
     protected static final CodeSource nullCS = new CodeSource((URL) null, (Certificate[]) null);
     protected final Set<Principal> pals;
     private final int hashCode;
-    private final Set<Permission> perms;
-    private final boolean privileged;
     @SuppressWarnings("unchecked")
     PrincipalGrant(Principal[] pals, Permission[] perm){
+        super(perm);
         if ( pals != null ){
 	    Set<Principal> palCol = new HashSet<Principal>(pals.length);
             palCol.addAll(Arrays.asList(pals));
@@ -63,25 +62,9 @@ class PrincipalGrant implements Permissi
         }else {
             this.pals = Collections.emptySet();
         }
-        if (perm == null || perm.length == 0) {
-            this.perms = Collections.emptySet();
-            privileged = false;
-        }else{
-            // PermissionComparator is used to avoid broken hashCode and equals
-	    Set<Permission> perms = new TreeSet<Permission>(new PermissionComparator());
-//            Set<Permission> perms = new HashSet<Permission>();
-            boolean privileged = false;
-            int l = perm.length;
-            for (int i = 0; i < l; i++){
-                perms.add(perm[i]);
-                if (perm[i] instanceof AllPermission) privileged = true;
-            }
-	    this.perms = Collections.unmodifiableSet(perms);
-            this.privileged = privileged;
-        }
         int hash = 5;
         hash = 97 * hash + (this.pals != null ? this.pals.hashCode() : 0);
-        Iterator<Permission> i = perms.iterator();
+        Iterator<Permission> i = getPermissions().iterator();
         while (i.hasNext()){
             Permission p = i.next();
             if (p instanceof UnresolvedPermission){
@@ -106,9 +89,7 @@ class PrincipalGrant implements Permissi
        if (o instanceof PrincipalGrant ){
            PrincipalGrant p = (PrincipalGrant) o;
            if (pals.equals(p.pals) 
-                   // To avoid broken equals:
-                   && perms.containsAll(p.perms) 
-                   && p.perms.containsAll(perms)) return true;
+                   && getPermissions().equals(p.getPermissions())) return true;
        }
        return false;
     }
@@ -128,7 +109,7 @@ class PrincipalGrant implements Permissi
               .append("\n");
         }
         sb.append("\nPermissions: \n");
-        Iterator<Permission> permIt = perms.iterator();
+        Iterator<Permission> permIt = getPermissions().iterator();
         while (permIt.hasNext()){
             sb.append(permIt.next().toString())
               .append("\n");
@@ -247,18 +228,15 @@ class PrincipalGrant implements Permissi
 
     public PermissionGrantBuilder getBuilderTemplate() {
         PermissionGrantBuilder pgb = PermissionGrantBuilder.newBuilder();
+        Collection<Permission> perms = getPermissions();
         pgb.context(PermissionGrantBuilder.PRINCIPAL)
            .principals(pals.toArray(new Principal[pals.size()]))
            .permissions(perms.toArray(new Permission[perms.size()]));
         return pgb;
     }
-
-    public Collection<Permission> getPermissions() {
-        return perms;
-    }
     
     public boolean isVoid() {        
-        if (perms.size() == 0 ) return true;
+        if (getPermissions().isEmpty() ) return true;
         return false;
     }
     
@@ -272,9 +250,4 @@ class PrincipalGrant implements Permissi
             throws InvalidObjectException{
         throw new InvalidObjectException("PermissionGrantBuilder required");
     }
-
-    @Override
-    public boolean isPrivileged() {
-        return privileged;
-    }
 }

Modified: river/jtsk/trunk/src/org/apache/river/api/security/RemotePolicy.java
URL: http://svn.apache.org/viewvc/river/jtsk/trunk/src/org/apache/river/api/security/RemotePolicy.java?rev=1355852&r1=1355851&r2=1355852&view=diff
==============================================================================
--- river/jtsk/trunk/src/org/apache/river/api/security/RemotePolicy.java (original)
+++ river/jtsk/trunk/src/org/apache/river/api/security/RemotePolicy.java Sun Jul  1 06:42:52 2012
@@ -63,7 +63,7 @@ import net.jini.security.policy.Umbrella
  * implementer wants a number of different layers of RemotePolicy, where
  * each layer represents a different administrator role or responsibility.  
  * The administrator's subject must hold the necessary permissions in order
- * to grant them, including RuntimePermission("getProtectionDomain").
+ * to grant them, including GrantPermission and PolicyPermission("REMOTE").
  * </p><p>
  * A node may join more than one djinn group, in this case RemotePolicy's may
  * be used as nested basePolicy's.
@@ -88,6 +88,7 @@ import net.jini.security.policy.Umbrella
  * @see PolicyParser
  * @see DefaultPolicyParser
  * @see DefaultPolicyScanner
+ * @see PolicyPermission
  */
 public interface RemotePolicy {
     /**

Modified: river/jtsk/trunk/src/org/apache/river/api/security/RemotePolicyProvider.java
URL: http://svn.apache.org/viewvc/river/jtsk/trunk/src/org/apache/river/api/security/RemotePolicyProvider.java?rev=1355852&r1=1355851&r2=1355852&view=diff
==============================================================================
--- river/jtsk/trunk/src/org/apache/river/api/security/RemotePolicyProvider.java (original)
+++ river/jtsk/trunk/src/org/apache/river/api/security/RemotePolicyProvider.java Sun Jul  1 06:42:52 2012
@@ -116,11 +116,8 @@ public class RemotePolicyProvider extend
          * separate policy's were to be combined, there may be some cases
          * where two permissions combined also implied a third permission, that
          * neither administrator intended to grant.
-         */ 
-        // because PermissionGrant's are given references to ProtectionDomain's
-        // we must check the caller has this permission.
+         */
         try {
-        protectionDomainPermission.checkGuard(null); 
         // Delegating to the underlying policy is not supported.
 	processRemotePolicyGrants(grants);
         // If we get to here, the caller has permission.
@@ -147,17 +144,17 @@ public class RemotePolicyProvider extend
 	// changes between now and gaining the lock, only the length of the
 	// HashSet is potentially not optimal, keeping the HashSet creation
 	// outside of the lock reduces the lock held duration.
-        Set<ProtectionDomain> domains = new HashSet<ProtectionDomain>();
+        Set<ProtectionDomain> domains = null;
         int l = grants.length;
         for (int i = 0; i < l; i++ ){
             if (grants[i] == null ) throw new NullPointerException("null PermissionGrant prohibited");
             // This causes a ProtectionDomain security check.
             final Class c = grants[i].getClass();
-            List<ProtectionDomain> doms = AccessController.doPrivileged(
-                new PrivilegedAction<List<ProtectionDomain>>() {
-                    public List<ProtectionDomain> run() {
+            domains = AccessController.doPrivileged(
+                new PrivilegedAction<Set<ProtectionDomain>>() {
+                    public Set<ProtectionDomain> run() {
                         Class[] classes = c.getDeclaredClasses();
-                        List<ProtectionDomain> domains = new ArrayList<ProtectionDomain>();
+                        Set<ProtectionDomain> domains = new HashSet<ProtectionDomain>();
                         int l = classes.length;
                         for ( int i = 0; i < l; i++ ){
                             domains.add(classes[i].getProtectionDomain());
@@ -165,7 +162,6 @@ public class RemotePolicyProvider extend
                         return domains;
                     }
                 });
-            domains.addAll(doms);
         }
         Iterator<ProtectionDomain> it = domains.iterator();
         while (it.hasNext()){

Modified: river/jtsk/trunk/src/org/apache/river/api/security/RevocablePolicy.java
URL: http://svn.apache.org/viewvc/river/jtsk/trunk/src/org/apache/river/api/security/RevocablePolicy.java?rev=1355852&r1=1355851&r2=1355852&view=diff
==============================================================================
--- river/jtsk/trunk/src/org/apache/river/api/security/RevocablePolicy.java (original)
+++ river/jtsk/trunk/src/org/apache/river/api/security/RevocablePolicy.java Sun Jul  1 06:42:52 2012
@@ -42,8 +42,11 @@ import net.jini.security.policy.DynamicP
  * </CITE>
  * </p><p>
  * In order for a Permission to be fully revoked, the permission must be
- * used to guard methods only, not Objects or their creation.  A Security 
- * Delegate, may be used as a wrapper with an identical interface to the object
+ * used to guard methods only, not Objects or their creation.  
+ * </p><p>
+ * See "Inside Java 2 Platform Security" 2nd Edition, ISBN:0-201-78791-1, page 176.
+ * </p><p>
+ * A Security Delegate, may be used as a wrapper with an identical interface to the object
  * it protects, a new Permission class must be implemented, for the Delegate's
  * use, in a checkPermission call, to protect access to the underlying
  * object's method. If an existing JVM Permission guards the underlying object,
@@ -67,28 +70,20 @@ import net.jini.security.policy.DynamicP
  * @see java.security.Permission
  * @see PermissionGrant
  * @see DelegatePermission
+ * @see DelegateSecurityManager
  */
 public interface RevocablePolicy extends DynamicPolicy {
+    
     /**
-     * Revokes a specific PermissionGrant from the Policy.
+     * A dynamic grant.
      * 
      * Caveat: Not all Permission's once granted can be revoked.  When a Permission
      * is checked, prior to passing a reference to a caller, that reference
      * has escaped any further Permission checks, meaning that the Permission
      * cannot be revoked for the caller holding a reference.
      * 
-     * Note that after revocation, the caller should call Policy.getPermissions
-     * to determine if the proxy has no or minimal permissions.  If the
-     * proxy still retains permission, the caller should throw a SecurityException.
-     * 
-     * @param p
-     * @return true if successfully
-     */
-    public boolean revoke(PermissionGrant p);
-    
-    /**
-     * A dynamic grant.
      * @param p
+     * @return true if successful 
      */
     public boolean grant(PermissionGrant p);
     /**