Skip to content

ML-DSA PFX support #115758

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

Merged
merged 8 commits into from
May 20, 2025
Merged

Conversation

PranavSenthilnathan
Copy link
Member

PFX support for ML-DSA. Similar to ML-KEM (#115271) and SLH-DSA (#115212).

Note: ML-DSA isn't supported in Microsoft.Bcl.Cryptography yet so the tests only cover S.S.C.

Contributes to #113502

@PranavSenthilnathan PranavSenthilnathan self-assigned this May 20, 2025
@Copilot Copilot AI review requested due to automatic review settings May 20, 2025 06:08
Copy link
Contributor

Tagging subscribers to this area: @dotnet/area-system-security, @bartonjs, @vcsjones
See info in area-owners.md if you want to be subscribed.

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

Adds support for multi-level DSA (ML-DSA) in the cryptography stack, mirroring existing ML-KEM and SLH-DSA support.

  • Extends native EVP PKEY family and algorithm checks for ML-DSA
  • Implements ML-DSA handlers in X509 certificate loaders, OpenSSL export provider, and PublicKey API
  • Adds a comprehensive set of ML-DSA tests for PEM, encrypted PEM, PFX, export, and key association

Reviewed Changes

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

Show a summary per file
File Description
pal_evp_pkey.h / pal_evp_pkey.c Add PalPKeyFamilyId_MLDsa and EVP checks for ML-DSA OIDs
X509Certificate2PemTests.cs / PublicKeyTests.cs / PfxTests.cs / ExportTests.cs / PrivateKeyAssociationTests.cs / CertTests.cs Add ML-DSA end-to-end tests for certificate import/export and key operations
X509CertificateLoader.Unix.cs / X509CertificateLoader.OpenSsl.cs / X509Certificate2.cs / PublicKey.cs / OpenSslExportProvider.cs Wire up ML-DSA into certificate loading, key extraction, and export paths
ref/System.Security.Cryptography.cs Add experimental PublicKey ctor overload for ML-DSA
MLDsaTestsData.cs Extend test data records with ML-DSA vectors and PFX payloads
Interop.EvpPkey.cs Expose EvpAlgorithmFamilyId.MLDsa for Unix interop
Comments suppressed due to low confidence (2)

src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/MLDsa/MLDsaTestsData.cs:6

  • The file uses [ConditionalFact], [ConditionalTheory], and [MemberData] attributes but the using Xunit; and using Microsoft.DotNet.XUnitExtensions; directives were removed. Re-add these usings so the test attributes resolve correctly.
-        using Microsoft.DotNet.XUnitExtensions;

src/libraries/System.Security.Cryptography/tests/X509Certificates/X509Certificate2PemTests.cs:499

  • [nitpick] Consider renaming the loop variable PrivateKeyPem to EncryptedPrivateKeyPem in the encrypted-PKCS#8 test to match the tuple element name and avoid confusion.
foreach ((string CertificatePem, string PrivateKeyPem, string Thumbprint) in cases)

),
];

foreach ((string CertificatePem, string PrivateKeyPem, string Thumbprint) in cases)
Copy link
Member

@bartonjs bartonjs May 20, 2025

Choose a reason for hiding this comment

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

since this is deconstructing, rather than declaring a tuple shape, the names should all be camelCase, not pascalCase.

Suggested change
foreach ((string CertificatePem, string PrivateKeyPem, string Thumbprint) in cases)
foreach ((string certificatePem, string privateKeyPem, string thumbprint) in cases)

Alternatively, make it a tuple

Suggested change
foreach ((string CertificatePem, string PrivateKeyPem, string Thumbprint) in cases)
foreach ((string CertificatePem, string PrivateKeyPem, string Thumbprint) info in cases)

Copy link
Member

@bartonjs bartonjs left a comment

Choose a reason for hiding this comment

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

The naming violations should be fixed; but if they're also wrong for other algorithms maybe they should just be fixed together in a cleanup pass.

@PranavSenthilnathan PranavSenthilnathan merged commit bc1249a into dotnet:main May 20, 2025
100 checks passed
SimaTian pushed a commit that referenced this pull request May 27, 2025
Co-authored-by: Kevin Jones <[email protected]>
Co-authored-by: Jeremy Barton <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants