Merging internal commits for release/8.0

src/libraries/System.Private.CoreLib/src/System/Number.BigInteger.cs

@@ -31,7 +31,9 @@ internal unsafe ref struct BigInteger
             private const int MaxBits = BitsForLongestBinaryMantissa + BitsForLongestDigitSequence + BitsPerBlock;
 
             private const int BitsPerBlock = sizeof(int) * 8;
-            private const int MaxBlockCount = (MaxBits + (BitsPerBlock - 1)) / BitsPerBlock;
+
+            // We need one extra block to make our shift left algorithm significantly simpler
+            private const int MaxBlockCount = ((MaxBits + (BitsPerBlock - 1)) / BitsPerBlock) + 1;
 
             private static ReadOnlySpan<uint> Pow10UInt32Table => new uint[]
             {
@@ -302,7 +304,8 @@ internal unsafe ref struct BigInteger
                 0xD9D61A05,
                 0x00000325,
 
-                // 9 Trailing blocks to ensure MaxBlockCount
+                // 10 Trailing blocks to ensure MaxBlockCount
+                0x00000000,
                 0x00000000,
                 0x00000000,
                 0x00000000,
@@ -358,11 +361,24 @@ public static void Add(scoped ref BigInteger lhs, scoped ref BigInteger rhs, out
                     resultIndex++;
                 }
 
+                int resultLength = largeLength;
+
                 // If there's still a carry, append a new block
                 if (carry != 0)
                 {
                     Debug.Assert(carry == 1);
-                    Debug.Assert((resultIndex == largeLength) && (largeLength < MaxBlockCount));
+                    Debug.Assert(resultIndex == resultLength);
+                    Debug.Assert(unchecked((uint)(resultLength)) < MaxBlockCount);
+
+                    if (unchecked((uint)(resultLength)) >= MaxBlockCount)
+                    {
+                        // We shouldn't reach here, and the above assert will help flag this
+                        // during testing, but we'll ensure that we return a safe value of
+                        // zero in the case we end up overflowing in any way.
+
+                        SetZero(out result);
+                        return;
+                    }
 
                     result._blocks[resultIndex] = 1;
                     result._length++;
@@ -733,16 +749,27 @@ public static void Multiply(scoped ref BigInteger lhs, uint value, out BigIntege
                     index++;
                 }
 
+                int resultLength = lhsLength;
+
                 if (carry != 0)
                 {
-                    Debug.Assert(unchecked((uint)(lhsLength)) + 1 <= MaxBlockCount);
+                    Debug.Assert(unchecked((uint)(resultLength)) < MaxBlockCount);
+
+                    if (unchecked((uint)(resultLength)) >= MaxBlockCount)
+                    {
+                        // We shouldn't reach here, and the above assert will help flag this
+                        // during testing, but we'll ensure that we return a safe value of
+                        // zero in the case we end up overflowing in any way.
+
+                        SetZero(out result);
+                        return;
+                    }
+
                     result._blocks[index] = carry;
-                    result._length = (lhsLength + 1);
-                }
-                else
-                {
-                    result._length = lhsLength;
+                    resultLength += 1;
                 }
+
+                result._length = resultLength;
             }
 
             public static void Multiply(scoped ref BigInteger lhs, scoped ref BigInteger rhs, out BigInteger result)
@@ -777,6 +804,16 @@ public static void Multiply(scoped ref BigInteger lhs, scoped ref BigInteger rhs
                 int maxResultLength = smallLength + largeLength;
                 Debug.Assert(unchecked((uint)(maxResultLength)) <= MaxBlockCount);
 
+                if (unchecked((uint)(maxResultLength)) > MaxBlockCount)
+                {
+                    // We shouldn't reach here, and the above assert will help flag this
+                    // during testing, but we'll ensure that we return a safe value of
+                    // zero in the case we end up overflowing in any way.
+
+                    SetZero(out result);
+                    return;
+                }
+
                 // Zero out result internal blocks.
                 result._length = maxResultLength;
                 result.Clear((uint)maxResultLength);
@@ -822,7 +859,19 @@ public static void Pow2(uint exponent, out BigInteger result)
             {
                 uint blocksToShift = DivRem32(exponent, out uint remainingBitsToShift);
                 result._length = (int)blocksToShift + 1;
+
                 Debug.Assert(unchecked((uint)result._length) <= MaxBlockCount);
+
+                if (unchecked((uint)result._length) > MaxBlockCount)
+                {
+                    // We shouldn't reach here, and the above assert will help flag this
+                    // during testing, but we'll ensure that we return a safe value of
+                    // zero in the case we end up overflowing in any way.
+
+                    SetZero(out result);
+                    return;
+                }
+
                 if (blocksToShift > 0)
                 {
                     result.Clear(blocksToShift);
@@ -1012,7 +1061,18 @@ public void Add(uint value)
                     }
                 }
 
-                Debug.Assert(unchecked((uint)(length)) + 1 <= MaxBlockCount);
+                Debug.Assert(unchecked((uint)(length)) < MaxBlockCount);
+
+                if (unchecked((uint)(length)) >= MaxBlockCount)
+                {
+                    // We shouldn't reach here, and the above assert will help flag this
+                    // during testing, but we'll ensure that we return a safe value of
+                    // zero in the case we end up overflowing in any way.
+
+                    SetZero(out this);
+                    return;
+                }
+
                 _blocks[length] = 1;
                 _length = length + 1;
             }
@@ -1074,9 +1134,20 @@ public void Multiply10()
 
                 if (carry != 0)
                 {
-                    Debug.Assert(unchecked((uint)(_length)) + 1 <= MaxBlockCount);
+                    Debug.Assert(unchecked((uint)(length)) < MaxBlockCount);
+
+                    if (unchecked((uint)(length)) >= MaxBlockCount)
+                    {
+                        // We shouldn't reach here, and the above assert will help flag this
+                        // during testing, but we'll ensure that we return a safe value of
+                        // zero in the case we end up overflowing in any way.
+
+                        SetZero(out this);
+                        return;
+                    }
+
                     _blocks[index] = (uint)carry;
-                    _length++;
+                    _length = length + 1;
                 }
             }
 
