Repository: pyca/cryptography (Python)

Issue:
Unknown OpenSSL error with latest python docker image
I'm seeing the following openssl error when I'm trying to use cryptography for aes-gcm. Attached are the detailed steps for repro and the results from my terminal. The outputs also contain the versions of python, openssl and cryptography.

# Repro

## Detailed Repro Steps
### Dockerfile
```dockerfile
FROM python:3.9

WORKDIR /app

RUN pip install --no-cache-dir cryptography
```

### Build and Run
```bash
docker build -f Dockerfile -t crypto-repro .
docker run --rm -it crypto-repro python3
```

### Code
```python
import cryptography
cryptography.__version__
from cryptography.hazmat.backends.openssl.backend import backend;print(backend.openssl_version_text())
from cryptography.hazmat.backends import default_backend
from cryptography.hazmat.primitives.ciphers import (Cipher, algorithms, modes)
Cipher(algorithms.AES('asdfasdfasdfasdfasdfasdfasdfasdf'.encode()), modes.GCM('asdasdfasdfasdfasdff'.encode(), 'asdfasdfaasdfasdfasdfsdf'.encode()), backend=default_backend()).decryptor()
```

##### Output
```python
Python 3.9.6 (default, Aug 17 2021, 15:37:33)
[GCC 10.2.1 20210110] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import cryptography
>>> cryptography.__version__
'3.4.8'
>>> from cryptography.hazmat.backends import default_backend
>>> from cryptography.hazmat.primitives.ciphers import (Cipher, algorithms, modes)
>>> from cryptography.hazmat.backends.openssl.backend import backend;print(backend.openssl_version_text())
OpenSSL 1.1.1l  24 Aug 2021
>>> Cipher(algorithms.AES('asdfasdfasdfasdfasdfasdfasdfasdf'.encode()), modes.GCM('asdasdfasdfasdfasdff'.encode(), 'asdfasdfaasdfasdfasdfsdf'.encode()), backend=default_backend()).decryptor()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/lib/python3.9/site-packages/cryptography/hazmat/primitives/ciphers/base.py", line 115, in decryptor
    ctx = self._backend.create_symmetric_decryption_ctx(
  File "/usr/local/lib/python3.9/site-packages/cryptography/hazmat/backends/openssl/backend.py", line 487, in create_symmetric_decryption_ctx
    return _CipherContext(self, cipher, mode, _CipherContext._DECRYPT)
  File "/usr/local/lib/python3.9/site-packages/cryptography/hazmat/backends/openssl/ciphers.py", line 103, in __init__
    self._backend.openssl_assert(res != 0)
  File "/usr/local/lib/python3.9/site-packages/cryptography/hazmat/backends/openssl/backend.py", line 241, in openssl_assert
    return binding._openssl_assert(self._lib, ok, errors=errors)
  File "/usr/local/lib/python3.9/site-packages/cryptography/hazmat/bindings/openssl/binding.py", line 80, in _openssl_assert
    raise InternalError(
cryptography.exceptions.InternalError: Unknown OpenSSL error. This error is commonly encountered when another library is not cleaning up the OpenSSL error stack. If you are using cryptography with another library that uses OpenSSL try disabling it before reporting a bug. Otherwise please file an issue at https://github.com/pyca/cryptography/issues with information on how to reproduce this. ([])
>>>
Please do not report security issues on Github! Follow the instructions in our
documentation for reporting security issues:
https://cryptography.io/en/latest/security.html
```

Please let me know if I'm using the library wrong, or if there's a missing dependency and how this can be fixed.


Hints:
Thanks for the bug report. The issue here is that you are passing a 24 byte tag. GCM tags are, at most, 16 bytes. This is still a bug in our library though because we should catch this and raise a `ValueError` instead.

Patch:
diff --git a/src/cryptography/hazmat/backends/openssl/ciphers.py b/src/cryptography/hazmat/backends/openssl/ciphers.py
--- a/src/cryptography/hazmat/backends/openssl/ciphers.py
+++ b/src/cryptography/hazmat/backends/openssl/ciphers.py
@@ -238,12 +238,19 @@ def finalize(self) -> bytes:
         return self._backend._ffi.buffer(buf)[: outlen[0]]
 
     def finalize_with_tag(self, tag: bytes) -> bytes:
-        if len(tag) < self._mode._min_tag_length:
+        tag_len = len(tag)
+        if tag_len < self._mode._min_tag_length:
             raise ValueError(
                 "Authentication tag must be {} bytes or longer.".format(
                     self._mode._min_tag_length
                 )
             )
+        elif tag_len > self._block_size_bytes:
+            raise ValueError(
+                "Authentication tag cannot be more than {} bytes.".format(
+                    self._block_size_bytes
+                )
+            )
         res = self._backend._lib.EVP_CIPHER_CTX_ctrl(
             self._ctx, self._backend._lib.EVP_CTRL_AEAD_SET_TAG, len(tag), tag
         )
diff --git a/src/cryptography/hazmat/primitives/ciphers/__init__.py b/src/cryptography/hazmat/primitives/ciphers/__init__.py
--- a/src/cryptography/hazmat/primitives/ciphers/__init__.py
+++ b/src/cryptography/hazmat/primitives/ciphers/__init__.py
@@ -3,13 +3,15 @@
 # for complete details.
 
 
+from cryptography.hazmat.primitives._cipheralgorithm import (
+    BlockCipherAlgorithm,
+    CipherAlgorithm,
+)
 from cryptography.hazmat.primitives.ciphers.base import (
     AEADCipherContext,
     AEADDecryptionContext,
     AEADEncryptionContext,
-    BlockCipherAlgorithm,
     Cipher,
-    CipherAlgorithm,
     CipherContext,
 )
 
diff --git a/src/cryptography/hazmat/primitives/ciphers/base.py b/src/cryptography/hazmat/primitives/ciphers/base.py
--- a/src/cryptography/hazmat/primitives/ciphers/base.py
+++ b/src/cryptography/hazmat/primitives/ciphers/base.py
@@ -16,20 +16,10 @@
 )
 from cryptography.hazmat.backends import _get_backend
 from cryptography.hazmat.backends.interfaces import Backend, CipherBackend
-from cryptography.hazmat.primitives._cipheralgorithm import (
-    CipherAlgorithm as CipherAlgorithm,
-)
+from cryptography.hazmat.primitives._cipheralgorithm import CipherAlgorithm
 from cryptography.hazmat.primitives.ciphers import modes
 
 
-class BlockCipherAlgorithm(metaclass=abc.ABCMeta):
-    @abc.abstractproperty
-    def block_size(self) -> int:
-        """
-        The size of a block as an integer in bits (e.g. 64, 128).
-        """
-
-
 class CipherContext(metaclass=abc.ABCMeta):
     @abc.abstractmethod
     def update(self, data: bytes) -> bytes:
diff --git a/src/cryptography/hazmat/primitives/ciphers/modes.py b/src/cryptography/hazmat/primitives/ciphers/modes.py
--- a/src/cryptography/hazmat/primitives/ciphers/modes.py
+++ b/src/cryptography/hazmat/primitives/ciphers/modes.py
@@ -7,7 +7,11 @@
 import typing
 
 from cryptography import utils
-from cryptography.hazmat.primitives._cipheralgorithm import CipherAlgorithm
+from cryptography.exceptions import UnsupportedAlgorithm, _Reasons
+from cryptography.hazmat.primitives._cipheralgorithm import (
+    BlockCipherAlgorithm,
+    CipherAlgorithm,
+)
 
 
 class Mode(metaclass=abc.ABCMeta):
@@ -229,3 +233,15 @@ def initialization_vector(self) -> bytes:
 
     def validate_for_algorithm(self, algorithm: CipherAlgorithm) -> None:
         _check_aes_key_length(self, algorithm)
+        if not isinstance(algorithm, BlockCipherAlgorithm):
+            raise UnsupportedAlgorithm(
+                "GCM requires a block cipher algorithm",
+                _Reasons.UNSUPPORTED_CIPHER,
+            )
+        block_size_bytes = algorithm.block_size // 8
+        if self._tag is not None and len(self._tag) > block_size_bytes:
+            raise ValueError(
+                "Authentication tag cannot be more than {} bytes.".format(
+                    block_size_bytes
+                )
+            )


Test patch:
diff --git a/tests/hazmat/primitives/test_aes_gcm.py b/tests/hazmat/primitives/test_aes_gcm.py
--- a/tests/hazmat/primitives/test_aes_gcm.py
+++ b/tests/hazmat/primitives/test_aes_gcm.py
@@ -168,12 +168,13 @@ def test_gcm_tag_decrypt_finalize(self, backend):
 
         decryptor.finalize_with_tag(tag)
 
-    def test_gcm_tag_decrypt_finalize_tag_length(self, backend):
+    @pytest.mark.parametrize("tag", [b"tagtooshort", b"toolong" * 12])
+    def test_gcm_tag_decrypt_finalize_tag_length(self, tag, backend):
         decryptor = base.Cipher(
             algorithms.AES(b"0" * 16), modes.GCM(b"0" * 12), backend=backend
         ).decryptor()
         with pytest.raises(ValueError):
-            decryptor.finalize_with_tag(b"tagtooshort")
+            decryptor.finalize_with_tag(tag)
 
     def test_buffer_protocol(self, backend):
         data = bytearray(b"helloworld")
diff --git a/tests/hazmat/primitives/test_ciphers.py b/tests/hazmat/primitives/test_ciphers.py
--- a/tests/hazmat/primitives/test_ciphers.py
+++ b/tests/hazmat/primitives/test_ciphers.py
@@ -209,6 +209,14 @@ def test_invalid_backend():
         )
 
 
+def test_invalid_gcm_algorithm():
+    with raises_unsupported_algorithm(_Reasons.UNSUPPORTED_CIPHER):
+        ciphers.Cipher(
+            ARC4(b"\x00" * 16),
+            modes.GCM(b"\x00" * 12),
+        )
+
+
 @pytest.mark.supported(
     only_if=lambda backend: backend.cipher_supported(
         AES(b"\x00" * 16), modes.ECB()
diff --git a/tests/hazmat/primitives/utils.py b/tests/hazmat/primitives/utils.py
--- a/tests/hazmat/primitives/utils.py
+++ b/tests/hazmat/primitives/utils.py
@@ -332,6 +332,13 @@ 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):
+        Cipher(
+            cipher_factory(binascii.unhexlify(b"0" * 32)),
+            mode_factory(binascii.unhexlify(b"0" * 24), b"toolong" * 12),
+            backend,
+        )
+
     with pytest.raises(ValueError):
         mode_factory(binascii.unhexlify(b"0" * 24), b"000000", 2)