Skip to content

Commit

Permalink
Fix two bugs with CommonCrypto GCM that can result in invalid output.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
reaperhulk committed Sep 11, 2014
1 parent b8599c0 commit 270b9d4
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 0 deletions.
10 changes: 10 additions & 0 deletions cryptography/hazmat/backends/commoncrypto/ciphers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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)
Expand Down
31 changes: 31 additions & 0 deletions tests/hazmat/backends/test_commoncrypto.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@

from __future__ import absolute_import, division, print_function

import binascii

import pytest

from cryptography import utils
Expand Down Expand Up @@ -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

0 comments on commit 270b9d4

Please sign in to comment.