[Bug]: ObtainRFC3161Timestamp occasionally fails with a CryptographicException and HRESULT TRUST_E_TIME_STAMP

[e455a81e-d3ba-41a2-bc6d-7aafb1d9a5cd]

Issue Description

Hi, I am using SecurityUtilities.SignFile to sign a ClickOnce manifest files but it occasionally fails with a System.Security.Cryptography.CryptographicException that stems from a call to ObtainRFC3161Timestamp.
I am directly calling the API using the Microsoft.Build.Tasks.Core nuget package.

Steps to Reproduce

This behaviour depends on the result of the random number generator in ObtainRFC3161Timestamp so the issues manifests on average once every couple of hundreds attempts. The following is a minimal example that reproduces the issue using code from SignedCmiManifest2.

void Main()
{

    for (int i = 1; i <= 1000; i++)
    {
        Console.Write(". ");

        try
        {
            ObtainRFC3161Timestamp(
                "http://timestamp.digicert.com",
                Convert.ToBase64String(System.Text.Encoding.ASCII.GetBytes("I'm a signature")),
                true);
        }
        catch (CryptographicException ex)
        {
            Console.WriteLine();
            Console.WriteLine($"i={i}, {ex.Message}, HResult: {ex.HResult:x}");
        }
    }

}

private static string ObtainRFC3161Timestamp(string timeStampUrl, string signatureValue, bool useSha256)
{
    byte[] sigValueBytes = Convert.FromBase64String(signatureValue);
    string timestamp = String.Empty;

    string algId = useSha256 ? Win32.szOID_NIST_sha256 : Win32.szOID_OIWSEC_sha1;

    unsafe
    {
        IntPtr ppTsContext = IntPtr.Zero;
        IntPtr ppTsSigner = IntPtr.Zero;
        IntPtr phStore = IntPtr.Zero;

        try
        {
            byte[] nonce = new byte[24];

            using (RandomNumberGenerator rng = RandomNumberGenerator.Create())
            {
                rng.GetBytes(nonce);
            }

            Win32.CRYPT_TIMESTAMP_PARA para = new Win32.CRYPT_TIMESTAMP_PARA()
            {
                fRequestCerts = true,
                pszTSAPolicyId = IntPtr.Zero,
            };

            fixed (byte* pbNonce = nonce)
            {
                para.Nonce.cbData = (uint)nonce.Length;
                para.Nonce.pbData = (IntPtr)pbNonce;

                if (!Win32.CryptRetrieveTimeStamp(
                        timeStampUrl,
                        0,
                        60 * 1000, // 1 minute timeout
                        algId,
                        ref para,
                        sigValueBytes,
                        sigValueBytes.Length,
                        ref ppTsContext,
                        ref ppTsSigner,
                        ref phStore))
                {
                    throw new CryptographicException(Marshal.GetLastWin32Error());
                }
            }

            var timestampContext = (Win32.CRYPT_TIMESTAMP_CONTEXT)Marshal.PtrToStructure(ppTsContext, typeof(Win32.CRYPT_TIMESTAMP_CONTEXT));
            byte[] encodedBytes = new byte[(int)timestampContext.cbEncoded];
            Marshal.Copy(timestampContext.pbEncoded, encodedBytes, 0, (int)timestampContext.cbEncoded);
            timestamp = Convert.ToBase64String(encodedBytes);
        }
        finally
        {
            if (ppTsContext != IntPtr.Zero)
                Win32.CryptMemFree(ppTsContext);

            if (ppTsSigner != IntPtr.Zero)
                Win32.CertFreeCertificateContext(ppTsSigner);

            if (phStore != IntPtr.Zero)
                Win32.CertCloseStore(phStore, 0);
        }
    }

    return timestamp;
}

internal static class Win32
{
    //
    // PInvoke dll's.
    //
    internal const String CRYPT32 = "crypt32.dll";
    internal const String KERNEL32 = "kernel32.dll";

    //
    // Structures.
    //
    [StructLayout(LayoutKind.Sequential, CharSet = CharSet.Unicode)]
    internal struct CRYPT_DATA_BLOB
    {
        internal uint cbData;
        internal IntPtr pbData;
    }
    //
    [DllImport(KERNEL32, CharSet = CharSet.Auto, SetLastError = true)]
    internal extern static
        IntPtr GetProcessHeap();

