Hold temporary keychain refs for certificates imported with X509Certi…

…ficate2Collection.Import

This corrects the reference counting for temporary keychains when using X509Certificate2Collection.Import,
both releasing a ref that is no longer needed, and adding a ref to the new/target keychain to prevent that
keychain from being deleted too soon.

src/libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.X509.macOS.cs

@@ -264,6 +264,8 @@ internal static SafeSecIdentityHandle X509CopyWithPrivateKey(
                 out identityHandle,
                 out osStatus);
 
+            SafeTemporaryKeychainHandle.TrackItem(identityHandle);
+
             if (result == 1)
             {
                 Debug.Assert(!identityHandle.IsInvalid);
@@ -288,13 +290,25 @@ internal static SafeSecIdentityHandle X509CopyWithPrivateKey(
         {
             SafeSecIdentityHandle identityHandle;
             int osStatus;
+            int result;
 
-            int result = AppleCryptoNative_X509MoveToKeychain(
-                cert,
-                targetKeychain,
-                privateKey ?? SafeSecKeyRefHandle.InvalidHandle,
-                out identityHandle,
-                out osStatus);
+            // Ensure the keychain is not disposed, if there is any associated one
+            using (SafeKeychainHandle keychain = Interop.AppleCrypto.SecKeychainItemCopyKeychain(cert))
+            {
+                // AppleCryptoNative_X509MoveToKeychain can change the keychain of the input
+                // certificate, so we need to reflect that change.
+                SafeTemporaryKeychainHandle.UntrackItem(cert.DangerousGetHandle());
+
+                result = AppleCryptoNative_X509MoveToKeychain(
+                    cert,
+                    targetKeychain,
+                    privateKey ?? SafeSecKeyRefHandle.InvalidHandle,
+                    out identityHandle,
+                    out osStatus);
+
+                SafeTemporaryKeychainHandle.TrackItem(cert);
+                SafeTemporaryKeychainHandle.TrackItem(identityHandle);
+            }
 
             if (result == 0)
             {

src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/AppleCertificatePal.ImportExport.iOS.cs

@@ -157,8 +157,5 @@ public static ICertificatePal FromBlob(
 
             return result ?? FromDerBlob(rawData, GetDerCertContentType(rawData), password, keyStorageFlags);
         }
-
-        // No temporary keychain on iOS
-        partial void DisposeTempKeychain();
     }
 }

src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/AppleCertificatePal.ImportExport.macOS.cs

@@ -12,8 +12,6 @@ namespace System.Security.Cryptography.X509Certificates
 {
     internal sealed partial class AppleCertificatePal : ICertificatePal
     {
-        private SafeKeychainHandle? _tempKeychain;
-
         public static ICertificatePal FromBlob(
             ReadOnlySpan<byte> rawData,
             SafePasswordHandle password,
@@ -53,20 +51,7 @@ public static ICertificatePal FromBlob(
 
                 using (keychain)
                 {
-                    AppleCertificatePal ret = ImportPkcs12(rawData, password, exportable, keychain);
-                    if (!persist)
-                    {
-                        // If we used temporary keychain we need to prevent deletion.
-                        // on 10.15+ if keychain is unlinked, certain certificate operations may fail.
-                        bool success = false;
-                        keychain.DangerousAddRef(ref success);
-                        if (success)
-                        {
-                            ret._tempKeychain = keychain;
-                        }
-                    }
-
-                    return ret;
+                    return ImportPkcs12(rawData, password, exportable, keychain);
                 }
             }
 
@@ -92,11 +77,6 @@ public static ICertificatePal FromBlob(
             throw new CryptographicException();
         }
 
-        public void DisposeTempKeychain()
-        {
-            Interlocked.Exchange(ref _tempKeychain, null)?.Dispose();
-        }
-
         internal unsafe byte[] ExportPkcs8(ReadOnlySpan<char> password)
         {
             Debug.Assert(_identityHandle != null);

src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/AppleCertificatePal.cs

@@ -100,8 +100,6 @@ public void Dispose()
 
             _certHandle = null!;
             _identityHandle = null;
-
-            DisposeTempKeychain();
         }
 
         internal SafeSecCertificateHandle CertificateHandle => _certHandle;

src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/StorePal.macOS.LoaderPal.cs

@@ -97,7 +97,7 @@ public void MoveTo(X509Certificate2Collection collection)
 
                     using (safeSecKeyRefHandle)
                     {
-                        ICertificatePal newPal;
+                        AppleCertificatePal newPal;
 
                         // SecItemImport doesn't seem to respect non-exportable import for PKCS#8,
                         // only PKCS#12.
@@ -119,6 +119,11 @@ public void MoveTo(X509Certificate2Collection collection)
 
                         X509Certificate2 cert = new X509Certificate2(newPal);
                         collection.Add(cert);
+
+                        if (newPal != pal)
+                        {
+                            pal.Dispose();
+                        }
                     }
                 }
             }