From 75fe4a15f6c81dab0773db0bbb2de6800be63d6e Mon Sep 17 00:00:00 2001 From: Jan-Michael Brummer Date: Mon, 23 Oct 2023 15:03:18 +0200 Subject: [PATCH 1/2] Allow keyfile to be passed as bytes In order to prevent application to create temporary files, provide keyfile as bytes. Fixes: https://github.com/libkeepass/pykeepass/issues/363 --- pykeepass/kdbx_parsing/common.py | 81 ++++++++++++++++++-------------- tests/tests.py | 23 +++++++++ 2 files changed, 68 insertions(+), 36 deletions(-) diff --git a/pykeepass/kdbx_parsing/common.py b/pykeepass/kdbx_parsing/common.py index 5b547366..13d2c31f 100644 --- a/pykeepass/kdbx_parsing/common.py +++ b/pykeepass/kdbx_parsing/common.py @@ -105,6 +105,25 @@ def aes_kdf(key, rounds, key_composite): return hashlib.sha256(transformed_key).digest() +def load_keyfile_composite(key: bytes): + try: + int(key, 16) + is_hex = True + except ValueError: + is_hex = False + # if the length is 32 bytes we assume it is the key + if len(key) == 32: + keyfile_composite = key + # if the length is 64 bytes we assume the key is hex encoded + elif len(key) == 64 and is_hex: + keyfile_composite = codecs.decode(key, 'hex') + # anything else may be a file to hash for the key + else: + keyfile_composite = hashlib.sha256(key).digest() + + return keyfile_composite + + def compute_key_composite(password=None, keyfile=None): """Compute composite key. Used in header verification and payload decryption.""" @@ -116,43 +135,33 @@ def compute_key_composite(password=None, keyfile=None): password_composite = b'' # hash the keyfile if keyfile: - # try to read XML keyfile - try: - with open(keyfile, 'r') as f: - tree = etree.parse(f).getroot() - version = tree.find('Meta/Version').text - data_element = tree.find('Key/Data') - if version.startswith('1.0'): - keyfile_composite = base64.b64decode(data_element.text) - elif version.startswith('2.0'): - # read keyfile data and convert to bytes - keyfile_composite = bytes.fromhex(data_element.text.strip()) - # validate bytes against hash - hash = bytes.fromhex(data_element.attrib['Hash']) - hash_computed = hashlib.sha256(keyfile_composite).digest()[:4] - assert hash == hash_computed, "Keyfile has invalid hash" - # otherwise, try to read plain keyfile - except (etree.XMLSyntaxError, UnicodeDecodeError): + if isinstance(keyfile, bytes): + keyfile_composite = keyfile_composite = load_keyfile_composite(keyfile) + else: + # try to read XML keyfile try: - with open(keyfile, 'rb') as f: - key = f.read() - - try: - int(key, 16) - is_hex = True - except ValueError: - is_hex = False - # if the length is 32 bytes we assume it is the key - if len(key) == 32: - keyfile_composite = key - # if the length is 64 bytes we assume the key is hex encoded - elif len(key) == 64 and is_hex: - keyfile_composite = codecs.decode(key, 'hex') - # anything else may be a file to hash for the key - else: - keyfile_composite = hashlib.sha256(key).digest() - except: - raise IOError('Could not read keyfile') + with open(keyfile, 'r') as f: + tree = etree.parse(f).getroot() + version = tree.find('Meta/Version').text + data_element = tree.find('Key/Data') + if version.startswith('1.0'): + keyfile_composite = base64.b64decode(data_element.text) + elif version.startswith('2.0'): + # read keyfile data and convert to bytes + keyfile_composite = bytes.fromhex(data_element.text.strip()) + # validate bytes against hash + hash = bytes.fromhex(data_element.attrib['Hash']) + hash_computed = hashlib.sha256(keyfile_composite).digest()[:4] + assert hash == hash_computed, "Keyfile has invalid hash" + # otherwise, try to read plain keyfile + except (etree.XMLSyntaxError, UnicodeDecodeError): + try: + with open(keyfile, 'rb') as f: + key = f.read() + + keyfile_composite = load_keyfile_composite(key) + except: + raise IOError('Could not read keyfile') else: keyfile_composite = b'' diff --git a/tests/tests.py b/tests/tests.py index 348eeb38..1e1e3992 100644 --- a/tests/tests.py +++ b/tests/tests.py @@ -1230,6 +1230,29 @@ def test_open_no_decrypt(self): self.assertEqual(kp.database_salt, salt) + + def test_keyfile_as_bytes(self): + + databases = [ + 'test4.kdbx', + ] + passwords = [ + 'password', + ] + keyfiles = [ + base_dir + '/test4.key' + ] + for database, password, keyfile in zip(databases, passwords, keyfiles): + with open(keyfile, "rb") as fh: + buf = fh.read() + + kp = PyKeePass( + os.path.join(base_dir, database), + password, + keyfile=buf + ) + + if __name__ == '__main__': unittest.main() From c53ecd084e8b9f41d7e1a43a8ac0726936a8097f Mon Sep 17 00:00:00 2001 From: evan Date: Mon, 27 Nov 2023 15:59:47 -0600 Subject: [PATCH 2/2] support file-like keyfile on read() --- README.rst | 4 +- pykeepass/kdbx_parsing/common.py | 78 +++++++--------- tests/tests.py | 155 ++++++++++++++----------------- 3 files changed, 106 insertions(+), 131 deletions(-) diff --git a/README.rst b/README.rst index bcd156fd..7ae6f547 100644 --- a/README.rst +++ b/README.rst @@ -366,7 +366,7 @@ Miscellaneous ------------- **read** (filename=None, password=None, keyfile=None, transformed_key=None, decrypt=False) -where ``filename``, ``password``, and ``keyfile`` are strings. ``filename`` is the path to the database, ``password`` is the master password string, and ``keyfile`` is the path to the database keyfile. At least one of ``password`` and ``keyfile`` is required. Alternatively, the derived key can be supplied directly through ``transformed_key``. ``decrypt`` tells whether the file should be decrypted or not. +where ``filename``, ``password``, and ``keyfile`` are strings ( ``filename`` and ``keyfile`` may also be file-like objects). ``filename`` is the path to the database, ``password`` is the master password string, and ``keyfile`` is the path to the database keyfile. At least one of ``password`` and ``keyfile`` is required. Alternatively, the derived key can be supplied directly through ``transformed_key``. ``decrypt`` tells whether the file should be decrypted or not. Can raise ``CredentialsError``, ``HeaderChecksumError``, or ``PayloadChecksumError``. @@ -376,7 +376,7 @@ reload database from disk using previous credentials **save** (filename=None) -where ``filename`` is the path of the file to save to. If ``filename`` is not given, the path given in ``read`` will be used. +where ``filename`` is the path of the file to save to (``filename`` may also be file-like object). If ``filename`` is not given, the path given in ``read`` will be used. **password** diff --git a/pykeepass/kdbx_parsing/common.py b/pykeepass/kdbx_parsing/common.py index 13d2c31f..2ef3d267 100644 --- a/pykeepass/kdbx_parsing/common.py +++ b/pykeepass/kdbx_parsing/common.py @@ -105,25 +105,6 @@ def aes_kdf(key, rounds, key_composite): return hashlib.sha256(transformed_key).digest() -def load_keyfile_composite(key: bytes): - try: - int(key, 16) - is_hex = True - except ValueError: - is_hex = False - # if the length is 32 bytes we assume it is the key - if len(key) == 32: - keyfile_composite = key - # if the length is 64 bytes we assume the key is hex encoded - elif len(key) == 64 and is_hex: - keyfile_composite = codecs.decode(key, 'hex') - # anything else may be a file to hash for the key - else: - keyfile_composite = hashlib.sha256(key).digest() - - return keyfile_composite - - def compute_key_composite(password=None, keyfile=None): """Compute composite key. Used in header verification and payload decryption.""" @@ -135,33 +116,44 @@ def compute_key_composite(password=None, keyfile=None): password_composite = b'' # hash the keyfile if keyfile: - if isinstance(keyfile, bytes): - keyfile_composite = keyfile_composite = load_keyfile_composite(keyfile) + if hasattr(keyfile, "read"): + keyfile_bytes = keyfile.read() else: - # try to read XML keyfile + with open(keyfile, 'rb') as f: + keyfile_bytes = f.read() + # try to read XML keyfile + try: + tree = etree.fromstring(keyfile_bytes) + version = tree.find('Meta/Version').text + data_element = tree.find('Key/Data') + if version.startswith('1.0'): + keyfile_composite = base64.b64decode(data_element.text) + elif version.startswith('2.0'): + # read keyfile data and convert to bytes + keyfile_composite = bytes.fromhex(data_element.text.strip()) + # validate bytes against hash + hash = bytes.fromhex(data_element.attrib['Hash']) + hash_computed = hashlib.sha256(keyfile_composite).digest()[:4] + assert hash == hash_computed, "Keyfile has invalid hash" + # otherwise, try to read plain keyfile + except (etree.XMLSyntaxError, UnicodeDecodeError): try: - with open(keyfile, 'r') as f: - tree = etree.parse(f).getroot() - version = tree.find('Meta/Version').text - data_element = tree.find('Key/Data') - if version.startswith('1.0'): - keyfile_composite = base64.b64decode(data_element.text) - elif version.startswith('2.0'): - # read keyfile data and convert to bytes - keyfile_composite = bytes.fromhex(data_element.text.strip()) - # validate bytes against hash - hash = bytes.fromhex(data_element.attrib['Hash']) - hash_computed = hashlib.sha256(keyfile_composite).digest()[:4] - assert hash == hash_computed, "Keyfile has invalid hash" - # otherwise, try to read plain keyfile - except (etree.XMLSyntaxError, UnicodeDecodeError): try: - with open(keyfile, 'rb') as f: - key = f.read() - - keyfile_composite = load_keyfile_composite(key) - except: - raise IOError('Could not read keyfile') + int(keyfile_bytes, 16) + is_hex = True + except ValueError: + is_hex = False + # if the length is 32 bytes we assume it is the key + if len(keyfile_bytes) == 32: + keyfile_composite = keyfile_bytes + # if the length is 64 bytes we assume the key is hex encoded + elif len(keyfile_bytes) == 64 and is_hex: + keyfile_composite = codecs.decode(keyfile_bytes, 'hex') + # anything else may be a file to hash for the key + else: + keyfile_composite = hashlib.sha256(keyfile_bytes).digest() + except: + raise IOError('Could not read keyfile') else: keyfile_composite = b'' diff --git a/tests/tests.py b/tests/tests.py index 1e1e3992..54a48f23 100644 --- a/tests/tests.py +++ b/tests/tests.py @@ -33,32 +33,32 @@ - expiry_time - get/set """ -base_dir = os.path.dirname(os.path.realpath(__file__)) +base_dir = Path(os.path.dirname(os.path.realpath(__file__))) logger = logging.getLogger("pykeepass") class KDBX3Tests(unittest.TestCase): - database = os.path.join(base_dir, 'test3.kdbx') + database = base_dir / 'test3.kdbx' password = 'password' - keyfile = os.path.join(base_dir, 'test3.key') + keyfile = base_dir / 'test3.key' - database_tmp = os.path.join(base_dir, 'test3_tmp.kdbx') - keyfile_tmp = os.path.join(base_dir, 'test3_tmp.key') + database_tmp = base_dir / 'test3_tmp.kdbx' + keyfile_tmp = base_dir / 'test3_tmp.key' # get some things ready before testing def setUp(self): shutil.copy(self.database, self.database_tmp) shutil.copy(self.keyfile, self.keyfile_tmp) self.kp = PyKeePass( - os.path.join(base_dir, self.database), + base_dir / self.database, password=self.password, - keyfile=os.path.join(base_dir, self.keyfile) + keyfile=base_dir / self.keyfile ) # for tests which modify the database, use this self.kp_tmp = PyKeePass( - os.path.join(base_dir, self.database_tmp), + base_dir / self.database_tmp, password=self.password, - keyfile=os.path.join(base_dir, self.keyfile_tmp) + keyfile=base_dir / self.keyfile_tmp ) def tearDown(self): @@ -67,12 +67,12 @@ def tearDown(self): class KDBX4Tests(KDBX3Tests): - database = os.path.join(base_dir, 'test4.kdbx') + database = base_dir / 'test4.kdbx' password = 'password' - keyfile = os.path.join(base_dir, 'test4.key') + keyfile = base_dir / 'test4.key' - database_tmp = os.path.join(base_dir, 'test4_tmp.kdbx') - keyfile_tmp = os.path.join(base_dir, 'test4_tmp.key') + database_tmp = base_dir / 'test4_tmp.kdbx' + keyfile_tmp = base_dir / 'test4_tmp.key' class EntryFindTests3(KDBX3Tests): @@ -823,7 +823,7 @@ class PyKeePassTests3(KDBX3Tests): def test_set_credentials(self): self.kp_tmp.password = 'f00bar' - self.kp_tmp.keyfile = os.path.join(base_dir, 'change.key') + self.kp_tmp.keyfile = base_dir / 'change.key' self.kp_tmp.save() self.kp_tmp = PyKeePass( self.kp_tmp.filename, @@ -990,7 +990,7 @@ class BugRegressionTests4(KDBX4Tests, BugRegressionTests3): class CtxManagerTests(unittest.TestCase): def test_ctx_manager(self): - with PyKeePass(os.path.join(base_dir, 'test4.kdbx'), password='password', keyfile=base_dir + '/test4.key') as kp: + with PyKeePass(base_dir / 'test4.kdbx', password='password', keyfile=base_dir / 'test4.key') as kp: results = kp.find_entries_by_username('foobar_user', first=True) self.assertEqual('foobar_user', results.username) @@ -1000,40 +1000,43 @@ class KDBXTests(unittest.TestCase): def test_open_save(self): """try to open all databases, save them, then open the result""" - with open(os.path.join(base_dir, 'test3.kdbx'), 'rb') as file: + # for database stream open test + with open(base_dir / 'test3.kdbx', 'rb') as file: stream = BytesIO(file.read()) + # for keyfile file descriptor test + keyfile_fd = open(base_dir / 'test4.key', 'rb') filenames_in = [ - os.path.join(base_dir, 'test3.kdbx'), # KDBX v3 - Path(base_dir).joinpath('test4.kdbx'), # KDBX v4 (and test pathlib) - os.path.join(base_dir, 'test4_aes.kdbx'), # KDBX v4 AES - os.path.join(base_dir, 'test4_aeskdf.kdbx'), # KDBX v3 AESKDF - os.path.join(base_dir, 'test4_chacha20.kdbx'), # KDBX v4 ChaCha - os.path.join(base_dir, 'test4_twofish.kdbx'), # KDBX v4 Twofish - os.path.join(base_dir, 'test4_hex.kdbx'), # legacy 64 byte hexadecimal keyfile - os.path.join(base_dir, 'test3_transformed.kdbx'), # KDBX v3 transformed_key open - os.path.join(base_dir, 'test4_transformed.kdbx'), # KDBX v4 transformed_key open + base_dir / 'test3.kdbx', # KDBX v3 + base_dir / 'test4_aes.kdbx', # KDBX v4 AES + base_dir / 'test4_aeskdf.kdbx', # KDBX v3 AESKDF + base_dir / 'test4_chacha20.kdbx', # KDBX v4 ChaCha + base_dir / 'test4_twofish.kdbx', # KDBX v4 Twofish + base_dir / 'test4_hex.kdbx', # legacy 64 byte hexadecimal keyfile + base_dir / 'test3_transformed.kdbx', # KDBX v3 transformed_key open + base_dir / 'test4_transformed.kdbx', # KDBX v4 transformed_key open stream, # test stream opening - os.path.join(base_dir, 'test4_aes_uncompressed.kdbx'),# KDBX v4 AES uncompressed - os.path.join(base_dir, 'test4_twofish_uncompressed.kdbx'),# KDBX v4 Twofish uncompressed - os.path.join(base_dir, 'test4_chacha20_uncompressed.kdbx'),# KDBX v4 ChaCha uncompressed - os.path.join(base_dir, 'test4_argon2id.kdbx'), # KDBX v4 Argon2id + base_dir / 'test4_aes_uncompressed.kdbx',# KDBX v4 AES uncompressed + base_dir / 'test4_twofish_uncompressed.kdbx',# KDBX v4 Twofish uncompressed + base_dir / 'test4_chacha20_uncompressed.kdbx',# KDBX v4 ChaCha uncompressed + base_dir / 'test4_argon2id.kdbx', # KDBX v4 Argon2id + base_dir / 'test4.kdbx', # KDBX v4 with keyfile file descriptor ] filenames_out = [ - os.path.join(base_dir, 'test3.kdbx.out'), - Path(base_dir).joinpath('test4.kdbx.out'), - os.path.join(base_dir, 'test4_aes.kdbx.out'), - os.path.join(base_dir, 'test4_aeskdf.kdbx.out'), - os.path.join(base_dir, 'test4_chacha20.kdbx.out'), - os.path.join(base_dir, 'test4_twofish.kdbx.out'), - os.path.join(base_dir, 'test4_hex.kdbx.out'), - os.path.join(base_dir, 'test3_transformed.kdbx.out'), - os.path.join(base_dir, 'test4_transformed.kdbx.out'), + base_dir / 'test3.kdbx.out', + base_dir / 'test4_aes.kdbx.out', + base_dir / 'test4_aeskdf.kdbx.out', + base_dir / 'test4_chacha20.kdbx.out', + base_dir / 'test4_twofish.kdbx.out', + base_dir / 'test4_hex.kdbx.out', + base_dir / 'test3_transformed.kdbx.out', + base_dir / 'test4_transformed.kdbx.out', BytesIO(), - os.path.join(base_dir, 'test4_aes_uncompressed.kdbx.out'), - os.path.join(base_dir, 'test4_twofish_uncompressed.kdbx.out'),# KDBX v4 Twofish uncompressed - os.path.join(base_dir, 'test4_chacha20_uncompressed.kdbx.out'),# KDBX v4 ChaCha uncompressed - os.path.join(base_dir, 'test4_argon2id.kdbx.out'), + base_dir / 'test4_aes_uncompressed.kdbx.out', + base_dir / 'test4_twofish_uncompressed.kdbx.out',# KDBX v4 Twofish uncompressed + base_dir / 'test4_chacha20_uncompressed.kdbx.out',# KDBX v4 ChaCha uncompressed + base_dir / 'test4_argon2id.kdbx.out', + base_dir / 'test4.kdbx.out', # KDBX v4 with keyfile file descriptor ] passwords = [ 'password', @@ -1042,7 +1045,6 @@ def test_open_save(self): 'password', 'password', 'password', - 'password', None, None, 'password', @@ -1050,6 +1052,7 @@ def test_open_save(self): 'password', 'password', 'password', + 'password', ] transformed_keys = [ None, @@ -1058,7 +1061,6 @@ def test_open_save(self): None, None, None, - None, b'\xfb\xb1!\x0e0\x94\xd4\x868\xa5\x04\xe6T\x9b<\xf9+\xb8\x82EN\xbc\xbe\xbc\xc8\xd3\xbbf\xfb\xde\xff.', b'\x95\x0be\x9ca\x9e<\xe0\x07\x02\x7f\xc3\xd8\xa1\xa6&\x985\x8f!\xa6\x18k\x13\xa2\xd2\r=\xf3\xebd\xc5', None, @@ -1066,26 +1068,26 @@ def test_open_save(self): None, None, None, - ] + None, + ] keyfiles = [ - 'test3.key', - Path('test4.key'), - 'test4.key', - 'test4.key', - 'test4.key', - 'test4.key', - 'test4_hex.key', + base_dir / 'test3.key', + base_dir / 'test4.key', + base_dir / 'test4.key', + base_dir / 'test4.key', + base_dir / 'test4.key', + base_dir / 'test4_hex.key', None, None, - 'test3.key', + base_dir / 'test3.key', None, None, None, None, + keyfile_fd ] encryption_algorithms = [ 'aes256', - 'chacha20', 'aes256', 'aes256', 'chacha20', @@ -1098,11 +1100,11 @@ def test_open_save(self): 'twofish', 'chacha20', 'aes256', + 'chacha20', ] kdf_algorithms = [ 'aeskdf', 'argon2', - 'argon2', 'aeskdf', 'argon2', 'argon2', @@ -1114,6 +1116,7 @@ def test_open_save(self): 'argon2', 'argon2', 'argon2id', + 'argon2', ] versions = [ (3, 1), @@ -1122,7 +1125,6 @@ def test_open_save(self): (4, 0), (4, 0), (4, 0), - (4, 0), (3, 1), (4, 0), (3, 1), @@ -1130,6 +1132,7 @@ def test_open_save(self): (4, 0), (4, 0), (4, 0), + (4, 0), ] for (filename_in, filename_out, password, transformed_key, @@ -1140,7 +1143,7 @@ def test_open_save(self): kp = PyKeePass( filename_in, password, - None if keyfile is None else os.path.join(base_dir, keyfile), + keyfile, transformed_key=transformed_key ) self.assertEqual(kp.encryption_algorithm, encryption_algorithm) @@ -1159,13 +1162,14 @@ def test_open_save(self): kp = PyKeePass( filename_out, password, - None if keyfile is None else os.path.join(base_dir, keyfile), + keyfile, transformed_key=transformed_key ) - for filename in os.listdir(base_dir): - if filename.endswith('.out'): - os.remove(os.path.join(base_dir, filename)) + for filename in base_dir.glob('*.out'): + os.remove(filename) + + keyfile_fd.close() def test_open_error(self): @@ -1201,13 +1205,15 @@ def test_open_error(self): for database, password, keyfile, error in zip(databases, passwords, keyfiles, errors): with self.assertRaises(error): PyKeePass( - os.path.join(base_dir, database), + base_dir / database, password, - os.path.join(base_dir, keyfile) + base_dir / keyfile ) def test_open_no_decrypt(self): + """Open database but do not decrypt payload. Needed for reading header data for OTP tokens""" + databases = [ 'test3.kdbx', @@ -1230,29 +1236,6 @@ def test_open_no_decrypt(self): self.assertEqual(kp.database_salt, salt) - - def test_keyfile_as_bytes(self): - - databases = [ - 'test4.kdbx', - ] - passwords = [ - 'password', - ] - keyfiles = [ - base_dir + '/test4.key' - ] - for database, password, keyfile in zip(databases, passwords, keyfiles): - with open(keyfile, "rb") as fh: - buf = fh.read() - - kp = PyKeePass( - os.path.join(base_dir, database), - password, - keyfile=buf - ) - - if __name__ == '__main__': unittest.main()