From 6a4e37a09587c9bc0b6de24acba593a4695893ff Mon Sep 17 00:00:00 2001 From: Adam Israel Date: Thu, 16 Mar 2017 22:31:59 -0400 Subject: [PATCH 1/5] Replace M2Crypt with pycrypto M2Crypt does not work with Python 3+, so I've replaced it with pycrypto, which does. --- onepassword/encryption_key.py | 18 +++++++++++------- onepassword/keychain.py | 8 ++++++-- requirements_dev.txt | 3 ++- setup.py | 5 +++-- 4 files changed, 22 insertions(+), 12 deletions(-) diff --git a/onepassword/encryption_key.py b/onepassword/encryption_key.py index 32a625b..69cd65e 100755 --- a/onepassword/encryption_key.py +++ b/onepassword/encryption_key.py @@ -1,6 +1,9 @@ from base64 import b64decode from hashlib import md5 -from M2Crypto import EVP + +from Crypto.Protocol.KDF import PBKDF2 +from Crypto.Cipher import AES + class SaltyString(object): SALTED_PREFIX = "Salted__" @@ -23,6 +26,7 @@ def __init__(self, data, iterations=0, validation="", identifier=None, level=None): self.identifier = identifier self.level = level + self._encrypted_key = SaltyString(data) self._decrypted_key = None self._set_iterations(iterations) @@ -50,8 +54,8 @@ def _validate_decrypted_key(self): return self.decrypt(self._validation) == self._decrypted_key def _aes_decrypt(self, key, iv, encrypted_data): - aes = EVP.Cipher("aes_128_cbc", key, iv, key_as_bytes=False, padding=False, op=0) - return self._strip_padding(aes.update(encrypted_data) + aes.final()) + aes = AES.new(key, AES.MODE_CBC, iv) + return self._strip_padding(aes.decrypt(encrypted_data)) def _strip_padding(self, decrypted): padding_size = ord(decrypted[-1]) @@ -61,11 +65,11 @@ def _strip_padding(self, decrypted): return decrypted[:-padding_size] def _derive_pbkdf2(self, password): - key_and_iv = EVP.pbkdf2( + key_and_iv = PBKDF2( password, self._encrypted_key.salt, - self.iterations, 32, + self.iterations, ) return ( key_and_iv[0:16], @@ -74,8 +78,8 @@ def _derive_pbkdf2(self, password): def _derive_openssl(self, key, salt): key = key[0:-16] - key_and_iv = "" - prev = "" + key_and_iv = b"" + prev = b"" while len(key_and_iv) < 32: prev = md5(prev + key + salt).digest() key_and_iv += prev diff --git a/onepassword/keychain.py b/onepassword/keychain.py index bf395ad..9e587a9 100644 --- a/onepassword/keychain.py +++ b/onepassword/keychain.py @@ -1,5 +1,6 @@ import json import os +import functools from fuzzywuzzy import process from onepassword.encryption_key import EncryptionKey @@ -13,9 +14,11 @@ def __init__(self, path): self._locked = True def unlock(self, password): - unlocker = lambda key: key.unlock(password) + def unlocker(key): + return key.unlock(password) + unlock_results = map(unlocker, self._encryption_keys.values()) - result = reduce(lambda x, y: x and y, unlock_results) + result = functools.reduce(lambda x, y: x and y, unlock_results) self._locked = not result return result @@ -114,6 +117,7 @@ def decrypt_with(self, keychain): ) encrypted_json = self._lazily_load("_encrypted_json") decrypted_json = key.decrypt(self._encrypted_json) + self._data = json.loads(decrypted_json) self.password = self._find_password() diff --git a/requirements_dev.txt b/requirements_dev.txt index 292fc48..5e5144d 100755 --- a/requirements_dev.txt +++ b/requirements_dev.txt @@ -1,4 +1,5 @@ mock nose -M2Crypto fuzzywuzzy +pycrypto +pbkdf2 diff --git a/setup.py b/setup.py index 5a05b31..ed17b48 100755 --- a/setup.py +++ b/setup.py @@ -2,15 +2,16 @@ from setuptools import setup - VERSION = "0.2.1" + def readme(): """ Load the contents of the README file """ readme_path = os.path.join(os.path.dirname(__file__), "README.txt") with open(readme_path, "r") as f: return f.read() + setup( name="1pass", version=VERSION, @@ -18,7 +19,7 @@ def readme(): author_email="george@georgebrock.com", description="A Python library and command line interface for 1Password", long_description=readme(), - install_requires=["M2Crypto", "fuzzywuzzy"], + install_requires=["pycrypto", "pbkdf2", "fuzzywuzzy"], license="MIT", url="http://github.com/georgebrock/1pass", classifiers=[], From 63cf2971edd2ff5ac324d334b2f84e5131042f18 Mon Sep 17 00:00:00 2001 From: Adam Israel Date: Fri, 31 Mar 2017 21:26:56 -0400 Subject: [PATCH 2/5] Merge --- onepassword/keychain.py | 1 + 1 file changed, 1 insertion(+) diff --git a/onepassword/keychain.py b/onepassword/keychain.py index 9e587a9..f8a9c95 100644 --- a/onepassword/keychain.py +++ b/onepassword/keychain.py @@ -19,6 +19,7 @@ def unlocker(key): unlock_results = map(unlocker, self._encryption_keys.values()) result = functools.reduce(lambda x, y: x and y, unlock_results) + self._locked = not result return result From 866e147a9675e8cf61634d467781c544c90162d8 Mon Sep 17 00:00:00 2001 From: Adam Israel Date: Fri, 31 Mar 2017 22:56:31 -0400 Subject: [PATCH 3/5] Fix module loading w/python3 --- onepassword/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/onepassword/__init__.py b/onepassword/__init__.py index 27abe03..fc79d2e 100644 --- a/onepassword/__init__.py +++ b/onepassword/__init__.py @@ -1 +1 @@ -from keychain import Keychain +from .keychain import Keychain From c7af6931b6ff2c8f203c11f1607d094c34adcb7c Mon Sep 17 00:00:00 2001 From: Adam Israel Date: Fri, 31 Mar 2017 22:58:55 -0400 Subject: [PATCH 4/5] Fix performance issue unlocking keychain --- onepassword/encryption_key.py | 16 +++++++++------- onepassword/keychain.py | 1 - 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/onepassword/encryption_key.py b/onepassword/encryption_key.py index 69cd65e..5aea06a 100755 --- a/onepassword/encryption_key.py +++ b/onepassword/encryption_key.py @@ -1,7 +1,6 @@ from base64 import b64decode -from hashlib import md5 +from hashlib import md5, pbkdf2_hmac -from Crypto.Protocol.KDF import PBKDF2 from Crypto.Cipher import AES @@ -65,11 +64,14 @@ def _strip_padding(self, decrypted): return decrypted[:-padding_size] def _derive_pbkdf2(self, password): - key_and_iv = PBKDF2( - password, - self._encrypted_key.salt, - 32, - self.iterations, + # Use hashlib.pbkdf2_hmac, which is significantly faster + # than Crypto.Protocol.KDF.PBKDF2 + key_and_iv = pbkdf2_hmac( + 'sha1', # hash type + password, # password + self._encrypted_key.salt, # salt + self.iterations, # iterations + 32 # length of key ) return ( key_and_iv[0:16], diff --git a/onepassword/keychain.py b/onepassword/keychain.py index f8a9c95..9e587a9 100644 --- a/onepassword/keychain.py +++ b/onepassword/keychain.py @@ -19,7 +19,6 @@ def unlocker(key): unlock_results = map(unlocker, self._encryption_keys.values()) result = functools.reduce(lambda x, y: x and y, unlock_results) - self._locked = not result return result From eda322aa78f6f343f8f88cd21105d5a8746f591d Mon Sep 17 00:00:00 2001 From: Adam Israel Date: Fri, 31 Mar 2017 22:58:55 -0400 Subject: [PATCH 5/5] Fix performance issue unlocking keychain Add Python3 compatibility --- onepassword/encryption_key.py | 26 +++++++++++++++++--------- onepassword/keychain.py | 4 +++- onepassword/utils.py | 5 +++++ tests/cli_test.py | 15 ++++++++++----- tests/encryption_key_test.py | 26 +++++++++++++------------- tests/integration_test.py | 10 +++++----- tests/keychain_test.py | 18 +++++++++--------- 7 files changed, 62 insertions(+), 42 deletions(-) create mode 100644 onepassword/utils.py diff --git a/onepassword/encryption_key.py b/onepassword/encryption_key.py index 69cd65e..55ba6ca 100755 --- a/onepassword/encryption_key.py +++ b/onepassword/encryption_key.py @@ -1,16 +1,18 @@ from base64 import b64decode -from hashlib import md5 +from hashlib import md5, pbkdf2_hmac -from Crypto.Protocol.KDF import PBKDF2 from Crypto.Cipher import AES +from .utils import is_python_3 + class SaltyString(object): - SALTED_PREFIX = "Salted__" - ZERO_INIT_VECTOR = "\x00" * 16 + SALTED_PREFIX = b"Salted__" + ZERO_INIT_VECTOR = b"\x00" * 16 def __init__(self, base64_encoded_string): decoded_data = b64decode(base64_encoded_string) + if decoded_data.startswith(self.SALTED_PREFIX): self.salt = decoded_data[8:16] self.data = decoded_data[16:] @@ -58,6 +60,9 @@ def _aes_decrypt(self, key, iv, encrypted_data): return self._strip_padding(aes.decrypt(encrypted_data)) def _strip_padding(self, decrypted): + if is_python_3(): + return decrypted.rstrip() + padding_size = ord(decrypted[-1]) if padding_size >= 16: return decrypted @@ -65,11 +70,14 @@ def _strip_padding(self, decrypted): return decrypted[:-padding_size] def _derive_pbkdf2(self, password): - key_and_iv = PBKDF2( - password, - self._encrypted_key.salt, - 32, - self.iterations, + # Use hashlib.pbkdf2_hmac, which is significantly faster + # than Crypto.Protocol.KDF.PBKDF2 + key_and_iv = pbkdf2_hmac( + 'sha1', # hash type + password.encode(), # password + self._encrypted_key.salt, # salt + self.iterations, # iterations + 32 # length of key ) return ( key_and_iv[0:16], diff --git a/onepassword/keychain.py b/onepassword/keychain.py index f8a9c95..20b9cee 100644 --- a/onepassword/keychain.py +++ b/onepassword/keychain.py @@ -19,7 +19,6 @@ def unlocker(key): unlock_results = map(unlocker, self._encryption_keys.values()) result = functools.reduce(lambda x, y: x and y, unlock_results) - self._locked = not result return result @@ -119,6 +118,9 @@ def decrypt_with(self, keychain): encrypted_json = self._lazily_load("_encrypted_json") decrypted_json = key.decrypt(self._encrypted_json) + if isinstance(decrypted_json, bytes): + decrypted_json = decrypted_json.decode('utf-8') + self._data = json.loads(decrypted_json) self.password = self._find_password() diff --git a/onepassword/utils.py b/onepassword/utils.py new file mode 100644 index 0000000..78e9ff6 --- /dev/null +++ b/onepassword/utils.py @@ -0,0 +1,5 @@ +import sys + + +def is_python_3(): + return sys.version.split(" ")[0].split(".")[0] =="3" diff --git a/tests/cli_test.py b/tests/cli_test.py index cb9c168..0e570a6 100644 --- a/tests/cli_test.py +++ b/tests/cli_test.py @@ -1,6 +1,10 @@ import os from unittest import TestCase -from StringIO import StringIO +# Python 2+3 compatibilty +try: + from StringIO import StringIO +except ImportError: + from io import StringIO from onepassword.cli import CLI @@ -14,9 +18,10 @@ def setUp(self): def test_cli_reading_web_form_password_with_multiple_password_attempts(self): password_attempts = (i for i in ("incorrect", "badger")) cli = self.build_cli( - getpass=lambda prompt: password_attempts.next(), + getpass=lambda prompt: next(password_attempts), arguments=("--path", self.keychain_path, "onetosix",), ) + cli.run() self.assert_output("123456\n") @@ -95,18 +100,18 @@ def assert_exit_status(self, expected_status, func): try: func() except SystemExit as exit: - self.assertEquals(expected_status, exit.code) + self.assertEqual(expected_status, exit.code) else: self.fail("Expected a SystemExit to be raised") def assert_output(self, expected_output): - self.assertEquals(expected_output, self.output.getvalue()) + self.assertEqual(expected_output, self.output.getvalue()) def assert_no_output(self): self.assert_output("") def assert_error_output(self, expected_output): - self.assertEquals(expected_output, self.error.getvalue()) + self.assertEqual(expected_output, self.error.getvalue()) def assert_no_error_output(self): self.assert_error_output("") diff --git a/tests/encryption_key_test.py b/tests/encryption_key_test.py index ee748b0..8161019 100644 --- a/tests/encryption_key_test.py +++ b/tests/encryption_key_test.py @@ -1,4 +1,4 @@ -from base64 import b64encode, b64decode +from base64 import b64encode from unittest import TestCase from onepassword.encryption_key import SaltyString, EncryptionKey @@ -6,40 +6,40 @@ class SaltyStringTest(TestCase): def test_unsalted_data(self): - unsalted = SaltyString(b64encode("Unsalted data")) - self.assertEquals("\x00" * 16, unsalted.salt) - self.assertEquals("Unsalted data", unsalted.data) + unsalted = SaltyString(b64encode(b"Unsalted data")) + self.assertEqual(b"\x00" * 16, unsalted.salt) + self.assertEqual(b"Unsalted data", unsalted.data) def test_salted_data(self): - salted = SaltyString(b64encode("Salted__SSSSSSSSDDDDDDDD")) - self.assertEquals("SSSSSSSS", salted.salt) - self.assertEquals("DDDDDDDD", salted.data) + salted = SaltyString(b64encode(b"Salted__SSSSSSSSDDDDDDDD")) + self.assertEqual(b"SSSSSSSS", salted.salt) + self.assertEqual(b"DDDDDDDD", salted.data) class EncryptionKeyTest(TestCase): def test_identifier(self): key = EncryptionKey(data="", identifier="ABC123") - self.assertEquals("ABC123", key.identifier) + self.assertEqual("ABC123", key.identifier) def test_level(self): key = EncryptionKey(data="", level="SL3") - self.assertEquals("SL3", key.level) + self.assertEqual("SL3", key.level) def test_iterations_with_string(self): key = EncryptionKey(data="", iterations="40000") - self.assertEquals(40000, key.iterations) + self.assertEqual(40000, key.iterations) def test_iterations_with_number(self): key = EncryptionKey(data="", iterations=5000) - self.assertEquals(5000, key.iterations) + self.assertEqual(5000, key.iterations) def test_iterations_default(self): key = EncryptionKey(data="") - self.assertEquals(1000, key.iterations) + self.assertEqual(1000, key.iterations) def test_iterations_minimum(self): key = EncryptionKey(data="", iterations=500) - self.assertEquals(1000, key.iterations) + self.assertEqual(1000, key.iterations) def test_unlocking_with_correct_password(self): key = EncryptionKey(**self.example_data) diff --git a/tests/integration_test.py b/tests/integration_test.py index 9d74f8f..7ce6590 100644 --- a/tests/integration_test.py +++ b/tests/integration_test.py @@ -15,27 +15,27 @@ def test_unlock_and_read_web_form_password(self): self.assertTrue(unlock_result) self.assertIsNone(keychain.item("does-not-exist")) - self.assertEquals("123456", keychain.item("onetosix").password) - self.assertEquals("abcdef", keychain.item("atof").password) + self.assertEqual("123456", keychain.item("onetosix").password) + self.assertEqual("abcdef", keychain.item("atof").password) def test_unlock_and_read_generated_password(self): keychain = Keychain(path=self.keychain_path) keychain.unlock("badger") - self.assertEquals("foobar", keychain.item("foobar").password) + self.assertEqual("foobar", keychain.item("foobar").password) def test_unlock_and_read_generic_account_password(self): keychain = Keychain(path=self.keychain_path) keychain.unlock("badger") - self.assertEquals("flibble", keychain.item("Generic Account").password) + self.assertEqual("flibble", keychain.item("Generic Account").password) def test_unlock_and_read_with_fuzzy_matching(self): keychain = Keychain(path=self.keychain_path) keychain.unlock("badger") item = keychain.item("foobr", fuzzy_threshold=70) - self.assertEquals("foobar", item.password) + self.assertEqual("foobar", item.password) @property def keychain_path(self): diff --git a/tests/keychain_test.py b/tests/keychain_test.py index b1bbede..1cdbc8e 100644 --- a/tests/keychain_test.py +++ b/tests/keychain_test.py @@ -15,8 +15,8 @@ def test_locked_flag(self): def test_key_by_security_level(self): keychain = Keychain(self.data_path) key = keychain.key(security_level="SL5") - self.assertEquals("525E210E0B4C49799D7E47DD8E789C78", key.identifier) - self.assertEquals("SL5", key.level) + self.assertEqual("525E210E0B4C49799D7E47DD8E789C78", key.identifier) + self.assertEqual("SL5", key.level) def test_key_by_id_with_bad_security_level(self): keychain = Keychain(self.data_path) @@ -26,8 +26,8 @@ def test_key_by_id_with_bad_security_level(self): def test_key_by_id(self): keychain = Keychain(self.data_path) key = keychain.key(identifier="525E210E0B4C49799D7E47DD8E789C78") - self.assertEquals("525E210E0B4C49799D7E47DD8E789C78", key.identifier) - self.assertEquals("SL5", key.level) + self.assertEqual("525E210E0B4C49799D7E47DD8E789C78", key.identifier) + self.assertEqual("SL5", key.level) def test_key_by_id_with_bad_id(self): keychain = Keychain(self.data_path) @@ -42,12 +42,12 @@ def data_path(self): class KeychainItemTest(TestCase): def test_initialisation_with_contents_data(self): item = KeychainItem.build(self.example_row, path=self.data_path) - self.assertEquals("onetosix", item.name) - self.assertEquals("CEA5EA6531FC4BE9B7D7F89B5BB18B66", item.identifier) + self.assertEqual("onetosix", item.name) + self.assertEqual("CEA5EA6531FC4BE9B7D7F89B5BB18B66", item.identifier) def test_key_identifier(self): item = KeychainItem.build(self.example_row, path=self.data_path) - self.assertEquals("525E210E0B4C49799D7E47DD8E789C78", item.key_identifier) + self.assertEqual("525E210E0B4C49799D7E47DD8E789C78", item.key_identifier) def test_security_level(self): item = KeychainItem.build( @@ -55,7 +55,7 @@ def test_security_level(self): "example.com", 12345, "", 0, "N"], path=self.data_path, ) - self.assertEquals("SL5", item.security_level) + self.assertEqual("SL5", item.security_level) def test_decrypt(self): mock_key = Mock() @@ -69,7 +69,7 @@ def test_decrypt(self): self.assertIsNone(item.password) item.decrypt_with(mock_keychain) - self.assertEquals("abcdef", item.password) + self.assertEqual("abcdef", item.password) @property def data_path(self):