Add support for ChaCha20 with LibreSSL #9209

facutuesca · 2023-07-10T10:50:37Z

This PR adds support for ChaCha20 with LibreSSL. Since cryptography's ChaCha20 uses OpenSSL's implementation (which uses a 64 bit counter + 64 bit nonce), and LibreSSL does the same, it is straightforward to use LibreSSL's API.

The only complication is that the current _CipherContext implementation assumes that the underlying cipher can be accessed through the EVP_CIPHER API. This is not the case for LibreSSL's ChaCha20, which has a separate CRYPTO_chacha_20() API for it.

In order to solve that, this PR makes _CipherContext an abstract class with two implementations, _CipherContextEVP (which is the previous _CipherContext) and _CipherContextChaCha, which is a simple context that is used only for the LibreSSL+ChaCha20 combination.

This means all of the code that uses _CipherContext remains the same, and we only have a single branching point isolated in the create_cipher_context() function, which selects the correct context depending on the cipher+backend.

It also leaves the option open in the future if we want to add BoringSSL's ChaCha20, since we can reuse the same _CipherContextChaCha for it (the API is almost the same, and we can deal with the differences in a C wrapper)

src/_cffi_src/openssl/chacha.py

alex · 2023-07-27T11:01:02Z

src/cryptography/hazmat/backends/openssl/backend.py

@@ -226,6 +229,12 @@ def hmac_supported(self, algorithm: hashes.HashAlgorithm) -> bool:
        return self.hash_supported(algorithm)

    def cipher_supported(self, cipher: CipherAlgorithm, mode: Mode) -> bool:
+        # ChaCha20 is supported in LibreSSL by a different API than OpenSSL,
+        # so checking for the corresponding EVP_CIPHER is not useful
+        if (self._lib.CRYPTOGRAPHY_IS_LIBRESSL) and isinstance(


Suggested change

if (self._lib.CRYPTOGRAPHY_IS_LIBRESSL) and isinstance(

if self._lib.CRYPTOGRAPHY_IS_LIBRESSL and isinstance(

alex · 2023-07-27T11:01:48Z

src/cryptography/hazmat/backends/openssl/ciphers.py

@@ -14,7 +15,114 @@
    from cryptography.hazmat.backends.openssl.backend import Backend


-class _CipherContext:
+class _CipherContext(metaclass=abc.ABCMeta):


Why is a new interface required? This seems to match our existing public cipher context ABC.

It's an interface for the backend cipher context, which needs a couple of methods not present in the existing public CipherContext ABC:

@abc.abstractmethod def authenticate_additional_data(self, data: bytes) -> None: ... @abc.abstractmethod def finalize_with_tag(self, tag: bytes) -> bytes: ...

I think it also makes sense to have separate interfaces for public vs backend ctx, since the public context wraps the backend one:

def _wrap_ctx( self, ctx: _BackendCipherContext, encrypt: bool ) -> typing.Union[ AEADEncryptionContext, AEADDecryptionContext, CipherContext ]: if isinstance(self.mode, modes.ModeWithAuthenticationTag): if encrypt: return _AEADEncryptionContext(ctx) else: return _AEADDecryptionContext(ctx) else: return _CipherContext(ctx)

alex · 2023-07-27T11:03:28Z

src/cryptography/hazmat/backends/openssl/ciphers.py

+            total_data_len,
+            self._backend._ffi.from_buffer(self._cipher.key),
+            self._backend._ffi.from_buffer(self._iv_nonce),
+            self._counter,


Hmm, shouldn't counter be advancing across multiple calls?

Yes, I missed that completely. I have rewritten the update_into() function as per our discussion

facutuesca · 2023-08-08T15:34:15Z

The current CI failure is due to not having coverage of these lines (the counter rollover during 64-bit overflow). Once the test vectors from this PR are merged, the coverage should go to 100%

alex · 2023-08-11T11:55:56Z

Ok. @reaperhulk has the TODO to review that one, once it merges I'll look here.

facutuesca · 2023-08-31T14:50:08Z

Ok. @reaperhulk has the TODO to review that one, once it merges I'll look here.

@alex The test vectors PR was merged and now the coverage for this code is 100%

alex · 2023-09-04T13:38:12Z

I now realize I'm doing this out of order, but did we ever talk to the LibreSSL maintainers about if they'd be interested in an EVP API for this?

facutuesca · 2023-09-04T17:03:44Z

I now realize I'm doing this out of order, but did we ever talk to the LibreSSL maintainers about if they'd be interested in an EVP API for this?

We haven't, I believe, because the main goal for these PRs was to improve BoringSSL feature parity, and this LibreSSL implementation was just a bonus due to the LibreSSL API being the same as BoringSSL's.

My idea was to follow up this PR with the BoringSSL implementation (which is trivial once this LibreSSL PR is merged). I don't know if we're currently interested in implementing the EVP API for ChaCha20 in LibreSSL (@woodruffw thoughts?)

woodruffw · 2023-09-05T18:59:10Z

I don't know if we're currently interested in implementing the EVP API for ChaCha20 in LibreSSL

I believe that doing that implementation in LibreSSL would be out of scope for us. That being said, we can certainly raise it upstream and see if they'd be interested in exposing ChaCha20 via EVP_CIPHER.

Would you prefer we do that before moving forwards here @alex?

botovq · 2023-10-19T22:54:23Z

I must be missing something. EVP_chacha20 has existed in LibreSSL since forever (it was committed on May 1, 2014, not even a month into the fork's existence). Regrettably, we adjusted it to be compatible with OpenSSL for OpenSSH to be able to use the OpenSSL 1.1 API in early 2020, even if that is wrong per spec.

facutuesca · 2023-10-23T13:24:09Z

I must be missing something. EVP_chacha20 has existed in LibreSSL since forever (it was committed on May 1, 2014, not even a month into the fork's existence).

Thanks for the clarification, I now see why I thought the EVP API was not available in LibreSSL: Turns out that cryptography uses EVP_get_cipherbyname() to get the EVP cipher using the corresponding name. When I tried this with LibreSSL, it seemed as if the cipher was not available.

However, the error was because the name for ChaCha20 in OpenSSL is different from LibreSSL. OpenSSL uses chacha20 and ChaCha20:

#define SN_chacha20             "ChaCha20"
#define LN_chacha20             "chacha20"
#define NID_chacha20            1019

Whereas LibresSSL uses chacha and ChaCha:

# ChaCha Stream Cipher
!Cname chacha20
			: ChaCha		: chacha

I'll close this PR and open a new one that conditionally changes the name according to the backend used.
@botovq Do you know why the names don't match?

cc @woodruffw

botovq · 2023-10-23T13:34:06Z

I see.

@botovq Do you know why the names don't match?

There will be no deeper reason than that it was added one way to LibreSSL and the other way to OpenSSL and no one noticed in the following 9 years... I guess there is no harm in adding an alias on our side to make OpenSSL's naming choice work, but that won't be available before 3.9.0

facutuesca · 2023-10-24T09:57:35Z

Closing in favor of #9758

OpenSSL has the 20 in the long and short names, so add aliases to the existing names to make things work. In particular, EVP_get_cipherbyname() will now return EVP_chacha20() for both 'ChaCha20' and 'chacha20'. Found by Facundo Tuesca when trying to add LibreSSL support for ChaCha20 in pyca/cryptography#9209 ok jsing

facutuesca force-pushed the chacha20-libressl branch 2 times, most recently from 58e6efe to 2cc4a53 Compare July 10, 2023 12:10

facutuesca mentioned this pull request Jul 11, 2023

Add test vectors for ChaCha20 counter overflow #9221

Merged

alex reviewed Jul 27, 2023

View reviewed changes

facutuesca force-pushed the chacha20-libressl branch 4 times, most recently from 1e90828 to 6cffe5a Compare August 8, 2023 14:58

facutuesca requested a review from alex August 8, 2023 17:01

facutuesca force-pushed the chacha20-libressl branch from 6cffe5a to b446e66 Compare August 31, 2023 14:25

Add support for ChaCha20 with LibreSSL

3ad3d35

facutuesca force-pushed the chacha20-libressl branch from b446e66 to 3ad3d35 Compare August 31, 2023 14:39

facutuesca mentioned this pull request Oct 24, 2023

Add support for ChaCha20 in LibreSSL #9758

Merged

facutuesca closed this Oct 24, 2023

facutuesca deleted the chacha20-libressl branch October 24, 2023 12:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for ChaCha20 with LibreSSL #9209

Add support for ChaCha20 with LibreSSL #9209

facutuesca commented Jul 10, 2023

alex Jul 27, 2023

facutuesca Aug 8, 2023

alex Jul 27, 2023

facutuesca Aug 8, 2023

alex Jul 27, 2023

facutuesca Aug 8, 2023

facutuesca commented Aug 8, 2023 •

edited

Loading

alex commented Aug 11, 2023

facutuesca commented Aug 31, 2023

alex commented Sep 4, 2023

facutuesca commented Sep 4, 2023 •

edited

Loading

woodruffw commented Sep 5, 2023

botovq commented Oct 19, 2023 •

edited

Loading

facutuesca commented Oct 23, 2023

botovq commented Oct 23, 2023 via email

facutuesca commented Oct 24, 2023

	if (self._lib.CRYPTOGRAPHY_IS_LIBRESSL) and isinstance(
	if self._lib.CRYPTOGRAPHY_IS_LIBRESSL and isinstance(

Add support for ChaCha20 with LibreSSL #9209

Add support for ChaCha20 with LibreSSL #9209

Conversation

facutuesca commented Jul 10, 2023

alex Jul 27, 2023

Choose a reason for hiding this comment

facutuesca Aug 8, 2023

Choose a reason for hiding this comment

alex Jul 27, 2023

Choose a reason for hiding this comment

facutuesca Aug 8, 2023

Choose a reason for hiding this comment

alex Jul 27, 2023

Choose a reason for hiding this comment

facutuesca Aug 8, 2023

Choose a reason for hiding this comment

facutuesca commented Aug 8, 2023 • edited Loading

alex commented Aug 11, 2023

facutuesca commented Aug 31, 2023

alex commented Sep 4, 2023

facutuesca commented Sep 4, 2023 • edited Loading

woodruffw commented Sep 5, 2023

botovq commented Oct 19, 2023 • edited Loading

facutuesca commented Oct 23, 2023

botovq commented Oct 23, 2023 via email

facutuesca commented Oct 24, 2023

facutuesca commented Aug 8, 2023 •

edited

Loading

facutuesca commented Sep 4, 2023 •

edited

Loading

botovq commented Oct 19, 2023 •

edited

Loading