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

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

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)

Comment on lines 32 to 44
for(auto& idx : indices) {
for(uint32_t i = 0; i < params.a(); ++i, ++offset) {
idx ^= (((message[offset >> 3] >> (offset & 0x7)) & 0x1) << i);
if(params.is_slh_dsa()) {
idx ^= (((message[offset >> 3] >> (~offset & 0x7)) & 0x1) << (params.a() - 1 - i));
} else {
idx ^= (((message[offset >> 3] >> (offset & 0x7)) & 0x1) << i);
}
}
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is the only logic that differs between SPHINCS+ Round 3.1 and SLH-DSA IPD .

@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 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.254% (-0.03%) from 91.279%
when pulling e907eb7 on Rohde-Schwarz:feature/slh-dsa
into 0639e82 on randombit:master.

FAlbertDev and others added 3 commits September 13, 2024 16:16
This commit applies the changes from SPHINCS+ Round 3.1 to SLH-DSA.
The documentation is updated accordingly.
@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.

@reneme reneme mentioned this pull request Sep 20, 2024
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