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();
}
}
}
