Improve password salt creation (#1229474) #421
Conversation
There was some question as to whether /dev/urandom was being used for generating user password salts, this makes it explicit and reuses the same code for the bootloader password generation. Also adds a test for the new function, iutil.encrypt_password Resolves: rhbz#1229474
| enc_pw = iutil.encrypt_password("DocBrown", algo, 16) | ||
| self.assertEqual(algo, enc_pw[:3]) | ||
| self.assertEqual(crypt.crypt("DocBrown", enc_pw), enc_pw) | ||
| self.assertNotEqual(crypt.crypt("Einstein", enc_pw), enc_pw) |
There was a problem hiding this comment.
Looking at this makes me wonder why we are not using crypt.crypt() instead of our custom code? Looks like it even got some improvements with Python 3 which were backported to 2.7.
There was a problem hiding this comment.
Looking at this makes me wonder why we are not using crypt.crypt()
We do use crypt.crypt (see iutil.encrypt_password), this is just testing that the created password works.
There was a problem hiding this comment.
I saw that. What I meant above is that AFAICT we duplicate things that crypt.crypt() now can do for us. There's crypt.mksalt() and crypt.crypt() can generate the salt on its own if we give it the hashing algorithm. The only missing thing seems to be the length of the salt, but I'm not sure if we really use that with any non-standard lengths.
There was a problem hiding this comment.
For 3.4 and later crypt.mksalt is ok, for 2.7 it isn't. Salt length is really always 16 since we don't allow DES. I guess I could simplify everything for master and leave this for rhel7-branch.
|
Looks good to me except for the above question. |
|
Pushed. |
There was some question as to whether /dev/urandom was being used for
generating user password salts, this makes it explicit and reuses the
same code for the bootloader password generation.
Also adds a test for the new function, iutil.encrypt_password
Resolves: rhbz#1229474