river-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Peter Firmstone <j...@zeus.net.au>
Subject Re: A non blocking (almost) DynamicPermissionCollection
Date Mon, 19 Dec 2011 08:41:05 GMT
Anyone notice a problem yet?

Even though a Permission probably could and should be immutable, there's 
an issue, some aren't.

Going back to our favourite bad example of a Permission implementation, 
SocketPermission:

SocketPermission mutates on implies calls.

Now, most of the state required to determine if SocketPermission is 
published after construction, however there are some fields relating to 
dns lookups and lookup failures.

Currently if multiple threads (each with their own thread stack context 
- AccessControlContext) running the same code concurrently require 
Permission checks, the threads context will or may contain identical 
ProtectionDomain's,

ProtectionDomain -> Policy-> Permissions -> PermissionCollection -> 
Permission

Our implementation:

ProtectionDomain -> DynamicPolicy -> ConcurrentPermissions -> 
DynamicPermissionCollection -> Permission

The existing policy implementation blocks all implies checks at the 
Permissions level (not Permission), so a SocketPermission check blocking 
on a ProtectionDomain will cause all other threads to block on that 
ProtectionDomain for any Permission check!

Now the problem with AccessControlContext is, that the ProtectionDomains 
it contains are checked sequentially, so if one blocks, they all sit 
there waiting to be checked.

No wonder people complain about SecurityManager performance!

All my improvements to date have only pushed synchronisation to the 
PermissionCollection level.

I've got a SecurityManager that executes checks on all ProtectionDomains 
in an AccessControlContext in parallel, so one blocking PD won't hold up 
the others.

This will reduce the remaining work when the blocking SocketPermission 
finally releases the lock, with my implementation, the blocking will 
only occur on SocketPermissionCollection checks.

A major problem with DynamicPermissionCollection is it assumes the 
Permission is immutable, the current calling thread retrieves a private 
array copy of each Permission (all belonging to the same class), then 
assembles these into an independent unshared instance of 
PermissionCollection, it doesn't block, since other threads can't see 
it, however the underlying Permission's are still shared by other threads.

And damn it, SocketPermission isn't thread safe!

So in this case, updated reference fields, will not be seen by other 
threads, in other words if a SocketPermission fails it's dns lookup, it 
will continue to do so for every thread that tries, since none ever 
write cached updated internal object references back to ram.

But wild card SocketPermissions don't mutate and if 
SocketPermissionCollection contains a wild card, non of the other 
permissions even matter!  Despite SocketPermissionCollection's 
implementation insisting otherwise.

It's getting very tempting to just re implement SocketPermission, 
however doing so opens another can of worms with static 
ProtectionDomains that never consult the policy.

Even in current sun policy implementations because a static 
ProtectionDomain copies it's Permissions (note "s" this is a 
PermissionCollection, not a Permission) objects, it's possible for 
SocketPermission to exist in separate PermissionCollection's, again 
creating possible stale unsynchronized state.

Cheers,

Peter.

Thoughts?

