From 9aec1e3b0b9ddc02b81bd115399f8951288b261b Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Wed, 11 Oct 2023 18:32:20 +0200 Subject: [PATCH] Support specifying multiple directories through SSL_CERT_DIR Co-authored-by: Jeremy Barton Co-authored-by: Kevin Jones --- .../OpenSslCachedSystemStoreProvider.cs | 232 +++++++++--------- .../X509Certificates/X509StoreTests.Unix.cs | 42 +++- 2 files changed, 157 insertions(+), 117 deletions(-) diff --git a/src/runtime/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/OpenSslCachedSystemStoreProvider.cs b/src/runtime/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/OpenSslCachedSystemStoreProvider.cs index 4c9643c01e2..e66b3d1ad11 100644 --- a/src/runtime/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/OpenSslCachedSystemStoreProvider.cs +++ b/src/runtime/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/OpenSslCachedSystemStoreProvider.cs @@ -21,14 +21,14 @@ internal sealed class OpenSslCachedSystemStoreProvider : IStorePal 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 DirectoryInfo? s_rootStoreDirectoryInfo = SafeOpenRootDirectoryInfo(); + private static string[]? s_rootStoreDirectories; private static bool s_defaultRootDir; - private static readonly FileInfo? s_rootStoreFileInfo = SafeOpenRootFileInfo(); + private static string? s_rootStoreFile; + private static DateTime[]? s_directoryLastWrite; + private static DateTime s_fileLastWrite; // Use non-Value-Tuple so that it's an atomic update. private static Tuple? s_nativeCollections; - private static DateTime s_directoryCertsLastWrite; - private static DateTime s_fileCertsLastWrite; private readonly bool _isRoot; @@ -93,18 +93,11 @@ public void Remove(ICertificatePal cert) { 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 && ContentWriteTime(fileInfo) != s_fileCertsLastWrite) || - (dirInfo != null && dirInfo.Exists && ContentWriteTime(dirInfo) != s_directoryCertsLastWrite)) + LastWriteTimesHaveChanged()) { - ret = LoadMachineStores(dirInfo, fileInfo); + ret = LoadMachineStores(); } } } @@ -113,9 +106,37 @@ public void Remove(ICertificatePal cert) return ret; } - private static Tuple LoadMachineStores( - DirectoryInfo? rootStorePath, - FileInfo? rootStoreFile) + private static bool LastWriteTimesHaveChanged() + { + Debug.Assert( + Monitor.IsEntered(s_recheckStopwatch), + "LastWriteTimesHaveChanged assumes a lock(s_recheckStopwatch)"); + + if (s_rootStoreFile != null) + { + _ = TryStatFile(s_rootStoreFile, out DateTime lastModified); + if (lastModified != s_fileLastWrite) + { + return true; + } + } + + if (s_rootStoreDirectories != null && s_directoryLastWrite != null) + { + for (int i = 0; i < s_rootStoreDirectories.Length; i++) + { + _ = TryStatDirectory(s_rootStoreDirectories[i], out DateTime lastModified); + if (lastModified != s_directoryLastWrite[i]) + { + return true; + } + } + } + + return false; + } + + private static Tuple LoadMachineStores() { Debug.Assert( Monitor.IsEntered(s_recheckStopwatch), @@ -126,61 +147,76 @@ public void Remove(ICertificatePal cert) SafeX509StackHandle intermedStore = Interop.Crypto.NewX509Stack(); Interop.Crypto.CheckValidOpenSslHandle(intermedStore); - DateTime newFileTime = default; - DateTime newDirTime = default; - var uniqueRootCerts = new HashSet(); var uniqueIntermediateCerts = new HashSet(); bool firstLoad = (s_nativeCollections == null); - if (rootStoreFile != null && rootStoreFile.Exists) + if (firstLoad) { - newFileTime = ContentWriteTime(rootStoreFile); - ProcessFile(rootStoreFile); + s_rootStoreDirectories = GetRootStoreDirectories(out s_defaultRootDir); + s_directoryLastWrite = new DateTime[s_rootStoreDirectories.Length]; + s_rootStoreFile = GetRootStoreFile(); + } + else + { + Debug.Assert(s_rootStoreDirectories is not null); + Debug.Assert(s_directoryLastWrite is not null); + } + + if (s_rootStoreFile != null) + { + ProcessFile(s_rootStoreFile, out s_fileLastWrite); } bool hasStoreData = false; - if (rootStorePath != null && rootStorePath.Exists) + for (int i = 0; i < s_rootStoreDirectories.Length; i++) { - newDirTime = ContentWriteTime(rootStorePath); - hasStoreData = ProcessDir(rootStorePath); + hasStoreData = ProcessDir(s_rootStoreDirectories[i], out s_directoryLastWrite[i]); } if (firstLoad && !hasStoreData && s_defaultRootDir) { - DirectoryInfo etcSslCerts = new DirectoryInfo("/etc/ssl/certs"); - - if (etcSslCerts.Exists) + const string DefaultCertDir = "/etc/ssl/certs"; + hasStoreData = ProcessDir(DefaultCertDir, out DateTime lastModified); + if (hasStoreData) { - DateTime tmpTime = ContentWriteTime(etcSslCerts); - hasStoreData = ProcessDir(etcSslCerts); - - if (hasStoreData) - { - newDirTime = tmpTime; - s_rootStoreDirectoryInfo = etcSslCerts; - } + s_rootStoreDirectories = new[] { DefaultCertDir }; + s_directoryLastWrite = new[] { lastModified }; } } - bool ProcessDir(DirectoryInfo dir) + bool ProcessDir(string dir, out DateTime lastModified) { + if (!TryStatDirectory(dir, out lastModified)) + { + return false; + } + bool hasStoreData = false; - foreach (FileInfo file in dir.EnumerateFiles()) + foreach (string file in Directory.EnumerateFiles(dir)) { - hasStoreData |= ProcessFile(file); + hasStoreData |= ProcessFile(file, out _, skipStat: true); } return hasStoreData; } - bool ProcessFile(FileInfo file) + bool ProcessFile(string file, out DateTime lastModified, bool skipStat = false) { bool readData = false; - using (SafeBioHandle fileBio = Interop.Crypto.BioNewFile(file.FullName, "rb")) + if (skipStat) + { + lastModified = default; + } + else if (!TryStatFile(file, out lastModified)) + { + return false; + } + + using (SafeBioHandle fileBio = Interop.Crypto.BioNewFile(file, "rb")) { // The handle may be invalid, for example when we don't have read permission for the file. if (fileBio.IsInvalid) @@ -274,114 +310,78 @@ bool ProcessFile(FileInfo file) // on every call. Volatile.Write(ref s_nativeCollections, newCollections); - s_directoryCertsLastWrite = newDirTime; - s_fileCertsLastWrite = newFileTime; s_recheckStopwatch.Restart(); return newCollections; } - private static FileInfo? SafeOpenRootFileInfo() + private static string? GetRootStoreFile() { 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 Path.GetFullPath(rootFile); } return null; } - private static DirectoryInfo? SafeOpenRootDirectoryInfo() + private static string[] GetRootStoreDirectories(out bool isDefault) { - string? rootDirectory = Interop.Crypto.GetX509RootStorePath(out s_defaultRootDir); + string rootDirectory = Interop.Crypto.GetX509RootStorePath(out isDefault) ?? ""; - 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; - } - - private static DateTime ContentWriteTime(FileInfo info) - { - string path = info.FullName; - string? target = Interop.Sys.ReadLink(path); - - if (string.IsNullOrEmpty(target)) - { - return info.LastWriteTimeUtc; - } + string[] directories = rootDirectory.Split(Path.PathSeparator, StringSplitOptions.RemoveEmptyEntries); - if (target[0] != '/') + for (int i = 0; i < directories.Length; i++) { - target = Path.Join(info.Directory?.FullName, target); + directories[i] = Path.GetFullPath(directories[i]); } - try + // Remove duplicates. + if (directories.Length > 1) { - var targetInfo = new FileInfo(target); - - if (targetInfo.Exists) + var set = new HashSet(directories, StringComparer.Ordinal); + if (set.Count != directories.Length) { - return targetInfo.LastWriteTimeUtc; + // Preserve the original order. + string[] directoriesTrimmed = new string[set.Count]; + int j = 0; + for (int i = 0; i < directories.Length; i++) + { + string directory = directories[i]; + if (set.Remove(directory)) + { + directoriesTrimmed[j++] = directory; + } + } + Debug.Assert(set.Count == 0); + directories = directoriesTrimmed; } } - catch (ArgumentException) - { - // If we can't load information about the link path, just treat it as not a link. - } - return info.LastWriteTimeUtc; + return directories; } - private static DateTime ContentWriteTime(DirectoryInfo info) - { - string path = info.FullName; - string? target = Interop.Sys.ReadLink(path); - - if (string.IsNullOrEmpty(target)) - { - return info.LastWriteTimeUtc; - } + private static bool TryStatFile(string path, out DateTime lastModified) + => TryStat(path, Interop.Sys.FileTypes.S_IFREG, out lastModified); - if (target[0] != '/') - { - target = Path.Join(info.Parent?.FullName, target); - } + private static bool TryStatDirectory(string path, out DateTime lastModified) + => TryStat(path, Interop.Sys.FileTypes.S_IFDIR, out lastModified); - try - { - var targetInfo = new DirectoryInfo(target); + private static bool TryStat(string path, int fileType, out DateTime lastModified) + { + lastModified = default; - if (targetInfo.Exists) - { - return targetInfo.LastWriteTimeUtc; - } - } - catch (ArgumentException) + Interop.Sys.FileStatus status; + // Use Stat to follow links. + if (Interop.Sys.Stat(path, out status) < 0 || + (status.Mode & Interop.Sys.FileTypes.S_IFMT) != fileType) { - // If we can't load information about the link path, just treat it as not a link. + return false; } - return info.LastWriteTimeUtc; + lastModified = DateTime.UnixEpoch + TimeSpan.FromTicks(status.MTime * TimeSpan.TicksPerSecond + status.MTimeNsec / TimeSpan.NanosecondsPerTick); + return true; } } } diff --git a/src/runtime/src/libraries/System.Security.Cryptography/tests/X509Certificates/X509StoreTests.Unix.cs b/src/runtime/src/libraries/System.Security.Cryptography/tests/X509Certificates/X509StoreTests.Unix.cs index 0efb6c12028..f460d6b9bd6 100644 --- a/src/runtime/src/libraries/System.Security.Cryptography/tests/X509Certificates/X509StoreTests.Unix.cs +++ b/src/runtime/src/libraries/System.Security.Cryptography/tests/X509Certificates/X509StoreTests.Unix.cs @@ -10,7 +10,6 @@ namespace System.Security.Cryptography.X509Certificates.Tests { public partial class X509StoreTests { - [ConditionalFact(nameof(NotRunningAsRootAndRemoteExecutorSupported))] // root can read '2.pem' [PlatformSpecific(TestPlatforms.Linux)] // Windows/OSX doesn't use SSL_CERT_{DIR,FILE}. private void X509Store_MachineStoreLoadSkipsInvalidFiles() @@ -50,6 +49,47 @@ private void X509Store_MachineStoreLoadSkipsInvalidFiles() }, new RemoteInvokeOptions { StartInfo = psi }).Dispose(); } + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [PlatformSpecific(TestPlatforms.Linux)] // Windows/OSX doesn't use SSL_CERT_{DIR,FILE}. + private void X509Store_MachineStoreLoadsMutipleSslCertDirectories() + { + // Create 3 certificates and place them in two directories that will be passed + // using SSL_CERT_DIR. + string sslCertDir1 = GetTestFilePath(); + Directory.CreateDirectory(sslCertDir1); + File.WriteAllBytes(Path.Combine(sslCertDir1, "1.pem"), TestData.SelfSigned1PemBytes); + File.WriteAllBytes(Path.Combine(sslCertDir1, "2.pem"), TestData.SelfSigned2PemBytes); + string sslCertDir2 = GetTestFilePath(); + Directory.CreateDirectory(sslCertDir2); + File.WriteAllBytes(Path.Combine(sslCertDir2, "3.pem"), TestData.SelfSigned3PemBytes); + + // Add a non-existing directory after each valid directory to verify they are ignored. + string sslCertDir = string.Join(Path.PathSeparator, + new[] { + sslCertDir1, + sslCertDir2, + "", // empty string + sslCertDir2, // duplicate directory + "/invalid2", // path that does not exist + }); + + var psi = new ProcessStartInfo(); + psi.Environment.Add("SSL_CERT_DIR", sslCertDir); + // Set SSL_CERT_FILE to avoid loading the default bundle file. + psi.Environment.Add("SSL_CERT_FILE", "/nonexisting"); + RemoteExecutor.Invoke(() => + { + Assert.NotNull(Environment.GetEnvironmentVariable("SSL_CERT_DIR")); + using (var store = new X509Store(StoreName.Root, StoreLocation.LocalMachine)) + { + store.Open(OpenFlags.OpenExistingOnly); + + // Check nr of certificates in store. + Assert.Equal(3, store.Certificates.Count); + } + }, new RemoteInvokeOptions { StartInfo = psi }).Dispose(); + } + public static bool NotRunningAsRootAndRemoteExecutorSupported => !Environment.IsPrivilegedProcess && RemoteExecutor.IsSupported; } } -- 2.41.0