From e23cb36ac03c5e3a368cb8c493927cf8babc8dbc Mon Sep 17 00:00:00 2001 From: Tin Lam Date: Thu, 30 Mar 2017 13:17:44 -0500 Subject: [PATCH] Replace pycrypto with cryptography The pycrypto library is unmaintained, and keystonemiddleware currently uses pycrypto to encrypt and decrpyt things before caching them. This patch set removes the pycrypto dependency and updates the code to use the cryptography library. See [1]. Replacing the cryptographic library is backward compatible. See [2]. [1] http://lists.openstack.org/pipermail/openstack-dev/2017-March/113568.html [2] http://paste.openstack.org/show/610186/ Change-Id: Iced7f5115e49ccf4f7f5bf6813cb5988b95c248b Closes-Bug: #1677308 --- .../auth_token/_memcache_crypt.py | 51 ++++++++++++------- .../unit/auth_token/test_memcache_crypt.py | 8 +-- .../notes/bug-1677308-a2fa7de67f21cd84.yaml | 15 ++++++ test-requirements.txt | 2 +- 4 files changed, 54 insertions(+), 22 deletions(-) create mode 100644 releasenotes/notes/bug-1677308-a2fa7de67f21cd84.yaml diff --git a/keystonemiddleware/auth_token/_memcache_crypt.py b/keystonemiddleware/auth_token/_memcache_crypt.py index 35067de6..554d0201 100644 --- a/keystonemiddleware/auth_token/_memcache_crypt.py +++ b/keystonemiddleware/auth_token/_memcache_crypt.py @@ -17,8 +17,8 @@ Utilities for memcache encryption and integrity check. Data should be serialized before entering these functions. Encryption -has a dependency on the pycrypto. If pycrypto is not available, -CryptoUnavailableError will be raised. +has a dependency on the cryptography module. If cryptography is not +available, CryptoUnavailableError will be raised. This module will not be called unless signing or encryption is enabled in the config. It will always validate signatures, and will decrypt @@ -33,17 +33,19 @@ import hashlib import hmac import math import os -import six - -from oslo_utils import secretutils from keystonemiddleware.i18n import _ +from oslo_utils import secretutils -# make sure pycrypto is available try: - from Crypto.Cipher import AES + from cryptography.hazmat import backends as crypto_backends + from cryptography.hazmat.primitives import ciphers + from cryptography.hazmat.primitives.ciphers import algorithms + from cryptography.hazmat.primitives.ciphers import modes + from cryptography.hazmat.primitives import padding except ImportError: - AES = None + ciphers = None + HASH_FUNCTION = hashlib.sha384 DIGEST_LENGTH = HASH_FUNCTION().digest_size @@ -74,10 +76,10 @@ class CryptoUnavailableError(Exception): def assert_crypto_availability(f): - """Ensure Crypto module is available.""" + """Ensure cryptography module is available.""" @functools.wraps(f) def wrapper(*args, **kwds): - if AES is None: + if ciphers is None: raise CryptoUnavailableError() return f(*args, **kwds) return wrapper @@ -116,24 +118,39 @@ def encrypt_data(key, data): Padding is n bytes of the value n, where 1 <= n <= blocksize. """ iv = os.urandom(16) - cipher = AES.new(key, AES.MODE_CBC, iv) - padding = 16 - len(data) % 16 - return iv + cipher.encrypt(data + six.int2byte(padding) * padding) + cipher = ciphers.Cipher( + algorithms.AES(key), + modes.CBC(iv), + backend=crypto_backends.default_backend()) + + # AES algorithm uses block size of 16 bytes = 128 bits, defined in + # algorithms.AES.block_size. Previously, we manually padded this using + # six.int2byte(padding) * padding. Using ``cryptography``, we will + # analogously use hazmat.primitives.padding to pad it to + # the 128-bit block size. + padder = padding.PKCS7(algorithms.AES.block_size).padder() + padded_data = padder.update(data) + padder.finalize() + encryptor = cipher.encryptor() + return iv + encryptor.update(padded_data) + encryptor.finalize() -@assert_crypto_availability def decrypt_data(key, data): """Decrypt the data with the given secret key.""" iv = data[:16] - cipher = AES.new(key, AES.MODE_CBC, iv) + cipher = ciphers.Cipher( + algorithms.AES(key), + modes.CBC(iv), + backend=crypto_backends.default_backend()) try: - result = cipher.decrypt(data[16:]) + decryptor = cipher.decryptor() + result = decryptor.update(data[16:]) + decryptor.finalize() except Exception: raise DecryptError(_('Encrypted data appears to be corrupted.')) # Strip the last n padding bytes where n is the last value in # the plaintext - return result[:-1 * six.byte2int([result[-1]])] + unpadder = padding.PKCS7(algorithms.AES.block_size).unpadder() + return unpadder.update(result) + unpadder.finalize() def protect_data(keys, data): diff --git a/keystonemiddleware/tests/unit/auth_token/test_memcache_crypt.py b/keystonemiddleware/tests/unit/auth_token/test_memcache_crypt.py index 48a7fb32..74fc38c5 100644 --- a/keystonemiddleware/tests/unit/auth_token/test_memcache_crypt.py +++ b/keystonemiddleware/tests/unit/auth_token/test_memcache_crypt.py @@ -67,10 +67,10 @@ class MemcacheCryptPositiveTests(utils.BaseTestCase): keys, protected[:-1]) self.assertIsNone(memcache_crypt.unprotect_data(keys, None)) - def test_no_pycrypt(self): - aes = memcache_crypt.AES - memcache_crypt.AES = None + def test_no_cryptography(self): + aes = memcache_crypt.ciphers + memcache_crypt.ciphers = None self.assertRaises(memcache_crypt.CryptoUnavailableError, memcache_crypt.encrypt_data, 'token', 'secret', 'data') - memcache_crypt.AES = aes + memcache_crypt.ciphers = aes diff --git a/releasenotes/notes/bug-1677308-a2fa7de67f21cd84.yaml b/releasenotes/notes/bug-1677308-a2fa7de67f21cd84.yaml new file mode 100644 index 00000000..52383917 --- /dev/null +++ b/releasenotes/notes/bug-1677308-a2fa7de67f21cd84.yaml @@ -0,0 +1,15 @@ +--- +fixes: + - | + [`bug 1677308 `_] + Removes ``pycrypto`` dependency as the library is unmaintained, and + replaces it with the ``cryptography`` library. +upgrade: + - | + [`bug 1677308 `_] + There is no upgrade impact when switching from ``pycrypto`` to + ``cryptography``. All data will be encrypted and decrypted using identical + blocksize, padding, algorithm (AES) and mode (CBC). Data previously + encrypted using ``pycrypto`` can be decrypted using both ``pycrypto`` and + ``cryptography``. The same is true of data encrypted using + ``cryptography``. diff --git a/test-requirements.txt b/test-requirements.txt index 55b1c2b2..0320253a 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -6,10 +6,10 @@ hacking<0.11,>=0.10.0 flake8-docstrings==0.2.1.post1 # MIT coverage!=4.4,>=4.0 # Apache-2.0 +cryptography>=1.6 # BSD/Apache-2.0 docutils>=0.11 # OSI-Approved Open Source, Public Domain fixtures>=3.0.0 # Apache-2.0/BSD mock>=2.0 # BSD -pycrypto>=2.6 # Public Domain oslosphinx>=4.7.0 # Apache-2.0 oslotest>=1.10.0 # Apache-2.0 reno>=1.8.0 # Apache-2.0