safing / jess

Jess is a cryptographic library and cli tool that focuses on usability and freedom.
GNU General Public License v3.0
42 stars 3 forks source link

Possible Poly1305 misuse #1

Closed FiloSottile closed 4 years ago

FiloSottile commented 4 years ago

Hello, I noticed this project is using x/crypto/poly1305 with a key that might be used multiple times. Poly1305 as implemented by x/crypto/poly1305 can't be used twice with the same key, or its whole security collapses. It's possible this tool allows trivial forgeries once an attacker observes two messages.

dhaavi commented 4 years ago

Hi @FiloSottile,

Thank you for pointing out this possible misuse. However, we are fully aware of that limitation.

In our case, every message uses a fresh nonce. Both the (iterating) key and nonce are fed into a KDF, which is then used to derive keys and nonces for every part of the encryption process. Thus, no key or nonce is ever used twice.

Additionally, we are currently in process of completing an audit of the cryptographic code (by @cure53 and @kaepora). Results and remediation steps will be released in the coming weeks.

Following your concern, I will think about moving internal packages to an internal path to prevent easy misuse by others.

nadimkobeissi commented 4 years ago

Hi everyone, This is indeed something that we looked out for during the audit, and we can confirm that it doesn’t appear to be an issue.

The unfortunate design decision here appears to be rather that Go’s crypto library exposes pure Poly1305, something which isn’t part of the original paper and implementation (which mixes it with AES specifically to avoid this particular type of misuse). Of course, the more common usage of Poly1305 is almost exclusively its pairing with Chacha20 to form an AEAD cipher.

Hopefully the Go crypto maintainers will fix this issue in how Poly1305 is exposed.

dhaavi commented 4 years ago

Thank you for chiming in, @kaepora.

I have added some checks to reduce the possibility of misuse in PR #2. Nevertheless, still evaluating making the package internal.

Edited to add: Depending on the future course of the project, the standalone use of Poly1305 might be removed altogether. Upcoming and already planned changes will remove its usage for now.