104080 custom rsapss salt length by henning-krause · Pull Request #119255 · dotnet/runtime

henning-krause · 2025-09-02T05:45:50Z

Added support for custom PSS salt length for RSA based signature operations. Resolves #104080.

With this PR, I have extended the RsaSignaturePadding class with a custom salt length as discussed in #104080. I've also updated the CmsSigner and the CertificateRequest class to support this.

CoseSigner did not need functionality updates, because the spec does not support custom salt length. I have added checks to prevent this to be configured on the CoseSigner.

Tests where either added or updated to test the new functionality.

All RSA implementations which support this (MAC doesn't seem to support this) were updated.

…ation/generation with RSA keys. dotnet#104080

…lt customizations

…pported there) Updated reference code

Cose does not support custom salt lengths with PSS.

Copilot

Pull Request Overview

This pull request adds support for custom PSS salt length for RSA-based signature operations. The implementation extends the RSASignaturePadding class with a new CreatePss(int saltLength) method and updates related cryptographic components to utilize custom salt lengths.

Key changes include:

Extended RSASignaturePadding to support custom PSS salt lengths with new constants and factory method
Updated all RSA implementation backends (OpenSSL, CNG, BCrypt, etc.) to handle custom salt lengths
Added comprehensive test coverage for various salt length scenarios
Updated high-level APIs like CmsSigner and CertificateRequest to support the new functionality

Reviewed Changes

Copilot reviewed 40 out of 41 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
src/native/libs/System.Security.Cryptography.Native/pal_evp_pkey_rsa.h	Added pssSaltLength parameter to RSA sign/verify functions
src/native/libs/System.Security.Cryptography.Native/pal_evp_pkey_rsa.c	Implemented custom salt length support in OpenSSL backend
src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/RSASignaturePadding.cs	Added CreatePss method, PssSaltLength property, and related constants
src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/RSAPssX509SignatureGenerator.cs	Updated to use custom salt lengths from padding configuration
src/libraries/Common/src/System/Security/Cryptography/RsaPaddingProcessor.cs	Modified PSS encoding/verification to accept custom salt lengths
src/libraries/Common/src/System/Security/Cryptography/RsaPaddingProcessor.DigestInfo.cs	Added salt length calculation logic and moved digest info constants
src/libraries/System.Security.Cryptography/tests/X509Certificates/CertificateCreation/RSAPssX509SignatureGeneratorTests.cs	Added comprehensive tests for custom PSS salt lengths
src/libraries/System.Security.Cryptography.Pkcs/src/System/Security/Cryptography/Pkcs/CmsSigner.cs	Updated validation to accept any PSS mode padding
src/libraries/System.Security.Cryptography.Cose/src/System/Security/Cryptography/Cose/CoseSigner.cs	Added validation to prevent custom salt lengths in COSE

…tes/CertificateCreation/CertificateRequestChainTests.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

….Cryptography.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

…ngProcessor.DigestInfo.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

bartonjs · 2026-01-13T17:41:35Z

-
            // 3. If emLen < hLen + sLen + 2, output "inconsistent" and stop.
-            if (emLen < hLen + sLen + 2)
+            if (emLen < hLen + saltLength + 2)


Everything else in this function is named what it is in the spec. I'd prefer if you reintroduced the sLen variable a few lines up, but instead of int sLen = hLen; it would be int sLen = saltLength; (so the parameter has a good name for callers, but the function uses the name from the spec)

And, of course, after introducing sLen, undo all of the sLen => saltLength changes in this function.

It looks like you undid the sLen to saltLength rename in EncodePss, but not in VerifyPss.

bartonjs · 2026-01-13T17:44:41Z

+                0x11, 0xf2, 0x10, 0xbd, 0x19, 0x47, 0x75, 0x3c, 0x30, 0xc4, 0x0f, 0xd0, 0x7b, 0xc2, 0x6b, 0x4b
+            };
+
+            RSASignaturePadding padding = RSASignaturePadding.CreatePss(validateWithCorrectSaltLength ? RSASignaturePadding.PssSaltLengthMax : 2);


Since max ends up being a specific number when paired with the key, there should be a test that signs with max and verifies with the number, and vice versa.

Probably not this one, since it's not visibly calling sign.

bartonjs · 2026-01-13T17:54:54Z

