[#2865] Support for switching to a new password scheme
| Summary | Support for switching to a new password scheme |
| Queue | Passwd |
| Queue Version | HEAD |
| Type | Enhancement |
| State | Resolved |
| Priority | 2. Medium |
| Owners | Horde Developers |
| Requester | ulrich-horde (at) topfen (dot) net |
| Created | 10/26/2005 (928 days ago) |
| Due | |
| Updated | 07/19/2007 (297 days ago) |
| Assigned | 11/28/2005 (895 days ago) |
| Resolved | 07/19/2007 (297 days ago) |
| Attachments | Driver.php.v2.patch  |
| Milestone | |
| Patch | 1 |
State ⇒ Resolved
Committed (finally), thanks!State ⇒ Assigned
New Attachment: Driver.php.v2.patch
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.
> I'm trying to wrap my head around whether or not this is a bad idea
> security-wise - it seems dodgy to me.
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.
> Also, your patch doesn't take into account _not_ finding the encryption
> type and never sets $encryption for that case.
Yes. Sorry, that must have slipped my mind, but is trivial to fix. Tell me
if you want me to do it.
State ⇒ Feedback
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.Summary ⇒ Support for switching to a new password scheme
Type ⇒ Enhancement
Queue ⇒ Passwd
Priority ⇒ 2. Medium
State ⇒ New
New Attachment: Driver.php.patch
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.