    [DllImport(KERNEL32, CharSet = CharSet.Auto, SetLastError = true)]
    [return: MarshalAs(UnmanagedType.Bool)]
    internal extern static
        bool HeapFree(
        [In] IntPtr hHeap,
        [In] uint dwFlags,
        [In] IntPtr lpMem);

    // hash algorithm OIDs
    internal const string szOID_OIWSEC_sha1 = "1.3.14.3.2.26";
    internal const string szOID_NIST_sha256 = "2.16.840.1.101.3.4.2.1";

    [StructLayout(LayoutKind.Sequential)]
    internal struct CRYPT_TIMESTAMP_CONTEXT
    {
        internal uint cbEncoded; // DWORD->unsigned int
        internal IntPtr pbEncoded; // BYTE*
        internal IntPtr pTimeStamp; // PCRYPT_TIMESTAMP_INFO->_CRYPT_TIMESTAMP_INFO*
    }

    [StructLayout(LayoutKind.Sequential)]
    internal struct CRYPTOAPI_BLOB
    {
        internal uint cbData;
        internal IntPtr pbData;
    }

    [StructLayout(LayoutKind.Sequential)]
    internal struct CRYPT_TIMESTAMP_PARA
    {
        internal IntPtr pszTSAPolicyId;
        internal bool fRequestCerts;
        internal CRYPTOAPI_BLOB Nonce;
        internal int cExtension;
        internal IntPtr rgExtension;
    }

    [DefaultDllImportSearchPaths(DllImportSearchPath.System32)]
    [DllImport(CRYPT32, CallingConvention = CallingConvention.Winapi, SetLastError = true)]
    [return: MarshalAs(UnmanagedType.Bool)]
    internal extern static
        bool CryptRetrieveTimeStamp(
        [In][MarshalAs(UnmanagedType.LPWStr)] string wszUrl,
        [In] uint dwRetrievalFlags,
        [In] int dwTimeout,
        [In][MarshalAs(UnmanagedType.LPStr)] string pszHashId,
        [In, Out] ref CRYPT_TIMESTAMP_PARA pPara,
        [In] byte[] pbData,
        [In] int cbData,
        [In, Out] ref IntPtr ppTsContext,
        [In, Out] ref IntPtr ppTsSigner,
        [In, Out] ref IntPtr phStore);

    [DefaultDllImportSearchPaths(DllImportSearchPath.System32)]
    [DllImport(CRYPT32, CallingConvention = CallingConvention.Winapi, SetLastError = true)]
    internal static extern bool CertFreeCertificateContext(IntPtr pCertContext);

    [DefaultDllImportSearchPaths(DllImportSearchPath.System32)]
    [DllImport(CRYPT32, CallingConvention = CallingConvention.Winapi, SetLastError = true)]
    internal static extern bool CertCloseStore(IntPtr pCertContext, int dwFlags);

    [DefaultDllImportSearchPaths(DllImportSearchPath.System32)]
    [DllImport(CRYPT32, CallingConvention = CallingConvention.Winapi)]
    internal static extern void CryptMemFree(IntPtr pv);
}

Expected Behavior

Signing the ClickOnce manifest file always succeeds.

Actual Behavior

Signing fails occasionally with a CryptographicException that has a HResult of TRUST_E_TIME_STAMP.

Analysis

The problem is that the Win32.CryptRetrieveTimeStamp() call within ObtainRFC3161Timestamp() sometimes returns a TRUST_E_TIME_STAMP result. The reason is because the generated nonce in ObtainRFC3161Timestamp() - just a evenly distributed random 192 bit number - is in an unexpected range.
When using the default ObtainRFC3161Timestamp() implementation of the 1000 calls about two TRUST_E_TIME_STAMP errors occur.
To test this can be made worse by doing nonce[^1] = 0xFF in ObtainRFC3161Timestamp() which lets 50% of calls fail.

The team responsible for nuget signing seems to have encountered a similiar issue and implemented a fix.
When implementing their fix nonce[^1] &= 0x7f; nonce[^1] |= 0x01; 100% of calls succeed.

Versions & Configurations

MSBuild version 17.7.2+d6990bcfa for .NET Framework
17.7.2.37605
dotnet --version
7.0.401

[AR-May]

The issue should be fixed by #9579. Please feel free to re-open the issue otherwise.