You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by Christopher Schultz <ch...@christopherschultz.net> on 2015/11/20 13:47:15 UTC

Adding CredentialHandler to ServletContext so applications can get to it

All,

I thought there was a BZ issue for this, but I didn't find one. It's
been suggested (and I agree completely) that an application ought to be
able to fetch the CredentialHandler for the context's realm so that it
could mutate user credentials in the same way that the Realm expects to
do. That allows applications to change user's passwords, for instance.

So I think I've identified where to put this: around
StandardContext.java:5131 (in Tomcat 9/trunk). But I have a few
questions about it.

It occurs to me that we won't want applications to change the
CredentialHandler in any way -- such as changing the salt length for a
DigestCredentialHandler, etc., so I think it makes sense to wrap the
CredentialHandler in a "safe" class that doesn't allow access to the
real CredentialHandler. That's fairly easy to do.

Also, we probably want the CredentialHandler in the application scope to
always be in sync with the CredentialHandler actually being used by the
Realm. Realm has a setCredentialHandler(CredentialHandler) method, so
theoretically, the CH could change at any time.

Also, the Realm could theoretically change at any time as well.

So, this "safe" wrapper could just call
getRealmInternal().getCredentialHandler().underlyingMethod and always
use that. But, ContainerBase.getRealmInternal uses a lock, and I'm not
sure how expensive that is. Is there any concern over performance, here?

Another option would be to override setRealm and capture the Realm,
there... except that StandardContext never calls setRealm directly.
(Presumably, it's called by another component when constructing the
StandardContext, but then why does StandardContext.startInternal call
getRealmInternal().start() when ContainerBase.setRealm already calls
Realm.start()?)

I'd appreciate some thoughts on the above; I don't want to have to
commit 12 patches for this due to misunderstandings about what's going
on with StandardContext.

Thanks,
-chris

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: Adding CredentialHandler to ServletContext so applications can get to it

Posted by Christopher Schultz <ch...@christopherschultz.net>.
Konstantin,

On 11/23/15 7:19 AM, Konstantin Kolinko wrote:
> 2015-11-22 22:26 GMT+03:00 Christopher Schultz <ch...@christopherschultz.net>:
>> Konstantin,
>>
>> On 11/21/15 10:07 AM, Konstantin Kolinko wrote:
>>> 2015-11-20 15:47 GMT+03:00 Christopher Schultz <ch...@christopherschultz.net>:
>>>> All,
>>>>
>>>> I thought there was a BZ issue for this, but I didn't find one. It's
>>>> been suggested (and I agree completely) that an application ought to be
>>>> able to fetch the CredentialHandler for the context's realm so that it
>>>> could mutate user credentials in the same way that the Realm expects to
>>>> do. That allows applications to change user's passwords, for instance.
>>>>
>>>
>>> In review of r1715434
>>>
>>> 1. I think this has to be an opt-in feature.
>>>
>>> I do not see high risk though.  In theory, if stored credentials are
>>> known then CredentialHandler allows untrusted applications to test
>>> potential passwords without triggering a lock-out timer.
>>
>> Hmm, I hadn't thought of that. I could pretty easily make this a setting
>> for the context.
>>
>>> I am OK with this being a StandardContext feature, though there is an
>>> alternative way: to implement publishing the attribute with a
>>> Listener.
>>
>> But then if you don't trust the application, how do you trust the
>> context's settings?
>>
> 
> 1) When SecurityManager is enabled then the value of "deployXML"
> attribute of a StandatdHost by default is "false".
> 
> It means that an administrator has to explicitly configure a
> conf/Catalina/localhost/appname.xml context file for an application,
> if it needs one.  The META-INF/context.xml file in a web application
> is not trusted.  (Moreover, Tomcat will refuse to start an application
> if META-INF/context.xml is present, but explicit context file in conf/
> is not configured).
> 
> 2) In case of "user" applications deployed by
> org.apache.catalina.startup.UserConfig listener:
> 
> A Context is created programmatically, with default settings (the
> value for StandardContext.configFile is null). The
> META-INF/context.xml file in a web application is not read.

Sounds good. Thanks for the clarification.

>> What kind of listener do you think would be appropriate to use, here?
>>
> 
> I just say that it is another possible way to implement this feature.
> A LifecycleListener that reacts on "after_start" or "configure_start"
> event of a Context.

I'm comfortable with a StandardContext feature for the moment. If others
feel strongly about a different strategy, please speak up and I'll move
this into a Listener.

On the other hand, a Listener requires (arguably) less configuration and
can be enabled either on a per-context basis or at the global level.

I'll have to think about this a bit more.

>>> 2. A web application is not allowed to access classes in org.apache.catalina.
>>>
>>> Interfaces exposed to web applications (InstanceManager etc.) are in
>>> org.apache.tomcat package
>>> and access to them is granted with the following lines in catalina.policy:
>>>
>>>     // All JSPs need to be able to read this package
>>>     permission java.lang.RuntimePermission
>>> "accessClassInPackage.org.apache.tomcat";
>>
>> Hmm... I didn't test with a SecurityManager enabled; didn't think about
>> that.
>>
>> I think Mark committed the original work for CredentialHandler based
>> upon my ideas... Mark, is there any objection to moving
>> CredentialHandler to org.apache.tomcat? It would be an API change, but
>> we could leave org.apache.catalina.CredentialHandler as a sub-interface
>> of the new one in org.apache.tomcat.
>>
>> Another idea would be continue to use the existing CredentialHandler
>> internally, and put the "SafeCredentialHandler" for access through the
>> ServletContext into org.apache.tomcat.
> 
> Your web application that uses CredentialHandler class, does it also
> use other internal classes?

No, not right now.

> Do you need access to a Realm?  Or you are accessing a database / LDAP
> directory directly, behind a Realm's back?

For writing new credentials (e.g. password-change, registration, etc.),
I write directly to the database, as Tomcat has no facilities for this
kind of thing. I have no need to access the Realm when its primary
purpose is authentication and authorization. That can already be
accomplished using HttpServletRequest.login if necessary.

> E.g. LockOutRealm has a cache of locked-out users. You may want to
> call LockOutRealm.unlock(username) if user's password is being reset
> by an administrator.

That exposes another opportunity to interact with Tomcat's
authentication mechanisms. I wonder how most people are actually working
with Tomcat's authentication due to it's non-portable nature. For $work,
we use securityfilter which is somewhat more portable (though it binds
us to sf, of course). The lack of standard interfaces for this kind of
stuff makes this type of work awkward.

I found myself writing code using reflection to make the calls to
CredentialHandler because I didn't want a compile-time dependency on Tomcat.

> If CredentialHandler is not the only class you need access to then
> moving it alone does not solve your issue.

Of course.

> To allow access to other classes there has to be an explicit grant in
> catalina.policy file, like there is one for Tomcat Manager
> application.
> 
> I think it is OK to leave CredentialHandler as is as long as it is not
> exposed by default.

I can imagine an application wanting to use both a SecurityManager *and*
having access to the CredentialHandler, and in the current state, the
two are mutually-exclusive. I would like to fix that. The question is
only how best to do it. I'll wait for some feedback from others before
making a decision.

-chris

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: Adding CredentialHandler to ServletContext so applications can get to it

Posted by Konstantin Kolinko <kn...@gmail.com>.
2015-11-22 22:26 GMT+03:00 Christopher Schultz <ch...@christopherschultz.net>:
> Konstantin,
>
> On 11/21/15 10:07 AM, Konstantin Kolinko wrote:
>> 2015-11-20 15:47 GMT+03:00 Christopher Schultz <ch...@christopherschultz.net>:
>>> All,
>>>
>>> I thought there was a BZ issue for this, but I didn't find one. It's
>>> been suggested (and I agree completely) that an application ought to be
>>> able to fetch the CredentialHandler for the context's realm so that it
>>> could mutate user credentials in the same way that the Realm expects to
>>> do. That allows applications to change user's passwords, for instance.
>>>
>>
>> In review of r1715434
>>
>> 1. I think this has to be an opt-in feature.
>>
>> I do not see high risk though.  In theory, if stored credentials are
>> known then CredentialHandler allows untrusted applications to test
>> potential passwords without triggering a lock-out timer.
>
> Hmm, I hadn't thought of that. I could pretty easily make this a setting
> for the context.
>
>> I am OK with this being a StandardContext feature, though there is an
>> alternative way: to implement publishing the attribute with a
>> Listener.
>
> But then if you don't trust the application, how do you trust the
> context's settings?
>

1) When SecurityManager is enabled then the value of "deployXML"
attribute of a StandatdHost by default is "false".

It means that an administrator has to explicitly configure a
conf/Catalina/localhost/appname.xml context file for an application,
if it needs one.  The META-INF/context.xml file in a web application
is not trusted.  (Moreover, Tomcat will refuse to start an application
if META-INF/context.xml is present, but explicit context file in conf/
is not configured).

2) In case of "user" applications deployed by
org.apache.catalina.startup.UserConfig listener:

A Context is created programmatically, with default settings (the
value for StandardContext.configFile is null). The
META-INF/context.xml file in a web application is not read.


> What kind of listener do you think would be appropriate to use, here?
>

I just say that it is another possible way to implement this feature.
A LifecycleListener that reacts on "after_start" or "configure_start"
event of a Context.

>> 2. A web application is not allowed to access classes in org.apache.catalina.
>>
>> Interfaces exposed to web applications (InstanceManager etc.) are in
>> org.apache.tomcat package
>> and access to them is granted with the following lines in catalina.policy:
>>
>>     // All JSPs need to be able to read this package
>>     permission java.lang.RuntimePermission
>> "accessClassInPackage.org.apache.tomcat";
>
> Hmm... I didn't test with a SecurityManager enabled; didn't think about
> that.
>
> I think Mark committed the original work for CredentialHandler based
> upon my ideas... Mark, is there any objection to moving
> CredentialHandler to org.apache.tomcat? It would be an API change, but
> we could leave org.apache.catalina.CredentialHandler as a sub-interface
> of the new one in org.apache.tomcat.
>
> Another idea would be continue to use the existing CredentialHandler
> internally, and put the "SafeCredentialHandler" for access through the
> ServletContext into org.apache.tomcat.

Your web application that uses CredentialHandler class, does it also
use other internal classes?

Do you need access to a Realm?  Or you are accessing a database / LDAP
directory directly, behind a Realm's back?

E.g. LockOutRealm has a cache of locked-out users. You may want to
call LockOutRealm.unlock(username) if user's password is being reset
by an administrator.

If CredentialHandler is not the only class you need access to then
moving it alone does not solve your issue.
To allow access to other classes there has to be an explicit grant in
catalina.policy file, like there is one for Tomcat Manager
application.


I think it is OK to leave CredentialHandler as is as long as it is not
exposed by default.

Best regards,
Konstantin Kolinko

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: Adding CredentialHandler to ServletContext so applications can get to it

Posted by Christopher Schultz <ch...@christopherschultz.net>.
Konstantin,

On 11/21/15 10:07 AM, Konstantin Kolinko wrote:
> 2015-11-20 15:47 GMT+03:00 Christopher Schultz <ch...@christopherschultz.net>:
>> All,
>>
>> I thought there was a BZ issue for this, but I didn't find one. It's
>> been suggested (and I agree completely) that an application ought to be
>> able to fetch the CredentialHandler for the context's realm so that it
>> could mutate user credentials in the same way that the Realm expects to
>> do. That allows applications to change user's passwords, for instance.
>>
> 
> In review of r1715434
> 
> 1. I think this has to be an opt-in feature.
> 
> I do not see high risk though.  In theory, if stored credentials are
> known then CredentialHandler allows untrusted applications to test
> potential passwords without triggering a lock-out timer.

Hmm, I hadn't thought of that. I could pretty easily make this a setting
for the context.

> I am OK with this being a StandardContext feature, though there is an
> alternative way: to implement publishing the attribute with a
> Listener.

But then if you don't trust the application, how do you trust the
context's settings?

What kind of listener do you think would be appropriate to use, here?

> 2. A web application is not allowed to access classes in org.apache.catalina.
> 
> Interfaces exposed to web applications (InstanceManager etc.) are in
> org.apache.tomcat package
> and access to them is granted with the following lines in catalina.policy:
> 
>     // All JSPs need to be able to read this package
>     permission java.lang.RuntimePermission
> "accessClassInPackage.org.apache.tomcat";

Hmm... I didn't test with a SecurityManager enabled; didn't think about
that.

I think Mark committed the original work for CredentialHandler based
upon my ideas... Mark, is there any objection to moving
CredentialHandler to org.apache.tomcat? It would be an API change, but
we could leave org.apache.catalina.CredentialHandler as a sub-interface
of the new one in org.apache.tomcat.

