Add MAYO signature scheme from NIST onramp #1707

bhess · 2024-02-26T19:01:44Z

Adds MAYO signature scheme from the NIST onramp.
The upstream implementation contains a C and an AVX2 implementation.

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.)
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.)

bhess · 2024-05-13T11:49:26Z

I'll also do a branch in oqs-provider to do the downstream test, other than this the PR is ready.

baentsch · 2024-05-13T12:59:15Z

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).

bhess · 2024-05-14T09:06:54Z

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.

baentsch · 2024-05-15T07:25:37Z

apparently because of #1789 (comment).

Tagging @ryjones

SWilson4

Thanks for this work, @bhess! I haven't reviewed the upstream source, and I don't have the expertise to review the non-"pure" C AES code, but everything else looks OK to me, barring a couple of minor comments.

src/common/aes/aes128_armv8.c

src/common/aes/aes_local.h

src/common/aes/aes_ossl.c

docs/algorithms/sig/mayo.yml

src/sig/mayo/sig_mayo_1.c

src/sig/mayo/sig_mayo_2.c

src/sig/mayo/sig_mayo_3.c

baentsch · 2024-05-17T04:52:00Z

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".

bhess · 2024-05-17T12:02:50Z

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.

cothan · 2024-05-20T01:58:50Z

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?

bhess · 2024-05-21T08:06:06Z

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)?

cothan · 2024-05-21T08:45:19Z

Thanks for the answer @bhess . Well, it's from the copy_from_upstream then we don't need to change.

baentsch · 2024-05-21T09:03:49Z

Thanks for the answer @bhess . Well, it's from the copy_from_upstream then 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.

SWilson4 · 2024-05-23T18:37:10Z

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.

bhess · 2024-05-30T11:51:17Z

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 #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.

Signed-off-by: Basil Hess <[email protected]>

SWilson4

Thanks for this work, @bhess! All of my comments are resolved and the oqs-provider integration tests are passing.

bhess · 2024-07-11T06:49:15Z

Thanks for this work, @bhess! All of my comments are resolved and the oqs-provider integration tests are passing.

Thanks for the review @SWilson4 !

baentsch

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 liboqs algs); 3) underscores require more backquotes (even in this PR); 4) in oqsprovider this breaks the convention that underscores separate classic from PQC algs... Or is the underscore an important feature for some reason?

bhess · 2024-07-11T13:52:02Z

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 liboqs algs); 3) underscores require more backquotes (even in this PR); 4) in oqsprovider this 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 open-quantum-safe/oqs-provider#413.

Signed-off-by: Basil Hess <[email protected]>

dstebila · 2024-07-11T18:01:00Z

src/common/aes/aes_impl.c

+	    oqs_aes128_ctr_enc_sch_armv8(iv, iv_len, schedule, out, out_len)
+	);
+}
+


This isn't something to change in this PR, but something I noticed just from looking at this code during review. Now that we have the ability to set callbacks, could we get rid of the C_OR_NI_OR_ARM macros and instead set the callbacks once (either during library init, or alternatively on first use if it's not set)? Again: not something to do in this PR, but if anyone else thinks it might be good, we could create a separate issue to track.

bhess · 2024-07-11T18:02:34Z

I'll update this PR and open-quantum-safe/oqs-provider#413.

Done

SWilson4 · 2024-07-11T18:25:29Z

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.h file.

Signed-off-by: Basil Hess <[email protected]>

bhess · 2024-07-11T19:16:30Z

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.h file.

Thanks for the hint @SWilson4, resolved the conflict accordingly.

docs/algorithms/sig/mayo.md

baentsch · 2024-07-12T06:59:06Z

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/Add MAYO oqs-provider#413 does not contain Mayo-5, right? Sensible to add and run CI?

baentsch

As promised, review done: LGTM with two (probably) nits (see separate comments). Thanks for the work here and in oqs-provider, @bhess!

bhess · 2024-07-12T20:23:48Z

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.

Signed-off-by: Basil Hess <[email protected]>

bhess · 2024-07-13T19:34:17Z

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

bhess force-pushed the bhe-nibbling-mayo branch 2 times, most recently from 3f239ff to d9793e5 Compare March 4, 2024 13:48

dstebila added this to the 0.11.0 milestone Apr 16, 2024

bhess force-pushed the bhe-nibbling-mayo branch 6 times, most recently from bde1c5c to b27cffd Compare May 12, 2024 08:16

bhess marked this pull request as ready for review May 13, 2024 05:52

bhess requested review from dstebila and baentsch as code owners May 13, 2024 05:52

bhess mentioned this pull request May 14, 2024

Add MAYO open-quantum-safe/oqs-provider#413

Merged

dstebila mentioned this pull request May 16, 2024

WASM compatibillity #1199

Open

SWilson4 reviewed May 16, 2024

View reviewed changes

baentsch mentioned this pull request May 21, 2024

OQS goal: non-committal research or production use? open-quantum-safe/tsc#1

Open

