-
Notifications
You must be signed in to change notification settings - Fork 478
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Pull FIPS draft versions of Kyber/Dilithium #1537
Conversation
+1 |
"merge without" would be my preference to move forward with oqsprovider & interop testing |
0c68ac4
to
cea8d5f
Compare
Regarding the aarch64 implementation, @mkannwischer is working on that and can provide an update on his path to that. As for naming, yes, I guess it makes sense to stick with the name Kyber for now and then switch to ML-KEM once the FIPS documents are finalized. @cryptojedi, any opinion? |
Douglas Stebila ***@***.***> wrote:
Regarding the aarch64 implementation, @mkannwischer is working on that
and can provide an update on his path to that.
As for naming, yes, I guess it makes sense to stick with the name
Kyber for now and then switch to ML-KEM once the FIPS documents are
finalized. @cryptojedi, any opinion?
Sounds good to me. Could also wait until the name is fixed for TLS
and/or SSH, right?
|
As well as the OIDs, yes. (Only) The reference code (with old names/proprietary OIDs) is waited for (integration) e.g., for interop testing. |
What about merging them into a(n equally named) "standard" branch in |
Sounds good to me @baentsch. For the full PR there are two open points:
|
Alright then -- can you re-target this PR to a different branch ("standard") then? Or just create a separate PR (or branch) for that (that we then maintain until this PR closes for good with the upstream updates merged)? |
See #1543. I think the PR here could stay open until completed. |
Thanks Basil for getting this ready so quickly. I guess this means we should re-question whether to include this in 0.9.0 or save for 0.10.0. I think it would be nice to get upstream approval of the fixes in the pq-crystals repository, but hopefully that's straightforward. So that would leave us in the position of deciding whether to push this 0.10.0 due the lack of aarch64 optimized implementation. Thoughts? Tagging @cryptojedi and @mkannwischer for input as well. |
In the interest to have release "0.9.0" support all "final" algorithms (and use that in interop testing with other implementations scheduled for end September) I would very much support including this in 0.9.0 (assuming we do this before end September) -- even with the consequence of dropping What worries me more in this PR though, is that the upstreams do not seem to have a notion of version. This has been updated only locally in #1543: I'd think this either needs to be added also to this PR (with |
@SWilson4 Please discuss in the next team meeting whether to heed the above (include in 0.9.0 --properly versioned-- or not). |
This PR and #1543 have the same source branch, the update is also applied here. Vadim posted a message that highlights a few inconsistencies/typos in the FIPS 204 draft, compared to the pq-crystals/standard implementation. The algorithms appear to match apart from one minor difference, which can lead to different KAT and signatures in a "literal" implementation of the draft. This might be relevant for interop. I'd therefore tend to state, for example, "FIPS 204 (Draft, August 24, 2023), implemented by pq-crystals/standard" (for spec-version, if we do this in liboqs). Any thoughts? |
Do we know if NIST is expected to make this change? |
I'm personally no big fan of version strings with more letters than numbers, let alone some code exegesis. I also wouldn't refer to FIPS if there is a possibility that the FIPS standard could be implemented differently. If pqcrystals doesn't assign a version, what about simply "pqcrystals 200823"? |
I talked with @cryptojedi this afternoon, and he tells me that there are incompatibilities between the current FIPS drafts and PQCrystals standard branch for both Kyber and Dilithium. He says that they've told NIST of the changes that they think should be made in the FIPS drafts, but there's no confirmation yet whether those changes will be made. And for Dilithium, there's still a change that he thinks should be made that isn't yet in the PQCrystals standard branch. His suggestion was that it would be better not to include the Kyber and Dilithium updates in the 0.9.0 release, and instead keep them in a separate branch for now which can be tagged for those wanting to use it in the IETF PQC PKI hackathon. |
OK -- then this PR may be kept in Draft until the discussion with NIST concludes. Given there's no date for this and the PR's anyway already in need of re-base, closing it seems the prudent thing to do. Please re-open if you disagree, @bhess . #1543 just merged to separate (arguably somewhat inaccurately named) "standard" branch. Will follow up with a PR for |
Pulls standard-version of pq-crystals repo (fips203/204 drafts).
TODOs:
couldstay with kyber/dilithium until FIPS graduates from draft status)Fixes #1521