+            private byte[] GetSignaturePaddingForCustomPssSaltLength(X509Certificate2 certificate, HashAlgorithmName hashAlgorithmName)
+            {
+                string digestOid = PkcsHelpers.GetOidFromHashAlgorithm(hashAlgorithmName);
+                using RSA? publicKey = certificate.GetRSAPublicKey();


In cryptography code, other than test code, we (almost?) always use braced using-Dispose statements so it's much clearer when objects are freed.

But, rather than re-opening a key handle here, could we maybe just save the key size in the call to SignCore?

@bartonjs So SignCore would return the signatureParameters via an out variable?

That seems reasonable, yeah.

bartonjs · 2026-01-15T00:11:10Z

                "07849DC26FCBB2F3BD5F57BDF214BAE374575F1BD4E6816482324799417CB379",
                messageDigestAttr.MessageDigest.ByteArrayToHex());

+


This looks like it turned a blank line into a "non-blank blank line" (only has spaces). That should be trimmed down to a non-diff.

bartonjs · 2026-01-15T00:57:48Z

+        {
+            using (RSA rsa = RSA.Create())
+            {
+                rsa.ImportParameters(TestData.RsaBigExponentParams);


@vcsjones when I saw this line I immediately thought "but didn't we (you) move us off of this key?", but it looks like no...

Am I entirely hallucinating, or what?

bartonjs · 2026-01-15T01:01:21Z

+                pssStructure.ReadEncodedValue(); // Ignore the hash algorithm OID
+                pssStructure.ReadEncodedValue(); // Ignore the mask generation function OID


The pre-existing test that verifies these values doesn't take a custom salt size. We should verify them here to show that we didn't break that codepath with custom salt sizes.

While we're at it, we should also add hash algorithm variance.

(We can verify the algorithm/MGF OIDs in a different test than the custom salt size, but we do want to verify both pieces of data in some test)

…/henning-krause/runtime into 104080-custom-rsapss-salt-length

bartonjs · 2026-01-27T00:22:12Z

-
            // 3. If emLen < hLen + sLen + 2, output "inconsistent" and stop.
-            if (emLen < hLen + sLen + 2)
+            if (emLen < hLen + saltLength + 2)


It looks like you undid the sLen to saltLength rename in EncodePss, but not in VerifyPss.

bartonjs · 2026-01-27T00:25:15Z

-                pssStructure.ReadEncodedValue(); // Ignore the hash algorithm OID
-                pssStructure.ReadEncodedValue(); // Ignore the mask generation function OID
+                ReadOnlyMemory<byte> hashAlgorithm = pssStructure.ReadEncodedValue(); // Ignore the hash algorithm OID
+                ReadOnlyMemory<byte> mgf = pssStructure.ReadEncodedValue(); // Ignore the mask generation function OID


The previous feedback was that the hashAlgorithm and mgf values needed to be verified. You have now captured them into locals, but I don't see you verifying that they're the expected values.

Again, the purpose is to make sure that we're writing down a confirming/correct block when a custom PSS size is specified, and I don't see any other test doing that.

I was interrupted last time I worked on this, so I committed early. I'll fix this next time together with the remaining open issues.

Copilot

Pull request overview

Copilot reviewed 38 out of 39 changed files in this pull request and generated 7 comments.

Copilot · 2026-03-24T06:28:07Z

+using System.Formats.Asn1;
+using System.Security.Cryptography.Asn1;


These using directives appear to be unused in this file. Consider removing them to avoid IDE0005 / analyzer warnings (which may be treated as errors in some build configurations).

Copilot · 2026-03-24T06:28:08Z

+        /// A constant value indicating that the PSS salt length should be the maximum allowable
+        /// </summary>
+        /// <remarks>A constant is used to define the upper limit for the salt length in PSS-based
+        /// cryptographic operations. The maximum length is determined by the hash algorithm's output size.</remarks>


The remarks for PssSaltLengthMax say the maximum salt length is determined by the hash algorithm output size, but it also depends on the RSA key size (max is emLen - hLen - 2). Consider updating the docs to reflect the key-size dependency.

Suggested change

/// A constant value indicating that the PSS salt length should be the maximum allowable

/// </summary>

/// <remarks>A constant is used to define the upper limit for the salt length in PSS-based

/// cryptographic operations. The maximum length is determined by the hash algorithm's output size.</remarks>

/// A constant value indicating that the PSS salt length should be the maximum allowable for the given RSA key and hash algorithm.

/// </summary>

/// <remarks>

/// When this value is used, the effective salt length is chosen by the implementation so that it does not exceed

/// the maximum permitted by RSASSA-PSS, which depends on both the RSA key size and the hash algorithm's output

/// size (at most <c>emLen - hLen - 2</c> bytes, where <c>emLen</c> is the encoded message length in bytes and

/// <c>hLen</c> is the hash output length in bytes).

/// </remarks>

Copilot · 2026-03-24T06:28:08Z

+        /// Specifies the salt length to use for PSS padding. This property is only relevant when the <see cref="Mode"/> is <see cref="RSASignaturePaddingMode.Pss"/>.
+        /// </summary>
+        /// <remarks>
+        /// This value must either be a positive number or one of the special constants <see cref="PssSaltLengthIsHashLength"/> or <see cref="PssSaltLengthMax"/>.


The PssSaltLength remarks say the value must be a "positive" number, but CreatePss allows 0 (and tests cover it). Consider updating the wording to "non-negative" to match the API behavior.

Suggested change

/// This value must either be a positive number or one of the special constants <see cref="PssSaltLengthIsHashLength"/> or <see cref="PssSaltLengthMax"/>.

/// This value must either be a non-negative number or one of the special constants <see cref="PssSaltLengthIsHashLength"/> or <see cref="PssSaltLengthMax"/>.

Copilot · 2026-03-24T06:28:08Z

+            if (certKind == CertKind.RsaPkcs1 || certKind == CertKind.RsaPss ||certKind == CertKind.RsaPssWithCustomSaltLength || certKind == CertKind.RsaPssWithMaxSaltLength)
            {


Minor formatting: missing space after "||" makes the condition harder to read and is inconsistent with typical formatting in this file.

Copilot · 2026-03-24T06:28:08Z

 {
    public static partial class SignedCmsTests
    {
+        private static bool AreCustomPssSaltLengthsSupported => PlatformDetection.IsWindows || PlatformDetection.IsLinux;


AreCustomPssSaltLengthsSupported is hard-coded to Windows/Linux, but custom PSS salt lengths are supported on other OpenSSL-based Unix platforms too. Consider reusing PlatformSupport.AreCustomSaltLengthsSupportedWithPss here to avoid unnecessarily skipping coverage on supported platforms (e.g., FreeBSD).

Suggested change

private static bool AreCustomPssSaltLengthsSupported => PlatformDetection.IsWindows || PlatformDetection.IsLinux;

private static bool AreCustomPssSaltLengthsSupported => PlatformSupport.AreCustomSaltLengthsSupportedWithPss;

Copilot · 2026-03-24T06:28:09Z

+            // PSS signature with parameters are only supported on .NET 11 or later
+#if NET11_0_OR_GREATER
+            if (PlatformSupport.AreCustomSaltLengthsSupportedWithPss)
+            {
+                signer.CheckSignature(true);
+            }
+            else
+            {
+                Assert.Throws<CryptographicException>(() => signer.CheckSignature(true));
+            }
+#else
            Assert.Throws<CryptographicException>(() => signer.CheckSignature(true));
+#endif


The comments/guards are inconsistent: this block says PSS parameters are only supported on .NET 11+, but the later cms.CheckSignature block mentions .NET 10+ without an equivalent compile-time guard. Consider aligning the comments (and/or the #if) so the minimum supported version is unambiguous.

Copilot · 2026-03-24T06:28:09Z

+        internal static int CalculatePssSaltLength(int pssSaltLength, int rsaKeySizeInBits, HashAlgorithmName hashAlgorithm)
+        {
+            int emLen = BytesRequiredForBitCount(rsaKeySizeInBits);
+            int hLen = HashLength(hashAlgorithm);
+            return pssSaltLength switch
+            {
+                PssSaltLengthMax => Math.Max(0, emLen - hLen - 2),
+                PssSaltLengthIsHashLength => hLen,
+                _ => pssSaltLength


CalculatePssSaltLength computes emLen from rsaKeySizeInBits, but PSS encoding/verification in this codebase uses emBits = keySize - 1. For RSA keys whose bit-length is not a multiple of 8, this will overestimate the maximum salt length by 1 byte and can cause CreatePss(PssSaltLengthMax) to fail with key-too-small for otherwise-valid keys. Consider basing emLen on (rsaKeySizeInBits - 1) to align with EncodePss/VerifyPss.

henning-krause and others added 23 commits May 16, 2025 11:32

Added support for a custom salt length when using PSS signature valid…

e0b1dae

…ation/generation with RSA keys. dotnet#104080

Review

05ed28b

Merge branch 'dotnet:main' into 104080-custom-rsapss-salt-length

21135d5

Review all usages for RSASignaturePadding to check for missing PSS sa…

d705126

…lt customizations

Added error handling for apple devices (custom PSS salt length not su…

3c6288c

…pported there) Updated reference code

Added sign and verify tests for custom salt lengths

4d3bd37

Added test for max salt length verification

d19d017

Added a test for the RSAPssX509SignatureGenerator

20be62b

Added test for CertificateRequest.LoadSigningRequest

c0d0ce0

More tests for the CertificateRequest

81432d5

Added custom salt support for SignedCms

a016382

Add error handling for PSS salt length in CoseSigner

9cde2fa

Cose does not support custom salt lengths with PSS.

Updated RSA

a6dd48a

Merged from main

71325e3

Updated CoseSigner

e2407be

Code fixes

7b65140

Code fixes

623199c

Code fixes

b14906e

Code fixes

9dcb4ea

Code fixes

fee0f09

Code fixes

a386bac

Updated a few test cases

430a846

CRL Builder tests extended

e0eb704

Copilot AI review requested due to automatic review settings September 2, 2025 05:45

github-actions Bot added the area-System.Security label Sep 2, 2025

dotnet-policy-service Bot added the community-contribution Indicates that the PR has been added by a community member label Sep 2, 2025

Copilot AI reviewed Sep 2, 2025

View reviewed changes

henning-krause and others added 3 commits September 2, 2025 07:51

Update src/libraries/System.Security.Cryptography/tests/X509Certifica…

dd64636

…tes/CertificateCreation/CertificateRequestChainTests.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update src/libraries/System.Security.Cryptography/ref/System.Security…

d97581c

….Cryptography.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update src/libraries/Common/src/System/Security/Cryptography/RsaPaddi…

e99f10d

…ngProcessor.DigestInfo.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

henning-krause and others added 5 commits January 6, 2026 07:19

Merged from main

4b292d3

Merge branch 'dotnet:main' into 104080-custom-rsapss-salt-length

966b17a

Merged from main

0264ed5

Changed .net10 #if to .net11

0c65d90

More #if replacements

30ff8ef

This was referenced Jan 6, 2026

CryptographicException in NonPowerOfTwoKeySizeOaepRoundtrip test #120606

Closed

iOS.Device test WorkItemExecutions #122874

Closed

Merge branch 'main' into 104080-custom-rsapss-salt-length

d335e43

bartonjs reviewed Jan 15, 2026

View reviewed changes

henning-krause added 2 commits January 23, 2026 08:28

Cleaned up more review issues

f9fb07e

Merge branch '104080-custom-rsapss-salt-length' of https://github.com…

89d94bf

…/henning-krause/runtime into 104080-custom-rsapss-salt-length

bartonjs reviewed Jan 27, 2026

View reviewed changes

jeffhandley added the needs-author-action An issue or pull request that requires more info or actions from the author. label Mar 8, 2026

dotnet-policy-service Bot added the no-recent-activity label Mar 23, 2026

Merged from main

3dc7da6

Copilot AI review requested due to automatic review settings March 24, 2026 06:19

dotnet-policy-service Bot removed needs-author-action An issue or pull request that requires more info or actions from the author. no-recent-activity labels Mar 24, 2026

Copilot started reviewing on behalf of henning-krause March 24, 2026 06:20 View session

Copilot AI reviewed Mar 24, 2026

View reviewed changes

This was referenced Apr 22, 2026

Android native build fails with EVP_PKEY_get0_RSA conflicting types on NDK 27 #127271

Closed

tvOS runtime tests fail with device listing error on DNCENGMAC065 #127272

Closed

		"07849DC26FCBB2F3BD5F57BDF214BAE374575F1BD4E6816482324799417CB379",
		messageDigestAttr.MessageDigest.ByteArrayToHex());

		pssStructure.ReadEncodedValue(); // Ignore the hash algorithm OID
		pssStructure.ReadEncodedValue(); // Ignore the mask generation function OID

		using System.Formats.Asn1;
		using System.Security.Cryptography.Asn1;

	/// This value must either be a positive number or one of the special constants <see cref="PssSaltLengthIsHashLength"/> or <see cref="PssSaltLengthMax"/>.
	/// This value must either be a non-negative number or one of the special constants <see cref="PssSaltLengthIsHashLength"/> or <see cref="PssSaltLengthMax"/>.

		if (certKind == CertKind.RsaPkcs1 \|\| certKind == CertKind.RsaPss \|\|certKind == CertKind.RsaPssWithCustomSaltLength \|\| certKind == CertKind.RsaPssWithMaxSaltLength)
		{

	private static bool AreCustomPssSaltLengthsSupported => PlatformDetection.IsWindows \|\| PlatformDetection.IsLinux;
	private static bool AreCustomPssSaltLengthsSupported => PlatformSupport.AreCustomSaltLengthsSupportedWithPss;

Conversation

henning-krause commented Sep 2, 2025 • edited by jeffhandley Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

henning-krause commented Sep 2, 2025 •

edited by jeffhandley

Loading