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

@@ -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)

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']:
+            if encrypt:
-                password = do_encrypt(plaintext_password, params['encrypt'], salt=salt)
+                password = do_encrypt(plaintext_password, encrypt, salt=salt)
                 ret.append(password)
             else:
                 ret.append(plaintext_password)

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),
+        '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),
+        '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),
+        '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),
+        '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)

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