-
Notifications
You must be signed in to change notification settings - Fork 5.1k
ML-DSA Sign/VerifyPreHash #117414
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
ML-DSA Sign/VerifyPreHash #117414
Conversation
Tagging subscribers to this area: @dotnet/area-system-security, @bartonjs, @vcsjones |
...aries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/MLDsa/MLDsaTests.cs
Outdated
Show resolved
Hide resolved
src/libraries/Common/tests/System/Security/Cryptography/CryptoUtils.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds “pre-hash” signing and verification functionality for both ML-DSA and SLH-DSA by extracting the common raw sign/verify logic into shared native functions, updating the PAL layers and managed bindings, and expanding the test suite.
- Introduce
pal_evp_pkey_raw_signverify
for generic EVP_PKEY message pre-encoding support. - Update SLH-DSA and ML-DSA PAL implementations to delegate their pure and pre-encoded operations to the new raw functions.
- Extend managed APIs and interop (Helpers.cs, SlhDsaOpenSsl/OpenSsl, MLDsaOpenSsl/OpenSsl, and related implementations) to expose
SignPreHash
andVerifyPreHash
. - Add
HashInfo
utility for tests and enhance existing test suites to cover the new pre-hash methods.
Reviewed Changes
Copilot reviewed 33 out of 34 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
src/native/libs/System.Security.Cryptography.Native/pal_evp_pkey_raw_signverify.[ch] | New generic native functions for “pure” and pre-encoded signing. |
src/native/libs/System.Security.Cryptography.Native/pal_evp_pkey_slh_dsa.[ch] | Delegate SLH-DSA to the shared raw sign/verify implementation. |
src/native/libs/System.Security.Cryptography.Native/pal_evp_pkey_ml_dsa.[ch] | Delegate ML-DSA pre-encoded operations to the raw implementation. |
src/native/libs/System.Security.Cryptography.Native/entrypoints.c | Register new MLDsa pre-hash entrypoints. |
src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/Helpers.cs | Centralize hash-OID length validation and pre-hash helper logic. |
src/libraries/System.Security.Cryptography/src/.../SlhDsaOpenSsl.OpenSsl.cs | Hook up the pre-hash callbacks for SLH-DSA. |
src/libraries/System.Security.Cryptography/src/.../MLDsaOpenSsl.OpenSsl.cs | Hook up the pre-hash callbacks for ML-DSA. |
src/libraries/Common/tests/.../HashInfo.cs | Introduce HashInfo helper and update tests for pre-hash flows. |
Comments suppressed due to low confidence (2)
src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/SlhDsaOpenSsl.OpenSsl.cs:89
- [nitpick] Add a
/// <inheritdoc />
comment above this override (and similarly aboveVerifyPreHashCore
) to maintain consistent XML documentation style.
protected override void SignPreHashCore(ReadOnlySpan<byte> hash, ReadOnlySpan<byte> context, string hashAlgorithmOid, Span<byte> destination) =>
src/native/libs/System.Security.Cryptography.Native/pal_evp_pkey_slh_dsa.c:74
- The SLH-DSA pre-encoded functions (CryptoNative_SlhDsaSignPreEncoded and CryptoNative_SlhDsaVerifyPreEncoded) are no longer referenced by managed code and can be removed to avoid dead code.
int32_t CryptoNative_SlhDsaSignPreEncoded(EVP_PKEY *pkey,
src/native/libs/System.Security.Cryptography.Native/pal_evp_pkey_raw_signverify.h
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
review
src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/Helpers.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/Helpers.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Security.Cryptography.Pkcs/src/Resources/Strings.resx
Outdated
Show resolved
Hide resolved
...tests/System/Security/Cryptography/AlgorithmImplementations/MLDsa/MLDsaTestImplementation.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about Windows? I believe it should be working there already...
src/libraries/Common/src/System/Security/Cryptography/Helpers.cs
Outdated
Show resolved
Hide resolved
src/libraries/Common/src/System/Security/Cryptography/Helpers.cs
Outdated
Show resolved
Hide resolved
In particular, I believe it's "just set |
The failures don't seem to be related to my changes - merging |
Contributes to: #113502
Adds pre-hash implementation.