liboqs
liboqs copied to clipboard
Add MAYO signature scheme from NIST onramp
Adds MAYO signature scheme from the NIST onramp. The upstream implementation contains a C and an AVX2 implementation.
- [x] C code import
- [x] AVX2 code import
- [x] docs update
- [x] constant-time tests
- [x] adds aes128ctr common code
- [x] Merge some changes upstream
- [x] tbc: add cmake switch to compile onramp algorithms:
NIST_SIG_ONRAMP - [x] fix a few failing builds
- [ ] Does this PR change the input/output behaviour of a cryptographic algorithm (i.e., does it change known answer test values)? (If so, a version bump will be required from x.y.z to x.(y+1).0.)
- [x] Does this PR change the list of algorithms available -- either adding, removing, or renaming? Does this PR otherwise change an API? (If so, PRs in fully supported downstream projects dependent on these, i.e., oqs-provider and OQS-OpenSSH will also need to be ready for review and merge by the time this is merged.)
I'll also do a branch in oqs-provider to do the downstream test, other than this the PR is ready.
I'll also do a branch in oqs-provider to do the downstream test, other than this the PR is ready.
That would be good. Please remember to use a "-tracker" branch name such as to trigger it from liboqs (this PR, best) prior to the merge (as per the wiki).
I'll also do a branch in oqs-provider to do the downstream test, other than this the PR is ready.
That would be good. Please remember to use a "-tracker" branch name such as to trigger it from
liboqs(this PR, best) prior to the merge (as per the wiki).
Added the -tracker branch in oqs-provider. Everything works when running locally. The [trigger downstream] test still fails, apparently because of access rights.
apparently because of https://github.com/open-quantum-safe/liboqs/issues/1789#issuecomment-2109640801.
Tagging @ryjones
Added the -tracker branch in oqs-provider. Everything works when running locally.
It would be good if PR could confirm this: OQS either uses and relies on CI (after getting it right) or it skips/does away with it and merges code based on local run results. The latter probably wouldn't be a step in a direction of a project aiming for "more productive use".
Thank you @SWilson4 for the review! I'll update the PR per the discussion.
It would be good if PR could confirm this: OQS either uses and relies on CI (after getting it right) or it skips/does away with it and merges code based on local run results. The latter probably wouldn't be a step in a direction of a project aiming for "more productive use".
Agree with the former and ok to hold on for green CI.
Hi,
I come across this PR and I have a few questions:
- Can we merge the MAYO_*_opt into a single folder, so it's easier to review?
- Can we merge the MAYO_*_avx2 into a single folder, share source code and different by macro, so it's easier to review?
- Is there a reason to have 6 directories instead of 2?
I come across this PR and I have a few questions:
- Can we merge the MAYO_*_opt into a single folder, so it's easier to review?
- Can we merge the MAYO_*_avx2 into a single folder, share source code and different by macro, so it's easier to review?
- Is there a reason to have 6 directories instead of 2?
Hi @cothan, the integration basically follows the "pqclean style" of having a single folder per variant/optimization. The import of the MAYO code from upstream is automated with copy_from_upstream that also follows this pattern. I agree having a single folder sharing the implementation would be nice to avoid code duplication, but I think it would need quite some refactoring of copy_from_upstream. Do you want to open this as a separate issue (and tackle this separately)?
Thanks for the answer @bhess . Well, it's from the copy_from_upstream then we don't need to change.
Thanks for the answer @bhess . Well, it's from the
copy_from_upstreamthen we don't need to change.
Well, the proposal by @bhess makes sense: liboqs is way too large/wasteful for practical deployment due to this limitation. It would be a good thing for the OQS team to consider such enhancement if it wanted to move the project to real world deployment (and limit the exposure to flaws in code by simply limiting the amount of code). For further experimentation-only use, I agree with @cothan though: Leave as-is.
It would be good if PR could confirm this: OQS either uses and relies on CI (after getting it right) or it skips/does away with it and merges code based on local run results. The latter probably wouldn't be a step in a direction of a project aiming for "more productive use".
Agree with the former and ok to hold on for green CI.
CI is working again.
The PR is updated and now includes MAYO_5, handling it the same way as the other schemes with large stack usage. Not sure yet about the two failing macOS builds using gcc-13. PR https://github.com/open-quantum-safe/liboqs/pull/1708 seems to fail for the same reason, so I assume it's independent from the changes here. The difference I see from a previous working build is the update of gcc from 13.2.0 -> 13.3.0. I have the same config with gcc 13.3.0 on my local Mac which is working fine, it could be a hickup in the github runner image.
Thanks for this work, @bhess! All of my comments are resolved and the oqs-provider integration tests are passing.
Thanks for the review @SWilson4 !
Thanks for this integration, @bhess. Would it be conceivable to use an algorithm naming convention a bit more in line with what we've been using so far, i.e., either no or "-" (dash) name/strength separation? Reasons: 1) Easier to type; 2) easier to remember (for users accustomed to using other
liboqsalgs); 3) underscores require more backquotes (even in this PR); 4) inoqsproviderthis breaks the convention that underscores separate classic from PQC algs... Or is the underscore an important feature for some reason?
I checked with the MAYO team and there was no objection changing the naming convention. The preference is to use "-" as a separator for consistency with the separators used in the FIPS drafts. I'll update this PR and https://github.com/open-quantum-safe/oqs-provider/pull/413.
Sorry for the merge conflicts introduced by https://github.com/open-quantum-safe/liboqs/pull/1832, @bhess. I think it's a straightforward resolution: just move the AES callback structure updates to the new aes_ops.h file.
Sorry for the merge conflicts introduced by #1832, @bhess. I think it's a straightforward resolution: just move the AES callback structure updates to the new
aes_ops.hfile.
Thanks for the hint @SWilson4, resolved the conflict accordingly.
Added the -tracker branch in oqs-provider. Everything works when running locally.
Thanks for that downstream addition. Checking it, two questions:
- Could you please trigger a re-run without "skip ci" so the latest code in this PR gets tested there?
- It seems that tracker/https://github.com/open-quantum-safe/oqs-provider/pull/413 does not contain Mayo-5, right? Sensible to add and run CI?
Thanks for the review @baentsch !
- Could you please trigger a re-run without "skip ci" so the latest code in this PR gets tested there?
Done - the downstream test passes: https://github.com/open-quantum-safe/oqs-provider/actions/runs/9912829274/job/27388523412
- It seems that tracker/Add MAYO oqs-provider#413 does not contain Mayo-5, right? Sensible to add and run CI?
Correct, I'll try this tomorrow.
- It seems that tracker/Add MAYO oqs-provider#413 does not contain Mayo-5, right? Sensible to add and run CI?
Correct, I'll try this tomorrow.
Added MAYO-5 to oqs-provider and downstream tests pass: https://github.com/open-quantum-safe/oqs-provider/actions/runs/9919738195