Am 17.09.2014 um 10:36 schrieb Mark Thomas:
On 16/09/2014 22:14, Christopher Schultz wrote:
Mark,

On 9/16/14 3:39 PM, Mark Thomas wrote:
Updated patch:
http://people.apache.org/~markt/patches/2014-09-16-bug56403-tc8-v2.patch
Looks good, but its missing a configuration for the digester to actually
read the configuration and set-up the CredentialHandler objects at
runtime. Existing MessageDigest-based configs will work, but explicit
class references won't.
Ack. The docs need updating as well.

Speaking of which, I'd like to be able to nest CredentialHandler
instances. The use case is when switching from one type of
password-derivation method to another. We have done this at $work twice
and being able to handle more than one kind of valid credential in the
database is essential.
OK. That seems like a reasonable requirement.
If the mutate method was removed from CredentialHandler we could implement a CombinedCredentialHandler, which holds a list of CredentialHandlers. Those could be asked to handle the credentials in order to support more than one derivation type.

The mutate method could be placed in an interface like MutatingCredentialHandler which then could be used as the basis for the abstract class CredentialHandlerBase.

I wonder if it would be a good idea to introduce a CredentialExtractor interface, which would extract the salt, algorithm, iteration, password-hash from a stored credential.

Given that we are giving better options to users than standard
single-pass MessageDigest password-mutators, we should help them
migrate. The only way to do that would be something like
CombinedCredentialHandler analogous to the CombinedRealm: you will
accept either MessageDigestCredentiaHandler{SHA1} /or/, say, PBKDF,
bcrypt, etc., by checking one CredentialHandler and then the second (or
third?) if the first one fails.

Use of a CombinedCredentialHandler might result in a lot of spurious
warnings in the log about invalid credentials. Maybe the
CombinedCredentialHandler could tell the individual child
CredentialHandlers that they should not log invalid credentials?
Yes, we'll need to make sure the logs don't fill up with false positives.

I'd like to get some other opinions on the public mutate() interface. I
think we might not be able to convince each other ;)
You might be surprised. I was looking at using mutate() from match to
reduce code duplication but if you limit mutate() to just generating new
passwords then I agree there is no need for any other parameters. A
protected method used by both mutate() and match() should work. I'll
take another look.

Regarding thread safety for the SecureRandom, we can do that now. It
doesn't cost us very much and it prevents it tripping us up in the future.

Mark


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



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

Reply via email to