Skip to content

PKCS#8 support for ML-DSA #115569

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

PranavSenthilnathan
Copy link
Member

@PranavSenthilnathan PranavSenthilnathan commented May 14, 2025

includes import/export for:

  • PKCS#8
  • Encrypted PKCS#8
  • PEM

The implementation is very similar to SLH-DSA overall and to ML-KEM for the seed related logic.

Contributes to #113502

@Copilot Copilot AI review requested due to automatic review settings May 14, 2025 17:00
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 pull request adds support for PKCS#8, Encrypted PKCS#8, and PEM import/export for ML‑DSA, extending functionality already present in related modules (SLH‑DSA and ML‑KEM).

  • Introduces new XML and code files for ML‑DSA private key ASN.1 encoding/decoding.
  • Modifies key implementation classes (MLDsaOpenSsl, MLDsaImplementation) to propagate seed and secret key information.
  • Adds new overloads and tests (including IETF test cases) to cover the new PKCS#8 support.

Reviewed Changes

Copilot reviewed 27 out of 27 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
System.Security.Cryptography.Tests.csproj Added AsnXml and Compile entries for new ML‑DSA private key files and test helpers.
MLDsaOpenSsl*.cs, MLDsaImplementation*.cs Extended constructors and key generation/import methods to handle seed and secret key flags.
Common/src/System/Security/Cryptography/MLDsaPkcs8.cs Implements PKCS#8 export logic for ML‑DSA keys using rented buffers.
Common/src/System/Security/Cryptography/Asn1/* Added generated ASN.1 encoder/decoder files for both and seed/expanded key representations.
Common/tests/System/Security/Cryptography/AlgorithmImplementations/MLDsa/* Introduced new test cases and helper methods for verifying the new functionality.
Interop/Unix/System.Security.Cryptography.Native/Interop.EvpPkey.MLDsa.cs Updated native interop to return additional boolean flags for seed and secret key presence.

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.


AsnWriter writer = new AsnWriter(AsnEncodingRules.DER);
// The ASN.1 overhead of a SubjectPublicKeyInfo encoding a public key is 22 bytes.
Copy link
Member Author

Choose a reason for hiding this comment

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

This was computed by comparing openssl genpkey -algorithm ML-DSA-87 | openssl pkey -pubout -outform DER | wc with the public key size for the algo.

private TResult ExportPkcs8PrivateKeyCallback<TResult>(ExportPkcs8PrivateKeyFunc<TResult> func)
{
// A PKCS#8 ML-DSA secret key has an ASN.1 overhead of 28 bytes, assuming no attributes.
// Make it an even 32 and that should give a good starting point for a buffer size.
Copy link
Member Author

Choose a reason for hiding this comment

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

Computed with openssl genpkey -algorithm ML-DSA-87 | openssl pkey -provparam ml-dsa.output_formats=priv-only -outform DER | wc and compared to secret key size.

Comment on lines +16 to +17
"d7b2b47254aae0db45e7930d4a98d2c97d8f1397d17" +
"89dafa17024b316e9bec94fc9946d42f19b79a7413bbaa33e7149cb42ed51156" +
Copy link
Member

Choose a reason for hiding this comment

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

Why offset at the beginning like this?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's how it looks in the IETF doc (it's in the middle of an ASN pretty print). Flushing to the left looked weird too so I just kept it like this.

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.

3 participants