Improve performance of Linux X509Chain building via caching
authorJeremy Barton <jbarton@microsoft.com>
Thu, 16 May 2019 23:56:35 +0000 (16:56 -0700)
committerGitHub <noreply@github.com>
Thu, 16 May 2019 23:56:35 +0000 (16:56 -0700)
X509Chain will use cached versions of CU\My, CU\CA, and CU\Root, in addition to the already cached LM\CA and LM\Root stores.

By avoiding the dips down to the filesystem it knocks a significant amount of I/O and computation off of the store reloads (~45% of one of my perf tests was spent doing SHA-1... for verifying the HMAC on opening the CU\My files).

The new heuristics are:

* CU\My, CU\CA, CU\Root
  * After one second elapses check to see if the LastWriteTimeUtc on the directory has changed, if so, invalidate the cache.
  * After 30 seconds invalidate the cache no matter what.
  * Cache invalidation is per store
* LM\CA, LM\Root
  * After 5 seconds elapses check to see if the LastWriteTimeUtc on either (or both) of the SSL_CERT_FILE file or SSL_CERT_DIR directory has changed (and if so, invalidate the cache)
  * After 5 minutes invalidate the cache no matter what.
  * System stores are rebuilt together.

All X509Store objects for LM\CA and LM\Root will get the same Pal instance (which just no-ops the Dispose) which is a projection over the cache.

All X509Store objects for CU\ stores will still use the existing "raw read" model, because otherwise the cache led to observable differences between Windows and Linux. Only X509Chain gets the cached stores.

In running a chain-build-only 1000 times in a loop test, before this change my machine reported ~13.6ms per chain build, and after was 0.822ms.

In running a particular SslStream-to-SslStream handshake test, 10000 iterations before the change was 7 minutes 51 seconds. After was 3 minutes 40 seconds.

This change also fixes a bug introduced in the previous perf enhancement where ERR_get_error was used instead of ERR_peek_last_error (wrong end of the queue, and destructive vs passive)... and that the managed code wasn't checking for the error anyways.

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

src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.X509.cs
src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_x509.c
src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_x509.h
src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Unix/CachedDirectoryStoreProvider.cs [new file with mode: 0644]
src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Unix/CachedSystemStoreProvider.cs [new file with mode: 0644]
src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Unix/CollectionBackedStoreProvider.cs [deleted file]
src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Unix/OpenSslX509ChainProcessor.cs
src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Unix/StorePal.cs
src/libraries/System.Security.Cryptography.X509Certificates/src/System.Security.Cryptography.X509Certificates.csproj

index b17af46..9c87d0b 100644 (file)
@@ -127,8 +127,20 @@ internal static partial class Interop
         [return: MarshalAs(UnmanagedType.Bool)]
         internal static extern bool X509ExtensionGetCritical(IntPtr ex);
 
-        [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_X509ChainNew")]
-        internal static extern SafeX509StoreHandle X509ChainNew(SafeX509StackHandle systemTrust, string userTrustPath);
+        [DllImport(Libraries.CryptoNative)]
+        private static extern SafeX509StoreHandle CryptoNative_X509ChainNew(SafeX509StackHandle systemTrust, SafeX509StackHandle userTrust);
+
+        internal static SafeX509StoreHandle X509ChainNew(SafeX509StackHandle systemTrust, SafeX509StackHandle userTrust)
+        {
+            SafeX509StoreHandle store = CryptoNative_X509ChainNew(systemTrust, userTrust);
+
+            if (store.IsInvalid)
+            {
+                throw CreateOpenSslCryptographicException();
+            }
+
+            return store;
+        }
 
         [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_X509StoreDestory")]
         internal static extern void X509StoreDestory(IntPtr v);
index 59ed2a5..afda385 100644 (file)
@@ -480,7 +480,7 @@ static X509* ReadNextPublicCert(DIR* dir, X509Stack* tmpStack, char* pathTmp, si
     return NULL;
 }
 
-X509_STORE* CryptoNative_X509ChainNew(X509Stack* systemTrust, const char* userTrustPath)
+X509_STORE* CryptoNative_X509ChainNew(X509Stack* systemTrust, X509Stack* userTrust)
 {
     X509_STORE* store = X509_STORE_new();
 
@@ -503,56 +503,30 @@ X509_STORE* CryptoNative_X509ChainNew(X509Stack* systemTrust, const char* userTr
         }
     }
 
-    if (userTrustPath != NULL)
+
+    if (userTrust != NULL)
     {
-        char* pathTmp;
-        size_t pathTmpSize;
-        char* nextFileWrite;
-        DIR* trustDir = OpenUserStore(userTrustPath, &pathTmp, &pathTmpSize, &nextFileWrite);
+        int count = sk_X509_num(userTrust);
+        int clearError = 0;
 
-        if (trustDir != NULL)
+        for (int i = 0; i < count; i++)
         {
-            X509* cert;
-            X509Stack* tmpStack = sk_X509_new_null();
-
-            while ((cert = ReadNextPublicCert(trustDir, tmpStack, pathTmp, pathTmpSize, nextFileWrite)) != NULL)
+            if (!X509_STORE_add_cert(store, sk_X509_value(userTrust, i)))
             {
-                // cert refcount is 1
-                if (!X509_STORE_add_cert(store, cert))
+                unsigned long error = ERR_peek_last_error();
+
+                if (error != ERR_PACK(ERR_LIB_X509, X509_F_X509_STORE_ADD_CERT, X509_R_CERT_ALREADY_IN_HASH_TABLE))
                 {
-                    // cert refcount is still 1
-                    if (ERR_get_error() !=
-                        ERR_PACK(ERR_LIB_X509, X509_F_X509_STORE_ADD_CERT, X509_R_CERT_ALREADY_IN_HASH_TABLE))
-                    {
-                        // cert refcount goes to 0
-                        X509_free(cert);
-                        X509_STORE_free(store);
-                        store = NULL;
-                        break;
-                    }
+                    X509_STORE_free(store);
+                    return NULL;
                 }
 
-                // if add_cert succeeded, reduce refcount to 1
-                // if add_cert failed (duplicate add), reduce refcount to 0
-                X509_free(cert);
-            }
-
-            sk_X509_free(tmpStack);
-            free(pathTmp);
-            closedir(trustDir);
-
-            // store is only NULL if X509_STORE_add_cert failed, in which case we
-            // want to leave the error state intact, so the exception will report
-            // what went wrong (probably out of memory).
-            if (store == NULL)
-            {
-                return NULL;
+                clearError = 1;
             }
+        }
 
-            // PKCS12_parse can cause spurious errors.
-            // d2i_PKCS12_fp may have failed for invalid files.
-            // X509_STORE_add_cert may have reported duplicate addition.
-            // Just clear it all.
+        if (clearError)
+        {
             ERR_clear_error();
         }
     }
index cf25d05..fa66f59 100644 (file)
@@ -337,10 +337,9 @@ Returns the input value.
 DLLEXPORT X509* CryptoNative_X509UpRef(X509* x509);
 
 /*
-Create a new X509_STORE, considering the certificates from systemTrust and any readable PFX
-in userTrustPath to be trusted
+Create a new X509_STORE, considering the certificates from systemTrust and userTrust
 */
-DLLEXPORT X509_STORE* CryptoNative_X509ChainNew(X509Stack* systemTrust, const char* userTrustPath);
+DLLEXPORT X509_STORE* CryptoNative_X509ChainNew(X509Stack* systemTrust, X509Stack* userTrust);
 
 /*
 Adds all of the simple certificates from null-or-empty-password PFX files in storePath to stack.
diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Unix/CachedDirectoryStoreProvider.cs b/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Unix/CachedDirectoryStoreProvider.cs
new file mode 100644 (file)
index 0000000..cbf5f5a
--- /dev/null
@@ -0,0 +1,77 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using System;
+using System.Diagnostics;
+using System.IO;
+using Microsoft.Win32.SafeHandles;
+
+namespace Internal.Cryptography.Pal
+{
+    internal sealed class CachedDirectoryStoreProvider
+    {
+        // These intervals are mostly arbitrary.
+        // Prior to this caching these stores were always read "hot" from disk, and 30 seconds
+        // seems like "long enough" for performance gains with "short enough" that if the filesystem
+        // has LastWrite updating disabled that the process will be mostly responsive.
+        private static readonly TimeSpan s_lastWriteRecheckInterval = TimeSpan.FromSeconds(1);
+        private static readonly TimeSpan s_assumeInvalidInterval = TimeSpan.FromSeconds(30);
+
+        private readonly Stopwatch _recheckStopwatch = new Stopwatch();
+        private readonly DirectoryInfo _storeDirectoryInfo;
+
+        private SafeX509StackHandle _nativeCollection;
+        private DateTime _loadLastWrite;
+
+        internal CachedDirectoryStoreProvider(string storeName)
+        {
+            string storePath = DirectoryBasedStoreProvider.GetStorePath(storeName);
+            _storeDirectoryInfo = new DirectoryInfo(storePath);
+        }
+
+        internal SafeX509StackHandle GetNativeCollection()
+        {
+            SafeX509StackHandle ret = _nativeCollection;
+
+            TimeSpan elapsed = _recheckStopwatch.Elapsed;
+
+            if (ret == null || elapsed >= s_lastWriteRecheckInterval)
+            {
+                lock (_recheckStopwatch)
+                {
+                    _storeDirectoryInfo.Refresh();
+                    DirectoryInfo info = _storeDirectoryInfo;
+
+                    if (ret == null ||
+                        elapsed >= s_assumeInvalidInterval ||
+                        (info.Exists && info.LastWriteTimeUtc != _loadLastWrite))
+                    {
+                        SafeX509StackHandle newColl = Interop.Crypto.NewX509Stack();
+                        Interop.Crypto.CheckValidOpenSslHandle(newColl);
+
+                        if (info.Exists)
+                        {
+                            Interop.Crypto.X509StackAddDirectoryStore(newColl, info.FullName);
+                            _loadLastWrite = info.LastWriteTimeUtc;
+                        }
+
+
+                        // The existing collection is not Disposed here, intentionally.
+                        // It could be in the gap between when they are returned from this method and
+                        // not yet used in a P/Invoke, which would result in an exception being thrown.
+                        // In order to maintain "finalization-free" this method would need to always
+                        // DangerousAddRef, and the callers would need to DangerousRelease,
+                        // adding more interlocked operations on every call.
+                        ret = newColl;
+                        _nativeCollection = newColl;
+                        _recheckStopwatch.Restart();
+                    }
+                }
+            }
+
+            Debug.Assert(ret != null);
+            return ret;
+        }
+    }
+}
diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Unix/CachedSystemStoreProvider.cs b/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Unix/CachedSystemStoreProvider.cs
new file mode 100644 (file)
index 0000000..863586f
--- /dev/null
@@ -0,0 +1,300 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using System;
+using System.Collections.Generic;
+using System.Diagnostics;
+using System.IO;
+using System.Linq;
+using System.Runtime.InteropServices;
+using System.Security.Cryptography.X509Certificates;
+using System.Threading;
+using Microsoft.Win32.SafeHandles;
+
+namespace Internal.Cryptography.Pal
+{
+    internal sealed class CachedSystemStoreProvider : IStorePal
+    {
+        // These intervals are mostly arbitrary.
+        // Prior to this refreshing cache the system collections were read just once per process, on the
+        // assumption that system trust changes would happen before the process start (or would come
+        // followed by a reboot for a kernel update, etc).
+        // Customers requested something more often than "never" and 5 minutes seems like a reasonable
+        // balance.
+        //
+        // Note that on Ubuntu the LastWrite test always fails, because the system default for SSL_CERT_DIR
+        // is a symlink, so the LastWrite value is always just when the symlink was created (and Ubuntu does
+        // not provide the single-file version at SSL_CERT_FILE, so the file update does not trigger) --
+        // meaning the "assume invalid" interval is Ubuntu's only refresh.
+        private static readonly TimeSpan s_lastWriteRecheckInterval = TimeSpan.FromSeconds(5);
+        private static readonly TimeSpan s_assumeInvalidInterval = TimeSpan.FromMinutes(5);
+        private static readonly Stopwatch s_recheckStopwatch = new Stopwatch();
+        private static readonly DirectoryInfo s_rootStoreDirectoryInfo = SafeOpenRootDirectoryInfo();
+        private static readonly FileInfo s_rootStoreFileInfo = SafeOpenRootFileInfo();
+
+        // Use non-Value-Tuple so that it's an atomic update.
+        private static Tuple<SafeX509StackHandle,SafeX509StackHandle> s_nativeCollections;
+        private static DateTime s_directoryCertsLastWrite;
+        private static DateTime s_fileCertsLastWrite;
+
+        private readonly bool _isRoot;
+
+        private CachedSystemStoreProvider(bool isRoot)
+        {
+            _isRoot = isRoot;
+        }
+
+        internal static CachedSystemStoreProvider MachineRoot { get; } =
+            new CachedSystemStoreProvider(true);
+
+        internal static CachedSystemStoreProvider MachineIntermediate { get; } =
+            new CachedSystemStoreProvider(false);
+
+
+        public void Dispose()
+        {
+            // No-op
+        }
+
+        public void CloneTo(X509Certificate2Collection collection)
+        {
+            Tuple<SafeX509StackHandle, SafeX509StackHandle> nativeColls = GetCollections();
+            SafeX509StackHandle nativeColl = _isRoot ? nativeColls.Item1 : nativeColls.Item2;
+
+            int count = Interop.Crypto.GetX509StackFieldCount(nativeColl);
+
+            for (int i = 0; i < count; i++)
+            {
+                X509Certificate2 clone = new X509Certificate2(Interop.Crypto.GetX509StackField(nativeColl, i));
+                collection.Add(clone);
+            }
+        }
+
+        internal static void GetNativeCollections(out SafeX509StackHandle root, out SafeX509StackHandle intermediate)
+        {
+            Tuple<SafeX509StackHandle, SafeX509StackHandle> nativeColls = GetCollections();
+            root = nativeColls.Item1;
+            intermediate = nativeColls.Item2;
+        }
+
+        public void Add(ICertificatePal cert)
+        {
+            // These stores can only be opened in ReadOnly mode.
+            throw new InvalidOperationException();
+        }
+
+        public void Remove(ICertificatePal cert)
+        {
+            // These stores can only be opened in ReadOnly mode.
+            throw new InvalidOperationException();
+        }
+
+        public SafeHandle SafeHandle => null;
+
+        private static Tuple<SafeX509StackHandle, SafeX509StackHandle> GetCollections()
+        {
+            TimeSpan elapsed = s_recheckStopwatch.Elapsed;
+            Tuple<SafeX509StackHandle, SafeX509StackHandle> ret = s_nativeCollections;
+
+            if (ret == null || elapsed > s_lastWriteRecheckInterval)
+            {
+                lock (s_recheckStopwatch)
+                {
+                    FileInfo fileInfo = s_rootStoreFileInfo;
+                    DirectoryInfo dirInfo = s_rootStoreDirectoryInfo;
+
+                    fileInfo?.Refresh();
+                    dirInfo?.Refresh();
+
+                    if (ret == null ||
+                        elapsed > s_assumeInvalidInterval ||
+                        (fileInfo != null && fileInfo.Exists && fileInfo.LastWriteTimeUtc != s_fileCertsLastWrite) ||
+                        (dirInfo != null && dirInfo.Exists && dirInfo.LastWriteTimeUtc != s_directoryCertsLastWrite))
+                    {
+                        ret = LoadMachineStores(dirInfo, fileInfo);
+                    }
+                }
+            }
+
+            Debug.Assert(ret != null);
+            return ret;
+        }
+
+        private static Tuple<SafeX509StackHandle, SafeX509StackHandle> LoadMachineStores(
+            DirectoryInfo rootStorePath,
+            FileInfo rootStoreFile)
+        {
+            Debug.Assert(
+                Monitor.IsEntered(s_recheckStopwatch),
+                "LoadMachineStores assumes a lock(s_recheckStopwatch)");
+
+            IEnumerable<FileInfo> trustedCertFiles;
+            DateTime newFileTime = default;
+            DateTime newDirTime = default;
+
+            if (rootStorePath != null && rootStorePath.Exists)
+            {
+                trustedCertFiles = rootStorePath.EnumerateFiles();
+                newDirTime = rootStorePath.LastWriteTimeUtc;
+            }
+            else
+            {
+                trustedCertFiles = Array.Empty<FileInfo>();
+            }
+
+            if (rootStoreFile != null && rootStoreFile.Exists)
+            {
+                trustedCertFiles = trustedCertFiles.Prepend(rootStoreFile);
+                newFileTime = rootStoreFile.LastWriteTimeUtc;
+            }
+
+            SafeX509StackHandle rootStore = Interop.Crypto.NewX509Stack();
+            Interop.Crypto.CheckValidOpenSslHandle(rootStore);
+            SafeX509StackHandle intermedStore = Interop.Crypto.NewX509Stack();
+            Interop.Crypto.CheckValidOpenSslHandle(intermedStore);
+
+            HashSet<X509Certificate2> uniqueRootCerts = new HashSet<X509Certificate2>();
+            HashSet<X509Certificate2> uniqueIntermediateCerts = new HashSet<X509Certificate2>();
+
+            foreach (FileInfo file in trustedCertFiles)
+            {
+                using (SafeBioHandle fileBio = Interop.Crypto.BioNewFile(file.FullName, "rb"))
+                {
+                    // The handle may be invalid, for example when we don't have read permission for the file.
+                    if (fileBio.IsInvalid)
+                    {
+                        Interop.Crypto.ErrClearError();
+                        continue;
+                    }
+
+                    ICertificatePal pal;
+
+                    // Some distros ship with two variants of the same certificate.
+                    // One is the regular format ('BEGIN CERTIFICATE') and the other
+                    // contains additional AUX-data ('BEGIN TRUSTED CERTIFICATE').
+                    // The additional data contains the appropriate usage (e.g. emailProtection, serverAuth, ...).
+                    // Because corefx doesn't validate for a specific usage, derived certificates are rejected.
+                    // For now, we skip the certificates with AUX data and use the regular certificates.
+                    while (OpenSslX509CertificateReader.TryReadX509PemNoAux(fileBio, out pal) ||
+                        OpenSslX509CertificateReader.TryReadX509Der(fileBio, out pal))
+                    {
+                        X509Certificate2 cert = new X509Certificate2(pal);
+
+                        // The HashSets are just used for uniqueness filters, they do not survive this method.
+                        if (StringComparer.Ordinal.Equals(cert.Subject, cert.Issuer))
+                        {
+                            if (uniqueRootCerts.Add(cert))
+                            {
+                                using (SafeX509Handle tmp = Interop.Crypto.X509UpRef(pal.Handle))
+                                {
+                                    if (!Interop.Crypto.PushX509StackField(rootStore, tmp))
+                                    {
+                                        throw Interop.Crypto.CreateOpenSslCryptographicException();
+                                    }
+
+                                    // The ownership has been transferred to the stack
+                                    tmp.SetHandleAsInvalid();
+                                }
+
+                                continue;
+                            }
+                        }
+                        else
+                        {
+                            if (uniqueIntermediateCerts.Add(cert))
+                            {
+                                using (SafeX509Handle tmp = Interop.Crypto.X509UpRef(pal.Handle))
+                                {
+                                    if (!Interop.Crypto.PushX509StackField(intermedStore, tmp))
+                                    {
+                                        throw Interop.Crypto.CreateOpenSslCryptographicException();
+                                    }
+
+                                    // The ownership has been transferred to the stack
+                                    tmp.SetHandleAsInvalid();
+                                }
+
+                                continue;
+                            }
+                        }
+
+                        // There's a good chance we'll encounter duplicates on systems that have both one-cert-per-file
+                        // and one-big-file trusted certificate stores. Anything that wasn't unique will end up here.
+                        cert.Dispose();
+                    }
+                }
+            }
+
+            foreach (X509Certificate2 cert in uniqueRootCerts)
+            {
+                cert.Dispose();
+            }
+
+            foreach (X509Certificate2 cert in uniqueIntermediateCerts)
+            {
+                cert.Dispose();
+            }
+
+            Tuple<SafeX509StackHandle, SafeX509StackHandle> newCollections =
+                Tuple.Create(rootStore, intermedStore);
+
+            Debug.Assert(
+                Monitor.IsEntered(s_recheckStopwatch),
+                "LoadMachineStores assumes a lock(s_recheckStopwatch)");
+
+            // The existing collections are not Disposed here, intentionally.
+            // They could be in the gap between when they are returned from this method and not yet used
+            // in a P/Invoke, which would result in exceptions being thrown.
+            // In order to maintain "finalization-free" the GetNativeCollections method would need to
+            // DangerousAddRef, and the callers would need to DangerousRelease, adding more interlocked operations
+            // on every call.
+
+            Volatile.Write(ref s_nativeCollections, newCollections);
+            s_directoryCertsLastWrite = newDirTime;
+            s_fileCertsLastWrite = newFileTime;
+            s_recheckStopwatch.Restart();
+            return newCollections;
+        }
+
+        private static FileInfo SafeOpenRootFileInfo()
+        {
+            string rootFile = Interop.Crypto.GetX509RootStoreFile();
+
+            if (!string.IsNullOrEmpty(rootFile))
+            {
+                try
+                {
+                    return new FileInfo(rootFile);
+                }
+                catch (ArgumentException)
+                {
+                    // If SSL_CERT_FILE is set to the empty string, or anything else which gives
+                    // "The path is not of a legal form", then the GetX509RootStoreFile value is ignored.
+                }
+            }
+
+            return null;
+        }
+
+        private static DirectoryInfo SafeOpenRootDirectoryInfo()
+        {
+            string rootDirectory = Interop.Crypto.GetX509RootStorePath();
+
+            if (!string.IsNullOrEmpty(rootDirectory))
+            {
+                try
+                {
+                    return new DirectoryInfo(rootDirectory);
+                }
+                catch (ArgumentException)
+                {
+                    // If SSL_CERT_DIR is set to the empty string, or anything else which gives
+                    // "The path is not of a legal form", then the GetX509RootStoreFile value is ignored.
+                }
+            }
+
+            return null;
+        }
+    }
+}
diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Unix/CollectionBackedStoreProvider.cs b/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Unix/CollectionBackedStoreProvider.cs
deleted file mode 100644 (file)
index df6c507..0000000
+++ /dev/null
@@ -1,93 +0,0 @@
-// Licensed to the .NET Foundation under one or more agreements.
-// The .NET Foundation licenses this file to you under the MIT license.
-// See the LICENSE file in the project root for more information.
-
-using System;
-using System.Collections.Generic;
-using System.Diagnostics;
-using System.Runtime.InteropServices;
-using System.Security.Cryptography.X509Certificates;
-using Microsoft.Win32.SafeHandles;
-
-namespace Internal.Cryptography.Pal
-{
-    internal sealed class CollectionBackedStoreProvider : IStorePal
-    {
-        private readonly List<X509Certificate2> _certs;
-        private SafeX509StackHandle _nativeCollection;
-
-        internal CollectionBackedStoreProvider(List<X509Certificate2> certs)
-        {
-            _certs = certs;
-        }
-
-        public void Dispose()
-        {
-            // Dispose is explicitly doing nothing here.
-            // The LM\Root and LM\CA stores are reused after being Disposed, because there's no
-            // point in re-allocating the array to instantiate them every time.
-            // But the interface requires Dispose.
-        }
-
-        public void CloneTo(X509Certificate2Collection collection)
-        {
-            Debug.Assert(collection != null);
-            Debug.Assert(_certs != null);
-
-            foreach (X509Certificate2 cert in _certs)
-            {
-                var certPal = (OpenSslX509CertificateReader)cert.Pal;
-                collection.Add(new X509Certificate2(certPal.DuplicateHandles()));
-            }
-        }
-
-        public void Add(ICertificatePal cert)
-        {
-            throw new InvalidOperationException();
-        }
-
-        public void Remove(ICertificatePal cert)
-        {
-            throw new InvalidOperationException();
-        }
-
-        SafeHandle IStorePal.SafeHandle
-        {
-            get { return null; }
-        }
-
-        internal SafeX509StackHandle GetNativeCollection()
-        {
-            if (_nativeCollection == null)
-            {
-                lock (_certs)
-                {
-                    if (_nativeCollection == null)
-                    {
-                        SafeX509StackHandle nativeCollection = Interop.Crypto.NewX509Stack();
-
-                        foreach (X509Certificate2 cert in _certs)
-                        {
-                            var certPal = (OpenSslX509CertificateReader)cert.Pal;
-
-                            using (SafeX509Handle tmp = Interop.Crypto.X509UpRef(certPal.SafeHandle))
-                            {
-                                if (!Interop.Crypto.PushX509StackField(nativeCollection, tmp))
-                                {
-                                    throw Interop.Crypto.CreateOpenSslCryptographicException();
-                                }
-
-                                // Ownership was transferred to the cert stack.
-                                tmp.SetHandleAsInvalid();
-                            }
-                        }
-
-                        _nativeCollection = nativeCollection;
-                    }
-                }
-            }
-
-            return _nativeCollection;
-        }
-    }
-}
index 04b1099..8037861 100644 (file)
@@ -20,14 +20,14 @@ namespace Internal.Cryptography.Pal
         // 10 is plenty big.
         private const int DefaultChainCapacity = 10;
 
-        private static readonly string s_userRootPath =
-            DirectoryBasedStoreProvider.GetStorePath(X509Store.RootStoreName);
+        private static readonly CachedDirectoryStoreProvider s_userRootStore =
+            new CachedDirectoryStoreProvider(X509Store.RootStoreName);
 
-        private static readonly string s_userIntermediatePath =
-            DirectoryBasedStoreProvider.GetStorePath(X509Store.IntermediateCAStoreName);
+        private static readonly CachedDirectoryStoreProvider s_userIntermediateStore =
+            new CachedDirectoryStoreProvider(X509Store.IntermediateCAStoreName);
 
-        private static readonly string s_userPersonalPath =
-            DirectoryBasedStoreProvider.GetStorePath(X509Store.MyStoreName);
+        private static readonly CachedDirectoryStoreProvider s_userPersonalStore =
+            new CachedDirectoryStoreProvider(X509Store.MyStoreName);
 
         private SafeX509Handle _leafHandle;
         private SafeX509StoreHandle _store;
@@ -82,8 +82,9 @@ namespace Internal.Cryptography.Pal
             DateTime verificationTime,
             TimeSpan remainingDownloadTime)
         {
-            SafeX509StackHandle systemTrust = StorePal.GetMachineRoot().GetNativeCollection();
-            SafeX509StackHandle systemIntermediate = StorePal.GetMachineIntermediate().GetNativeCollection();
+            CachedSystemStoreProvider.GetNativeCollections(
+                out SafeX509StackHandle systemTrust,
+                out SafeX509StackHandle systemIntermediate);
 
             SafeX509StoreHandle store = null;
             SafeX509StackHandle untrusted = null;
@@ -91,11 +92,11 @@ namespace Internal.Cryptography.Pal
 
             try
             {
-                store = Interop.Crypto.X509ChainNew(systemTrust, s_userRootPath);
+                store = Interop.Crypto.X509ChainNew(systemTrust, s_userRootStore.GetNativeCollection());
 
                 untrusted = Interop.Crypto.NewX509Stack();
-                Interop.Crypto.X509StackAddDirectoryStore(untrusted, s_userIntermediatePath);
-                Interop.Crypto.X509StackAddDirectoryStore(untrusted, s_userPersonalPath);
+                Interop.Crypto.X509StackAddMultiple(untrusted, s_userIntermediateStore.GetNativeCollection());
+                Interop.Crypto.X509StackAddMultiple(untrusted, s_userPersonalStore.GetNativeCollection());
                 Interop.Crypto.X509StackAddMultiple(untrusted, systemIntermediate);
                 Interop.Crypto.X509StoreSetVerifyTime(store, verificationTime);
 
index 509c319..f2976f1 100644 (file)
@@ -5,22 +5,14 @@
 using System;
 using System.Collections.Generic;
 using System.Diagnostics;
-using System.IO;
-using System.Linq;
-using System.Runtime.InteropServices;
 using System.Security.Cryptography;
 using System.Security.Cryptography.X509Certificates;
-using System.Threading;
 using Microsoft.Win32.SafeHandles;
 
 namespace Internal.Cryptography.Pal
 {
     internal sealed partial class StorePal
     {
-        private static CollectionBackedStoreProvider s_machineRootStore;
-        private static CollectionBackedStoreProvider s_machineIntermediateStore;
-        private static readonly object s_machineLoadLock = new object();
-
         public static IStorePal FromHandle(IntPtr storeHandle)
         {
             throw new PlatformNotSupportedException();
@@ -140,22 +132,6 @@ namespace Internal.Cryptography.Pal
             return new ExportProvider(certificates);
         }
 
-        internal static CollectionBackedStoreProvider GetMachineRoot()
-        {
-            return (CollectionBackedStoreProvider)FromSystemStore(
-                X509Store.RootStoreName,
-                StoreLocation.LocalMachine,
-                OpenFlags.ReadOnly);
-        }
-
-        internal static CollectionBackedStoreProvider GetMachineIntermediate()
-        {
-            return (CollectionBackedStoreProvider)FromSystemStore(
-                X509Store.IntermediateCAStoreName,
-                StoreLocation.LocalMachine,
-                OpenFlags.ReadOnly);
-        }
-
         public static IStorePal FromSystemStore(string storeName, StoreLocation storeLocation, OpenFlags openFlags)
         {
             if (storeLocation == StoreLocation.CurrentUser)
@@ -180,25 +156,15 @@ namespace Internal.Cryptography.Pal
             // The static store approach here is making an optimization based on not
             // having write support.  Once writing is permitted the stores would need
             // to fresh-read whenever being requested.
-            if (s_machineRootStore == null)
-            {
-                lock (s_machineLoadLock)
-                {
-                    if (s_machineRootStore == null)
-                    {
-                        LoadMachineStores();
-                    }
-                }
-            }
 
             if (X509Store.RootStoreName.Equals(storeName, StringComparison.OrdinalIgnoreCase))
             {
-                return s_machineRootStore;
+                return CachedSystemStoreProvider.MachineRoot;
             }
 
             if (X509Store.IntermediateCAStoreName.Equals(storeName, StringComparison.OrdinalIgnoreCase))
             {
-                return s_machineIntermediateStore;
+                return CachedSystemStoreProvider.MachineIntermediate;
             }
 
             throw new CryptographicException(
@@ -215,113 +181,5 @@ namespace Internal.Cryptography.Pal
         {
             return new CertCollectionLoader(certPals);
         }
-
-        private static void LoadMachineStores()
-        {
-            Debug.Assert(
-                Monitor.IsEntered(s_machineLoadLock),
-                "LoadMachineStores assumes a lock(s_machineLoadLock)");
-
-            var rootStore = new List<X509Certificate2>();
-            var intermedStore = new List<X509Certificate2>();
-
-            DirectoryInfo rootStorePath = null;
-            IEnumerable<FileInfo> trustedCertFiles;
-
-            try
-            {
-                rootStorePath = new DirectoryInfo(Interop.Crypto.GetX509RootStorePath());
-            }
-            catch (ArgumentException)
-            {
-                // If SSL_CERT_DIR is set to the empty string, or anything else which gives
-                // "The path is not of a legal form", then the GetX509RootStorePath value is ignored.
-            }
-
-            if (rootStorePath != null && rootStorePath.Exists)
-            {
-                trustedCertFiles = rootStorePath.EnumerateFiles();
-            }
-            else
-            {
-                trustedCertFiles = Array.Empty<FileInfo>();
-            }
-
-            FileInfo rootStoreFile = null;
-
-            try
-            {
-                rootStoreFile = new FileInfo(Interop.Crypto.GetX509RootStoreFile());
-            }
-            catch (ArgumentException)
-            {
-                // If SSL_CERT_FILE is set to the empty string, or anything else which gives
-                // "The path is not of a legal form", then the GetX509RootStoreFile value is ignored.
-            }
-
-            if (rootStoreFile != null && rootStoreFile.Exists)
-            {
-                trustedCertFiles = trustedCertFiles.Prepend(rootStoreFile);
-            }
-
-            HashSet<X509Certificate2> uniqueRootCerts = new HashSet<X509Certificate2>();
-            HashSet<X509Certificate2> uniqueIntermediateCerts = new HashSet<X509Certificate2>();
-
-            foreach (FileInfo file in trustedCertFiles)
-            {
-                using (SafeBioHandle fileBio = Interop.Crypto.BioNewFile(file.FullName, "rb"))
-                {
-                    // The handle may be invalid, for example when we don't have read permission for the file.
-                    if (fileBio.IsInvalid)
-                    {
-                        Interop.Crypto.ErrClearError();
-                        continue;
-                    }
-
-                    ICertificatePal pal;
-
-                    // Some distros ship with two variants of the same certificate.
-                    // One is the regular format ('BEGIN CERTIFICATE') and the other
-                    // contains additional AUX-data ('BEGIN TRUSTED CERTIFICATE').
-                    // The additional data contains the appropriate usage (e.g. emailProtection, serverAuth, ...).
-                    // Because corefx doesn't validate for a specific usage, derived certificates are rejected.
-                    // For now, we skip the certificates with AUX data and use the regular certificates.
-                    while (OpenSslX509CertificateReader.TryReadX509PemNoAux(fileBio, out pal) ||
-                        OpenSslX509CertificateReader.TryReadX509Der(fileBio, out pal))
-                    {
-                        X509Certificate2 cert = new X509Certificate2(pal);
-
-                        // The HashSets are just used for uniqueness filters, they do not survive this method.
-                        if (StringComparer.Ordinal.Equals(cert.Subject, cert.Issuer))
-                        {
-                            if (uniqueRootCerts.Add(cert))
-                            {
-                                rootStore.Add(cert);
-                                continue;
-                            }
-                        }
-                        else
-                        {
-                            if (uniqueIntermediateCerts.Add(cert))
-                            {
-                                intermedStore.Add(cert);
-                                continue;
-                            }
-                        }
-
-                        // There's a good chance we'll encounter duplicates on systems that have both one-cert-per-file
-                        // and one-big-file trusted certificate stores. Anything that wasn't unique will end up here.
-                        cert.Dispose();
-                    }
-                }
-            }
-
-            var rootStorePal = new CollectionBackedStoreProvider(rootStore);
-            s_machineIntermediateStore = new CollectionBackedStoreProvider(intermedStore);
-
-            // s_machineRootStore's nullarity is the loaded-state sentinel, so write it with Volatile.
-            Debug.Assert(Monitor.IsEntered(s_machineLoadLock), "LoadMachineStores assumes a lock(s_machineLoadLock)");
-            Volatile.Write(ref s_machineRootStore, rootStorePal);
-        }
     }
 }
index e326a88..182cb68 100644 (file)
       <DependentUpon>System\Security\Cryptography\X509Certificates\Asn1\DistributionPointNameAsn.xml</DependentUpon>
     </Compile>
     <Compile Include="System\Security\Cryptography\X509Certificates\Asn1\ReasonFlagsAsn.cs" />
+    <Compile Include="Internal\Cryptography\Pal.Unix\CachedDirectoryStoreProvider.cs" />
+    <Compile Include="Internal\Cryptography\Pal.Unix\CachedSystemStoreProvider.cs" />
     <Compile Include="Internal\Cryptography\Pal.Unix\CertCollectionLoader.cs" />
     <Compile Include="Internal\Cryptography\Pal.Unix\CertificateAssetDownloader.cs" />
     <Compile Include="Internal\Cryptography\Pal.Unix\CertificatePal.cs" />
     <Compile Include="Internal\Cryptography\Pal.Unix\ChainPal.cs" />
-    <Compile Include="Internal\Cryptography\Pal.Unix\CollectionBackedStoreProvider.cs" />
     <Compile Include="Internal\Cryptography\Pal.Unix\CrlCache.cs" />
     <Compile Include="Internal\Cryptography\Pal.Unix\DirectoryBasedStoreProvider.cs" />
     <Compile Include="Internal\Cryptography\Pal.Unix\ExportProvider.cs" />