Make CertificateRequest et al work with ML-DSA #114357

bartonjs · 2025-04-07T22:51:21Z

A few things happen in this PR:

CRL Builder Tests now run for every supported signing algorithm
CopyWithPrivateKey was under-tested for all algorithms (particularly around the exception model), that has been rectified
Our test CertificateAuthority class gained algorithm agility
- On my system where MLDsa.IsSupported reports true, I changed the default key algorithm from RSASSA-2048+SHA-2-256 to ML-DSA-65, and everything passed.

Those pieces could be broken out into a pure test enhancement change for cleanliness.

The impetus of the change, however, was to add first-class support for ML-DSA to CertificateRequest.

Add ctors to CertificateRequest
Enlighten CertificateRequest that future signing algorithms might not require a HashAlgorithmName
Add support to CertificateRequestListBuilder
Add cert.GetMLDsaPublicKey/GetMLDsaPrivateKey/CopyWithPrivateKey to power the above.

Contributes to #113502

dotnet-issue-labeler · 2025-04-07T22:51:28Z

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

dotnet-issue-labeler · 2025-04-07T22:51:29Z

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

Copilot

Copilot reviewed 26 out of 29 changed files in this pull request and generated 2 comments.

Files not reviewed (3)

src/libraries/System.Security.Cryptography/ref/System.Security.Cryptography.csproj: Language not supported
src/libraries/System.Security.Cryptography/src/Resources/Strings.resx: Language not supported
src/libraries/System.Security.Cryptography/src/System.Security.Cryptography.csproj: Language not supported

...ecurity.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificateRequest.cs

src/libraries/Common/src/System/Security/Cryptography/MLDsaAlgorithm.cs

dotnet-policy-service · 2025-04-07T22:51:57Z

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

bartonjs · 2025-04-08T00:45:56Z

Looks like I forgot to try to build System.Net.Security.Tests, which also includes CertificateAuthority.cs

...curity.Cryptography/tests/X509Certificates/CertificateCreation/PrivateKeyAssociationTests.cs

....Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X509Certificate2.cs

...yptography/src/System/Security/Cryptography/X509Certificates/OpenSslX509CertificateReader.cs

...ryptography/src/System/Security/Cryptography/X509Certificates/MLDsaX509SignatureGenerator.cs

...libraries/Common/tests/System/Security/Cryptography/X509Certificates/CertificateAuthority.cs

src/libraries/System.Security.Cryptography/ref/System.Security.Cryptography.csproj

...ecurity.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificateRequest.cs

...ryptography/src/System/Security/Cryptography/X509Certificates/MLDsaX509SignatureGenerator.cs

vcsjones · 2025-04-08T17:31:03Z

...yptography/src/System/Security/Cryptography/X509Certificates/OpenSslX509CertificateReader.cs

@@ -17,6 +17,7 @@ internal sealed class OpenSslX509CertificateReader : ICertificatePal

        private SafeX509Handle _cert;
        private SafeEvpPKeyHandle? _privateKey;
+        private MLDsa? _mldsaPrivateKey;


Should this be added to Dispose like _privateKey?

vcsjones · 2025-04-08T17:32:08Z

.../System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/PublicKey.cs

@@ -314,6 +314,16 @@ public static PublicKey CreateFromSubjectPublicKeyInfo(ReadOnlySpan<byte> source
            return MLKem.ImportSubjectPublicKeyInfo(ExportSubjectPublicKeyInfo());
        }

+        [Experimental(Experimentals.PostQuantumCryptographyDiagId)]
+        [UnsupportedOSPlatform("browser")]
+        public MLDsa? GetMLDsaPublicKey()


....Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X509Certificate2.cs

vcsjones · 2025-04-08T17:45:33Z

...ity.Cryptography/src/System/Security/Cryptography/X509Certificates/X509SignatureGenerator.cs

@@ -32,5 +34,13 @@ public static X509SignatureGenerator CreateForRSA(RSA key, RSASignaturePadding s

            throw new ArgumentException(SR.Cryptography_InvalidPaddingMode);
        }
+
+        [Experimental(Experimentals.PostQuantumCryptographyDiagId)]
+        public static X509SignatureGenerator CreateForMLDsa(MLDsa key)


Docs would be good.

bartonjs · 2025-04-08T19:39:56Z

The longer the PR has been up, the more dirty I feel about a huge passel of test/infrastructure changes and the "enable ML-DSA" part. Partially because the "enable ML-DSA" part is supposed to serve as a template for SLH-DSA and CompositeMLDsa... so I think I'm going to break this into two pieces, even though that's more work.

Make CertificateRequest et al work with ML-DSA

a6fb0b8

bartonjs added the area-System.Security label Apr 7, 2025

bartonjs added this to the 10.0.0 milestone Apr 7, 2025

bartonjs self-assigned this Apr 7, 2025

Copilot AI review requested due to automatic review settings April 7, 2025 22:51

dotnet-issue-labeler bot added the new-api-needs-documentation label Apr 7, 2025

Copilot AI reviewed Apr 7, 2025

View reviewed changes

...ecurity.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificateRequest.cs Show resolved Hide resolved

src/libraries/Common/src/System/Security/Cryptography/MLDsaAlgorithm.cs Show resolved Hide resolved

build-analysis bot mentioned this pull request Apr 8, 2025

[wasm][AOT] emcc : error - received SIGKILL (-9) #89402

Open

krwq reviewed Apr 8, 2025

View reviewed changes

...curity.Cryptography/tests/X509Certificates/CertificateCreation/PrivateKeyAssociationTests.cs Show resolved Hide resolved

krwq reviewed Apr 8, 2025

View reviewed changes

....Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X509Certificate2.cs Show resolved Hide resolved

krwq reviewed Apr 8, 2025

View reviewed changes

...yptography/src/System/Security/Cryptography/X509Certificates/OpenSslX509CertificateReader.cs Show resolved Hide resolved

krwq reviewed Apr 8, 2025

View reviewed changes

...ryptography/src/System/Security/Cryptography/X509Certificates/MLDsaX509SignatureGenerator.cs Show resolved Hide resolved

Make CertificateAuthority build in other projects, too

124c328