briansmith / ring

Safe, fast, small crypto using Rust
Other
3.7k stars 698 forks source link

Add Kyber #1685

Open briansmith opened 11 months ago

briansmith commented 11 months ago

Proposal:

PLMK if/when we want me to bring in the crypto/kyber/* files from BoringSSL.

@mouse07410 already commented on this proposal in another thread:

that would be great if Kyber can be added to ring. I've one question: why use a C implementation from BoringSSL instead of pure Rust implementation like https://github.com/Argyle-Software/kyber.git ?

In some sense, the plan above is basically "I'll import BoringSSL's Kyber implementation if somebody else posts a PR to integrate it into ring and add the tests, on the assumption that BoringSSL's implementation is correct." I.e. this is the fastest and requires the least effort (on my part) to do it.

A priori I prefer a Rust implementation but we'd need to find a way to evaluate its quality. My proposal above kind of punts on judging which implementation is better than another.

briansmith commented 11 months ago

BTW, in case people are interested in why I'm suggesting we add Kyber and ignoring all the other algorithms: Some people said they could help review PRs and help with other work in ring, and this is kind of horse-trading for that. Since Google and Cloudflare both are deploying Kyber, it makes sense for us to bring it in. I personally stay out of trying to pick winners myself.

briansmith commented 11 months ago

Also, I propose that public API exposed from ring only exposes the hybrid versions, and only the hybrids that are actually deployed on the web.

My understanding is that there will be changes to Kyber after the NIST standardization process. And there will be change sto the hybrid modes after the IETF process. It would be helpful for others to expand this proposal to suggest how we would migrate to the new standardized modes. My strawman is that name the old versions "_PROTOTYPE" and that when the final versions are ready, we add the final standardized versions, remove the "_PROTOTYPE" versions, and do a major version bump of ring (for SemVer reasons).

inflation commented 7 months ago

There's a new formally verified Kyber (ML-KEM) Rust implementation. The performance is good and the API looks promising.

mouse07410 commented 7 months ago

@briansmith I disagree with exposing only hybrid in public API. As a cryptographer and as a practical user.

For example, CNSA-2.0 does not define or require hybrid.

briansmith commented 7 months ago

Contrasting with what I wrote earlier, I would OK with exposing Kyber directly in a no-hybrid fashion.

In the short term, the main priority should be choosing an implementation that is computationally compatible with FIPS 203 and which we could integrate into the ring FIPS 140 module we're working on.