dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
14.55k stars 4.54k forks source link

[API Proposal]: RSA PSS salt length #104080

Open krwq opened 1 week ago

krwq commented 1 week ago

Background and motivation

Some implementations of RSA PSS by default use different salt length for RSA PSS than .NET and it's currently not possible to change it.

.NET currently follows https://datatracker.ietf.org/doc/html/rfc3447#section-9.1 recommendation and uses salt length equal to hash length but spec doesn't forbid other values. On OpenSSL some providers (i.e. tpm2) use different salt length making it impossible to validate such signature (interestingly signature validates on Linux<-->Linux by default uses salt length detection during validation: RSA_PSS_SALTLEN_AUTO)

We should also support typical placeholder values as well when implementing this:

and ideally detection of salt length or at least support for sLen=hLen and RSA_PSS_SALTLEN_MAX which seem to be most frequently used.

The detection should be easily implemented because after unmasking encoded message PS length can be detected by counting number of leading zeros which should be followed by 0x01. From that remainder is salt.

API Proposal

Not proposing specific APIs but for example:

partial class RSASignaturePadding
{
    public const int PSSSaltLengthEqualToHashLength = -1;
    public const int PSSSaltLengthMax = -2;
    public RSASignaturePadding(RSASignaturePaddingMode mode, int saltLength = PSSSaltLengthEqualToHashLength) {}
    // or
    public static CreatePSS(int saltLength) {}
}

API Usage

RSASignaturePadding pssWithCustomSalt = RSASignaturePadding.CreatePSS(RSASignaturePadding.PSSSaltLengthMax);
// ..
rsa.VerifyData(data, sig, hashName, pssWithCustomSalt);

Alternative Designs

No response

Risks

No response

dotnet-policy-service[bot] commented 1 week ago

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

bartonjs commented 1 week ago

The detection should be easily implemented because after unmasking encoded message PS length can be detected

While we have code for doing PSS, we prefer to let the OS do it. I don't think there's enough demand for this feature that we'd change our policy there, which means, basically "we can't do this".

On OpenSSL some providers (i.e. tpm2) use different salt length

If the provider isn't respecting the salt length we've set, it certainly won't respect some other value. So that doesn't seem like justification for new API.

0 - zero salt length

"Why?" or "No.". Take your pick :smile:.

krwq commented 1 week ago

For "why?" it's compat. I don't think this is super blocking in any way given it's tpm2 provider who should be creating signatures as requested but I think on our side it would be really nice to have an opt-in switch for either manual salt length or "accept whatever salt length" (emphasize on opt-in with warning that is theoretically making security worse). Also it would make debugging these sort of issues easier since there is currently no way to make non-standard size salt work (and spec doesn't forbid sLen != hLen - it only recommends to use equal lengths).

vcsjones commented 1 week ago

Bear in mind that it is the TPM that has opinions about salt lengths. The Platform Crypto Provider appears to reject any RSA signing operation using a PSS salt where $hLen \neq sLen$.

Output

Salt size 32: It worked.
Salt size 31: FAILED 0x40290423
Salt size 30: FAILED 0x40290423
Salt size 33: FAILED 0x40290423
Salt size 34: FAILED 0x40290423

0x40290423 is TPM_E_PCP_UNSUPPORTED_PSS_SALT. At least in the case of Windows for the specified use case, I don't see a new API accomplishing anything since the TPM will reject any value other than the one we are providing.

using System.Security.Cryptography;
using System.Runtime.InteropServices;
using Microsoft.Win32.SafeHandles;
using System;
using System.Text;

#pragma warning disable CA1416 // Thingy I do not care about.

public class Program {
    static unsafe void Main() {
        CngKey? key = null;

        try {
            key = CngKey.Create(CngAlgorithm.Rsa, RandomNumberGenerator.GetHexString(32), new CngKeyCreationParameters {
                Provider = CngProvider.MicrosoftPlatformCryptoProvider
            });

            TryRsaWithSaltSize(key, SHA256.HashSizeInBytes);
            TryRsaWithSaltSize(key, SHA256.HashSizeInBytes - 1);
            TryRsaWithSaltSize(key, SHA256.HashSizeInBytes - 2);
            TryRsaWithSaltSize(key, SHA256.HashSizeInBytes + 1);
            TryRsaWithSaltSize(key, SHA256.HashSizeInBytes + 2);
        }
        finally {
            key.Delete();
        }
    }

    static unsafe void TryRsaWithSaltSize(CngKey key, int saltSize) {
        // SHA256\0 as UTF16-LE
        ReadOnlySpan<byte> alg = [0x53, 0x00, 0x48, 0x00, 0x41, 0x00, 0x32, 0x00, 0x35, 0x00, 0x36, 0x00, 0x00, 0x00];
        byte[] hash = RandomNumberGenerator.GetBytes(SHA256.HashSizeInBytes); // A "hash" to sign.
        byte[] sigBuffer = new byte[1024]; // Buffer to receive a signature. It's oversized. We'll slice it later.

        fixed (byte* pSigBuffer = sigBuffer)
        fixed (byte* pHash = hash)
        fixed (byte* pAlg = alg) {
            BCRYPT_PSS_PADDING_INFO padding = new() {
                pszAlgId = pAlg,
                cbSalt = saltSize
            };

            int ret = InteropStuff.NCryptSignHash(key.Handle, &padding, pHash, SHA256.HashSizeInBytes, pSigBuffer, sigBuffer.Length, out int sigWritten, AsymmetricPaddingMode.NCRYPT_PAD_PSS_FLAG);

            if (ret != 0) {
                Console.WriteLine($"Salt size {saltSize}: FAILED 0x{ret:X08}");
            }
            else {
                Console.WriteLine($"Salt size {saltSize}: It worked.");
            }
        }
    }
}

internal partial class InteropStuff {
    [LibraryImport("ncrypt.dll", StringMarshalling = StringMarshalling.Utf16, SetLastError = true)]
    internal static unsafe partial int NCryptSignHash(SafeNCryptKeyHandle hKey, void* pPaddingInfo, byte* pbHashValue, int cbHashValue, byte* pbSignature, int cbSignature, out int pcbResult, AsymmetricPaddingMode dwFlags);
}

internal enum AsymmetricPaddingMode : int {
    None = 0x00000000,
    NCRYPT_NO_PADDING_FLAG = 0x00000001,
    NCRYPT_PAD_PKCS1_FLAG = 0x00000002,
    NCRYPT_PAD_OAEP_FLAG = 0x00000004,
    NCRYPT_PAD_PSS_FLAG = 0x00000008,
}

[StructLayout(LayoutKind.Sequential)]
internal unsafe struct BCRYPT_PSS_PADDING_INFO {
    internal byte* pszAlgId;
    internal int cbSalt;
}
krwq commented 4 days ago

@vcsjones TPM 2.0 spec (https://trustedcomputinggroup.org/resource/tpm-library-specification/) Part 1 B.7:

image

but I agree per latest TPM 2.0 + FIPS 186-5 combined basically tells in many words that sLen == hLen. I'm not sure if there is any statement that we're not allowed to validate this though (also I'm not in US so FIPS is just foreign country recommendation to me not a strict enforcement).

vcsjones commented 4 days ago

My point is more than TPMs do not have a provision to let clients pick a salt size. The TPM is going to use the salt size that it is configured to use.

These are the Table 169 RSA schemes that only need a hash algorithm as a scheme parameter. For the TPM_ALG_RSAPSS signing scheme, the same hash algorithm is used for digesting TPM-generated data (an attestation structure) and in the KDF used for the masking operation. The salt size is always the largest salt value that will fit into the available space.

Table 169 — Definition of {RSA} Types for RSA Signature Schemes

Type Name Description
TPMS_SCHEME_HASH TPMS_SIGSCHEME!ALG.AX

Where TPMS_SCHEME_HASH has one parameter, hashAlg, of type TPMI_ALG_HASH.

My interpretation of the TPM spec indicates to me that the most control you have over a PSS signature is the hash algorithm. The salt size is dictated by the specification. What it is dictated to depends on how the TPM adheres to FIPS, some have a "FIPS mode" (FIPS_140_2 attribute) that may alter behavior, but the signing operation itself does not pick the salt length.

So, even if we added some new API for configuring the salt size, the TPM would probably have to error, or ignore, every value other than the one that the TPM uses.

krwq commented 3 days ago

Note that my main focus of this API is validation of signatures produced by TPM - not signing. I'm already convinced signing is not important for other lengths given latest specs only allow single salt size. I'd really love to say that the problem is only with some old TPM modules but according to https://github.com/tpm2-software/tpm2-openssl/issues/115#issuecomment-2200945970 it seems that currently TPMs mostly use different salt lengths than sLen==hLen - this will probably change in the following years but I expect in couple of years that will still be relatively large number.

Perhaps it would be possible to at least add some opt-in APIs for relaxing this a bit for validation only (and enforce sLen==hLen on signing). I.e. setting could be called according to FIPS version so at least people know what they're following. And honestly I can only see supporting 4 settings for validation:

so possibly it could be just a list of allowed lengths defaulting to just hLen with couple of predefined settings which match different versions of FIPS so that people know which one is the latest.