Skip to content

Fix SLH-DSA key to use SHA-1 MAC for Windows support #115271

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 2 commits into from
May 6, 2025

Conversation

vcsjones
Copy link
Member

@vcsjones vcsjones commented May 3, 2025

The PKCS#12 document in IetfSlhDsaSha2_128sCertificatePfx_Pbes1 used pbeWithSHA1And3-KeyTripleDES-CBC for key and certificate encryption, but it still used SHA-256 for the MAC, which Windows 10 Threshold does not support. This pull request re-encodes it to use a SHA-1 MAC.

Contributes to #115270 (not marking as fixing until Build Analysis shows a 0 24-hour hit count)

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

Updates the PKCS#12 test fixture to use a SHA-1 MAC for Windows compatibility, replacing the previous SHA-256 MAC–encoded blob.

  • Replaces the base64 PFX data for IetfSlhDsaSha2_128sCertificatePfx_Pbes1 with a new blob using SHA-1 MAC.
  • Ensures Windows 10 Threshold can import the test PFX.
Comments suppressed due to low confidence (1)

src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/SlhDsa/SlhDsaTestData.cs:385

  • There’s no test that validates the MAC algorithm on the new PFX blob. Consider adding a test that loads this PFX and asserts its MAC OID is the SHA-1 OID (1.3.14.3.2.26) to prevent regressions.
internal static byte[] IetfSlhDsaSha2_128sCertificatePfx_Pbes1 => field ??= Convert.FromBase64String(@"

Copy link
Member

@PranavSenthilnathan PranavSenthilnathan left a comment

Choose a reason for hiding this comment

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

Thanks!

@vcsjones
Copy link
Member Author

vcsjones commented May 6, 2025

@bartonjs @PranavSenthilnathan something dead lettered? Can this be ba-g'ed?

@bartonjs
Copy link
Member

bartonjs commented May 6, 2025

/ba-g This test doesn't even run on wasm.

@vcsjones vcsjones merged commit dd75c45 into dotnet:main May 6, 2025
83 of 85 checks passed
@vcsjones vcsjones deleted the slhdsa-mac branch May 6, 2025 00:25
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