Another idea would be continue to use the existing CredentialHandler
internally, and put the "SafeCredentialHandler" for access through the
ServletContext into org.apache.tomcat.

WDYT

Thanks,
-chris

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: Adding CredentialHandler to ServletContext so applications can get to it

Posted by Konstantin Kolinko <kn...@gmail.com>.
2015-11-20 15:47 GMT+03:00 Christopher Schultz <ch...@christopherschultz.net>:
> All,
>
> I thought there was a BZ issue for this, but I didn't find one. It's
> been suggested (and I agree completely) that an application ought to be
> able to fetch the CredentialHandler for the context's realm so that it
> could mutate user credentials in the same way that the Realm expects to
> do. That allows applications to change user's passwords, for instance.
>

In review of r1715434

1. I think this has to be an opt-in feature.

I do not see high risk though.  In theory, if stored credentials are
known then CredentialHandler allows untrusted applications to test
potential passwords without triggering a lock-out timer.

I am OK with this being a StandardContext feature, though there is an
alternative way: to implement publishing the attribute with a
Listener.


2. A web application is not allowed to access classes in org.apache.catalina.

Interfaces exposed to web applications (InstanceManager etc.) are in
org.apache.tomcat package
and access to them is granted with the following lines in catalina.policy:

    // All JSPs need to be able to read this package
    permission java.lang.RuntimePermission
"accessClassInPackage.org.apache.tomcat";


Best regards,
Konstantin Kolinko

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: Adding CredentialHandler to ServletContext so applications can get to it

Posted by Christopher Schultz <ch...@christopherschultz.net>.
Mark,

On 11/20/15 9:26 AM, Mark Thomas wrote:
> On 20/11/2015 14:23, Mark Thomas wrote:
>>> 2015-11-20 13:47 GMT+01:00 Christopher Schultz <chris@christopherschultz.net
> 
> <snip/>
> 
>>>> why does StandardContext.startInternal call
>>>> getRealmInternal().start() when ContainerBase.setRealm already calls
>>>> Realm.start()?)
>>
>> Not sure right now. I'll take a quick look.
> 
> Because setRealm() only calls start() on the Realm if the container is
> already started. i.e. it handles replacement of the Realm while the
> container is running.

Gotcha. I don't think I'm going to mess with that anyway.

Thanks,
-chris

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: Adding CredentialHandler to ServletContext so applications can get to it

Posted by Mark Thomas <ma...@apache.org>.
On 20/11/2015 14:23, Mark Thomas wrote:
>> 2015-11-20 13:47 GMT+01:00 Christopher Schultz <chris@christopherschultz.net

<snip/>

>>> why does StandardContext.startInternal call
>>> getRealmInternal().start() when ContainerBase.setRealm already calls
>>> Realm.start()?)
> 
> Not sure right now. I'll take a quick look.

Because setRealm() only calls start() on the Realm if the container is
already started. i.e. it handles replacement of the Realm while the
container is running.

Mark


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: Adding CredentialHandler to ServletContext so applications can get to it

Posted by Christopher Schultz <ch...@christopherschultz.net>.
Mark,

On 11/20/15 9:23 AM, Mark Thomas wrote:
> On 20/11/2015 13:07, Rémy Maucherat wrote:
>> 2015-11-20 13:47 GMT+01:00 Christopher Schultz <chris@christopherschultz.net
>>> :
>>
>>> All,
>>>
>>> I thought there was a BZ issue for this, but I didn't find one. It's
>>> been suggested (and I agree completely) that an application ought to be
>>> able to fetch the CredentialHandler for the context's realm so that it
>>> could mutate user credentials in the same way that the Realm expects to
>>> do. That allows applications to change user's passwords, for instance.
>>>
>>> So I think I've identified where to put this: around
>>> StandardContext.java:5131 (in Tomcat 9/trunk). But I have a few
>>> questions about it.
>>>
>>> It occurs to me that we won't want applications to change the
>>> CredentialHandler in any way -- such as changing the salt length for a
>>> DigestCredentialHandler, etc., so I think it makes sense to wrap the
>>> CredentialHandler in a "safe" class that doesn't allow access to the
>>> real CredentialHandler. That's fairly easy to do.
>>>
>>> Also, we probably want the CredentialHandler in the application scope to
>>> always be in sync with the CredentialHandler actually being used by the
>>> Realm. Realm has a setCredentialHandler(CredentialHandler) method, so
>>> theoretically, the CH could change at any time.
>>>
>>> Also, the Realm could theoretically change at any time as well.
>>>
>>> So, this "safe" wrapper could just call
>>> getRealmInternal().getCredentialHandler().underlyingMethod and always
>>> use that. But, ContainerBase.getRealmInternal uses a lock, and I'm not
>>> sure how expensive that is. Is there any concern over performance, here?
> 
> For what an application is going to use this for? I don't think there is
> a performance concern here. I'd go this route.

I tend to agree, but I thought I'd put it to the group.

One more thing: what happens if the CredentialHandler is null? Should I
just let it throw a NPE?

-chris

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: Adding CredentialHandler to ServletContext so applications can get to it

Posted by Mark Thomas <ma...@apache.org>.
On 20/11/2015 13:07, Rémy Maucherat wrote:
> 2015-11-20 13:47 GMT+01:00 Christopher Schultz <chris@christopherschultz.net
>> :
> 
>> All,
>>
>> I thought there was a BZ issue for this, but I didn't find one. It's
>> been suggested (and I agree completely) that an application ought to be
>> able to fetch the CredentialHandler for the context's realm so that it
>> could mutate user credentials in the same way that the Realm expects to
>> do. That allows applications to change user's passwords, for instance.
>>
>> So I think I've identified where to put this: around
>> StandardContext.java:5131 (in Tomcat 9/trunk). But I have a few
>> questions about it.
>>
>> It occurs to me that we won't want applications to change the
>> CredentialHandler in any way -- such as changing the salt length for a
>> DigestCredentialHandler, etc., so I think it makes sense to wrap the
>> CredentialHandler in a "safe" class that doesn't allow access to the
>> real CredentialHandler. That's fairly easy to do.
>>
>> Also, we probably want the CredentialHandler in the application scope to
>> always be in sync with the CredentialHandler actually being used by the
>> Realm. Realm has a setCredentialHandler(CredentialHandler) method, so
>> theoretically, the CH could change at any time.
>>
>> Also, the Realm could theoretically change at any time as well.
>>
>> So, this "safe" wrapper could just call
>> getRealmInternal().getCredentialHandler().underlyingMethod and always
>> use that. But, ContainerBase.getRealmInternal uses a lock, and I'm not
>> sure how expensive that is. Is there any concern over performance, here?

For what an application is going to use this for? I don't think there is
a performance concern here. I'd go this route.

>> Another option would be to override setRealm and capture the Realm,
>> there... except that StandardContext never calls setRealm directly.
>> (Presumably, it's called by another component when constructing the
>> StandardContext, but then why does StandardContext.startInternal call
>> getRealmInternal().start() when ContainerBase.setRealm already calls
>> Realm.start()?)

Not sure right now. I'll take a quick look.

>> I'd appreciate some thoughts on the above; I don't want to have to
>> commit 12 patches for this due to misunderstandings about what's going
>> on with StandardContext.
>>
> +1 for removing those pointless locks.

-1 for removing those entirely necessary locks. See svn log for explanation.

Mark


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: Adding CredentialHandler to ServletContext so applications can get to it

Posted by Rémy Maucherat <re...@apache.org>.
2015-11-20 15:17 GMT+01:00 Christopher Schultz <chris@christopherschultz.net
>:

> Rémy,
>
>
> > +1 for removing those pointless locks.
>
> Okay. I'm not going to remove those myself, though. If I don't hear from
> anyone in a while, I'll commit my patch which always calls delegates to
> getRealmInternal.getCredentialHandler.call, as its the simplest patch
> that will cover all the cases I can think of.
>
> I was kidding, it's not very useful (replacing a realm on a live server,
mmmm), but it's not an expensive sync either.

Originally, there was supposed to be some user editing features
(UserDatabaseRealm from Tomcat 4.1). But it stayed as a memory realm, that
doesn't use the credential handler.

Rémy

Re: Adding CredentialHandler to ServletContext so applications can get to it

Posted by Christopher Schultz <ch...@christopherschultz.net>.
Rémy,

On 11/20/15 8:07 AM, Rémy Maucherat wrote:
> 2015-11-20 13:47 GMT+01:00 Christopher Schultz <chris@christopherschultz.net
>> :
> 
>> All,
>>
>> I thought there was a BZ issue for this, but I didn't find one. It's
>> been suggested (and I agree completely) that an application ought to be
>> able to fetch the CredentialHandler for the context's realm so that it
>> could mutate user credentials in the same way that the Realm expects to
>> do. That allows applications to change user's passwords, for instance.
>>
>> So I think I've identified where to put this: around
>> StandardContext.java:5131 (in Tomcat 9/trunk). But I have a few
>> questions about it.
>>
>> It occurs to me that we won't want applications to change the
>> CredentialHandler in any way -- such as changing the salt length for a
>> DigestCredentialHandler, etc., so I think it makes sense to wrap the
>> CredentialHandler in a "safe" class that doesn't allow access to the
>> real CredentialHandler. That's fairly easy to do.
>>
>> Also, we probably want the CredentialHandler in the application scope to
>> always be in sync with the CredentialHandler actually being used by the
>> Realm. Realm has a setCredentialHandler(CredentialHandler) method, so
>> theoretically, the CH could change at any time.
>>
>> Also, the Realm could theoretically change at any time as well.
>>
>> So, this "safe" wrapper could just call
>> getRealmInternal().getCredentialHandler().underlyingMethod and always
>> use that. But, ContainerBase.getRealmInternal uses a lock, and I'm not
>> sure how expensive that is. Is there any concern over performance, here?
>>
>> Another option would be to override setRealm and capture the Realm,
>> there... except that StandardContext never calls setRealm directly.
>> (Presumably, it's called by another component when constructing the
>> StandardContext, but then why does StandardContext.startInternal call
>> getRealmInternal().start() when ContainerBase.setRealm already calls
>> Realm.start()?)
>>
>> I'd appreciate some thoughts on the above; I don't want to have to
>> commit 12 patches for this due to misunderstandings about what's going
>> on with StandardContext.
>
> +1 for removing those pointless locks.

Okay. I'm not going to remove those myself, though. If I don't hear from
anyone in a while, I'll commit my patch which always calls delegates to
getRealmInternal.getCredentialHandler.call, as its the simplest patch
that will cover all the cases I can think of.

-chris

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: Adding CredentialHandler to ServletContext so applications can get to it

Posted by Rémy Maucherat <re...@apache.org>.
2015-11-20 13:47 GMT+01:00 Christopher Schultz <chris@christopherschultz.net
>:

> All,
>
> I thought there was a BZ issue for this, but I didn't find one. It's
> been suggested (and I agree completely) that an application ought to be
> able to fetch the CredentialHandler for the context's realm so that it
> could mutate user credentials in the same way that the Realm expects to
> do. That allows applications to change user's passwords, for instance.
>
> So I think I've identified where to put this: around
> StandardContext.java:5131 (in Tomcat 9/trunk). But I have a few
> questions about it.
>
> It occurs to me that we won't want applications to change the
> CredentialHandler in any way -- such as changing the salt length for a
> DigestCredentialHandler, etc., so I think it makes sense to wrap the
> CredentialHandler in a "safe" class that doesn't allow access to the
> real CredentialHandler. That's fairly easy to do.
>
> Also, we probably want the CredentialHandler in the application scope to
> always be in sync with the CredentialHandler actually being used by the
> Realm. Realm has a setCredentialHandler(CredentialHandler) method, so
> theoretically, the CH could change at any time.
>
> Also, the Realm could theoretically change at any time as well.
>
> So, this "safe" wrapper could just call
> getRealmInternal().getCredentialHandler().underlyingMethod and always
> use that. But, ContainerBase.getRealmInternal uses a lock, and I'm not
> sure how expensive that is. Is there any concern over performance, here?
>
> Another option would be to override setRealm and capture the Realm,
> there... except that StandardContext never calls setRealm directly.
> (Presumably, it's called by another component when constructing the
> StandardContext, but then why does StandardContext.startInternal call
> getRealmInternal().start() when ContainerBase.setRealm already calls
> Realm.start()?)
>
> I'd appreciate some thoughts on the above; I don't want to have to
> commit 12 patches for this due to misunderstandings about what's going
> on with StandardContext.
>
> +1 for removing those pointless locks.

Rémy