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.

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

Cryptodoc Update for SLH-DSA #235

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Cryptodoc Update for SLH-DSA #235

wants to merge 2 commits into from

Conversation

FAlbertDev
Copy link
Collaborator

With the published SLH-DSA specification (FIPS 205) and the Botan's upcoming changes in randombit/botan#4291, our cryptodoc must be adapted accordingly.

For Reviewers

Note that the first commit contains the main changes, while the second one only changes internals without modifying any content.

atreiber94

This comment was marked as resolved.

@FAlbertDev FAlbertDev requested a review from atreiber94 October 21, 2024 10:18
atreiber94

This comment was marked as resolved.

@FAlbertDev FAlbertDev self-assigned this Oct 22, 2024
@reneme reneme added this to the Botan 3.6.0 milestone Oct 22, 2024
@reneme reneme force-pushed the cryptodoc/slh-dsa branch from fade6a9 to 1615c60 Compare October 22, 2024 13:44
@reneme

This comment was marked as outdated.

@reneme

This comment was marked as resolved.


.. _signatures/slh_dsa/address:

Address
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
Address
Addresses

Address
^^^^^^^

Botan's SLH-DSA addresses wrap the address specification of [FIPS-205]_
Copy link
Owner

Choose a reason for hiding this comment

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

I think it would be useful to refer to section 4.2 of FIPS 205.
Moreover, maybe an introductory sentence, like the first sentence in section 4.2 of FIPS 205 doesn't hurt here.

``sphincsplus_shake`` modules, enabling their selection for key creation.
As with the SLH-DSA instances, they are provided to the constructors of the
SLH-DSA keys.
These instances are maintained solely for version compatibility. It is strongly
Copy link
Owner

Choose a reason for hiding this comment

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

What does "version compatibility" mean? Maybe you want to say "backwards compatibility" here?

------------------

**Remark:** Signature creation with non-empty contexts is currently not
supported in Botan. Support for the pre-hash variant of SLH-DSA is also not yet
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
supported in Botan. Support for the pre-hash variant of SLH-DSA is also not yet
supported in Botan. Support for the pre-hash variant (HashSLH-DSA) of SLH-DSA is also not yet

- Steps 3.3, 3.5, 3.6: ``SK.pub_seed`` is omitted as an input because the hash functions are already instantiated with a corresponding member variable.
- ``SK`` is passed to ``slh_sign_internal`` via member variables.

Signature Validation
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
Signature Validation
Signature Verification

An SLH-DSA signature is verified in the following manner, following
Algorithm 24 of [FIPS-205]_ (see :srcref:`[src/lib/pubkey/sphincsplus/sphincsplus_common]/sphincsplus.cpp:203|is_valid_signature`):

.. admonition:: SLH-DSA Signature Validation
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
.. admonition:: SLH-DSA Signature Validation
.. admonition:: SLH-DSA signature verification

Copy link
Owner

Choose a reason for hiding this comment

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

or in Camel Case if preferred but I clearly vote for "verification" instead of "validation"

Copy link
Owner

Choose a reason for hiding this comment

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

I think validation menas something different. If I'm not mistaken, this needs to be changed in other places as well, like for ML-DSA.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants