Bug 30981 - Unsalted user password hashes in pwhistory
Unsalted user password hashes in pwhistory
Status: CLOSED FIXED
Product: UCS
Classification: Unclassified
Component: UMC - Users
UCS 3.1
Other Linux
: P5 normal (vote)
: UCS 3.2
Assigned To: Lukas Walter
Felix Botner
: interim-2
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-04-04 16:32 CEST by Stefan Gohmann
Modified: 2013-11-19 06:44 CET (History)
3 users (show)

See Also:
What kind of report is it?: ---
What type of bug is this?: ---
Who will be affected by this bug?: ---
How will those affected feel about the bug?: ---
User Pain:
Enterprise Customer affected?:
School Customer affected?:
ISV affected?:
Waiting Support:
Ticket number:
Bug group (optional):
Max CVSS v3 score:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Stefan Gohmann univentionstaff 2013-04-04 16:32:28 CEST
Currently the pwhistory user attribute uses unsalted password hashes.
Comment 1 Fabian Zimmermann 2013-04-04 16:52:54 CEST
it's not a bug, it's a feature and allows password synchronization to GoogleApps. Would be nice to allow some kind of configuration, like:

* Default/Neu-Install: use salted hashes
* Optional/Backward: use unsalted hash
Comment 2 Moritz Muehlenhoff univentionstaff 2013-05-31 10:45:20 CEST
We will not ship a UCS 3.1-2 release; the next UCS release will be UCS 3.2.

As such, this bug is moved to the new target milestone.
Comment 3 Lukas Walter univentionstaff 2013-09-12 17:32:35 CEST
A salted password history is impossible, or at least anything but trivial, because a new random salt is/should be generated for every new password which means that comparing the new salted password hash would never match any old salted password hash in the history.


In course of this bug I have changed another strange thing about how passwords were stored in the history:

Formerly, they have been stored as SHA1 hash while the current password salted hash is created using the hashing method defined in the UCRV "password/hashing/method".

I've adapted the password history to also evaluate the UCRV "password/hashing/method".
Comment 4 Felix Botner univentionstaff 2013-09-27 10:26:15 CEST
OK this seems to work , but (In reply to Lukas Walter from comment #3)
> A salted password history is impossible, or at least anything but trivial,
> because a new random salt is/should be generated for every new password
> which means that comparing the new salted password hash would never match
> any old salted password hash in the history.
> 

Why?

save the salted hash in pwHistory 

# {crypt}$method_id$salt$hash
{crypt}$6$ClLgXni.ShrHOMhS$T7s5LGJ1yK6aMZ4Gv5xDrHqREvlkQ6MhgQyAzPH1QJyXonnuPihsQz7YG8aaBic.r5Co.fByrHxZg/5wOPOyb1

and during password change for every entry in pwHistory do

get salt (ClLgXni.ShrHOMhS), method_id (6) and run crypt.crypt() with the new password

crypt.crypt("univention".encode('utf-8'), '$%s$%s$' % (6, "ClLgXni.ShrHOMhS",))

'$6$ClLgXni.ShrHOMhS$T7s5LGJ1yK6aMZ4Gv5xDrHqREvlkQ6MhgQyAzPH1QJyXonnuPihsQz7YG8aaBic.r5Co.fByrHxZg/5wOPOyb1'

and than compare this with the pwHistory entry 


Second problem:

If we use a new pwHistory compare function all old pwHistory entries are pretty much useless so we need to check both, the new and the old way




and then
Comment 5 Lukas Walter univentionstaff 2013-10-01 12:12:17 CEST
Passwords will be stored in history as suggested. For backwards compatibility, the password history check defaults to the 3.1-1 behaviour for comparison when detecting an old style password hash.


univention-directory-manager-modules (9.0.40-1)
svn r44619


Changelog entry has been modified to match the new changes.
Comment 6 Felix Botner univentionstaff 2013-10-01 14:13:23 CEST
OK - old pwHistory entries are checked correctly
OK - new pwHistory entries are stored as salted crypt hashes (according to 
     password/hashing/method)
OK - new pwHistory entries are checked correctly 
OK - changing password/hashing/method does not break pwHistory
OK - Changelog
Comment 7 Stefan Gohmann univentionstaff 2013-10-09 20:42:00 CEST
(In reply to Fabian Zimmermann from comment #1)
> it's not a bug, it's a feature and allows password synchronization to
> GoogleApps. Would be nice to allow some kind of configuration, like:

Today we released a SAML app for UCS. We also documented the Google Apps for business configuration:
 http://wiki.univention.de/index.php?title=SAML_Identity_Provider#Example_configuration_of_Google_Apps_for_business_as_a_service_provider
Comment 8 Stefan Gohmann univentionstaff 2013-11-19 06:44:23 CET
UCS 3.2 has been released:
 http://docs.univention.de/release-notes-3.2-en.html
 http://docs.univention.de/release-notes-3.2-de.html

If this error occurs again, please use "Clone This Bug".