Merged PR 38843: [release/7.0] Fix unbound MAC work in GetCertContentType

Fix unbound MAC work in GetCertContentType

Author: steveharter

src/libraries/System.Security.Cryptography.X509Certificates/tests/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("2233907", ce.Message);
+            }, options).Dispose();
+        }
     }
 }

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

@@ -3372,6 +3372,26 @@ internal static DSAParameters GetDSA1024Params()
             "04020105000420AD0EB570ACFB8357A8E99B17672353CFBA69C76FFE5B6BC113" +
             "05577F12AE24040408D04E60444B79672302030927C1";
 
+        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 const string Pkcs12OpenSslOneCertDefaultEmptyPassword =
             "308209CF0201033082098506092A864886F70D010701A0820976048209723082" +
             "096E308203E206092A864886F70D010706A08203D3308203CF020100308203C8" +

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

@@ -119,7 +119,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 +128,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/native/libs/System.Security.Cryptography.Native.Apple/pal_x509.c

@@ -106,14 +106,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)
@@ -140,41 +144,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;