Handle empty and disposed certificates in X509Store and X509Chain.
authorFilip Navara <filip.navara@gmail.com>
Sat, 14 Jul 2018 09:48:00 +0000 (11:48 +0200)
committerJeremy Barton <jbarton@microsoft.com>
Sat, 14 Jul 2018 09:48:00 +0000 (02:48 -0700)
* Ignore removing empty or disposed certificates from X509Store.

* Fix X509Chain.Build with empty certificate.

Commit migrated from https://github.com/dotnet/corefx/commit/906725f53f9f84a911bcddc60e72ba7abb8adaf8

src/libraries/System.Security.Cryptography.X509Certificates/src/System/Security/Cryptography/X509Certificates/X509Chain.cs
src/libraries/System.Security.Cryptography.X509Certificates/src/System/Security/Cryptography/X509Certificates/X509Store.cs
src/libraries/System.Security.Cryptography.X509Certificates/tests/ChainTests.cs
src/libraries/System.Security.Cryptography.X509Certificates/tests/X509StoreTests.cs

index 870fc9a..ae9a279 100644 (file)
@@ -109,7 +109,7 @@ namespace System.Security.Cryptography.X509Certificates
         {
             lock (_syncRoot)
             {
-                if (certificate == null)
+                if (certificate == null || certificate.Pal == null)
                     throw new ArgumentException(SR.Cryptography_InvalidContextHandle, nameof(certificate));
 
                 Reset();
index 011dbe9..f816865 100644 (file)
@@ -154,7 +154,7 @@ namespace System.Security.Cryptography.X509Certificates
             if (_storePal == null)
                 throw new CryptographicException(SR.Cryptography_X509_StoreNotOpen);
 
-            if (certificate.Handle == IntPtr.Zero)
+            if (certificate.Pal == null)
                 throw new CryptographicException(SR.Cryptography_InvalidHandle, "pCertContext");
 
             _storePal.Add(certificate.Pal);
@@ -194,6 +194,9 @@ namespace System.Security.Cryptography.X509Certificates
             if (_storePal == null)
                 throw new CryptographicException(SR.Cryptography_X509_StoreNotOpen);
 
+            if (certificate.Pal == null)
+                return;
+
             _storePal.Remove(certificate.Pal);
         }
 
index 9a5258e..83f65cd 100644 (file)
@@ -633,6 +633,16 @@ namespace System.Security.Cryptography.X509Certificates.Tests
         }
 
         [Fact]
+        public static void BuildChainInvalidValues()
+        {
+            using (var chain = X509Chain.Create())
+            {
+                AssertExtensions.Throws<ArgumentException>("certificate", () => chain.Build(null));
+                AssertExtensions.Throws<ArgumentException>("certificate", () => chain.Build(new X509Certificate2()));
+            }
+        }
+
+        [Fact]
         public static void InvalidSelfSignedSignature()
         {
             X509ChainStatusFlags expectedFlags;
index fe2db40..d6393a0 100644 (file)
@@ -309,6 +309,18 @@ namespace System.Security.Cryptography.X509Certificates.Tests
             }
         }
 
+        [Fact]
+        public static void RemoveDisposedIsIgnored()
+        {
+            using (X509Store store = new X509Store(StoreName.My, StoreLocation.CurrentUser))
+            using (X509Certificate2 cert = new X509Certificate2(TestData.MsCertificate))
+            {
+                store.Open(OpenFlags.ReadWrite);
+                cert.Dispose();
+                store.Remove(cert);
+            }
+        }
+
         /* Placeholder information for these tests until they can be written to run reliably.
          * Currently such tests would create physical files (Unix) and\or certificates (Windows)
          * which can collide with other running tests that use the same cert, or from a