Restrict permissions to the dev cert directory (#56985)

* Create directories with secure permissions

If we're creating it, make it 700.  If it already exists, warn if it's not 700.

* Don't create a directory specified by the user

Author: amcasey

src/ProjectTemplates/Shared/DevelopmentCertificate.cs

@@ -35,7 +35,7 @@ private static string EnsureDevelopmentCertificates(string certificatePath, stri
         var manager = CertificateManager.Instance;
         var certificate = manager.CreateAspNetCoreHttpsDevelopmentCertificate(now, now.AddYears(1));
         var certificateThumbprint = certificate.Thumbprint;
-        CertificateManager.ExportCertificate(certificate, path: certificatePath, includePrivateKey: true, certificatePassword, CertificateKeyExportFormat.Pfx);
+        manager.ExportCertificate(certificate, path: certificatePath, includePrivateKey: true, certificatePassword, CertificateKeyExportFormat.Pfx);
 
         return certificateThumbprint;
     }

src/Shared/CertificateGeneration/CertificateManager.cs

@@ -323,6 +323,14 @@ public EnsureCertificateResult EnsureAspNetCoreHttpsDevelopmentCertificate(
         {
             try
             {
+                // If the user specified a non-existent directory, we don't want to be responsible
+                // for setting the permissions appropriately, so we'll bail.
+                var exportDir = Path.GetDirectoryName(path);
+                if (!string.IsNullOrEmpty(exportDir) && !Directory.Exists(exportDir))
+                {
+                    throw new InvalidOperationException($"The directory '{exportDir}' does not exist.  Choose permissions carefully when creating it.");
+                }
+
                 ExportCertificate(certificate, path, includePrivateKey, password, keyExportFormat);
             }
             catch (Exception e)
@@ -484,7 +492,13 @@ public void CleanupHttpsCertificates()
 
     protected abstract IList<X509Certificate2> GetCertificatesToRemove(StoreName storeName, StoreLocation storeLocation);
 
-    internal static void ExportCertificate(X509Certificate2 certificate, string path, bool includePrivateKey, string? password, CertificateKeyExportFormat format)
+    protected abstract void CreateDirectoryWithPermissions(string directoryPath);
+
+    /// <remarks>
+    /// Will create directories to make it possible to write to <paramref name="path"/>.
+    /// If you don't want that, check for existence before calling this method.
+    /// </remarks>
+    internal void ExportCertificate(X509Certificate2 certificate, string path, bool includePrivateKey, string? password, CertificateKeyExportFormat format)
     {
         if (Log.IsEnabled())
         {
@@ -500,7 +514,7 @@ internal static void ExportCertificate(X509Certificate2 certificate, string path
         if (!string.IsNullOrEmpty(targetDirectoryPath))
         {
             Log.CreateExportCertificateDirectory(targetDirectoryPath);
-            Directory.CreateDirectory(targetDirectoryPath);
+            CreateDirectoryWithPermissions(targetDirectoryPath);
         }
 
         byte[] bytes;
@@ -1230,6 +1244,9 @@ public sealed class CertificateManagerEventSource : EventSource
         [Event(111, Level = EventLevel.LogAlways, Message = "For OpenSSL trust to take effect, '{0}' must be listed in the {2} environment variable. " +
             "See https://aka.ms/dev-certs-trust for more information.")]
         internal void UnixSuggestSettingEnvironmentVariableWithoutExample(string certDir, string envVarName) => WriteEvent(111, certDir, envVarName);
+
+        [Event(112, Level = EventLevel.Warning, Message = "Directory '{0}' may be readable by other users.")]
+        internal void DirectoryPermissionsNotSecure(string directoryPath) => WriteEvent(112, directoryPath);
     }
 
     internal sealed class UserCancelledTrustException : Exception

src/Shared/CertificateGeneration/MacOSCertificateManager.cs

@@ -18,6 +18,8 @@ namespace Microsoft.AspNetCore.Certificates.Generation;
 /// </remarks>
 internal sealed class MacOSCertificateManager : CertificateManager
 {
+    private const UnixFileMode DirectoryPermissions = UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute;
+
     // User keychain. Guard with quotes when using in command lines since users may have set
     // their user profile (HOME) directory to a non-standard path that includes whitespace.
     private static readonly string MacOSUserKeychain = Environment.GetFolderPath(Environment.SpecialFolder.UserProfile) + "/Library/Keychains/login.keychain-db";
@@ -93,6 +95,7 @@ protected override TrustLevel TrustCertificateCore(X509Certificate2 publicCertif
         var tmpFile = Path.GetTempFileName();
         try
         {
+            // We can't guarantee that the temp file is in a directory with sensible permissions, but we're not exporting the private key
             ExportCertificate(publicCertificate, tmpFile, includePrivateKey: false, password: null, CertificateKeyExportFormat.Pfx);
             if (Log.IsEnabled())
             {
@@ -134,9 +137,7 @@ internal override void CorrectCertificateState(X509Certificate2 candidate)
     {
         try
         {
-            // Ensure that the directory exists before writing to the file.
-            Directory.CreateDirectory(MacOSUserHttpsCertificateLocation);
-
+            // This path is in a well-known folder, so we trust the permissions.
             var certificatePath = GetCertificateFilePath(candidate);
             ExportCertificate(candidate, certificatePath, includePrivateKey: true, null, CertificateKeyExportFormat.Pfx);
         }
@@ -152,6 +153,7 @@ public override TrustLevel GetTrustLevel(X509Certificate2 certificate)
         var tmpFile = Path.GetTempFileName();
         try
         {
+            // We can't guarantee that the temp file is in a directory with sensible permissions, but we're not exporting the private key
             ExportCertificate(certificate, tmpFile, includePrivateKey: false, password: null, CertificateKeyExportFormat.Pem);
 
             using var checkTrustProcess = Process.Start(new ProcessStartInfo(
@@ -316,7 +318,7 @@ protected override X509Certificate2 SaveCertificateCore(X509Certificate2 certifi
             }
 
             // Ensure that the directory exists before writing to the file.
-            Directory.CreateDirectory(MacOSUserHttpsCertificateLocation);
+            CreateDirectoryWithPermissions(MacOSUserHttpsCertificateLocation);
 
             File.WriteAllBytes(GetCertificateFilePath(certificate), certBytes);
         }
@@ -474,4 +476,22 @@ protected override void RemoveCertificateFromUserStoreCore(X509Certificate2 cert
             RemoveCertificateFromKeychain(MacOSUserKeychain, certificate);
         }
     }
+
+    protected override void CreateDirectoryWithPermissions(string directoryPath)
+    {
+#pragma warning disable CA1416 // Validate platform compatibility (not supported on Windows)
+        var dirInfo = new DirectoryInfo(directoryPath);
+        if (dirInfo.Exists)
+        {
+            if ((dirInfo.UnixFileMode & ~DirectoryPermissions) != 0)
+            {
+                Log.DirectoryPermissionsNotSecure(dirInfo.FullName);
+            }
+        }
+        else
+        {
+            Directory.CreateDirectory(directoryPath, DirectoryPermissions);
+        }
+#pragma warning restore CA1416 // Validate platform compatibility
+    }
 }

src/Shared/CertificateGeneration/UnixCertificateManager.cs

@@ -20,6 +20,8 @@ namespace Microsoft.AspNetCore.Certificates.Generation;
 /// </remarks>
 internal sealed partial class UnixCertificateManager : CertificateManager
 {
+	private const UnixFileMode DirectoryPermissions = UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute;
+
     /// <summary>The name of an environment variable consumed by OpenSSL to locate certificates.</summary>
     private const string OpenSslCertificateDirectoryVariableName = "SSL_CERT_DIR";
 
@@ -74,7 +76,8 @@ public override TrustLevel GetTrustLevel(X509Certificate2 certificate)
             Log.UnixNotTrustedByDotnet();
         }
 
-        var nickname = GetCertificateNickname(certificate);
+        // Will become the name of the file on disk and the nickname in the NSS DBs
+        var certificateNickname = GetCertificateNickname(certificate);
 
         var sslCertDirString = Environment.GetEnvironmentVariable(OpenSslCertificateDirectoryVariableName);
         if (string.IsNullOrEmpty(sslCertDirString))
@@ -88,7 +91,7 @@ public override TrustLevel GetTrustLevel(X509Certificate2 certificate)
             var sslCertDirs = sslCertDirString.Split(Path.PathSeparator);
             foreach (var sslCertDir in sslCertDirs)
             {
-                var certPath = Path.Combine(sslCertDir, nickname + ".pem");
+                var certPath = Path.Combine(sslCertDir, certificateNickname + ".pem");
                 if (File.Exists(certPath))
                 {
                     var candidate = X509CertificateLoader.LoadCertificateFromFile(certPath);
@@ -125,7 +128,7 @@ public override TrustLevel GetTrustLevel(X509Certificate2 certificate)
             {
                 foreach (var nssDb in nssDbs)
                 {
-                    if (IsCertificateInNssDb(nickname, nssDb))
+                    if (IsCertificateInNssDb(certificateNickname, nssDb))
                     {
                         sawTrustSuccess = true;
                     }
@@ -138,6 +141,7 @@ public override TrustLevel GetTrustLevel(X509Certificate2 certificate)
             }
         }
 
+        // Success & Failure => Partial; Success => Full; Failure => None
         return sawTrustSuccess
             ? sawTrustFailure
                 ? TrustLevel.Partial
@@ -244,7 +248,7 @@ protected override TrustLevel TrustCertificateCore(X509Certificate2 certificate)
         if (needToExport)
         {
             // Security: we don't need the private key for trust, so we don't export it.
-            // Note that this will create directories as needed.
+            // Note that this will create directories as needed.  We control `certPath`, so the permissions should be fine.
             ExportCertificate(certificate, certPath, includePrivateKey: false, password: null, CertificateKeyExportFormat.Pem);
         }
 
@@ -449,6 +453,24 @@ protected override IList<X509Certificate2> GetCertificatesToRemove(StoreName sto
         return ListCertificates(StoreName.My, StoreLocation.CurrentUser, isValid: false, requireExportable: false);
     }
 
+    protected override void CreateDirectoryWithPermissions(string directoryPath)
+    {
+#pragma warning disable CA1416 // Validate platform compatibility (not supported on Windows)
+        var dirInfo = new DirectoryInfo(directoryPath);
+        if (dirInfo.Exists)
+        {
+            if ((dirInfo.UnixFileMode & ~DirectoryPermissions) != 0)
+            {
+                Log.DirectoryPermissionsNotSecure(dirInfo.FullName);
+            }
+        }
+        else
+        {
+            Directory.CreateDirectory(directoryPath, DirectoryPermissions);
+        }
+#pragma warning restore CA1416 // Validate platform compatibility
+    }
+
     private static string GetChromiumNssDb(string homeDirectory)
     {
         return Path.Combine(homeDirectory, ".pki", "nssdb");

src/Shared/CertificateGeneration/WindowsCertificateManager.cs

@@ -5,8 +5,10 @@
 using System.Collections.Generic;
 using System.Linq;
 using System.Runtime.Versioning;
+using System.Security.AccessControl;
 using System.Security.Cryptography;
 using System.Security.Cryptography.X509Certificates;
+using System.Security.Principal;
 
 namespace Microsoft.AspNetCore.Certificates.Generation;
 
@@ -126,4 +128,41 @@ protected override IList<X509Certificate2> GetCertificatesToRemove(StoreName sto
     {
         return ListCertificates(storeName, storeLocation, isValid: false);
     }
+
+    protected override void CreateDirectoryWithPermissions(string directoryPath)
+    {
+        var dirInfo = new DirectoryInfo(directoryPath);
+
+        if (!dirInfo.Exists)
+        {
+            // We trust the default permissions on Windows enough not to apply custom ACLs.
+            // We'll warn below if things seem really off.
+            dirInfo.Create();
+        }
+
+        var currentUser = WindowsIdentity.GetCurrent();
+        var currentUserSid = currentUser.User;
+        var systemSid = new SecurityIdentifier(WellKnownSidType.LocalSystemSid, domainSid: null);
+        var adminGroupSid = new SecurityIdentifier(WellKnownSidType.BuiltinAdministratorsSid, domainSid: null);
+
+        var dirSecurity = dirInfo.GetAccessControl();
+        var accessRules = dirSecurity.GetAccessRules(true, true, typeof(SecurityIdentifier));
+
+        foreach (FileSystemAccessRule rule in accessRules)
+        {
+            var idRef = rule.IdentityReference;
+            if (rule.AccessControlType == AccessControlType.Allow &&
+                !idRef.Equals(currentUserSid) &&
+                !idRef.Equals(systemSid) &&
+                !idRef.Equals(adminGroupSid))
+            {
+                // This is just a heuristic - determining whether the cumulative effect of the rules
+                // is to allow access to anyone other than the current user, system, or administrators
+                // is very complicated.  We're not going to do anything but log, so an approximation
+                // is fine.
+                Log.DirectoryPermissionsNotSecure(dirInfo.FullName);
+                break;
+            }
+        }
+    }
 }

src/Tools/FirstRunCertGenerator/test/CertificateManagerTests.cs

@@ -352,6 +352,30 @@ public void EnsureCreateHttpsCertificate_CanExportTheCertInPemFormat_WithoutPass
         Assert.Equal(httpsCertificate.GetCertHashString(), exportedCertificate.GetCertHashString());
     }
 
+    [ConditionalFact]
+    [SkipOnHelix("https://github.com/dotnet/aspnetcore/issues/6720", Queues = "All.OSX")]
+    public void EnsureCreateHttpsCertificate_CannotExportToNonExistentDirectory()
+    {
+        // Arrange
+        const string CertificateName = nameof(EnsureCreateHttpsCertificate_CannotExportToNonExistentDirectory) + ".pem";
+
+        _fixture.CleanupCertificates();
+
+        var now = DateTimeOffset.UtcNow;
+        now = new DateTimeOffset(now.Year, now.Month, now.Day, now.Hour, now.Minute, now.Second, 0, now.Offset);
+        var creation = _manager.EnsureAspNetCoreHttpsDevelopmentCertificate(now, now.AddYears(1), path: null, trust: false, isInteractive: false);
+        Output.WriteLine(creation.ToString());
+        ListCertificates();
+
+        // Act
+        // Export the certificate (same method, but this time with an output path)
+        var result = _manager
+            .EnsureAspNetCoreHttpsDevelopmentCertificate(now, now.AddYears(1), Path.Combine("NoSuchDirectory", CertificateName));
+
+        // Assert
+        Assert.Equal(EnsureCertificateResult.ErrorExportingTheCertificate, result);
+    }
+
     [Fact]
     public void EnsureCreateHttpsCertificate_ReturnsExpiredCertificateIfVersionIsIncorrect()
     {