Skip to content
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

Remove unsupported Kyber upstreams #1915

Open
baentsch opened this issue Sep 6, 2024 · 14 comments
Open

Remove unsupported Kyber upstreams #1915

baentsch opened this issue Sep 6, 2024 · 14 comments

Comments

@baentsch
Copy link
Member

baentsch commented Sep 6, 2024

Upstreams cease(d) to support Kyber in favour of ML-KEM. OQS not following suit entails additional and constantly growing effort. See also #1914 (comment).

@baentsch
Copy link
Member Author

baentsch commented Sep 6, 2024

Arguments for removing it right away here.

@SWilson4
Copy link
Member

SWilson4 commented Sep 6, 2024

It would be a shame to remove Kyber right after completing the arduous process of importing a formally verified version. I would advocate for keeping (at least) the libjade version around until libjade updates their implementation to ML-KEM. This will not only support people who want to use the libjade implementation but also ensure that our infrastructure for pulling updates from libjade continues to work, making it easier to pull in formally verified ML-KEM whenever it's available. This also gives some level of Kyber support (although only on Mac and Linux on x86_64) for those who still want to interop. Also, correct me if I'm wrong @praveksharma, but I believe the libjade version of Kyber is also actively maintained (unlike PQClean and now seemingly pq-crystals), meaning minimal work for us to keep up with any patches.

@baentsch
Copy link
Member Author

baentsch commented Sep 6, 2024

Good and valid points, @SWilson4 (even though there is the concept of sunk costs that may play into this argument). But in general, what about --as a to-be established OQS "policy"-- removing stuff that is no longer maintained upstream? That would keep the libjade Kyber versions, but possibly not all (others), possibly limiting the platforms for which an algorithm is supported or the amount of variants.

@praveksharma
Copy link
Member

I agree with @SWilson4 regarding keeping the libjade implementation around to ensure our infrastructure needs minimal changes to pull in formally verified ML-KEM/ML-DSA/SLH-DSA etc once libjade provides those algorithms. libjade continues to support libjade-Kyber at the moment, at least until libjade-ML-KEM is ready. Although if we intend on retaining libjade-Kyber just to support Kyber, I'll add that in addition to platform limitations dropping PQClean-Kyber would also mean dropping the Kyber-1024 variant altogether - libjade only provides Kyber-512 and Kyber-768.

what about --as a to-be established OQS "policy"-- removing stuff that is no longer maintained upstream?

Might it be worth documenting a sunset period for unsupported algorithms/features as part of such a policy to avoid pulling out the rug from underneath unsuspecting users who depend on said algorithm/feature? Although this assumes a (possibly artificial) use case that can't be met by building oqsprovider against an older liboqs release as @baentsch describes here.

@dstebila
Copy link
Member

dstebila commented Sep 6, 2024

Here's a TLS mailing list post from Cloudflare about their plans (though no specific timeline): https://mailarchive.ietf.org/arch/msg/tls/hli5ogDbUudAA4tZXskVbOqeor4/

@baentsch baentsch changed the title Remove Kyber Remove unsupported Kyber upstreams Sep 7, 2024
@baentsch
Copy link
Member Author

baentsch commented Sep 7, 2024

Might it be worth documenting a sunset period for unsupported algorithms/features as part of such a policy to avoid pulling out the rug from underneath unsuspecting users who depend on said algorithm/feature?

A sunset period surely could be part of such removal policy. Could we maybe mirror what PQClean does in this regard, @dstebila ?

@dstebila
Copy link
Member

dstebila commented Sep 9, 2024

Might it be worth documenting a sunset period for unsupported algorithms/features as part of such a policy to avoid pulling out the rug from underneath unsuspecting users who depend on said algorithm/feature?

A sunset period surely could be part of such removal policy. Could we maybe mirror what PQClean does in this regard, @dstebila ?

Does PQClean have a stated sunset period / removal policy? I took a quick peruse through the repo but couldn't find one.

@baentsch
Copy link
Member Author

baentsch commented Sep 9, 2024

Does PQClean have a stated sunset period / removal policy? I took a quick peruse through the repo but couldn't find one.

Me neither -- I thought you have more insight to that project. But if there is none, how can OQS have one for code it pulls from PQClean? I guess one more aspect for the "Roadmap" discussion: How/by whom to tackle this new risk (PQClean moving at different trajectories)?

@dstebila
Copy link
Member

dstebila commented Sep 9, 2024

Does PQClean have a stated sunset period / removal policy? I took a quick peruse through the repo but couldn't find one.

Me neither -- I thought you have more insight to that project. But if there is none, how can OQS have one for code it pulls from PQClean? I guess one more aspect for the "Roadmap" discussion: How/by whom to tackle this new risk (PQClean moving at different trajectories)?

The future of PQClean is unclear to me at the moment.

If PQClean were to have a sunset process that is faster than OQS wants it to be (i.e., they remove faster than OQS does), then OQS would be in trouble in terms of relying on PQClean as an upstream.

If PQClean were to have a sunset process that is slower than OQS wants it to be (i.e., the remove slower than OQS does) or doesn't have a sunset process at all, then OQS is fine in terms of relying on PQClean as an upstream.

I think we'll end up in the latter scenario.

@baentsch
Copy link
Member Author

baentsch commented Sep 9, 2024

If PQClean [...] doesn't have a sunset process at all, then OQS is fine in terms of relying on PQClean as an upstream.

That logic I don't understand: The comment starting this issue seems to indicate that PQClean has simply sunset Kyber (round 3) (without "process") in favour of MLKEM, so creating an obligation to OQS to manually create patches to still support Kyber (round 3). I wouldn't call this "fine" for OQS: Or did I misunderstand anything here?

@cothan
Copy link
Contributor

cothan commented Sep 9, 2024

Just a side comment, if we move away from PQClean, I would like to refactor SPHINCS+/FIPS205 or mceliece (if I have time) into a central unified dir/ and build from parameterized defined flags. Reason: 50% of our liboqs LOC are SPHINCS+ and mceliece, we have many duplications.

@dstebila
Copy link
Member

dstebila commented Sep 9, 2024

If PQClean [...] doesn't have a sunset process at all, then OQS is fine in terms of relying on PQClean as an upstream.

That logic I don't understand: The comment starting this issue seems to indicate that PQClean has simply sunset Kyber (round 3) (without "process") in favour of MLKEM, so creating an obligation to OQS to manually create patches to still support Kyber (round 3). I wouldn't call this "fine" for OQS: Or did I misunderstand anything here?

Ah, I didn't remember that. Then yes, there would still be an obligation for OQS to maintain Kyber patches during the period that we keep it supported.

@baentsch
Copy link
Member Author

Ah, I didn't remember that. Then yes, there would still be an obligation for OQS to maintain Kyber patches during the period that we keep it supported.

Which brings back my question: Who takes on this obligation and for how long? In sum it means this statement of yours now holds unconditionally @dstebila , right:

OQS would be in trouble in terms of relying on PQClean as an upstream.

Together with your statement

The future of PQClean is unclear to me at the moment.

allow me to reiterate my (general) policy suggestion (maybe another TSC issue, sigh):

Remove unsupported upstreams

  • Kyber from PQClean to begin with
  • possibly with a grace period as per @praveksharma 's suggestion -- although I agree with the "possibly artificial" designation providing this -- all the while seeing this offset by a very real risk: OQS not including sensible/sensitive upstream updates due to the effort involved backporting each and every commit also applicable for an algorithm (family) no longer supported upstream.

To be done by a team anyway already pretty small for comfort: IMO we're dropping more and more balls -- and this would be another one we'd need to pick up...

Plus, this risk is not theoretical, but very real: #1914 is blocked on someone creating this patch as we can't simply run the copy_from_upstream script any more.

@dstebila
Copy link
Member

I think we can close this in light of #1989.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Todo
Development

No branches or pull requests

5 participants