Merge pull request #1201 from alex/no-more-truncation

Fixes #1200 -- disallow GCM truncation by default

6 files changed, 37 insertions, 19 deletions

diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index e057b636..13bc23f8 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -6,6 +6,11 @@ Changelog
 
 .. note:: This version is not yet released and is under active development.
 
+* **BACKWARDS INCOMPATIBLE:**
+  :class:`~cryptography.hazmat.primitives.ciphers.modes.GCM` no longer allows
+  truncation of tags by default. Previous versions of ``cryptography`` allowed
+  tags to be truncated by default, applications wishing to preserve this
+  behavior (not recommended) can pass the ``min_tag_length`` argument.
 * Added :class:`~cryptography.hazmat.primitives.kdf.hkdf.HKDFExpand`.
 * Added :class:`~cryptography.hazmat.primitives.ciphers.modes.CFB8` support
   for :class:`~cryptography.hazmat.primitives.ciphers.algorithms.AES` and
diff --git a/cryptography/__about__.py b/cryptography/__about__.py
index ee53902b..ccbcdfe8 100644
--- a/cryptography/__about__.py
+++ b/cryptography/__about__.py
@@ -28,4 +28,4 @@ __author__ = "The cryptography developers"
 __email__ = "cryptography-dev@python.org"
 
 __license__ = "Apache License, Version 2.0"
-__copyright__ = "Copyright 2013-2014 %s" % __author__
+__copyright__ = "Copyright 2013-2014 {0}".format(__author__)
diff --git a/cryptography/hazmat/primitives/ciphers/modes.py b/cryptography/hazmat/primitives/ciphers/modes.py
index e70a9db5..509b4de2 100644
--- a/cryptography/hazmat/primitives/ciphers/modes.py
+++ b/cryptography/hazmat/primitives/ciphers/modes.py
@@ -97,13 +97,16 @@ class CTR(object):
 class GCM(object):
     name = "GCM"
 
-    def __init__(self, initialization_vector, tag=None):
+    def __init__(self, initialization_vector, tag=None, min_tag_length=16):
         # len(initialization_vector) must in [1, 2 ** 64), but it's impossible
         # to actually construct a bytes object that large, so we don't check
         # for it
-        if tag is not None and len(tag) < 4:
+        if min_tag_length < 4:
+            raise ValueError("min_tag_length must be >= 4")
+        if tag is not None and len(tag) < min_tag_length:
             raise ValueError(
-                "Authentication tag must be 4 bytes or longer."
+                "Authentication tag must be {0} bytes or longer.".format(
+                    min_tag_length)
             )
 
         self.initialization_vector = initialization_vector
diff --git a/docs/hazmat/primitives/symmetric-encryption.rst b/docs/hazmat/primitives/symmetric-encryption.rst
index abc2b076..586285b7 100644
--- a/docs/hazmat/primitives/symmetric-encryption.rst
+++ b/docs/hazmat/primitives/symmetric-encryption.rst
@@ -288,7 +288,7 @@ Modes
         Must be the same number of bytes as the ``block_size`` of the cipher.
         Do not reuse an ``initialization_vector`` with a given ``key``.
 
-.. class:: GCM(initialization_vector, tag=None)
+.. class:: GCM(initialization_vector, tag=None, min_tag_length=16)
 
     .. danger::
 
@@ -318,13 +318,23 @@ Modes
         You can shorten a tag by truncating it to the desired length but this
         is **not recommended** as it lowers the security margins of the
         authentication (`NIST SP-800-38D`_ recommends 96-bits or greater).
-        If you must shorten the tag the minimum allowed length is 4 bytes
-        (32-bits). Applications **must** verify the tag is the expected length
-        to guarantee the expected security margin.
+        Applications wishing to allow truncation must pass the
+        ``min_tag_length`` parameter.
+
+        .. versionchanged:: 0.5
+
+            The ``min_tag_length`` parameter was added in ``0.5``, previously
+            truncation down to ``4`` bytes was always allowed.
 
     :param bytes tag: The tag bytes to verify during decryption. When
         encrypting this must be ``None``.
 
+    :param bytes min_tag_length: The minimum length ``tag`` must be. By default
+        this is ``16``, meaning tag truncation is not allowed. Allowing tag
+        truncation is strongly discouraged for most applications.
+
+    :raises ValueError: This is raised if ``len(tag) < min_tag_length``.
+
     .. testcode::
 
         import os
@@ -356,11 +366,6 @@ Modes
             return (iv, ciphertext, encryptor.tag)
 
         def decrypt(key, associated_data, iv, ciphertext, tag):
-            if len(tag) != 16:
-                raise ValueError(
-                    "tag must be 16 bytes -- truncation not supported"
-                )
-
             # Construct a Cipher object, with the key, iv, and additionally the
             # GCM tag used for authenticating the message.
             decryptor = Cipher(
diff --git a/tests/hazmat/primitives/test_aes.py b/tests/hazmat/primitives/test_aes.py
index 173075d6..5bde7d3c 100644
--- a/tests/hazmat/primitives/test_aes.py
+++ b/tests/hazmat/primitives/test_aes.py
@@ -225,6 +225,6 @@ class TestAESModeGCM(object):
             "gcmEncryptExtIV192.rsp",
             "gcmEncryptExtIV256.rsp",
         ],
-        lambda key: algorithms.AES(key),
-        lambda iv, tag: modes.GCM(iv, tag),
+        algorithms.AES,
+        modes.GCM,
     )
diff --git a/tests/hazmat/primitives/utils.py b/tests/hazmat/primitives/utils.py
index 49b73f01..6428b03e 100644
--- a/tests/hazmat/primitives/utils.py
+++ b/tests/hazmat/primitives/utils.py
@@ -90,7 +90,8 @@ def aead_test(backend, cipher_factory, mode_factory, params):
         cipher = Cipher(
             cipher_factory(binascii.unhexlify(params["key"])),
             mode_factory(binascii.unhexlify(params["iv"]),
-                         binascii.unhexlify(params["tag"])),
+                         binascii.unhexlify(params["tag"]),
+                         len(binascii.unhexlify(params["tag"]))),
             backend
         )
         decryptor = cipher.decryptor()
@@ -108,12 +109,13 @@ def aead_test(backend, cipher_factory, mode_factory, params):
         encryptor.authenticate_additional_data(binascii.unhexlify(aad))
         actual_ciphertext = encryptor.update(binascii.unhexlify(plaintext))
         actual_ciphertext += encryptor.finalize()
-        tag_len = len(params["tag"])
-        assert binascii.hexlify(encryptor.tag)[:tag_len] == params["tag"]
+        tag_len = len(binascii.unhexlify(params["tag"]))
+        assert binascii.hexlify(encryptor.tag[:tag_len]) == params["tag"]
         cipher = Cipher(
             cipher_factory(binascii.unhexlify(params["key"])),
             mode_factory(binascii.unhexlify(params["iv"]),
-                         binascii.unhexlify(params["tag"])),
+                         binascii.unhexlify(params["tag"]),
+                         min_tag_length=tag_len),
             backend
         )
         decryptor = cipher.decryptor()
@@ -309,6 +311,9 @@ def aead_tag_exception_test(backend, cipher_factory, mode_factory):
     with pytest.raises(ValueError):
         mode_factory(binascii.unhexlify(b"0" * 24), b"000")
 
+    with pytest.raises(ValueError):
+        mode_factory(binascii.unhexlify(b"0" * 24), b"000000", 2)
+
     cipher = Cipher(
         cipher_factory(binascii.unhexlify(b"0" * 32)),
         mode_factory(binascii.unhexlify(b"0" * 24), b"0" * 16),