From fdf5dd02b3f17bf92060febdc62985460c7deb35 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Wed, 26 Aug 2020 14:54:38 -0500 Subject: [PATCH] Updates for password hashing (#71120) * Validate salt when using crypt. Respect salt_size in password lookup. Repair salt for bcrypt. Fixes #71107. Fixes #53750. Fixes #36129. * Handle algorithms we don't know about, and make sure to return the salt * Account for old passlib * Add tests for salt constraints * Add changelog fragment * Add test for #36129 --- changelogs/fragments/71107-encryption.yml | 7 ++++ lib/ansible/plugins/lookup/password.py | 16 +++++---- lib/ansible/utils/encrypt.py | 35 +++++++++++++----- test/units/utils/test_encrypt.py | 44 +++++++++++++++++++++++ 4 files changed, 87 insertions(+), 15 deletions(-) create mode 100644 changelogs/fragments/71107-encryption.yml diff --git a/changelogs/fragments/71107-encryption.yml b/changelogs/fragments/71107-encryption.yml new file mode 100644 index 00000000000..d1bae62d61e --- /dev/null +++ b/changelogs/fragments/71107-encryption.yml @@ -0,0 +1,7 @@ +bugfixes: +- password lookup - Try to automatically generate salts using known salt sizes + (https://github.com/ansible/ansible/issues/53750) +- bcrypt hashing - Ensure we repair the salt, to avoid warnings + (https://github.com/ansible/ansible/issues/36129) +- password hashing - Ensure we validate salts against allowed characters and length + when using ``crypt`` (https://github.com/ansible/ansible/issues/71107) diff --git a/lib/ansible/plugins/lookup/password.py b/lib/ansible/plugins/lookup/password.py index 1442515cc5c..f69c610991c 100644 --- a/lib/ansible/plugins/lookup/password.py +++ b/lib/ansible/plugins/lookup/password.py @@ -107,7 +107,7 @@ from ansible.errors import AnsibleError, AnsibleAssertionError from ansible.module_utils._text import to_bytes, to_native, to_text from ansible.parsing.splitter import parse_kv from ansible.plugins.lookup import LookupBase -from ansible.utils.encrypt import do_encrypt, random_password, random_salt +from ansible.utils.encrypt import BaseHash, do_encrypt, random_password, random_salt from ansible.utils.path import makedirs_safe @@ -325,20 +325,24 @@ class LookupModule(LookupBase): else: plaintext_password, salt = _parse_content(content) - if params['encrypt'] and not salt: + encrypt = params['encrypt'] + if encrypt and not salt: changed = True - salt = random_salt() + try: + salt = random_salt(BaseHash.algorithms[encrypt].salt_size) + except KeyError: + salt = random_salt() if changed and b_path != to_bytes('/dev/null'): - content = _format_content(plaintext_password, salt, encrypt=params['encrypt']) + content = _format_content(plaintext_password, salt, encrypt=encrypt) _write_password_file(b_path, content) if first_process: # let other processes continue _release_lock(lockfile) - if params['encrypt']: - password = do_encrypt(plaintext_password, params['encrypt'], salt=salt) + if encrypt: + password = do_encrypt(plaintext_password, encrypt, salt=salt) ret.append(password) else: ret.append(plaintext_password) diff --git a/lib/ansible/utils/encrypt.py b/lib/ansible/utils/encrypt.py index 4128901a2a5..21c9c003d67 100644 --- a/lib/ansible/utils/encrypt.py +++ b/lib/ansible/utils/encrypt.py @@ -7,6 +7,7 @@ __metaclass__ = type import crypt import multiprocessing import random +import re import string import sys @@ -23,7 +24,10 @@ try: import passlib import passlib.hash from passlib.utils.handlers import HasRawSalt - + try: + from passlib.utils.binary import bcrypt64 + except ImportError: + from passlib.utils import bcrypt64 PASSLIB_AVAILABLE = True except Exception: pass @@ -61,12 +65,12 @@ def random_salt(length=8): class BaseHash(object): - algo = namedtuple('algo', ['crypt_id', 'salt_size', 'implicit_rounds']) + algo = namedtuple('algo', ['crypt_id', 'salt_size', 'implicit_rounds', 'salt_exact']) algorithms = { - 'md5_crypt': algo(crypt_id='1', salt_size=8, implicit_rounds=None), - 'bcrypt': algo(crypt_id='2a', salt_size=22, implicit_rounds=None), - 'sha256_crypt': algo(crypt_id='5', salt_size=16, implicit_rounds=5000), - 'sha512_crypt': algo(crypt_id='6', salt_size=16, implicit_rounds=5000), + 'md5_crypt': algo(crypt_id='1', salt_size=8, implicit_rounds=None, salt_exact=False), + 'bcrypt': algo(crypt_id='2a', salt_size=22, implicit_rounds=None, salt_exact=True), + 'sha256_crypt': algo(crypt_id='5', salt_size=16, implicit_rounds=5000, salt_exact=False), + 'sha512_crypt': algo(crypt_id='6', salt_size=16, implicit_rounds=5000, salt_exact=False), } def __init__(self, algorithm): @@ -91,7 +95,14 @@ class CryptHash(BaseHash): def _salt(self, salt, salt_size): salt_size = salt_size or self.algo_data.salt_size - return salt or random_salt(salt_size) + ret = salt or random_salt(salt_size) + if re.search(r'[^./0-9A-Za-z]', ret): + raise AnsibleError("invalid characters in salt") + if self.algo_data.salt_exact and len(ret) != self.algo_data.salt_size: + raise AnsibleError("invalid salt size") + elif not self.algo_data.salt_exact and len(ret) > self.algo_data.salt_size: + raise AnsibleError("invalid salt size") + return ret def _rounds(self, rounds): if rounds == self.algo_data.implicit_rounds: @@ -148,9 +159,15 @@ class PasslibHash(BaseHash): if not salt: return None elif issubclass(self.crypt_algo, HasRawSalt): - return to_bytes(salt, encoding='ascii', errors='strict') + ret = to_bytes(salt, encoding='ascii', errors='strict') else: - return to_text(salt, encoding='ascii', errors='strict') + ret = to_text(salt, encoding='ascii', errors='strict') + + # Ensure the salt has the correct padding + if self.algorithm == 'bcrypt': + ret = bcrypt64.repair_unused(ret) + + return ret def _clean_rounds(self, rounds): algo_data = self.algorithms.get(self.algorithm) diff --git a/test/units/utils/test_encrypt.py b/test/units/utils/test_encrypt.py index 2cbe828a60c..b3eb9cf372b 100644 --- a/test/units/utils/test_encrypt.py +++ b/test/units/utils/test_encrypt.py @@ -166,3 +166,47 @@ def test_random_salt(): assert len(res) == 8 for res_char in res: assert res_char in expected_salt_candidate_chars + + +def test_invalid_crypt_salt(): + pytest.raises( + AnsibleError, + encrypt.CryptHash('bcrypt')._salt, + '_', + None + ) + encrypt.CryptHash('bcrypt')._salt('1234567890123456789012', None) + pytest.raises( + AnsibleError, + encrypt.CryptHash('bcrypt')._salt, + 'kljsdf', + None + ) + encrypt.CryptHash('sha256_crypt')._salt('123456', None) + pytest.raises( + AnsibleError, + encrypt.CryptHash('sha256_crypt')._salt, + '1234567890123456789012', + None + ) + + +def test_passlib_bcrypt_salt(recwarn): + passlib_exc = pytest.importorskip("passlib.exc") + + secret = 'foo' + salt = '1234567890123456789012' + repaired_salt = '123456789012345678901u' + expected = '$2b$12$123456789012345678901uMv44x.2qmQeefEGb3bcIRc1mLuO7bqa' + + p = encrypt.PasslibHash('bcrypt') + + result = p.hash(secret, salt=salt) + passlib_warnings = [w.message for w in recwarn if isinstance(w.message, passlib_exc.PasslibHashWarning)] + assert len(passlib_warnings) == 0 + assert result == expected + + recwarn.clear() + + result = p.hash(secret, salt=repaired_salt) + assert result == expected