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

2 files changed, 41 insertions, 0 deletions

diff --git a/cryptography/hazmat/backends/commoncrypto/ciphers.py b/cryptography/hazmat/backends/commoncrypto/ciphers.py
index 525500c8..4f723487 100644
--- a/cryptography/hazmat/backends/commoncrypto/ciphers.py
+++ b/cryptography/hazmat/backends/commoncrypto/ciphers.py
@@ -151,6 +151,11 @@ class _GCMCipherContext(object):
             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 @@ class _GCMCipherContext(object):
         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 28d1a6ca..3ea7f016 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 @@ class TestCommonCrypto(object):
         )
         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