Committed (finally), thanks!

Oh, and one other thing. (It's not a security issue though, rather an

interoperability problem.)



In Auth::getCryptedPassword(), there are two password schemes that have

"{MD5}" as an identifier, neither of which have "md5" as the parameter

name. As the default scheme is hex-encoded MD5 and not base64-encoded,

this makes my patch not work with LDAP backends.



An updated version that also fixes the issue of not setting $encryption

if not finding a scheme identifier is attached.

As you may have guessed, I don't really see a security problem with this

patch -- it's just a way of finding out what scheme a password is stored

in, and then comparing the plaintext accordingly. Here's two points that

your response made me think about and my reasons for why they're not

security problems.

In case that an unknown (or no) encryption scheme is found,

Auth::getCryptedPassword() assumes hex-encoded MD5 as default. In the

worst case, we may end up comparing two passwords hashed with different

functions. As finding collisions (i.e. two different inputs that produce

the same output) is assumed to be infeasible for any (good) hash function,

and, more generally, finding the pre-image of a hash function (i.e.

finding the input given only the output) is also thought to be difficult,

finding any input value for $plaintext for which the MD5-hex hash is the

same as $encrypted (hashed and encoded with a different scheme) is

difficult as well.

The regular expression for finding the scheme is also very simple -- it

looks for anything within curly brackets and uses it as the scheme name.

Doing Bad Things(tm) with that is impossible because (a) $encrypted is

from a trusted source and (b) if the scheme can't be found, the above

kicks in.



Maybe this behaviour could be turned on by a configuration option, with

the default set to disabled.
Yes. Sorry, that must have slipped my mind, but is trivial to fix. Tell me

if you want me to do it.

I'm trying to wrap my head around whether or not this is a bad idea 
security-wise - it seems dodgy to me. Also, your patch doesn't take 
into account _not_ finding the encryption type and never sets 
$encryption for that case.

Passwd's comparePassword() assumes that the old password is in the 
same format the new one will be in (set in the configuration). This 
need not always be true (in our case, we have a number of legacy CRYPT 
passwords but we'd like the new ones to be SSHA).



The attached patch looks for {scheme} and uses this as the password 
(encryption) scheme to pass to Auth::getCryptedPassword(). It also 
compares the passwords with the scheme identification stripped off, as 
this makes the scheme identification case-insensitive. This sould also 
fix the remaining issue of bug #2708.