HSS-LMS Signature Algorithm Implementation #3716

FAlbertDev · 2023-09-27T10:13:06Z

Pull Request Dependencies

Refactor: Final iteration on load/store #3869

Description

This is adding the Hierarchical Signature System (HSS) with Leighton-Micali Hash-Based Signatures (LMS) as defined in RFC 8554.

The first commit (9dddcf6) contains some preparations, mainly common utility functions, and minor cleanup. HSS-LMS is implemented in the second commit (ba36c5e).

For this algorithm, we tried to find a way to generalize the logic to construct a Merkle tree, which is used in all hash-based signature algorithms so far. We introduce the tree_hash.h containing a generalized (and highly flexible) logic for creating Merkle trees. We only apply it for HSS-LMS in this PR but aim to apply it for (at least) SPHINCS+ in a follow-up PR.

If you are interested, you can drop some comments and suggestions.

#3105

coveralls · 2023-09-28T07:32:07Z

coverage: 91.857% (-0.2%) from 92.028%
when pulling 24c8b6c on Rohde-Schwarz:pl/lms
into bf86f1a on randombit:master.

doc/api_ref/pubkey.rst

src/lib/pubkey/hss_lms/hss.cpp

src/lib/pubkey/hss_lms/hss_lms.cpp

src/lib/pubkey/hss_lms/hss_lms_utils.h

src/lib/pubkey/hss_lms/lm_ots.cpp

src/lib/pubkey/hss_lms/lms.cpp

src/tests/test_hss_lms.cpp

FAlbertDev · 2023-09-29T14:40:04Z

This PR is now ready! Any review is welcome ❤️

reneme

No full review at all. Mainly just looked at the stuff in src/lib/utils.

src/lib/utils/concepts.h

src/lib/utils/loadstor.h

src/lib/utils/stl_util.h

src/lib/utils/tree_hash.h

reneme · 2023-10-05T08:45:31Z

src/lib/utils/tree_hash.h

+template <concepts::contiguous_strong_type TreeNode,
+          concepts::strong_span AuthPathSS,
+          concepts::tree_node_index TreeNodeIndex,
+          concepts::tree_layer_index TreeLayerIndex,
+          typename Address>
+   requires concepts::tree_address<Address, TreeLayerIndex, TreeNodeIndex>
+inline void treehash(
+   StrongSpan<TreeNode> out_root,
+   std::optional<AuthPathSS> out_auth_path,
+   std::optional<TreeNodeIndex> leaf_idx,
+   size_t node_size,
+   TreeLayerIndex total_tree_height,
+   uint32_t idx_offset,
+   concepts::tree_hash_node_pair<TreeNodeIndex, TreeLayerIndex, Address, StrongSpan<TreeNode>> auto node_pair_hash,
+   concepts::tree_gen_leaf<TreeNodeIndex, TreeLayerIndex, Address, StrongSpan<TreeNode>> auto gen_leaf,
+   Address& tree_address) {


😲

Any chance we could wrap that into a reasonable class that removes some complexity from this interface? Off the top of my head, I don't know enough to give useful suggestions. Though, on first glance, that seems like a nightmare to use and/or understand.

Can we go through the usage of this in a pair-programming session at one point please?

Yeah, I admit that this construct looked much better in my head. The goal of this template hell is that the calling party only calls this function with its own strong types while all types of treehash are deduced automatically without the need to pass any template parameters. But yeah, I would love to get your input on this in a pair-programming session!

randombit

Started a review. I haven't gotten into the core implementation yet so this is just some initial comments, I will do a full review asap.

src/build-data/oids.txt

src/lib/utils/stl_util.h

src/lib/utils/tree_hash.h

src/lib/pubkey/hss_lms/lms.h

src/lib/pubkey/hss_lms/hss_lms_utils.h

src/lib/pubkey/hss_lms/hss_lms.h

reneme · 2023-10-30T15:31:42Z

src/lib/pubkey/hss_lms/lms.cpp

+            throw Decoding_Error("Unsupported LMS algorithm type");
+      }
+      throw Decoding_Error("Unsupported LMS algorithm type");


Please use unique error messages: If you see one of those in some bug report log, you'll want to know which code path was taken.