@@ -1152,7 +1223,17 @@ public void ShiftLeft(uint shift)
                 // Check if the shift is block aligned
                 if (remainingBitsToShift == 0)
                 {
-                    Debug.Assert(writeIndex < MaxBlockCount);
+                    Debug.Assert(unchecked((uint)(length)) < MaxBlockCount);
+
+                    if (unchecked((uint)(length)) >= MaxBlockCount)
+                    {
+                        // We shouldn't reach here, and the above assert will help flag this
+                        // during testing, but we'll ensure that we return a safe value of
+                        // zero in the case we end up overflowing in any way.
+
+                        SetZero(out this);
+                        return;
+                    }
 
                     while (readIndex >= 0)
                     {
@@ -1169,8 +1250,19 @@ public void ShiftLeft(uint shift)
                 else
                 {
                     // We need an extra block for the partial shift
+
                     writeIndex++;
-                    Debug.Assert(writeIndex < MaxBlockCount);
+                    Debug.Assert(unchecked((uint)(length)) < MaxBlockCount);
+
+                    if (unchecked((uint)(length)) >= MaxBlockCount)
+                    {
+                        // We shouldn't reach here, and the above assert will help flag this
+                        // during testing, but we'll ensure that we return a safe value of
+                        // zero in the case we end up overflowing in any way.
+
+                        SetZero(out this);
+                        return;
+                    }
 
                     // Set the length to hold the shifted blocks
                     _length = writeIndex + 1;

src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X509Pal.macOS.cs

@@ -111,6 +111,7 @@ private static DSA DecodeDsaPublicKey(byte[] encodedKeyValue, byte[] encodedPara
             public X509ContentType GetCertContentType(ReadOnlySpan<byte> rawData)
             {
                 const int errSecUnknownFormat = -25257;
+
                 if (rawData.IsEmpty)
                 {
                     // Throw to match Windows and Unix behavior.
@@ -119,7 +120,7 @@ public X509ContentType GetCertContentType(ReadOnlySpan<byte> rawData)
 
                 X509ContentType contentType = Interop.AppleCrypto.X509GetContentType(rawData);
 
-                // Apple doesn't seem to recognize PFX files with no MAC, so try a quick maybe-it's-a-PFX test
+                // Apple's native check can't check for PKCS12, so do a quick decode test to see if it is PKCS12 / PFX.
                 if (contentType == X509ContentType.Unknown)
                 {
                     try
@@ -128,9 +129,11 @@ public X509ContentType GetCertContentType(ReadOnlySpan<byte> rawData)
                         {
                             fixed (byte* pin = rawData)
                             {
+                                AsnValueReader reader = new AsnValueReader(rawData, AsnEncodingRules.BER);
+
                                 using (var manager = new PointerMemoryManager<byte>(pin, rawData.Length))
                                 {
-                                    PfxAsn.Decode(manager.Memory, AsnEncodingRules.BER);
+                                    PfxAsn.Decode(ref reader, manager.Memory, out _);
                                 }
 
                                 contentType = X509ContentType.Pkcs12;

src/libraries/System.Security.Cryptography/tests/X509Certificates/PfxIterationCountTests.X509Certificate2.cs

@@ -1,8 +1,13 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using Microsoft.DotNet.RemoteExecutor;
+using Microsoft.DotNet.XUnitExtensions;
+using Xunit;
+
 namespace System.Security.Cryptography.X509Certificates.Tests
 {
+    [SkipOnPlatform(TestPlatforms.Browser, "Browser doesn't support X.509 certificates")]
     public class PfxIterationCountTests_X509Certificate2 : PfxIterationCountTests
     {
         internal override X509Certificate Import(byte[] blob)
@@ -22,5 +27,29 @@ internal override X509Certificate Import(string fileName, string password)
 
         internal override X509Certificate Import(string fileName, SecureString password)
             => new X509Certificate2(fileName, password);
+
+
+        [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
+        public static void Import_IterationCountLimitExceeded_ThrowsInAllottedTime()
+        {
+            const int AllottedTime = 5000;
+
+            if (!PfxTests.Pkcs12PBES2Supported)
+            {
+                throw new SkipTestException("Pkcs12NoPassword100MRounds uses PBES2, which is not supported on this version.");
+            }
+
+            RemoteInvokeOptions options = new()
+            {
+                TimeOut = AllottedTime
+            };
+
+            RemoteExecutor.Invoke(static () =>
+            {
+                byte[] blob = TestData.Pkcs12NoPassword100MRounds;
+                CryptographicException ce = Assert.Throws<CryptographicException>(() => new X509Certificate2(blob));
+                Assert.Contains(FwlinkId, ce.Message);
+            }, options).Dispose();
+        }
     }
 }

src/libraries/System.Security.Cryptography/tests/X509Certificates/TestData.cs

@@ -3437,6 +3437,26 @@ internal static DSAParameters GetDSA1024Params()
             "04020105000420AD0EB570ACFB8357A8E99B17672353CFBA69C76FFE5B6BC113" +
             "05577F12AE24040408D04E60444B79672302030927C1").HexToByteArray();
 
+        internal static readonly byte[] Pkcs12NoPassword100MRounds = Convert.FromBase64String(
+            "MIIDygIBAzCCA4QGCSqGSIb3DQEHAaCCA3UEggNxMIIDbTCCA2kGCSqGSIb3DQEHBqCCA1owggNW" +
+            "AgEAMIIDTwYJKoZIhvcNAQcBMF4GCSqGSIb3DQEFDTBRMDAGCSqGSIb3DQEFDDAjBBCNparJkj/3" +
+            "Uk8N7n0KCMeQAgEBMAwGCCqGSIb3DQILBQAwHQYJYIZIAWUDBAEqBBAcqpBrSDFcXYAWVWKcsEi9" +
+            "gIIC4P/ANdPYWI1vBH1U5sZGMIwLjY96pYaBelyZd0ZfKA8QfGHVNP9+E9hplBKGvRfIMiqmFutj" +
+            "RO4v7Ls8HZEk0hwBt9+6zXPWDJLxBDfSMHUd08+ZAH1yzEqq8aBMyIRVHOQkJFuFuCQJ9Ke5HzVi" +
+            "39S1rgHpnKYFvy+xZAhgI9OO1YxuFt4P9nhlEV/JCoyEQ/2iY99kKc3z7ArrV7BBFhfYGKhWQCBu" +
+            "kAmNBKweRldNWgDuW21WJEl5sByOmyDwpiK55Zxy1K1aIY8DYJTtIzzcX4CILaj6tClMH1G9w4jW" +
+            "BkQI2CG4vCsMl/28BbIP9EyH2C+gBAxvc1N32y3NSvO0/GPVenmQFF9KBMc4FVy4Z21syMKzUkBi" +
+            "PtIbDkcQbGAfyPgFk4SXCgn8OpIIvOOGI50/r+Hj14qex9VIrlwAAWCH8Y+YjwqFAQJYHQpb47zp" +
+            "B1fTwJFOrsXrBgLUzJLZKLR43yW2E9u6b8RsTuFHjh985naCHLuWPYOXS1zduBpHKpwoPUyCwD2r" +
+            "DAokCvA7RCsSXroUkpJarN4CAqsEB8COnzV1Dl2xcAYMerJxrTCKX6WIQUYo0/qeCoqTT38lDAlE" +
+            "7Ydjyx12iVM6eWejAdjORvlVtCQQtCxz8fZpdFGbMP8rf35A8hu++e4u0CLHnhTx38zPIm6H6YfN" +
+            "qj5h1Kz0xLzqnRfa7EGfDEERSHOy/DqNY4nUNG2DTjGOHy1QJelToG7Vo2L7CCZV+leX0nwLNExf" +
+            "hKEp+uQCiYSJe9iDm9fS9VymED79OJbr2bxdq3MggEGksLZv/H0ZT8Wsue0vq9jQ6J6YIEM+DKYr" +
+            "Zt2l4WgTBEKbpqmRvOqYRh9O8Sp+3IRNPzMC2ehzlYXqoPbtG4vxpoRsAMCM/W2x61jbsBSaNSFA" +
+            "eaUwcnKswRg30UonHUAIOJkqtadI57WE/Rat5eHVyya9f7ZN8bTFZjx0BQs6Bo8PK9yfqoidSN8w" +
+            "PTAfMAcGBSsOAwIaBBTt8zpgzygINykjoAwr2GKEywYFwgQUA+L1vfCVASwiE++gTfRgIScMGycC" +
+            "BAX14QA=");
+
         internal static readonly byte[] Pkcs12OpenSslOneCertDefaultEmptyPassword =
             ("308209CF0201033082098506092A864886F70D010701A0820976048209723082" +
             "096E308203E206092A864886F70D010706A08203D3308203CF020100308203C8" +

src/native/libs/System.Security.Cryptography.Native.Apple/pal_x509.c

@@ -70,14 +70,18 @@ PAL_X509ContentType AppleCryptoNative_X509GetContentType(uint8_t* pbData, int32_
     // The sniffing order is:
     // * X509 DER
     // * PKCS7 PEM/DER
-    // * PKCS12 DER (or PEM if Apple has non-standard support for that)
     // * X509 PEM or PEM aggregate (or DER, but that already matched)
     //
     // If the X509 PEM check is done first SecItemImport will erroneously match
     // some PKCS#7 blobs and say they were certificates.
     //
     // Likewise, if the X509 DER check isn't done first, Apple will report it as
     // being a PKCS#7.
+    //
+    // This does not attempt to open a PFX / PKCS12 as Apple does not provide
+    // a suitable API to determine if it is PKCS12 without doing potentially
+    // unbound MAC / KDF work. Instead, let that return Unknown and let the managed
+    // decoding do the check.
     SecCertificateRef certref = SecCertificateCreateWithData(NULL, cfData);
 
     if (certref != NULL)
@@ -104,41 +108,6 @@ PAL_X509ContentType AppleCryptoNative_X509GetContentType(uint8_t* pbData, int32_
         }
     }
 
-    dataFormat = kSecFormatPKCS12;
-    actualFormat = dataFormat;
-    itemType = kSecItemTypeAggregate;
-    actualType = itemType;
-
-    osStatus = SecItemImport(cfData, NULL, &actualFormat, &actualType, 0, NULL, NULL, NULL);
-
-    if (osStatus == errSecPassphraseRequired)
-    {
-        dataFormat = kSecFormatPKCS12;
-        actualFormat = dataFormat;
-        itemType = kSecItemTypeAggregate;
-        actualType = itemType;
-
-        SecItemImportExportKeyParameters importParams;
-        memset(&importParams, 0, sizeof(SecItemImportExportKeyParameters));
-
-        importParams.version = SEC_KEY_IMPORT_EXPORT_PARAMS_VERSION;
-        importParams.passphrase = CFSTR("");
-
-        osStatus = SecItemImport(cfData, NULL, &actualFormat, &actualType, 0, &importParams, NULL, NULL);
-
-        CFRelease(importParams.passphrase);
-        importParams.passphrase = NULL;
-    }
-
-    if (osStatus == noErr || osStatus == errSecPkcs12VerifyFailure)
-    {
-        if (actualType == itemType && actualFormat == dataFormat)
-        {
-            CFRelease(cfData);
-            return PAL_Pkcs12;
-        }
-    }
-
     dataFormat = kSecFormatX509Cert;
     actualFormat = dataFormat;
     itemType = kSecItemTypeCertificate;