diff --git a/src/libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.X509.macOS.cs b/src/libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.X509.macOS.cs index 8245a870d782fb..de60db4c936ede 100644 --- a/src/libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.X509.macOS.cs +++ b/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) { diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/AppleCertificatePal.ImportExport.iOS.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/AppleCertificatePal.ImportExport.iOS.cs index d042ade14b0be1..b6150abb135187 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/AppleCertificatePal.ImportExport.iOS.cs +++ b/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(); } } diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/AppleCertificatePal.ImportExport.macOS.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/AppleCertificatePal.ImportExport.macOS.cs index 4e9d87d798c3f1..27c1657eb3a756 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/AppleCertificatePal.ImportExport.macOS.cs +++ b/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 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 password) { Debug.Assert(_identityHandle != null); diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/AppleCertificatePal.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/AppleCertificatePal.cs index fb8c18acdcb39e..969c5e01cc6e1e 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/AppleCertificatePal.cs +++ b/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; diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/StorePal.macOS.LoaderPal.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/StorePal.macOS.LoaderPal.cs index c6f33bbf1ce3ed..fa00011b419c12 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/StorePal.macOS.LoaderPal.cs +++ b/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(); + } } } }