From 583d1499e25bb02f2c24a95e16a5547e31935120 Mon Sep 17 00:00:00 2001 From: AdmiringWorm Date: Tue, 25 Jul 2023 10:21:59 +0200 Subject: [PATCH 1/2] (#3281) Add validation for cache folder permissions These changes introduces a new validation check to ensure that the system cache folder that is used for storing NuGet responses have been properly locked down to administrators. When the directory exists, and allows modifications or creations of files by normal user this will output a validation warning about steps that can be taken to lock down the directory. When the directory does not exist, this same validation check ensure that the directory is created while only allowing Administrators to modify, create or delete anything in the folder. --- src/chocolatey/chocolatey.csproj | 3 +- .../ApplicationParameters.cs | 7 +- .../ChocolateyRegistrationModule.cs | 3 +- .../CacheFolderValidationLockdown.cs | 127 ++++++++++++++++ .../filesystem/DotNetFileSystem.cs | 142 ++++++++++++++++-- .../infrastructure/filesystem/IFileSystem.cs | 4 + 6 files changed, 265 insertions(+), 21 deletions(-) create mode 100644 src/chocolatey/infrastructure.app/validations/CacheFolderValidationLockdown.cs diff --git a/src/chocolatey/chocolatey.csproj b/src/chocolatey/chocolatey.csproj index c4b9ab931a..cbc7c14524 100644 --- a/src/chocolatey/chocolatey.csproj +++ b/src/chocolatey/chocolatey.csproj @@ -1,4 +1,4 @@ - + @@ -241,6 +241,7 @@ + diff --git a/src/chocolatey/infrastructure.app/ApplicationParameters.cs b/src/chocolatey/infrastructure.app/ApplicationParameters.cs index 122e6ddfa9..4f28197ce0 100644 --- a/src/chocolatey/infrastructure.app/ApplicationParameters.cs +++ b/src/chocolatey/infrastructure.app/ApplicationParameters.cs @@ -74,6 +74,9 @@ public static class ApplicationParameters System.Environment.GetFolderPath(System.Environment.SpecialFolder.UserProfile, System.Environment.SpecialFolderOption.DoNotVerify) : CommonAppDataChocolatey; public static readonly string HttpCacheUserLocation = _fileSystem.CombinePaths(UserProfilePath, ".chocolatey", "http-cache"); + // CommonAppDataChocolatey is always set to ProgramData\Chocolatey. + // So we append HttpCache to that name if it is possible. + public static readonly string HttpCacheSystemLocation = CommonAppDataChocolatey + "HttpCache"; public static readonly string HttpCacheLocation = GetHttpCacheLocation(); public static readonly string UserLicenseFileLocation = _fileSystem.CombinePaths(UserProfilePath, "chocolatey.license.xml"); @@ -112,9 +115,7 @@ private static string GetHttpCacheLocation() { if (ProcessInformation.IsElevated() || string.IsNullOrEmpty(System.Environment.GetFolderPath(System.Environment.SpecialFolder.UserProfile, System.Environment.SpecialFolderOption.DoNotVerify))) { - // CommonAppDataChocolatey is always set to ProgramData\Chocolatey. - // So we append HttpCache to that name if it is possible. - return CommonAppDataChocolatey + "HttpCache"; + return HttpCacheSystemLocation; } else { diff --git a/src/chocolatey/infrastructure.app/registration/ChocolateyRegistrationModule.cs b/src/chocolatey/infrastructure.app/registration/ChocolateyRegistrationModule.cs index 334b8aa580..dc48004502 100644 --- a/src/chocolatey/infrastructure.app/registration/ChocolateyRegistrationModule.cs +++ b/src/chocolatey/infrastructure.app/registration/ChocolateyRegistrationModule.cs @@ -82,7 +82,8 @@ public void RegisterDependencies(IContainerRegistrator registrator, ChocolateyCo registrator.RegisterService( typeof(GlobalConfigurationValidation), - typeof(SystemStateValidation)); + typeof(SystemStateValidation), + typeof(CacheFolderLockdownValidation)); // Rule registrations registrator.RegisterService(); diff --git a/src/chocolatey/infrastructure.app/validations/CacheFolderValidationLockdown.cs b/src/chocolatey/infrastructure.app/validations/CacheFolderValidationLockdown.cs new file mode 100644 index 0000000000..653e603b57 --- /dev/null +++ b/src/chocolatey/infrastructure.app/validations/CacheFolderValidationLockdown.cs @@ -0,0 +1,127 @@ +// Copyright © 2017 - 2023 Chocolatey Software, Inc +// Copyright © 2011 - 2017 RealDimensions Software, LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +namespace chocolatey.infrastructure.app.validations +{ + using System; + using System.Collections.Generic; + using chocolatey.infrastructure.app.configuration; + using chocolatey.infrastructure.filesystem; + using chocolatey.infrastructure.information; + using chocolatey.infrastructure.validations; + + public sealed class CacheFolderLockdownValidation : IValidation + { + private readonly IFileSystem _fileSystem; + + public CacheFolderLockdownValidation(IFileSystem fileSystem) + { + _fileSystem = fileSystem; + } + + public ICollection Validate(ChocolateyConfiguration config) + { + this.Log().Debug("Cache Folder Lockdown Checks:"); + + var result = new List(); + + if (!ProcessInformation.IsElevated() && !string.IsNullOrEmpty(Environment.GetFolderPath(Environment.SpecialFolder.UserProfile, Environment.SpecialFolderOption.DoNotVerify))) + { + this.Log().Debug(" - Elevated State = Failed"); + + result.Add(new ValidationResult + { + ExitCode = 0, + Message = "User Cache directory is valid.", + Status = ValidationStatus.Success + }); + + return result; + } + + this.Log().Debug(" - Elevated State = Checked"); + + var cacheFolderPath = ApplicationParameters.HttpCacheLocation; + + if (_fileSystem.DirectoryExists(cacheFolderPath)) + { + this.Log().Debug(" - Folder Exists = Checked"); + + if (_fileSystem.IsLockedDirectory(cacheFolderPath)) + { + this.Log().Debug(" - Folder lockdown = Checked"); + + result.Add(new ValidationResult + { + ExitCode = 0, + Message = "System Cache directory is locked down to administrators.", + Status = ValidationStatus.Success + }); + } + else + { + this.Log().Debug(" - Folder lockdown = Failed"); + + result.Add(new ValidationResult + { + ExitCode = 0, + Message = "System Cache directory is not locked down to administrators. Remove the directory '{0}' to have Chocolatey CLI create it with the proper permissions.".FormatWith(cacheFolderPath), + Status = ValidationStatus.Warning + }); + } + + return result; + } + + this.Log().Debug(" - Folder Exists = Failed"); + + if (_fileSystem.LockDirectory(cacheFolderPath)) + { + this.Log().Debug(" - Folder lockdown update = Success"); + + result.Add(new ValidationResult + { + ExitCode = 0, + Message = "System Cache directory successfullly created and locked down to administrators.", + Status = ValidationStatus.Success, + }); + } + else + { + this.Log().Debug(" - Folder lockdown update = Failed"); + + result.Add(new ValidationResult + { + ExitCode = 1, // Should we error? + Message = "System Cache directory was not created, or could not be locked down to administrators.", + Status = ValidationStatus.Error + }); + } + + return result; + } + +#pragma warning disable IDE1006 + + [Obsolete("This overload is deprecated and will be removed in v3.")] + public ICollection validate(ChocolateyConfiguration config) + { + return Validate(config); + } + +#pragma warning restore IDE1006 + } +} \ No newline at end of file diff --git a/src/chocolatey/infrastructure/filesystem/DotNetFileSystem.cs b/src/chocolatey/infrastructure/filesystem/DotNetFileSystem.cs index 410b765340..048cba2866 100644 --- a/src/chocolatey/infrastructure/filesystem/DotNetFileSystem.cs +++ b/src/chocolatey/infrastructure/filesystem/DotNetFileSystem.cs @@ -23,6 +23,8 @@ namespace chocolatey.infrastructure.filesystem using System.IO; using System.Linq; using System.Runtime.InteropServices; + using System.Security.AccessControl; + using System.Security.Principal; using System.Text; using System.Threading; using adapters; @@ -646,19 +648,7 @@ public dynamic GetFileDirectoryInfo(string filePath) public void CreateDirectory(string directoryPath) { - this.Log().Debug(ChocolateyLoggers.Verbose, () => "Attempting to create directory \"{0}\".".FormatWith(GetFullPath(directoryPath))); - AllowRetries( - () => - { - try - { - Directory.CreateDirectory(directoryPath); - } - catch (IOException) - { - Alphaleonis.Win32.Filesystem.Directory.CreateDirectory(directoryPath); - } - }); + CreateDirectory(directoryPath, isSilent: false); } public void MoveDirectory(string directoryPath, string newDirectoryPath) @@ -762,19 +752,139 @@ public void EnsureDirectoryExists(string directoryPath) EnsureDirectoryExists(directoryPath, false); } - private void EnsureDirectoryExists(string directoryPath, bool ignoreError) + public bool IsLockedDirectory(string directoryPath) + { + try + { + var permissions = Directory.GetAccessControl(directoryPath); + + var rules = permissions.GetAccessRules(includeExplicit: true, includeInherited: true, typeof(NTAccount)); + var builtinAdmins = new SecurityIdentifier(WellKnownSidType.BuiltinAdministratorsSid, null).Translate(typeof(NTAccount)); + var localSystem = new SecurityIdentifier(WellKnownSidType.LocalSystemSid, null).Translate(typeof(NTAccount)); + + foreach (FileSystemAccessRule rule in rules) + { + if (rule.IdentityReference != builtinAdmins && rule.IdentityReference != localSystem && + AllowsAnyFlag(rule, FileSystemRights.CreateFiles, FileSystemRights.AppendData, FileSystemRights.WriteExtendedAttributes, FileSystemRights.WriteAttributes)) + { + return false; + } + } + + return true; + } + catch (Exception ex) + { + this.Log().Debug("Unable to read directory '{0}'.", directoryPath); + this.Log().Debug("Exception: {0}", ex.Message); + + // If we do not have access, we assume the directory is locked. + return true; + } + } + + public bool LockDirectory(string directoryPath) + { + try + { + EnsureDirectoryExists(directoryPath, ignoreError: false, isSilent: true); + + this.Log().Debug(" - Folder Created = Success"); + + var permissions = Directory.GetAccessControl(directoryPath); + + var rules = permissions.GetAccessRules(includeExplicit: true, includeInherited: true, typeof(NTAccount)); + + // We first need to remove all rules + foreach (FileSystemAccessRule rule in rules) + { + permissions.RemoveAccessRuleAll(rule); + } + + this.Log().Debug(" - Pending normal access removed = Checked"); + + var bultinAdmins = new SecurityIdentifier(WellKnownSidType.BuiltinAdministratorsSid, null).Translate(typeof(NTAccount)); + var localsystem = new SecurityIdentifier(WellKnownSidType.LocalSystemSid, null).Translate(typeof(NTAccount)); + var builtinUsers = new SecurityIdentifier(WellKnownSidType.BuiltinUsersSid, null).Translate(typeof(NTAccount)); + + var inheritanceFlags = InheritanceFlags.ContainerInherit | InheritanceFlags.ObjectInherit; + + permissions.SetAccessRule(new FileSystemAccessRule(bultinAdmins, FileSystemRights.FullControl, inheritanceFlags, PropagationFlags.None, AccessControlType.Allow)); + permissions.SetAccessRule(new FileSystemAccessRule(localsystem, FileSystemRights.FullControl, inheritanceFlags, PropagationFlags.None, AccessControlType.Allow)); + permissions.SetAccessRule(new FileSystemAccessRule(builtinUsers, FileSystemRights.ReadAndExecute, inheritanceFlags, PropagationFlags.None, AccessControlType.Allow)); + this.Log().Debug(" - Pending write permissions for Administrators = Checked"); + + permissions.SetOwner(bultinAdmins); + this.Log().Debug(" - Pending Administrators as Owners = Checked"); + + permissions.SetAccessRuleProtection(isProtected: true, preserveInheritance: false); + this.Log().Debug(" - Pending removing inheritance with no copy = Checked"); + + Directory.SetAccessControl(directoryPath, permissions); + this.Log().Debug(" - Access Permissions updated = Success"); + + return true; + } + catch (Exception ex) + { + this.Log().Debug(" - Access Permissions updated = Failure"); + this.Log().Debug("Exception: {0}", ex.Message); + + return false; + } + } + + private bool AllowsAnyFlag(FileSystemAccessRule rule, params FileSystemRights[] flags) + { + foreach (var flag in flags) + { + if (rule.AccessControlType == AccessControlType.Allow && rule.FileSystemRights.HasFlag(flag)) + { + return true; + } + } + + return false; + } + + private void CreateDirectory(string directoryPath, bool isSilent) + { + if (!isSilent) + { + this.Log().Debug(ChocolateyLoggers.Verbose, () => "Attempting to create directory \"{0}\".".FormatWith(GetFullPath(directoryPath))); + } + + AllowRetries( + () => + { + try + { + Directory.CreateDirectory(directoryPath); + } + catch (IOException) + { + Alphaleonis.Win32.Filesystem.Directory.CreateDirectory(directoryPath); + } + }, isSilent: isSilent); + } + + private void EnsureDirectoryExists(string directoryPath, bool ignoreError, bool isSilent = false) { if (!DirectoryExists(directoryPath)) { try { - CreateDirectory(directoryPath); + CreateDirectory(directoryPath, isSilent); } catch (SystemException e) { if (!ignoreError) { - this.Log().Error("Cannot create directory \"{0}\". Error was:{1}{2}", GetFullPath(directoryPath), Environment.NewLine, e); + if (!isSilent) + { + this.Log().Error("Cannot create directory \"{0}\". Error was:{1}{2}", GetFullPath(directoryPath), Environment.NewLine, e); + } + throw; } } diff --git a/src/chocolatey/infrastructure/filesystem/IFileSystem.cs b/src/chocolatey/infrastructure/filesystem/IFileSystem.cs index 661cb51754..34c36c8de3 100644 --- a/src/chocolatey/infrastructure/filesystem/IFileSystem.cs +++ b/src/chocolatey/infrastructure/filesystem/IFileSystem.cs @@ -444,6 +444,10 @@ public interface IFileSystem /// Should this method be silent? false by default void DeleteDirectoryChecked(string directoryPath, bool recursive, bool overrideAttributes, bool isSilent); + bool IsLockedDirectory(string directoryPath); + + bool LockDirectory(string directoryPath); + #endregion /// From 2217d63826f144bab6c7a3f659eec4a3ebbc4420 Mon Sep 17 00:00:00 2001 From: AdmiringWorm Date: Tue, 25 Jul 2023 11:39:53 +0200 Subject: [PATCH 2/2] (maint) Add helper to split on max line lengths These changes introduces a new string helper that ensures that each line in the output will not be any longer than the configured maximum line length. And attempt to split these lines at the last available non-letter and non-digit before the length limitation. --- src/chocolatey/StringExtensions.cs | 80 +++++++++++++++++++ .../CacheFolderValidationLockdown.cs | 4 +- 2 files changed, 82 insertions(+), 2 deletions(-) diff --git a/src/chocolatey/StringExtensions.cs b/src/chocolatey/StringExtensions.cs index 4b830da846..5da1455870 100644 --- a/src/chocolatey/StringExtensions.cs +++ b/src/chocolatey/StringExtensions.cs @@ -17,10 +17,15 @@ namespace chocolatey { using System; + using System.Collections.Generic; using System.Globalization; + using System.Linq; using System.Runtime.InteropServices; using System.Security; + using System.Text; using System.Text.RegularExpressions; + using System.Threading; + using System.Web.UI; using infrastructure.app; using infrastructure.logging; @@ -50,6 +55,81 @@ public static string FormatWith(this string input, params object[] formatting) } } + /// + /// Splits any Newline elements and ensures that each line is no longer than the configured . + /// Lines longer than the specified line length will be split on the last non-letter or digit before the max length. + /// + /// The input to split any lines on. + /// The line prefix used for all lines not being the first line. + /// Maximum length of the line. + /// The splitted formatted lines. + /// Not recommended to be used in hot paths. + public static string SplitOnSpace(this string input, string linePrefix = "", int maxLineLength = 70) + { + if (string.IsNullOrWhiteSpace(input)) + { + return string.Empty; + } + + var sb = new StringBuilder(input.Length); + var firstLine = true; + var stack = new Stack(input.Split('\n').Reverse()); + + while (stack.Count > 0) + { + var currentLine = stack.Pop(); + + if (currentLine.Length <= maxLineLength) + { + if (!firstLine && !string.IsNullOrEmpty(currentLine)) + { + sb.Append(linePrefix); + } + + sb.AppendLine(currentLine.TrimEnd()); + } + else + { + var index = 70 - 1; + + for (; index >= 0; index--) + { + if (char.IsWhiteSpace(currentLine[index]) || !char.IsLetterOrDigit(currentLine[index])) + { + break; + } + } + + if (index <= 0) + { + index = maxLineLength; + } + + if (!firstLine) + { + sb.Append(linePrefix); + } + + var subLine = currentLine.Substring(0, index); + sb.AppendLine(subLine.TrimEnd()); + + if (stack.Count > 0) + { + var nextLine = currentLine.Substring(index + 1).TrimStart() + stack.Pop(); + stack.Push(nextLine); + } + else + { + stack.Push(currentLine.Substring(index + 1).TrimStart()); + } + } + + firstLine = false; + } + + return sb.ToString(); + } + /// /// Performs a trim only if the item is not null /// diff --git a/src/chocolatey/infrastructure.app/validations/CacheFolderValidationLockdown.cs b/src/chocolatey/infrastructure.app/validations/CacheFolderValidationLockdown.cs index 653e603b57..17cf157a6b 100644 --- a/src/chocolatey/infrastructure.app/validations/CacheFolderValidationLockdown.cs +++ b/src/chocolatey/infrastructure.app/validations/CacheFolderValidationLockdown.cs @@ -78,7 +78,7 @@ public ICollection Validate(ChocolateyConfiguration config) result.Add(new ValidationResult { ExitCode = 0, - Message = "System Cache directory is not locked down to administrators. Remove the directory '{0}' to have Chocolatey CLI create it with the proper permissions.".FormatWith(cacheFolderPath), + Message = "System Cache directory is not locked down to administrators.\nRemove the directory '{0}' to have Chocolatey CLI create it with the proper permissions.".FormatWith(cacheFolderPath).SplitOnSpace(linePrefix: " "), Status = ValidationStatus.Warning }); } @@ -106,7 +106,7 @@ public ICollection Validate(ChocolateyConfiguration config) result.Add(new ValidationResult { ExitCode = 1, // Should we error? - Message = "System Cache directory was not created, or could not be locked down to administrators.", + Message = "System Cache directory was not created, or could not be locked down to administrators.".SplitOnSpace(linePrefix: " "), Status = ValidationStatus.Error }); }