Add Stateful Signature (XMSS and LMS) #1650

ashman-p · 2024-01-04T00:49:00Z

Adds implementations of stateful signatures to liboqs for XMSS and LMS. The feature include new OQS APIs to generate key-pairs, signature generation, verification, and secret key state-management. Actual secret key storage is left up to the application.

This PR also includes an enhancement to OQS SHA2 API to allow handling updates with arbitrary length buffers.

[ No ] 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.)
[ Yes ] 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.)
OQS provider support is a separate and forthcoming feature. This is separated because of the need to manage the stateful of secret keys.

.CMake/alg_support.cmake

src/common/sha2/sha2_armv8.c

src/common/sha2/sha2_c.c

src/sig_stfl/sig_stfl.h

baentsch · 2024-01-04T07:09:14Z

src/sig_stfl/sig_stfl.h

+ *
+ * Caller is responsible for allocating sufficient memory for `public_key` based
+ * on the `length_*` members in this object or the per-scheme compile-time macros
+ * `OQS_SIG_STFL_*_length_*`. The caller is also responsible for initializing


Out of curiosity: Why is there so much responsibility put onto the user to "do things right"? Can't the library "help" (ensure there's less room for user error)?

tests/helpers.py

tests/dump_alg_info.c

baentsch

OK, I'm impressed by the amount of new functionality -- but please forgive me for being tired by now looking through this: Please see/address the single comments. One general question: I do not see any specific new CI test configs, but many new config variables: Wouldn't at least some of them be worthwhile exercising in CI? For example, is it possible to set/unset all config vars with the new algs, e.g., OQS_MINIMAL_BUILD? Does liboqs build OK with & without the new algs, e.g., OQS_ENABLE_SIG_STFL_XMSS=OFF/ON? Is memory leak testing for SIG_STFL out of scope or did I overlook the tests for that?

ducnguyen-sb · 2024-01-05T20:01:46Z

TODO: Address @baentsch comments here: #1098 (comment)

README.md

src/CMakeLists.txt

tests/CMakeLists.txt

SWilson4 · 2024-01-05T21:28:04Z

Thanks very much for this gargantuan effort @ashman-p @ducnguyen-sb!

Since this is such a big PR, I'm going to review a few files at a time and occasionally leave a batch of comments. It will probably take me at least a couple of days to get through everything thoroughly. More to come next week...

* Simplify security assumption in docs * Get rid of commented functions * Rename sm variable; use OQS_MEM_cleanse * Clean up TODOs * Update markdown files

SWilson4 · 2024-05-14T21:00:53Z

There are several unresolved comments from @baentsch and @SWilson4, will those be addressed before merging?

I had forgotten about those comments (they were hidden in the GitHub UI). They are all resolved with the latest commit.

EDIT: Should clarify that by "those comments" I meant my own; Michael spoke to his above.

cothan · 2024-05-15T02:21:49Z

The checks are all green. Comments are resolved. I can't wait for this PR to be merged!

SWilson4 · 2024-05-15T14:11:57Z

The checks are all green. Comments are resolved. I can't wait for this PR to be merged!

At this point I would approve the PR, but, as the author of a number of commits, I think that I should not be the one to push it over the "ready to merge" threshold.

baentsch

I think that I should not be the one to push it over the "ready to merge" threshold.

Well, strictly speaking, me neither given I have not taken a serious new look at it -- primarily out of fear of finding more things... and I already feel guilty of having caused a lot of work. So let me approve regardless and we then deal with any consequences later :-) I think it's called YOLO.

baentsch · 2024-04-26T04:40:24Z

.CMake/alg_support.cmake

+cmake_dependent_option(OQS_ENABLE_SIG_STFL_lms_sha256_h20_w8_h15_w8 "" ON "OQS_ENABLE_SIG_STFL_LMS" OFF)
+cmake_dependent_option(OQS_ENABLE_SIG_STFL_lms_sha256_h20_w8_h20_w8 "" ON "OQS_ENABLE_SIG_STFL_LMS" OFF)
+
+option(OQS_EXPERIMENTAL_ENABLE_SIG_STFL_KEY_SIG_GEN "Enable stateful key and signature generation for research and experimentation" OFF)


Didn't we hear suggestions to make this config option sound really dangerous and hard to accidentally enable? What about naming it "OQS_ENABLE_EXPERIMENTAL_AND_BORDERING_ON_IRRESPONSIBLE_STATEFUL_HASHBASED_SIGNATURE_AND_KEYGEN_FEATURE" (basically quoting one such feedback)?

How about OQS_HAZARDOUS_EXPERIMENTAL_ENABLE_SIG_STFL_KEY_SIG_GEN as something that sounds reasonably professional while conveying the essence of "bordering on irresponsible"?

Sounds good to me.

CONFIGURE.md

baentsch · 2024-05-15T14:54:25Z

Hmm -- DCO is not clean. But then again, I'm not sure we really care about this, do we?

SWilson4 · 2024-05-16T18:27:38Z

Hmm -- DCO is not clean. But then again, I'm not sure we really care about this, do we?

Given that this PR predates DCO being enabled, perhaps we could bypass it just this once? We would need to either have one almighty sign-off (which seems wrong, given that the PR has four different authors) or have the authors sign off on their own commits individually (which seems like an inordinate amount of busywork).

SWilson4 · 2024-05-29T01:38:29Z

As discussed in today's OQS meeting, this is the last call for reviews of this PR. It will be merged at the end of the week.

ryjones · 2024-05-30T15:17:54Z

I've provided instructions on how to fix DCO to @dstebila. It will involve 1 commit each from four people.

ryjones · 2024-05-30T15:41:11Z

Please see this topic

I, Douglas Stebila, retroactively sign off on these commits: commit b0c06fa Fix API and build issues commit 7b59154 Add SIG_STFL to tests/dump_alg_info commit 8e1dd5c Update sig_stfl dummy scheme and add basic test program commit c9c3835 Re-add OQS_SECRET_KEY (#1493) Signed-off-by: Douglas Stebila <[email protected]>

commit 001e96a Update GitHub Actions workflows for stateful signatures (#1692) commit 8524a16 Post-rebase cleanup commit 5da49e3 Satisfy astyle commit a535114 Fix macOS build error: lld -> llu commit 71ee535 Bring EVP_DigestUpdate calls in line with main commit 154d8e4 Fix test program linkage for cross-compiling commit e92aab3 Stateful sigs: Rename keygen / sign option, add more tests, fix memory errors (#1755) commit b075878 Clean up OQS_SIG_STFL_SECRET_KEY_free commit db000c2 Remove unused sig member commit 9b60f60 Naming convention for serialize / deserialize functions commit 7dd4ea0 Test stateful sigs on arm64, s390x, and powerpc (#1772) commit 31bdf13 Clean up unresolved comments on stateful-sigs PR (#1793) commit 8e75f98 Update config variable name commit ca27922 Strengthen warning in CONFIGURE.md Signed-off-by: Spencer Wilson <[email protected]>

commit 244288f Add XMSS parameter xmss_sha256_h10 (#1482) commit a7e26d9 Add 12 XMSS and 16 XMSSMT parameters. (#1489) commit 4694fc3 Add secret key object to XMSS (#1530) commit 99067be Add XMSS Serialize/Deserialize (#1542) commit 2dbfc40 Update XMSS secret key object APIs, sync with LMS (#1588) commit 47740ad Enforce idx from unsigned int to uint32_t. (#1611) commit 9610576 Fix windows-x86 and arm compiling error. (#1634) commit bb658b7 Address stateful-sigs comments in #1650 (#1656) commit 7db8ddf Update `sig_stfl.h` document for #1650 (#1655) commit c3e5750 Add Apache 2.0 and MIT License to XMSS (#1662) commit e1f02b2 Change XMSS License from `(Apache 2.0 AND MIT)` to `(Apache 2.0 OR MIT) AND CC0-1.0` (#1697) commit 17c12c3 Add return status for XMSS lock/unlock functions. (#1712) commit 1941636 Add return check for lock/unlock function (#1727) commit b45415c Use `abort()` instead of exit to get the trace log. (#1728) commit ba63672 Reduce number of `malloc/free` call in `XMSS/external` (#1724) Signed-off-by: Duc Tri Nguyen <[email protected]>

commit e356ebf Na lms (#1486) commit 55094c3 LMS H5_W1 (#1513) commit 4d773d7 Convert to use OQS_SIG_STFL_SECRET_KEY struct (#1525) commit 245aede LMS updated to use new SK API (#1533) commit a85a9aa Stateful sigs secret key storage callback (#1553) commit 3934949 Na statful sig lock (#1559) commit 3db6b44 Secret Key Query (#1572) commit 2446c64 Na stateful sigs lms var (#1574) commit 8df2539 Stateful sigs XMSS updates (#1590) commit a7b2987 SHA2 Increment with arbitrary length (non-block sizes) (#1614) commit 2dd9e07 Na lms kat multi level (#1620) commit 982b440 Fix Build Errors (#1635) commit ddae644 Various fixes commit cc50ef0 Fix warning commit cf03392 Update README.md commit 9325713 Update README.md commit a52b217 Update README.md commit d442ac9 Update README.md commit 72ab478 Update README.md commit 5967f12 Update src/CMakeLists.txt commit fc6d512 Update documentation and license text. (#1663) commit e7a83c7 Disable Stateful Signatures in the build by default (#1676) commit 6c81bae Na stateful macro (#1687) Signed-off-by: Norman Ashley <[email protected]>

Signed-off-by: Spencer Wilson <[email protected]>

SWilson4 · 2024-06-05T19:55:50Z

As discussed in today's OQS meeting, this is the last call for reviews of this PR. It will be merged at the end of the week.

Better late then never. CI is now passing thanks to #1805 and recent DCO fixes, so I am going ahead with the merge.

lingxinh-aws · 2024-06-07T00:52:22Z

Sorry for being late to this... But it just happened today:

"#include <oqs/sig_stfl.h>" was added in oqs.h in 971773a. It caused an error of "unknown type name 'OQS_SIG' " when compiling oqs-provider/oqsprov/oqs_sig.c, which includessig.h, which includes oqs.h. Then, OQS_SIG is referred in sig_stfl.h, but it is defined after #include <oqs/oqs.h> in the body of sig.h, so compilation failed.

I suggest that #include <oqs/sig_stfl.h> be added to the end of sig.h, similar to the treatment of other signature algorithm, i.e. enclosed in a #ifdef and #endif block.

ryjones · 2024-06-07T11:57:03Z

@lingxinh-aws please file an issue for this

ashman-p · 2024-06-07T12:46:38Z

Thanks @lingxinh-aws, as ryjones suggested please open an issue, I will take a look at it shortly, in the mean time.

lingxinh-aws · 2024-06-07T17:15:12Z

Hi, I filed the issue here: #1815 as suggested. Regards, Xinhua (Frank) Ling, PhD CISSP GCED Amazon Web Services From: Norman Ashley ***@***.***> Sent: Friday, June 7, 2024 8:47 AM To: open-quantum-safe/liboqs ***@***.***> Cc: Ling, Frank ***@***.***>; Mention ***@***.***> Subject: Re: [open-quantum-safe/liboqs] Add Stateful Signature (XMSS and LMS) (PR #1650) Thanks @lingxinh-aws<https://github.com/lingxinh-aws>, as ryjones suggested please open an issue, I will take a look at it shortly, in the mean time. — Reply to this email directly, view it on GitHub<#1650 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AWSVHCM2QTUK4YNRHYOR3VDZGGTUNAVCNFSM6AAAAABBMGCRI6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNJUG43DKOJXGM>. You are receiving this because you were mentioned.Message ID: ***@***.******@***.***>>

ashman-p requested review from dstebila, baentsch and bhess as code owners January 4, 2024 00:49

ashman-p requested review from SWilson4, dstebila, baentsch and ducnguyen-sb and removed request for bhess, dstebila and baentsch January 4, 2024 00:49