From 17fbb5aec6db4e4f226dd0af8f0ba1537b6b8184 Mon Sep 17 00:00:00 2001 From: Junyi Ou Date: Fri, 6 Nov 2026 15:01:31 -0500 Subject: [PATCH] fix(installer): stop granting Users read access to the data folder The installer gave BUILTIN\Users read access to %ProgramData%\Devolutions\Gateway, and everything the Gateway stores there inherited it. Only SYSTEM, administrators and the service account now get access. Install, upgrade and repair already reset the data folder's DACL and the entries below it, so existing files lose the entry on upgrade. Issue: DGW-504 Co-Authored-By: Claude Opus 5.5 --- .../DataFolderTests.cs | 26 +++++++++++++++++++ package/WindowsManaged/Resources/Includes.cs | 7 +++-- 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/package/WindowsManaged.UnitTests/DataFolderTests.cs b/package/WindowsManaged.UnitTests/DataFolderTests.cs index 8e432dc57..cf907c7d8 100644 --- a/package/WindowsManaged.UnitTests/DataFolderTests.cs +++ b/package/WindowsManaged.UnitTests/DataFolderTests.cs @@ -441,6 +441,32 @@ public void OnlySystemAdministratorsAndTheServiceAccountCanChangeTheData(string Assert.Equal(expected, writers); } + [Theory] + // oracle: mutant an entry that lets Users or another account read the data folder or what it passes on + [InlineData("the data folder")] + // oracle: mutant an entry that lets Users or another account read the users database + [InlineData("the users database")] + public void OnlySystemAdministratorsAndTheServiceAccountCanReadTheData(string template) + { + SecurityIdentifier service = new("S-1-5-80-1-2-3-4-5"); + RawSecurityDescriptor descriptor = new(Templates[template](service)); + // Reading a file's content, including through a generic right. + const int reading = 0x1 | 0x10000000 | unchecked((int)0x80000000); + + string[] readers = descriptor.DiscretionaryAcl + .Cast() + .Where(ace => ace.AceQualifier == AceQualifier.AccessAllowed && (ace.AccessMask & reading) != 0) + .Select(ace => ace.SecurityIdentifier.Value) + .Distinct() + .OrderBy(sid => sid, StringComparer.Ordinal) + .ToArray(); + string[] expected = new[] { "S-1-5-18", "S-1-5-32-544", service.Value } + .OrderBy(sid => sid, StringComparer.Ordinal) + .ToArray(); + + Assert.Equal(expected, readers); + } + // A service account that is LOCAL SERVICE gets its rights through the service account's entries. // oracle: mutant an entry of its own for LOCAL SERVICE, such as read access [Theory] diff --git a/package/WindowsManaged/Resources/Includes.cs b/package/WindowsManaged/Resources/Includes.cs index 9a5190737..70d5ae370 100644 --- a/package/WindowsManaged/Resources/Includes.cs +++ b/package/WindowsManaged/Resources/Includes.cs @@ -34,15 +34,14 @@ internal static class Includes /// Local System (SY) Full Access (FA) /// Service account Read, Execute, Write, Delete Subfolders and Files on the folder; also Delete on everything below it /// Administrators (BA) Full Access (FA) - /// Users (BU) Read, Execute - /// Only SYSTEM, administrators and the service account can change what the Gateway stores there, so no other - /// account gets more than read access. + /// Only SYSTEM, administrators and the service account can read or change what the Gateway stores there, because + /// it holds session recordings and private keys. /// LOCAL SERVICE, which earlier installers gave write access, has no entry, and the installer doesn't accept it as /// the service account. /// The service account gets no Delete on the folder itself, so that only SYSTEM and administrators can rename or /// replace it, while an inherit-only entry gives it Delete on the files and subfolders it manages below. /// - private const string PROGRAM_DATA_SDDL_TEMPLATE = "O:SYG:SYD:PAI(A;OICI;FA;;;SY)(A;OICI;0x1201ff;;;{0})(A;OICIIO;0x1301ff;;;{0})(A;OICI;FA;;;BA)(A;OICI;0x1200a9;;;BU)"; + private const string PROGRAM_DATA_SDDL_TEMPLATE = "O:SYG:SYD:PAI(A;OICI;FA;;;SY)(A;OICI;0x1201ff;;;{0})(A;OICIIO;0x1301ff;;;{0})(A;OICI;FA;;;BA)"; /// /// SDDL template for the desired %programdata%\devolutions\gateway\users.txt owner and ACL. `{0}` is the SID of the service account.