... same below, for the hash functions.

src/lib/pubkey/hss_lms/lms.cpp

lieser · 2023-10-31T10:18:53Z

rebased to current master

reneme · 2024-01-12T08:45:31Z

I force pushed the changes here, to:

take advantage of the changes in Refactor: Final iteration on load/store #3869
squash and clean up the history

reneme · 2024-02-16T15:38:30Z

Rebased after the underlying #3869 needed to get conflicts resolved after #3908 was merged. Also, the code here needed minor conflict resolving due to #3908.

randombit · 2024-03-21T17:09:48Z

#3869 is approved; I'll review LMS after this is rebased post merge

reneme · 2024-03-21T20:25:04Z

I'll review LMS after this is rebased post merge

Done. I merged #3869 and rebased. No history rewrite was done.

randombit

Various small comments/questions but overall lgtm

src/lib/pubkey/hss_lms/hss_lms.h

randombit · 2024-03-25T08:44:32Z

src/lib/pubkey/hss_lms/lms.h

+       * @return The LMS public key.
+       * @throws Decoding_Error If parsing the public key fails.
+       */
+      static LMS_PublicKey from_bytes_or_throw(BufferSlicer& slicer);


This is an internal API so maybe doesn’t matter that much but passing a BufferSlicer as a parameter feels very odd to me. I would think you would just pass a span and then as required construct a BufferSlicer to perform your parsing.

I agree that we can pass a span here instead. This would be suitable but inconsistent with signature parsing, where passing the slicer is quite sensible (see below). I have no hard feelings for both options.

Perhaps my two cents; talking from experience in #4024: I totally agree, that any loosely coupled API shouldn't pass around BufferSlicer or BufferStuffer references. Though, for tightly coupled internal APIs (e.g. utility functions in the same compilation unit), I find this very much convenient.

Accepting a span, as @randombit suggested, has the drawback that the caller must know how many bytes the callee will want to consume or emit. Usually, that can be determined, but it typically requires some extra calculations that were already done to determine the overall size of the final serialized structure. Just passing the Buffer*** reference and trusting that the callee will be responsible, is saving some cycles and also noise in the code.

That said: We should always remember to validate that the Buffer*** is full/empty when expected at the very end of the marshalling, to check that everything checked out as expected.

src/lib/pubkey/hss_lms/lms.h

src/lib/pubkey/hss_lms/lm_ots.cpp

src/lib/pubkey/hss_lms/hss.cpp

src/lib/pubkey/hss_lms/hss_lms.cpp

src/lib/pubkey/hss_lms/lms.cpp

reneme · 2024-04-08T11:33:28Z

Rebased to fix conflicts on master (likely introduced by merging #3933).

src/lib/utils/int_utils.h

src/lib/pubkey/sphincsplus/sphincsplus_common/sp_treehash.cpp

src/lib/pubkey/hss_lms/hss.h

reneme

Just a few ad-hoc remarks. Will go through the implementation one final time tomorrow.

doc/api_ref/pubkey.rst

src/lib/pubkey/hss_lms/hss.h

src/lib/utils/concepts.h

src/lib/pubkey/hss_lms/hss.cpp

reneme

Still didn't find time for another full review pass. But no general objection to merging this from my side.

src/lib/pubkey/hss_lms/hss.cpp

src/lib/pubkey/hss_lms/lms.cpp

Some cleanup and addition of common utility functions Co-authored-by: Philippe Lieser <philippe.lieser@rohde-schwarz.com> Co-authored-by: René Meusel <rene.meusel@rohde-schwarz.com>

Implementation of the Hierarchical Signature System (HSS) with Leighton-Micali Hash-Based Signatures (LMS). Based on RFC 8554. Co-authored-by: Philippe Lieser <philippe.lieser@rohde-schwarz.com> Co-authored-by: René Meusel <rene.meusel@rohde-schwarz.com>

Integration into pk_algs, tests and documentation. Co-authored-by: Philippe Lieser <philippe.lieser@rohde-schwarz.com> Co-authored-by: René Meusel <rene.meusel@rohde-schwarz.com>

FAlbertDev force-pushed the pl/lms branch 2 times, most recently from c19b7a4 to 9516b5c Compare September 27, 2023 13:17

FAlbertDev force-pushed the pl/lms branch from e8954a1 to ba36c5e Compare September 28, 2023 14:29

lieser reviewed Sep 29, 2023

View reviewed changes

FAlbertDev force-pushed the pl/lms branch from ba36c5e to 4cc77c9 Compare September 29, 2023 13:10

FAlbertDev marked this pull request as ready for review September 29, 2023 14:38

randombit self-requested a review September 29, 2023 21:14

reneme reviewed Oct 5, 2023

View reviewed changes

randombit reviewed Oct 5, 2023

View reviewed changes

FAlbertDev mentioned this pull request Oct 6, 2023

HSS/LMS Crypto Documentation sehlen-bsi/botan-docs#140

Open

randombit added this to the Botan 3.3.0 milestone Oct 6, 2023

FAlbertDev force-pushed the pl/lms branch from 1900c9e to 20a5b9f Compare October 18, 2023 10:36

This comment was marked as outdated.

Sign in to view

reneme reviewed Oct 30, 2023

View reviewed changes

This comment was marked as outdated.

Sign in to view

lieser force-pushed the pl/lms branch from a4c5df2 to 344e6dc Compare October 31, 2023 10:18

reneme mentioned this pull request Oct 31, 2023

FIX: Non-KEX-algos may choose to not implement ::generate_another() #3786

Merged

reneme added a commit to Rohde-Schwarz/botan that referenced this pull request Nov 2, 2023

update OIDs to not clash with randombit#3716

dffc54c

reneme added a commit to Rohde-Schwarz/botan that referenced this pull request Nov 2, 2023

update OIDs to not clash with randombit#3716

76c251d

reneme mentioned this pull request Nov 21, 2023

[std::span] {load,store}_{le,be} #3707

Merged

reneme mentioned this pull request Dec 8, 2023

Extend documentation for encryption and signing parameters #3823

Merged

lieser force-pushed the pl/lms branch from 6945b3b to 6c11f31 Compare December 8, 2023 16:39

reneme mentioned this pull request Jan 11, 2024

Refactor: Final iteration on load/store #3869

Merged

reneme force-pushed the pl/lms branch from f4f6168 to 1b9c547 Compare January 12, 2024 08:42

FAlbertDev mentioned this pull request Feb 5, 2024

PQC: Classic McEliece #3883

Open

13 tasks

reneme force-pushed the pl/lms branch from 81537b9 to 1bc7e50 Compare February 16, 2024 15:37

reneme force-pushed the pl/lms branch from 1bc7e50 to 4683103 Compare March 21, 2024 20:23

randombit approved these changes Mar 25, 2024

View reviewed changes

reneme force-pushed the pl/lms branch 2 times, most recently from c299e55 to fe90321 Compare April 8, 2024 11:32

randombit modified the milestones: Botan 3.4.0, Botan 3.5.0 Apr 8, 2024

FAlbertDev force-pushed the pl/lms branch 2 times, most recently from e82658a to 5f1db48 Compare April 23, 2024 10:53

reneme reviewed Apr 23, 2024

View reviewed changes

src/lib/utils/int_utils.h Show resolved Hide resolved

src/lib/pubkey/sphincsplus/sphincsplus_common/sp_treehash.cpp Outdated Show resolved Hide resolved

src/lib/pubkey/hss_lms/hss.h Outdated Show resolved Hide resolved

FAlbertDev force-pushed the pl/lms branch 2 times, most recently from cc8845d to 10ec77a Compare April 23, 2024 11:25

reneme mentioned this pull request Apr 24, 2024

Centralize 'integralish' concept and strong-type unwrapping #4029

Open

reneme reviewed Apr 29, 2024

View reviewed changes

reneme approved these changes Apr 30, 2024

View reviewed changes

src/lib/pubkey/hss_lms/hss.cpp Outdated Show resolved Hide resolved

src/lib/pubkey/hss_lms/hss.cpp Show resolved Hide resolved

src/lib/pubkey/hss_lms/hss.cpp Outdated Show resolved Hide resolved

reneme approved these changes May 22, 2024

View reviewed changes

src/lib/pubkey/hss_lms/lms.cpp Outdated Show resolved Hide resolved

reneme mentioned this pull request May 22, 2024

Internal: load_be/le should accept a BufferSlicer #4068

Open

FAlbertDev and others added 3 commits May 23, 2024 14:41

Preparations for HSS-LMS

471671c

Some cleanup and addition of common utility functions Co-authored-by: Philippe Lieser <philippe.lieser@rohde-schwarz.com> Co-authored-by: René Meusel <rene.meusel@rohde-schwarz.com>

Implementation of HSS-LMS

eef647f

Implementation of the Hierarchical Signature System (HSS) with Leighton-Micali Hash-Based Signatures (LMS). Based on RFC 8554. Co-authored-by: Philippe Lieser <philippe.lieser@rohde-schwarz.com> Co-authored-by: René Meusel <rene.meusel@rohde-schwarz.com>

Integration of HSS-LMS

2c31a2f

Integration into pk_algs, tests and documentation. Co-authored-by: Philippe Lieser <philippe.lieser@rohde-schwarz.com> Co-authored-by: René Meusel <rene.meusel@rohde-schwarz.com>

reneme approved these changes May 23, 2024

View reviewed changes

FAlbertDev force-pushed the pl/lms branch from 24c8b6c to 2c31a2f Compare May 23, 2024 12:52

FAlbertDev merged commit 19567e3 into randombit:master May 23, 2024
43 checks passed

FAlbertDev deleted the pl/lms branch May 23, 2024 13:25

reneme mentioned this pull request May 23, 2024

Introduce Public_Key::raw_public_key_bits() #3985

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HSS-LMS Signature Algorithm Implementation #3716

HSS-LMS Signature Algorithm Implementation #3716

FAlbertDev commented Sep 27, 2023 •

edited by reneme

coveralls commented Sep 28, 2023 •

edited

FAlbertDev commented Sep 29, 2023

reneme left a comment

reneme Oct 5, 2023

FAlbertDev Oct 6, 2023

randombit left a comment

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

reneme Oct 30, 2023

reneme Oct 30, 2023

This comment was marked as outdated.

lieser commented Oct 31, 2023

reneme commented Jan 12, 2024

reneme commented Feb 16, 2024

randombit commented Mar 21, 2024

reneme commented Mar 21, 2024

randombit left a comment

randombit Mar 25, 2024

FAlbertDev Mar 25, 2024

reneme Apr 23, 2024 •

edited

reneme commented Apr 8, 2024

reneme left a comment

reneme left a comment

HSS-LMS Signature Algorithm Implementation #3716

HSS-LMS Signature Algorithm Implementation #3716

Conversation

FAlbertDev commented Sep 27, 2023 • edited by reneme

Pull Request Dependencies

Description

coveralls commented Sep 28, 2023 • edited

FAlbertDev commented Sep 29, 2023

reneme left a comment

Choose a reason for hiding this comment

reneme Oct 5, 2023

Choose a reason for hiding this comment

😲

FAlbertDev Oct 6, 2023

Choose a reason for hiding this comment

randombit left a comment

Choose a reason for hiding this comment

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

reneme Oct 30, 2023

Choose a reason for hiding this comment

reneme Oct 30, 2023

Choose a reason for hiding this comment

This comment was marked as outdated.

lieser commented Oct 31, 2023

reneme commented Jan 12, 2024

reneme commented Feb 16, 2024

randombit commented Mar 21, 2024

reneme commented Mar 21, 2024

randombit left a comment

Choose a reason for hiding this comment

randombit Mar 25, 2024

Choose a reason for hiding this comment

FAlbertDev Mar 25, 2024

Choose a reason for hiding this comment

reneme Apr 23, 2024 • edited

Choose a reason for hiding this comment

reneme commented Apr 8, 2024

reneme left a comment

Choose a reason for hiding this comment

reneme left a comment

Choose a reason for hiding this comment

FAlbertDev commented Sep 27, 2023 •

edited by reneme

coveralls commented Sep 28, 2023 •

edited

reneme Apr 23, 2024 •

edited