Skip to content

Refactor with Cryptography #62

Open

Description

@silverdaz

Instead of using libsodium, and the binding to 7 functions (in sodium.c), we could use the cryptography package.
The latter already handles the bindings and does implement x25519 and chacha20poly1305 that we need (including AEAD for version 2).

I very much like that idea: we offload the custom compilation (be it C or Rust) to the (well-maintainted) cryptography package.

We could rename sodium.c into crypto.py, as suggested in PR #59 by Cito and Hoeze.

Note: We should make sure the speed is similar.
I don't want to swap a working implementation with a slower one.

@Hoeze, please refactor your PR (or create a new one) to only include the above issue.
Dumping 3 gigantic PRs without opening issues, (ie a place to discuss ideas), is not a proper practice. Morover, the PRs touch many files with different concerns, so it makes it a "all-or-nothing" which is not ideal.
One thing at a time (and after refactoring, we'll probably close the 3 current PRs, in favor for the new coming ones).

There is a small issue with the openssh keys parsing.
We were using 2 libsodium functions to convert ed25519 to x25519. This doesn't seem to be in Cryptography, since you re-implemented it in PR #59.
I'm surprised it is not, so maybe we can double-check before implementing a python version.

Since I'm working on a plugin system to allow loading Crypt4GH-compatible keys from different sources (like keyctl in the kernel, pem-formatted files, openssh, pkcs#n formats...), the openssh-keys parsing will get moved out. Probably to a separate package.

So here is my suggestion: let's keep using the libsodium functions, but not from the C-code but from PyNaCl (like before). We moved away from that latter, because of the lack of flexibility for buffer allocation and re-use.
Although it keeps the dependency, crypt4gh would still remain a pure python package.
We avoid the need to custom compile C-code, and we can close PR#57 about the GIL (we won't need it)

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions