Don't delete private keys detected from SerializedCert imports

diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Windows/CertificatePal.Import.cs b/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Windows/CertificatePal.Import.cs
index 3ad280b3eddd6bee1f4b5c3c2bf542efdbdb0615..8c24813b3302dcea22aca6a7314b0e7258c16b58 100644 (file)
--- a/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Windows/CertificatePal.Import.cs
+++ b/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Windows/CertificatePal.Import.cs
@@ -35,7 +35,7 @@ namespace Internal.Cryptography.Pal
             bool loadFromFile = (fileName != null);
 
             PfxCertStoreFlags pfxCertStoreFlags = MapKeyStorageFlags(keyStorageFlags);
-            bool persistKeySet = (0 != (keyStorageFlags & X509KeyStorageFlags.PersistKeySet));
+            bool deleteKeyContainer = false;
 
             CertEncodingType msgAndCertEncodingType;
             ContentType contentType;
@@ -87,9 +87,16 @@ namespace Internal.Cryptography.Pal
                         if (loadFromFile)
                             rawData = File.ReadAllBytes(fileName!);
                         pCertContext = FilterPFXStore(rawData!, password, pfxCertStoreFlags);
+
+                        // If PersistKeySet is set we don't delete the key, so that it persists.
+                        // If EphemeralKeySet is set we don't delete the key, because there's no file, so it's a wasteful call.
+                        const X509KeyStorageFlags DeleteUnless =
+                            X509KeyStorageFlags.PersistKeySet | X509KeyStorageFlags.EphemeralKeySet;
+
+                        deleteKeyContainer = ((keyStorageFlags & DeleteUnless) == 0);
                     }
 
-                    CertificatePal pal = new CertificatePal(pCertContext, deleteKeyContainer: !persistKeySet);
+                    CertificatePal pal = new CertificatePal(pCertContext, deleteKeyContainer);
                     pCertContext = null;
                     return pal;
                 }

diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/tests/CertTests.cs b/src/libraries/System.Security.Cryptography.X509Certificates/tests/CertTests.cs
index 95a9d7499d39c2deabe2a918c438f22bab17006c..f53fa2e31f3aef96bdad62b05361d387f566ef63 100644 (file)
--- a/src/libraries/System.Security.Cryptography.X509Certificates/tests/CertTests.cs
+++ b/src/libraries/System.Security.Cryptography.X509Certificates/tests/CertTests.cs
@@ -372,6 +372,28 @@ namespace System.Security.Cryptography.X509Certificates.Tests
             }
         }
 
+        [Fact]
+        [PlatformSpecific(TestPlatforms.Windows)]
+        public static void SerializedCertDisposeDoesNotRemoveKeyFile()
+        {
+            using (X509Certificate2 fromPfx = new X509Certificate2(TestData.PfxData, TestData.PfxDataPassword))
+            {
+                Assert.True(fromPfx.HasPrivateKey, "fromPfx.HasPrivateKey - before");
+
+                byte[] serializedCert = fromPfx.Export(X509ContentType.SerializedCert);
+
+                using (X509Certificate2 fromSerialized = new X509Certificate2(serializedCert))
+                {
+                    Assert.True(fromSerialized.HasPrivateKey, "fromSerialized.HasPrivateKey");
+                }
+
+                using (RSA key = fromPfx.GetRSAPrivateKey())
+                {
+                    key.SignData(serializedCert, HashAlgorithmName.SHA256, RSASignaturePadding.Pkcs1);
+                }
+            }
+        }
+
         public static IEnumerable<object[]> StorageFlags => CollectionImportTests.StorageFlags;
     }
 }

diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/tests/CollectionTests.cs b/src/libraries/System.Security.Cryptography.X509Certificates/tests/CollectionTests.cs
index f3db79aa9a200c93e50460926f867e1bde5e38de..d280c9b53c941d5420509813624c1ad8ad5a4e8b 100644 (file)
--- a/src/libraries/System.Security.Cryptography.X509Certificates/tests/CollectionTests.cs
+++ b/src/libraries/System.Security.Cryptography.X509Certificates/tests/CollectionTests.cs
@@ -1397,6 +1397,35 @@ namespace System.Security.Cryptography.X509Certificates.Tests
             }
         }
 
+        [Fact]
+        [PlatformSpecific(TestPlatforms.Windows)]
+        public static void SerializedCertDisposeDoesNotRemoveKeyFile()
+        {
+            using (X509Certificate2 fromPfx = new X509Certificate2(TestData.PfxData, TestData.PfxDataPassword))
+            {
+                Assert.True(fromPfx.HasPrivateKey, "fromPfx.HasPrivateKey - before");
+
+                byte[] serializedCert = fromPfx.Export(X509ContentType.SerializedCert);
+
+                X509Certificate2 fromSerialized;
+
+                using (ImportedCollection imported = Cert.Import(serializedCert))
+                {
+                    fromSerialized = imported.Collection[0];
+                    Assert.True(fromSerialized.HasPrivateKey, "fromSerialized.HasPrivateKey");
+                    Assert.NotEqual(IntPtr.Zero, fromSerialized.Handle);
+                }
+
+                // The certificate got disposed by the collection holder.
+                Assert.Equal(IntPtr.Zero, fromSerialized.Handle);
+
+                using (RSA key = fromPfx.GetRSAPrivateKey())
+                {
+                    key.SignData(serializedCert, HashAlgorithmName.SHA256, RSASignaturePadding.Pkcs1);
+                }
+            }
+        }
+
         private static void TestExportSingleCert_SecureStringPassword(X509ContentType ct)
         {
             using (var pfxCer = new X509Certificate2(TestData.PfxData, TestData.CreatePfxDataPasswordSecureString(), Cert.EphemeralIfPossible))