Skip to content

Conversation

bartonjs
Copy link
Member

...SecretKey... got renamed to ...PrivateKey... during the API review for ML-DSA.

Similar to the cleanup for SLH-DSA, this does not change anything in the Interop classes or the crypto shim, but should have everything else.

  • All remaining usages of "secret key" found by VS in System.Security.Cryptography are related to HMAC, AES-GCM, or AES-CCM.
  • All remaining usages of "secretkey" found by VS in System.Security.Cryptography are for Composite ML-DSA, one place in SLH-DSA that got missed, or the SecretKeySeed/SecretKeyPrf in SLH-DSA tests.
  • Comments above do not include src/Common/src/Interop

I did a manual check of the approved shape against the ref.cs, and it matches except for SignMu/VerifyMu (which were added in a followup review).

Fixes #117911.

@bartonjs bartonjs added this to the 10.0.0 milestone Jul 31, 2025
@bartonjs bartonjs self-assigned this Jul 31, 2025
@Copilot Copilot AI review requested due to automatic review settings July 31, 2025 20:16
Copy link
Contributor

@Copilot Copilot AI left a 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 renames "SecretKey" to "PrivateKey" throughout the ML-DSA (Module-Lattice-Based Digital Signature Algorithm) implementation to align with API review decisions. The change is primarily cosmetic, updating method names, property names, variables, and resource strings while maintaining the same underlying functionality.

Key changes include:

  • Renaming methods from ExportMLDsaSecretKey to ExportMLDsaPrivateKey and ImportMLDsaSecretKey to ImportMLDsaPrivateKey
  • Updating the SecretKeySizeInBytes property to PrivateKeySizeInBytes in MLDsaAlgorithm
  • Changing internal field names and variables from secretKey/hasSecretKey to privateKey/hasPrivateKey

Reviewed Changes

Copilot reviewed 30 out of 30 changed files in this pull request and generated no comments.

Show a summary per file
File Description
ref/System.Security.Cryptography.cs Updates public API surface with renamed methods and properties
src/System/Security/Cryptography/MLDsa*.cs Core implementation files with renamed methods and internal state
tests/**/*.cs Test files updated to use new method names and variable names
src/Resources/Strings.resx Resource strings updated from "secret key" to "private key"
Common/src/**/*.cs Shared implementation code with renamed methods and variables
Comments suppressed due to low confidence (1)

src/libraries/System.Security.Cryptography/ref/System.Security.Cryptography.cs:1850

  • The error message resource string 'Argument_PrivateKeyWrongSizeForAlgorithm' is referenced but doesn't exist in the updated resource files. The resource files show 'Argument_SecretKeyWrongSizeForAlgorithm' was removed but no corresponding 'Argument_PrivateKeyWrongSizeForAlgorithm' was added.
    {

@PranavSenthilnathan
Copy link
Member

@bartonjs
Copy link
Member Author

There's always just one more library to check... 😄

@bartonjs bartonjs added the breaking-change Issue or PR that represents a breaking API or functional change over a prerelease. label Jul 31, 2025
Copy link
Contributor

Added needs-breaking-change-doc-created label because this PR has the breaking-change label.

When you commit this breaking change:

  1. Create and link to this PR and the issue a matching issue in the dotnet/docs repo using the breaking change documentation template, then remove this needs-breaking-change-doc-created label.
  2. Ask a committer to mail the .NET Breaking Change Notification DL.

Tagging @dotnet/compat for awareness of the breaking change.

@dotnet-policy-service dotnet-policy-service bot added the needs-breaking-change-doc-created Breaking changes need an issue opened with https://github.com/dotnet/docs/issues/new?template=dotnet label Jul 31, 2025
@bartonjs
Copy link
Member Author

Doc template filled out: dotnet/docs#47691

@bartonjs bartonjs removed the needs-breaking-change-doc-created Breaking changes need an issue opened with https://github.com/dotnet/docs/issues/new?template=dotnet label Jul 31, 2025
@bartonjs bartonjs merged commit 3ef8cf5 into dotnet:main Aug 4, 2025
87 checks passed
@bartonjs bartonjs deleted the mldsa_api_alignment branch August 4, 2025 17:25
@github-actions github-actions bot locked and limited conversation to collaborators Sep 4, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Security breaking-change Issue or PR that represents a breaking API or functional change over a prerelease.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[API Proposal]: ML-DSA
2 participants