Re: runAs support in Shiro - JIRA issue 21

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: runAs support in Shiro - JIRA issue 21

Manoj Khangaonkar
cc ing shiro-user

On Sun, Jun 21, 2009 at 11:13 AM, Manoj Khangaonkar <[hidden email]> wrote:
Hi,
 
Newbie to this mailing list. Was scanning through the JIRA issues list.
 
The runAs support discussion caught my attention and if the discussion in the following threads is not closed, I would like
to add a few cents.
 
 
 
Some of the proposed methods in the above threads are
 
subject.assumeIdentity( Object principal );
subject.runAs( Object principal );
subject.switchUser( Object principal );

Doing a runAs and switching identity based on only the principal is in my view a security hole.
Any developer could introduce a malignant line code with a call to runAs using the prinicipal of another
user and hijack the other users privilege. The runAs method should have an authenticated Subject as a parameter.
 
The method could be
 
subject.runAs(Subject runAsSubject) ;
 
What runAs should do is execute a piece of code under the assume identity. And when the execution of the code
completes, revert back to the original identity without the programmer having to make additional method calls.
 
What piece of code are we talking about ? This needs to be specified as an additional parameter. We need an
interface to specify the code to execute. Let me craft something really simple for discussion purposes.
 
interface Work {
    public void run() {
 
        // code to execute here
 
    }
 
}
 
and the runAs signature become:
 
subject.runAs(Subject runasSubject, Work codetoexcute)
 
This eliminates the need for some of the other methods discussed in the threads above such as relinquishAssumedIdentity,
getAssumedIdentity etc.
 
One additional advantage of this approach is the you can do multiple runAs calls without getting complicated.
For example, Authenticated user joe does a runAs Mike. Mike does runAs Judy. Judy does runAs Hal. This is possible
with 3 nested runAs calls and when each call ends, the identity is reset correctly to whatever it was prior to the call.
 
This is similar to the approach taken by doAs* methods of javax.security.auth.Subject.
 
I am very new to Shiro. So if I overlooked anything obvious, please excuse the ignorance.
 
regards
 
Manoj
 
 
 

Reply | Threaded
Open this post in threaded view
|

Re: runAs support in Shiro - JIRA issue 21

Les Hazlewood-2
Hi Manoj,


> Some of the proposed methods in the above threads are
>
> subject.assumeIdentity( Object principal );
> subject.runAs( Object principal );
> subject.switchUser( Object principal );
>
> Doing a runAs and switching identity based on only the principal is in my
> view a security hole.
> Any developer could introduce a malignant line code with a call to runAs
> using the prinicipal of another
> user and hijack the other users privilege.

The purpose of an application security framework is not necessarily to protect the application from the developer.  It exists primarily to protect the application from the application's end-user;  the developer writes the application security checks after all! If you can't trust the developer, then they shouldn't be writing security-sensitive code in the first place.

That is, Shiro has no control over whether a developer writes this:

if ( currentSubject.isPermitted("account:transferMoney") ) {
    transferMoney();
}

or this:

transferMoney();

It is the developer's responsibility to use the security framework as necessary for the application's requirements.

> What piece of code are we talking about ? This needs to be specified as an
> additional parameter. We need an
> interface to specify the code to execute. Let me craft something really
> simple for discussion purposes.
>
> interface Work {
>     public void run() {
>
>         // code to execute here
>
>     }
>
> }

Your example is exactly why, if we add a method to the Subject to assume another user's identity and hold on to it a while, I believe it should not be called 'runAs'. 

Your example however is very appropriate for a 'runAs' naming convention, because that code explicitly 'runs' something - an argument passed to it.  The other difference is that the 'assumed identity' would be relinquished immediately after the method call returns, whereas an assumed identity would be retained until a developer explicitly relinquishes it.

So, your proposal to me makes way for two different use cases and scenarios:

A Subject.runAs( PrincipalCollection identity, Runnable work );

This method would relinquish the assumed identity immediately after the .runAs method call returns.  The other scenario is to hold on to an assumed identity for a while, and only relinquish it when the developer decides:

Subject.assumeIdentity(PrincipalCollection identity);
//call a few methods
//work as another user for a while
Subject.relinquishAssumedIdentity();

Both approaches are valuable.  The first is transient and the developer does not have to 'clean up' or relinquish anything later. 

The 2nd approach is quite nice for the ability to see what another user sees and act as them (but Shiro would never 'lose' the original identity due to logging/security traceability).  This is useful when an administrator wants to act as of one of their users - for example, to see exactly what that user might see when they use the application, which can vary significantly if the administrator usually sees everything.  This capability is a great tool for application help/support in addition to other useful features.  You wouldn't want to relinquish the assumed identity until, say, the admin explicitly clicks a link/button that releases it, at which point the developer would call Subject.relinquishAssumedIdentity();

I really appreciate you bringing up this suggestion, because it is the first time I can clearly see the use for two methods simultaneously:  'runAs' and 'assumeIdentity', which have a clearer delineation (to me at least) on why they should differ.

Best,

Les



Reply | Threaded
Open this post in threaded view
|

Re: runAs support in Shiro - JIRA issue 21

Manoj Khangaonkar
Hi Lez,
 
thanks for the comments.
I will take a crack at implementing both approaches.
 
Few more comments are below..
On Mon, Jun 22, 2009 at 6:34 AM, Les Hazlewood <[hidden email]> wrote:
Hi Manoj,


> Some of the proposed methods in the above threads are
>
> subject.assumeIdentity( Object principal );
> subject.runAs( Object principal );
> subject.switchUser( Object principal );
>
> Doing a runAs and switching identity based on only the principal is in my
> view a security hole.
> Any developer could introduce a malignant line code with a call to runAs
> using the prinicipal of another
> user and hijack the other users privilege.

>> The purpose of an application security framework is not necessarily to protect the application from the developer.  >> It exists primarily to protect the application from the application's end-user;  the developer writes the application
>> security checks after all! If you can't trust the developer, then they shouldn't be writing security-sensitive code in >>the first place.

>>That is, Shiro has no control over whether a developer writes this:

>>if ( currentSubject.isPermitted("account:transferMoney") ) {
    transferMoney();
>>}

>>or this:

>>transferMoney();

>>It is the developer's responsibility to use the security framework as necessary for the application's requirements.

Many security breaches are caused by people that are supposed to be trusted. So using an unauthenticated principal as a parameter does cause me some concern. Using a principal seems fine only if the caller is an "Administrator" or
    a "superuser". But letting any using any user assume the identity of any other user opens an opportunity for foul play.
 
   >> deleted