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

PQC: SLH-DSA #4291

Merged
merged 4 commits into from
Oct 15, 2024
Merged

PQC: SLH-DSA #4291

merged 4 commits into from
Oct 15, 2024

Conversation

FAlbertDev
Copy link
Collaborator

@FAlbertDev FAlbertDev commented Aug 6, 2024

PQC: SLH-DSA

Similar to PRs #3893 and #4270, this PR integrates the SLH-DSA (FIPS 205) instances into our SPHINCS+ implementation. The difference to the current SPHINCS+ round 3.1 implementation is marginal.

Module Hierarchy

I added new modules to allow users to activate only the final SLH-DSA instances. Since the logic is almost the same, no new logic is added to these modules. However, I think it's quite handy for users to allow only SLH-DSA instances via modules (for example, using a policy).

Future Work

I already prepared most of the logic necessary for the support of contexts and the pre-hashed instances. We are currently working on an improved API for the creation of signers, which will provide us with the API necessary for supporting the aforementioned features (see #4318). We do not expect this improved API available with Botan 3.6.0 so (non-empty) contexts and prehashed instances are postponed until then.

Also, currently, all SLH-DSA-related files are still named sphincsplus or sp_*. The same goes for the class and method names. I want to move this refactoring work into a follow-up PR since it would make reviewing this one very difficult.

SLH-DSA Specification Release - TODOs

  • X.509
    • Exchange IPD with SLH-DSA OIDs
    • Read through the IETF draft and validate that our X.509 implementation handles SLH-DSA keys correctly
  • Check (and possibly adopt) SLH-DSA test vectors
    • ACVP Tests: #1, #2, #3 We generated our own KATS using py-acvp-pqc since the ACVP tests are not compatible with our API.
  • Go through FIPS 205 again and see if all validations are implemented (add tests)
  • Implement PrehashSLH-DSA → PrehashSLH-DSA will be addressed in a future PR (probably for Botan 3.7.0)
  • Implement the new "context" parameter in signing/verifying → Contexts will be addressed in a future PR (probably for Botan 3.7.0)
  • Add dedicated raw key loading functions to FFI/Python (see also Generic de-/encoding of raw public keys via FFI (in particular for KEMs)? #4366)

@FAlbertDev FAlbertDev force-pushed the feature/slh-dsa branch 3 times, most recently from 00a177b to a80c9df Compare August 7, 2024 08:50
@randombit

This comment was marked as outdated.

@randombit
Copy link
Owner

https://nvlpubs.nist.gov/nistpubs/fips/nist.fips.205.pdf

@reneme
Copy link
Collaborator

reneme commented Aug 13, 2024

Appendix A.1 mentions the introduction of HashSLH-DSA that describes an addtional domain separated signing mode when providing the actual message or a hash of the message, similar to HashML-DSA. Also here, I would suggest to implement HashSLH-DSA in a future pull request.

@randombit
Copy link
Owner

To me seems fine to implement the prehashed variant later, or not at all.

@reneme reneme mentioned this pull request Aug 15, 2024
@reneme reneme changed the title SLH-DSA integration into SPHINCS+ PQC: SLH-DSA Aug 21, 2024
@FAlbertDev
Copy link
Collaborator Author

Update: I have now implemented SLH-DSA (without context and without prehash). However, I already prepared contexts and pre-hashes for the next iteration. Also, I did not rename the classes and files to not bloat this PR. A follow-up PR will handle this, which will not contain any logical changes.
Test vectors are still missing, but they are coming soon. Also, I still want to double-check the current implementation with the SLH-DSA spec, so we do not miss anything new.

@coveralls
Copy link

coveralls commented Sep 13, 2024

Coverage Status

coverage: 91.116% (-0.4%) from 91.512%
when pulling bfb9130 on Rohde-Schwarz:feature/slh-dsa
into 41619a2 on randombit:master.

@FAlbertDev FAlbertDev marked this pull request as ready for review September 13, 2024 14:33
@FAlbertDev
Copy link
Collaborator Author

Update: Test vectors and double-checks are integrated. Next week, I will look into SLH-DSA with X.509. Otherwise, this PR should be ready for review (and side-channel analysis (@aewag)) :)

reneme

This comment was marked as resolved.

@FAlbertDev
Copy link
Collaborator Author

FAlbertDev commented Sep 20, 2024

Thanks for your review, @reneme! I applied your review suggestions and sprinkled in some StrongTypes. Regarding the logic separation between SLH-DSA and SPHINCS+:
The total difference between both versions is about 15 lines of code. Since we cannot simply reflect this logic in the sp_hash hierarchy, I think it's not worth building an entirely new hierarchy(including new modules and classes).

reneme

This comment was marked as resolved.

@FAlbertDev FAlbertDev force-pushed the feature/slh-dsa branch 2 times, most recently from b3d8d33 to 16a9a13 Compare September 20, 2024 13:16
@FAlbertDev FAlbertDev force-pushed the feature/slh-dsa branch 2 times, most recently from 11ce0c7 to e907eb7 Compare September 20, 2024 13:43
@reneme
Copy link
Collaborator

reneme commented Oct 14, 2024

Rebased after #4367 caused conflicts.

src/lib/ffi/ffi_pkey_algs.cpp Outdated Show resolved Hide resolved
@randombit
Copy link
Owner

IMO fine to ship this in 3.6.0 but we can wait for 3.7.0 if you want context support, prehashing etc which is blocked on #4318

@reneme reneme modified the milestones: Botan 3.7.0, Botan 3.6.0 Oct 15, 2024
FAlbertDev and others added 4 commits October 15, 2024 14:40
This commit applies the changes from SPHINCS+ Round 3.1 to SLH-DSA
(FIPS 205). The documentation is updated accordingly.
@reneme reneme merged commit 7d6c239 into randombit:master Oct 15, 2024
38 checks passed
@reneme reneme deleted the feature/slh-dsa branch October 15, 2024 13:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants