[Hoeze]

tl;dr

• Since 1.8, crypt4gh.sodium holds the GIL while ChaCha20-Poly1305 runs, so threads that encrypt or decrypt run one at a time. Up to 1.7, PyNaCl released it. This PR releases it around the libsodium calls.
• chacha20poly1305_encrypt/decrypt now reject keys that aren't 32 bytes.
• kx_server/kx_client now raise their own error instead of a SystemError.
• CI now runs on pull requests, including those from forks.
• A new test decrypts files that were written by crypt4gh v1.8.6, so CI notices changes that would break existing files.

GIL

PyNaCl is built with cffi, and cffi releases the GIL around every C call. The 1.8 bindings don't, so parallel callers serialize.

The libsodium calls now run between Py_BEGIN_ALLOW_THREADS and Py_END_ALLOW_THREADS. That's safe because the Py_buffer views keep the buffers alive and prevent resizing until they are released. The key exchange and key conversion functions run once per header, so they keep the GIL.

With 4 threads, ChaCha20-Poly1305 throughput now scales by 2.5x to 3x. Before, it dropped below the single-thread rate.

Key sizes

key_len was commented out, but libsodium always reads crypto_aead_chacha20poly1305_ietf_KEYBYTES (32) bytes of key. Both functions now raise ValueError("Invalid key size") for any other length, as PyNaCl did.

kx error paths

When crypto_kx_*_session_keys failed, e.g. for an all-zero peer key, the functions set a ValueError but still returned a key. Python turns that into SystemError: <built-in function kx_client> returned a result with an exception set. They now return NULL, so the ValueError comes through. kx_client had the same pattern (Py_RETURN_NONE with an exception set) when argument parsing failed.

CI on pull requests

testsuite.yml only ran on pushes, so PRs from forks got no CI. It now also runs on pull_request and workflow_dispatch, with a read-only token (permissions: contents: read) and persist-credentials: false. A concurrency group cancels runs that a newer push replaced. For a branch of this repo with an open PR, the testsuite now runs twice, once per event.

Test with files from 1.8.6

bats encrypts and decrypts with the same code. A bug shared by both directions passes every test and still breaks all existing files. tests/compatibility.bats decrypts two files that the released 1.8.6 wrote for Alice, one sent with Bob's crypt4gh key and one with Bob's ssh key. For the second, --sender_pk also checks the conversion of Bob's ssh public key. Only the public half of that ssh test key is committed.

Stack

This PR is part 1 of 3. Two PRs build on it:

• part 2, build: portable wheels, buildable sdist, pyproject.toml instead of setup.py (#58)
• part 3, refactor: replace libsodium with cryptography (#59)

[Hoeze]

Hi @silverdaz, thanks for reading through all of them.

Regarding pytest, and format correctness, this is not the place to do it.
Especially the non-repeating nonce. It's correct to have repeating nonces. It's not recommended as it weakens the encryption, but it's a valid format.

Agreed, this PR is not the place for it. I'd still strongly recommend adding such a test for crypt4gh later.
I see it as a test of the implementation: It checks that crypt4gh itself never writes a repeated nonce.
It would not reject valid files from other tools that do.

Done right, such tests could also be reused for other implementations.
My reason for caring is EGA-archive/crypt4gh-rust#5, which uses one nonce for all segments of a file. I'd appreciate your view on how serious this is for files written by crypt4gh-rust. I'm happy to share what I found by email.

PRs are in such a way that it feels like "take all or nothing".

I'm sorry for having created this feeling. I split the work into three parts that build on each other, each adding fewer than 300 lines:

• part 1: +92/-10
• part 2: +158/-171
• part 3: +277/-900

Most deletions in part 3 remove sodium.c and the libsodium build.
The later PRs look bigger than they are. GitHub cannot stack PRs from forks, so each PR also contains the parts before it.

These links show only the new commits of each PR:

• Part 2 (build: portable wheels, buildable sdist, pyproject.toml instead of setup.py #58):
  https://github.com/EGA-archive/crypt4gh/pull/58/changes/014ac2649e104c39cfb5c836c82cb0bdb7e9eab8..fde2c263aec673439b23adb8819275f06a8500e5
• Part 3 (refactor: replace libsodium with cryptography #59):
  https://github.com/EGA-archive/crypt4gh/pull/59/changes/fde2c263aec673439b23adb8819275f06a8500e5..f596e87b81bbb66269e661cdefd327652ce6413c

I'm happy to split or reorder them differently if that helps.

I've made changes to the upcoming version 2, and I'd like to merge with those efforts too.

I didn't know about the version 2 work. Is there a branch I could look at? I'm happy to rebase onto it.