From 3f74bc08cefccec791c9dc5315185d2396e5c5ac Mon Sep 17 00:00:00 2001 From: Martin Krizek Date: Thu, 18 Jan 2024 08:23:01 +0100 Subject: [PATCH] Remove crypt support from ansible.utils.encrypt (#81721) Fixes #81717 Co-authored-by: Matt Clay --- .../81717-remove-deprecated-crypt-support.yml | 2 + lib/ansible/utils/encrypt.py | 106 +----------------- test/integration/targets/become/aliases | 1 + test/integration/targets/filter_core/aliases | 1 + .../targets/filter_core/meta/main.yml | 3 - .../targets/filter_core/tasks/main.yml | 20 ---- .../targets/keyword_inheritance/aliases | 1 + .../targets/lookup_password/aliases | 1 + .../targets/lookup_password/runme.sh | 4 - test/integration/targets/module_utils/aliases | 1 + test/integration/targets/omit/aliases | 1 + .../targets/setup_passlib_controller/runme.sh | 12 ++ test/integration/targets/slurp/aliases | 1 + test/integration/targets/unarchive/aliases | 1 + test/sanity/ignore.txt | 1 - test/units/utils/test_encrypt.py | 98 +--------------- 16 files changed, 27 insertions(+), 227 deletions(-) create mode 100644 changelogs/fragments/81717-remove-deprecated-crypt-support.yml delete mode 100644 test/integration/targets/filter_core/meta/main.yml create mode 100755 test/integration/targets/setup_passlib_controller/runme.sh diff --git a/changelogs/fragments/81717-remove-deprecated-crypt-support.yml b/changelogs/fragments/81717-remove-deprecated-crypt-support.yml new file mode 100644 index 00000000000..b6b0b71c842 --- /dev/null +++ b/changelogs/fragments/81717-remove-deprecated-crypt-support.yml @@ -0,0 +1,2 @@ +removed_features: + - Remove deprecated crypt support from ansible.utils.encrypt (https://github.com/ansible/ansible/issues/81717) diff --git a/lib/ansible/utils/encrypt.py b/lib/ansible/utils/encrypt.py index 5e1b488b15e..3a279b7cc00 100644 --- a/lib/ansible/utils/encrypt.py +++ b/lib/ansible/utils/encrypt.py @@ -4,9 +4,7 @@ from __future__ import annotations import random -import re import string -import sys from collections import namedtuple @@ -16,8 +14,8 @@ from ansible.module_utils.six import text_type from ansible.module_utils.common.text.converters import to_text, to_bytes from ansible.utils.display import Display -PASSLIB_E = CRYPT_E = None -HAS_CRYPT = PASSLIB_AVAILABLE = False +PASSLIB_E = None +PASSLIB_AVAILABLE = False try: import passlib import passlib.hash @@ -30,12 +28,6 @@ try: except Exception as e: PASSLIB_E = e -try: - import crypt - HAS_CRYPT = True -except Exception as e: - CRYPT_E = e - display = Display() @@ -83,96 +75,6 @@ class BaseHash(object): self.algorithm = algorithm -class CryptHash(BaseHash): - def __init__(self, algorithm): - super(CryptHash, self).__init__(algorithm) - - if not HAS_CRYPT: - raise AnsibleError("crypt.crypt cannot be used as the 'crypt' python library is not installed or is unusable.", orig_exc=CRYPT_E) - - if sys.platform.startswith('darwin'): - raise AnsibleError("crypt.crypt not supported on Mac OS X/Darwin, install passlib python module") - - if algorithm not in self.algorithms: - raise AnsibleError("crypt.crypt does not support '%s' algorithm" % self.algorithm) - - display.deprecated( - "Encryption using the Python crypt module is deprecated. The " - "Python crypt module is deprecated and will be removed from " - "Python 3.13. Install the passlib library for continued " - "encryption functionality.", - version="2.17", - ) - - self.algo_data = self.algorithms[algorithm] - - def hash(self, secret, salt=None, salt_size=None, rounds=None, ident=None): - salt = self._salt(salt, salt_size) - rounds = self._rounds(rounds) - ident = self._ident(ident) - return self._hash(secret, salt, rounds, ident) - - def _salt(self, salt, salt_size): - salt_size = salt_size or self.algo_data.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 self.algorithm == 'bcrypt': - # crypt requires 2 digits for rounds - return rounds or self.algo_data.implicit_rounds - elif rounds == self.algo_data.implicit_rounds: - # Passlib does not include the rounds if it is the same as implicit_rounds. - # Make crypt lib behave the same, by not explicitly specifying the rounds in that case. - return None - else: - return rounds - - def _ident(self, ident): - if not ident: - return self.algo_data.crypt_id - if self.algorithm == 'bcrypt': - return ident - return None - - def _hash(self, secret, salt, rounds, ident): - saltstring = "" - if ident: - saltstring = "$%s" % ident - - if rounds: - if self.algorithm == 'bcrypt': - saltstring += "$%d" % rounds - else: - saltstring += "$rounds=%d" % rounds - - saltstring += "$%s" % salt - - # crypt.crypt throws OSError on Python >= 3.9 if it cannot parse saltstring. - try: - result = crypt.crypt(secret, saltstring) - orig_exc = None - except OSError as e: - result = None - orig_exc = e - - # None as result would be interpreted by some modules (user module) - # as no password at all. - if not result: - raise AnsibleError( - "crypt.crypt does not support '%s' algorithm" % self.algorithm, - orig_exc=orig_exc, - ) - - return result - - class PasslibHash(BaseHash): def __init__(self, algorithm): super(PasslibHash, self).__init__(algorithm) @@ -273,6 +175,4 @@ def passlib_or_crypt(secret, algorithm, salt=None, salt_size=None, rounds=None, def do_encrypt(result, encrypt, salt_size=None, salt=None, ident=None, rounds=None): if PASSLIB_AVAILABLE: return PasslibHash(encrypt).hash(result, salt=salt, salt_size=salt_size, rounds=rounds, ident=ident) - if HAS_CRYPT: - return CryptHash(encrypt).hash(result, salt=salt, salt_size=salt_size, rounds=rounds, ident=ident) - raise AnsibleError("Unable to encrypt nor hash, either crypt or passlib must be installed.", orig_exc=CRYPT_E) + raise AnsibleError("Unable to encrypt nor hash, passlib must be installed", orig_exc=PASSLIB_E) diff --git a/test/integration/targets/become/aliases b/test/integration/targets/become/aliases index 0c490f1c5ff..2d93d9aa49f 100644 --- a/test/integration/targets/become/aliases +++ b/test/integration/targets/become/aliases @@ -2,3 +2,4 @@ destructive shippable/posix/group1 context/target gather_facts/no +setup/always/setup_passlib_controller # required for setup_test_user diff --git a/test/integration/targets/filter_core/aliases b/test/integration/targets/filter_core/aliases index 3005e4b26d0..9f907740da5 100644 --- a/test/integration/targets/filter_core/aliases +++ b/test/integration/targets/filter_core/aliases @@ -1 +1,2 @@ shippable/posix/group4 +setup/always/setup_passlib_controller # required for setup_test_user diff --git a/test/integration/targets/filter_core/meta/main.yml b/test/integration/targets/filter_core/meta/main.yml deleted file mode 100644 index e430ea6f9a5..00000000000 --- a/test/integration/targets/filter_core/meta/main.yml +++ /dev/null @@ -1,3 +0,0 @@ -dependencies: - - role: setup_passlib - when: ansible_facts.distribution == 'MacOSX' diff --git a/test/integration/targets/filter_core/tasks/main.yml b/test/integration/targets/filter_core/tasks/main.yml index 9493b69d55b..8b325a93279 100644 --- a/test/integration/targets/filter_core/tasks/main.yml +++ b/test/integration/targets/filter_core/tasks/main.yml @@ -475,38 +475,18 @@ - password_hash_2 is failed - "'not support' in password_hash_2.msg" -- name: install passlib if needed - pip: - name: passlib - state: present - register: installed_passlib - - name: test using passlib with an unsupported hash type set_fact: foo: '{{"hey"|password_hash("msdcc")}}' ignore_errors: yes register: unsupported_hash_type -- name: remove passlib if it was installed - pip: - name: passlib - state: absent - when: installed_passlib.changed - - assert: that: - unsupported_hash_type.msg == msg vars: msg: "msdcc is not in the list of supported passlib algorithms: md5, blowfish, sha256, sha512" -- name: test password_hash can work with bcrypt without passlib installed - debug: - msg: "{{ 'somestring'|password_hash('bcrypt') }}" - register: crypt_bcrypt - # Some implementations of crypt do not fail outright and return some short value. - failed_when: crypt_bcrypt is failed or (crypt_bcrypt.msg|length|int) != 60 - when: ansible_facts.os_family in ['RedHat', 'Debian'] - - name: Verify to_uuid throws on weird namespace set_fact: foo: '{{"hey"|to_uuid(namespace=22)}}' diff --git a/test/integration/targets/keyword_inheritance/aliases b/test/integration/targets/keyword_inheritance/aliases index a6a33411a34..01741b943d5 100644 --- a/test/integration/targets/keyword_inheritance/aliases +++ b/test/integration/targets/keyword_inheritance/aliases @@ -1,3 +1,4 @@ shippable/posix/group4 context/controller needs/target/setup_test_user +setup/always/setup_passlib_controller # required for setup_test_user diff --git a/test/integration/targets/lookup_password/aliases b/test/integration/targets/lookup_password/aliases index b59832142f2..cf6140f6c47 100644 --- a/test/integration/targets/lookup_password/aliases +++ b/test/integration/targets/lookup_password/aliases @@ -1 +1,2 @@ shippable/posix/group3 +setup/always/setup_passlib_controller # required for setup_test_user diff --git a/test/integration/targets/lookup_password/runme.sh b/test/integration/targets/lookup_password/runme.sh index a3637a7e227..d1a12936372 100755 --- a/test/integration/targets/lookup_password/runme.sh +++ b/test/integration/targets/lookup_password/runme.sh @@ -4,8 +4,4 @@ set -eux source virtualenv.sh -# Requirements have to be installed prior to running ansible-playbook -# because plugins and requirements are loaded before the task runs -pip install passlib - ANSIBLE_ROLES_PATH=../ ansible-playbook runme.yml -e "output_dir=${OUTPUT_DIR}" "$@" diff --git a/test/integration/targets/module_utils/aliases b/test/integration/targets/module_utils/aliases index a1fba9699a9..8474c8a68d8 100644 --- a/test/integration/targets/module_utils/aliases +++ b/test/integration/targets/module_utils/aliases @@ -4,3 +4,4 @@ needs/target/setup_test_user needs/target/setup_remote_tmp_dir context/target destructive +setup/always/setup_passlib_controller # required for setup_test_user diff --git a/test/integration/targets/omit/aliases b/test/integration/targets/omit/aliases index 1bff31cb09e..fea0458b107 100644 --- a/test/integration/targets/omit/aliases +++ b/test/integration/targets/omit/aliases @@ -1,3 +1,4 @@ shippable/posix/group5 needs/target/setup_test_user context/controller +setup/always/setup_passlib_controller # required for setup_test_user diff --git a/test/integration/targets/setup_passlib_controller/runme.sh b/test/integration/targets/setup_passlib_controller/runme.sh new file mode 100755 index 00000000000..c9f13e7825b --- /dev/null +++ b/test/integration/targets/setup_passlib_controller/runme.sh @@ -0,0 +1,12 @@ +#!/usr/bin/env bash + +set -eux + +# Temporary hack for PEP 668 on newer systems. +# Remove once ansible-test can provide targets their own virtual environment. +# Tests which can manage their own virtual environment should not use this approach. +export PIP_BREAK_SYSTEM_PACKAGES=1 + +# Requirements have to be installed prior to running ansible-playbook +# because plugins and requirements are loaded before the task runs +python -m pip install passlib diff --git a/test/integration/targets/slurp/aliases b/test/integration/targets/slurp/aliases index 6eae8bd8ddc..069c660c332 100644 --- a/test/integration/targets/slurp/aliases +++ b/test/integration/targets/slurp/aliases @@ -1,2 +1,3 @@ shippable/posix/group1 destructive +setup/always/setup_passlib_controller # required for setup_test_user diff --git a/test/integration/targets/unarchive/aliases b/test/integration/targets/unarchive/aliases index 961b20518e2..62ef2f75a2e 100644 --- a/test/integration/targets/unarchive/aliases +++ b/test/integration/targets/unarchive/aliases @@ -1,3 +1,4 @@ needs/root shippable/posix/group2 destructive +setup/always/setup_passlib_controller # required for setup_test_user diff --git a/test/sanity/ignore.txt b/test/sanity/ignore.txt index fe7a4ea358b..66683c80bd1 100644 --- a/test/sanity/ignore.txt +++ b/test/sanity/ignore.txt @@ -197,4 +197,3 @@ README.md pymarkdown:line-length test/integration/targets/ansible-vault/invalid_format/README.md pymarkdown:no-bare-urls test/support/README.md pymarkdown:no-bare-urls test/units/cli/test_data/role_skeleton/README.md pymarkdown:line-length -lib/ansible/utils/encrypt.py pylint:ansible-deprecated-version # 2.17 deprecation diff --git a/test/units/utils/test_encrypt.py b/test/units/utils/test_encrypt.py index e99ae9fddf7..4683ce216a8 100644 --- a/test/units/utils/test_encrypt.py +++ b/test/units/utils/test_encrypt.py @@ -3,8 +3,6 @@ from __future__ import annotations -import sys - import pytest from ansible.errors import AnsibleError, AnsibleFilterError @@ -12,18 +10,6 @@ from ansible.plugins.filter.core import get_encrypted_password from ansible.utils import encrypt -class passlib_off(object): - def __init__(self): - self.orig = encrypt.PASSLIB_AVAILABLE - - def __enter__(self): - encrypt.PASSLIB_AVAILABLE = False - return self - - def __exit__(self, exception_type, exception_value, traceback): - encrypt.PASSLIB_AVAILABLE = self.orig - - def assert_hash(expected, secret, algorithm, **settings): assert encrypt.do_encrypt(secret, algorithm, **settings) == expected @@ -35,31 +21,12 @@ def assert_hash(expected, secret, algorithm, **settings): assert excinfo.value.args[0] == "passlib must be installed and usable to hash with '%s'" % algorithm -@pytest.mark.skipif(sys.platform.startswith('darwin'), reason='macOS requires passlib') -@pytest.mark.skipif(sys.version_info >= (3, 13), reason='Python <= 3.12 required') -def test_passlib_or_crypt_no_passlib(): - with passlib_off(): - expected = "$5$rounds=5000$12345678$uAZsE3BenI2G.nA8DpTl.9Dc8JiqacI53pEqRr5ppT7" - assert encrypt.passlib_or_crypt("123", "sha256_crypt", salt="12345678", rounds=5000) == expected - - -def test_passlib_or_crypt(): +@pytest.mark.skipif(not encrypt.PASSLIB_AVAILABLE, reason='passlib must be installed to run this test') +def test_passlib(): expected = "$5$12345678$uAZsE3BenI2G.nA8DpTl.9Dc8JiqacI53pEqRr5ppT7" assert encrypt.passlib_or_crypt("123", "sha256_crypt", salt="12345678", rounds=5000) == expected -@pytest.mark.skipif(sys.platform.startswith('darwin'), reason='macOS requires passlib') -@pytest.mark.skipif(sys.version_info >= (3, 13), reason='Python <= 3.12 required') -def test_encrypt_with_rounds_no_passlib(): - with passlib_off(): - assert_hash("$5$rounds=5000$12345678$uAZsE3BenI2G.nA8DpTl.9Dc8JiqacI53pEqRr5ppT7", - secret="123", algorithm="sha256_crypt", salt="12345678", rounds=5000) - assert_hash("$5$rounds=10000$12345678$JBinliYMFEcBeAXKZnLjenhgEhTmJBvZn3aR8l70Oy/", - secret="123", algorithm="sha256_crypt", salt="12345678", rounds=10000) - assert_hash("$6$rounds=5000$12345678$LcV9LQiaPekQxZ.OfkMADjFdSO2k9zfbDQrHPVcYjSLqSdjLYpsgqviYvTEP/R41yPmhH3CCeEDqVhW1VHr3L.", - secret="123", algorithm="sha512_crypt", salt="12345678", rounds=5000) - - @pytest.mark.skipif(not encrypt.PASSLIB_AVAILABLE, reason='passlib must be installed to run this test') def test_encrypt_with_ident(): assert_hash("$2$12$123456789012345678901ufd3hZRrev.WXCbemqGIV/gmWaTGLImm", @@ -88,20 +55,6 @@ def test_encrypt_with_rounds(): secret="123", algorithm="sha512_crypt", salt="12345678", rounds=5000) -@pytest.mark.skipif(sys.platform.startswith('darwin'), reason='macOS requires passlib') -@pytest.mark.skipif(sys.version_info >= (3, 13), reason='Python <= 3.12 required') -def test_encrypt_default_rounds_no_passlib(): - with passlib_off(): - assert_hash("$1$12345678$tRy4cXc3kmcfRZVj4iFXr/", - secret="123", algorithm="md5_crypt", salt="12345678") - assert_hash("$5$12345678$uAZsE3BenI2G.nA8DpTl.9Dc8JiqacI53pEqRr5ppT7", - secret="123", algorithm="sha256_crypt", salt="12345678") - assert_hash("$6$12345678$LcV9LQiaPekQxZ.OfkMADjFdSO2k9zfbDQrHPVcYjSLqSdjLYpsgqviYvTEP/R41yPmhH3CCeEDqVhW1VHr3L.", - secret="123", algorithm="sha512_crypt", salt="12345678") - - assert encrypt.CryptHash("md5_crypt").hash("123") - - # If passlib is not installed. this is identical to the test_encrypt_default_rounds_no_passlib() test @pytest.mark.skipif(not encrypt.PASSLIB_AVAILABLE, reason='passlib must be installed to run this test') def test_encrypt_default_rounds(): @@ -115,17 +68,6 @@ def test_encrypt_default_rounds(): assert encrypt.PasslibHash("md5_crypt").hash("123") -@pytest.mark.skipif(sys.platform.startswith('darwin'), reason='macOS requires passlib') -@pytest.mark.skipif(sys.version_info >= (3, 13), reason='Python <= 3.12 required') -def test_password_hash_filter_no_passlib(): - with passlib_off(): - assert not encrypt.PASSLIB_AVAILABLE - assert get_encrypted_password("123", "md5", salt="12345678") == "$1$12345678$tRy4cXc3kmcfRZVj4iFXr/" - - with pytest.raises(AnsibleFilterError): - get_encrypted_password("123", "crypt16", salt="12") - - @pytest.mark.skipif(not encrypt.PASSLIB_AVAILABLE, reason='passlib must be installed to run this test') def test_password_hash_filter_passlib(): @@ -153,17 +95,6 @@ def test_password_hash_filter_passlib(): assert get_encrypted_password("123", "pbkdf2_sha256", ident='invalid_ident') -@pytest.mark.skipif(sys.platform.startswith('darwin'), reason='macOS requires passlib') -@pytest.mark.skipif(sys.version_info >= (3, 13), reason='Python <= 3.12 required') -def test_do_encrypt_no_passlib(): - with passlib_off(): - assert not encrypt.PASSLIB_AVAILABLE - assert encrypt.do_encrypt("123", "md5_crypt", salt="12345678") == "$1$12345678$tRy4cXc3kmcfRZVj4iFXr/" - - with pytest.raises(AnsibleError): - encrypt.do_encrypt("123", "crypt16", salt="12") - - @pytest.mark.skipif(not encrypt.PASSLIB_AVAILABLE, reason='passlib must be installed to run this test') def test_do_encrypt_passlib(): with pytest.raises(AnsibleError): @@ -189,31 +120,6 @@ def test_random_salt(): assert res_char in expected_salt_candidate_chars -@pytest.mark.skipif(sys.platform.startswith('darwin'), reason='macOS requires passlib') -@pytest.mark.skipif(sys.version_info >= (3, 13), reason='Python <= 3.12 required') -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")