dotnet8.0/runtime-92920-multiple-ssl-...

417 lines
18 KiB
Diff

From 9aec1e3b0b9ddc02b81bd115399f8951288b261b Mon Sep 17 00:00:00 2001
From: Tom Deseyn <tom.deseyn@gmail.com>
Date: Wed, 11 Oct 2023 18:32:20 +0200
Subject: [PATCH] Support specifying multiple directories through SSL_CERT_DIR
Co-authored-by: Jeremy Barton <jbarton@microsoft.com>
Co-authored-by: Kevin Jones <vcsjones@github.com>
---
.../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<SafeX509StackHandle, SafeX509StackHandle>? 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<SafeX509StackHandle, SafeX509StackHandle> 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<SafeX509StackHandle, SafeX509StackHandle> 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<X509Certificate2>();
var uniqueIntermediateCerts = new HashSet<X509Certificate2>();
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<string>(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