From: "Brian C. Lane" <bcl(a)redhat.com&gt; 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 --- pyanaconda/bootloader.py | 13 ++----------- pyanaconda/constants.py | 1 + pyanaconda/iutil.py | 18 +++++++++++++++++- pyanaconda/users.py | 18 ++---------------- tests/pyanaconda_tests/iutil_test.py | 10 ++++++++++ 5 files changed, 32 insertions(+), 28 deletions(-) diff --git a/pyanaconda/bootloader.py b/pyanaconda/bootloader.py index 264f8a6..1e1a95e 100644 --- a/pyanaconda/bootloader.py +++ b/pyanaconda/bootloader.py @@ -1104,17 +1104,8 @@ def _encrypt_password(self): if not self.password: raise BootLoaderError("cannot encrypt empty password") - # Used for ascii_letters and digits constants - import string # pylint: disable=deprecated-module - import crypt - import random - salt = "$6$" - salt_len = 16 - salt_chars = string.ascii_letters + string.digits + './' - - rand_gen = random.SystemRandom() - salt += "".join(rand_gen.choice(salt_chars) for i in range(salt_len)) - self.encrypted_password = crypt.crypt(self.password, salt) + # Encrypt using sha512 and 16 character salt + self.encrypted_password = iutil.encrypt_password(self.password, "$6$", 16) def write_config_password(self, config): """ Write password-related configuration. """ diff --git a/pyanaconda/constants.py b/pyanaconda/constants.py index df1a171..0141aad 100644 --- a/pyanaconda/constants.py +++ b/pyanaconda/constants.py @@ -152,6 +152,7 @@ # all ASCII characters PW_ASCII_CHARS = string.digits + string.ascii_letters + string.punctuation + " " +SALT_CHARS = string.ascii_letters + string.digits + './' # Recognizing a tarfile TAR_SUFFIX = (".tar", ".tbz", ".tgz", ".txz", ".tar.bz2", "tar.gz", "tar.xz") diff --git a/pyanaconda/iutil.py b/pyanaconda/iutil.py index 5a327ba..46506c5 100644 --- a/pyanaconda/iutil.py +++ b/pyanaconda/iutil.py @@ -35,6 +35,8 @@ import gettext import signal import sys +import crypt +import random import requests from requests_file import FileAdapter @@ -47,7 +49,7 @@ from pyanaconda.flags import flags from pyanaconda.constants import DRACUT_SHUTDOWN_EJECT, TRANSLATIONS_UPDATE_DIR, UNSUPPORTED_HW -from pyanaconda.constants import SCREENSHOTS_DIRECTORY, SCREENSHOTS_TARGET_DIRECTORY +from pyanaconda.constants import SCREENSHOTS_DIRECTORY, SCREENSHOTS_TARGET_DIRECTORY, SALT_CHARS from pyanaconda.regexes import URL_PARSE from pyanaconda.i18n import _ @@ -1373,3 +1375,17 @@ def save_screenshots(): except OSError: log.exception("saving screenshots to installed system failed") + +def encrypt_password(password, algo, salt_len): + """ Encrypt a password using the selected algorithm and salt length. + + :param str password: password to encrypt + :param str algo: Algorithm to use, from crypt(3) manpage + :param int salt_len: Lenth of salt to generate + + This uses the system urandom as the random number generator + """ + salt = algo or "" + rand_gen = random.SystemRandom() + salt += "".join(rand_gen.choice(SALT_CHARS) for i in range(salt_len)) + return crypt.crypt(password, salt) diff --git a/pyanaconda/users.py b/pyanaconda/users.py index 4fc38d1..9f6035a 100644 --- a/pyanaconda/users.py +++ b/pyanaconda/users.py @@ -20,9 +20,6 @@ # # Used for ascii_letters and digits constants -import string # pylint: disable=deprecated-module -import crypt -import random import os import os.path import subprocess @@ -58,21 +55,10 @@ def getPassAlgo(authconfigStr): # $6$ SHA512 def cryptPassword(password, algo=None): salts = {'md5': '$1$', 'sha256': '$5$', 'sha512': '$6$'} - saltlen = 2 - - if algo is None: + if algo not in salts: algo = 'sha512' - if algo == 'md5' or algo == 'sha256' or algo == 'sha512': - saltlen = 16 - - saltstr = salts[algo] - - for _i in range(saltlen): - saltstr = saltstr + random.choice(string.ascii_letters + - string.digits + './') - - cryptpw = crypt.crypt(password, saltstr) + cryptpw = iutil.encrypt_password(password, salts[algo], 16) if cryptpw is None: exn = PasswordCryptError(algo=algo) if errorHandler.cb(exn) == ERROR_RAISE: diff --git a/tests/pyanaconda_tests/iutil_test.py b/tests/pyanaconda_tests/iutil_test.py index 55d5df2..a286ea6 100644 --- a/tests/pyanaconda_tests/iutil_test.py +++ b/tests/pyanaconda_tests/iutil_test.py @@ -28,6 +28,7 @@ import tempfile import signal import shutil +import crypt from .test_constants import ANACONDA_TEST_DIR from timer import timer @@ -807,3 +808,12 @@ def open_with_perm_test(self): os.umask(old_umask) finally: shutil.rmtree(test_dir) + +class EncryptPasswordTests(unittest.TestCase): + def encrypt_password_test(self): + """ Test the encrypt_password function""" + for algo in ["$1$", "$5$", "$6$"]: + 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) -- To view this commit on github, visit https://github.com/rhinstaller/anaconda/commit/be93ef62b238d6c4f717f823d3...

We do use crypt.crypt (see iutil.encrypt_password), this is just testing that the created password works. -- To view this pull request on github, visit https://github.com/rhinstaller/anaconda/pull/421#discussion_r42771566

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. -- To view this pull request on github, visit https://github.com/rhinstaller/anaconda/pull/421#discussion_r43018351