Repository: pyca/cryptography (Python)

Issue:
openssl aes "xts duplicated keys" error is reported as "Unknown OpenSSL Error"
Recent versions of openssl [explicitly reject duplicated AES-XTS keys](https://github.com/openssl/openssl/blob/c39352/crypto/evp/e_aes.c#L3119). Cryptography openssl backend reports this as an Unknown OpenSSL Error.

* Versions of Python, ``cryptography``, ``cffi``, ``pip``, and ``setuptools``
  you're using
  * Python 3.8.5
  * cryptopraphy main branch (f08a7de65)
  * cffi==1.14.5
  * pip 21.0.1
* How you installed ``cryptography``
  * pip install -e .
* Clear steps for reproducing your bug

```
>>> from cryptography.hazmat.primitives.ciphers import Cipher, algorithms, modes
>>> from cryptography.hazmat.backends import default_backend
>>> badkey = b'\xEE'*32
>>> cipher = Cipher(algorithms.AES(badkey), modes.XTS(b'\x00'*16), backend=default_backend())
>>> cipher.encryptor().update(b'\x00'*16)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/angus/tmp/cryptography/src/cryptography/hazmat/primitives/ciphers/base.py", line 109, in encryptor
    ctx = self._backend.create_symmetric_encryption_ctx(
  File "/home/angus/tmp/cryptography/src/cryptography/hazmat/backends/openssl/backend.py", line 460, in create_symmetric_encryption_ctx
    return _CipherContext(self, cipher, mode, _CipherContext._ENCRYPT)
  File "/home/angus/tmp/cryptography/src/cryptography/hazmat/backends/openssl/ciphers.py", line 115, in __init__
    self._backend.openssl_assert(res != 0)
  File "/home/angus/tmp/cryptography/src/cryptography/hazmat/backends/openssl/backend.py", line 210, in openssl_assert
    return binding._openssl_assert(self._lib, ok, errors=errors)
  File "/home/angus/tmp/cryptography/src/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. ([_OpenSSLErrorWithText(code=101511351, lib=6, func=207, reason=183, reason_text=b'error:060CF0B7:digital envelope routines:aesni_xts_init_key:xts duplicated keys')])
```

We can probably submit a patch to convert this into a more user-friendly error. If that sounds good, I was wondering how you prefer to do this. For example a couple of ideas:

1. Python-side check that throws something like a ValueError when parsing the key+mode. (This makes the assumption that cryptography is OK to always reject vulnerable duplicate keys regardless of what the backend does, so I wanted to check that's OK and not going to break anything.)
2. Check the openssl error in hazmat/backends/openssl/ciphers.py and raise something specific there based on `EVP_R_XTS_DUPLICATED_KEYS` error.

Context: This was submitted by one of our users https://github.com/espressif/esptool/issues/621

Thanks very much for all the great work on this library, we really appreciate being able to rely on it!


Hints:
Extra thing-to-maybe-care-about is that in non-FIPS mode, openssl allows the duplicate key for decrypting. So it might be reasonable to do the check in Python but only if encrypting? or do the checks in the Python side of the backend and match the FIPS/not-FIPS logic in Python?
I think handling the `EVP_R_XTS_DUPLICATED_KEYS` error case is the best path, assuming that error occurs in LibreSSL as well (which our CI can help confirm). If it doesn't we'll revisit. If you want to send a PR that'd be great, thank you for the kind words about the library 😄 

Patch:
diff --git a/src/_cffi_src/openssl/cryptography.py b/src/_cffi_src/openssl/cryptography.py
--- a/src/_cffi_src/openssl/cryptography.py
+++ b/src/_cffi_src/openssl/cryptography.py
@@ -41,6 +41,8 @@
 
 #define CRYPTOGRAPHY_OPENSSL_110F_OR_GREATER \
     (OPENSSL_VERSION_NUMBER >= 0x1010006f && !CRYPTOGRAPHY_IS_LIBRESSL)
+#define CRYPTOGRAPHY_OPENSSL_111D_OR_GREATER \
+    (OPENSSL_VERSION_NUMBER >= 0x10101040 && !CRYPTOGRAPHY_IS_LIBRESSL)
 #define CRYPTOGRAPHY_OPENSSL_300_OR_GREATER \
     (OPENSSL_VERSION_NUMBER >= 0x30000000 && !CRYPTOGRAPHY_IS_LIBRESSL)
 
@@ -62,6 +64,7 @@
 
 TYPES = """
 static const int CRYPTOGRAPHY_OPENSSL_110F_OR_GREATER;
+static const int CRYPTOGRAPHY_OPENSSL_111D_OR_GREATER;
 static const int CRYPTOGRAPHY_OPENSSL_300_OR_GREATER;
 
 static const int CRYPTOGRAPHY_OPENSSL_LESS_THAN_111;
diff --git a/src/_cffi_src/openssl/err.py b/src/_cffi_src/openssl/err.py
--- a/src/_cffi_src/openssl/err.py
+++ b/src/_cffi_src/openssl/err.py
@@ -15,6 +15,7 @@
 static const int PKCS12_R_PKCS12_CIPHERFINAL_ERROR;
 static const int PEM_R_UNSUPPORTED_ENCRYPTION;
 static const int EVP_R_UNKNOWN_PBE_ALGORITHM;
+static const int EVP_R_XTS_DUPLICATED_KEYS;
 
 static const int ERR_LIB_EVP;
 static const int ERR_LIB_PEM;
@@ -51,4 +52,8 @@
 #ifndef ERR_LIB_PROV
 #define ERR_LIB_PROV 0
 #endif
+
+#if !CRYPTOGRAPHY_OPENSSL_111D_OR_GREATER
+static const int EVP_R_XTS_DUPLICATED_KEYS = 0;
+#endif
 """
diff --git a/src/_cffi_src/openssl/provider.py b/src/_cffi_src/openssl/provider.py
--- a/src/_cffi_src/openssl/provider.py
+++ b/src/_cffi_src/openssl/provider.py
@@ -17,6 +17,7 @@
 typedef ... OSSL_LIB_CTX;
 
 static const long PROV_R_BAD_DECRYPT;
+static const long PROV_R_XTS_DUPLICATED_KEYS;
 static const long PROV_R_WRONG_FINAL_BLOCK_LENGTH;
 """
 
@@ -33,6 +34,7 @@
 typedef void OSSL_PROVIDER;
 typedef void OSSL_LIB_CTX;
 static const long PROV_R_BAD_DECRYPT = 0;
+static const long PROV_R_XTS_DUPLICATED_KEYS = 0;
 static const long PROV_R_WRONG_FINAL_BLOCK_LENGTH = 0;
 OSSL_PROVIDER *(*OSSL_PROVIDER_load)(OSSL_LIB_CTX *, const char *) = NULL;
 int (*OSSL_PROVIDER_unload)(OSSL_PROVIDER *) = NULL;
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
@@ -112,7 +112,28 @@ def __init__(self, backend, cipher, mode, operation):
             iv_nonce,
             operation,
         )
-        self._backend.openssl_assert(res != 0)
+
+        # Check for XTS mode duplicate keys error
+        errors = self._backend._consume_errors()
+        lib = self._backend._lib
+        if res == 0 and (
+            (
+                lib.CRYPTOGRAPHY_OPENSSL_111D_OR_GREATER
+                and errors[0]._lib_reason_match(
+                    lib.ERR_LIB_EVP, lib.EVP_R_XTS_DUPLICATED_KEYS
+                )
+            )
+            or (
+                lib.Cryptography_HAS_PROVIDERS
+                and errors[0]._lib_reason_match(
+                    lib.ERR_LIB_PROV, lib.PROV_R_XTS_DUPLICATED_KEYS
+                )
+            )
+        ):
+            raise ValueError("In XTS mode duplicated keys are not allowed")
+
+        self._backend.openssl_assert(res != 0, errors=errors)
+
         # We purposely disable padding here as it's handled higher up in the
         # API.
         self._backend._lib.EVP_CIPHER_CTX_set_padding(ctx, 0)


Test patch:
diff --git a/tests/hazmat/primitives/test_aes.py b/tests/hazmat/primitives/test_aes.py
--- a/tests/hazmat/primitives/test_aes.py
+++ b/tests/hazmat/primitives/test_aes.py
@@ -60,6 +60,19 @@ def test_xts_too_short(self):
         with pytest.raises(ValueError):
             enc.update(b"0" * 15)
 
+    @pytest.mark.supported(
+        only_if=lambda backend: (
+            backend._lib.CRYPTOGRAPHY_OPENSSL_111D_OR_GREATER
+        ),
+        skip_message="duplicate key encryption error added in OpenSSL 1.1.1d",
+    )
+    def test_xts_no_duplicate_keys_encryption(self, backend):
+        key = bytes(range(16)) * 2
+        tweak = b"\x00" * 16
+        cipher = base.Cipher(algorithms.AES(key), modes.XTS(tweak))
+        with pytest.raises(ValueError, match="duplicated keys"):
+            cipher.encryptor()
+
 
 @pytest.mark.supported(
     only_if=lambda backend: backend.cipher_supported(