From 270b9d46efdfdff9faea86a48ccb98147348418b Mon Sep 17 00:00:00 2001 From: Paul Kehrer Date: Thu, 11 Sep 2014 18:04:48 -0500 Subject: [PATCH] Fix two bugs with CommonCrypto GCM that can result in invalid output. Bug #1: Call to AAD but no call to update. Get null tag bytes. Bug #2: Call to update without call to AAD. Get null ciphertext bytes. Fixes #1329 --- .../hazmat/backends/commoncrypto/ciphers.py | 10 ++++++ tests/hazmat/backends/test_commoncrypto.py | 31 +++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/cryptography/hazmat/backends/commoncrypto/ciphers.py b/cryptography/hazmat/backends/commoncrypto/ciphers.py index 525500c8b35b..4f72348757ca 100644 --- a/cryptography/hazmat/backends/commoncrypto/ciphers.py +++ b/cryptography/hazmat/backends/commoncrypto/ciphers.py @@ -151,6 +151,11 @@ def __init__(self, backend, cipher, mode, operation): len(mode.initialization_vector) ) self._backend._check_cipher_response(res) + # CommonCrypto has a bug where calling update without at least one + # call to authenticate_additional_data will result in null byte output + # for ciphertext. The following empty byte string call prevents the + # issue, which is present in at least 10.8 and 10.9. + self.authenticate_additional_data(b"") def update(self, data): buf = self._backend._ffi.new("unsigned char[]", len(data)) @@ -164,6 +169,11 @@ def update(self, data): return self._backend._ffi.buffer(buf)[:] def finalize(self): + # CommonCrypto has a yet another bug where you must make at least one + # call to update. If you pass just AAD and call finalize without a call + # to update you'll get null bytes for tag. The following update call + # prevents this issue, which is present in at least 10.8 and 10.9. + self.update(b"") tag_size = self._cipher.block_size // 8 tag_buf = self._backend._ffi.new("unsigned char[]", tag_size) tag_len = self._backend._ffi.new("size_t *", tag_size) diff --git a/tests/hazmat/backends/test_commoncrypto.py b/tests/hazmat/backends/test_commoncrypto.py index 28d1a6caa724..3ea7f0164aea 100644 --- a/tests/hazmat/backends/test_commoncrypto.py +++ b/tests/hazmat/backends/test_commoncrypto.py @@ -13,6 +13,8 @@ from __future__ import absolute_import, division, print_function +import binascii + import pytest from cryptography import utils @@ -68,3 +70,32 @@ def test_nonexistent_aead_cipher(self): ) with raises_unsupported_algorithm(_Reasons.UNSUPPORTED_CIPHER): cipher.encryptor() + + def test_gcm_tag_with_only_aad(self): + from cryptography.hazmat.backends.commoncrypto.backend import Backend + b = Backend() + key = binascii.unhexlify("1dde380d6b04fdcb004005b8a77bd5e3") + iv = binascii.unhexlify("5053bf901463f97decd88c33") + aad = binascii.unhexlify("f807f5f6133021d15cb6434d5ad95cf7d8488727") + tag = binascii.unhexlify("4bebf3ff2cb67bb5444dda53bd039e22") + + cipher = Cipher(AES(key), GCM(iv), backend=b) + encryptor = cipher.encryptor() + encryptor.authenticate_additional_data(aad) + encryptor.finalize() + assert encryptor.tag == tag + + def test_gcm_ciphertext_with_no_aad(self): + from cryptography.hazmat.backends.commoncrypto.backend import Backend + b = Backend() + key = binascii.unhexlify("e98b72a9881a84ca6b76e0f43e68647a") + iv = binascii.unhexlify("8b23299fde174053f3d652ba") + ct = binascii.unhexlify("5a3c1cf1985dbb8bed818036fdd5ab42") + tag = binascii.unhexlify("23c7ab0f952b7091cd324835043b5eb5") + pt = binascii.unhexlify("28286a321293253c3e0aa2704a278032") + + cipher = Cipher(AES(key), GCM(iv), backend=b) + encryptor = cipher.encryptor() + computed_ct = encryptor.update(pt) + encryptor.finalize() + assert computed_ct == ct + assert encryptor.tag == tag