pyca / cryptography

cryptography is a package designed to expose cryptographic primitives and recipes to Python developers.
https://cryptography.io
Other
6.66k stars 1.53k forks source link

[Patch pre-check] Fernet in Rust? #9095

Closed Hooksie closed 3 months ago

Hooksie commented 1 year ago

Hello all,

We've been doing some profiling on our application hotspots, and this recently led us to our usage of cryptography.Fernet. One of our Engineers noticed that an alternative implementation is currently available, rfernet, which is roughly 3.5x to 7.5x faster in our benchmarks than the cryptography implementation.

I'm assuming this is mostly due to rfernet being fully compiled, while much of the cryptograph implementation is in Python.

Is there any interest in a patch that would move the cryptography implementation into Rust? Would like to feel out the maintainers opinions here on what should and shouldn't be in Rust before jumping in

-- Benchmarks for completeness (against latest version), running on 2021 M1 pro.

Performance gap decreases as payload size increases. (Our use case is for small, ~25 byte payloads)

In [12]: payload = b"<...>" # ~25 bytes

In [13]: %timeit r_fernet.encrypt(payload)
2.25 µs ± 9.96 ns per loop (mean ± std. dev. of 7 runs, 100,000 loops each)

In [14]: %timeit cryptography_fernet.encrypt(payload)
19.4 µs ± 42.3 ns per loop (mean ± std. dev. of 7 runs, 100,000 loops each)
reaperhulk commented 1 year ago

We’re thrilled to have more things migrate to Rust (and have recently migrated our OpenSSL bindings so that we can use the openssl crate alongside our older cffi C bindings). You can look at https://github.com/pyca/cryptography/pull/9024, https://github.com/pyca/cryptography/pull/8768, https://github.com/pyca/cryptography/pull/8691, and https://github.com/pyca/cryptography/pull/7933 for examples of how we've done some of that migration in the past.

That said, Fernet currently depends on our Cipher layer, which isn't a trivial migration job (and is also likely a significant part of your perf issues, as we create a lot of Python objects for every Fernet invocation). The ideal conversion here would be migrating Cipher (while removing a layer of indirection probably as well) and then migrating Fernet after that.

alex commented 1 year ago

FWIW, I'm the original author of the rust module that rfernet uses, so everything comes full circle:-)

On Tue, Jun 20, 2023, 3:04 PM Paul Kehrer @.***> wrote:

We’re thrilled to have more things migrate to Rust (and have recently migrated our OpenSSL bindings so that we can use the openssl crate alongside our older cffi C bindings). You can look at #9024 https://github.com/pyca/cryptography/pull/9024, #8768 https://github.com/pyca/cryptography/pull/8768, #8691 https://github.com/pyca/cryptography/pull/8691, and #7933 https://github.com/pyca/cryptography/pull/7933 for examples of how we've done some of that migration in the past.

That said, Fernet currently depends on our Cipher layer, which isn't a trivial migration job (and is also likely a significant part of your perf issues, as we create a lot of Python objects for every Fernet invocation). The ideal conversion here would be migrating Cipher (while removing a layer of indirection probably as well) and then migrating Fernet after that.

— Reply to this email directly, view it on GitHub https://github.com/pyca/cryptography/issues/9095#issuecomment-1599355961, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAAGBHHDV7Z4YOMYD6ZSPTXMHX47ANCNFSM6AAAAAAZNV4KVQ . You are receiving this because you are subscribed to this thread.Message ID: @.***>

Hooksie commented 1 year ago

@alex Small internet :)

@reaperhulk Thanks for the heads up there. As you probably guessed, I havent profiled anything beyond the surface API, but hearing that makes sense. I can take a look at Cipher as well. I doubt I'll have any quick results here, but if you have suggestions on the structure of a rusty-ciphers that would definitely be helpful

alex commented 1 year ago

8770 tracks migrating various things to Rust, so you can see what each of those PRs looks like.

Please note that migrating cipher is likely to be a substantial undertaking (it's why I've been procrastinating on doing it)

reaperhulk commented 3 months ago

Closing this since there hasn't been activity in a while. All of cipher is in rust now though so this is possible