Skip to content

Switch "crypto" implementation to Python - #457

Draft
jlaine wants to merge 1 commit into
aiortc:mainfrom
jlaine:python-crypto
Draft

Switch "crypto" implementation to Python#457
jlaine wants to merge 1 commit into
aiortc:mainfrom
jlaine:python-crypto

Conversation

@jlaine

@jlaine jlaine commented Jan 16, 2024

Copy link
Copy Markdown
Contributor

This simplifies the build process and avoids needing to ship a vendor'd OpenSSL.

@codecov

codecov Bot commented Jan 16, 2024

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (483a66f) to head (aca1ddd).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #457   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           25        26    +1     
  Lines         5121      5186   +65     
=========================================
+ Hits          5121      5186   +65     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jlaine

jlaine commented Jan 23, 2024

Copy link
Copy Markdown
Contributor Author

Initial testing suggest a 10x slowdown of packet protection compared to the current C code.

@jlaine
jlaine force-pushed the python-crypto branch 2 times, most recently from e28fb8a to eb3a5f2 Compare January 23, 2024 19:05
@rthalley

Copy link
Copy Markdown
Contributor

I haven't tried measuring it yet. My prior experience is that cryptography is pretty fast, and I'm wondering if the header protection part might be the slow thing. If it were, it might be possible to do the crypto with cryptography and then do the masking with a tiny bit of C code.

@jlaine

jlaine commented Feb 27, 2024

Copy link
Copy Markdown
Contributor Author

I haven't tried measuring it yet. My prior experience is that cryptography is pretty fast, and I'm wondering if the header protection part might be the slow thing. If it were, it might be possible to do the crypto with cryptography and then do the masking with a tiny bit of C code.

AFAICT it's the actual crypto code unfortunately that is slower.

@rthalley

Copy link
Copy Markdown
Contributor

Can you say how you are benchmarking? I'm guessing I'm doing something wrong, but I'm testing by using the example client to pull 20MiB from nginx, and while it is slower it's not too bad, only around 1.074 times the non-cryptography version.

Regular aioquic (main from your branch with python-crypto):

2024-02-27 08:04:13,412 INFO client Response received for GET /foo.bin : 20971520 bytes in 0.4 s (428.916 Mbps)
2024-02-27 08:04:13,414 INFO quic [1da750bf1e625c3a] Connection close sent (code 0x100, reason )
python examples/http3_client.py https://localhost:8080/foo.bin  0.46s user 0.11s system 84% cpu 0.674 total

Your python-crypto branch:

024-02-27 08:12:30,026 INFO client Response received for GET /foo.bin : 20971520 bytes in 0.4 s (389.185 Mbps)
2024-02-27 08:12:30,028 INFO quic [254cea9a35deb3ff] Connection close sent (code 0x100, reason )
python examples/http3_client.py https://localhost:8080/foo.bin  0.41s user 0.07s system 66% cpu 0.724 total

@jlaine

jlaine commented Feb 27, 2024

Copy link
Copy Markdown
Contributor Author

Here is what I use:

import binascii
import time

from aioquic.quic.crypto import CryptoPair

from tests.test_crypto import (
    LONG_CLIENT_PACKET_NUMBER,
    LONG_CLIENT_PLAIN_HEADER,
    LONG_CLIENT_PLAIN_PAYLOAD,
    PROTOCOL_VERSION,
)

pair = CryptoPair()
pair.setup_initial(
    cid=binascii.unhexlify("8394c8f03e515708"),
    is_client=True,
    version=PROTOCOL_VERSION,
)
start = time.time()
for i in range(100000):
    pair.encrypt_packet(
        LONG_CLIENT_PLAIN_HEADER,
        LONG_CLIENT_PLAIN_PAYLOAD,
        LONG_CLIENT_PACKET_NUMBER,
    )
elapsed = time.time() - start
print(elapsed)

A typical run for me puts the C code at about 70ms and the Python code at around 800ms.

Notes:

  • Make sure you purge your build directory when switching to the python-crypto branch, otherwise you will still have the C version in the form of a .so :)
  • The main branch of cryptography may perform differently as more code has been moved to rust.
  • ChaCha20 is going to regress even further, but there is a WIP patch which helps: ChaCha20 API does not allow updating the nonce pyca/cryptography#10193

@rthalley

Copy link
Copy Markdown
Contributor

Yeah, I discovered the need to purge while I was benchmarking. The timings I reported are after making sure I was testing in clean setups.

@rthalley

Copy link
Copy Markdown
Contributor

And of course both of our testing could be accurate, as you're testing just the crypto whereas I'm testing what the overall effect in typical use is.

@jlaine

jlaine commented May 19, 2024

Copy link
Copy Markdown
Contributor Author

I wish cryptography 43 were already out, preferably featuring:

pyca/cryptography#10437

As things currently stand, the build of the C code has broken again through no change of our own:

https://github.com/jlaine/aioquic/actions/runs/9150448224

@jlaine
jlaine force-pushed the python-crypto branch 3 times, most recently from 157e70c to 427ddf2 Compare May 19, 2024 21:10
@jlaine jlaine mentioned this pull request Jul 21, 2024
@jlaine
jlaine force-pushed the python-crypto branch 2 times, most recently from f0face7 to 0d13a52 Compare October 7, 2025 17:44
This simplifies the build process and avoids needing to ship a vendor'd
OpenSSL.
@mvollrath

Copy link
Copy Markdown

Ran the benchmark today to see how well it performs on the optimized upstream cryptography. I modified the above posted bench to refer to test_crypto_v2 instead of test_crypto and increased the number of iterations to 1,000,000.

This branch, with the _crypto.abi3.so removed: 3.882s
main branch: 1.642s

Tested with Python 3.14 via uv, which resolved this tree:

$ uv tree
Resolved 14 packages in 1ms
aioquic
├── certifi v2025.11.12
├── cryptography v46.0.3
│   └── cffi v2.0.0
│       └── pycparser v2.23
├── pylsqpack v0.3.23
├── pyopenssl v25.3.0
│   └── cryptography v46.0.3 (*)
├── service-identity v24.2.0
│   ├── attrs v25.4.0
│   ├── cryptography v46.0.3 (*)
│   ├── pyasn1 v0.6.1
│   └── pyasn1-modules v0.4.2
│       └── pyasn1 v0.6.1
└── coverage[toml] v7.12.0 (extra: dev)
(*) Package tree already displayed

I think 250k encrypts/s is well within what Python users expect from Python.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants