diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8013a3fdcd..d0243c860f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1394,6 +1394,21 @@ jobs: run: dotnet test package/AgentWindowsManaged.Tests/DevolutionsAgent.Installer.Tests.csproj shell: pwsh + gateway-installer-tests: + name: Gateway installer tests + runs-on: windows-2022 + needs: [preflight] + + steps: + - name: Checkout ${{ github.repository }} + uses: actions/checkout@v6 + with: + ref: ${{ needs.preflight.outputs.ref }} + + - name: Tests + run: dotnet test package/WindowsManaged.UnitTests/DevolutionsGateway.Installer.Tests.csproj + shell: pwsh + winapi-sanitizer-tests: name: Windows API sanitizer tests runs-on: windows-2022 @@ -1655,7 +1670,7 @@ jobs: success: name: Success if: ${{ always() }} - needs: [tests, agent-tunnel-e2e, agent-policy-e2e, lints, oracle-markers, oracle-markers-tests, skill-links, check-dependencies, jetsocat-lipo, devolutions-gateway-powershell, gateway-service-account-tests, devolutions-gateway, devolutions-gateway-merge, devolutions-pedm-desktop, devolutions-agent, devolutions-agent-merge, devolutions-pedm-client, dotnet-utils-tests, agent-identity-vectors, agent-installer-event-log-tests, winapi-sanitizer-tests, winapi-miri, pedm-simulator, secure-memory-verifier] + needs: [tests, agent-tunnel-e2e, agent-policy-e2e, lints, oracle-markers, oracle-markers-tests, skill-links, check-dependencies, jetsocat-lipo, devolutions-gateway-powershell, gateway-service-account-tests, devolutions-gateway, devolutions-gateway-merge, devolutions-pedm-desktop, devolutions-agent, devolutions-agent-merge, devolutions-pedm-client, dotnet-utils-tests, agent-identity-vectors, agent-installer-event-log-tests, gateway-installer-tests, winapi-sanitizer-tests, winapi-miri, pedm-simulator, secure-memory-verifier] runs-on: ubuntu-latest steps: diff --git a/package/WindowsManaged.UnitTests/DataFolderTests.cs b/package/WindowsManaged.UnitTests/DataFolderTests.cs new file mode 100644 index 0000000000..eba7fb2d3f --- /dev/null +++ b/package/WindowsManaged.UnitTests/DataFolderTests.cs @@ -0,0 +1,504 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Security.AccessControl; +using System.Security.Principal; +using System.Text.RegularExpressions; + +using DevolutionsGateway.Actions; +using DevolutionsGateway.Properties; +using DevolutionsGateway.Resources; + +using Record = Microsoft.Deployment.WindowsInstaller.Record; + +using WixSharp; + +using Xunit; +using Xunit.Abstractions; + +using static DevolutionsGateway.Installer.Tests.InstallerSession; + +namespace DevolutionsGateway.Installer.Tests; + +[Collection(InstallerSession.Collection)] +public sealed class DataFolderTests +{ + // Values of Evaluate's checkAccess, which only Devolutions gets. + private const bool Devolutions = true; + private const bool Gateway = false; + + // The DACL of a Devolutions folder that SYSTEM created in ProgramData, as read on Windows 11: inherited full control + // for SYSTEM and Administrators, an inherit-only CREATOR OWNER entry, read access for Users, and the Users entry that + // only adds files and subdirectories. + private const string InheritedDacl = + "D:AI(A;OICIID;FA;;;SY)(A;OICIID;FA;;;BA)(A;OICIIOID;GA;;;CO)(A;OICIID;0x1200a9;;;BU)(A;CIID;DCLCRPCR;;;BU)"; + + private const string Users = "S-1-5-32-545"; + + private const string Everyone = "S-1-1-0"; + + private static readonly SecurityIdentifier CurrentUser = WindowsIdentity.GetCurrent().User; + + private static readonly string GatewaySddl = $"O:{CurrentUser}D:P(A;OICI;FA;;;{CurrentUser})(A;OICI;FA;;;SY)"; + + private static readonly string[] DataFolderActions = + [ + nameof(CustomActions.CheckProgramDataDirectory), + nameof(CustomActions.CreateProgramDataDirectory), + nameof(CustomActions.SetUsersDatabaseFilePermissions), + ]; + + private readonly ILogger logger; + + public DataFolderTests(ITestOutputHelper output) => this.logger = new OutputLogger(output); + + [Theory] + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [InlineData("SYSTEM owns Devolutions", Devolutions, "O:SY" + InheritedDacl, "none")] + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [InlineData("Administrators own Devolutions", Devolutions, "O:BA" + InheritedDacl, "none")] + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [InlineData("Users own Devolutions", Devolutions, "O:BU" + InheritedDacl, "Owner " + Users)] + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [InlineData("Everyone owns Devolutions", Devolutions, "O:WD" + InheritedDacl, "Owner " + Everyone)] + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [InlineData("Users own Gateway", Gateway, "O:BU" + InheritedDacl, "Owner " + Users)] + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [InlineData("NETWORK SERVICE owns Gateway", Gateway, "O:NS" + InheritedDacl, "Owner S-1-5-20")] + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [InlineData("Users may delete Devolutions", Devolutions, "O:SYD:(A;;SD;;;BU)", "Access " + Users)] + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [InlineData("Users may change the DACL", Devolutions, "O:SYD:(A;;WD;;;BU)", "Access " + Users)] + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [InlineData("Users may take ownership", Devolutions, "O:SYD:(A;;WO;;;BU)", "Access " + Users)] + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [InlineData("Users may delete entries", Devolutions, "O:SYD:(A;;0x40;;;BU)", "Access " + Users)] + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [InlineData("Users have generic all", Devolutions, "O:SYD:(A;;GA;;;BU)", "Access " + Users)] + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [InlineData("Users may delete through a conditional entry", Devolutions, "O:SYD:(XA;;SD;;;BU;(Member_of {SID(BU)}))", "Access " + Users)] + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [InlineData("null DACL", Devolutions, "O:SYD:NO_ACCESS_CONTROL", "Access " + Everyone)] + // oracle: reference Devolutions/devolutions-gateway@9b8b60380f6c:crates/private-fs/src/windows/security.rs Policy::check_ancestor + [InlineData("Users get full control of entries created below", Devolutions, "O:SYD:(A;OICIIO;FA;;;BU)", "none")] + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [InlineData("Users may only add entries", Devolutions, "O:SYD:(A;CI;DCLCRPCR;;;BU)", "none")] + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [InlineData("Users are denied deletion", Devolutions, "O:SYD:(D;;SD;;;BU)", "none")] + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [InlineData("SYSTEM has full control", Devolutions, "O:SYD:(A;OICI;FA;;;SY)", "none")] + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [InlineData("Administrators have full control", Devolutions, "O:SYD:(A;OICI;FA;;;BA)", "none")] + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [InlineData("Everyone may delete Devolutions", Devolutions, "O:SYD:(A;;SD;;;WD)", "Access " + Everyone)] + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [InlineData("Everyone may change the DACL", Devolutions, "O:SYD:(A;;WD;;;WD)", "Access " + Everyone)] + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [InlineData("Everyone may take ownership", Devolutions, "O:SYD:(A;;WO;;;WD)", "Access " + Everyone)] + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [InlineData("Everyone may delete entries", Devolutions, "O:SYD:(A;;0x40;;;WD)", "Access " + Everyone)] + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [InlineData("Everyone has generic all", Devolutions, "O:SYD:(A;;GA;;;WD)", "Access " + Everyone)] + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [InlineData("NETWORK SERVICE has full control", Devolutions, "O:SYD:(A;OICI;FA;;;NS)", "Access S-1-5-20")] + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [InlineData("TrustedInstaller has full control", Devolutions, "O:SYD:(A;;FA;;;S-1-5-80-956008885-3418522649-1831038044-1853292631-2271478464)", "none")] + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [InlineData("no owner", Devolutions, "D:(A;;FA;;;SY)", "Unreadable")] + // oracle: reference Devolutions/devolutions-gateway@9b8b60380f6c:crates/private-fs/src/windows/security.rs Policy::check_ancestor + [InlineData("OWNER RIGHTS has full control", Devolutions, "O:SYD:(A;OICI;FA;;;OW)", "none")] + // oracle: reference Devolutions/devolutions-gateway@9b8b60380f6c:crates/private-fs/src/windows/security.rs Policy::check_ancestor + [InlineData("an AppContainer package has full control", Devolutions, "O:SYD:(A;OICI;FA;;;S-1-15-2-1)", "none")] + // oracle: reference Devolutions/devolutions-gateway@9b8b60380f6c:crates/private-fs/src/windows/security.rs Policy::check_ancestor + [InlineData("an AppContainer capability has full control", Devolutions, "O:SYD:(A;OICI;FA;;;S-1-15-3-1)", "none")] + // oracle: reference Devolutions/devolutions-gateway@9b8b60380f6c:crates/private-fs/src/windows/security.rs Policy::check_ancestor + [InlineData("Users are denied deletion through an object entry", Devolutions, "O:SYD:(OD;;SD;bf967aba-0de6-11d0-a285-00aa003049e2;;BU)", "none")] + // oracle: reference Devolutions/devolutions-gateway@9b8b60380f6c:crates/private-fs/src/windows/security.rs Policy::check_ancestor + [InlineData("Users may delete through an object entry", Devolutions, "O:SYD:(OA;;SD;bf967aba-0de6-11d0-a285-00aa003049e2;;BU)", "Unreadable")] + // oracle: reference Devolutions/devolutions-gateway@9b8b60380f6c:crates/private-fs/src/windows/security.rs Policy::check_ancestor + [InlineData("an inherit-only object entry", Devolutions, "O:SYD:(OA;CIIO;SD;bf967aba-0de6-11d0-a285-00aa003049e2;;BU)", "Unreadable")] + // oracle: reference Devolutions/devolutions-gateway@9b8b60380f6c:crates/private-fs/src/windows/security.rs Policy::check_ancestor + [InlineData("Users may delete through a conditional object entry", Devolutions, "O:SYD:(ZA;;SD;bf967aba-0de6-11d0-a285-00aa003049e2;;BU;(Member_of {SID(BU)}))", "Unreadable")] + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [InlineData("Users have full control of Gateway", Gateway, "O:SYD:(A;OICI;FA;;;BU)", "none")] + public void RefusalFollowsTheOwnerAndTheDacl(string scenario, bool checkAccess, string sddl, string expected) + { + DataFolderRefusal refusal = DataFolders.Evaluate( + "folder", new RawSecurityDescriptor(sddl), isLink: false, isDirectory: true, checkAccess, this.logger); + + Assert.Equal((scenario, expected), (scenario, Describe(refusal))); + } + + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [Theory] + [InlineData(Devolutions)] + [InlineData(Gateway)] + public void LinkIsRefusedWhateverItsSecurity(bool checkAccess) + { + DataFolderRefusal refusal = DataFolders.Evaluate( + "folder", new RawSecurityDescriptor("O:SY" + InheritedDacl), isLink: true, isDirectory: true, checkAccess, this.logger); + + Assert.Equal("Link", Describe(refusal)); + } + + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [Theory] + [InlineData(Devolutions)] + [InlineData(Gateway)] + public void FileIsRefusedWhateverItsSecurity(bool checkAccess) + { + DataFolderRefusal refusal = DataFolders.Evaluate( + "folder", new RawSecurityDescriptor("O:SY" + InheritedDacl), isLink: false, isDirectory: false, checkAccess, this.logger); + + Assert.Equal("File", Describe(refusal)); + } + + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [Fact] + public void CheckRefusesAFileAtDevolutions() + { + using TempFolder folder = new(); + string devolutions = Path.Combine(folder.Path, "Devolutions"); + System.IO.File.WriteAllText(devolutions, ""); + + DataFolderRefusal refusal = DataFolders.Check(folder.Path, this.logger); + + Assert.Equal((devolutions, "File"), (refusal?.Folder, Describe(refusal))); + } + + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [Fact] + public void CreateRefusesAFileAtDevolutionsAndLeavesIt() + { + using TempFolder folder = new(); + string devolutions = Path.Combine(folder.Path, "Devolutions"); + System.IO.File.WriteAllText(devolutions, "kept"); + + DataFolderRefusal refusal = DataFolders.Create(folder.Path, GatewaySddl, this.logger); + + Assert.Equal((devolutions, "File", "kept"), (refusal?.Folder, Describe(refusal), System.IO.File.ReadAllText(devolutions))); + } + + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [Fact] + public void CheckRefusesAJunctionAtDevolutions() + { + using TempFolder folder = new(); + string devolutions = Path.Combine(folder.Path, "Devolutions"); + TempFolder.CreateJunction(devolutions, folder.CreateDirectory("target")); + + DataFolderRefusal refusal = DataFolders.Check(folder.Path, this.logger); + + Assert.Equal((devolutions, "Link"), (refusal?.Folder, Describe(refusal))); + } + + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [Fact] + public void CreateRefusesAJunctionAtDevolutionsAndCreatesNothingInItsTarget() + { + using TempFolder folder = new(); + string target = folder.CreateDirectory("target"); + string devolutions = Path.Combine(folder.Path, "Devolutions"); + TempFolder.CreateJunction(devolutions, target); + + DataFolderRefusal refusal = DataFolders.Create(folder.Path, GatewaySddl, this.logger); + + Assert.Equal((devolutions, "Link"), (refusal?.Folder, Describe(refusal))); + Assert.Empty(Directory.GetFileSystemEntries(target)); + } + + [Theory] + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [InlineData("Users may delete Devolutions", "SD")] + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [InlineData("Users may delete entries of Devolutions", "0x40")] + public void CheckRefusesADevolutionsFolderThatUsersMayReplaceOrWhoseEntriesTheyMayReplace(string scenario, string right) + { + using TempFolder folder = new(); + string devolutions = Path.Combine(folder.Path, "Devolutions"); + Assert.True(SecureDirectory.Create(devolutions, ReplaceableByUsers(right)), scenario); + + DataFolderRefusal refusal = DataFolders.Check(folder.Path, this.logger); + + Assert.Equal((devolutions, "Access " + Users), (refusal?.Folder, Describe(refusal))); + } + + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [Fact] + public void CreateRefusesADevolutionsFolderThatUsersMayDeleteAndCreatesNothingInIt() + { + using TempFolder folder = new(); + string devolutions = Path.Combine(folder.Path, "Devolutions"); + Assert.True(SecureDirectory.Create(devolutions, ReplaceableByUsers("SD"))); + + DataFolderRefusal refusal = DataFolders.Create(folder.Path, GatewaySddl, this.logger); + + Assert.Equal((devolutions, "Access " + Users), (refusal?.Folder, Describe(refusal))); + Assert.Empty(Directory.GetFileSystemEntries(devolutions)); + } + + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [Fact] + public void CheckRefusesADevolutionsFolderItCantOpenAsUnreadable() + { + using TempFolder folder = new(); + string devolutions = Path.Combine(folder.Path, "Devolutions"); + // Opening the folder fails with access denied, not with not-found: the current user, its owner, is granted + // neither READ_CONTROL, as the OWNER RIGHTS entry takes away the owner's implicit one, nor FILE_TRAVERSE. + // The user keeps the rights to list the folder, read its attributes and synchronize, which deleting it needs. + Assert.True(SecureDirectory.Create(devolutions, $"O:{CurrentUser}D:P(A;;0;;;OW)(A;OICI;FA;;;SY)(A;;0x100081;;;{CurrentUser})")); + + DataFolderRefusal refusal = DataFolders.Check(folder.Path, this.logger); + + Assert.Equal((devolutions, "Unreadable"), (refusal?.Folder, Describe(refusal))); + } + + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [Fact] + public void CheckPassesWhenTheFoldersDontExist() + { + using TempFolder folder = new(); + + Assert.Null(DataFolders.Check(folder.Path, this.logger)); + } + + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [Fact] + public void ErrorNamesTheFolderAndTheAccountOrItsSidWhenItHasNone() + { + const string Folder = @"C:\ProgramData\Devolutions"; + const string Unnamed = "S-1-5-21-1-2-3-1001"; + SecurityIdentifier users = new(Users); + + string owned = new DataFolderRefusal(Folder, DataFolderProblem.Owner, users).Message("1033"); + string replaceable = new DataFolderRefusal(Folder, DataFolderProblem.Access, new SecurityIdentifier(Unnamed)).Message("1033"); + + Assert.Contains(Folder, owned); + Assert.Contains(users.Translate(typeof(NTAccount)).Value, owned); + Assert.Contains(Folder, replaceable); + Assert.Contains(Unnamed, replaceable); + } + + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [Fact] + public void ErrorIsInTheInstallerLanguage() + { + DataFolderRefusal refusal = new(@"C:\ProgramData\Devolutions", DataFolderProblem.Link); + string[] messages = new[] { "1033", "1036", "1031" }.Select(refusal.Message).ToArray(); + + Assert.Equal(messages.Length, messages.Distinct().Count()); + // A language the installer doesn't have falls back to English, as the rest of the installer does. + Assert.Equal(messages[0], refusal.Message("1053")); + } + + // The error names a folder, whose path may hold brackets, which the installer would otherwise read as property + // references. + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [Fact] + public void ErrorIsShownAsWritten() + { + const string Text = @"C:\[SECRET]\x] [y [\[]"; + + string shown = WithSession(["SECRET=value"], session => + { + using Record record = CustomActions.MessageRecord(Text); + return session.FormatRecord(record); + }); + + Assert.Equal(Text, shown); + } + + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [Theory] + [InlineData(Strings.DataFolderIsLink)] + [InlineData(Strings.DataFolderIsFile)] + [InlineData(Strings.DataFolderOwnedByUntrustedAccount)] + [InlineData(Strings.DataFolderReplaceableByUntrustedAccount)] + [InlineData(Strings.DataFolderSecurityUnreadable)] + public void ErrorIsTranslatedWithTheEnglishPlaceholders(string id) + { + string english = InstallerStrings.Localize("1033", id); + + Assert.NotEqual(id, english); + Assert.All(new[] { "1036", "1031" }, language => + { + string translated = InstallerStrings.Localize(language, id); + Assert.NotEqual(id, translated); + Assert.NotEqual(english, translated); + Assert.Equal(Placeholders(english), Placeholders(translated)); + }); + } + + [Theory] + [InlineData(nameof(CustomActions.CheckProgramDataDirectory))] + [InlineData(nameof(CustomActions.CreateProgramDataDirectory))] + public void RefusalFailsTheRunInTheInstallerLanguage(string method) + { + ManagedAction action = ActionOf(method); + + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + Assert.Equal(Return.check, action.Return); + // A deferred action reads only the properties its action data carries. + // oracle: spec Windows Installer §Obtaining Context Information for Deferred Execution Custom Actions + Assert.Contains("ProductLanguage=[ProductLanguage]", action.UsesProperties.Split(';')); + } + + // The security descriptors these actions apply name the service account, and a deferred action reads only the + // properties its action data carries. + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + // oracle: spec Windows Installer §Obtaining Context Information for Deferred Execution Custom Actions + [Theory] + [InlineData(nameof(CustomActions.CreateProgramDataDirectory))] + [InlineData(nameof(CustomActions.SetUsersDatabaseFilePermissions))] + public void SecurityResetGetsTheServiceAccount(string method) + { + string id = GatewayProperties.serviceAccountSid.Id; + + Assert.Contains($"{id}=[{id}]", ActionOf(method).UsesProperties.Split(';')); + } + + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [Theory] + [InlineData(nameof(CustomActions.CreateProgramDataDirectory))] + [InlineData(nameof(CustomActions.SetUsersDatabaseFilePermissions))] + public void FailedSecurityResetFailsTheRun(string method) + { + Assert.Equal(Return.check, ActionOf(method).Return); + } + + [Theory] + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [InlineData("first install", true)] + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [InlineData("major upgrade", true, "WIX_UPGRADE_DETECTED={00000000-0000-0000-0000-000000000001}")] + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [InlineData("repair", true, "Installed=00:00:00", "REINSTALL=ALL")] + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [InlineData("maintenance", true, "Installed=00:00:00")] + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [InlineData("full uninstall", false, "Installed=00:00:00", "REMOVE=ALL")] + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [InlineData("old product removed by a major upgrade", false, "Installed=00:00:00", "REMOVE=ALL", "UPGRADINGPRODUCTCODE={00000000-0000-0000-0000-000000000002}")] + public void DataFoldersAreCheckedAndResetOnEveryRunButAFullUninstall(string scenario, bool expected, params string[] properties) + { + bool[] evaluated = DataFolderActions + .Select(method => EvaluateCondition(ActionOf(method).Condition.ToString(), properties)) + .ToArray(); + + Assert.True(evaluated.All(e => e == expected), $"{scenario}: {string.Join(", ", evaluated)}"); + } + + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [Fact] + public void CheckRunsBeforeTheMajorUpgradeRemovesTheInstalledProduct() + { + Assert.Equal( + (When.Before, Step.RemoveExistingProducts.ToString(), UpgradeSchedule.afterInstallInitialize), + (GatewayActions.checkProgramDataDirectory.When, + GatewayActions.checkProgramDataDirectory.Step.ToString(), + Program.MajorUpgrade.Schedule)); + } + + [Theory] + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [InlineData("the data folder")] + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [InlineData("the users database")] + public void OnlySystemAdministratorsAndTheServiceAccountCanChangeTheData(string template) + { + SecurityIdentifier service = new("S-1-5-80-1-2-3-4-5"); + RawSecurityDescriptor descriptor = new(Templates[template](service)); + // Every right that changes an entry, its content or its security, including through a generic right. + const int changing = 0x2 | 0x4 | 0x10 | 0x40 | 0x100 | 0x10000 | 0x40000 | 0x80000 | 0x10000000 | 0x40000000; + + string[] writers = descriptor.DiscretionaryAcl + .Cast() + .Where(ace => ace.AceQualifier == AceQualifier.AccessAllowed && (ace.AccessMask & changing) != 0) + .Select(ace => ace.SecurityIdentifier.Value) + .Distinct() + .OrderBy(sid => sid, StringComparer.Ordinal) + .ToArray(); + string[] expected = new[] { "S-1-5-18", "S-1-5-32-544", service.Value } + .OrderBy(sid => sid, StringComparer.Ordinal) + .ToArray(); + + Assert.Equal(expected, writers); + } + + // A service account that is LOCAL SERVICE gets its rights through the service account's entries. + // oracle: mutant an entry of its own for LOCAL SERVICE, such as read access + [Theory] + [InlineData("the data folder")] + [InlineData("the users database")] + public void LocalServiceHasNoEntryOfItsOwn(string template) + { + RawSecurityDescriptor descriptor = new(Templates[template](new SecurityIdentifier("S-1-5-80-1-2-3-4-5"))); + + Assert.DoesNotContain( + descriptor.DiscretionaryAcl.Cast(), + ace => ace.SecurityIdentifier.Equals(new SecurityIdentifier(WellKnownSidType.LocalServiceSid, null))); + } + + // The service account is the current user, who can then create entries in the folder without elevation. + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [Fact] + public void ServiceAccountCantDeleteTheGatewayFolderButCanDeleteWhatIsBelowIt() + { + using TempFolder folder = new(); + string gateway = folder.CreateDirectory("Gateway"); + DirectorySecurity security = new(); + security.SetSecurityDescriptorSddlForm(Includes.ProgramDataSddl(CurrentUser), AccessControlSections.Access); + Directory.SetAccessControl(gateway, security); + string file = Path.Combine(gateway, "file.txt"); + System.IO.File.WriteAllText(file, ""); + string subfolder = Directory.CreateDirectory(Path.Combine(gateway, "sub")).FullName; + + FileSystemRights onFolder = RightsOf(CurrentUser, Directory.GetAccessControl(gateway)); + FileSystemRights onFile = RightsOf(CurrentUser, System.IO.File.GetAccessControl(file)); + FileSystemRights onSubfolder = RightsOf(CurrentUser, Directory.GetAccessControl(subfolder)); + + Assert.Equal( + (false, true, true, true), + (onFolder.HasFlag(FileSystemRights.Delete), + onFolder.HasFlag(FileSystemRights.DeleteSubdirectoriesAndFiles), + onFile.HasFlag(FileSystemRights.Delete), + onSubfolder.HasFlag(FileSystemRights.Delete))); + } + + /// + /// A security descriptor that grants Users on the folder. + /// + /// + /// Protected, so that the folder inherits nothing from the temporary directory, which grants the current user full + /// control, and owned by the current user, who can't make another account the owner without elevation. + /// + private static string ReplaceableByUsers(string right) => + $"O:{CurrentUser}D:P(A;OICI;FA;;;SY)(A;OICI;FA;;;BA)(A;OICI;0x1200a9;;;BU)(A;;{right};;;BU)"; + + /// + /// The problem and the account names, or none. + /// + private static string Describe(DataFolderRefusal refusal) => + refusal is null ? "none" : $"{refusal.Problem} {refusal.Account}".TrimEnd(); + + private static readonly Dictionary> Templates = new() + { + ["the data folder"] = Includes.ProgramDataSddl, + ["the users database"] = Includes.UsersFileSddl, + }; + + /// + /// The rights the allow entries of grant on the object itself. + /// + private static FileSystemRights RightsOf(SecurityIdentifier account, FileSystemSecurity security) => + security.GetAccessRules(true, true, typeof(SecurityIdentifier)) + .Cast() + .Where(rule => rule.IdentityReference.Equals(account) + && rule.AccessControlType == AccessControlType.Allow + && !rule.PropagationFlags.HasFlag(PropagationFlags.InheritOnly)) + .Aggregate((FileSystemRights)0, (rights, rule) => rights | rule.FileSystemRights); + + private static string[] Placeholders(string text) => + Regex.Matches(text, @"\{\d+\}").Cast().Select(m => m.Value).OrderBy(p => p).ToArray(); +} diff --git a/package/WindowsManaged.UnitTests/DevolutionsGateway.Installer.Tests.csproj b/package/WindowsManaged.UnitTests/DevolutionsGateway.Installer.Tests.csproj new file mode 100644 index 0000000000..ea4b7d97ad --- /dev/null +++ b/package/WindowsManaged.UnitTests/DevolutionsGateway.Installer.Tests.csproj @@ -0,0 +1,21 @@ + + + net471 + latest + false + DevolutionsGateway.Installer.Tests + + + + + + + runtime; build; native; contentfiles; analyzers; buildtransitive + all + + + + + + + diff --git a/package/WindowsManaged.UnitTests/InstallerSession.cs b/package/WindowsManaged.UnitTests/InstallerSession.cs new file mode 100644 index 0000000000..d4b3616193 --- /dev/null +++ b/package/WindowsManaged.UnitTests/InstallerSession.cs @@ -0,0 +1,82 @@ +using System; +using System.IO; +using System.Linq; + +using DevolutionsGateway.Actions; + +using Microsoft.Deployment.WindowsInstaller; + +using WixSharp; + +using Xunit; + +using MsiInstaller = Microsoft.Deployment.WindowsInstaller.Installer; + +namespace DevolutionsGateway.Installer.Tests; + +/// +/// The installer's actions, and Windows Installer sessions to evaluate them in. +/// +internal static class InstallerSession +{ + /// + /// The test collection of every test class that opens a session. + /// + /// + /// Windows Installer opens one package at a time in a process, and xUnit runs the tests of one collection one at a + /// time. + /// + public const string Collection = "Windows Installer session"; + + /// + /// The installer's action that runs the custom action . + /// + public static ManagedAction ActionOf(string methodName) => + Assert.Single(GatewayActions.Actions.OfType(), a => a.MethodName == methodName); + + /// + /// Evaluates with the Windows Installer engine, in a session where + /// (NAME=value) are set. + /// + public static bool EvaluateCondition(string condition, string[] properties) => + WithSession(properties, session => session.EvaluateCondition(condition)); + + public static T WithSession(string[] properties, Func action) => + WithSession(AppContext.BaseDirectory, properties, action); + + /// + /// Runs in a Windows Installer session of an empty package in + /// where (NAME=value) are set. + /// + public static T WithSession(string folder, string[] properties, Func action) + { + string path = Path.Combine(folder, $"session-{Guid.NewGuid():N}.msi"); + + try + { + using (Database database = new(path, DatabaseOpenMode.CreateDirect)) + { + database.Execute("CREATE TABLE `Property` (`Property` CHAR(72) NOT NULL, `Value` CHAR(0) NOT NULL LOCALIZABLE PRIMARY KEY `Property`)"); + database.Execute($"INSERT INTO `Property` (`Property`, `Value`) VALUES ('ProductCode', '{Guid.NewGuid():B}')"); + database.SummaryInfo.Template = "x64;1033"; + database.SummaryInfo.RevisionNumber = Guid.NewGuid().ToString("B"); + database.SummaryInfo.PageCount = 500; + database.Commit(); + } + + using Session session = MsiInstaller.OpenPackage(path, ignoreMachineState: true); + + foreach (string property in properties) + { + string[] parts = property.Split(['='], 2); + session[parts[0]] = parts[1]; + } + + return action(session); + } + finally + { + System.IO.File.Delete(path); + } + } +} diff --git a/package/WindowsManaged.UnitTests/OutputLogger.cs b/package/WindowsManaged.UnitTests/OutputLogger.cs new file mode 100644 index 0000000000..72b399f8d3 --- /dev/null +++ b/package/WindowsManaged.UnitTests/OutputLogger.cs @@ -0,0 +1,15 @@ +using DevolutionsGateway.Actions; + +using Xunit.Abstractions; + +namespace DevolutionsGateway.Installer.Tests; + +/// +/// Writes the log of the code under test to the test's output. +/// +internal sealed class OutputLogger(ITestOutputHelper output) : ILogger +{ + public void Log(string msg) => output.WriteLine(msg); + + public void Log(string format, params object[] args) => output.WriteLine(format, args); +} diff --git a/package/WindowsManaged.UnitTests/SecureDirectoryTests.cs b/package/WindowsManaged.UnitTests/SecureDirectoryTests.cs new file mode 100644 index 0000000000..6d9c18ecf6 --- /dev/null +++ b/package/WindowsManaged.UnitTests/SecureDirectoryTests.cs @@ -0,0 +1,647 @@ +using System; +using System.ComponentModel; +using System.IO; +using System.Linq; +using System.Security.AccessControl; +using System.Security.Principal; + +using DevolutionsGateway.Actions; + +using Microsoft.Win32.SafeHandles; + +using Xunit; +using Xunit.Abstractions; + +namespace DevolutionsGateway.Installer.Tests; + +public sealed class SecureDirectoryTests +{ + private static readonly SecurityIdentifier CurrentUser = WindowsIdentity.GetCurrent().User; + + private static readonly SecurityIdentifier LocalSystem = new(WellKnownSidType.LocalSystemSid, null); + + private static readonly SecurityIdentifier LocalService = new(WellKnownSidType.LocalServiceSid, null); + + private static readonly SecurityIdentifier NetworkService = new(WellKnownSidType.NetworkServiceSid, null); + + // Owned by the current user, with a protected DACL granting full control to the current user and SYSTEM only. + // A folder under the temporary directory inherits more entries, such as one for Administrators. + private static readonly string ProtectedSddl = $"O:{CurrentUser}D:P(A;OICI;FA;;;{CurrentUser})(A;OICI;FA;;;SY)"; + + private static readonly string[] GatewayNames = ["Devolutions", "Gateway"]; + + // Entries for files and folders, for files only, and for folders only. + // No generic right, which the file system stores mapped where .NET stores it as it is. + // No CREATOR OWNER entry, because Windows keeps the entry it gave a folder's owner as an explicit one when the + // folder's parent didn't mark its DACL auto-inherited, where SetSecurityAndInheritBelow counts it as inherited, + // which grants the same access. + // No LOCAL SERVICE entry, which SetSecurityAndInheritBelow removes where Windows keeps it. + private static readonly string TreeSddl = + $"O:{CurrentUser}D:PAI(A;OICI;FA;;;{CurrentUser})(A;OICI;FA;;;SY)(A;OI;FR;;;BU)(A;CI;0x1200a9;;;AU)"; + + private static readonly string[] UsersFileNames = ["Devolutions", "Gateway", "users.txt"]; + + private readonly ILogger logger; + + public SecureDirectoryTests(ITestOutputHelper output) => this.logger = new OutputLogger(output); + + // oracle: spec Microsoft Learn CreateFileW §dwShareMode, FILE_SHARE_DELETE + [Fact] + public void HeldDirectoryCantBeRenamedUntilItsHandleIsClosed() + { + AssertHoldsUntilClosed(path => SecureDirectory.Hold(path)); + } + + // oracle: spec Microsoft Learn CreateFileW §dwShareMode, FILE_SHARE_DELETE + [Fact] + public void DirectoryOpenedToReadAndHoldCantBeRenamedUntilItsHandleIsClosed() + { + AssertHoldsUntilClosed(path => SecureDirectory.OpenToRead(path, hold: true)); + } + + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [Fact] + public void SetSecurityAndInheritBelowRefusesAFileAndLeavesItUnchanged() + { + using TempFolder folder = new(); + string path = Path.Combine(folder.Path, "data"); + File.WriteAllText(path, ""); + string before = FileSddl(path); + using SafeFileHandle handle = SecureDirectory.HoldToSetSecurity(path); + + Assert.Throws( + () => SecureDirectory.SetSecurityAndInheritBelow(handle, path, ProtectedSddl, [], this.logger)); + + Assert.Equal(before, FileSddl(path)); + } + + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [Fact] + public void SetSecurityAndInheritBelowAppliesTheOwnerAndTheProtectedDacl() + { + using TempFolder folder = new(); + string gateway = CreateGatewayFolder(folder.Path); + + this.SetSecurityAndInheritBelow(folder.Path, ProtectedSddl); + + AssertProtectedSecurity(new DirectoryInfo(gateway).GetAccessControl()); + } + + [Fact] + public void CreateGivesTheNewDirectoryExactlyTheSecurityDescriptor() + { + using TempFolder folder = new(); + string path = Path.Combine(folder.Path, "data"); + + Assert.True(SecureDirectory.Create(path, ProtectedSddl.Replace("D:P", "D:PAI"))); + + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + AssertProtectedSecurity(new DirectoryInfo(path).GetAccessControl()); + // The installer's templates mark the DACL auto-inherited, so that the entries created below hold what they + // inherit as inherited entries, which a later reset tells apart from their explicit ones without the folder's + // earlier DACL. + // oracle: mutant Create stores the DACL without its auto-inherited mark + Assert.Equal( + ControlFlags.DiscretionaryAclAutoInherited, + new RawSecurityDescriptor(RawSddl(path)).ControlFlags & ControlFlags.DiscretionaryAclAutoInherited); + } + + [Theory] + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [InlineData("SYSTEM", "S-1-5-18", true)] + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [InlineData("Administrators", "S-1-5-32-544", true)] + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [InlineData("TrustedInstaller", "S-1-5-80-956008885-3418522649-1831038044-1853292631-2271478464", true)] + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [InlineData("Everyone", "S-1-1-0", false)] + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [InlineData("Users", "S-1-5-32-545", false)] + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [InlineData("LOCAL SERVICE", "S-1-5-19", false)] + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [InlineData("NETWORK SERVICE", "S-1-5-20", false)] + public void WellKnownOwnerIsTrustedOnlyWhenPrivileged(string name, string sid, bool expected) + { + Assert.Equal( + (name, expected), (name, SecureDirectory.IsTrustedOwner(new SecurityIdentifier(sid), this.logger))); + } + + // oracle: property round-trip + [Fact] + public void ReadSecurityReadsTheOwnerAndTheDaclThatCreateApplied() + { + using TempFolder folder = new(); + string path = Path.Combine(folder.Path, "data"); + Assert.True(SecureDirectory.Create(path, ProtectedSddl)); + + using SafeFileHandle handle = SecureDirectory.OpenToRead(path, hold: false); + (RawSecurityDescriptor security, bool isReparsePoint, bool isDirectory) = SecureDirectory.ReadSecurity(handle); + + // Both sides go through the same SDDL rendering, which writes some account SIDs as aliases such as LA. + Assert.Equal( + new RawSecurityDescriptor(ProtectedSddl).GetSddlForm(AccessControlSections.Owner | AccessControlSections.Access), + security.GetSddlForm(AccessControlSections.Owner | AccessControlSections.Access)); + Assert.False(isReparsePoint); + Assert.True(isDirectory); + } + + // Windows computes what an entry inherits when a directory's DACL is passed on, and when the entry's DACL + // protection is removed, through SetNamedSecurityInfo, which .NET calls to persist a FileSecurity or a + // DirectorySecurity; SetSecurityAndInheritBelow must give each file and folder the DACL these calls give it, + // without resolving any path again. + // oracle: reference System.IO.Directory.SetAccessControl and File.SetAccessControl, then SetAccessRuleProtection(false, false) top-down, run live + [Theory] + [InlineData("a file directly in the folder")] + [InlineData("a file in a subfolder", "sub")] + [InlineData("a file two subfolders down", "sub", "deeper")] + public void InheritBelowGivesAProtectedFileTheDaclWindowsGivesItWhenItsProtectionIsRemoved( + string scenario, params string[] subfolders) + { + AssertInheritsAsWindows(scenario, subfolders, (gateway, file) => ProtectFile(file), TreeSddl); + } + + // An administrator who protects a folder below Gateway decides who may use it, so the folder keeps its DACL, and + // the files below it get what it passes on. + // oracle: reference System.IO.Directory.SetAccessControl and File.SetAccessControl, then SetAccessRuleProtection(false, false) top-down on files and unprotected folders, run live + [Theory] + [InlineData("a folder directly in the folder", "sub")] + [InlineData("a folder two levels down", "sub", "deeper")] + public void InheritBelowKeepsTheDaclOfAProtectedFolderAndResetsTheFilesBelowItAgainstIt( + string scenario, params string[] subfolders) + { + AssertInheritsAsWindows( + scenario, + subfolders, + (gateway, file) => + { + ProtectDirectory(Path.GetDirectoryName(file)); + ProtectFile(file); + }, + TreeSddl); + } + + // A file or folder created before the folder got its current DACL holds the entries it inherited then, such as + // none for a service account chosen later, or one for an account that no longer has access, until its inherited + // entries are computed again. + // Earlier installers stored the folder's DACL without marking it auto-inherited, so the entries below hold what + // they inherited as plain entries, which only the folder's earlier DACL tells apart from explicit ones. + // oracle: reference System.IO.Directory.SetAccessControl and File.SetAccessControl, then SetAccessRuleProtection(false, false) top-down, run live + [Theory] + [InlineData("a file directly in the folder", true)] + [InlineData("a file in a subfolder", true, "sub")] + [InlineData("a file two subfolders down", true, "sub", "deeper")] + [InlineData("a file directly in a folder not marked auto-inherited", false)] + [InlineData("a file in a subfolder of a folder not marked auto-inherited", false, "sub")] + [InlineData("a file two subfolders down a folder not marked auto-inherited", false, "sub", "deeper")] + public void InheritBelowGivesEntriesWithStaleInheritedEntriesTheDaclWindowsGivesThem( + string scenario, bool autoInherited, params string[] subfolders) + { + AssertInheritsAsWindows( + scenario, + subfolders, + (gateway, file) => { }, + $"O:{CurrentUser}D:PAI(A;OICI;FA;;;{CurrentUser})(A;OICI;FA;;;SY)(A;OICI;0x1301ff;;;NS)", + autoInherited ? TreeSddl : TreeSddl.Replace("D:PAI", "D:P")); + } + + // An earlier installer gave Gateway a DACL that LOCAL SERVICE could write through, stored without being marked + // auto-inherited, so a folder below holds what it inherited as plain entries. + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [Fact] + public void InheritBelowReplacesTheLocalServiceEntriesAnEarlierInstallerPassedOnByTheServiceAccount() + { + using TempFolder folder = new(); + folder.CreateDirectory("Devolutions"); + string gateway = Path.Combine(folder.Path, "Devolutions", "Gateway"); + Assert.True(SecureDirectory.Create( + gateway, $"O:{CurrentUser}D:P(A;OICI;FA;;;{CurrentUser})(A;OICI;FA;;;SY)(A;OICI;0x1201bf;;;LS)")); + Assert.Equal( + (ControlFlags)0, + new RawSecurityDescriptor(RawSddl(gateway)).ControlFlags & ControlFlags.DiscretionaryAclAutoInherited); + string recordings = folder.CreateDirectory("Devolutions", "Gateway", "recordings"); + string file = Path.Combine(recordings, "file.txt"); + File.WriteAllText(file, ""); + // As stored, since .NET reads a DACL through GetNamedSecurityInfo, which tells inherited entries apart. + Assert.True( + new RawSecurityDescriptor(RawSddl(recordings)).DiscretionaryAcl + .Cast() + .Any(ace => ace.SecurityIdentifier == LocalService && (ace.AceFlags & AceFlags.Inherited) == 0), + "the subfolder starts with a plain entry for LOCAL SERVICE"); + Assert.True(Allows(LocalService, File.GetAccessControl(file)), "the file starts with an entry for LOCAL SERVICE"); + + // The installer's template, with NETWORK SERVICE as the service account and the current user as the owner. + this.SetSecurityAndInheritBelow( + folder.Path, + $"O:{CurrentUser}D:PAI(A;OICI;FA;;;SY)(A;OICI;0x1201ff;;;NS)(A;OICIIO;0x1301ff;;;NS)(A;OICI;FA;;;{CurrentUser})"); + + Assert.Equal( + (false, true, false, true), + (Allows(LocalService, Directory.GetAccessControl(recordings)), + Allows(NetworkService, Directory.GetAccessControl(recordings)), + Allows(LocalService, File.GetAccessControl(file)), + Allows(NetworkService, File.GetAccessControl(file)))); + } + + // Earlier installers gave LOCAL SERVICE write access, which a protected folder holds as explicit entries. + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [Fact] + public void InheritBelowRemovesLocalServiceFromAProtectedFolderAndAFileBelowItAndKeepsTheirOtherEntries() + { + using TempFolder folder = new(); + folder.CreateDirectory("Devolutions"); + string gateway = Path.Combine(folder.Path, "Devolutions", "Gateway"); + Assert.True(SecureDirectory.Create(gateway, TreeSddl)); + string sub = folder.CreateDirectory("Devolutions", "Gateway", "sub"); + DirectorySecurity subSecurity = new(); + subSecurity.SetSecurityDescriptorSddlForm( + $"D:P(A;OICI;FA;;;{CurrentUser})(A;OICI;0x1301ff;;;LS)(A;OICI;FR;;;BU)", AccessControlSections.Access); + Directory.SetAccessControl(sub, subSecurity); + string file = Path.Combine(sub, "file.txt"); + File.WriteAllText(file, ""); + FileSecurity fileSecurity = new(); + fileSecurity.SetSecurityDescriptorSddlForm( + $"D:P(A;;FA;;;{CurrentUser})(A;;0x1301ff;;;LS)", AccessControlSections.Access); + File.SetAccessControl(file, fileSecurity); + + this.SetSecurityAndInheritBelow(folder.Path, TreeSddl); + + DirectorySecurity subAfter = Directory.GetAccessControl(sub); + FileSecurity fileAfter = File.GetAccessControl(file); + Assert.Equal( + (true, false, true, true, false, false, true, true), + (subAfter.AreAccessRulesProtected, + Allows(LocalService, subAfter), + Allows(CurrentUser, subAfter), + Allows(new SecurityIdentifier(WellKnownSidType.BuiltinUsersSid, null), subAfter), + fileAfter.AreAccessRulesProtected, + Allows(LocalService, fileAfter), + Allows(CurrentUser, fileAfter), + Allows(new SecurityIdentifier(WellKnownSidType.BuiltinUsersSid, null), fileAfter))); + } + + // Another action gives the users database a DACL that only SYSTEM, administrators and the service account can + // read, so resetting it here first would let users read it, through what Gateway passes on, until then. + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [Fact] + public void InheritBelowLeavesAnEntryItIsToldToKeepUnchangedAndResetsTheOthers() + { + using TempFolder folder = new(); + folder.CreateDirectory("Devolutions"); + string gateway = Path.Combine(folder.Path, "Devolutions", "Gateway"); + Assert.True(SecureDirectory.Create(gateway, TreeSddl)); + string users = CreateProtectedFile(Path.Combine(gateway, "users.txt")); + string other = CreateProtectedFile(Path.Combine(gateway, "other.txt")); + string before = FileSddl(users); + + this.SetSecurityAndInheritBelow(folder.Path, TreeSddl, "users.txt"); + + Assert.Equal( + (before, false), + (FileSddl(users), File.GetAccessControl(other).AreAccessRulesProtected)); + } + + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [Fact] + public void InheritBelowLeavesTheFilesBehindAJunctionInsideTheFolderUnchanged() + { + using TempFolder folder = new(); + string gateway = CreateGatewayFolder(folder.Path); + string outside = folder.CreateDirectory("outside"); + string target = CreateProtectedFile(Path.Combine(outside, "file.txt")); + string before = FileSddl(target); + TempFolder.CreateJunction(Path.Combine(gateway, "link"), outside); + + this.SetSecurityAndInheritBelow(folder.Path, ProtectedSddl); + + Assert.Equal(before, FileSddl(target)); + } + + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [Fact] + public void InheritBelowLeavesAFileWithAnotherHardLinkUnchanged() + { + using TempFolder folder = new(); + string gateway = CreateGatewayFolder(folder.Path); + string outside = CreateProtectedFile(Path.Combine(folder.CreateDirectory("outside"), "file.txt")); + string before = FileSddl(outside); + TempFolder.CreateHardLink(Path.Combine(gateway, "link.txt"), outside); + + this.SetSecurityAndInheritBelow(folder.Path, ProtectedSddl); + + Assert.Equal(before, FileSddl(outside)); + } + + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [Fact] + public void InheritBelowRefusesAJunctionAtTheFolderAndLeavesItsTargetAndTheFilesBehindItUnchanged() + { + using TempFolder folder = new(); + folder.CreateDirectory("Devolutions"); + string outside = folder.CreateDirectory("outside"); + string target = CreateProtectedFile(Path.Combine(outside, "file.txt")); + (string, string) before = (Sddl(outside), FileSddl(target)); + TempFolder.CreateJunction(Path.Combine(folder.Path, "Devolutions", "Gateway"), outside); + + Assert.Throws(() => this.SetSecurityAndInheritBelow(folder.Path, ProtectedSddl)); + + Assert.Equal(before, (Sddl(outside), FileSddl(target))); + } + + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [Fact] + public void SetFileSecurityAppliesTheOwnerAndTheProtectedDacl() + { + using TempFolder folder = new(); + CreateGatewayFolder(folder.Path); + string path = Path.Combine(folder.Path, "Devolutions", "Gateway", "users.txt"); + File.WriteAllText(path, ""); + + SecureDirectory.SetFileSecurity(folder.Path, UsersFileNames, ProtectedSddl); + + AssertProtectedSecurity(File.GetAccessControl(path)); + } + + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [Fact] + public void SetFileSecurityRefusesAJunctionAndLeavesItsTargetUnchanged() + { + using TempFolder folder = new(); + string gateway = CreateGatewayFolder(folder.Path); + string target = folder.CreateDirectory("outside"); + string before = Sddl(target); + TempFolder.CreateJunction(Path.Combine(gateway, "users.txt"), target); + + Assert.Throws(() => SecureDirectory.SetFileSecurity(folder.Path, UsersFileNames, ProtectedSddl)); + + Assert.Equal(before, Sddl(target)); + } + + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [Fact] + public void SetFileSecurityRefusesAJunctionAboveTheFileAndLeavesTheFileBehindItUnchanged() + { + using TempFolder folder = new(); + string target = folder.CreateDirectory("outside"); + string file = CreateProtectedFile(Path.Combine(target, "users.txt")); + string before = FileSddl(file); + folder.CreateDirectory("Devolutions"); + TempFolder.CreateJunction(Path.Combine(folder.Path, "Devolutions", "Gateway"), target); + + Assert.Throws(() => SecureDirectory.SetFileSecurity(folder.Path, UsersFileNames, ProtectedSddl)); + + Assert.Equal(before, FileSddl(file)); + } + + // oracle: intent package/WindowsManaged/INTENT.md §Data folder + [Fact] + public void SetFileSecurityRefusesAFileWithAnotherHardLinkAndLeavesItUnchanged() + { + using TempFolder folder = new(); + string gateway = CreateGatewayFolder(folder.Path); + string outside = CreateProtectedFile(Path.Combine(folder.CreateDirectory("outside"), "file.txt")); + string before = FileSddl(outside); + TempFolder.CreateHardLink(Path.Combine(gateway, "users.txt"), outside); + + Assert.Throws(() => SecureDirectory.SetFileSecurity(folder.Path, UsersFileNames, ProtectedSddl)); + + Assert.Equal(before, FileSddl(outside)); + } + + // The installer accepts a missing users database, which only exists once the web application has a user, by + // catching this error. + // oracle: mutant a missing users database failing the install + [Theory] + [InlineData("missing file")] + [InlineData("missing folder", "Devolutions")] + public void SetFileSecurityReportsAMissingFileAsNotFound(string scenario, params string[] existing) + { + using TempFolder folder = new(); + folder.CreateDirectory(existing); + + Win32Exception e = Assert.Throws( + () => SecureDirectory.SetFileSecurity(folder.Path, UsersFileNames, ProtectedSddl)); + + Assert.True(SecureDirectory.IsNotFound(e), $"{scenario}: {e.NativeErrorCode}"); + } + + /// + /// Asserts that , given , gives + /// Gateway, the folders Gateway\ and a file in the last one the DACLs + /// that .NET gives the same entries in an identical tree when it sets the DACL of Gateway, which Windows + /// passes on, then sets each folder below its own DACL again, which Windows passes on, and removes the protection + /// of the file, from the top down. + /// + /// + /// Changes each tree, given the path of Gateway and of the file, before the DACLs are set. + /// + /// The security descriptor Gateway is created with. + private void AssertInheritsAsWindows( + string scenario, string[] subfolders, Action prepare, string sddl, string treeSddl = null) + { + using TempFolder folder = new(); + string actual = Path.Combine(folder.Path, "actual"); + string expected = Path.Combine(folder.Path, "expected"); + CreateTree(actual, subfolders, prepare, treeSddl ?? TreeSddl); + CreateTree(expected, subfolders, prepare, treeSddl ?? TreeSddl); + string[] directories = Enumerable.Range(0, subfolders.Length + 1) + .Select(depth => Path.Combine([.. GatewayNames, .. subfolders.Take(depth)])) + .ToArray(); + string file = Path.Combine([.. GatewayNames, .. subfolders, "file.txt"]); + + // .NET reorders the entries of a DACL it sets and splits an entry with a generic right, so both trees get the + // DACL in the form .NET writes. + DirectorySecurity gateway = new(); + gateway.SetSecurityDescriptorSddlForm(sddl, AccessControlSections.Owner | AccessControlSections.Access); + string written = gateway.GetSecurityDescriptorSddlForm(AccessControlSections.Owner | AccessControlSections.Access); + + this.SetSecurityAndInheritBelow(actual, written); + + Directory.SetAccessControl(Path.Combine(expected, directories[0]), gateway); + + // Top-down, because SetNamedSecurityInfo computes what an entry inherits from its parent's current DACL. + // A protected folder keeps its DACL, and an unprotected one gets what its parent now passes on. + foreach (string directory in directories.Skip(1)) + { + DirectorySecurity reference = Directory.GetAccessControl(Path.Combine(expected, directory)); + Directory.SetAccessControl(Path.Combine(expected, directory), reference); + } + + FileSecurity fileReference = File.GetAccessControl(Path.Combine(expected, file)); + fileReference.SetAccessRuleProtection(false, false); + File.SetAccessControl(Path.Combine(expected, file), fileReference); + + string[] Sddls(string root) => + [ + .. directories.Select(directory => DirectorySddl(Path.Combine(root, directory))), + FileSddl(Path.Combine(root, file)), + ]; + + Assert.True( + Sddls(expected).SequenceEqual(Sddls(actual)), + $"{scenario}: {string.Join(" ", Sddls(expected))} {string.Join(" ", Sddls(actual))}"); + } + + /// + /// Calls on Devolutions\Gateway under + /// , opened as the installer opens it, keeping the entries named . + /// + private void SetSecurityAndInheritBelow(string root, string sddl, params string[] kept) + { + string gateway = Path.Combine(root, "Devolutions", "Gateway"); + using SafeFileHandle handle = SecureDirectory.HoldToSetSecurity(gateway); + SecureDirectory.SetSecurityAndInheritBelow(handle, gateway, sddl, kept, this.logger); + } + + /// + /// Creates Devolutions\Gateway\ under , with the security + /// descriptor on Gateway, and a file in the last folder, then calls + /// with the paths of Gateway and of the file. + /// + /// The path of the file. + private static string CreateTree(string root, string[] subfolders, Action prepare, string sddl) + { + Directory.CreateDirectory(Path.Combine(root, "Devolutions")); + string gateway = Path.Combine(root, "Devolutions", "Gateway"); + Assert.True(SecureDirectory.Create(gateway, sddl)); + string path = gateway; + + foreach (string subfolder in subfolders) + { + path = Path.Combine(path, subfolder); + Directory.CreateDirectory(path); + } + + string file = Path.Combine(path, "file.txt"); + File.WriteAllText(file, ""); + prepare(gateway, file); + + return file; + } + + /// + /// Creates Devolutions\Gateway under , inheriting the temporary directory's DACL. + /// + private static string CreateGatewayFolder(string root) => + Directory.CreateDirectory(Path.Combine(root, "Devolutions", "Gateway")).FullName; + + /// + /// Creates the file with a protected DACL that grants full control to the current user and + /// read access to Users. + /// + private static string CreateProtectedFile(string path) + { + File.WriteAllText(path, ""); + ProtectFile(path); + return path; + } + + /// + /// Gives the file a protected DACL that grants full control to the current user and read + /// access to Users. + /// + private static void ProtectFile(string path) + { + FileSecurity security = new(); + security.SetSecurityDescriptorSddlForm($"D:P(A;;FA;;;{CurrentUser})(A;;FR;;;BU)", AccessControlSections.Access); + File.SetAccessControl(path, security); + Assert.True(File.GetAccessControl(path).AreAccessRulesProtected); + } + + /// + /// Gives the directory a protected DACL that grants full control to the current user and + /// read access to Users. + /// + private static void ProtectDirectory(string path) + { + DirectorySecurity security = new(); + security.SetSecurityDescriptorSddlForm( + $"D:P(A;OICI;FA;;;{CurrentUser})(A;OICI;FR;;;BU)", + AccessControlSections.Access); + Directory.SetAccessControl(path, security); + Assert.True(Directory.GetAccessControl(path).AreAccessRulesProtected); + } + + /// + /// Whether an allow entry of names . + /// + private static bool Allows(SecurityIdentifier account, FileSystemSecurity security) => + security.GetAccessRules(true, true, typeof(SecurityIdentifier)) + .Cast() + .Any(rule => rule.IdentityReference.Equals(account) && rule.AccessControlType == AccessControlType.Allow); + + /// + /// Asserts that a directory opens can't be renamed while the handle is open, and can once it + /// is closed. + /// + private static void AssertHoldsUntilClosed(Func hold) + { + using TempFolder folder = new(); + string path = folder.CreateDirectory("held"); + string renamed = Path.Combine(folder.Path, "renamed"); + + using (hold(path)) + { + Assert.Throws(() => Directory.Move(path, renamed)); + Assert.True(Directory.Exists(path)); + } + + Directory.Move(path, renamed); + Assert.True(Directory.Exists(renamed)); + } + + /// + /// Asserts that has exactly the owner and the protected DACL of + /// , with no inherited entry. + /// + private static void AssertProtectedSecurity(FileSystemSecurity security) + { + Assert.Equal(CurrentUser, security.GetOwner(typeof(SecurityIdentifier))); + Assert.True(security.AreAccessRulesProtected); + + string[] rules = security.GetAccessRules(true, true, typeof(SecurityIdentifier)) + .Cast() + .Select(r => $"{r.IdentityReference} {r.AccessControlType} {r.FileSystemRights} inherited={r.IsInherited}") + .OrderBy(r => r, StringComparer.Ordinal) + .ToArray(); + string[] expected = new[] { CurrentUser, LocalSystem } + .Select(sid => $"{sid} Allow FullControl inherited=False") + .OrderBy(r => r, StringComparer.Ordinal) + .ToArray(); + + Assert.Equal(expected, rules); + } + + /// + /// The owner and DACL of , following a link at that path. + /// + private static string Sddl(string path) => + new DirectoryInfo(path) + .GetAccessControl(AccessControlSections.Owner | AccessControlSections.Access) + .GetSecurityDescriptorSddlForm(AccessControlSections.Owner | AccessControlSections.Access); + + /// + /// The owner, group and DACL of the file , with the DACL's control flags. + /// + private static string FileSddl(string path) => + File.GetAccessControl(path) + .GetSecurityDescriptorSddlForm(AccessControlSections.Owner | AccessControlSections.Group | AccessControlSections.Access); + + /// + /// The owner, group and DACL of the directory , with the DACL's control flags. + /// + private static string DirectorySddl(string path) => + Directory.GetAccessControl(path) + .GetSecurityDescriptorSddlForm(AccessControlSections.Owner | AccessControlSections.Group | AccessControlSections.Access); + + /// + /// The owner and DACL of the directory as stored, without the canonical ordering that + /// applies when it reads them. + /// + private static string RawSddl(string path) + { + using SafeFileHandle handle = SecureDirectory.OpenToRead(path, hold: false); + return SecureDirectory.ReadSecurity(handle).Security + .GetSddlForm(AccessControlSections.Owner | AccessControlSections.Access); + } +} diff --git a/package/WindowsManaged.UnitTests/TempFolder.cs b/package/WindowsManaged.UnitTests/TempFolder.cs new file mode 100644 index 0000000000..71ddbc0bbe --- /dev/null +++ b/package/WindowsManaged.UnitTests/TempFolder.cs @@ -0,0 +1,79 @@ +using System; +using System.Diagnostics; +using System.IO; +using System.Runtime.InteropServices; + +using Xunit; + +namespace DevolutionsGateway.Installer.Tests; + +/// +/// A new, empty folder under the temporary directory, deleted with its content on disposal. +/// +internal sealed class TempFolder : IDisposable +{ + public string Path { get; } = System.IO.Path.Combine(System.IO.Path.GetTempPath(), $"installer-tests-{Guid.NewGuid():N}"); + + public TempFolder() => Directory.CreateDirectory(this.Path); + + /// + /// Creates the directory below this folder, with its parents. + /// + public string CreateDirectory(params string[] names) => + Directory.CreateDirectory(System.IO.Path.Combine([this.Path, .. names])).FullName; + + /// + /// Creates a junction at to the directory , which needs no + /// elevation. + /// + public static void CreateJunction(string link, string target) + { + ProcessStartInfo startInfo = new("cmd.exe", $"/c mklink /J \"{link}\" \"{target}\"") + { + UseShellExecute = false, + CreateNoWindow = true, + RedirectStandardOutput = true, + RedirectStandardError = true, + }; + + using Process process = Process.Start(startInfo); + string output = process.StandardOutput.ReadToEnd() + process.StandardError.ReadToEnd(); + process.WaitForExit(); + + Assert.True(process.ExitCode == 0, output); + } + + /// + /// Creates a hard link at to the file , which needs no elevation. + /// + public static void CreateHardLink(string link, string target) + { + Assert.True(CreateHardLinkW(link, target, IntPtr.Zero), $"CreateHardLink failed: {Marshal.GetLastWin32Error()}"); + } + + [DllImport("kernel32", CharSet = CharSet.Unicode, SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + private static extern bool CreateHardLinkW(string lpFileName, string lpExistingFileName, IntPtr lpSecurityAttributes); + + public void Dispose() + { + // A recursive Directory.Delete fails on a junction without elevation, so junctions go first. + RemoveJunctions(this.Path); + Directory.Delete(this.Path, recursive: true); + } + + private static void RemoveJunctions(string directory) + { + foreach (string entry in Directory.GetDirectories(directory)) + { + if (File.GetAttributes(entry).HasFlag(FileAttributes.ReparsePoint)) + { + Directory.Delete(entry); + } + else + { + RemoveJunctions(entry); + } + } + } +} diff --git a/package/WindowsManaged/Actions/CustomActions.cs b/package/WindowsManaged/Actions/CustomActions.cs index 208c1fe337..9ab61638db 100644 --- a/package/WindowsManaged/Actions/CustomActions.cs +++ b/package/WindowsManaged/Actions/CustomActions.cs @@ -23,6 +23,7 @@ using System.Security.Principal; using System.ServiceProcess; using System.Text; +using System.Text.RegularExpressions; using System.Threading; using System.Threading.Tasks; using WixSharp; @@ -468,6 +469,24 @@ public static ActionResult ConfigureWebAppUser(Session session) return ExecuteCommand(session, command); } + [CustomAction] + public static ActionResult CheckProgramDataDirectory(Session session) + { + try + { + DataFolderRefusal refusal = DataFolders.Check( + Environment.GetFolderPath(Environment.SpecialFolder.CommonApplicationData), + LogDelegate.WithSession(session)); + + return refusal is null ? ActionResult.Success : Refuse(session, refusal); + } + catch (Exception e) + { + session.Log($"failed to check {ProgramDataDirectory}: {e}"); + return ActionResult.Failure; + } + } + [CustomAction] public static ActionResult CreateProgramDataDirectory(Session session) { @@ -475,10 +494,17 @@ public static ActionResult CreateProgramDataDirectory(Session session) try { - if (!Directory.Exists(path)) + DataFolderRefusal refusal = DataFolders.Create( + Environment.GetFolderPath(Environment.SpecialFolder.CommonApplicationData), + Includes.ProgramDataSddl(GetServiceAccountSid(session)), + LogDelegate.WithSession(session)); + + if (refusal is not null) { - Directory.CreateDirectory(path); + return Refuse(session, refusal); } + + session.Log($"created directory at {path} or already exists"); } catch (Exception e) { @@ -489,6 +515,34 @@ public static ActionResult CreateProgramDataDirectory(Session session) return ActionResult.Success; } + /// + /// Logs the error for in the installer's language and shows it, which a silent install + /// only logs. + /// + private static ActionResult Refuse(Session session, DataFolderRefusal refusal) + { + // UNTESTED: a refusal is sent as an error message, which a silent install logs and an interactive one shows. + // No test runs the data folder actions, which read the machine's own ProgramData. + // When changing this, keep sending it with InstallMessage.Error, in a record built by MessageRecord so that the + // installer shows the text as written. + string message = refusal.Message(session.Property("ProductLanguage")); + session.Log(message); + using Record record = MessageRecord(message); + session.Message(InstallMessage.Error | (uint)MessageButtons.OK, record); + return ActionResult.Failure; + } + + /// + /// A record that the installer formats to as written, rather than reading brackets in it + /// as property references. + /// + /// + /// The installer replaces a property reference in a field it inserts as well as in the format string, so each + /// bracket is escaped instead. + /// + internal static Record MessageRecord(string text) => + new(0) { FormatString = Regex.Replace(text, @"[\[\]]", m => $"[\\{m.Value}]") }; + [CustomAction] public static ActionResult GetInstallDirFromRegistry(Session session) { @@ -901,43 +955,6 @@ public static ActionResult SetInstallId(Session session) return ActionResult.Success; } - [CustomAction] - public static ActionResult SetProgramDataDirectoryPermissions(Session session) - { - try - { - SetFileSecurity(session, ProgramDataDirectory, Includes.ProgramDataSddl(GetServiceAccountSid(session))); - - // Files created before the service account was granted access to the program data directory - // don't retroactively inherit the new ACE - // We fix this by removing access rule protection on the files - // and then reapplying the ACL - DirectoryInfo dir = new DirectoryInfo(ProgramDataDirectory); - - foreach (FileInfo file in dir.GetFiles("*", SearchOption.AllDirectories)) - { - try - { - FileSecurity fileSecurity = file.GetAccessControl(); - fileSecurity.SetAccessRuleProtection(false, false); - file.SetAccessControl(fileSecurity); - } - catch (Exception e) - { - session.Log($"failed to reset permissions on path {file.FullName}: {e}"); - return ActionResult.Failure; - } - } - - return ActionResult.Success; - } - catch (Exception e) - { - session.Log($"failed to set permissions: {e}"); - return ActionResult.Failure; - } - } - [CustomAction] public static ActionResult SetCertificatePrivateKeyPermissions(Session session) { @@ -1030,13 +1047,15 @@ public static ActionResult SetUsersDatabaseFilePermissions(Session session) { try { - string path = Path.Combine(ProgramDataDirectory, DefaultUsersFile); - if (!File.Exists(path)) - { - session.Log("no users database; no user file permissions to update"); - return ActionResult.Success; - } - SetFileSecurity(session, path, Includes.UsersFileSddl(GetServiceAccountSid(session))); + SecureDirectory.SetFileSecurity( + Environment.GetFolderPath(Environment.SpecialFolder.CommonApplicationData), + ["Devolutions", "Gateway", DefaultUsersFile], + Includes.UsersFileSddl(GetServiceAccountSid(session))); + return ActionResult.Success; + } + catch (Win32Exception e) when (SecureDirectory.IsNotFound(e)) + { + session.Log($"no users database; no user file permissions to update: {e.Message}"); return ActionResult.Success; } catch (Exception e) @@ -1290,35 +1309,6 @@ private static string FormatPowerShellCommand(Session session, string command) return $"\"{session.Property(GatewayProperties.powerShellPath.Id)}\" -ep Bypass -Command \"& Import-Module '{session.Property(GatewayProperties.InstallDir)}PowerShell\\Modules\\DevolutionsGateway'; {command}\""; } - public static void SetFileSecurity(Session session, string path, string sddl) - { - const uint sdRevision = 1; - IntPtr pSd = new IntPtr(); - UIntPtr pSzSd = new UIntPtr(); - - try - { - if (!WinAPI.ConvertStringSecurityDescriptorToSecurityDescriptorW(sddl, sdRevision, out pSd, out pSzSd)) - { - session.Log($"ConvertStringSecurityDescriptorToSecurityDescriptorW failed (error: {Marshal.GetLastWin32Error()})"); - throw new Win32Exception(Marshal.GetLastWin32Error()); - } - - if (!WinAPI.SetFileSecurityW(path, WinAPI.DACL_SECURITY_INFORMATION, pSd)) - { - session.Log($"SetFileSecurityW failed (error: {Marshal.GetLastWin32Error()})"); - throw new Win32Exception(Marshal.GetLastWin32Error()); - } - } - finally - { - if (pSd != IntPtr.Zero) - { - WinAPI.LocalFree(pSd); - } - } - } - private static bool TryGetGatewayStartupType(Session session, out ServiceStartMode startMode) { startMode = ServiceStartMode.Disabled; diff --git a/package/WindowsManaged/Actions/DataFolders.cs b/package/WindowsManaged/Actions/DataFolders.cs new file mode 100644 index 0000000000..0242b665b4 --- /dev/null +++ b/package/WindowsManaged/Actions/DataFolders.cs @@ -0,0 +1,392 @@ +using DevolutionsGateway.Resources; +using Microsoft.Win32.SafeHandles; +using System; +using System.ComponentModel; +using System.IO; +using System.Security.AccessControl; +using System.Security.Principal; + +namespace DevolutionsGateway.Actions; + +/// +/// Why the installer refuses to use a folder of the Gateway's data path. +/// +internal enum DataFolderProblem +{ + /// + /// The folder is a link, which can point anywhere. + /// + Link, + + /// + /// The entry is a file, not a folder. + /// + File, + + /// + /// The folder's owner, who can always change its permissions, is not trusted. + /// + Owner, + + /// + /// The folder's DACL lets an untrusted account replace the folder or what it contains. + /// + Access, + + /// + /// The folder's owner or DACL can't be read. + /// + Unreadable, +} + +/// +/// A folder of the Gateway's data path that the installer refuses to use. +/// +internal sealed class DataFolderRefusal(string folder, DataFolderProblem problem, SecurityIdentifier account = null) +{ + internal string Folder { get; } = folder; + + internal DataFolderProblem Problem { get; } = problem; + + /// + /// The owner of the folder for , or the account the offending entry grants for + /// . + /// + internal SecurityIdentifier Account { get; } = account; + + /// + /// The error the installer shows, in the language whose LCID is . + /// + internal string Message(string productLanguage) + { + string id = this.Problem switch + { + DataFolderProblem.Link => Strings.DataFolderIsLink, + DataFolderProblem.File => Strings.DataFolderIsFile, + DataFolderProblem.Owner => Strings.DataFolderOwnedByUntrustedAccount, + DataFolderProblem.Access => Strings.DataFolderReplaceableByUntrustedAccount, + DataFolderProblem.Unreadable => Strings.DataFolderSecurityUnreadable, + _ => throw new ArgumentOutOfRangeException(nameof(this.Problem), this.Problem, null), + }; + + return string.Format(InstallerStrings.Localize(productLanguage, id), this.Folder, AccountName(this.Account)); + } + + /// + /// The account name of , or its SID string when it has none. + /// + private static string AccountName(SecurityIdentifier sid) + { + if (sid is null) + { + return null; + } + + try + { + return sid.Translate(typeof(NTAccount)).Value; + } + catch (Exception) + { + return sid.Value; + } + } +} + +/// +/// Checks Devolutions and Devolutions\Gateway under ProgramData before the installer uses them. +/// +/// +/// The installer writes the Gateway's data there as SYSTEM, so an account that controls either folder could redirect +/// or replace what the installer and the Gateway write. +/// A folder that doesn't exist yet passes. +/// +internal static class DataFolders +{ + private const string Devolutions = "Devolutions"; + + private const string Gateway = "Gateway"; + + /// + /// The rights that let an account delete, rename or replace a folder or an entry in it, or give itself any right. + /// + private const uint ReplaceRights = + WinAPI.DELETE | WinAPI.WRITE_DAC | WinAPI.WRITE_OWNER | WinAPI.FILE_DELETE_CHILD | WinAPI.GENERIC_ALL; + + /// + /// OWNER RIGHTS, whose entry applies to the owner, which is checked on its own. + /// + private static readonly SecurityIdentifier OwnerRights = new("S-1-3-4"); + + /// + /// Checks Devolutions and Devolutions\Gateway under without changing anything. + /// + /// The first refusal, or null when both folders pass. + internal static DataFolderRefusal Check(string root, ILogger logger) + { + string devolutions = Path.Combine(root, Devolutions); + DataFolderRefusal refusal = Inspect( + devolutions, path => SecureDirectory.OpenToRead(path, hold: true), checkAccess: true, logger, + out SafeFileHandle handle); + + using (handle) + { + if (refusal is not null || handle is null) + { + return refusal; + } + + // UNTESTED: an existing Gateway is refused when it is a link or has an untrusted owner, here and in Create. + // A test reaches it only below a Devolutions folder that passes, which needs an owner that a test without + // elevation can't set. + // When changing this, keep Gateway checked in both, without the DACL rule, as the installer resets its DACL. + refusal = Inspect( + Path.Combine(devolutions, Gateway), path => SecureDirectory.OpenToRead(path, hold: false), + checkAccess: false, logger, out SafeFileHandle gateway); + gateway?.Dispose(); + + return refusal; + } + } + + /// + /// Creates Devolutions\Gateway under with the security descriptor + /// , after checking Devolutions, or checks an existing Gateway and resets its + /// owner and DACL to those of . + /// + /// + /// A missing Devolutions is created with the DACL it inherits, then checked like an existing one. + /// Gateway gets its security descriptor when it is created, so that it never exists with the DACL it would + /// inherit from ProgramData, which lets users add entries. + /// The entries below an existing Gateway get what it now passes on, and lose what it passed on before, which + /// only its DACL before the reset tells apart from their explicit entries. + /// The users database directly below keeps its security here, because another action replaces it, and the entries + /// Gateway passes on would let users read it until then. + /// + /// + /// The first refusal, in which case nothing is created or changed below Devolutions, or null. + /// + internal static DataFolderRefusal Create(string root, string sddl, ILogger logger) + { + string devolutions = Path.Combine(root, Devolutions); + string gateway = Path.Combine(devolutions, Gateway); + + SecureDirectory.Create(devolutions, sddl: null); + + // UNTESTED: the handle on Devolutions stays open until Gateway is created, or checked and reset, below it, so + // that this happens in the folder just checked, not in one put at that path afterwards. + // Tests check that a held folder can't be renamed, not how long this handle stays open. + // When changing this, keep the handle open until Gateway is done. + DataFolderRefusal refusal = Inspect( + devolutions, path => SecureDirectory.OpenToRead(path, hold: true), checkAccess: true, logger, + out SafeFileHandle handle); + + using (handle) + { + if (refusal is not null) + { + return refusal; + } + + if (handle is null) + { + throw new IOException($"{devolutions} was deleted after it was created"); + } + + if (SecureDirectory.Create(gateway, sddl)) + { + logger.Log($"created {gateway}"); + return null; + } + + refusal = Inspect( + gateway, SecureDirectory.HoldToSetSecurity, checkAccess: false, logger, out SafeFileHandle existing); + + using (existing) + { + if (refusal is not null) + { + return refusal; + } + + if (existing is null) + { + throw new IOException($"{gateway} was deleted after it was found"); + } + + // UNTESTED: setting the owner to SYSTEM succeeds only for a caller allowed to give an object another + // owner, as the installer's SYSTEM process is; tests run as a user and set the user as the owner. + // When changing this, keep resetting the security of an existing Gateway on this handle, so that only + // the accounts the installer provisions can change what the Gateway stores there. + // Files and folders created before Gateway got its current DACL keep the entries they inherited then, + // such as none for a service account chosen later, or one for an account that no longer has access, + // so each one gets the entries Gateway now passes on, and loses the ones it passed on before. + // UNTESTED: the entries below an existing Gateway lose what it passed on before only when they are + // reset in the same call that changes its DACL, while its previous DACL can still be read. + // Tests check SetSecurityAndInheritBelow, not that an existing Gateway is reset with it here, because + // the user running them isn't a trusted owner. + // When changing this, reset the entries below in the same call that changes the DACL of Gateway, and + // keep the users database out of it, so that it never gets what Gateway passes on to users. + SecureDirectory.SetSecurityAndInheritBelow( + existing, gateway, sddl, [CustomActions.DefaultUsersFile], logger); + logger.Log($"reset the owner and the DACL of {gateway} and the entries below it"); + + return null; + } + } + } + + /// + /// Whether the installer refuses to use , given its security descriptor and whether it is a + /// link. + /// + /// + /// The DACL rule trusts the accounts the owner rule trusts, because an account that may own the folder gains nothing + /// from an entry it could give itself as the owner, and a Devolutions folder that an administrator's + /// application created first, without elevation, carries a full-control entry for that administrator. + /// + /// + /// Whether to refuse a DACL that lets an account other than SYSTEM, Administrators, TrustedInstaller, a member of + /// Administrators or OWNER RIGHTS replace the folder or what it contains. + /// + /// The refusal, or null when the folder passes. + internal static DataFolderRefusal Evaluate( + string folder, RawSecurityDescriptor security, bool isLink, bool isDirectory, bool checkAccess, ILogger logger) + { + if (isLink) + { + return new DataFolderRefusal(folder, DataFolderProblem.Link); + } + + if (!isDirectory) + { + return new DataFolderRefusal(folder, DataFolderProblem.File); + } + + if (security.Owner is null) + { + return new DataFolderRefusal(folder, DataFolderProblem.Unreadable); + } + + // The DACL is checked before the owner, so that a refusal names an account that may replace the folder first. + if (checkAccess) + { + // A null DACL grants every right to everyone. + if (security.DiscretionaryAcl is null) + { + return new DataFolderRefusal( + folder, DataFolderProblem.Access, new SecurityIdentifier(WellKnownSidType.WorldSid, null)); + } + + foreach (GenericAce ace in security.DiscretionaryAcl) + { + switch (ace.AceType) + { + // A deny entry only takes access away. + case AceType.AccessDenied: + case AceType.AccessDeniedCallback: + case AceType.AccessDeniedObject: + case AceType.AccessDeniedCallbackObject: + continue; + case AceType.AccessAllowed: + case AceType.AccessAllowedCallback: + break; + // An entry of another type, such as an object entry, which directory service objects use, isn't + // expected on a folder, so the installer refuses rather than guesses what it grants. + default: + return new DataFolderRefusal(folder, DataFolderProblem.Unreadable); + } + + // An inherit-only entry applies to the entries created below the folder, not to the folder. + if ((ace.AceFlags & AceFlags.InheritOnly) != 0) + { + continue; + } + + CommonAce entry = (CommonAce)ace; + + // UNTESTED: an entry for a member of Administrators, such as an administrator's user account, is trusted + // as the owner rule trusts it; no test runs with such an account. + // When changing this, keep trusting in an entry exactly the accounts trusted as the owner. + if ((unchecked((uint)entry.AccessMask) & ReplaceRights) != 0 + && entry.SecurityIdentifier != OwnerRights + && !IsAppContainer(entry.SecurityIdentifier) + && !IsTrusted(entry.SecurityIdentifier, logger)) + { + return new DataFolderRefusal(folder, DataFolderProblem.Access, entry.SecurityIdentifier); + } + } + } + + return IsTrusted(security.Owner, logger) + ? null + : new DataFolderRefusal(folder, DataFolderProblem.Owner, security.Owner); + } + + /// + /// Whether may own the Gateway's data, as + /// decides. + /// + private static bool IsTrusted(SecurityIdentifier sid, ILogger logger) + { + try + { + return SecureDirectory.IsTrustedOwner(sid, logger); + } + catch (Exception e) + { + // An account whose group membership can't be resolved isn't known to be an administrator. + logger.Log($"failed to tell whether {sid} is a member of Administrators, so it isn't trusted: {e}"); + return false; + } + } + + /// + /// Opens the entry names with , which opens a link itself rather than + /// its target, and evaluates its security. + /// + /// The open entry, or null when nothing exists at . + /// The refusal, or null when the entry passes or doesn't exist. + private static DataFolderRefusal Inspect( + string path, Func open, bool checkAccess, ILogger logger, out SafeFileHandle handle) + { + handle = null; + + try + { + handle = open(path); + (RawSecurityDescriptor security, bool isLink, bool isDirectory) = SecureDirectory.ReadSecurity(handle); + DataFolderRefusal refusal = Evaluate(path, security, isLink, isDirectory, checkAccess, logger); + + logger.Log(refusal is null + ? $"checked {path}" + : $"refused {path}: {refusal.Problem} {refusal.Account}"); + + return refusal; + } + catch (Win32Exception e) when (handle is null && SecureDirectory.IsNotFound(e)) + { + return null; + } + catch (Exception e) + { + logger.Log($"failed to read the owner and the DACL of {path}: {e}"); + return new DataFolderRefusal(path, DataFolderProblem.Unreadable); + } + } + + /// + /// Whether is an AppContainer package SID (S-1-15-2-…) or capability SID + /// (S-1-15-3-…). + /// + /// + /// An entry for one never grants access by itself: Windows grants an AppContainer process only the access that both + /// its user and its package and capabilities are granted, and no other process holds these SIDs. + /// + private static bool IsAppContainer(SecurityIdentifier sid) + { + // SECURITY_APP_PACKAGE_AUTHORITY, then SECURITY_APP_PACKAGE_BASE_RID or SECURITY_CAPABILITY_BASE_RID. + string[] parts = sid.Value.Split('-'); + return parts.Length > 3 && parts[2] == "15" && parts[3] is "2" or "3"; + } +} diff --git a/package/WindowsManaged/Actions/GatewayActions.cs b/package/WindowsManaged/Actions/GatewayActions.cs index aa732d135c..c9394a8982 100644 --- a/package/WindowsManaged/Actions/GatewayActions.cs +++ b/package/WindowsManaged/Actions/GatewayActions.cs @@ -148,34 +148,52 @@ internal static class GatewayActions // Deferred sequence /// - /// Create the path %programdata%\Devolutions\Gateway if it does not exist + /// The action data that passes the installer's language to a deferred action, which shows its errors in it. + /// + private const string ProductLanguage = "ProductLanguage=[ProductLanguage]"; + + /// + /// Fail the run, with an error naming the folder, when %programdata%\Devolutions or %programdata%\Devolutions\Gateway + /// is a link or is controlled by an account that is not an administrator + /// + /// + /// The major upgrade removes the installed product at RemoveExistingProducts, right after InstallInitialize, so + /// running before it fails an install, an upgrade, a repair or a maintenance run before it changes anything. + /// A full uninstall is never refused, so that a machine can always be uninstalled. + /// + internal static readonly ElevatedManagedAction checkProgramDataDirectory = new( + new Id($"CA.{nameof(checkProgramDataDirectory)}"), + CustomActions.CheckProgramDataDirectory, + Return.check, + When.Before, Step.RemoveExistingProducts, + Condition.NOT_BeingRemoved, + Sequence.InstallExecuteSequence) + { + Execute = Execute.deferred, + Impersonate = false, + UsesProperties = ProductLanguage, + }; + + /// + /// Create the path %programdata%\Devolutions\Gateway with its owner and ACL if it does not exist, or reset both on + /// an existing one and let the files and folders below inherit its entries, after checking its folders again as + /// does /// /// /// It's hard to tell the installer not to remove directories on uninstall. Since we want this folder to persist, /// it's easy to create it with a custom action than workaround Windows Installer. /// - private static readonly ElevatedManagedAction createProgramDataDirectory = new( + internal static readonly ElevatedManagedAction createProgramDataDirectory = new( new Id($"CA.{nameof(createProgramDataDirectory)}"), CustomActions.CreateProgramDataDirectory, Return.check, When.After, Step.CreateFolders, - Condition.Always, - Sequence.InstallExecuteSequence); - - /// - /// Set or reset the ACL on %programdata%\Devolutions\Gateway - /// - private static readonly ElevatedManagedAction setProgramDataDirectoryPermissions = new( - new Id($"CA.{nameof(setProgramDataDirectoryPermissions)}"), - CustomActions.SetProgramDataDirectoryPermissions, - Return.check, - When.After, new Step(createProgramDataDirectory.Id), Condition.NOT_BeingRemoved, Sequence.InstallExecuteSequence) { Execute = Execute.deferred, Impersonate = false, - UsesProperties = UseProperties(new IWixProperty[] { GatewayProperties.serviceAccountSid }), + UsesProperties = $"{UseProperties(new IWixProperty[] { GatewayProperties.serviceAccountSid })};{ProductLanguage}", }; /// @@ -556,8 +574,8 @@ private static ElevatedManagedAction BuildConfigureAction( queryGatewayServiceAccount, validateServiceAccount, setArpInstallLocation, + checkProgramDataDirectory, createProgramDataDirectory, - setProgramDataDirectoryPermissions, setUserDatabasePermissions, cleanGatewayConfigIfNeeded, cleanGatewayConfigIfNeededRollback, diff --git a/package/WindowsManaged/Actions/InstallerStrings.cs b/package/WindowsManaged/Actions/InstallerStrings.cs new file mode 100644 index 0000000000..bf7f472f5f --- /dev/null +++ b/package/WindowsManaged/Actions/InstallerStrings.cs @@ -0,0 +1,58 @@ +using System; +using System.IO; +using System.Linq; +using System.Reflection; +using System.Xml; + +namespace DevolutionsGateway.Actions; + +/// +/// The installer strings, for custom actions, which the managed UI's localization isn't available to. +/// +internal static class InstallerStrings +{ + private const string English = "1033"; + + /// + /// The installer string in the language whose LCID is , or + /// in English when the installer doesn't have that language. + /// + /// + /// Custom actions pass the ProductLanguage property, which the language transform Windows Installer applied + /// sets, so that their messages match the rest of the installer. + /// + /// The string, or when no localization file has it. + internal static string Localize(string productLanguage, string id) + { + XmlDocument strings = Load(productLanguage) ?? Load(English); + XmlNode node = strings?.GetElementsByTagName("String") + .Cast() + .FirstOrDefault(s => s.Attributes["Id"]?.Value == id); + + return node?.InnerText ?? id; + } + + /// + /// The localization file embedded in this assembly whose language is the LCID , or + /// null when there is none. + /// + private static XmlDocument Load(string language) + { + Assembly assembly = Assembly.GetExecutingAssembly(); + + foreach (string name in assembly.GetManifestResourceNames() + .Where(n => n.EndsWith(".wxl", StringComparison.OrdinalIgnoreCase))) + { + using Stream stream = assembly.GetManifestResourceStream(name); + XmlDocument xml = new(); + xml.Load(stream); + + if (xml.DocumentElement?.GetAttribute("Language") == language) + { + return xml; + } + } + + return null; + } +} diff --git a/package/WindowsManaged/Actions/SecureDirectory.cs b/package/WindowsManaged/Actions/SecureDirectory.cs new file mode 100644 index 0000000000..ce30544072 --- /dev/null +++ b/package/WindowsManaged/Actions/SecureDirectory.cs @@ -0,0 +1,803 @@ +using Microsoft.Win32.SafeHandles; +using System; +using System.Collections.Generic; +using System.ComponentModel; +using System.IO; +using System.Linq; +using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; +using System.Security.AccessControl; +using System.Security.Principal; + +[assembly: InternalsVisibleTo("DevolutionsGateway.Installer.Tests")] + +namespace DevolutionsGateway.Actions; + +/// +/// Directory and file operations that act on the entry a path names, never on the target of a link at that path. +/// +/// +/// Each path is opened with FILE_FLAG_OPEN_REPARSE_POINT, which opens a junction or a symbolic link itself, and +/// every decision is taken from the opened handle, so a link swapped in after a check is never followed. +/// +internal static class SecureDirectory +{ + private static readonly SecurityIdentifier Administrators = new(WellKnownSidType.BuiltinAdministratorsSid, null); + + private static readonly SecurityIdentifier LocalService = new(WellKnownSidType.LocalServiceSid, null); + + /// + /// NT SERVICE\TrustedInstaller, the Windows Modules Installer service. + /// + private static readonly SecurityIdentifier TrustedInstaller = + new("S-1-5-80-956008885-3418522649-1831038044-1853292631-2271478464"); + + /// + /// The share mode of a handle that holds its entry: renaming or replacing an entry needs delete access to it, which + /// this denies, so no path through a held directory can be redirected to another folder. + /// + private static readonly uint HeldShare = WinAPI.FILE_SHARE_READ | WinAPI.FILE_SHARE_WRITE; + + private static readonly uint AnyShare = WinAPI.FILE_SHARE_READ | WinAPI.FILE_SHARE_WRITE | WinAPI.FILE_SHARE_DELETE; + + private const uint OwnerAndDacl = WinAPI.OWNER_SECURITY_INFORMATION | WinAPI.DACL_SECURITY_INFORMATION; + + private const uint OwnerGroupAndDacl = OwnerAndDacl | WinAPI.GROUP_SECURITY_INFORMATION; + + /// + /// Creates the directory with the security descriptor , so that it + /// never exists with the DACL it would inherit from its parent, or with the security it inherits when + /// is null. + /// + /// false when an entry named already exists, which is left unchanged. + internal static bool Create(string path, string sddl) + { + using SecurityDescriptor descriptor = sddl is null ? null : new(sddl); + bool autoInherited = descriptor is not null && RequestAutoInherit(descriptor, sddl); + WinAPI.SECURITY_ATTRIBUTES attributes = new() + { + nLength = (uint)Marshal.SizeOf(), + lpSecurityDescriptor = descriptor?.Pointer ?? IntPtr.Zero, + }; + + if (WinAPI.CreateDirectory(path, ref attributes)) + { + // CreateDirectory stores the DACL without marking it auto-inherited, even with SE_DACL_AUTO_INHERIT_REQ set, + // so the DACL is set again through the new directory's handle; until then, the directory has the same + // entries without the mark. + // The handle doesn't hold the directory, because the check and the set both act on the entry it opened. + if (autoInherited) + { + using SafeFileHandle handle = Open( + path, WinAPI.READ_CONTROL | WinAPI.WRITE_DAC | WinAPI.FILE_READ_ATTRIBUTES, AnyShare); + + if (!IsPlainDirectory(GetInformation(handle).dwFileAttributes)) + { + throw new IOException($"{path} changed while it was being created"); + } + + if (!WinAPI.SetKernelObjectSecurity(handle, WinAPI.DACL_SECURITY_INFORMATION, descriptor.Pointer)) + { + throw new Win32Exception(Marshal.GetLastWin32Error()); + } + } + + return true; + } + + int error = Marshal.GetLastWin32Error(); + + if (error == WinAPI.ERROR_ALREADY_EXISTS) + { + return false; + } + + throw new Win32Exception(error); + } + + /// + /// Sets the owner and the DACL of the file + /// \[0]\…\[n-1] to those of + /// , without following any link. + /// + /// + /// A file with more than one hard link is refused, because its security is shared with every other name of the + /// file, which may be outside the directories checked. + /// + /// + /// An entry above the file is a reparse point or not a directory, or the file is a reparse point, a directory or a + /// file with more than one hard link. + /// + /// The file or a directory above it doesn't exist, or can't be opened. + internal static void SetFileSecurity(string root, string[] names, string sddl) + { + List parents = new(); + + try + { + string target = Path.Combine(HoldParents(root, names, parents), names[names.Length - 1]); + using SafeFileHandle handle = OpenToSetSecurity(target); + WinAPI.BY_HANDLE_FILE_INFORMATION information = GetInformation(handle); + + const uint notAFile = WinAPI.FILE_ATTRIBUTE_REPARSE_POINT | WinAPI.FILE_ATTRIBUTE_DIRECTORY; + + if ((information.dwFileAttributes & notAFile) != 0) + { + throw new IOException($"{target} is a reparse point or not a file"); + } + + if (information.nNumberOfLinks > 1) + { + throw new IOException($"{target} has {information.nNumberOfLinks} hard links"); + } + + Apply(handle, sddl); + } + finally + { + parents.ForEach(parent => parent.Dispose()); + } + } + + /// + /// Sets the owner and the DACL of the directory holds, opened from + /// by , to those of , then resets the + /// security of the files and directories below it, without following any link. + /// + /// + /// + /// A file gets an unprotected DACL made of its explicit entries and the entries it inherits from the directory that + /// holds it: what SetNamedSecurityInfo computes when it removes the file's DACL protection. + /// An unprotected directory gets its explicit entries and the entries it inherits, as when Windows passes a new + /// DACL on, and a protected directory keeps its DACL, since an administrator protected it on purpose. + /// An entry gets the entries its directory now passes on, and loses the ones the directory passed on before: an + /// entry whose DACL isn't marked auto-inherited holds what it inherited as explicit entries, which are told apart + /// by comparing them with what its directory passed on before the change, as Windows does. + /// The inherited entries are computed from the security read through the directory handle held, rather than by + /// SetSecurityInfo, which reads the parent's security through a path it resolves again, and written through + /// the entry's handle. + /// A directory gets its DACL before the entries below it, which inherit from its new DACL. + /// + /// + /// Every allow entry for LOCAL SERVICE is removed below the directory, explicit or inherited, in a protected + /// directory too, so that none is left when has none: only SYSTEM, administrators and the + /// service account may change what the Gateway stores there, + /// and earlier installers gave LOCAL SERVICE write access, which a protected directory or a DACL not marked + /// auto-inherited holds as explicit entries. + /// The installer doesn't accept LOCAL SERVICE as the service account, so this never removes the service account's + /// access. + /// + /// + /// Each directory is held while the entries below it are opened, so no path below it can be redirected. + /// A reparse point, such as a junction or a symbolic link, is neither followed nor changed, and a file with more + /// than one hard link is skipped, because its security is shared with every other name of the file, which may be + /// outside the tree. + /// A directory replaced while the walk reaches it fails the walk. + /// + /// + /// + /// The names of the entries directly below the directory whose security is left as it is, because another step + /// sets it. + /// + /// The directory is a reparse point or not a directory. + internal static void SetSecurityAndInheritBelow( + SafeFileHandle directory, string path, string sddl, string[] kept, ILogger logger) + { + if (!IsPlainDirectory(GetInformation(directory).dwFileAttributes)) + { + throw new IOException($"{path} is a reparse point or not a directory"); + } + + byte[] previous = ReadDescriptor(directory, OwnerGroupAndDacl); + Apply(directory, sddl); + + using WindowsIdentity identity = WindowsIdentity.GetCurrent(); + InheritBelow(directory, path, previous, kept, identity.Token, logger); + } + + /// + /// Opens the entry names, a link itself rather than its target, to read its security and to + /// set it. + /// + private static SafeFileHandle OpenToSetSecurity(string path) => + Open( + path, + WinAPI.READ_CONTROL | WinAPI.WRITE_DAC | WinAPI.WRITE_OWNER | WinAPI.FILE_READ_ATTRIBUTES, + AnyShare); + + /// + /// Opens the entry names, a link itself rather than its target, to read its security and to + /// set it with , holding it as + /// does. + /// + internal static SafeFileHandle HoldToSetSecurity(string path) => + Hold(path, WinAPI.READ_CONTROL | WinAPI.WRITE_DAC | WinAPI.WRITE_OWNER); + + /// + /// Opens the entry names, a link itself rather than its target, to read its security. + /// + /// Whether to hold the entry as does. + internal static SafeFileHandle OpenToRead(string path, bool hold) => + hold + ? Hold(path, WinAPI.READ_CONTROL) + : Open(path, WinAPI.READ_CONTROL | WinAPI.FILE_READ_ATTRIBUTES, AnyShare); + + /// + /// Opens the directory names, a link itself rather than its target, with + /// , so that it can't be renamed or replaced while the handle is open. + /// + /// + /// A share mode binds only a handle with an access right that takes part in sharing checks, which + /// READ_CONTROL and FILE_READ_ATTRIBUTES don't, so the handle also has FILE_TRAVERSE, the right + /// that resolving a path below the directory needs. + /// + internal static SafeFileHandle Hold(string path, uint access = 0) => + Open(path, access | WinAPI.FILE_TRAVERSE | WinAPI.FILE_READ_ATTRIBUTES, HeldShare); + + /// + /// The owner and the DACL of the entry holds, whether that entry is a reparse point, and + /// whether it is a directory. + /// + internal static (RawSecurityDescriptor Security, bool IsReparsePoint, bool IsDirectory) ReadSecurity( + SafeFileHandle handle) + { + uint attributes = GetInformation(handle).dwFileAttributes; + bool isReparsePoint = (attributes & WinAPI.FILE_ATTRIBUTE_REPARSE_POINT) != 0; + bool isDirectory = (attributes & WinAPI.FILE_ATTRIBUTE_DIRECTORY) != 0; + + return (new RawSecurityDescriptor(ReadDescriptor(handle, OwnerAndDacl), 0), isReparsePoint, isDirectory); + } + + /// + /// Whether is SYSTEM, Administrators or TrustedInstaller. + /// + internal static bool IsPrivileged(SecurityIdentifier sid) => + sid.IsWellKnown(WellKnownSidType.LocalSystemSid) || sid == Administrators || sid == TrustedInstaller; + + /// + /// Whether may own the Gateway's data: SYSTEM, Administrators, TrustedInstaller, or a + /// member of Administrators. + /// + /// + /// Membership is decided as the Devolutions Agent and its installer decide it (owner_is_administrator_member + /// in crates/agent-identity/src/windows.rs), so that both installers accept the same owners of the + /// Devolutions folder they share: by the groups Authz resolves for the owner or, when Authz fails, by the + /// local member list of Administrators. + /// + /// Authz failed and the local member list of Administrators can't be read. + internal static bool IsTrustedOwner(SecurityIdentifier owner, ILogger logger) + { + if (IsPrivileged(owner)) + { + return true; + } + + // UNTESTED: a member of Administrators, such as an administrator's user account, is trusted through the groups + // Authz resolves or, when Authz fails, as it may for a domain account, through the local member list. + // No test runs with such an account, and this keeps both installers accepting the same owners. + // When changing this, keep it in line with owner_is_administrator_member in crates/agent-identity/src/windows.rs. + List groups; + + try + { + groups = AuthzGroups(owner); + } + catch (Exception e) + { + logger.Log($"failed to resolve the groups of {owner}, checking the local members of Administrators: {e}"); + return LocalAdministrators().Contains(owner); + } + + return groups.Contains(Administrators); + } + + internal static bool IsNotFound(Win32Exception e) => + e.NativeErrorCode is WinAPI.ERROR_FILE_NOT_FOUND or WinAPI.ERROR_PATH_NOT_FOUND; + + /// + /// Holds each directory \[0] to + /// \[0]\…\[n-2] in + /// , which the caller disposes. + /// + /// The path of the last directory held, or when there is one name. + /// An entry on the path is a reparse point or not a directory. + private static string HoldParents(string root, string[] names, List held) + { + string path = root; + + foreach (string name in names.Take(names.Length - 1)) + { + path = Path.Combine(path, name); + SafeFileHandle parent = Hold(path); + held.Add(parent); + + if (!IsPlainDirectory(GetInformation(parent).dwFileAttributes)) + { + throw new IOException($"{path} is a reparse point or not a directory"); + } + } + + return path; + } + + /// + /// Resets the security of each entry below the directory holds, opened from + /// by with READ_CONTROL, but those named in + /// , as describes, given the security descriptor + /// the directory had before its DACL changed. + /// + private static void InheritBelow( + SafeFileHandle directory, string path, byte[] previous, string[] kept, IntPtr token, ILogger logger) + { + byte[] parent = ReadDescriptor(directory, OwnerGroupAndDacl); + + // UNTESTED: each directory handle stays open until the entries below the directory are done, and a directory is + // walked only when the handle held is the entry first opened, so that no path below it can be redirected while + // the walk runs. + // Tests check that a held directory can't be renamed and that a link isn't followed, not when the walk closes a + // handle or a directory swapped between the two opens. + // When changing how the walk opens or closes a directory, keep each handle open that long and keep the check. + foreach (string entry in Directory.GetFileSystemEntries(path)) + { + // The entry is opened with a right that takes no part in sharing checks, so that another process using a + // file doesn't fail the walk, then opened again with the rights its kind needs. + using SafeFileHandle handle = Open(entry, WinAPI.FILE_READ_ATTRIBUTES, AnyShare); + WinAPI.BY_HANDLE_FILE_INFORMATION information = GetInformation(handle); + + if (kept.Contains(Path.GetFileName(entry), StringComparer.OrdinalIgnoreCase)) + { + logger.Log($"kept the security of {entry}: another step sets it"); + } + else if ((information.dwFileAttributes & WinAPI.FILE_ATTRIBUTE_REPARSE_POINT) != 0) + { + logger.Log($"kept the security of {entry}: it is a reparse point"); + } + else if ((information.dwFileAttributes & WinAPI.FILE_ATTRIBUTE_DIRECTORY) != 0) + { + // ReOpenFile refuses a directory, so the directory is held through its path, which the directories held + // above pin, and compared with the entry first opened. + using SafeFileHandle child = Hold(entry, WinAPI.READ_CONTROL | WinAPI.WRITE_DAC); + WinAPI.BY_HANDLE_FILE_INFORMATION held = GetInformation(child); + + if (!IsSameEntry(information, held) || !IsPlainDirectory(held.dwFileAttributes)) + { + throw new IOException($"{entry} changed while its security was being reset"); + } + + byte[] before = ReadDescriptor(child, OwnerGroupAndDacl); + + if ((new RawSecurityDescriptor(before, 0).ControlFlags & ControlFlags.DiscretionaryAclProtected) != 0) + { + RemoveLocalService(child); + } + else + { + Inherit(child, parent, previous, isDirectory: true, token); + } + + InheritBelow(child, entry, before, [], token, logger); + } + else if (information.nNumberOfLinks > 1) + { + logger.Log($"kept the security of {entry}: it has {information.nNumberOfLinks} hard links"); + } + else + { + using SafeFileHandle file = Reopen(handle, WinAPI.READ_CONTROL | WinAPI.WRITE_DAC, AnyShare); + Inherit(file, parent, previous, isDirectory: false, token); + } + } + } + + /// + /// Replaces the DACL of the file or directory holds by an unprotected one made of its + /// explicit entries and the entries it inherits from the security descriptor of its + /// directory, which had the security descriptor when the entry got its DACL. + /// + private static void Inherit(SafeFileHandle entry, byte[] parent, byte[] previous, bool isDirectory, IntPtr token) + { + WinAPI.GENERIC_MAPPING mapping = WinAPI.FileGenericMapping; + byte[] descriptor = ReadDescriptor(entry, OwnerGroupAndDacl); + RawSecurityDescriptor current = new(descriptor, 0); + + // A DACL not marked auto-inherited holds the entries it inherited as explicit ones, as an entry created in a + // folder whose DACL isn't marked auto-inherited does, so these are told apart by comparing them with what the + // parent passed on before its DACL changed, as Windows does when it passes a new DACL on. + if ((current.ControlFlags & ControlFlags.DiscretionaryAclAutoInherited) == 0) + { + if (!WinAPI.ConvertToAutoInheritPrivateObjectSecurity( + previous, descriptor, out IntPtr converted, IntPtr.Zero, isDirectory, ref mapping)) + { + throw new Win32Exception(Marshal.GetLastWin32Error()); + } + + try + { + current = ToRaw(converted); + } + finally + { + WinAPI.DestroyPrivateObjectSecurity(ref converted); + } + } + + RawAcl explicitEntries = null; + + if (current.DiscretionaryAcl is not null) + { + explicitEntries = new RawAcl(current.DiscretionaryAcl.Revision, current.DiscretionaryAcl.Count); + + foreach (GenericAce ace in current.DiscretionaryAcl) + { + if ((ace.AceFlags & AceFlags.Inherited) == 0 && !AllowsLocalService(ace)) + { + explicitEntries.InsertAce(explicitEntries.Count, ace); + } + } + } + + // An unprotected, auto-inherited creator DACL with only the explicit entries, so that the new DACL keeps them, + // and adds the entries the parent passes on to an entry of its kind, as Windows does when a DACL's protection + // is removed. + RawSecurityDescriptor unprotected = new( + (current.ControlFlags & ~ControlFlags.DiscretionaryAclProtected) | ControlFlags.DiscretionaryAclAutoInherited, + current.Owner, + current.Group, + null, + explicitEntries); + byte[] creator = new byte[unprotected.BinaryLength]; + unprotected.GetBinaryForm(creator, 0); + + if (!WinAPI.CreatePrivateObjectSecurityEx( + parent, + creator, + out IntPtr computed, + IntPtr.Zero, + isDirectory, + WinAPI.SEF_DACL_AUTO_INHERIT | WinAPI.SEF_AVOID_OWNER_CHECK | WinAPI.SEF_AVOID_PRIVILEGE_CHECK, + token, + ref mapping)) + { + throw new Win32Exception(Marshal.GetLastWin32Error()); + } + + try + { + // Without this request, the file system stores the DACL without marking it auto-inherited. + if (!WinAPI.SetSecurityDescriptorControl( + computed, WinAPI.SE_DACL_AUTO_INHERIT_REQ, WinAPI.SE_DACL_AUTO_INHERIT_REQ)) + { + throw new Win32Exception(Marshal.GetLastWin32Error()); + } + + if (!WinAPI.SetKernelObjectSecurity(entry, WinAPI.DACL_SECURITY_INFORMATION, computed)) + { + throw new Win32Exception(Marshal.GetLastWin32Error()); + } + } + finally + { + WinAPI.DestroyPrivateObjectSecurity(ref computed); + } + } + + /// + /// Removes each allow entry for LOCAL SERVICE from the DACL of the entry holds, keeping + /// its other entries and its control flags, such as its protection. + /// + private static void RemoveLocalService(SafeFileHandle handle) + { + RawSecurityDescriptor current = new(ReadDescriptor(handle, WinAPI.DACL_SECURITY_INFORMATION), 0); + RawAcl dacl = current.DiscretionaryAcl; + + if (dacl is null || !dacl.Cast().Any(AllowsLocalService)) + { + return; + } + + RawAcl remaining = new(dacl.Revision, dacl.Count); + + foreach (GenericAce ace in dacl) + { + if (!AllowsLocalService(ace)) + { + remaining.InsertAce(remaining.Count, ace); + } + } + + RawSecurityDescriptor updated = new(current.ControlFlags, null, null, null, remaining); + byte[] binary = new byte[updated.BinaryLength]; + updated.GetBinaryForm(binary, 0); + GCHandle pinned = GCHandle.Alloc(binary, GCHandleType.Pinned); + + try + { + IntPtr pointer = pinned.AddrOfPinnedObject(); + + // Without this request, the file system stores the DACL without marking it auto-inherited. + if ((current.ControlFlags & ControlFlags.DiscretionaryAclAutoInherited) != 0 + && !WinAPI.SetSecurityDescriptorControl( + pointer, WinAPI.SE_DACL_AUTO_INHERIT_REQ, WinAPI.SE_DACL_AUTO_INHERIT_REQ)) + { + throw new Win32Exception(Marshal.GetLastWin32Error()); + } + + if (!WinAPI.SetKernelObjectSecurity(handle, WinAPI.DACL_SECURITY_INFORMATION, pointer)) + { + throw new Win32Exception(Marshal.GetLastWin32Error()); + } + } + finally + { + pinned.Free(); + } + } + + /// + /// Whether allows LOCAL SERVICE any access. + /// + private static bool AllowsLocalService(GenericAce ace) => + ace is QualifiedAce qualified + && qualified.AceQualifier == AceQualifier.AccessAllowed + && qualified.SecurityIdentifier == LocalService; + + /// + /// The self-relative security descriptor at , copied. + /// + private static RawSecurityDescriptor ToRaw(IntPtr descriptor) + { + byte[] binary = new byte[WinAPI.GetSecurityDescriptorLength(descriptor)]; + Marshal.Copy(descriptor, binary, 0, binary.Length); + return new RawSecurityDescriptor(binary, 0); + } + + /// + /// Sets the owner and the DACL of the entry holds to those of . + /// + private static void Apply(SafeFileHandle handle, string sddl) + { + using SecurityDescriptor descriptor = new(sddl); + RequestAutoInherit(descriptor, sddl); + + if (!WinAPI.SetKernelObjectSecurity(handle, OwnerAndDacl, descriptor.Pointer)) + { + throw new Win32Exception(Marshal.GetLastWin32Error()); + } + } + + /// + /// Asks the file system to store the DACL of marked auto-inherited when + /// , its source, marks it so. + /// + /// + /// Without this request, setting the DACL stores it without the mark, and the entries created below then hold what + /// they inherit as explicit entries. + /// + /// Whether the mark was requested. + private static bool RequestAutoInherit(SecurityDescriptor descriptor, string sddl) + { + if ((new RawSecurityDescriptor(sddl).ControlFlags & ControlFlags.DiscretionaryAclAutoInherited) == 0) + { + return false; + } + + if (!WinAPI.SetSecurityDescriptorControl( + descriptor.Pointer, WinAPI.SE_DACL_AUTO_INHERIT_REQ, WinAPI.SE_DACL_AUTO_INHERIT_REQ)) + { + throw new Win32Exception(Marshal.GetLastWin32Error()); + } + + return true; + } + + /// + /// The parts names of the security descriptor of the entry + /// holds, in self-relative form. + /// + private static byte[] ReadDescriptor(SafeFileHandle handle, uint information) + { + // The first call only reports the size the second needs. + WinAPI.GetKernelObjectSecurity(handle, information, null, 0, out uint size); + int error = Marshal.GetLastWin32Error(); + + if (size == 0) + { + throw new Win32Exception(error); + } + + byte[] descriptor = new byte[size]; + + if (!WinAPI.GetKernelObjectSecurity(handle, information, descriptor, size, out _)) + { + throw new Win32Exception(Marshal.GetLastWin32Error()); + } + + return descriptor; + } + + /// + /// Opens the entry names, a link itself rather than its target. + /// + private static SafeFileHandle Open(string path, uint access, uint share) + { + SafeFileHandle handle = WinAPI.CreateFile( + path, + access, + share, + IntPtr.Zero, + WinAPI.OPEN_EXISTING, + WinAPI.FILE_FLAG_BACKUP_SEMANTICS | WinAPI.FILE_FLAG_OPEN_REPARSE_POINT, + IntPtr.Zero); + + if (handle.IsInvalid) + { + int error = Marshal.GetLastWin32Error(); + handle.Dispose(); + throw new Win32Exception(error); + } + + return handle; + } + + /// + /// Opens the entry holds again, a link itself rather than its target, with + /// and . + /// + private static SafeFileHandle Reopen(SafeFileHandle handle, uint access, uint share) + { + SafeFileHandle reopened = WinAPI.ReOpenFile( + handle, access, share, WinAPI.FILE_FLAG_BACKUP_SEMANTICS | WinAPI.FILE_FLAG_OPEN_REPARSE_POINT); + + if (reopened.IsInvalid) + { + int error = Marshal.GetLastWin32Error(); + reopened.Dispose(); + throw new Win32Exception(error); + } + + return reopened; + } + + private static WinAPI.BY_HANDLE_FILE_INFORMATION GetInformation(SafeFileHandle handle) + { + if (!WinAPI.GetFileInformationByHandle(handle, out WinAPI.BY_HANDLE_FILE_INFORMATION information)) + { + throw new Win32Exception(Marshal.GetLastWin32Error()); + } + + return information; + } + + private static bool IsSameEntry(WinAPI.BY_HANDLE_FILE_INFORMATION a, WinAPI.BY_HANDLE_FILE_INFORMATION b) => + a.dwVolumeSerialNumber == b.dwVolumeSerialNumber + && a.nFileIndexHigh == b.nFileIndexHigh + && a.nFileIndexLow == b.nFileIndexLow; + + private static bool IsPlainDirectory(uint attributes) => + (attributes & WinAPI.FILE_ATTRIBUTE_DIRECTORY) != 0 && (attributes & WinAPI.FILE_ATTRIBUTE_REPARSE_POINT) == 0; + + /// + /// The groups Authz resolves for , nested and domain groups included. + /// + private static List AuthzGroups(SecurityIdentifier sid) + { + if (!WinAPI.AuthzInitializeResourceManager( + WinAPI.AUTHZ_RM_FLAG_NO_AUDIT, IntPtr.Zero, IntPtr.Zero, IntPtr.Zero, null, out IntPtr manager)) + { + throw new Win32Exception(Marshal.GetLastWin32Error()); + } + + try + { + byte[] binary = new byte[sid.BinaryLength]; + sid.GetBinaryForm(binary, 0); + + if (!WinAPI.AuthzInitializeContextFromSid( + 0, binary, manager, IntPtr.Zero, default, IntPtr.Zero, out IntPtr context)) + { + throw new Win32Exception(Marshal.GetLastWin32Error()); + } + + try + { + // The first call only reports the size the second needs. + WinAPI.AuthzGetInformationFromContext( + context, WinAPI.AuthzContextInfoGroupsSids, 0, out uint size, IntPtr.Zero); + int error = Marshal.GetLastWin32Error(); + int offset = Marshal.OffsetOf(nameof(WinAPI.TOKEN_GROUPS.Groups)).ToInt32(); + + if (size < offset) + { + throw new Win32Exception(error); + } + + IntPtr buffer = Marshal.AllocHGlobal((int)size); + + try + { + if (!WinAPI.AuthzGetInformationFromContext( + context, WinAPI.AuthzContextInfoGroupsSids, size, out size, buffer)) + { + throw new Win32Exception(Marshal.GetLastWin32Error()); + } + + int count = Marshal.ReadInt32(buffer); + int stride = Marshal.SizeOf(); + + if (count < 0 || offset + (long)count * stride > size) + { + throw new InvalidDataException("Authz returned a group list larger than its buffer"); + } + + return Enumerable.Range(0, count) + .Select(i => new SecurityIdentifier(Marshal.ReadIntPtr(buffer, offset + i * stride))) + .ToList(); + } + finally + { + Marshal.FreeHGlobal(buffer); + } + } + finally + { + WinAPI.AuthzFreeContext(context); + } + } + finally + { + WinAPI.AuthzFreeResourceManager(manager); + } + } + + /// + /// The direct members of the local Administrators group. + /// + private static List LocalAdministrators() + { + // NetLocalGroupGetMembers takes the group's name, which is localized, without its BUILTIN domain. + string name = Administrators.Translate(typeof(NTAccount)).Value; + name = name.Substring(name.LastIndexOf('\\') + 1); + + uint status = WinAPI.NetLocalGroupGetMembers( + null, name, 0, out IntPtr buffer, WinAPI.MAX_PREFERRED_LENGTH, out uint read, out _, IntPtr.Zero); + + if (status != WinAPI.NERR_Success) + { + throw new Win32Exception((int)status); + } + + try + { + // Level 0 returns LOCALGROUP_MEMBERS_INFO_0 entries, each a single SID pointer. + return Enumerable.Range(0, (int)read) + .Select(i => new SecurityIdentifier(Marshal.ReadIntPtr(buffer, i * IntPtr.Size))) + .ToList(); + } + finally + { + WinAPI.NetApiBufferFree(buffer); + } + } + + /// + /// A security descriptor converted from SDDL, freed on disposal. + /// + private sealed class SecurityDescriptor : IDisposable + { + internal IntPtr Pointer { get; } + + internal SecurityDescriptor(string sddl) + { + const uint sdRevision = 1; + + if (!WinAPI.ConvertStringSecurityDescriptorToSecurityDescriptorW(sddl, sdRevision, out IntPtr pointer, out _)) + { + throw new Win32Exception(Marshal.GetLastWin32Error()); + } + + this.Pointer = pointer; + } + + public void Dispose() => WinAPI.LocalFree(this.Pointer); + } +} diff --git a/package/WindowsManaged/Actions/WinAPI.cs b/package/WindowsManaged/Actions/WinAPI.cs index bdbae20ed9..0f12599858 100644 --- a/package/WindowsManaged/Actions/WinAPI.cs +++ b/package/WindowsManaged/Actions/WinAPI.cs @@ -8,30 +8,70 @@ namespace DevolutionsGateway.Actions; internal static class WinAPI { + internal const uint AUTHZ_RM_FLAG_NO_AUDIT = 0x1; + + internal const int AuthzContextInfoGroupsSids = 2; + internal static uint CREATE_ALWAYS = 2; internal static uint CREATE_NO_WINDOW = 0x08000000; internal const uint DACL_SECURITY_INFORMATION = 0x00000004; + internal const uint DELETE = 0x00010000; + internal const int EM_SETCUEBANNER = 0x1501; - + + internal const int ERROR_ALREADY_EXISTS = 183; + + internal const int ERROR_FILE_NOT_FOUND = 2; + + internal const int ERROR_PATH_NOT_FOUND = 3; + + internal const uint FILE_ATTRIBUTE_DIRECTORY = 0x00000010; + internal static uint FILE_ATTRIBUTE_NORMAL = 0x00000080; + internal const uint FILE_ATTRIBUTE_REPARSE_POINT = 0x00000400; + + internal const uint FILE_DELETE_CHILD = 0x00000040; + + internal const uint FILE_FLAG_BACKUP_SEMANTICS = 0x02000000; + + internal const uint FILE_FLAG_OPEN_REPARSE_POINT = 0x00200000; + + internal const uint FILE_READ_ATTRIBUTES = 0x00000080; + + internal const uint FILE_SHARE_DELETE = 0x00000004; + internal static uint FILE_SHARE_READ = 0x00000001; internal static uint FILE_SHARE_WRITE = 0x00000002; + internal const uint FILE_TRAVERSE = 0x00000020; + + internal const uint GROUP_SECURITY_INFORMATION = 0x00000002; + internal const uint LOGON32_LOGON_NETWORK = 3; internal const uint LOGON32_PROVIDER_DEFAULT = 0; + internal const uint MAX_PREFERRED_LENGTH = 0xFFFFFFFF; + internal static uint MOVEFILE_DELAY_UNTIL_REBOOT = 0x04; + internal const uint NERR_Success = 0; + + internal const uint OPEN_EXISTING = 3; + + internal const uint OWNER_SECURITY_INFORMATION = 0x00000001; + internal const uint POLICY_CREATE_ACCOUNT = 0x00000010; internal const uint POLICY_LOOKUP_NAMES = 0x00000800; + internal const uint READ_CONTROL = 0x00020000; + internal const uint SC_MANAGER_ALL_ACCESS = 0xF003F; internal const uint SC_MANAGER_CONNECT = 0x0001; @@ -56,15 +96,27 @@ internal static class WinAPI internal const uint SERVICE_STOPPED = 0x00000001; + internal const ushort SE_DACL_AUTO_INHERIT_REQ = 0x0100; + + internal const uint SEF_AVOID_OWNER_CHECK = 0x10; + + internal const uint SEF_AVOID_PRIVILEGE_CHECK = 0x08; + + internal const uint SEF_DACL_AUTO_INHERIT = 0x01; + internal static uint STARTF_USESTDHANDLES = 0x00000100; internal static int STD_INPUT_HANDLE = -10; internal static uint WAIT_TIMEOUT = 0x00000102; + internal const uint WRITE_DAC = 0x00040000; + + internal const uint WRITE_OWNER = 0x00080000; + /* Generic access rights */ - internal const uint GENERAL_ALL = 0x10000000; + internal const uint GENERIC_ALL = 0x10000000; internal const uint GENERIC_EXECUTE = 0x20000000; @@ -72,6 +124,62 @@ internal static class WinAPI internal const uint GENERIC_READ = 0x80000000; + /// + /// The rights each generic right maps to for a file: FILE_GENERIC_READ, FILE_GENERIC_WRITE, + /// FILE_GENERIC_EXECUTE and FILE_ALL_ACCESS. + /// + internal static GENERIC_MAPPING FileGenericMapping => new() + { + GenericRead = 0x00120089, + GenericWrite = 0x00120116, + GenericExecute = 0x001200A0, + GenericAll = 0x001F01FF, + }; + + [StructLayout(LayoutKind.Sequential)] + internal struct BY_HANDLE_FILE_INFORMATION + { + internal uint dwFileAttributes; + + internal System.Runtime.InteropServices.ComTypes.FILETIME ftCreationTime; + + internal System.Runtime.InteropServices.ComTypes.FILETIME ftLastAccessTime; + + internal System.Runtime.InteropServices.ComTypes.FILETIME ftLastWriteTime; + + internal uint dwVolumeSerialNumber; + + internal uint nFileSizeHigh; + + internal uint nFileSizeLow; + + internal uint nNumberOfLinks; + + internal uint nFileIndexHigh; + + internal uint nFileIndexLow; + } + + [StructLayout(LayoutKind.Sequential)] + internal struct GENERIC_MAPPING + { + internal uint GenericRead; + + internal uint GenericWrite; + + internal uint GenericExecute; + + internal uint GenericAll; + } + + [StructLayout(LayoutKind.Sequential)] + internal struct LUID + { + internal uint LowPart; + + internal int HighPart; + } + [StructLayout(LayoutKind.Sequential)] internal struct QUERY_SERVICE_CONFIG { @@ -202,6 +310,14 @@ internal struct SERVICE_STATUS_PROCESS internal uint dwServiceFlags; } + [StructLayout(LayoutKind.Sequential)] + internal struct SID_AND_ATTRIBUTES + { + internal IntPtr Sid; + + internal uint Attributes; + } + [StructLayout(LayoutKind.Sequential)] internal struct STARTUPINFO { @@ -242,6 +358,53 @@ internal struct STARTUPINFO internal IntPtr hStdError; } + [StructLayout(LayoutKind.Sequential)] + internal struct TOKEN_GROUPS + { + internal uint GroupCount; + + // The first of GroupCount entries. + internal SID_AND_ATTRIBUTES Groups; + } + + [DllImport("authz")] + [return: MarshalAs(UnmanagedType.Bool)] + internal static extern bool AuthzFreeContext(IntPtr hAuthzClientContext); + + [DllImport("authz")] + [return: MarshalAs(UnmanagedType.Bool)] + internal static extern bool AuthzFreeResourceManager(IntPtr hAuthzResourceManager); + + [DllImport("authz", SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + internal static extern bool AuthzGetInformationFromContext( + IntPtr hAuthzClientContext, + int InfoClass, + uint BufferSize, + out uint pSizeRequired, + IntPtr Buffer); + + [DllImport("authz", SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + internal static extern bool AuthzInitializeContextFromSid( + uint Flags, + byte[] UserSid, + IntPtr hAuthzResourceManager, + IntPtr pExpirationTime, + LUID Identifier, + IntPtr DynamicGroupArgs, + out IntPtr phAuthzClientContext); + + [DllImport("authz", SetLastError = true, CharSet = CharSet.Unicode)] + [return: MarshalAs(UnmanagedType.Bool)] + internal static extern bool AuthzInitializeResourceManager( + uint Flags, + IntPtr pfnDynamicAccessCheck, + IntPtr pfnComputeDynamicGroups, + IntPtr pfnFreeDynamicGroups, + string szResourceManagerName, + out IntPtr phAuthzResourceManager); + [DllImport("advapi32", EntryPoint = "ChangeServiceConfigW", SetLastError = true)] internal static extern bool ChangeServiceConfig( IntPtr hService, @@ -270,6 +433,10 @@ internal static extern bool ChangeServiceConfig( [return: MarshalAs(UnmanagedType.Bool)] public static extern bool ControlService(IntPtr hService, uint dwControl, IntPtr lpServiceStatus); + [DllImport("kernel32", EntryPoint = "CreateDirectoryW", CharSet = CharSet.Unicode, SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + internal static extern bool CreateDirectory(string lpPathName, ref SECURITY_ATTRIBUTES lpSecurityAttributes); + [DllImport("kernel32", EntryPoint = "CreateFileW", CharSet = CharSet.Unicode, SetLastError = true)] internal static extern SafeFileHandle CreateFile( [MarshalAs(UnmanagedType.LPWStr)] string lpFileName, @@ -294,6 +461,28 @@ internal static extern bool CreateProcess( IntPtr lpStartupInfo, IntPtr lpProcessInformation); + [DllImport("advapi32", SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + internal static extern bool ConvertToAutoInheritPrivateObjectSecurity( + byte[] ParentDescriptor, + byte[] CurrentSecurityDescriptor, + out IntPtr NewSecurityDescriptor, + IntPtr ObjectType, + [MarshalAs(UnmanagedType.U1)] bool IsDirectoryObject, + ref GENERIC_MAPPING GenericMapping); + + [DllImport("advapi32", SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + internal static extern bool CreatePrivateObjectSecurityEx( + byte[] ParentDescriptor, + byte[] CreatorDescriptor, + out IntPtr NewDescriptor, + IntPtr ObjectType, + [MarshalAs(UnmanagedType.Bool)] bool IsContainerObject, + uint AutoInheritFlags, + IntPtr Token, + ref GENERIC_MAPPING GenericMapping); + [DllImport("kernel32", EntryPoint = "DeleteFileW", CharSet = CharSet.Unicode, SetLastError = true)] [return: MarshalAs(UnmanagedType.Bool)] internal static extern bool DeleteFile( @@ -301,10 +490,20 @@ internal static extern bool DeleteFile( string lpFileName ); + [DllImport("advapi32", SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + internal static extern bool DestroyPrivateObjectSecurity(ref IntPtr ObjectDescriptor); + [DllImport("kernel32", SetLastError = true)] [return: MarshalAs(UnmanagedType.Bool)] internal static extern bool GetExitCodeProcess(IntPtr hProcess, out uint lpExitCode); + [DllImport("kernel32", SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + internal static extern bool GetFileInformationByHandle( + SafeFileHandle hFile, + out BY_HANDLE_FILE_INFORMATION lpFileInformation); + [DllImport("Kernel32", EntryPoint = "GetFinalPathNameByHandleW", CharSet = CharSet.Auto, SetLastError = true)] internal static extern uint GetFinalPathNameByHandle( IntPtr hFile, @@ -313,6 +512,18 @@ internal static extern uint GetFinalPathNameByHandle( uint cchFilePath, uint dwFlags); + [DllImport("advapi32", SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + internal static extern bool GetKernelObjectSecurity( + SafeFileHandle Handle, + uint RequestedInformation, + byte[] pSecurityDescriptor, + uint nLength, + out uint lpnLengthNeeded); + + [DllImport("advapi32")] + internal static extern uint GetSecurityDescriptorLength(IntPtr pSecurityDescriptor); + [DllImport("kernel32", SetLastError = true)] internal static extern IntPtr GetStdHandle(int nStdHandle); @@ -384,6 +595,20 @@ internal static extern bool MoveFileEx( uint dwFlags ); + [DllImport("netapi32")] + internal static extern uint NetApiBufferFree(IntPtr Buffer); + + [DllImport("netapi32", CharSet = CharSet.Unicode)] + internal static extern uint NetLocalGroupGetMembers( + string servername, + string localgroupname, + uint level, + out IntPtr bufptr, + uint prefmaxlen, + out uint entriesread, + out uint totalentries, + IntPtr resumehandle); + [DllImport("advapi32", EntryPoint = "OpenSCManagerW", SetLastError = true)] internal static extern IntPtr OpenSCManager(IntPtr machineName, IntPtr databaseName, uint dwAccess); @@ -402,6 +627,13 @@ internal static extern bool QueryServiceConfig(IntPtr hService, IntPtr lpService public static extern bool QueryServiceStatusEx(IntPtr serviceHandle, uint infoLevel, IntPtr buffer, uint bufferSize, out uint bytesNeeded); + [DllImport("kernel32", SetLastError = true)] + internal static extern SafeFileHandle ReOpenFile( + SafeFileHandle hOriginalFile, + uint dwDesiredAccess, + uint dwShareMode, + uint dwFlagsAndAttributes); + [DllImport("user32", EntryPoint = "SendMessageW", CharSet = CharSet.Unicode)] internal static extern int SendMessage( IntPtr hWnd, @@ -409,8 +641,19 @@ internal static extern int SendMessage( int wParam, [MarshalAs(UnmanagedType.LPWStr)] string lParam); - [DllImport("advapi32", SetLastError = true, CharSet = CharSet.Unicode)] - internal static extern bool SetFileSecurityW(string lpFileName, uint SecurityInformation, IntPtr pSecurityDescriptor); + [DllImport("advapi32", SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + internal static extern bool SetSecurityDescriptorControl( + IntPtr pSecurityDescriptor, + ushort ControlBitsOfInterest, + ushort ControlBitsToSet); + + [DllImport("advapi32", SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + internal static extern bool SetKernelObjectSecurity( + SafeFileHandle Handle, + uint SecurityInformation, + IntPtr SecurityDescriptor); [DllImport("advapi32", EntryPoint = "StartServiceW", SetLastError = true)] internal static extern bool StartService(IntPtr hService, uint dwNumServiceArgs, IntPtr lpServiceArgVectors); diff --git a/package/WindowsManaged/INTENT.md b/package/WindowsManaged/INTENT.md new file mode 100644 index 0000000000..44e261dc78 --- /dev/null +++ b/package/WindowsManaged/INTENT.md @@ -0,0 +1,7 @@ +## Data folder + +- The installer gives `%ProgramData%\Devolutions\Gateway` its owner and its permissions when it creates the folder, and resets both on every install, upgrade and repair, so that only SYSTEM, administrators and the service account can change what the Gateway stores there, and only SYSTEM and administrators can rename or replace it. +- An install, upgrade, repair or feature change fails before it changes anything, with an error in the installer's language naming the folder and the account at fault, when `%ProgramData%\Devolutions` or `%ProgramData%\Devolutions\Gateway` is a link, can't be read, or has an owner other than SYSTEM, Administrators, TrustedInstaller or a member of Administrators, or when `%ProgramData%\Devolutions` lets another account rename or replace it or an entry of it. + The installer refuses rather than repairs, because a folder another account controlled may hold a configuration or keys that account left there, and the Gateway trusts what it finds in its data folder. + A full uninstall never refuses, so a machine can always be uninstalled. +- The installer never follows a link when it sets the security of the data folder or of an entry in it, so that a link can't make it change the security of anything else. diff --git a/package/WindowsManaged/Program.cs b/package/WindowsManaged/Program.cs index 06b98cd031..9a4949b46c 100644 --- a/package/WindowsManaged/Program.cs +++ b/package/WindowsManaged/Program.cs @@ -210,6 +210,18 @@ private static string ProjectLangId private static KeyValuePair deDE => Languages.First(x => x.Key == "de-DE"); + /// + /// The major upgrade, which removes the installed product after InstallInitialize, so before the actions that + /// check and reset the data folder. + /// + internal static MajorUpgrade MajorUpgrade => new() + { + AllowDowngrades = false, + AllowSameVersionUpgrades = true, + DowngradeErrorMessage = "!(loc.NewerInstalled)", + Schedule = UpgradeSchedule.afterInstallInitialize, + }; + static void Main() { ManagedProject project = new(Includes.PRODUCT_NAME) @@ -231,13 +243,7 @@ static void Main() BackgroundImage = "Resources/WixUIDialog.jpg", ValidateBackgroundImage = false, OutFileName = PackageName, - MajorUpgrade = new MajorUpgrade - { - AllowDowngrades = false, - AllowSameVersionUpgrades = true, - DowngradeErrorMessage = "!(loc.NewerInstalled)", - Schedule = UpgradeSchedule.afterInstallInitialize, - }, + MajorUpgrade = MajorUpgrade, Media = new List { new() diff --git a/package/WindowsManaged/Resources/DevolutionsGateway_de-de.wxl b/package/WindowsManaged/Resources/DevolutionsGateway_de-de.wxl index 6fe0a2a9fb..1921e5ecfc 100644 --- a/package/WindowsManaged/Resources/DevolutionsGateway_de-de.wxl +++ b/package/WindowsManaged/Resources/DevolutionsGateway_de-de.wxl @@ -9,6 +9,11 @@ Beim Zugriff auf den System-Zertifikatspeicher ist ein unerwarteter Fehler aufgetreten: {0} Das Authentifizierungstoken ist erforderlich + {0} kann nicht verwendet werden, da es eine Datei und kein Ordner ist. Löschen Sie die Datei und versuchen Sie es erneut. + Der Ordner {0} kann nicht verwendet werden, da er ein Link ist. Löschen Sie den Link und versuchen Sie es erneut. + Der Ordner {0} kann nicht verwendet werden, da er {1} gehört. Nur SYSTEM oder ein Administrator darf ihn besitzen. Ändern Sie den Besitzer und versuchen Sie es erneut. + Der Ordner {0} kann nicht verwendet werden, da {1} ihn oder seinen Inhalt ersetzen kann. Entfernen Sie diese Berechtigung und versuchen Sie es erneut. + Der Besitzer oder die Berechtigungen des Ordners {0} können nicht gelesen werden. Überprüfen Sie sie und versuchen Sie es erneut. Die Domäne ist erforderlich und muss ein gültiger Hostname sein Das Produkt erfordert Microsoft .NET Framework 4.7.1. Microsoft .NET Framework 4.8 wird dringend empfohlen. Möchten Sie es jetzt herunterladen? Microsoft .NET Framework 4.8 wird dringend empfohlen. Möchten Sie es jetzt herunterladen? diff --git a/package/WindowsManaged/Resources/DevolutionsGateway_en-us.wxl b/package/WindowsManaged/Resources/DevolutionsGateway_en-us.wxl index 0f94f6aa73..1cc976b98b 100644 --- a/package/WindowsManaged/Resources/DevolutionsGateway_en-us.wxl +++ b/package/WindowsManaged/Resources/DevolutionsGateway_en-us.wxl @@ -9,6 +9,11 @@ An unexpected error occurred accessing the system certificate store: {0} The authentication token is required + Can't use {0} because it is a file, not a folder. Delete the file, then try again. + Can't use the folder {0} because it is a link. Delete the link, then try again. + Can't use the folder {0} because it is owned by {1}. Only SYSTEM or an administrator may own it. Change its owner, then try again. + Can't use the folder {0} because {1} can replace it or what it contains. Remove that permission, then try again. + Can't read the owner or the permissions of the folder {0}. Check them, then try again. The domain is required and must be a valid hostname The product requires Microsoft .NET Framework 4.7.1. Microsoft .NET Framework 4.8 is strongly recommended. Would you like to download it now? Microsoft .NET Framework 4.8 is strongly recommended. Would you like to download it now? diff --git a/package/WindowsManaged/Resources/DevolutionsGateway_fr-fr.wxl b/package/WindowsManaged/Resources/DevolutionsGateway_fr-fr.wxl index 757192242b..1e9da0df43 100644 --- a/package/WindowsManaged/Resources/DevolutionsGateway_fr-fr.wxl +++ b/package/WindowsManaged/Resources/DevolutionsGateway_fr-fr.wxl @@ -61,6 +61,11 @@ Une erreur inattendue est survenue au moment d'accéder au magasin de certificats du système : {0} Le jeton d'authentification est requis + Impossible d'utiliser {0}, car c'est un fichier et non un dossier. Supprimez le fichier, puis réessayez. + Impossible d'utiliser le dossier {0}, car c'est un lien. Supprimez le lien, puis réessayez. + Impossible d'utiliser le dossier {0}, car il appartient à {1}. Seul SYSTEM ou un administrateur peut en être propriétaire. Changez son propriétaire, puis réessayez. + Impossible d'utiliser le dossier {0}, car {1} peut remplacer le dossier ou son contenu. Retirez cette autorisation, puis réessayez. + Impossible de lire le propriétaire ou les autorisations du dossier {0}. Vérifiez-les, puis réessayez. Le domaine est requis et doit être un nom d'hôte valide Le produit nécessite Microsoft .NET Framework 4.7.1. Microsoft .NET Framework 4.8 est fortement conseillé. Souhaiteriez-vous le télécharger maintenant? Microsoft .NET Framework 4.8 est fortement conseillé. Souhaiteriez-vous le télécharger maintenant? diff --git a/package/WindowsManaged/Resources/Includes.cs b/package/WindowsManaged/Resources/Includes.cs index 4ff836fbb5..9a5190737f 100644 --- a/package/WindowsManaged/Resources/Includes.cs +++ b/package/WindowsManaged/Resources/Includes.cs @@ -27,31 +27,36 @@ internal static class Includes internal static string INFO_URL = "https://server.devolutions.net"; /// - /// SDDL template for the desired %programdata%\devolutions\gateway ACL. `{0}` is the SID of the service account. + /// SDDL template for the desired %programdata%\devolutions\gateway owner and ACL. `{0}` is the SID of the service account. /// Easiest way to generate an SDDL is to configure the required access, and then query the path with PowerShell: `Get-Acl | Format-List` /// /// /// Local System (SY) Full Access (FA) - /// Local Service (LS) Read, Execute - /// Service account Read, Execute, Write, Delete Subfolders and Files + /// Service account Read, Execute, Write, Delete Subfolders and Files on the folder; also Delete on everything below it /// Administrators (BA) Full Access (FA) /// Users (BU) Read, Execute + /// Only SYSTEM, administrators and the service account can change what the Gateway stores there, so no other + /// account gets more than read access. + /// LOCAL SERVICE, which earlier installers gave write access, has no entry, and the installer doesn't accept it as + /// the service account. + /// The service account gets no Delete on the folder itself, so that only SYSTEM and administrators can rename or + /// replace it, while an inherit-only entry gives it Delete on the files and subfolders it manages below. /// - private const string PROGRAM_DATA_SDDL_TEMPLATE = "O:SYG:SYD:PAI(A;OICI;FA;;;SY)(A;OICI;0x1201bf;;;LS)(A;OICI;0x1301ff;;;{0})(A;OICI;FA;;;BA)(A;OICI;0x1200a9;;;BU)"; + private const string PROGRAM_DATA_SDDL_TEMPLATE = "O:SYG:SYD:PAI(A;OICI;FA;;;SY)(A;OICI;0x1201ff;;;{0})(A;OICIIO;0x1301ff;;;{0})(A;OICI;FA;;;BA)(A;OICI;0x1200a9;;;BU)"; /// - /// SDDL template for the desired %programdata%\devolutions\gateway\users.txt ACL. `{0}` is the SID of the service account. + /// SDDL template for the desired %programdata%\devolutions\gateway\users.txt owner and ACL. `{0}` is the SID of the service account. /// /// /// Owner : NT AUTHORITY\SYSTEM /// Group : NT AUTHORITY\SYSTEM /// Access : /// Local System (SY) Full Access (FA) - /// Local Service (LS) Read, Execute, Modify (Write) - /// Service account Read, Execute, Modify (Write) + /// Service account Read, Write, Execute /// Administrators (BA) Full Access (FA) + /// Only SYSTEM, administrators and the service account can read or change the users database. /// - private const string USERS_FILE_SDDL_TEMPLATE = "O:SYG:SYD:PAI(A;;FA;;;SY)(A;;0x1201bf;;;LS)(A;;0x1201bf;;;{0})(A;;FA;;;BA)"; + private const string USERS_FILE_SDDL_TEMPLATE = "O:SYG:SYD:PAI(A;;FA;;;SY)(A;;0x1201bf;;;{0})(A;;FA;;;BA)"; internal static string ProgramDataSddl(SecurityIdentifier serviceAccount) => string.Format(PROGRAM_DATA_SDDL_TEMPLATE, serviceAccount.Value); diff --git a/package/WindowsManaged/Resources/Strings.g.cs b/package/WindowsManaged/Resources/Strings.g.cs index 2e2811b3fc..2194e29614 100644 --- a/package/WindowsManaged/Resources/Strings.g.cs +++ b/package/WindowsManaged/Resources/Strings.g.cs @@ -121,6 +121,26 @@ public static string I18n(this MsiRuntime runtime, string res) /// public const string WindowsPowerShell51IsRequired = "WindowsPowerShell51IsRequired"; /// + /// Can't use the folder {0} because it is a link. Delete the link, then try again. + /// + public const string DataFolderIsLink = "DataFolderIsLink"; + /// + /// Can't use {0} because it is a file, not a folder. Delete the file, then try again. + /// + public const string DataFolderIsFile = "DataFolderIsFile"; + /// + /// Can't use the folder {0} because it is owned by {1}. Only SYSTEM or an administrator may own it. Change its owner, then try again. + /// + public const string DataFolderOwnedByUntrustedAccount = "DataFolderOwnedByUntrustedAccount"; + /// + /// Can't use the folder {0} because {1} can replace it or what it contains. Remove that permission, then try again. + /// + public const string DataFolderReplaceableByUntrustedAccount = "DataFolderReplaceableByUntrustedAccount"; + /// + /// Can't read the owner or the permissions of the folder {0}. Check them, then try again. + /// + public const string DataFolderSecurityUnreadable = "DataFolderSecurityUnreadable"; + /// /// Find your public key for {0} or {1} /// public const string FindYourPublicKeyForXorX = "FindYourPublicKeyForXorX"; diff --git a/package/WindowsManaged/Resources/Strings_de-DE.json b/package/WindowsManaged/Resources/Strings_de-DE.json index 12dfbb6c8e..9a6b0f5ae8 100644 --- a/package/WindowsManaged/Resources/Strings_de-DE.json +++ b/package/WindowsManaged/Resources/Strings_de-DE.json @@ -115,6 +115,26 @@ { "id": "WindowsPowerShell51IsRequired", "text": "Dieses Produkt erfordert Windows PowerShell 5.1" + }, + { + "id": "DataFolderIsLink", + "text": "Der Ordner {0} kann nicht verwendet werden, da er ein Link ist. Löschen Sie den Link und versuchen Sie es erneut." + }, + { + "id": "DataFolderIsFile", + "text": "{0} kann nicht verwendet werden, da es eine Datei und kein Ordner ist. Löschen Sie die Datei und versuchen Sie es erneut." + }, + { + "id": "DataFolderOwnedByUntrustedAccount", + "text": "Der Ordner {0} kann nicht verwendet werden, da er {1} gehört. Nur SYSTEM oder ein Administrator darf ihn besitzen. Ändern Sie den Besitzer und versuchen Sie es erneut." + }, + { + "id": "DataFolderReplaceableByUntrustedAccount", + "text": "Der Ordner {0} kann nicht verwendet werden, da {1} ihn oder seinen Inhalt ersetzen kann. Entfernen Sie diese Berechtigung und versuchen Sie es erneut." + }, + { + "id": "DataFolderSecurityUnreadable", + "text": "Der Besitzer oder die Berechtigungen des Ordners {0} können nicht gelesen werden. Überprüfen Sie sie und versuchen Sie es erneut." } ], "links": [ diff --git a/package/WindowsManaged/Resources/Strings_en-US.json b/package/WindowsManaged/Resources/Strings_en-US.json index 300008db16..e8cc8267d1 100644 --- a/package/WindowsManaged/Resources/Strings_en-US.json +++ b/package/WindowsManaged/Resources/Strings_en-US.json @@ -115,6 +115,26 @@ { "id": "WindowsPowerShell51IsRequired", "text": "This product requires Windows PowerShell 5.1" + }, + { + "id": "DataFolderIsLink", + "text": "Can't use the folder {0} because it is a link. Delete the link, then try again." + }, + { + "id": "DataFolderIsFile", + "text": "Can't use {0} because it is a file, not a folder. Delete the file, then try again." + }, + { + "id": "DataFolderOwnedByUntrustedAccount", + "text": "Can't use the folder {0} because it is owned by {1}. Only SYSTEM or an administrator may own it. Change its owner, then try again." + }, + { + "id": "DataFolderReplaceableByUntrustedAccount", + "text": "Can't use the folder {0} because {1} can replace it or what it contains. Remove that permission, then try again." + }, + { + "id": "DataFolderSecurityUnreadable", + "text": "Can't read the owner or the permissions of the folder {0}. Check them, then try again." } ], "links": [ diff --git a/package/WindowsManaged/Resources/Strings_fr-FR.json b/package/WindowsManaged/Resources/Strings_fr-FR.json index 41473ccb92..7c4ae625d5 100644 --- a/package/WindowsManaged/Resources/Strings_fr-FR.json +++ b/package/WindowsManaged/Resources/Strings_fr-FR.json @@ -318,6 +318,26 @@ { "id": "YouMustEnterAValidUrl", "text": "Vous devez entrer une URL valide" + }, + { + "id": "DataFolderIsLink", + "text": "Impossible d'utiliser le dossier {0}, car c'est un lien. Supprimez le lien, puis réessayez." + }, + { + "id": "DataFolderIsFile", + "text": "Impossible d'utiliser {0}, car c'est un fichier et non un dossier. Supprimez le fichier, puis réessayez." + }, + { + "id": "DataFolderOwnedByUntrustedAccount", + "text": "Impossible d'utiliser le dossier {0}, car il appartient à {1}. Seul SYSTEM ou un administrateur peut en être propriétaire. Changez son propriétaire, puis réessayez." + }, + { + "id": "DataFolderReplaceableByUntrustedAccount", + "text": "Impossible d'utiliser le dossier {0}, car {1} peut remplacer le dossier ou son contenu. Retirez cette autorisation, puis réessayez." + }, + { + "id": "DataFolderSecurityUnreadable", + "text": "Impossible de lire le propriétaire ou les autorisations du dossier {0}. Vérifiez-les, puis réessayez." } ], "propertyGroups": [