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.