river-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gregg Wonderly <gr...@wonderly.org>
Subject Re: Learnings from a RevokeableDynamicPolicy & A Future Roadmap
Date Tue, 10 Aug 2010 14:49:47 GMT
For quite some time, I've been using a package that I put together which uses 
XML to create an interface and a SecurityDelegate as you describe below.  It was 
embedded in one of my applications for some time, but a few years back, I pulled 
it out into a separate codebase so that I could use it in several other projects 
which had grown to need more granular security control.

I've put it out on authlib.dev.java.net.  There really is not a lot of 
documentation at this point.  This is more or less a "throw it over the wall" 
move.

Here's a summary of how it is used, and how it works.

It provides for the definition of users and groups/roles.  There is a whole GUI 
component to the code which allows editing.  This data is access/stored using a 
JDBC based backing store.

The class, org.wonderly.authlib.BuildAccess processes the XML file.  There is a 
sample XML file which shows most of what is possible, but the parsing in 
BuildAccess is the real clue to what is possible.

There are object types, which you designate the type of permission that applies 
to that object.  An example from the access.xml in the CVS tree is:

	<object type="view" access="update,read"
			permission="PermissionTwo"/>

This says that there are parameters in method class which are "view" type 
references.  PermissionTwo is used to control access to those type objects.  The 
"access" attribute represents the types of permissions that the permission 
implementation supports for its "access" parameter during construction.

A single method definition looks like the following:

	<method return="String" name="getDBUnitView" qualifiers="public">
		<descr>
		This method is used to get the description of the named view.
		</descr>
		<security>
			<filter access="read" filtered="view" filteredType="String"
				params="view" filterGeneric="String" class="TestUpdateAuthFilter"/>
			<!-- limit access="read" type="view" arg="view"/ -->
		</security>
		<args>
			<arg name="view" type="String" descr="name of the view to get the definition 
of"/>
		</args>
		<exceptions>
			<throws name="SQLException" descr="when a database access error occurs"/>
		</exceptions>
	</method>

The method signature and JavaDoc is generated from this definition.

The security section says that a filter operation should be performed to return 
the value if it is accessible, instead of throwing an exception as <limit> does.
The class, TestUpdateAuthFilter performs the filtering operation.  It implements 
a standard interface to define how the filtering works.

Here's the generated code:

	public String getDBUnitView( String view ) throws
				 java.rmi.RemoteException , SQLException {
		// Check for required access rights.
		String XXXval = null;
		String XXXtype = "filter";
		String XXXaccess = "read";


		try {
			// Call the method, authorization passes
			// Get Initial data
			String XXXlist = XXXacc.getDBUnitView( view );
			UpdateAuthFilter<String> XXXfilt = null;
			try {
				XXXfilt = new TestUpdateAuthFilter<String>();
			} catch( RuntimeException ex ) {
				log.log( Level.SEVERE, ex.toString(), ex );
				throw new AuthorizationException(
					ex.toString(), ex );
			}
			String XXXret = XXXfilt.filterList( new Object[] {view},
				 XXXlist, "read", XXXauth, XXXdb );
			secureLog(  Level.INFO, "{0}[{1}]:
					"+"getDBUnitView("+view+") ==> "+XXXret,
					 XXXauth.getUserName(), XXXaccess );
			return XXXret;
		} catch( RuntimeException ex ) {
			// Make the exception source visible
			log.log( Level.SEVERE, ex.toString(), ex );
			// Rethrow it to stop the operation
			throw ex;
		}
	}

There's a quite a bit more I can talk about and document, but I just wanted to 
throw this out.  It's licensed under the Apache license, but I did not put the 
license into every source file, yet.

Gregg Wonderly

Peter Firmstone wrote:
> Please help identify any fallacies or oversights in the following 
> arguments.
> 
> A Permission may be revoked, at any point in time after a revocation, 
> untrusted code may hold a reference to a privileged object.
> 
> Some Permission's protect methods, such as Thread.interrupt(), these are 
> effectively revoked with the existing Java security model, however other 
> objects are only protected in their constructor, the responsibility 
> being on the trusted code, not to let their references escape, such as 
> FileOutputStream.
> 
> The moment code holding a reference becomes untrusted, the reference has 
> escaped.
> 
> Instead of using a GuardedObject, or checking permission in 
> constructors, to deal with Permission's that can be revoked, we need to 
> encapsulate the object that needs protection with a SecurityDelegate.
> 
> During a checkPermission call, the current Thread's AccessControlContext 
> is obtained, and (gross simplification) is asked to checkPermission.  
> The AccessControlContext contains all the ProtectionDomain's on the 
> stack, all ProtectionDomains on the stack must have the Permission for 
> it to succeed.  The ProtectionDomain's contained by the 
> AccessControlContext are related to the class and object methods called 
> and returned, the ProtectionDomain's are dynamically added or removed.
> 
> So the thinking behind the SecurityDelegate's private check method is 
> that an object must be protected in a dynamically changing environment:
> 
>   1. Has the RevokeableDynamicPolicy advised that a check must be
>      performed?
>   2. Is this the same thread that the last security check was made
>      against?  If we haven't been advised that there is a reduction of
>      trust in our dynamic Security environment and the last
>      checkPermission call succeeded on this thread, then we can assume
>      that this Tread is still safe.
>   3. If this thread is different or new, then we must checkPermission,
>      regardless of whether trust has changed recently or not.
> 
> The costs:
> 
>   1. Multi-threading is penalised (although a WeakMap could be
>      utilised, with threads as keys, and boolean check values, where
>      all are set true by the notify() call).
>   2. The three "if" calls on every method invocation, check, null and
>      == Thread.
>   3. Replicating the check method on all implementers (this will
>      require a helper class to implement the check).
>   4. The RevokeableDynamicPolicy will need to notify all
>      SecurityDelegate's every time a reduction in trust occurs, it will
>      rely on GC to clean up and remove SecurityDelegates.
> 
> The assumption is if the current Thread was trustworthy last call and 
> the environment hasn't experienced a reduction of trust, we can still 
> trust this thread.  There is of course a risk that a Thread may have a 
> new ProtectionDomain on it's stack that isn't trusted, however this 
> could still happen in the case of the guarded object, where the 
> environment doesn't experience a reduction of trust and the trusted code 
> must be trusted not to let the reference escape it's own 
> ProtectionDomain.  Any code that experiences a reduction of trust will 
> receive an AccessControlException.
> 
> Cheers,
> 
> Peter.
> 
> Peter Firmstone wrote:
>> Tim Blackman wrote:
>>> On Jul 31, 2010, at 11:53 PM, Peter Firmstone wrote:
>>>
>>> <snip>
>>>> A RevokeableDynamicPolicy supports the addition or removal of 
>>>> PermissionGrant's
>>>>     
>>> <snip>
>>>   
>>
>>> Hmmm.
>>>
>>> I remember talking with Bob and Mike Warres about this.  The problem 
>>> with removing permission grants is that  when code is granted a 
>>> permission, it can very likely squirrel away something -- an object, 
>>> or another capability available through the granted permission -- 
>>> that will permit it to perform the same operation again without the 
>>> JVM checking for the permission again.  Our conclusion was that there 
>>> was probably no effective way to implement removal of permission grants.
>>>
>>> Perhaps there is something about the particulars of what you have 
>>> done here to negate this argument -- and I have not had the time to 
>>> check the details of your stuff myself to be sure -- but my guess is 
>>> that it will be difficult or impossible to do this in an airtight 
>>> manner.
>>>   
>>
>> First I'd better point out that this is still an experiment and may 
>> not make a release, but I'm glad you've responded, as security is a 
>> difficult issue and all help is appreciated.
>>
>> The SecurityDelegate's I mentioned earlier, are of course a 
>> compromise, many existing security guards on existing Java classes are 
>> on constructors or methods that return object's, however object's such 
>> as OutputStream and the likes, have unguarded methods, so once these 
>> objects have been released, the reference can be stored, by what may 
>> later become untrusted, and the methods called by untrusted objects.
>>
>> The SecurityDelegate is a wrapper class that implements the same 
>> interface as the guarded object, but once notified of a Permission 
>> revocation ensures the next thread to access the guarded object, 
>> through the SecurityDelegate has AccessController.checkPermission 
>> called again.
>>
>> Because of the cost of the Permission check, it's too expensive to 
>> call on every method invocation.
>>
>> However there is still a flaw in this, consider for a moment an object 
>> protected by a SecurityDelegate, permission's are revoked, the policy 
>> notifies all SecurityDelegate's of a revocation, they ensure the next 
>> method call rechecks permission.  The flaw is that once the 
>> SecurityDelegate, containing the protected object is in untrusted 
>> hands, we don't know how many references to it exist.  The next call 
>> might have permission, however there is no guarantee that another 
>> following will.  The mixing of untrusted and trusted code is the risk.
>>
>> While it cannot be eliminated entirely, every method requiring 
>> protection, should execute a private method something like the 
>> following (Constructors and methods excluded for clarity):
>>
>> class ProtectedOutputStream extends OutputStream implements 
>> SecurityDelegate {
>>
>>    private final OutputStream protected;
>>    private volatile boolean check = true;
>>    private volatile Thread currentThread = null;
>>
>>    public void notify(){
>>       check = true;
>>    }
>>
>>    private void check(){
>>       if ( check == true) {
>>          AccessController.checkPermission(perm);
>>          currentThread = Thread.currentThread();
>>          check = false;
>>          return;
>>       }
>>       if ( currentThread != null ) {
>>          if (Thread.currentThread() == currentThread ) {
>>             return;
>>          }
>>       }
>>        AccessController.checkPermission(perm);
>>        currentThread = Thread.currentThread();
>>        return;
>>    }
>>      public void write(byte[] b) {
>>        check();
>>        protected.write(b);
>>    }
>>
>> }
>>
>> Still, I'm not sure if this is enough to protect the object, there are 
>> some Thread timing issues I've ignored here, but those aside, I don't 
>> want to call the AccessController every invocation, for obvious 
>> efficiency reasons.
>>
>> Thought's and ideas?
>>
>> Cheers,
>>
>> Peter.
>>
>>
> 
> 


Mime
View raw message