-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Require parameters in ECPrivateKey for Composite ML-DSA #120601
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
Require parameters in ECPrivateKey for Composite ML-DSA #120601
Conversation
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 updates the Composite ML-DSA implementation to comply with Draft 12 of the specification, which now requires parameters to be present in ECPrivateKey structures. The key change is that ECPrivateKey objects must now include curve parameters (OID) for composite ML-DSA algorithms.
- Updates ECPrivateKey generation to include required curve parameters
- Modifies validation logic to enforce parameter presence and correctness
- Updates size calculations to account for the additional parameter data
Reviewed Changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| CompositeMLDsaManaged.cs | Updates draft specification references from version 08 to 12 |
| CompositeMLDsaManaged.ECDsa.cs | Adds parameter validation and generation for ECPrivateKey structures |
| CompositeMLDsaAlgorithm.cs | Updates size calculations to include parameter overhead |
| CompositeMLDsaTestHelpers.cs | Updates expected key size calculations with specific values per algorithm |
| CompositeMLDsaFactoryTests.cs | Updates test cases to reflect new parameter requirements and validation |
src/libraries/Common/src/System/Security/Cryptography/CompositeMLDsaAlgorithm.cs
Show resolved
Hide resolved
...m/Security/Cryptography/AlgorithmImplementations/CompositeMLDsa/CompositeMLDsaTestHelpers.cs
Show resolved
Hide resolved
jeffhandley
left a comment
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.
src/libraries/Common/src/System/Security/Cryptography/CompositeMLDsaManaged.ECDsa.cs
Outdated
Show resolved
Hide resolved
.../Security/Cryptography/AlgorithmImplementations/CompositeMLDsa/CompositeMLDsaFactoryTests.cs
Show resolved
Hide resolved
src/libraries/Common/src/System/Security/Cryptography/CompositeMLDsaManaged.ECDsa.cs
Show resolved
Hide resolved
…eMLDsaManaged.ECDsa.cs Co-authored-by: Kevin Jones <vcsjones@github.com>
|
No APIs changed but, following our past convention, implementing an updated PQC draft requires a breaking change doc. Assuming we get this in for .NET 10, we can just modify dotnet/docs#48901. |
|
Added When you commit this breaking change:
Tagging @dotnet/compat for awareness of the breaking change. |
|
/ba-g #103347 |
|
/backport to release/10.0 |
|
Started backporting to |
|
@PranavSenthilnathan backporting to git am output$ git am --3way --empty=keep --ignore-whitespace --keep-non-patch changes.patch
Applying: Require parameters in ECPrivateKey for Composite ML-DSA
Applying: draft 12
Applying: Add comments.
Applying: Update src/libraries/Common/src/System/Security/Cryptography/CompositeMLDsaManaged.ECDsa.cs
Applying: Fix null-conditional operator usage
Applying: Add test cases for implicit and explicit curves
error: sha1 information is lacking or useless (src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/CompositeMLDsa/CompositeMLDsaFactoryTests.cs).
error: could not build fake ancestor
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Patch failed at 0006 Add test cases for implicit and explicit curves
Error: The process '/usr/bin/git' failed with exit code 128 |
[Draft 12](https://datatracker.ietf.org/doc/html/draft-ietf-lamps-pq-composite-sigs-12) of the Composite ML-DSA spec now requires the parameters to be present for `ECPrivateKey`. This PR implements these changes in our managed Composite ML-DSA implementation. --------- Co-authored-by: Kevin Jones <vcsjones@github.com>
…0961) (#121555) Backport of #120601 and #120961 to release/10.0 # Description Backports Draft 12 and Draft 13 spec changes for Composite ML-DSA. This PR combines two related updates: **Draft 12 changes (#120601):** - Mandate parameters field in ECPrivateKey (previously omitted) - `CompositeMLDsaAlgorithm.cs`: Calculate parameters field size for EC curves (P256/P384/P521/brainpool variants) - `CompositeMLDsaManaged.ECDsa.cs`: Validate parameters presence and curve match; write parameters with context-specific tag [0] - `CompositeMLDsaManaged.cs`: Update spec references from draft-08 to draft-12 - Test updates: Add validation for wrong/missing/implicit/explicit curves; update expected key sizes per spec Table 4 **Draft 13 changes (#120961):** - Update OIDs from experimental range (2.16.840.1.114027.80.9.1.*) to official IANA-assigned range (1.3.6.1.5.5.7.6.*) - `Oids.cs`: Update all Composite ML-DSA OID constants to new range - `CompositeMLDsaManaged.cs`: Add "ECDSA" to domain separation strings (e.g., "COMPSIG-MLDSA65-P256-SHA512" → "COMPSIG-MLDSA65-ECDSA-P256-SHA512") - Test data and helpers: Update to reflect new OIDs and domain strings # Customer Impact Without these fixes, Composite ML-DSA keys generated in .NET 10 would not conform to Draft 12 and Draft 13 of the IETF spec, causing interoperability failures with other implementations following the updated standards. # Regression No. This updates implementation to match spec evolution from Draft 8 to Draft 13. # Testing All 1,015 CompositeMLDsa tests pass. Added test coverage for: - Wrong curve OID rejection - Missing parameters rejection - Implicit curve parameters rejection - Explicit curve parameters rejection - Correct parameter serialization for all supported curves - New OID and domain string validation # Risk Low. Changes are confined to Composite ML-DSA implementation (preview feature). Validates existing behavior is maintained while adding required spec compliance. Breaking changes are intentional and necessary for spec conformance. <!-- START COPILOT CODING AGENT TIPS --> --- ✨ Let Copilot coding agent [set things up for you](https://github.com/dotnet/runtime/issues/new?title=✨+Set+up+Copilot+instructions&body=Configure%20instructions%20for%20this%20repository%20as%20documented%20in%20%5BBest%20practices%20for%20Copilot%20coding%20agent%20in%20your%20repository%5D%28https://gh.io/copilot-coding-agent-tips%29%2E%0A%0A%3COnboard%20this%20repo%3E&assignees=copilot) — coding agent works faster and does higher quality work when set up for your repo. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Pranav Senthilnathan <pranas@microsoft.com> Co-authored-by: Kevin Jones <vcsjones@github.com>
Draft 12 of the Composite ML-DSA spec now requires the parameters to be present for
ECPrivateKey. This PR implements these changes in our managed Composite ML-DSA implementation.