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.

Sign up for GitHub

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

Jump to bottom

HSS/LMS Crypto Documentation #140

Merged
merged 1 commit into from
Jun 13, 2024
Merged

HSS/LMS Crypto Documentation #140

merged 1 commit into from
Jun 13, 2024

Conversation

FAlbertDev
Copy link
Collaborator

@FAlbertDev FAlbertDev commented Oct 6, 2023 •
edited

This is the documentation for Botan's HSS/LMS implementation according to RFC 8554. Botan's PR for HSS/LMS has yet to be thoroughly reviewed so some changes may occur even in this document.

I marked this PR as a draft to notify that the documentation still needs to be internally reviewed. Also, since Botan's PR still needs to be merged, the CI for this PR will fail due to the missing source links.

@FAlbertDev FAlbertDev requested a review from lieser October 6, 2023 10:57
lieser
lieser reviewed Oct 9, 2023
View reviewed changes
Copy link
Collaborator

@lieser lieser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, mostly just some smaller comments.

I also still need to look at the algorithm descriptions at the end.

docs/cryptodoc/src/90_bibliographie.rst Outdated Show resolved Hide resolved
docs/cryptodoc/src/05_06_hss_lms.rst Outdated Show resolved Hide resolved
docs/cryptodoc/src/05_06_hss_lms.rst Outdated Show resolved Hide resolved
docs/cryptodoc/src/05_06_hss_lms.rst Outdated Show resolved Hide resolved
docs/cryptodoc/src/05_06_hss_lms.rst Show resolved Hide resolved
docs/cryptodoc/src/05_06_hss_lms.rst Outdated Show resolved Hide resolved
docs/cryptodoc/src/05_06_hss_lms.rst Outdated Show resolved Hide resolved
docs/cryptodoc/src/05_06_hss_lms.rst Outdated Show resolved Hide resolved
docs/cryptodoc/src/05_06_hss_lms.rst Outdated Show resolved Hide resolved
docs/cryptodoc/src/05_06_hss_lms.rst Show resolved Hide resolved
lieser
lieser reviewed Oct 10, 2023
View reviewed changes
Copy link
Collaborator

@lieser lieser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Besides my faulty commend about SP800-208 requiring the usage of Appendix A LGTM now.

@FAlbertDev FAlbertDev marked this pull request as ready for review October 17, 2023 06:36
@lieser lieser added this to the Botan 3.3.0 milestone Oct 24, 2023
@reneme reneme modified the milestones: Botan 3.3.0, Botan 3.4.0 Jan 8, 2024
fluppe2
fluppe2 requested changes Jan 19, 2024
View reviewed changes
Copy link
Collaborator

@fluppe2 fluppe2 left a comment •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left suggestions and remarks that should be addressed.
Great text.

@FAlbertDev
Copy link
Collaborator Author

For internal review, @atreiber94, could you give the changes in ba720ba a quick scan?

atreiber94
atreiber94 approved these changes Jan 22, 2024
View reviewed changes
Copy link
Collaborator

@atreiber94 atreiber94 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, though I only checked language. Left a few minor suggestions.

docs/cryptodoc/src/05_06_hss_lms.rst Outdated Show resolved Hide resolved
docs/cryptodoc/src/05_06_hss_lms.rst Outdated Show resolved Hide resolved
docs/cryptodoc/src/05_06_hss_lms.rst Outdated Show resolved Hide resolved
docs/cryptodoc/src/05_06_hss_lms.rst Outdated Show resolved Hide resolved
@FAlbertDev
Copy link
Collaborator Author

Thanks for your review, @fluppe2, and thanks for reviewing my changes, @atreiber94. All suggestions are applied in 21f5a9a.

@FAlbertDev FAlbertDev mentioned this pull request Feb 14, 2024
Open
2 tasks
@reneme reneme force-pushed the cryptodoc/hss_lms branch 2 times, most recently from 7a4b470 to c433b5e Compare February 22, 2024 15:47
@reneme
Copy link
Collaborator

reneme commented Feb 22, 2024

Rebased to resolve pending merge conflicts.

sehlen-bsi
sehlen-bsi approved these changes Mar 5, 2024
View reviewed changes
Copy link
Owner

@sehlen-bsi sehlen-bsi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some comments on the documentation.
Overall the text is quite good.

docs/cryptodoc/src/05_06_hss_lms.rst Outdated Show resolved Hide resolved
docs/cryptodoc/src/05_06_hss_lms.rst Outdated
-------------------

The Hierarchical Signature System (HSS) with Leighton-Micali
Hash-Based Signatures (HSS/LMS) consists of three building blocks.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are the three building blocks?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Meant are OTS, LMS, and HSS. The term building blocks may be misleading. I'll go with multiple components.

docs/cryptodoc/src/05_06_hss_lms.rst Outdated
of a Merkle tree. This composition is the basis of the Leighton-Micali Signature (LMS)
method. The root node of the LMS Merkle tree defines its public key. [RFC8554]_
also provides HSS, a hypertree composition of multiple LMS trees, where the leaves
of LMS trees sign the public keys of other LMS trees.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Terminology: sometimes its called an LMS tree and sometimes an LMS Merkle tree. Strictly speaking, no definition of a LMS Merkle tree has been given and maybe its sufficient to call it simply a Merkle tree throughout?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! I agree that this is inconsistent. However, I prefer the term LMS tree, which is also frequently used within RFC 8554. I'll introduce the term better and make it more consistent, though.

docs/cryptodoc/src/05_06_hss_lms.rst Outdated Show resolved Hide resolved
docs/cryptodoc/src/05_06_hss_lms.rst Outdated Show resolved Hide resolved
docs/cryptodoc/src/05_06_hss_lms.rst Outdated Show resolved Hide resolved
docs/cryptodoc/src/05_06_hss_lms.rst Outdated Show resolved Hide resolved
docs/cryptodoc/src/05_06_hss_lms.rst Outdated Show resolved Hide resolved
docs/cryptodoc/src/05_06_hss_lms.rst Outdated Show resolved Hide resolved
docs/cryptodoc/src/05_06_hss_lms.rst Outdated Show resolved Hide resolved
@FAlbertDev
Copy link
Collaborator Author

Rebased to main

@FAlbertDev FAlbertDev force-pushed the cryptodoc/hss_lms branch 2 times, most recently from 02e1fac to 8cee0fa Compare May 28, 2024 12:18
@FAlbertDev FAlbertDev modified the milestones: Botan 3.4.0, Botan 3.5.0 May 28, 2024
@reneme
Copy link
Collaborator

reneme commented Jun 13, 2024

Rebased to latest main, in the hope to fix the build. See #216.

@FAlbertDev FAlbertDev merged commit 56276e8 into main Jun 13, 2024
25 checks passed
@FAlbertDev FAlbertDev deleted the cryptodoc/hss_lms branch June 13, 2024 09:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lieser lieser lieser left review comments

@fluppe2 fluppe2 fluppe2 approved these changes

@atreiber94 atreiber94 atreiber94 approved these changes

@sehlen-bsi sehlen-bsi sehlen-bsi approved these changes

Assignees
No one assigned
Labels
cryptodoc
Projects
None yet
Milestone
Botan 3.5.0
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@FAlbertDev @reneme @lieser @fluppe2 @atreiber94 @sehlen-bsi