Peter Firmstone wrote:
> Thoughts?
>
>
> /*
> * 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 net.jini.security;
>
> import java.io.IOException;
> import java.io.InvalidObjectException;
> import java.io.ObjectInputStream;
> import java.io.ObjectOutputStream;
> import java.io.Serializable;
> import java.security.Permission;
> import java.security.PermissionCollection;
> import java.util.ArrayList;
> import java.util.Arrays;
> import java.util.Collection;
> import java.util.Collections;
> import java.util.Comparator;
> import java.util.Enumeration;
> import org.apache.river.impl.util.CollectionsConcurrent;
>
> /**
> * This homogenous PermissionCollection is designed to overcome some 
> shortfalls with existing
> * PermissionCollection's that block for potentially long durations
> * on implies(), like SocketPermissionCollection.
> *
> * @author peter
> */
> final class DynamicPermissionCollection extends PermissionCollection {
>    private static final long serialVersionUID = 1L;
>       private final Collection<Permission> perms;
>    private final Class cl;
>    private final Comparator<Permission> comp;
>       DynamicPermissionCollection(Comparator<Permission> c, Class cl){
>        perms = CollectionsConcurrent.multiReadCollection(new 
> ArrayList<Permission>());
>        this.cl = cl;
>        comp = c;
>    }
>       private DynamicPermissionCollection(Class cl, 
> Comparator<Permission> c){
>        this.cl = cl;
>        comp = c;
>        Collection<Permission> col = new ArrayList<Permission>();
>        perms = CollectionsConcurrent.multiReadCollection(col);
>    }
>
>    @Override
>    public void add(Permission permission) {
>        if ( ! cl.isInstance(permission))
>            throw new IllegalArgumentException("invalid permission: "+ 
> permission);
>        if (isReadOnly()) throw new 
> SecurityException("PermissionCollection is read only");
>        perms.add(permission);
>    }
>
>    @Override
>    public boolean implies(Permission permission) {
>        if ( ! cl.isInstance(permission)) return false;
>        Permission [] p = perms.toArray(new Permission[0]); 
> //perms.size() may change
>        if (comp != null){
>            Arrays.sort(p, comp);
>        }
>        PermissionCollection pc = permission.newPermissionCollection();
>        int l = p.length;
>        if (pc != null) {
>            for ( int i = 0; i < l ; i++ ){
>                pc.add(p[i]);
>            }
>            return pc.implies(permission);
>        }
>        for ( int i = 0; i < l ; i++ ){
>            if (p[i].implies(permission)) return true;
>        }
>        return false;
>    }
>
>    @Override
>    public Enumeration<Permission> elements() {
>        return Collections.enumeration(perms);
>    }
>       private Collection<Permission> getPermissions(){
>        return perms;
>    }
>       private void readObject(ObjectInputStream stream) throws
>                InvalidObjectException, IOException, 
> ClassNotFoundException {
>            throw new InvalidObjectException("Serialization Proxy 
> required");
>    }
>       private Object writeReplace(){
>            PermissionCollection pc =
>                new SerializationProxy(cl, comp, this);
>            if (isReadOnly()) pc.setReadOnly();
>            return pc;
>    }
>       /**
>     * A serialization proxy has been provided to allow the fields in 
> DynamicPermissionCollection
>     * to be final.
>     *
>     * The serialization proxy extends PermissionCollection for cases 
> where
>     * it may contain a Permission that refers to it, eg a 
> DelegatePermission.
>     * To fix the readResolve bug.
>     *
>     * This has been provided to implement Serialization, it is better to
>     * avoid serializing a PermissionCollection.
>     */
>    private final static class SerializationProxy extends 
> PermissionCollection {
>        private static final long serialVersionUID = 1L;
>        private Permission[] permissions;
>        private Class clazz;
>        private Comparator<Permission> comp;
>        private transient DynamicPermissionCollection resolved;
>        SerializationProxy(Class cl, Comparator<Permission> c, 
> DynamicPermissionCollection pc){
>            permissions = null;
>            clazz = cl;
>            comp = c;
>            resolved = pc;
>        }
>               private Object readResolve() {
>            PermissionCollection pc =
>                    new DynamicPermissionCollection(clazz, comp);
>            int length = permissions.length;
>            for ( int i = 0 ; i < length ; i++){
>                pc.add(permissions[i]);
>            }
>            if (isReadOnly()) pc.setReadOnly();
>            return pc;
>        }
>               private void writeObject(ObjectOutputStream s) throws 
> IOException{
>            permissions = resolved.getPermissions().toArray(new 
> Permission[0]);
>            s.defaultWriteObject();
>        }
>               private void readObject(ObjectInputStream stream) throws
>                InvalidObjectException, IOException, 
> ClassNotFoundException {
>            stream.defaultReadObject();
>        }
>
>        @Override
>        public void add(Permission permission) {
>            if ( resolved != null ){
>                resolved.add(permission);
>                return;
>            }
>            throw new IllegalStateException("unresolved after 
> serialization");
>        }
>
>        @Override
>        public boolean implies(Permission permission) {
>            if ( resolved != null ){
>                return resolved.implies(permission);
>            }
>            throw new IllegalStateException("unresolved after 
> serialization");
>        }
>
>        @Override
>        public Enumeration<Permission> elements() {
>            if ( resolved != null ){
>                return resolved.elements();
>            }
>            throw new IllegalStateException("unresolved after 
> serialization");
>        }
>               @Override
>        public void setReadOnly(){
>            super.setReadOnly();
>            resolved.setReadOnly();
>        }
>        }
>
> }
>
>


Mime
View raw message