You are viewing a plain text version of this content. The canonical link for it is here.
Posted to user@shiro.apache.org by Manoj Khangaonkar <kh...@gmail.com> on 2009/06/21 21:01:59 UTC

Re: runAs support in Shiro - JIRA issue 21

cc ing shiro-user

On Sun, Jun 21, 2009 at 11:13 AM, Manoj Khangaonkar
<kh...@gmail.com>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.
>
> http://markmail.org/message/hnex52p2puw2pip5
>
> http://markmail.org/message/nc7mqs5uxainqg7c
>
> 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
>
>
>
>

Re: runAs support in Shiro - JIRA issue 21

Posted by Manoj Khangaonkar <kh...@gmail.com>.
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 <lh...@apache.org>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

Re: runAs support in Shiro - JIRA issue 21

Posted by Manoj Khangaonkar <kh...@gmail.com>.
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 <lh...@apache.org>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

Re: runAs support in Shiro - JIRA issue 21

Posted by Les Hazlewood <lh...@apache.org>.
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

Re: runAs support in Shiro - JIRA issue 21

Posted by Les Hazlewood <lh...@apache.org>.
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