diff --git a/package/AgentWindowsManaged.Tests/AgentIdentityTests.cs b/package/AgentWindowsManaged.Tests/AgentIdentityTests.cs
index 97154eeee..a14b0a9f6 100644
--- a/package/AgentWindowsManaged.Tests/AgentIdentityTests.cs
+++ b/package/AgentWindowsManaged.Tests/AgentIdentityTests.cs
@@ -17,8 +17,11 @@
using Property = WixSharp.Property;
using Record = Microsoft.Deployment.WindowsInstaller.Record;
+using static DevolutionsAgent.Installer.Tests.InstallerSession;
+
namespace DevolutionsAgent.Installer.Tests;
+[Collection(InstallerSession.Collection)]
public sealed class AgentIdentityTests
{
// A well-formed token (crates/agent-identity/src/token.rs) whose bag names https://example.test/dvls.
@@ -410,12 +413,6 @@ string ErrorIn(string productLanguage)
Assert.Equal(english, ErrorIn("1031"));
}
- ///
- /// The installer's action that runs the custom action .
- ///
- private static ManagedAction ActionOf(string methodName) =>
- Assert.Single(AgentActions.Actions.OfType(), a => a.MethodName == methodName);
-
///
/// Runs the custom action that reads the enrollment token file, in a session of a package in
/// where (NAME=value) are set.
@@ -454,62 +451,4 @@ private static (ActionResult Result, string Token, List<(InstallMessage Type, st
MsiInstaller.SetExternalUI(previous, InstallLogModes.None);
}
}
-
- ///
- /// Evaluates with the Windows Installer engine, in a session where
- /// (NAME=value) are set.
- ///
- private static bool EvaluateCondition(string condition, string[] properties) =>
- WithSession(properties, session => session.EvaluateCondition(condition));
-
- private 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.
- ///
- private 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);
- }
- }
-
- ///
- /// A new, empty folder under the temporary directory, deleted with its content on disposal.
- ///
- private sealed class TempFolder : IDisposable
- {
- public string Path { get; } = System.IO.Path.Combine(System.IO.Path.GetTempPath(), $"agent-identity-{Guid.NewGuid():N}");
-
- public TempFolder() => Directory.CreateDirectory(Path);
-
- public void Dispose() => Directory.Delete(Path, recursive: true);
- }
}
diff --git a/package/AgentWindowsManaged.Tests/DataFolderTests.cs b/package/AgentWindowsManaged.Tests/DataFolderTests.cs
new file mode 100644
index 000000000..fbc9a1dc3
--- /dev/null
+++ b/package/AgentWindowsManaged.Tests/DataFolderTests.cs
@@ -0,0 +1,393 @@
+using System.IO;
+using System.Linq;
+using System.Security.AccessControl;
+using System.Security.Principal;
+using System.Text.RegularExpressions;
+
+using DevolutionsAgent.Actions;
+using DevolutionsAgent.Resources;
+
+using WixSharp;
+
+using Xunit;
+using Xunit.Abstractions;
+
+using static DevolutionsAgent.Installer.Tests.InstallerSession;
+
+namespace DevolutionsAgent.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 Agent = 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 AgentSddl = $"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.SetProgramDataDirectoryPermissions),
+ nameof(CustomActions.SetProgramDataPedmDirectoryPermissions),
+ ];
+
+ private readonly ILogger logger;
+
+ public DataFolderTests(ITestOutputHelper output) => this.logger = new OutputLogger(output);
+
+ [Theory]
+ // oracle: intent package/AgentWindowsManaged/INTENT.md §Data folder
+ [InlineData("SYSTEM owns Devolutions", Devolutions, "O:SY" + InheritedDacl, "none")]
+ // oracle: intent package/AgentWindowsManaged/INTENT.md §Data folder
+ [InlineData("Administrators own Devolutions", Devolutions, "O:BA" + InheritedDacl, "none")]
+ // oracle: intent package/AgentWindowsManaged/INTENT.md §Data folder
+ [InlineData("Users own Devolutions", Devolutions, "O:BU" + InheritedDacl, "Owner " + Users)]
+ // oracle: intent package/AgentWindowsManaged/INTENT.md §Data folder
+ [InlineData("Everyone owns Devolutions", Devolutions, "O:WD" + InheritedDacl, "Owner " + Everyone)]
+ // oracle: intent package/AgentWindowsManaged/INTENT.md §Data folder
+ [InlineData("Users own Agent", Agent, "O:BU" + InheritedDacl, "Owner " + Users)]
+ // oracle: reference Devolutions/devolutions-gateway@9b8b60380f6c:crates/private-fs/src/windows/security.rs Policy::check_ancestor
+ [InlineData("Users may delete Devolutions", Devolutions, "O:SYD:(A;;SD;;;BU)", "Access " + Users)]
+ // The built-in Administrator account (LA) is a member of Administrators on every machine.
+ // oracle: intent package/AgentWindowsManaged/INTENT.md §Data folder
+ [InlineData("a member of Administrators may delete Devolutions", Devolutions, "O:SYD:(A;OICI;FA;;;LA)", "none")]
+ // oracle: reference Devolutions/devolutions-gateway@9b8b60380f6c:crates/private-fs/src/windows/security.rs Policy::check_ancestor
+ [InlineData("Users may change the DACL", Devolutions, "O:SYD:(A;;WD;;;BU)", "Access " + Users)]
+ // oracle: reference Devolutions/devolutions-gateway@9b8b60380f6c:crates/private-fs/src/windows/security.rs Policy::check_ancestor
+ [InlineData("Users may take ownership", Devolutions, "O:SYD:(A;;WO;;;BU)", "Access " + Users)]
+ // oracle: reference Devolutions/devolutions-gateway@9b8b60380f6c:crates/private-fs/src/windows/security.rs Policy::check_ancestor
+ [InlineData("Users may delete entries", Devolutions, "O:SYD:(A;;0x40;;;BU)", "Access " + Users)]
+ // oracle: reference Devolutions/devolutions-gateway@9b8b60380f6c:crates/private-fs/src/windows/security.rs Policy::check_ancestor
+ [InlineData("Users have generic all", Devolutions, "O:SYD:(A;;GA;;;BU)", "Access " + Users)]
+ // oracle: reference Devolutions/devolutions-gateway@9b8b60380f6c:crates/private-fs/src/windows/security.rs Policy::check_ancestor
+ [InlineData("Users may delete through a conditional entry", Devolutions, "O:SYD:(XA;;SD;;;BU;(Member_of {SID(BU)}))", "Access " + Users)]
+ // oracle: intent package/AgentWindowsManaged/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: reference Devolutions/devolutions-gateway@9b8b60380f6c:crates/private-fs/src/windows/security.rs Policy::check_ancestor
+ [InlineData("Users may only add entries", Devolutions, "O:SYD:(A;CI;DCLCRPCR;;;BU)", "none")]
+ // oracle: reference Devolutions/devolutions-gateway@9b8b60380f6c:crates/private-fs/src/windows/security.rs Policy::check_ancestor
+ [InlineData("Users are denied deletion", Devolutions, "O:SYD:(D;;SD;;;BU)", "none")]
+ // oracle: intent package/AgentWindowsManaged/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/AgentWindowsManaged/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/AgentWindowsManaged/INTENT.md §Data folder
+ [InlineData("Users have full control of Agent", Agent, "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);
+
+ Assert.True(expected == Describe(refusal), $"{scenario}: {Describe(refusal)}");
+ }
+
+ // oracle: intent package/AgentWindowsManaged/INTENT.md §Data folder
+ [Theory]
+ [InlineData(Devolutions)]
+ [InlineData(Agent)]
+ public void LinkIsRefusedWhateverItsSecurity(bool checkAccess)
+ {
+ DataFolderRefusal refusal = DataFolders.Evaluate(
+ "folder", new RawSecurityDescriptor("O:SY" + InheritedDacl), isLink: true, isDirectory: true, checkAccess);
+
+ Assert.Equal("Link", Describe(refusal));
+ }
+
+ // oracle: intent package/AgentWindowsManaged/INTENT.md §Data folder
+ [Theory]
+ [InlineData(Devolutions)]
+ [InlineData(Agent)]
+ public void FileIsRefusedWhateverItsSecurity(bool checkAccess)
+ {
+ DataFolderRefusal refusal = DataFolders.Evaluate(
+ "folder", new RawSecurityDescriptor("O:SY" + InheritedDacl), isLink: false, isDirectory: false, checkAccess);
+
+ Assert.Equal("File", Describe(refusal));
+ }
+
+ // The agent checks Agent as a directory above its data, so the security the installer gives Agent must pass that
+ // check.
+ // oracle: reference Devolutions/devolutions-gateway@9b8b60380f6c:crates/private-fs/src/windows/security.rs Policy::check_ancestor
+ [Fact]
+ public void AgentFolderSecurityPassesTheDaclRule()
+ {
+ DataFolderRefusal refusal = DataFolders.Evaluate(
+ "folder", new RawSecurityDescriptor(Includes.PROGRAM_DATA_SDDL), isLink: false, isDirectory: true, Devolutions);
+
+ Assert.Equal("none", Describe(refusal));
+ }
+
+ // oracle: intent package/AgentWindowsManaged/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/AgentWindowsManaged/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, AgentSddl, this.logger);
+
+ Assert.Equal((devolutions, "File", "kept"), (refusal?.Folder, Describe(refusal), System.IO.File.ReadAllText(devolutions)));
+ }
+
+ // oracle: intent package/AgentWindowsManaged/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)));
+ }
+
+ [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, AgentSddl, this.logger);
+
+ // oracle: intent package/AgentWindowsManaged/INTENT.md §Data folder
+ Assert.Equal((devolutions, "Link"), (refusal?.Folder, Describe(refusal)));
+ // oracle: intent package/AgentWindowsManaged/INTENT.md §Data folder
+ Assert.Empty(Directory.GetFileSystemEntries(target));
+ }
+
+ [Theory]
+ // oracle: intent package/AgentWindowsManaged/INTENT.md §Data folder
+ [InlineData("Users may delete Devolutions", "SD")]
+ // oracle: intent package/AgentWindowsManaged/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/AgentWindowsManaged/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, AgentSddl, this.logger);
+
+ Assert.Equal((devolutions, "Access " + Users), (refusal?.Folder, Describe(refusal)));
+ Assert.Empty(Directory.GetFileSystemEntries(devolutions));
+ }
+
+ // oracle: intent package/AgentWindowsManaged/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/AgentWindowsManaged/INTENT.md §Data folder
+ [Fact]
+ public void CheckPassesWhenTheFoldersDontExist()
+ {
+ using TempFolder folder = new();
+
+ Assert.Null(DataFolders.Check(folder.Path, this.logger));
+ }
+
+ // oracle: intent package/AgentWindowsManaged/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/AgentWindowsManaged/INTENT.md §Data folder
+ [Fact]
+ public void ErrorIsInTheInstallerLanguage()
+ {
+ DataFolderRefusal refusal = new(@"C:\ProgramData\Devolutions", DataFolderProblem.Link);
+ string english = refusal.Message("1033");
+
+ Assert.NotEqual(english, refusal.Message("1036"));
+ Assert.NotEqual(english, refusal.Message("1053"));
+ // Windows Installer applies no language transform for German, so the rest of the installer is in English too.
+ Assert.Equal(english, refusal.Message("1031"));
+ }
+
+ // oracle: intent package/AgentWindowsManaged/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 = Program.Localize("1033", id);
+
+ Assert.NotEqual(id, english);
+ Assert.All(new[] { "1036", "1053" }, language =>
+ {
+ string translated = Program.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/AgentWindowsManaged/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(';'));
+ }
+
+ // oracle: intent package/AgentWindowsManaged/INTENT.md §Data folder
+ [Theory]
+ [InlineData(nameof(CustomActions.SetProgramDataDirectoryPermissions))]
+ [InlineData(nameof(CustomActions.SetProgramDataPedmDirectoryPermissions))]
+ public void FailedSecurityResetFailsTheRun(string method)
+ {
+ Assert.Equal(Return.check, ActionOf(method).Return);
+ }
+
+ [Theory]
+ // oracle: intent package/AgentWindowsManaged/INTENT.md §Data folder
+ [InlineData("first install", true)]
+ // oracle: intent package/AgentWindowsManaged/INTENT.md §Data folder
+ [InlineData("major upgrade", true, "WIX_UPGRADE_DETECTED={00000000-0000-0000-0000-000000000001}")]
+ // oracle: intent package/AgentWindowsManaged/INTENT.md §Data folder
+ [InlineData("repair", true, "Installed=00:00:00", "REINSTALL=ALL")]
+ // oracle: intent package/AgentWindowsManaged/INTENT.md §Data folder
+ [InlineData("maintenance adding a feature", true, "Installed=00:00:00", "ADDLOCAL=F.Session")]
+ // oracle: intent package/AgentWindowsManaged/INTENT.md §Data folder
+ [InlineData("maintenance removing a feature", true, "Installed=00:00:00", "REMOVE=F.Session")]
+ // oracle: intent package/AgentWindowsManaged/INTENT.md §Data folder
+ [InlineData("full uninstall", false, "Installed=00:00:00", "REMOVE=ALL")]
+ // oracle: intent package/AgentWindowsManaged/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/AgentWindowsManaged/INTENT.md §Data folder
+ [Fact]
+ public void CheckRunsBeforeTheMajorUpgradeRemovesTheInstalledProduct()
+ {
+ Assert.Equal(When.Before, AgentActions.checkProgramDataDirectory.When);
+ Assert.Equal(Step.RemoveExistingProducts.ToString(), AgentActions.checkProgramDataDirectory.Step.ToString());
+ }
+
+ // oracle: intent package/AgentWindowsManaged/INTENT.md §Data folder
+ [Fact]
+ public void PedmFolderIsCreatedAfterTheAgentFolder()
+ {
+ // Created first, the pedm folder would also create Agent, with the DACL it inherits.
+ Assert.Equal(When.After, AgentActions.createProgramDataPedmDirectories.When);
+ Assert.Equal(AgentActions.createProgramDataDirectory.Id, AgentActions.createProgramDataPedmDirectories.Step.ToString());
+ }
+
+ ///
+ /// 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 string[] Placeholders(string text) =>
+ Regex.Matches(text, @"\{\d+\}").Cast().Select(m => m.Value).OrderBy(p => p).ToArray();
+}
diff --git a/package/AgentWindowsManaged.Tests/InstallerSession.cs b/package/AgentWindowsManaged.Tests/InstallerSession.cs
new file mode 100644
index 000000000..72e7e5fc7
--- /dev/null
+++ b/package/AgentWindowsManaged.Tests/InstallerSession.cs
@@ -0,0 +1,82 @@
+using System;
+using System.IO;
+using System.Linq;
+
+using DevolutionsAgent.Actions;
+
+using Microsoft.Deployment.WindowsInstaller;
+
+using WixSharp;
+
+using Xunit;
+
+using MsiInstaller = Microsoft.Deployment.WindowsInstaller.Installer;
+
+namespace DevolutionsAgent.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(AgentActions.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/AgentWindowsManaged.Tests/OutputLogger.cs b/package/AgentWindowsManaged.Tests/OutputLogger.cs
new file mode 100644
index 000000000..1b8b73e61
--- /dev/null
+++ b/package/AgentWindowsManaged.Tests/OutputLogger.cs
@@ -0,0 +1,15 @@
+using DevolutionsAgent.Actions;
+
+using Xunit.Abstractions;
+
+namespace DevolutionsAgent.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/AgentWindowsManaged.Tests/SecureDirectoryTests.cs b/package/AgentWindowsManaged.Tests/SecureDirectoryTests.cs
new file mode 100644
index 000000000..1c2dfb617
--- /dev/null
+++ b/package/AgentWindowsManaged.Tests/SecureDirectoryTests.cs
@@ -0,0 +1,303 @@
+using System;
+using System.IO;
+using System.Linq;
+using System.Security.AccessControl;
+using System.Security.Principal;
+
+using DevolutionsAgent.Actions;
+
+using Microsoft.Win32.SafeHandles;
+
+using Xunit;
+using Xunit.Abstractions;
+
+namespace DevolutionsAgent.Installer.Tests;
+
+public sealed class SecureDirectoryTests
+{
+ private static readonly SecurityIdentifier CurrentUser = WindowsIdentity.GetCurrent().User;
+
+ private static readonly SecurityIdentifier LocalSystem = new(WellKnownSidType.LocalSystemSid, 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[] IdentityNames = ["Devolutions", "Agent", "identity"];
+
+ private readonly ILogger logger;
+
+ public SecureDirectoryTests(ITestOutputHelper output) => this.logger = new OutputLogger(output);
+
+ // oracle: intent package/AgentWindowsManaged/INTENT.md §Agent Identity
+ [Fact]
+ public void DeleteTreeKeepsWhatAJunctionInsideTheTreePointsTo()
+ {
+ using TempFolder folder = new();
+ string outside = folder.CreateDirectory("outside");
+ File.WriteAllText(Path.Combine(outside, "file.txt"), "kept");
+ string identity = folder.CreateDirectory(IdentityNames);
+ string pending = Directory.CreateDirectory(Path.Combine(identity, "pending")).FullName;
+ File.WriteAllText(Path.Combine(pending, "token.dat"), "deleted");
+ TempFolder.CreateJunction(Path.Combine(pending, "link"), outside);
+
+ SecureDirectory.DeleteTree(folder.Path, IdentityNames, this.logger);
+
+ Assert.False(Directory.Exists(identity));
+ Assert.True(File.Exists(Path.Combine(outside, "file.txt")));
+ }
+
+ // oracle: intent package/AgentWindowsManaged/INTENT.md §Agent Identity
+ [Fact]
+ public void DeleteTreeDeletesAJunctionAtThePathButNotItsTarget()
+ {
+ using TempFolder folder = new();
+ string outside = folder.CreateDirectory("outside");
+ File.WriteAllText(Path.Combine(outside, "file.txt"), "kept");
+ folder.CreateDirectory("Devolutions", "Agent");
+ string identity = Path.Combine(folder.Path, "Devolutions", "Agent", "identity");
+ TempFolder.CreateJunction(identity, outside);
+
+ SecureDirectory.DeleteTree(folder.Path, IdentityNames, this.logger);
+
+ Assert.False(Directory.Exists(identity));
+ Assert.True(File.Exists(Path.Combine(outside, "file.txt")));
+ }
+
+ // oracle: intent package/AgentWindowsManaged/INTENT.md §Agent Identity
+ [Fact]
+ public void DeleteTreeDeletesNothingThroughAJunctionAboveThePath()
+ {
+ using TempFolder folder = new();
+ string outside = folder.CreateDirectory("outside");
+ string outsideIdentity = folder.CreateDirectory("outside", "identity");
+ File.WriteAllText(Path.Combine(outsideIdentity, "file.txt"), "kept");
+ folder.CreateDirectory("Devolutions");
+ TempFolder.CreateJunction(Path.Combine(folder.Path, "Devolutions", "Agent"), outside);
+
+ SecureDirectory.DeleteTree(folder.Path, IdentityNames, this.logger);
+
+ Assert.True(File.Exists(Path.Combine(outsideIdentity, "file.txt")));
+ }
+
+ // oracle: intent package/AgentWindowsManaged/INTENT.md §Agent Identity
+ [Fact]
+ public void DeleteTreeKeepsGoingAfterAFailureAndKeepsOnlyWhatItCouldntDelete()
+ {
+ using TempFolder folder = new();
+ string identity = folder.CreateDirectory(IdentityNames);
+ string pending = folder.CreateDirectory([.. IdentityNames, "pending"]);
+ string held = Path.Combine(pending, "held.dat");
+ File.WriteAllText(held, "kept");
+ File.WriteAllText(Path.Combine(pending, "sibling.dat"), "deleted");
+ File.WriteAllText(Path.Combine(identity, "other.dat"), "deleted");
+
+ // Opened without delete sharing, the file can't be deleted while the stream is open.
+ using (new FileStream(held, FileMode.Open, FileAccess.Read, FileShare.ReadWrite))
+ {
+ SecureDirectory.DeleteTree(folder.Path, IdentityNames, this.logger);
+ }
+
+ Assert.Equal([held], Directory.GetFileSystemEntries(identity, "*", SearchOption.AllDirectories)
+ .Where(File.Exists)
+ .ToArray());
+ Assert.Equal([pending], Directory.GetDirectories(identity));
+ }
+
+ // oracle: intent package/AgentWindowsManaged/INTENT.md §Agent Identity
+ [Fact]
+ public void HeldDirectoryCantBeRenamedUntilItsHandleIsClosed()
+ {
+ AssertHoldsUntilClosed(path => SecureDirectory.Hold(path));
+ }
+
+ // oracle: intent package/AgentWindowsManaged/INTENT.md §Data folder
+ [Fact]
+ public void DirectoryOpenedToReadAndHoldCantBeRenamedUntilItsHandleIsClosed()
+ {
+ AssertHoldsUntilClosed(path => SecureDirectory.OpenToRead(path, hold: true));
+ }
+
+ // oracle: intent package/AgentWindowsManaged/INTENT.md §Data folder
+ [Fact]
+ public void SetSecurityRefusesAJunctionAndLeavesItsTargetUnchanged()
+ {
+ using TempFolder folder = new();
+ string target = folder.CreateDirectory("target");
+ string before = Sddl(target);
+ TempFolder.CreateJunction(Path.Combine(folder.Path, "link"), target);
+
+ Assert.Throws(() => SecureDirectory.SetSecurity(folder.Path, ["link"], ProtectedSddl));
+
+ Assert.Equal(before, Sddl(target));
+ }
+
+ // oracle: intent package/AgentWindowsManaged/INTENT.md §Data folder
+ [Fact]
+ public void SetSecurityRefusesAJunctionAboveTheFolderAndLeavesTheFolderBelowItsTargetUnchanged()
+ {
+ using TempFolder folder = new();
+ string target = folder.CreateDirectory("target");
+ string below = folder.CreateDirectory("target", "data");
+ string before = Sddl(below);
+ TempFolder.CreateJunction(Path.Combine(folder.Path, "link"), target);
+
+ Assert.Throws(() => SecureDirectory.SetSecurity(folder.Path, ["link", "data"], ProtectedSddl));
+
+ Assert.Equal(before, Sddl(below));
+ }
+
+ // oracle: intent package/AgentWindowsManaged/INTENT.md §Data folder
+ [Fact]
+ public void SetSecurityRefusesAFileAndLeavesItUnchanged()
+ {
+ using TempFolder folder = new();
+ string path = Path.Combine(folder.Path, "data");
+ File.WriteAllText(path, "");
+ string before = File.GetAccessControl(path).GetSecurityDescriptorSddlForm(AccessControlSections.All);
+
+ Assert.Throws(() => SecureDirectory.SetSecurity(folder.Path, ["data"], ProtectedSddl));
+
+ Assert.Equal(before, File.GetAccessControl(path).GetSecurityDescriptorSddlForm(AccessControlSections.All));
+ }
+
+ // oracle: intent package/AgentWindowsManaged/INTENT.md §Data folder
+ [Fact]
+ public void SetSecurityAppliesTheOwnerAndTheProtectedDacl()
+ {
+ using TempFolder folder = new();
+ string path = folder.CreateDirectory("parent", "data");
+
+ SecureDirectory.SetSecurity(folder.Path, ["parent", "data"], ProtectedSddl);
+
+ AssertProtectedSecurity(path);
+ }
+
+ // The agent requires its directories to have exactly the DACL it provisions, so propagated entries would make it
+ // refuse them.
+ // oracle: intent crates/agent-identity/INTENT.md §Local state
+ [Fact]
+ public void SetSecurityLeavesExistingChildrenUnchanged()
+ {
+ using TempFolder folder = new();
+ folder.CreateDirectory("data");
+ string child = folder.CreateDirectory("data", "child");
+ string before = RawSddl(child);
+
+ SecureDirectory.SetSecurity(folder.Path, ["data"], ProtectedSddl);
+
+ Assert.Equal(before, RawSddl(child));
+ }
+
+ // oracle: intent package/AgentWindowsManaged/INTENT.md §Data folder
+ [Fact]
+ public void CreateGivesTheNewDirectoryExactlyTheSecurityDescriptor()
+ {
+ using TempFolder folder = new();
+ string path = Path.Combine(folder.Path, "data");
+
+ Assert.True(SecureDirectory.Create(path, ProtectedSddl));
+
+ AssertProtectedSecurity(path);
+ }
+
+ [Theory]
+ // oracle: intent package/AgentWindowsManaged/INTENT.md §Data folder
+ [InlineData("SYSTEM", "S-1-5-18", true)]
+ // oracle: intent package/AgentWindowsManaged/INTENT.md §Data folder
+ [InlineData("Administrators", "S-1-5-32-544", true)]
+ // oracle: intent package/AgentWindowsManaged/INTENT.md §Data folder
+ [InlineData("TrustedInstaller", "S-1-5-80-956008885-3418522649-1831038044-1853292631-2271478464", true)]
+ // oracle: intent package/AgentWindowsManaged/INTENT.md §Data folder
+ [InlineData("Everyone", "S-1-1-0", false)]
+ // oracle: intent package/AgentWindowsManaged/INTENT.md §Data folder
+ [InlineData("Users", "S-1-5-32-545", false)]
+ // oracle: intent package/AgentWindowsManaged/INTENT.md §Data folder
+ [InlineData("LOCAL SERVICE", "S-1-5-19", false)]
+ public void OwnerIsTrustedOnlyWhenPrivileged(string name, string sid, bool expected)
+ {
+ Assert.True(expected == SecureDirectory.IsTrustedOwner(new SecurityIdentifier(sid)), name);
+ }
+
+ // oracle: property round-trip
+ [Fact]
+ public void ReadSecurityReadsTheOwnerAndTheDaclThatSetSecurityApplied()
+ {
+ using TempFolder folder = new();
+ string path = folder.CreateDirectory("data");
+ SecureDirectory.SetSecurity(folder.Path, ["data"], 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);
+ }
+
+ ///
+ /// 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(string path)
+ {
+ DirectorySecurity security = new DirectoryInfo(path).GetAccessControl();
+
+ 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} {r.InheritanceFlags} {r.PropagationFlags} inherited={r.IsInherited}")
+ .OrderBy(r => r, StringComparer.Ordinal)
+ .ToArray();
+ string[] expected = new[] { CurrentUser, LocalSystem }
+ .Select(sid => $"{sid} Allow FullControl ContainerInherit, ObjectInherit None 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 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/AgentWindowsManaged.Tests/TempFolder.cs b/package/AgentWindowsManaged.Tests/TempFolder.cs
new file mode 100644
index 000000000..d43a21625
--- /dev/null
+++ b/package/AgentWindowsManaged.Tests/TempFolder.cs
@@ -0,0 +1,66 @@
+using System;
+using System.Diagnostics;
+using System.IO;
+
+using Xunit;
+
+namespace DevolutionsAgent.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);
+ }
+
+ 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/AgentWindowsManaged/Actions/AgentActions.cs b/package/AgentWindowsManaged/Actions/AgentActions.cs
index 795fc6648..ccf722921 100644
--- a/package/AgentWindowsManaged/Actions/AgentActions.cs
+++ b/package/AgentWindowsManaged/Actions/AgentActions.cs
@@ -110,19 +110,45 @@ internal static class AgentActions
// Deferred sequence
///
- /// Create the path %ProgramData%\Devolutions\Agent if it does not exist
+ /// Fail the run, with an error naming the folder, when %ProgramData%\Devolutions or %ProgramData%\Devolutions\Agent
+ /// 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,
+ new Condition("NOT (REMOVE=\"ALL\")"),
+ Sequence.InstallExecuteSequence)
+ {
+ Execute = Execute.deferred,
+ Impersonate = false,
+ UsesProperties = "ProductLanguage=[ProductLanguage]",
+ };
+
+ ///
+ /// Create the path %ProgramData%\Devolutions\Agent if it does not exist, 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);
+ new Condition("NOT (REMOVE=\"ALL\")"),
+ Sequence.InstallExecuteSequence)
+ {
+ UsesProperties = "ProductLanguage=[ProductLanguage]",
+ };
///
/// Create the path %ProgramData%\Devolutions\Agent\Pedm and subfolders if they do not exist
@@ -130,12 +156,14 @@ internal static class AgentActions
///
/// 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.
+ /// It runs after , which checks the folders above and creates Agent with its
+ /// own security descriptor, so that it never creates Agent itself with an inherited DACL.
///
- private static readonly ElevatedManagedAction createProgramDataPedmDirectories = new(
+ internal static readonly ElevatedManagedAction createProgramDataPedmDirectories = new(
new Id($"CA.{nameof(createProgramDataPedmDirectories)}"),
CustomActions.CreateProgramDataPedmDirectories,
Return.check,
- When.After, Step.CreateFolders,
+ When.After, new Step(createProgramDataDirectory.Id),
Features.PEDM_FEATURE.BeingInstall(),
Sequence.InstallExecuteSequence);
@@ -145,9 +173,9 @@ internal static class AgentActions
private static readonly ElevatedManagedAction setProgramDataDirectoryPermissions = new(
new Id($"CA.{nameof(setProgramDataDirectoryPermissions)}"),
CustomActions.SetProgramDataDirectoryPermissions,
- Return.ignore,
+ Return.check,
When.After, new Step(createProgramDataDirectory.Id),
- Condition.Always,
+ new Condition("NOT (REMOVE=\"ALL\")"),
Sequence.InstallExecuteSequence)
{
Execute = Execute.deferred,
@@ -155,14 +183,14 @@ internal static class AgentActions
};
///
- /// Set or reset the ACL on %ProgramData%\Devolutions\Agent\pedm
+ /// Set or reset the ACL on %ProgramData%\Devolutions\Agent\pedm, when it exists
///
private static readonly ElevatedManagedAction setProgramDataPedmDirectoryPermissions = new(
new Id($"CA.{nameof(setProgramDataPedmDirectoryPermissions)}"),
CustomActions.SetProgramDataPedmDirectoryPermissions,
- Return.ignore,
+ Return.check,
When.After, new Step(createProgramDataPedmDirectories.Id),
- Condition.Always,
+ new Condition("NOT (REMOVE=\"ALL\")"),
Sequence.InstallExecuteSequence)
{
Execute = Execute.deferred,
@@ -586,6 +614,7 @@ private static string UseProperties(IEnumerable properties)
configurePsuAgent,
rollbackConfigurePsuAgent,
commitConfigurePsuAgent,
+ checkProgramDataDirectory,
createProgramDataDirectory,
setProgramDataDirectoryPermissions,
createProgramDataPedmDirectories,
diff --git a/package/AgentWindowsManaged/Actions/CustomActions.cs b/package/AgentWindowsManaged/Actions/CustomActions.cs
index e41c1f7ae..0c23053cd 100644
--- a/package/AgentWindowsManaged/Actions/CustomActions.cs
+++ b/package/AgentWindowsManaged/Actions/CustomActions.cs
@@ -148,6 +148,24 @@ public static ActionResult CleanAgentConfigRollback(Session session)
return ActionResult.Failure;
}
+ [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)
{
@@ -155,8 +173,17 @@ public static ActionResult CreateProgramDataDirectory(Session session)
try
{
- DirectoryInfo di = Directory.CreateDirectory(path);
- session.Log($"created directory at {di.FullName} or already exists");
+ DataFolderRefusal refusal = DataFolders.Create(
+ Environment.GetFolderPath(Environment.SpecialFolder.CommonApplicationData),
+ Includes.PROGRAM_DATA_SDDL,
+ LogDelegate.WithSession(session));
+
+ if (refusal is not null)
+ {
+ return Refuse(session, refusal);
+ }
+
+ session.Log($"created directory at {path} or already exists");
}
catch (Exception e)
{
@@ -167,6 +194,22 @@ 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, as ReadIdentityEnrollmentTokenFile does.
+ string message = refusal.Message(session.Property("ProductLanguage"));
+ session.Log(message);
+ using Record record = new(0) { FormatString = EscapeFormatted(message) };
+ session.Message(InstallMessage.Error | (uint)MessageButtons.OK, record);
+ return ActionResult.Failure;
+ }
+
[CustomAction]
public static ActionResult CreateProgramDataPedmDirectories(Session session)
{
@@ -805,22 +848,18 @@ public static ActionResult RemoveAgentIdentity(Session session)
}
}
- string identityDirectory = Path.Combine(ProgramDataDirectory, "identity");
-
+ // The deletion never follows a link, so that a link in the data directory can't make the uninstall delete
+ // files elsewhere.
try
{
- if (Directory.Exists(identityDirectory))
- {
- // UNTESTED: Directory.Delete deletes a junction or symbolic link without recursing into its target.
- // Otherwise a link planted in the identity folder would make the uninstall delete files elsewhere.
- // When replacing this call, keep the deletion from following reparse points.
- Directory.Delete(identityDirectory, true);
- session.Log($"deleted {identityDirectory}");
- }
+ SecureDirectory.DeleteTree(
+ Environment.GetFolderPath(Environment.SpecialFolder.CommonApplicationData),
+ ["Devolutions", "Agent", "identity"],
+ LogDelegate.WithSession(session));
}
catch (Exception e)
{
- session.Log($"failed to delete {identityDirectory}: {e}");
+ session.Log($"failed to delete the Agent Identity state: {e}");
}
return ActionResult.Success;
@@ -1889,7 +1928,10 @@ public static ActionResult SetProgramDataDirectoryPermissions(Session session)
{
try
{
- SetFileSecurity(session, ProgramDataDirectory, Includes.PROGRAM_DATA_SDDL);
+ SecureDirectory.SetSecurity(
+ Environment.GetFolderPath(Environment.SpecialFolder.CommonApplicationData),
+ ["Devolutions", "Agent"],
+ Includes.PROGRAM_DATA_SDDL);
return ActionResult.Success;
}
catch (Exception e)
@@ -1904,7 +1946,18 @@ public static ActionResult SetProgramDataPedmDirectoryPermissions(Session sessio
{
try
{
- SetFileSecurity(session, Path.Combine(ProgramDataDirectory, "pedm"), Includes.PROGRAM_DATA_PEDM_SDDL);
+ SecureDirectory.SetSecurity(
+ Environment.GetFolderPath(Environment.SpecialFolder.CommonApplicationData),
+ ["Devolutions", "Agent", "pedm"],
+ Includes.PROGRAM_DATA_PEDM_SDDL);
+ return ActionResult.Success;
+ }
+ catch (Win32Exception e) when (e.NativeErrorCode == WinAPI.ERROR_FILE_NOT_FOUND)
+ {
+ // UNTESTED: a missing pedm folder doesn't fail the run, as only the PEDM feature creates it.
+ // No test runs this action, which reads the machine's own ProgramData.
+ // When changing this, keep accepting a missing pedm folder and nothing else that fails to open.
+ session.Log($"no pedm folder to set permissions on: {e.Message}");
return ActionResult.Success;
}
catch (Exception e)
diff --git a/package/AgentWindowsManaged/Actions/DataFolders.cs b/package/AgentWindowsManaged/Actions/DataFolders.cs
new file mode 100644
index 000000000..128aff4f8
--- /dev/null
+++ b/package/AgentWindowsManaged/Actions/DataFolders.cs
@@ -0,0 +1,370 @@
+using DevolutionsAgent.Resources;
+using Microsoft.Win32.SafeHandles;
+using System;
+using System.ComponentModel;
+using System.IO;
+using System.Security.AccessControl;
+using System.Security.Principal;
+
+namespace DevolutionsAgent.Actions;
+
+///
+/// Why the installer refuses to use a folder of the agent'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 agent'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(Program.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\Agent under ProgramData before the installer uses them.
+///
+///
+/// The installer writes the agent's data there as SYSTEM, so an account that controls either folder could redirect
+/// or replace what the installer and the agent write.
+/// A folder that doesn't exist yet passes.
+///
+internal static class DataFolders
+{
+ private const string Devolutions = "Devolutions";
+
+ private const string Agent = "Agent";
+
+ ///
+ /// 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\Agent 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 Agent 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 Agent checked in both, without the DACL rule, as the installer resets its DACL.
+ refusal = Inspect(
+ Path.Combine(devolutions, Agent), path => SecureDirectory.OpenToRead(path, hold: false),
+ checkAccess: false, logger, out SafeFileHandle agent);
+ agent?.Dispose();
+
+ return refusal;
+ }
+ }
+
+ ///
+ /// Creates Devolutions\Agent under with the security descriptor
+ /// , after checking Devolutions, or checks an existing Agent 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.
+ /// Agent 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 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 agent = Path.Combine(devolutions, Agent);
+
+ SecureDirectory.Create(devolutions, sddl: null);
+
+ // UNTESTED: the handle on Devolutions stays open until Agent 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 Agent 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(agent, sddl))
+ {
+ logger.Log($"created {agent}");
+ return null;
+ }
+
+ refusal = Inspect(
+ agent, SecureDirectory.OpenToSetSecurity, checkAccess: false, logger, out SafeFileHandle existing);
+
+ using (existing)
+ {
+ if (refusal is not null)
+ {
+ return refusal;
+ }
+
+ if (existing is null)
+ {
+ throw new IOException($"{agent} 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 Agent on this handle, as the agent
+ // refuses a data folder whose DACL isn't the one provisioned.
+ SecureDirectory.SetSecurity(existing, agent, sddl);
+ logger.Log($"reset the owner and the DACL of {agent}");
+
+ return null;
+ }
+ }
+ }
+
+ ///
+ /// Whether the installer refuses to use , given its security descriptor and whether it is a
+ /// link.
+ ///
+ ///
+ /// The DACL rule refuses what the agent refuses for a directory above its data, so that the installer never prepares
+ /// a folder the agent then refuses to use.
+ ///
+ ///
+ /// 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)
+ {
+ 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, whose trust may need a directory lookup.
+ 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;
+ // The agent refuses an entry of any other type, so the installer does too.
+ 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;
+
+ if ((unchecked((uint)entry.AccessMask) & ReplaceRights) != 0
+ && entry.SecurityIdentifier != OwnerRights
+ && !IsAppContainer(entry.SecurityIdentifier)
+ && !IsTrusted(entry.SecurityIdentifier))
+ {
+ return new DataFolderRefusal(folder, DataFolderProblem.Access, entry.SecurityIdentifier);
+ }
+ }
+ }
+
+ return IsTrusted(security.Owner) ? null : new DataFolderRefusal(folder, DataFolderProblem.Owner, security.Owner);
+ }
+
+ ///
+ /// Whether is SYSTEM, Administrators, TrustedInstaller or a member of Administrators, the
+ /// accounts that may own the folder or rename and replace it.
+ ///
+ private static bool IsTrusted(SecurityIdentifier sid)
+ {
+ try
+ {
+ return SecureDirectory.IsTrustedOwner(sid);
+ }
+ catch (Exception)
+ {
+ // An account whose group membership can't be resolved isn't known to be an administrator.
+ 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.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/AgentWindowsManaged/Actions/SecureDirectory.cs b/package/AgentWindowsManaged/Actions/SecureDirectory.cs
new file mode 100644
index 000000000..3ecb6d53e
--- /dev/null
+++ b/package/AgentWindowsManaged/Actions/SecureDirectory.cs
@@ -0,0 +1,499 @@
+using Microsoft.Win32.SafeHandles;
+using System;
+using System.Collections.Generic;
+using System.ComponentModel;
+using System.IO;
+using System.Linq;
+using System.Runtime.InteropServices;
+using System.Security.AccessControl;
+using System.Security.Principal;
+
+namespace DevolutionsAgent.Actions;
+
+///
+/// Directory 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);
+
+ ///
+ /// 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;
+
+ ///
+ /// 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);
+ WinAPI.SECURITY_ATTRIBUTES attributes = new()
+ {
+ nLength = (uint)Marshal.SizeOf(),
+ lpSecurityDescriptor = descriptor?.Pointer ?? IntPtr.Zero,
+ };
+
+ if (WinAPI.CreateDirectory(path, ref attributes))
+ {
+ 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 directory
+ /// \[0]\…\[n-1] to those of
+ /// , without following any link.
+ ///
+ ///
+ /// Each directory above the target is held while the next one is opened, so the target is below the directories
+ /// checked.
+ ///
+ /// An entry on the path is a reparse point or not a directory.
+ internal static void SetSecurity(string root, string[] names, string sddl)
+ {
+ List parents = new();
+
+ try
+ {
+ string path = root;
+
+ foreach (string name in names.Take(names.Length - 1))
+ {
+ path = Path.Combine(path, name);
+ SafeFileHandle parent = Hold(path);
+ parents.Add(parent);
+
+ if (!IsPlainDirectory(GetAttributes(parent)))
+ {
+ throw new IOException($"{path} is a reparse point or not a directory");
+ }
+ }
+
+ string target = Path.Combine(path, names[names.Length - 1]);
+ using SafeFileHandle handle = OpenToSetSecurity(target);
+ SetSecurity(handle, target, sddl);
+ }
+ finally
+ {
+ parents.ForEach(parent => parent.Dispose());
+ }
+ }
+
+ ///
+ /// Opens the entry names, a link itself rather than its target, to read its security and to
+ /// set it with .
+ ///
+ internal static SafeFileHandle OpenToSetSecurity(string path) =>
+ Open(
+ path,
+ WinAPI.READ_CONTROL | WinAPI.WRITE_DAC | WinAPI.WRITE_OWNER | WinAPI.FILE_READ_ATTRIBUTES,
+ AnyShare);
+
+ ///
+ /// Sets the owner and the DACL of the directory holds, opened from
+ /// by , to those of .
+ ///
+ ///
+ /// Only the directory changes: unlike SetSecurityInfo, this doesn't propagate inheritable entries to existing
+ /// children, so the entries below keep the security they have.
+ ///
+ /// The entry is a reparse point or not a directory.
+ internal static void SetSecurity(SafeFileHandle handle, string path, string sddl)
+ {
+ if (!IsPlainDirectory(GetAttributes(handle)))
+ {
+ throw new IOException($"{path} is a reparse point or not a directory");
+ }
+
+ using SecurityDescriptor descriptor = new(sddl);
+
+ if (!WinAPI.SetKernelObjectSecurity(
+ handle,
+ WinAPI.OWNER_SECURITY_INFORMATION | WinAPI.DACL_SECURITY_INFORMATION,
+ descriptor.Pointer))
+ {
+ throw new Win32Exception(Marshal.GetLastWin32Error());
+ }
+ }
+
+ ///
+ /// 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 = GetAttributes(handle);
+ bool isReparsePoint = (attributes & WinAPI.FILE_ATTRIBUTE_REPARSE_POINT) != 0;
+ bool isDirectory = (attributes & WinAPI.FILE_ATTRIBUTE_DIRECTORY) != 0;
+ const uint information = WinAPI.OWNER_SECURITY_INFORMATION | WinAPI.DACL_SECURITY_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 (new RawSecurityDescriptor(descriptor, 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 agent's data: SYSTEM, Administrators, TrustedInstaller, or a member
+ /// of Administrators.
+ ///
+ ///
+ /// Membership is decided as the agent decides it for the owner of a pending-enrollment file
+ /// (owner_is_administrator_member in crates/agent-identity/src/windows.rs), so that the installer and the
+ /// agent accept the same owners: 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)
+ {
+ 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 the installer accepting the owners the agent accepts.
+ // 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)
+ {
+ return LocalAdministrators().Contains(owner);
+ }
+
+ return groups.Contains(Administrators);
+ }
+
+ ///
+ /// Deletes \[0]\…\[n-1] and everything
+ /// below it, without following any link.
+ ///
+ ///
+ /// Every name but the last must be a directory that is not a reparse point, or nothing is deleted.
+ /// A reparse point is deleted itself, never its target, and a directory is emptied first.
+ /// Each failure is logged and the deletion goes on with the next entry; nothing is thrown.
+ ///
+ internal static void DeleteTree(string root, string[] names, ILogger logger)
+ {
+ string target = Path.Combine([root, .. names]);
+
+ // UNTESTED: each directory handle stays open until the entries below the directory are done, so that no path
+ // below it can be redirected while the deletion runs.
+ // Tests check that a held directory can't be renamed, not when the deletion closes its handle.
+ // When changing how the deletion opens or closes a directory, keep each handle open that long.
+ List handles = new();
+
+ try
+ {
+ string path = root;
+
+ foreach (string name in names.Take(names.Length - 1))
+ {
+ path = Path.Combine(path, name);
+ SafeFileHandle parent = Hold(path);
+ handles.Add(parent);
+
+ if (!IsPlainDirectory(GetAttributes(parent)))
+ {
+ logger.Log($"kept {target}: {path} is a reparse point or not a directory");
+ return;
+ }
+ }
+
+ SafeFileHandle handle = Open(target, WinAPI.DELETE | WinAPI.FILE_READ_ATTRIBUTES, HeldShare);
+ handles.Add(handle);
+ Delete(handle, target, logger);
+ logger.Log($"deleted {target}");
+ }
+ catch (Win32Exception e) when (IsNotFound(e))
+ {
+ // Nothing to delete.
+ }
+ catch (Exception e)
+ {
+ logger.Log($"failed to delete {target}: {e}");
+ }
+ finally
+ {
+ handles.ForEach(handle => handle.Dispose());
+ }
+ }
+
+ ///
+ /// Deletes the entry holds, after the entries below it when it is a directory that is not a
+ /// reparse point.
+ ///
+ ///
+ /// The entry goes once is closed.
+ ///
+ private static void Delete(SafeFileHandle handle, string path, ILogger logger)
+ {
+ if (IsPlainDirectory(GetAttributes(handle)))
+ {
+ foreach (string entry in Directory.GetFileSystemEntries(path))
+ {
+ try
+ {
+ using SafeFileHandle child = Open(entry, WinAPI.DELETE | WinAPI.FILE_READ_ATTRIBUTES, HeldShare);
+ Delete(child, entry, logger);
+ }
+ catch (Win32Exception e) when (IsNotFound(e))
+ {
+ // Already gone.
+ }
+ catch (Exception e)
+ {
+ logger.Log($"failed to delete {entry}: {e}");
+ }
+ }
+ }
+
+ WinAPI.FILE_DISPOSITION_INFO disposition = new() { DeleteFile = true };
+
+ if (!WinAPI.SetFileInformationByHandle(
+ handle,
+ WinAPI.FileDispositionInfo,
+ ref disposition,
+ (uint)Marshal.SizeOf()))
+ {
+ throw new Win32Exception(Marshal.GetLastWin32Error());
+ }
+ }
+
+ ///
+ /// 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;
+ }
+
+ private static uint GetAttributes(SafeFileHandle handle)
+ {
+ if (!WinAPI.GetFileInformationByHandle(handle, out WinAPI.BY_HANDLE_FILE_INFORMATION information))
+ {
+ throw new Win32Exception(Marshal.GetLastWin32Error());
+ }
+
+ return information.dwFileAttributes;
+ }
+
+ private static bool IsPlainDirectory(uint attributes) =>
+ (attributes & WinAPI.FILE_ATTRIBUTE_DIRECTORY) != 0 && (attributes & WinAPI.FILE_ATTRIBUTE_REPARSE_POINT) == 0;
+
+ internal static bool IsNotFound(Win32Exception e) =>
+ e.NativeErrorCode is WinAPI.ERROR_FILE_NOT_FOUND or WinAPI.ERROR_PATH_NOT_FOUND;
+
+ ///
+ /// 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/AgentWindowsManaged/Actions/WinAPI.cs b/package/AgentWindowsManaged/Actions/WinAPI.cs
index f9244070d..21a291a31 100644
--- a/package/AgentWindowsManaged/Actions/WinAPI.cs
+++ b/package/AgentWindowsManaged/Actions/WinAPI.cs
@@ -7,30 +7,72 @@ namespace DevolutionsAgent.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 const uint FILE_DELETE_CHILD = 0x00000040;
+
internal static uint FILE_ATTRIBUTE_NORMAL = 0x00000080;
+ internal const uint FILE_ATTRIBUTE_REPARSE_POINT = 0x00000400;
+
+ 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 int FileDispositionInfo = 4;
+
+ internal const uint GENERIC_ALL = 0x10000000;
+
internal static uint GENERIC_WRITE = 0x40000000;
+ internal const uint MAX_PREFERRED_LENGTH = 0xFFFFFFFF;
+
internal static uint MOVEFILE_REPLACE_EXISTING = 0x1;
internal static uint MOVEFILE_DELAY_UNTIL_REBOOT = 0x04;
internal const uint NCRYPT_MACHINE_KEY_FLAG = 0x00000020;
+ internal const uint NERR_Success = 0;
+
internal const int NTE_NO_MORE_ITEMS = unchecked((int)0x8009002A);
+ internal const uint OPEN_EXISTING = 3;
+
+ internal const uint OWNER_SECURITY_INFORMATION = 0x00000001;
+
+ internal const uint READ_CONTROL = 0x00020000;
+
internal const uint SC_MANAGER_ALL_ACCESS = 0xF003F;
internal const uint SC_MANAGER_CONNECT = 0x0001;
@@ -61,6 +103,48 @@ internal static class WinAPI
internal static uint WAIT_TIMEOUT = 0x00000102;
+ internal const uint WRITE_DAC = 0x00040000;
+
+ internal const uint WRITE_OWNER = 0x00080000;
+
+ [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 FILE_DISPOSITION_INFO
+ {
+ [MarshalAs(UnmanagedType.U1)] internal bool DeleteFile;
+ }
+
+ [StructLayout(LayoutKind.Sequential)]
+ internal struct LUID
+ {
+ internal uint LowPart;
+
+ internal int HighPart;
+ }
+
[StructLayout(LayoutKind.Sequential)]
internal struct QUERY_SERVICE_CONFIG
{
@@ -143,6 +227,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
{
@@ -183,6 +275,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,
@@ -211,6 +350,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,
@@ -245,6 +388,12 @@ internal static extern bool DeleteFile(
[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,
@@ -252,6 +401,15 @@ 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("kernel32", SetLastError = true)]
internal static extern IntPtr GetStdHandle(int nStdHandle);
@@ -273,6 +431,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("ncrypt", CharSet = CharSet.Unicode)]
internal static extern int NCryptEnumKeys(
IntPtr hProvider,
@@ -315,9 +487,24 @@ internal static extern int SendMessage(
int wParam,
[MarshalAs(UnmanagedType.LPWStr)] string lParam);
+ [DllImport("kernel32", SetLastError = true)]
+ [return: MarshalAs(UnmanagedType.Bool)]
+ internal static extern bool SetFileInformationByHandle(
+ SafeFileHandle hFile,
+ int FileInformationClass,
+ ref FILE_DISPOSITION_INFO lpFileInformation,
+ uint dwBufferSize);
+
[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 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/AgentWindowsManaged/INTENT.md b/package/AgentWindowsManaged/INTENT.md
index 658f07439..0ddf4d606 100644
--- a/package/AgentWindowsManaged/INTENT.md
+++ b/package/AgentWindowsManaged/INTENT.md
@@ -1,3 +1,11 @@
+## Data folder
+
+- The installer gives `%ProgramData%\Devolutions\Agent` its owner and its permissions when it creates the folder, and resets both on every install, upgrade and repair, so that only SYSTEM and administrators can rename or replace what the agent stores there.
+- 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\Agent` 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 entries that account left there, and it refuses the folders the agent refuses, so that a deployment never reports success for an agent that can't use 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 a data folder, so that a link can't make it change the security of another folder.
+
## Agent Identity
- The token is passed as the `IDENTITY_ENROLLMENT_TOKEN` property, which is hidden so that it never appears in installer logs.
@@ -10,6 +18,7 @@
- Given a token, the MSI runs `identity enroll -` on a first install or a major upgrade, never on a repair, a maintenance change or an uninstall, and never without a token.
If the command fails, for example on a malformed token, the install fails, so a deployment never reports success for a machine that cannot enroll; the install never waits for the authority, whose availability the service handles.
- Only a full uninstall deletes the `identity` folder of the agent's data directory, rejection markers included, and every device key with the agent's prefix, without contacting any authority.
+ The uninstall never follows a link while it deletes that folder, so that a link can't make it delete files elsewhere.
A reinstall therefore behaves like a freshly imaged device: after a revocation, the same still-valid token enrolls a new identity and uses up a token use.
A failed cleanup never fails the uninstall, so a machine can always be uninstalled.
- A major upgrade keeps the identities and their keys.
diff --git a/package/AgentWindowsManaged/Resources/DevolutionsAgent_en-us.wxl b/package/AgentWindowsManaged/Resources/DevolutionsAgent_en-us.wxl
index 8a2fd42f1..4c27de9a5 100644
--- a/package/AgentWindowsManaged/Resources/DevolutionsAgent_en-us.wxl
+++ b/package/AgentWindowsManaged/Resources/DevolutionsAgent_en-us.wxl
@@ -21,6 +21,11 @@
Devolutions Inc.
Devolutions
+ 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 product requires Microsoft .NET Framework 4.8. Would you like to download it now?
Set IDENTITY_ENROLLMENT_TOKEN or IDENTITY_ENROLLMENT_TOKEN_FILE, not both.
The first line of the enrollment token file {0} is empty.
diff --git a/package/AgentWindowsManaged/Resources/DevolutionsAgent_fr-fr.wxl b/package/AgentWindowsManaged/Resources/DevolutionsAgent_fr-fr.wxl
index df95d22ba..1da485c57 100644
--- a/package/AgentWindowsManaged/Resources/DevolutionsAgent_fr-fr.wxl
+++ b/package/AgentWindowsManaged/Resources/DevolutionsAgent_fr-fr.wxl
@@ -15,6 +15,11 @@
Devolutions Inc.
Devolutions
+ 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 produit nécessite Microsoft .NET Framework 4.8. Souhaiteriez-vous le télécharger maintenant?
Définissez IDENTITY_ENROLLMENT_TOKEN ou IDENTITY_ENROLLMENT_TOKEN_FILE, mais pas les deux.
La première ligne du fichier de jeton d'enrôlement {0} est vide.
diff --git a/package/AgentWindowsManaged/Resources/DevolutionsAgent_sv-se.wxl b/package/AgentWindowsManaged/Resources/DevolutionsAgent_sv-se.wxl
index 2fb1d65a7..d933999bf 100644
--- a/package/AgentWindowsManaged/Resources/DevolutionsAgent_sv-se.wxl
+++ b/package/AgentWindowsManaged/Resources/DevolutionsAgent_sv-se.wxl
@@ -21,6 +21,11 @@
Devolutions Inc.
Devolutions
+ Det går inte att använda {0} eftersom det är en fil, inte en mapp. Ta bort filen och försök igen.
+ Det går inte att använda mappen {0} eftersom den är en länk. Ta bort länken och försök igen.
+ Det går inte att använda mappen {0} eftersom den ägs av {1}. Endast SYSTEM eller en administratör får äga den. Byt ägare och försök igen.
+ Det går inte att använda mappen {0} eftersom {1} kan ersätta den eller dess innehåll. Ta bort den behörigheten och försök igen.
+ Det går inte att läsa ägaren eller behörigheterna för mappen {0}. Kontrollera dem och försök igen.
Produkten kräver Microsoft .NET Framework 4.8. Vill du hämta den nu?
Ange IDENTITY_ENROLLMENT_TOKEN eller IDENTITY_ENROLLMENT_TOKEN_FILE, inte båda.
Första raden i registreringstokenfilen {0} är tom.
diff --git a/package/AgentWindowsManaged/Resources/Strings.g.cs b/package/AgentWindowsManaged/Resources/Strings.g.cs
index 599f5abbf..659e58d8a 100644
--- a/package/AgentWindowsManaged/Resources/Strings.g.cs
+++ b/package/AgentWindowsManaged/Resources/Strings.g.cs
@@ -117,6 +117,26 @@ public static string I18n(this MsiRuntime runtime, string res)
///
public const string IdentityEnrollmentTokenFileEmpty = "IdentityEnrollmentTokenFileEmpty";
///
+ /// 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";
+ ///
/// View
///
public const string ViewButton = "ViewButton";
diff --git a/package/AgentWindowsManaged/Resources/Strings_en-US.json b/package/AgentWindowsManaged/Resources/Strings_en-US.json
index 59f7829f2..963990482 100644
--- a/package/AgentWindowsManaged/Resources/Strings_en-US.json
+++ b/package/AgentWindowsManaged/Resources/Strings_en-US.json
@@ -111,6 +111,26 @@
{
"id": "IdentityEnrollmentTokenFileEmpty",
"text": "The first line of the enrollment token file {0} is empty."
+ },
+ {
+ "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."
}
],
"buttons": [
diff --git a/package/AgentWindowsManaged/Resources/Strings_fr-FR.json b/package/AgentWindowsManaged/Resources/Strings_fr-FR.json
index 567a8060f..c8b5aed76 100644
--- a/package/AgentWindowsManaged/Resources/Strings_fr-FR.json
+++ b/package/AgentWindowsManaged/Resources/Strings_fr-FR.json
@@ -87,6 +87,26 @@
{
"id": "IdentityEnrollmentTokenFileEmpty",
"text": "La première ligne du fichier de jeton d'enrôlement {0} est vide."
+ },
+ {
+ "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."
}
],
"buttons": [
diff --git a/package/AgentWindowsManaged/Resources/Strings_sv-SE.json b/package/AgentWindowsManaged/Resources/Strings_sv-SE.json
index bcb2e544f..896e7c936 100644
--- a/package/AgentWindowsManaged/Resources/Strings_sv-SE.json
+++ b/package/AgentWindowsManaged/Resources/Strings_sv-SE.json
@@ -111,6 +111,26 @@
{
"id": "IdentityEnrollmentTokenFileEmpty",
"text": "Första raden i registreringstokenfilen {0} är tom."
+ },
+ {
+ "id": "DataFolderIsLink",
+ "text": "Det går inte att använda mappen {0} eftersom den är en länk. Ta bort länken och försök igen."
+ },
+ {
+ "id": "DataFolderIsFile",
+ "text": "Det går inte att använda {0} eftersom det är en fil, inte en mapp. Ta bort filen och försök igen."
+ },
+ {
+ "id": "DataFolderOwnedByUntrustedAccount",
+ "text": "Det går inte att använda mappen {0} eftersom den ägs av {1}. Endast SYSTEM eller en administratör får äga den. Byt ägare och försök igen."
+ },
+ {
+ "id": "DataFolderReplaceableByUntrustedAccount",
+ "text": "Det går inte att använda mappen {0} eftersom {1} kan ersätta den eller dess innehåll. Ta bort den behörigheten och försök igen."
+ },
+ {
+ "id": "DataFolderSecurityUnreadable",
+ "text": "Det går inte att läsa ägaren eller behörigheterna för mappen {0}. Kontrollera dem och försök igen."
}
],
"buttons": [