bhess force-pushed the bhe-nibbling-mayo branch from ab63e1e to e68b556 Compare May 28, 2024 16:09

planetf1 mentioned this pull request May 31, 2024

CI: macOS build failures #1804

Open

bhess added 2 commits July 8, 2024 14:27

Add MAYO_5 to NIST_SIG_ONRAMP algorithms

9d2bc83

Signed-off-by: Basil Hess <[email protected]>

Add --formula tag to fix brew errors

63b0fc8

Signed-off-by: Basil Hess <[email protected]>

bhess force-pushed the bhe-nibbling-mayo branch from 9725725 to 63b0fc8 Compare July 8, 2024 13:32

remove PQC_AES128_STATESIZE macro [trigger downstream]

ca5d956

Signed-off-by: Basil Hess <[email protected]>

SWilson4 approved these changes Jul 10, 2024

View reviewed changes

baentsch reviewed Jul 11, 2024

View reviewed changes

MAYO naming convention update: - instead of _ [trigger downstream]

b2260b6

Signed-off-by: Basil Hess <[email protected]>

bhess force-pushed the bhe-nibbling-mayo branch from 86efabc to b2260b6 Compare July 11, 2024 14:49

dstebila reviewed Jul 11, 2024

View reviewed changes

dstebila approved these changes Jul 11, 2024

View reviewed changes

bhess added 2 commits July 11, 2024 20:52

Merge branch 'main' into bhe-nibbling-mayo

2223225

update after merge [trigger downstream]

c12c293

Signed-off-by: Basil Hess <[email protected]>

bhess force-pushed the bhe-nibbling-mayo branch from c9ab2e9 to c12c293 Compare July 11, 2024 19:03

baentsch reviewed Jul 12, 2024

View reviewed changes

docs/algorithms/sig/mayo.md Outdated Show resolved Hide resolved

baentsch approved these changes Jul 12, 2024

View reviewed changes

SWilson4 mentioned this pull request Jul 12, 2024

Use OQS_*_set_callbacks instead of C_OR_NI_OR_ARM macros #1842

Open

presistent spec version (trigger again) [trigger downstream]

2dff07f

Signed-off-by: Basil Hess <[email protected]>

bhess force-pushed the bhe-nibbling-mayo branch from baefc1d to 2dff07f Compare July 13, 2024 12:02

Add missing runner to ubuntu.yml

ef7fde5

Signed-off-by: Basil Hess <[email protected]>

bhess merged commit 4cc8884 into main Jul 13, 2024
117 checks passed

bhess deleted the bhe-nibbling-mayo branch July 15, 2024 06:55

BrewTestBot mentioned this pull request Sep 27, 2024

liboqs 0.11.0 Homebrew/homebrew-core#192129

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add MAYO signature scheme from NIST onramp #1707

Add MAYO signature scheme from NIST onramp #1707

bhess commented Feb 26, 2024 •

edited

Loading

bhess commented May 13, 2024

baentsch commented May 13, 2024

bhess commented May 14, 2024

baentsch commented May 15, 2024

SWilson4 left a comment

baentsch commented May 17, 2024

bhess commented May 17, 2024

cothan commented May 20, 2024

bhess commented May 21, 2024

cothan commented May 21, 2024

baentsch commented May 21, 2024

SWilson4 commented May 23, 2024

bhess commented May 30, 2024

SWilson4 left a comment

bhess commented Jul 11, 2024

baentsch left a comment

bhess commented Jul 11, 2024

dstebila Jul 11, 2024

bhess commented Jul 11, 2024

SWilson4 commented Jul 11, 2024

bhess commented Jul 11, 2024

baentsch commented Jul 12, 2024

baentsch left a comment

bhess commented Jul 12, 2024

bhess commented Jul 13, 2024

Add MAYO signature scheme from NIST onramp #1707

Add MAYO signature scheme from NIST onramp #1707

Conversation

bhess commented Feb 26, 2024 • edited Loading

bhess commented May 13, 2024

baentsch commented May 13, 2024

bhess commented May 14, 2024

baentsch commented May 15, 2024

SWilson4 left a comment

Choose a reason for hiding this comment

baentsch commented May 17, 2024

bhess commented May 17, 2024

cothan commented May 20, 2024

bhess commented May 21, 2024

cothan commented May 21, 2024

baentsch commented May 21, 2024

SWilson4 commented May 23, 2024

bhess commented May 30, 2024

SWilson4 left a comment

Choose a reason for hiding this comment

bhess commented Jul 11, 2024

baentsch left a comment

Choose a reason for hiding this comment

bhess commented Jul 11, 2024

dstebila Jul 11, 2024

Choose a reason for hiding this comment

bhess commented Jul 11, 2024

SWilson4 commented Jul 11, 2024

bhess commented Jul 11, 2024

baentsch commented Jul 12, 2024

baentsch left a comment

Choose a reason for hiding this comment

bhess commented Jul 12, 2024

bhess commented Jul 13, 2024

bhess commented Feb 26, 2024 •

edited

Loading