From cab4eb1f5f11081e65d3a8dc54417833550d73fb Mon Sep 17 00:00:00 2001 From: Abhijeet Kasurde Date: Fri, 11 Oct 2024 12:28:47 -0700 Subject: [PATCH] Review requests Signed-off-by: Abhijeet Kasurde --- lib/ansible/module_utils/basic.py | 27 +++++++++++---- lib/ansible/module_utils/common/hashing.py | 30 ++++++++++++++--- test/units/utils/test_hashing.py | 39 +++++++++++++++++++--- 3 files changed, 82 insertions(+), 14 deletions(-) diff --git a/lib/ansible/module_utils/basic.py b/lib/ansible/module_utils/basic.py index eccd732d2a1..6dd58344b1d 100644 --- a/lib/ansible/module_utils/basic.py +++ b/lib/ansible/module_utils/basic.py @@ -1459,6 +1459,10 @@ class AnsibleModule(object): def digest_from_file(self, filename, algorithm): """ Return hex digest of local file for a digest_method specified by name, or None if file is not present. """ + self.deprecate( + msg="API digest_from_file is deprecated. Use generate_secure_file_checksum instead.", + version="2.22" + ) b_filename = to_bytes(filename, errors='surrogate_or_strict') if not os.path.exists(b_filename): @@ -1475,18 +1479,17 @@ class AnsibleModule(object): self.module.fail_json( msg=f"Could not find file {filename} to calculate hash: {e}" ) - - if algorithm not in AVAILABLE_HASH_ALGORITHMS: - self.fail_json( - msg=f"Could not hash file '{filename}' with algorithm '{algorithm}'. Available algorithms: {', '.join(AVAILABLE_HASH_ALGORITHMS)}" - ) + except ValueError as e: + self.module.fail_json(msg=f"{e}") try: - return generate_secure_file_checksum(filename, hash_func=AVAILABLE_HASH_ALGORITHMS[algorithm]) + return generate_secure_file_checksum(filename, hash_func=algorithm) except IOError as e: self.module.fail_json( msg=f"Could not find file {filename} to calculate hash: {e}" ) + except ValueError as e: + self.module.fail_json(msg=f"{e}") def md5(self, filename): """ Return MD5 hex digest of local file using digest_from_file(). @@ -1499,16 +1502,28 @@ class AnsibleModule(object): Most uses of this function can use the module.sha1 function instead. """ + self.deprecate( + msg="Using md5 is not recommended. Use generate_secure_file_checksum with SHA256 instead.", + version="2.22" + ) if 'md5' not in AVAILABLE_HASH_ALGORITHMS: raise ValueError('MD5 not available. Possibly running in FIPS mode') return self.digest_from_file(filename, 'md5') def sha1(self, filename): """ Return SHA1 hex digest of local file using digest_from_file(). """ + self.deprecate( + msg="Using SHA1 is not recommended. Use generate_secure_file_checksum with SHA256 instead.", + version="2.22" + ) return self.digest_from_file(filename, 'sha1') def sha256(self, filename): """ Return SHA-256 hex digest of local file using digest_from_file(). """ + self.deprecate( + msg="Use generate_secure_file_checksum with SHA256 instead.", + version="2.22" + ) return self.digest_from_file(filename, 'sha256') def backup_local(self, fn): diff --git a/lib/ansible/module_utils/common/hashing.py b/lib/ansible/module_utils/common/hashing.py index a7a0e08a087..283889de118 100644 --- a/lib/ansible/module_utils/common/hashing.py +++ b/lib/ansible/module_utils/common/hashing.py @@ -15,6 +15,7 @@ except ImportError: _md5 = None # type: ignore[assignment] from ansible.module_utils.common.text.converters import to_bytes +from ansible.module_utils.six import string_types def _get_available_hash_algorithms(): @@ -39,16 +40,26 @@ def _get_available_hash_algorithms(): AVAILABLE_HASH_ALGORITHMS = _get_available_hash_algorithms() -def generate_secure_checksum(data, hash_func=hashlib.sha256): +def generate_secure_checksum(data, hash_func='sha256'): """Generates a secure checksum for the given data using the specified hash function. Args: data: The data to be hashed. - hash_func: The hash function to use (default: hashlib.sha256). + hash_func: The hash function to use (default: sha256). Returns: A hexadecimal string representing the checksum. """ + if hash_func is None: + raise ValueError("The parameter 'hash_func' can not be None") + + if isinstance(hash_func, string_types): + if hash_func not in AVAILABLE_HASH_ALGORITHMS: + raise ValueError(f"{hash_func} is not available. Available algorithms: {', '.join(AVAILABLE_HASH_ALGORITHMS)}") + hash_func = getattr(hashlib, hash_func, None) + + if not callable(hash_func): + raise ValueError("The parameter value of 'hash_func' is not callable. Please make sure hash_func is either string or method.") digest = hash_func() data = to_bytes(data, errors='surrogate_or_strict') @@ -56,12 +67,12 @@ def generate_secure_checksum(data, hash_func=hashlib.sha256): return digest.hexdigest() -def generate_secure_file_checksum(filename, hash_func=hashlib.sha256, write_to=None): +def generate_secure_file_checksum(filename, hash_func='sha256', write_to=None): """Return a secure hash hex digest of local file, None if file is not present or a directory. Args: filename: The filename to be hashed. - hash_func: The hash function to use (default: hashlib.sha256). + hash_func: The hash function to use (default: sha256). write_to: The file handle to write to (default: None). Returns: @@ -72,6 +83,17 @@ def generate_secure_file_checksum(filename, hash_func=hashlib.sha256, write_to=N if not os.path.exists(b_filename) or os.path.isdir(to_bytes(filename, errors='strict')): return None + if hash_func is None: + raise ValueError("The parameter 'hash_func' can not be None") + + if isinstance(hash_func, string_types): + if hash_func not in AVAILABLE_HASH_ALGORITHMS: + raise ValueError(f"{hash_func} is not available. Available algorithms: {', '.join(AVAILABLE_HASH_ALGORITHMS)}") + hash_func = getattr(hashlib, hash_func, None) + + if not callable(hash_func): + raise ValueError("The parameter value of 'hash_func' is not callable. Please make sure hash_func is either string or method.") + digest = hash_func() blocksize = 64 * 1024 try: diff --git a/test/units/utils/test_hashing.py b/test/units/utils/test_hashing.py index 5b549bdb159..9597d57c7dd 100644 --- a/test/units/utils/test_hashing.py +++ b/test/units/utils/test_hashing.py @@ -14,21 +14,39 @@ import pytest secure_hash_testdata = [ - pytest.param(hashlib.sha1, "a94a8fe5ccb19ba61c4c0873d391e987982fbbd3", id="sha1"), + pytest.param("sha1", "a94a8fe5ccb19ba61c4c0873d391e987982fbbd3", id="sha1_str"), + pytest.param( + "sha224", + "90a3ed9e32b2aaf4c61c410eb925426119e1a9dc53d4286ade99a809", + id="sha224_str", + ), + pytest.param( + "sha256", + "9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08", + id="sha256_str", + ), + pytest.param( + "sha384", + "768412320f7b0aa5812fce428dc4706b3cae50e02a64caa16a782249bfe8efc4b7ef1ccb126255d196047dfedf17a0a9", + id="sha384_str", + ), + pytest.param( + hashlib.sha1, "a94a8fe5ccb19ba61c4c0873d391e987982fbbd3", id="sha1_call" + ), pytest.param( hashlib.sha224, "90a3ed9e32b2aaf4c61c410eb925426119e1a9dc53d4286ade99a809", - id="sha224", + id="sha224_call", ), pytest.param( hashlib.sha256, "9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08", - id="sha256", + id="sha256_call", ), pytest.param( hashlib.sha384, "768412320f7b0aa5812fce428dc4706b3cae50e02a64caa16a782249bfe8efc4b7ef1ccb126255d196047dfedf17a0a9", - id="sha384", + id="sha384_call", ), ] @@ -52,3 +70,16 @@ def test_generate_secure_file_checksum(hash_func, expected): assert ( hashing.generate_secure_file_checksum(text_file.name, hash_func) == expected ) + + +def test_generate_secure_checksum_fail_none(): + with pytest.raises(ValueError, match=r"^The parameter 'hash_func'"): + hashing.generate_secure_checksum("test", hash_func=None) + + +def test_generate_secure_file_checksum_fail_none(): + with pytest.raises(ValueError, match=r"^The parameter 'hash_func'"): + with tempfile.NamedTemporaryFile() as text_file: + text_file.write(to_bytes("test")) + text_file.flush() + hashing.generate_secure_file_checksum(text_file.name, hash_func=None)