Skip to content

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
@dotnet-policy-service
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 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]>
@github-actions github-actions bot locked and limited conversation to collaborators Jun 20, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants