pynacl icon indicating copy to clipboard operation
pynacl copied to clipboard

AeadKx

Open gonatienza opened this issue 1 year ago • 5 comments

Recently thought about building this wrapper to simplify and harden the usage of newer key exchange apis along the newer aead constructs:

crypto_kx_client_session_keys(unsigned char rx[crypto_kx_SESSIONKEYBYTES],
                              unsigned char tx[crypto_kx_SESSIONKEYBYTES],
                              const unsigned char client_pk[crypto_kx_PUBLICKEYBYTES],
                              const unsigned char client_sk[crypto_kx_SECRETKEYBYTES],
                              const unsigned char server_pk[crypto_kx_PUBLICKEYBYTES]);

crypto_kx_server_session_keys(unsigned char rx[crypto_kx_SESSIONKEYBYTES],
                              unsigned char tx[crypto_kx_SESSIONKEYBYTES],
                              const unsigned char server_pk[crypto_kx_PUBLICKEYBYTES],
                              const unsigned char server_sk[crypto_kx_SECRETKEYBYTES],
                              const unsigned char client_pk[crypto_kx_PUBLICKEYBYTES]);

It would provide an xchacha20poly1305_ietf option for nacl.public. Besides the existing xsalsa20poly1305 from crypto_box. Plus the capability of using rx and tx keys (which would help with nonce counters use cases).

Here's a simple test as a quick visual aid:

from nacl.public import PrivateKx, AeadClient, AeadServer

skbob = PrivateKx.generate()
pkbob = skbob.public_key
skalice = PrivateKx.generate()
pkalice = skalice.public_key

bob_aead = AeadServer(skbob, pkalice)

message = b"Kill all humans"

alice_aead = AeadClient(skalice, pkbob)

encrypted = bob_aead.encrypt(message)

plaintext = alice_aead.decrypt(encrypted)

new_plaintext = bob_aead.decrypt_beforetx(encrypted)

encoded = bob_aead.encode()

new_aead = AeadClient.decode(encoded)

last_plaintext = new_aead.decrypt_beforetx(encrypted)

assert (
	bob_aead.rx_key() ==
	alice_aead.tx_key() ==
	new_aead.rx_key()
)

assert (
	alice_aead.rx_key() ==
	bob_aead.tx_key() ==
	new_aead.tx_key()
)

assert (
	plaintext ==
	new_plaintext ==
	last_plaintext
) 

Will do the docs afterwards, if merged.

Let me know what you think. Or if it feels like an overkill I will close the request.

HTH.

gonatienza avatar Sep 14 '24 06:09 gonatienza

I think wrapping the newer APIs in libsodium is a good idea (we should stay up-to-date!), but I am a bit concerned about how to represent it to callers when we have a lot of partially overlapping concepts. Do you see that as solely a documentation problem?

reaperhulk avatar Sep 16 '24 18:09 reaperhulk

Hey @reaperhulk, I think pynacl users follow the docs guidance before looking into docstring and trying to figure things out by themselves for sure. I always saw the wrappers and their documentation as an excellent way in which PYCA recommends, simplifies and hardens implementation practices of the bindings.

The reason for these wrappers was simply to provide within nacl.public an xchacha20 alternative. Which I really believe more and more users will tend to go towards to by default.

Are the PrivateKx and PublicKx the classes that provide the overlapping concept feeling? There could be ways to mitigate that, but would involve touching (without affecting its current default behavior) the PrivateKey and PublicKey classes. I originally wrote the wrappers using those classes as they are today. But the fix sizes being mapped to crypto_box_* sizes (although they were the same value) did not feel it was the right way of future proofing. And touching them did not feel like the right way to go either (I tested that too).

gonatienza avatar Sep 16 '24 19:09 gonatienza

Hey @reaperhulk , just submitted some modifications for readability and to implement the new base class as an abstract base class instead. More consistent with the rest of the code in PyNacl.

gonatienza avatar Sep 19 '24 20:09 gonatienza

Hey @reaperhulk , should I kill this PR?

gonatienza avatar Sep 30 '24 00:09 gonatienza

I've been too busy to review this unfortunately. I don't think you should kill it until I've at least had a chance to look more closely. At that point I'll let you know, sorry about the delay.

reaperhulk avatar Oct 01 '24 15:10 reaperhulk