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: Learnings from a RevokeableDynamicPolicy & A Future Roadmap
Date Wed, 11 Aug 2010 11:48:57 GMT
Hi Gregg,

I'll try having another look later, was looking in trunk but didn't find 
any java source, then got an Internal Server Error.

Cheers,

Peter.

Gregg Wonderly wrote:
> 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