DisnakeDev / disnake

An API wrapper for Discord written in Python.
https://docs.disnake.dev
MIT License
721 stars 137 forks source link

feat(voice): add `aead_xchacha20_poly1305_rtpsize` encryption mode, remove old modes #1228

Open shiftinv opened 2 months ago

shiftinv commented 2 months ago

Summary

https://github.com/discord/discord-api-docs/commit/fe89a9c9e54a477a366fc271ad20f649dc26e4d0#diff-a1da547b9f7beb426699bee30117616d1a5bae58b74cd0751a68909db97ef69bR211-R219

Adds aead_xchacha20_poly1305_rtpsize voice encryption mode (pynacl docs), and removes the previous xsalsa20_poly1305* modes.

Note that this does not implement the other new (and "preferred") aead_aes256_gcm_rtpsize mode, since it's not exposed by PyNaCl despite being part of libsodium. I don't really want to add another dependency just for this, and it can always be implemented separately by subclassing VoiceProtocol if desired. Alternatively, we could bundle libsodium dlls, similar to libopus, and do all the ctypes interop manually. Also not ideal, though.

Checklist

shiftinv commented 2 months ago

For possibly future reference, this works for the other new encryption mode, which isn't supported by PyNaCl. Using https://cryptography.io/en/latest/hazmat/primitives/aead/#cryptography.hazmat.primitives.ciphers.aead.AESGCM, others like pycryptodome likely work just fine as well.

def _encrypt_aead_aes256_gcm_rtpsize(self, header: bytes, data) -> bytes:
    from cryptography.hazmat.primitives.ciphers.aead import AESGCM

    box = AESGCM(bytes(self.secret_key))
    # see https://doc.libsodium.org/secret-key_cryptography/aead/aes-256-gcm#notes for nonce length
    nonce, padded_nonce = self._get_nonce(12)

    return header + box.encrypt(padded_nonce, bytes(data), bytes(header)) + nonce
elenakrittik commented 2 months ago

Do you know if there's a specific reason pynacl doesn't expose that part of the API? Maybe it's just an oversight

shiftinv commented 2 months ago

Do you know if there's a specific reason pynacl doesn't expose that part of the API? Maybe it's just an oversight

As far as I can tell based on https://github.com/pyca/pynacl/issues/515#issuecomment-471287243 and a few other comments, this is intentional; pynacl doesn't want to give users a choice. The aes256-gcm implementation in libsodium relies on AES-NI support in the CPU (which is a reasonable requirement for any CPU released in the last decade, but still); their docs also recommend against using that encryption mode, though in this case that's not our call to make, but Discord's. cryptography uses openssl bindings instead, which uses AES-NI if available but falls back to a software implementation if not.

It's unfortunate that pynacl doesn't at least supply low-level bindings (without a high-level primitive like nacl.secret.SecretBox/Aead), which would be fine to use here.