diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f6d8729c9..4d42b3ecc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1113,6 +1113,21 @@ jobs: run: dotnet test utils/dotnet/GatewayUtils.sln shell: pwsh + agent-installer-tests: + name: Agent installer tests + runs-on: windows-2022 + needs: [preflight] + + steps: + - name: Checkout ${{ github.repository }} + uses: actions/checkout@v6 + with: + ref: ${{ needs.preflight.outputs.ref }} + + - name: Tests + run: dotnet test package/AgentWindowsManaged.Tests/DevolutionsAgent.Installer.Tests.csproj + shell: pwsh + winapi-sanitizer-tests: name: Windows API sanitizer tests @@ -1357,7 +1372,7 @@ jobs: success: name: Success if: ${{ always() }} - needs: [tests, agent-tunnel-e2e, agent-policy-e2e, lints, check-dependencies, jetsocat-lipo, devolutions-gateway-powershell, devolutions-gateway, devolutions-gateway-merge, devolutions-pedm-desktop, devolutions-agent, devolutions-agent-merge, devolutions-pedm-client, dotnet-utils-tests, winapi-sanitizer-tests, winapi-miri, pedm-simulator, secure-memory-verifier] + needs: [tests, agent-tunnel-e2e, agent-policy-e2e, lints, check-dependencies, jetsocat-lipo, devolutions-gateway-powershell, devolutions-gateway, devolutions-gateway-merge, devolutions-pedm-desktop, devolutions-agent, devolutions-agent-merge, devolutions-pedm-client, dotnet-utils-tests, agent-installer-tests, winapi-sanitizer-tests, winapi-miri, pedm-simulator, secure-memory-verifier] runs-on: ubuntu-latest steps: diff --git a/package/AgentWindowsManaged.Tests/DevolutionsAgent.Installer.Tests.csproj b/package/AgentWindowsManaged.Tests/DevolutionsAgent.Installer.Tests.csproj new file mode 100644 index 000000000..e54f1240d --- /dev/null +++ b/package/AgentWindowsManaged.Tests/DevolutionsAgent.Installer.Tests.csproj @@ -0,0 +1,21 @@ + + + net48 + latest + false + DevolutionsAgent.Installer.Tests + + + + + + + runtime; build; native; contentfiles; analyzers; buildtransitive + all + + + + + + + diff --git a/package/AgentWindowsManaged.Tests/PackageBrokerInstallerTests.cs b/package/AgentWindowsManaged.Tests/PackageBrokerInstallerTests.cs new file mode 100644 index 000000000..3e7c19b01 --- /dev/null +++ b/package/AgentWindowsManaged.Tests/PackageBrokerInstallerTests.cs @@ -0,0 +1,715 @@ +using DevolutionsAgent; +using DevolutionsAgent.Actions; +using Microsoft.Deployment.WindowsInstaller; +using System; +using System.ComponentModel; +using System.Diagnostics; +using System.IO; +using System.Linq; +using System.Runtime.InteropServices; +using System.Security.AccessControl; +using System.Security.Principal; +using System.Text; +using WixSharp; +using Xunit; +using Action = WixSharp.Action; +using File = System.IO.File; +using RegistryHive = WixSharp.RegistryHive; + +namespace DevolutionsAgent.Installer.Tests; + +public sealed class PackageBrokerInstallerTests +{ + [Fact] + public void DedicatedPolicyAclAcceptsOnlySystemAndAdministrators() + { + DirectorySecurity security = + DirectorySecurity(DevolutionsAgent.Resources.Includes.PROGRAM_DATA_PACKAGE_BROKER_SDDL); + + PackageBrokerPolicyActions.VerifyPackageBrokerSecurity(security); + PackageBrokerPolicyActions.VerifySecurityDescriptor( + security, + DevolutionsAgent.Resources.Includes.PROGRAM_DATA_PACKAGE_BROKER_SDDL); + } + + [Theory] + [InlineData("O:BAG:SYD:P(A;;FA;;;SY)(A;;FA;;;BA)(A;;GR;;;LS)")] + [InlineData("O:SYG:SYD:AI(A;;FA;;;SY)(A;;FA;;;BA)")] + [InlineData("O:SYG:SYD:P(A;;FA;;;SY)(A;;FA;;;BA)(A;;GR;;;LS)")] + public void DedicatedPolicyAclRejectsAnythingOutsideStrictContract(string sddl) + { + Assert.Throws( + () => PackageBrokerPolicyActions.VerifyPackageBrokerSecurity(Security(sddl))); + } + + [Theory] + [InlineData("O:SYG:SYD:(A;;FA;;;SY)(A;;FA;;;BA)(A;;GR;;;LS)")] + [InlineData("O:BAG:SYD:(A;;FA;;;SY)(A;;FA;;;BA)(A;;GR;;;BU)")] + public void LegacySourceAllowsTrustedOwnerAndUntrustedRead(string sddl) + { + Assert.True( + PackageBrokerPolicyActions.TryVerifyLegacyPolicySourceSecurity( + Security(sddl), + out string diagnostic), + diagnostic); + } + + [Theory] + [InlineData("GW", "LS")] + [InlineData("0x2", "LS")] + [InlineData("GA", "BU")] + [InlineData("WD", "AU")] + [InlineData("WO", "LS")] + [InlineData("DC", "BU")] + public void LegacySourceRejectsUntrustedWriteOrTamperRights(string rights, string sid) + { + FileSecurity security = Security($"O:SYG:SYD:(A;;FA;;;SY)(A;;FA;;;BA)(A;;{rights};;;{sid})"); + + Assert.False( + PackageBrokerPolicyActions.TryVerifyLegacyPolicySourceSecurity( + security, + out string diagnostic)); + Assert.Contains("unsafe write or tamper rights", diagnostic); + } + + [Fact] + public void LegacySourceRejectsUntrustedOwner() + { + FileSecurity security = Security("O:BUG:SYD:(A;;FA;;;SY)(A;;FA;;;BA)"); + + Assert.False( + PackageBrokerPolicyActions.TryVerifyLegacyPolicySourceSecurity( + security, + out string diagnostic)); + Assert.Contains("untrusted owner", diagnostic); + } + + [Theory] + [InlineData("O:SYG:SY")] + [InlineData("O:SYG:SYD:P")] + public void LegacySourceRejectsNullOrEmptyDacl(string sddl) + { + Assert.False( + PackageBrokerPolicyActions.TryVerifyLegacyPolicySourceSecurity( + Security(sddl), + out string diagnostic)); + Assert.Contains("DACL", diagnostic); + } + + [Theory] + [InlineData("O:SYG:SY")] + [InlineData("O:SYG:SYD:P")] + public void TrustedAncestorRejectsNullOrEmptyDacl(string sddl) + { + InvalidOperationException error = Assert.Throws( + () => PackageBrokerPolicyActions.VerifyTrustedDirectorySecurity( + DirectorySecurity(sddl))); + Assert.Contains("DACL", error.Message); + } + + [Fact] + public void StrictConfigWithoutPolicyPathAllowsSourceCleanup() + { + Assert.True( + PackageBrokerPolicyActions.TryReadConfiguredPolicyPath( + """{"PackageBroker":{}}""", + out string configuredPath, + out string diagnostic), + diagnostic); + Assert.Null(configuredPath); + } + + [Fact] + public void StrictConfigReturnsAbsoluteJsonPolicyPath() + { + const string path = @"C:\ProgramData\Devolutions\Agent\package-broker-policy.json"; + + Assert.True( + PackageBrokerPolicyActions.TryReadConfiguredPolicyPath( + $"{{\"PackageBroker\":{{\"PolicyPath\":\"{path.Replace(@"\", @"\\")}\"}}}}", + out string configuredPath, + out string diagnostic), + diagnostic); + Assert.Equal(path, configuredPath); + } + + [Theory] + [InlineData(@"\\server\share\policy.json")] + [InlineData(@"\policy.json")] + [InlineData(@"C:policy.json")] + [InlineData(@"\\?\UNC\server\share\policy.json")] + [InlineData(@"\??\UNC\server\share\policy.json")] + [InlineData(@"\\?\GLOBALROOT\Device\HarddiskVolumeShadowCopy1\policy.json")] + [InlineData(@"\\.\C:\policy.json")] + [InlineData(@"\Device\HarddiskVolume1\policy.json")] + [InlineData(@"C:\policy.json:stream")] + [InlineData(@"C:\folder\..\policy.json")] + [InlineData(@"C:\folder\\policy.json")] + [InlineData(@"C:\policy.yaml")] + public void ConfiguredPolicyPathRejectsUnsafeOrRemoteShapesBeforeProbe(string path) + { + Assert.False( + PackageBrokerPolicyActions.TryValidateConfiguredLocalPolicyPath( + path, + out string diagnostic)); + Assert.False(string.IsNullOrWhiteSpace(diagnostic)); + } + + [Fact] + public void ConfiguredPolicyPathAcceptsLocalVolumeGuid() + { + string volumeRoot = GetSystemVolumeGuidRoot(); + string path = $"{volumeRoot}Devolutions\\PackageBroker\\policy.json"; + + Assert.True( + PackageBrokerPolicyActions.TryValidateConfiguredLocalPolicyPath( + path, + out string diagnostic), + diagnostic); + } + + [Theory] + [InlineData("""{"PackageBroker":{"PolicyPath":"C:\\policy.json",},}""")] + [InlineData("""{"PackageBroker":{/*comment*/"PolicyPath":"C:\\policy.json"}}""")] + [InlineData("""{'PackageBroker':{'PolicyPath':'C:\\policy.json'}}""")] + [InlineData("""{PackageBroker:{PolicyPath:"C:\\policy.json"}}""")] + [InlineData("""{"PackageBroker":{"PolicyPath":"C:\\first.json","PolicyPath":"C:\\second.json"}}""")] + [InlineData("""{"PackageBroker":{}} {}""")] + [InlineData("""{"PackageBroker":{"PolicyPath":42}}""")] + [InlineData("""{"PackageBroker":{"PolicyPath":"relative.json"}}""")] + public void AmbiguousConfigPreservesLegacySource(string json) + { + Assert.False( + PackageBrokerPolicyActions.TryReadConfiguredPolicyPath( + json, + out _, + out string diagnostic)); + Assert.False(string.IsNullOrWhiteSpace(diagnostic)); + } + + [Fact] + public void MigrationMarkerRoundTripsAllBindings() + { + PackageBrokerPolicyActions.MigrationRecord record = + new("source-id", "source-digest", "destination-id", "destination-digest"); + + PackageBrokerPolicyActions.MigrationRecord parsed = + PackageBrokerPolicyActions.ReadMigrationMarkerJson(record.ToJson()); + + Assert.Equal(record.SourceIdentity, parsed.SourceIdentity); + Assert.Equal(record.SourceDigest, parsed.SourceDigest); + Assert.Equal(record.DestinationIdentity, parsed.DestinationIdentity); + Assert.Equal(record.DestinationDigest, parsed.DestinationDigest); + } + + [Theory] + [InlineData("{}")] + [InlineData("""{"SourceIdentity":"id","SourceDigest":"digest"}""")] + public void MigrationMarkerRejectsIncompleteBindings(string json) + { + Assert.Throws( + () => PackageBrokerPolicyActions.ReadMigrationMarkerJson(json)); + } + + [Fact] + public void PinnedFileIdentityAndDigestDetectContentMutation() + { + using TempDirectory temp = new(); + string path = Path.Combine(temp.Path, "policy.json"); + File.WriteAllText(path, "before"); + + string identity; + string digest; + using (PackageBrokerPolicyActions.PinnedPath pinned = PinFile(path, WinAPI.GENERIC_READ)) + { + identity = PackageBrokerPolicyActions.FileIdentity(pinned.Leaf); + digest = PackageBrokerPolicyActions.FileContentDigest(pinned.Leaf); + Assert.True(PackageBrokerPolicyActions.FileIdentityAndDigestMatch(pinned.Leaf, identity, digest)); + } + + File.WriteAllText(path, "after"); + using PackageBrokerPolicyActions.PinnedPath changed = PinFile(path, WinAPI.GENERIC_READ); + Assert.Equal(identity, PackageBrokerPolicyActions.FileIdentity(changed.Leaf)); + Assert.False(PackageBrokerPolicyActions.FileIdentityAndDigestMatch(changed.Leaf, identity, digest)); + } + + [Fact] + public void MissingPinnedLeafDoesNotCreateIt() + { + using TempDirectory temp = new(); + string path = Path.Combine(temp.Path, "missing.json"); + + using PackageBrokerPolicyActions.PinnedPath pinned = + PackageBrokerPolicyActions.PinPathWithoutReparse( + path, + leafIsDirectory: false, + allowMissingLeaf: true, + leafAccess: WinAPI.FILE_READ_ATTRIBUTES); + + Assert.Null(pinned.Leaf); + Assert.False(File.Exists(path)); + } + + [Fact] + public void HandleTargetedDeletionDeletesPinnedFile() + { + using TempDirectory temp = new(); + string path = Path.Combine(temp.Path, "delete.json"); + File.WriteAllText(path, "{}"); + + using (PackageBrokerPolicyActions.PinnedPath pinned = PinFile( + path, + WinAPI.GENERIC_READ | WinAPI.DELETE | WinAPI.FILE_READ_ATTRIBUTES)) + { + string identity = PackageBrokerPolicyActions.FileIdentity(pinned.Leaf); + string digest = PackageBrokerPolicyActions.FileContentDigest(pinned.Leaf); + Assert.True( + PackageBrokerPolicyActions.DeleteFileIfIdentityAndDigestMatch( + pinned.Leaf, + identity, + digest)); + } + + Assert.False(File.Exists(path)); + } + + [Fact] + public void ChangedIdentityIsPreservedByDeleteBinding() + { + using TempDirectory temp = new(); + string path = Path.Combine(temp.Path, "source.json"); + File.WriteAllText(path, "original"); + string identity; + string digest; + using (PackageBrokerPolicyActions.PinnedPath source = PinFile(path, WinAPI.GENERIC_READ)) + { + identity = PackageBrokerPolicyActions.FileIdentity(source.Leaf); + digest = PackageBrokerPolicyActions.FileContentDigest(source.Leaf); + } + + File.Delete(path); + File.WriteAllText(path, "replacement"); + using PackageBrokerPolicyActions.PinnedPath replacement = PinFile( + path, + WinAPI.GENERIC_READ | WinAPI.DELETE | WinAPI.FILE_READ_ATTRIBUTES); + Assert.False( + PackageBrokerPolicyActions.DeleteFileIfIdentityAndDigestMatch( + replacement.Leaf, + identity, + digest)); + Assert.True(File.Exists(path)); + } + + [Fact] + public void UnavailableDeleteHandlePreservesLegacySource() + { + using TempDirectory temp = new(); + string path = Path.Combine(temp.Path, "source.json"); + File.WriteAllText(path, "{}"); + using PackageBrokerPolicyActions.PinnedPath source = PinFile(path, WinAPI.GENERIC_READ); + PackageBrokerPolicyActions.MigrationRecord record = new( + PackageBrokerPolicyActions.FileIdentity(source.Leaf), + PackageBrokerPolicyActions.FileContentDigest(source.Leaf), + "destination", + "digest"); + using FileStream blocker = new(path, FileMode.Open, FileAccess.Read, FileShare.Read); + string diagnostic = null; + + Assert.False( + PackageBrokerPolicyActions.TryDeleteLegacyPolicySource( + message => diagnostic = message, + path, + record)); + Assert.True(File.Exists(path)); + Assert.Contains("preserving both copies", diagnostic); + } + + [Fact] + public void NoReplaceMovePreservesCollisionAndCleansOnlyBoundTemporary() + { + using TempDirectory temp = new(); + string source = Path.Combine(temp.Path, "legacy.json"); + string temporary = Path.Combine(temp.Path, "migration.tmp"); + string destination = Path.Combine(temp.Path, "managed.json"); + File.WriteAllText(source, "legacy"); + File.WriteAllText(temporary, "migrated"); + File.WriteAllText(destination, "external"); + string temporaryIdentity; + string temporaryDigest; + using (PackageBrokerPolicyActions.PinnedPath pinned = PinFile(temporary, WinAPI.GENERIC_READ)) + { + temporaryIdentity = PackageBrokerPolicyActions.FileIdentity(pinned.Leaf); + temporaryDigest = PackageBrokerPolicyActions.FileContentDigest(pinned.Leaf); + } + + Assert.Equal( + PackageBrokerPolicyActions.NoReplaceMoveResult.DestinationExists, + PackageBrokerPolicyActions.MoveFileNoReplace(temporary, destination)); + Assert.Equal("legacy", File.ReadAllText(source)); + Assert.Equal("external", File.ReadAllText(destination)); + + using (PackageBrokerPolicyActions.PinnedPath pinned = + PackageBrokerPolicyActions.PinTemporaryForCleanup(temporary, allowMissing: false)) + { + Assert.True( + PackageBrokerPolicyActions.DeleteFileIfIdentityAndDigestMatch( + pinned.Leaf, + temporaryIdentity, + temporaryDigest)); + } + Assert.False(File.Exists(temporary)); + Assert.Equal("external", File.ReadAllText(destination)); + } + + [Fact] + public void TemporaryCleanupHandleSupportsDigestBindingAndPreservesMutation() + { + using TempDirectory temp = new(); + string path = Path.Combine(temp.Path, "migration.tmp"); + File.WriteAllText(path, "migrated"); + string identity; + string digest; + using (PackageBrokerPolicyActions.PinnedPath original = PinFile(path, WinAPI.GENERIC_READ)) + { + identity = PackageBrokerPolicyActions.FileIdentity(original.Leaf); + digest = PackageBrokerPolicyActions.FileContentDigest(original.Leaf); + } + + using (PackageBrokerPolicyActions.PinnedPath cleanup = + PackageBrokerPolicyActions.PinTemporaryForCleanup(path, allowMissing: false)) + { + Assert.True( + PackageBrokerPolicyActions.FileIdentityAndDigestMatch( + cleanup.Leaf, + identity, + digest)); + } + + File.WriteAllText(path, "mutated"); + using PackageBrokerPolicyActions.PinnedPath mutated = + PackageBrokerPolicyActions.PinTemporaryForCleanup(path, allowMissing: false); + Assert.False( + PackageBrokerPolicyActions.FileIdentityAndDigestMatch( + mutated.Leaf, + identity, + digest)); + Assert.True(File.Exists(path)); + } + + [Fact] + public void NoReplaceMovePublishesWhenDestinationIsMissing() + { + using TempDirectory temp = new(); + string temporary = Path.Combine(temp.Path, "migration.tmp"); + string destination = Path.Combine(temp.Path, "managed.json"); + File.WriteAllText(temporary, "migrated"); + + Assert.Equal( + PackageBrokerPolicyActions.NoReplaceMoveResult.Moved, + PackageBrokerPolicyActions.MoveFileNoReplace(temporary, destination)); + Assert.False(File.Exists(temporary)); + Assert.Equal("migrated", File.ReadAllText(destination)); + } + + [Fact] + public void NoReplaceMovePropagatesUnrelatedErrors() + { + using TempDirectory temp = new(); + string missing = Path.Combine(temp.Path, "missing.tmp"); + string destination = Path.Combine(temp.Path, "managed.json"); + + Assert.Throws( + () => PackageBrokerPolicyActions.MoveFileNoReplace(missing, destination)); + Assert.False(File.Exists(destination)); + } + + [Fact] + public void SecurityDescriptorAllocationUsesCheckedLength() + { + Assert.Equal(256, PackageBrokerPolicyActions.AllocateSecurityDescriptorBuffer(256).Length); + Assert.Throws( + () => PackageBrokerPolicyActions.AllocateSecurityDescriptorBuffer(uint.MaxValue)); + } + + [Fact] + public void LegacyYamlProbeDistinguishesFileMissingAndDirectory() + { + using TempDirectory temp = new(); + string file = Path.Combine(temp.Path, "policy.yaml"); + string missing = Path.Combine(temp.Path, "missing.yaml"); + string directory = Directory.CreateDirectory(Path.Combine(temp.Path, "directory.yaml")).FullName; + File.WriteAllText(file, "not read"); + + Assert.True(PackageBrokerPolicyActions.TryProbePinnedOrdinaryFile(file, out string fileDiagnostic)); + Assert.Null(fileDiagnostic); + Assert.False(PackageBrokerPolicyActions.TryProbePinnedOrdinaryFile(missing, out string missingDiagnostic)); + Assert.Null(missingDiagnostic); + Assert.False(PackageBrokerPolicyActions.TryProbePinnedOrdinaryFile(directory, out string directoryDiagnostic)); + Assert.Contains("could not safely inspect", directoryDiagnostic); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public void LegacyYamlProbeRejectsJunctionWithoutFollowingTarget(bool dangling) + { + using TempDirectory temp = new(); + string target = Directory.CreateDirectory(Path.Combine(temp.Path, "target")).FullName; + string sentinel = Path.Combine(target, "sentinel"); + File.WriteAllText(sentinel, "untouched"); + string link = Path.Combine(temp.Path, "policy.yaml"); + CreateDirectoryJunction(link, target); + if (dangling) + { + File.Delete(sentinel); + Directory.Delete(target); + } + + Assert.False(PackageBrokerPolicyActions.TryProbePinnedOrdinaryFile(link, out string diagnostic)); + Assert.Contains("could not safely inspect", diagnostic); + if (!dangling) + { + Assert.Equal("untouched", File.ReadAllText(sentinel)); + } + Directory.Delete(link); + } + + [Fact] + public void LegacyYamlProbeRejectsRemotePathBeforeAccess() + { + string remote = $@"\\127.0.0.1\missing-{Guid.NewGuid():N}\policy.yaml"; + + Assert.False(PackageBrokerPolicyActions.TryProbePinnedOrdinaryFile(remote, out string diagnostic)); + Assert.Contains("refusing to inspect remote", diagnostic); + } + + [Fact] + public void HardLinkedFileIsRejected() + { + using TempDirectory temp = new(); + string path = Path.Combine(temp.Path, "policy.json"); + string alias = Path.Combine(temp.Path, "alias.json"); + File.WriteAllText(path, "{}"); + Assert.True(CreateHardLink(alias, path, IntPtr.Zero)); + + Assert.Throws(() => + { + using PackageBrokerPolicyActions.PinnedPath _ = PinFile(path, WinAPI.FILE_READ_ATTRIBUTES); + }); + } + + [Fact] + public void DirectoryReparsePointIsRejectedWithoutTouchingTarget() + { + using TempDirectory temp = new(); + string target = Directory.CreateDirectory(Path.Combine(temp.Path, "target")).FullName; + string link = Path.Combine(temp.Path, "link"); + CreateDirectoryJunction(link, target); + + Assert.Throws(() => + { + using PackageBrokerPolicyActions.PinnedPath _ = + PackageBrokerPolicyActions.PinPathWithoutReparse( + link, + leafIsDirectory: true, + allowMissingLeaf: false, + leafAccess: WinAPI.FILE_READ_ATTRIBUTES); + }); + Assert.Empty(Directory.EnumerateFileSystemEntries(target)); + Directory.Delete(link); + } + + private static void CreateDirectoryJunction(string link, string target) + { + using Process process = Process.Start(new ProcessStartInfo + { + FileName = "cmd.exe", + Arguments = $"/d /c mklink /J \"{link}\" \"{target}\"", + CreateNoWindow = true, + UseShellExecute = false, + }); + process.WaitForExit(); + Assert.Equal(0, process.ExitCode); + } + + private static string GetSystemVolumeGuidRoot() + { + using Process process = Process.Start(new ProcessStartInfo + { + FileName = "mountvol.exe", + Arguments = @"C:\ /L", + CreateNoWindow = true, + RedirectStandardOutput = true, + UseShellExecute = false, + }); + string output = process.StandardOutput.ReadToEnd().Trim(); + process.WaitForExit(); + Assert.Equal(0, process.ExitCode); + Assert.StartsWith(@"\\?\Volume{", output, StringComparison.OrdinalIgnoreCase); + Assert.EndsWith(@"\", output); + return output; + } + + [Fact] + public void SecureDirectoryCreationAppliesDescriptorAtCreation() + { + using TempDirectory temp = new(); + string path = Path.Combine(temp.Path, "secured"); + string sid = WindowsIdentity.GetCurrent().User.Value; + string sddl = $"O:{sid}G:{sid}D:P(A;OICI;FA;;;{sid})"; + + PackageBrokerPolicyActions.CreateDirectoryWithSecurity(path, sddl); + + PackageBrokerPolicyActions.VerifySecurityDescriptor( + new DirectoryInfo(path).GetAccessControl(), + sddl); + } + + [Fact] + public void SecurityDescriptorComparisonIgnoresAuditRules() + { + const string expected = "O:SYG:SYD:P(A;OICI;FA;;;SY)(A;OICI;FA;;;BA)"; + DirectorySecurity actual = + DirectorySecurity($"{expected}S:(AU;SA;FA;;;WD)"); + + PackageBrokerPolicyActions.VerifySecurityDescriptor(actual, expected); + } + + [Theory] + [InlineData("O:BAG:SYD:P(A;OICI;FA;;;SY)(A;OICI;FA;;;BA)")] + [InlineData("O:SYG:SYD:P(A;OICI;FA;;;SY)")] + [InlineData("O:SYG:SYD:AI(A;OICI;FA;;;SY)(A;OICI;FA;;;BA)")] + public void SecurityDescriptorComparisonRejectsOwnerDaclOrProtectionChanges(string actualSddl) + { + const string expected = "O:SYG:SYD:P(A;OICI;FA;;;SY)(A;OICI;FA;;;BA)"; + + Assert.Throws( + () => PackageBrokerPolicyActions.VerifySecurityDescriptor( + DirectorySecurity(actualSddl), + expected)); + } + + [Fact] + public void MigrationActionsUseDeferredRollbackCommitSequence() + { + ManagedAction ensure = ActionFor(nameof(PackageBrokerPolicyActions.EnsureProgramDataPackageBrokerDirectory)); + ManagedAction rollback = ActionFor(nameof(PackageBrokerPolicyActions.RollbackLegacyPackageBrokerPolicyMigration)); + ManagedAction migrate = ActionFor(nameof(PackageBrokerPolicyActions.MigrateLegacyPackageBrokerPolicy)); + ManagedAction commit = ActionFor(nameof(PackageBrokerPolicyActions.CommitLegacyPackageBrokerPolicyMigration)); + + Assert.Equal(Execute.deferred, ensure.Execute); + Assert.Equal(Execute.rollback, rollback.Execute); + Assert.Equal(Execute.deferred, migrate.Execute); + Assert.Equal(Execute.commit, commit.Execute); + Assert.False(ensure.Impersonate); + Assert.False(rollback.Impersonate); + Assert.False(migrate.Impersonate); + Assert.False(commit.Impersonate); + Assert.Equal(Return.ignore, rollback.Return); + Assert.Equal(Return.check, ensure.Return); + Assert.Equal(Return.check, migrate.Return); + Assert.Equal(Return.ignore, commit.Return); + Assert.Equal(When.Before, rollback.When); + Assert.Equal(When.After, migrate.When); + Assert.Equal(When.After, commit.When); + Assert.Equal(migrate.Id, rollback.Step.ToString()); + Assert.Equal(ensure.Id, migrate.Step.ToString()); + Assert.Contains("createProgramDataDirectory", ensure.Step.ToString()); + Assert.Equal(migrate.Id, commit.Step.ToString()); + Assert.Equal(Condition.NOT_BeingRemoved.ToString(), ensure.Condition.ToString()); + Assert.Equal(Condition.NOT_BeingRemoved.ToString(), migrate.Condition.ToString()); + } + + [Theory] + [InlineData("marker inspection")] + [InlineData("marker deletion")] + [InlineData("source cleanup")] + public void CommitCleanupFailuresRemainSuccessful(string stage) + { + string diagnostic = null; + + ActionResult result = PackageBrokerPolicyActions.RunBestEffortCommit( + message => diagnostic = message, + () => throw new IOException(stage)); + + Assert.Equal(ActionResult.Success, result); + Assert.Contains(stage, diagnostic); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public void EventLogSourceUsesNativeMsiRegistryLifecycle(bool win64) + { + RegValue value = Program.CreateEventLogSourceRegistryValue(win64); + + Assert.Equal(RegistryHive.LocalMachine, value.Root); + Assert.Equal( + @"SYSTEM\CurrentControlSet\Services\EventLog\Application\Devolutions Agent", + value.Key); + Assert.Equal("EventMessageFile", value.Name); + Assert.Equal("[INSTALLDIR]DevolutionsAgent.exe", value.Value); + Assert.Equal(win64, value.Win64); + Assert.Equal(RegistryKeyAction.createAndRemoveOnUninstall, value.RegistryKeyAction); + Assert.False(value.ForceCreateOnInstall); + Assert.False(value.ForceDeleteOnUninstall); + Assert.Contains("Type=string", value.AttributesDefinition); + } + + private static ManagedAction ActionFor(string methodName) => + Assert.IsAssignableFrom( + AgentActions.Actions.Single( + action => action is ManagedAction managed && managed.MethodName == methodName)); + + private static PackageBrokerPolicyActions.PinnedPath PinFile(string path, uint access) => + PackageBrokerPolicyActions.PinPathWithoutReparse( + path, + leafIsDirectory: false, + allowMissingLeaf: false, + leafAccess: access); + + private static FileSecurity Security(string sddl) + { + RawSecurityDescriptor descriptor = new(sddl); + byte[] binary = new byte[descriptor.BinaryLength]; + descriptor.GetBinaryForm(binary, 0); + FileSecurity security = new(); + security.SetSecurityDescriptorBinaryForm(binary); + return security; + } + + private static DirectorySecurity DirectorySecurity(string sddl) + { + RawSecurityDescriptor descriptor = new(sddl); + byte[] binary = new byte[descriptor.BinaryLength]; + descriptor.GetBinaryForm(binary, 0); + DirectorySecurity security = new(); + security.SetSecurityDescriptorBinaryForm(binary); + return security; + } + + [DllImport("kernel32", EntryPoint = "CreateHardLinkW", CharSet = CharSet.Unicode, SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + private static extern bool CreateHardLink(string fileName, string existingFileName, IntPtr securityAttributes); + + private sealed class TempDirectory : IDisposable + { + internal TempDirectory() + { + Path = System.IO.Path.Combine( + System.IO.Path.GetTempPath(), + $"DevolutionsAgentInstallerTests-{Guid.NewGuid():N}"); + Directory.CreateDirectory(Path); + } + + internal string Path { get; } + + public void Dispose() + { + if (Directory.Exists(Path)) + { + Directory.Delete(Path, recursive: true); + } + } + } +} diff --git a/package/AgentWindowsManaged/Actions/AgentActions.cs b/package/AgentWindowsManaged/Actions/AgentActions.cs index c167ee43c..3c9f98c3d 100644 --- a/package/AgentWindowsManaged/Actions/AgentActions.cs +++ b/package/AgentWindowsManaged/Actions/AgentActions.cs @@ -121,6 +121,18 @@ internal static class AgentActions Features.PEDM_FEATURE.BeingInstall(), Sequence.InstallExecuteSequence); + private static readonly ElevatedManagedAction ensureProgramDataPackageBrokerDirectory = new( + new Id($"CA.{nameof(ensureProgramDataPackageBrokerDirectory)}"), + PackageBrokerPolicyActions.EnsureProgramDataPackageBrokerDirectory, + Return.check, + When.After, new Step(createProgramDataDirectory.Id), + Condition.NOT_BeingRemoved, + Sequence.InstallExecuteSequence) + { + Execute = Execute.deferred, + Impersonate = false, + }; + /// /// Set or reset the ACL on %ProgramData%\Devolutions\Agent /// @@ -151,6 +163,45 @@ internal static class AgentActions Impersonate = false, }; + private static readonly ElevatedManagedAction migrateLegacyPackageBrokerPolicy = new( + new Id($"CA.{nameof(migrateLegacyPackageBrokerPolicy)}"), + PackageBrokerPolicyActions.MigrateLegacyPackageBrokerPolicy, + Return.check, + When.After, new Step(ensureProgramDataPackageBrokerDirectory.Id), + Condition.NOT_BeingRemoved, + Sequence.InstallExecuteSequence) + { + Execute = Execute.deferred, + Impersonate = false, + UsesProperties = UseProperties(new[] { AgentProperties.installId }), + }; + + private static readonly ElevatedManagedAction rollbackLegacyPackageBrokerPolicyMigration = new( + new Id($"CA.{nameof(rollbackLegacyPackageBrokerPolicyMigration)}"), + PackageBrokerPolicyActions.RollbackLegacyPackageBrokerPolicyMigration, + Return.ignore, + When.Before, new Step(migrateLegacyPackageBrokerPolicy.Id), + Condition.NOT_BeingRemoved, + Sequence.InstallExecuteSequence) + { + Execute = Execute.rollback, + Impersonate = false, + UsesProperties = UseProperties(new[] { AgentProperties.installId }), + }; + + private static readonly ElevatedManagedAction commitLegacyPackageBrokerPolicyMigration = new( + new Id($"CA.{nameof(commitLegacyPackageBrokerPolicyMigration)}"), + PackageBrokerPolicyActions.CommitLegacyPackageBrokerPolicyMigration, + Return.ignore, + When.After, new Step(migrateLegacyPackageBrokerPolicy.Id), + Condition.NOT_BeingRemoved, + Sequence.InstallExecuteSequence) + { + Execute = Execute.commit, + Impersonate = false, + UsesProperties = UseProperties(new[] { AgentProperties.installId }), + }; + private static readonly ElevatedManagedAction cleanAgentConfigIfNeeded = new( new Id($"CA.{nameof(cleanAgentConfigIfNeeded)}"), CustomActions.CleanAgentConfig, @@ -499,6 +550,10 @@ private static string UseProperties(IEnumerable properties) setProgramDataDirectoryPermissions, createProgramDataPedmDirectories, setProgramDataPedmDirectoryPermissions, + ensureProgramDataPackageBrokerDirectory, + rollbackLegacyPackageBrokerPolicyMigration, + migrateLegacyPackageBrokerPolicy, + commitLegacyPackageBrokerPolicyMigration, initAgentConfigIfNeeded, registerExplorerCommand, registerExplorerCommandRollback, diff --git a/package/AgentWindowsManaged/Actions/PackageBrokerPolicyActions.cs b/package/AgentWindowsManaged/Actions/PackageBrokerPolicyActions.cs new file mode 100644 index 000000000..8c1cb948c --- /dev/null +++ b/package/AgentWindowsManaged/Actions/PackageBrokerPolicyActions.cs @@ -0,0 +1,1409 @@ +using DevolutionsAgent.Properties; +using DevolutionsAgent.Resources; +using Microsoft.Deployment.WindowsInstaller; +using Microsoft.Win32.SafeHandles; +using Newtonsoft.Json; +using Newtonsoft.Json.Linq; +using System; +using System.Collections.Generic; +using System.ComponentModel; +using System.IO; +using System.Linq; +using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; +using System.Security.AccessControl; +using System.Security.Cryptography; +using System.Security.Principal; +using System.Text; + +[assembly: InternalsVisibleTo("DevolutionsAgent.Installer.Tests")] + +namespace DevolutionsAgent.Actions; + +public static class PackageBrokerPolicyActions +{ + // Generic access bits may survive ACL conversion without expansion to specific rights. + private const FileSystemRights GenericWrite = (FileSystemRights)0x40000000; + private const FileSystemRights GenericAll = (FileSystemRights)0x10000000; + + private static string ProgramDataDirectory => Path.Combine( + Environment.GetFolderPath(Environment.SpecialFolder.CommonApplicationData), + "Devolutions", + "Agent"); + + internal static string ProgramDataPackageBrokerDirectory => Path.Combine( + Environment.GetFolderPath(Environment.SpecialFolder.CommonApplicationData), + "Devolutions", + "PackageBroker"); + + private static string DestinationPolicyPath => + Path.Combine(ProgramDataPackageBrokerDirectory, "package-broker-policy.json"); + + private static string LegacyPolicyPath => + Path.Combine(ProgramDataDirectory, "package-broker-policy.json"); + + private static uint PackageBrokerSecurityInformation => + WinAPI.OWNER_SECURITY_INFORMATION | + WinAPI.GROUP_SECURITY_INFORMATION | + WinAPI.DACL_SECURITY_INFORMATION | + WinAPI.PROTECTED_DACL_SECURITY_INFORMATION; + + [CustomAction] + public static ActionResult EnsureProgramDataPackageBrokerDirectory(Session session) + { + try + { + EnsureSecureDirectoryTree( + Environment.GetFolderPath(Environment.SpecialFolder.CommonApplicationData), + ProgramDataPackageBrokerDirectory); + session.Log($"securely created or verified {ProgramDataPackageBrokerDirectory}"); + return ActionResult.Success; + } + catch (Exception error) + { + session.Log($"failed to securely create or verify {ProgramDataPackageBrokerDirectory}: {error}"); + return ActionResult.Failure; + } + } + + [CustomAction] + public static ActionResult MigrateLegacyPackageBrokerPolicy(Session session) + { + string destination = DestinationPolicyPath; + string sourcePath = LegacyPolicyPath; + string temporary = Path.Combine( + ProgramDataPackageBrokerDirectory, + $".package-broker-policy.migration-{Guid.NewGuid():N}.tmp"); + string marker = MigrationMarkerPath(session); + bool migrationStarted = false; + MigrationRecord? migrationRecord = null; + + try + { + LogLegacyYamlMigrationRequired(session, destination); + using PinnedPath destinationPath = PinPathWithoutReparse( + destination, + leafIsDirectory: false, + allowMissingLeaf: true, + leafAccess: WinAPI.FILE_READ_ATTRIBUTES | WinAPI.READ_CONTROL); + if (destinationPath.Leaf != null) + { + VerifyPackageBrokerSecurity(SecurityFromHandle(destinationPath.Leaf, isDirectory: false)); + session.Log($"package broker policy already exists at {destination}; legacy migration skipped"); + return ActionResult.Success; + } + + using PinnedPath source = PinPathWithoutReparse( + sourcePath, + leafIsDirectory: false, + allowMissingLeaf: true, + leafAccess: WinAPI.GENERIC_READ | WinAPI.READ_CONTROL); + if (source.Leaf == null) + { + return ActionResult.Success; + } + + if (!TryVerifyLegacyPolicySourceSecurity( + SecurityFromHandle(source.Leaf, isDirectory: false), + out string sourceSecurityDiagnostic)) + { + session.Log( + $"skipping automatic package broker policy migration from {sourcePath}: " + + $"{sourceSecurityDiagnostic}. The source was left untouched and no destination was created. " + + "Restrict the source owner and write access to SYSTEM/Administrators, then validate and migrate it manually."); + return ActionResult.Success; + } + + string sourceIdentity = FileIdentity(source.Leaf); + string sourceDigest = FileContentDigest(source.Leaf); + migrationStarted = true; + using (FileStream sourceStream = OpenPinnedFileStream(source.Leaf)) + using (FileStream target = new(temporary, FileMode.CreateNew, FileAccess.ReadWrite, FileShare.None)) + { + sourceStream.CopyTo(target); + target.Flush(true); + } + + SetFileSecurity(temporary, Includes.PROGRAM_DATA_PACKAGE_BROKER_FILE_SDDL); + using (PinnedPath temporaryPath = PinPathWithoutReparse( + temporary, + leafIsDirectory: false, + allowMissingLeaf: false, + leafAccess: WinAPI.GENERIC_READ | WinAPI.FILE_READ_ATTRIBUTES | WinAPI.READ_CONTROL)) + { + VerifyPackageBrokerSecurity(SecurityFromHandle(temporaryPath.Leaf, isDirectory: false)); + migrationRecord = new MigrationRecord( + sourceIdentity, + sourceDigest, + FileIdentity(temporaryPath.Leaf), + FileContentDigest(temporaryPath.Leaf)); + } + + MigrationRecord record = migrationRecord.Value; + using PinnedPath markerPath = WriteMigrationMarker(marker, record); + if (MoveFileNoReplace(temporary, destination) == NoReplaceMoveResult.DestinationExists) + { + VerifyPackageBrokerSecurity(SecurityFromHandle(markerPath.Leaf, isDirectory: false)); + DeleteFileByHandle(markerPath.Leaf); + session.Log( + $"package broker policy appeared at {destination} during migration; " + + "the external destination and legacy source were preserved"); + return ActionResult.Success; + } + + using PinnedPath migratedPath = PinPathWithoutReparse( + destination, + leafIsDirectory: false, + allowMissingLeaf: false, + leafAccess: WinAPI.GENERIC_READ | WinAPI.FILE_READ_ATTRIBUTES | WinAPI.READ_CONTROL); + VerifyPackageBrokerSecurity(SecurityFromHandle(migratedPath.Leaf, isDirectory: false)); + if (!FileIdentityAndDigestMatch( + migratedPath.Leaf, + record.DestinationIdentity, + record.DestinationDigest)) + { + throw new InvalidOperationException("migrated package broker policy identity changed unexpectedly"); + } + + session.Log($"migrated legacy package broker policy from {sourcePath} to {destination}"); + return ActionResult.Success; + } + catch (Exception error) + { + if (!migrationStarted) + { + session.Log( + $"skipping automatic package broker policy migration because its paths could not be trusted: {error}"); + return ActionResult.Success; + } + session.Log($"failed to migrate legacy package broker policy: {error}"); + return ActionResult.Failure; + } + finally + { + TryDeleteTemporaryFile( + session, + temporary, + migrationRecord?.DestinationIdentity, + migrationRecord?.DestinationDigest); + } + } + + [CustomAction] + public static ActionResult RollbackLegacyPackageBrokerPolicyMigration(Session session) + { + string marker = MigrationMarkerPath(session); + string destination = DestinationPolicyPath; + + try + { + using PinnedPath markerPath = PinPathWithoutReparse( + marker, + leafIsDirectory: false, + allowMissingLeaf: true, + leafAccess: WinAPI.GENERIC_READ | WinAPI.DELETE | WinAPI.FILE_READ_ATTRIBUTES | WinAPI.READ_CONTROL); + if (markerPath.Leaf == null) + { + return ActionResult.Success; + } + + VerifyPackageBrokerSecurity(SecurityFromHandle(markerPath.Leaf, isDirectory: false)); + MigrationRecord record = ReadMigrationMarker(markerPath.Leaf); + + using PinnedPath destinationPath = PinPathWithoutReparse( + destination, + leafIsDirectory: false, + allowMissingLeaf: true, + leafAccess: WinAPI.GENERIC_READ | WinAPI.DELETE | WinAPI.FILE_READ_ATTRIBUTES | WinAPI.READ_CONTROL); + if (destinationPath.Leaf != null && + FileIdentityAndDigestMatch( + destinationPath.Leaf, + record.DestinationIdentity, + record.DestinationDigest)) + { + VerifyPackageBrokerSecurity(SecurityFromHandle(destinationPath.Leaf, isDirectory: false)); + DeleteFileByHandle(destinationPath.Leaf); + } + + VerifyPackageBrokerSecurity(SecurityFromHandle(markerPath.Leaf, isDirectory: false)); + DeleteFileByHandle(markerPath.Leaf); + } + catch (Exception error) + { + session.Log($"failed to roll back legacy package broker policy migration: {error}"); + } + + return ActionResult.Success; + } + + [CustomAction] + public static ActionResult CommitLegacyPackageBrokerPolicyMigration(Session session) => + RunBestEffortCommit( + session.Log, + () => CommitLegacyPackageBrokerPolicyMigrationCore(session)); + + internal static ActionResult RunBestEffortCommit(Action log, Action commit) + { + try + { + commit(); + } + catch (Exception error) + { + log($"failed to commit legacy package broker policy migration: {error}"); + } + + return ActionResult.Success; + } + + private static void CommitLegacyPackageBrokerPolicyMigrationCore(Session session) + { + string marker = MigrationMarkerPath(session); + string sourcePath = LegacyPolicyPath; + using PinnedPath markerPath = PinPathWithoutReparse( + marker, + leafIsDirectory: false, + allowMissingLeaf: true, + leafAccess: WinAPI.GENERIC_READ | WinAPI.DELETE | WinAPI.FILE_READ_ATTRIBUTES | WinAPI.READ_CONTROL); + if (markerPath.Leaf == null) + { + return; + } + + VerifyPackageBrokerSecurity(SecurityFromHandle(markerPath.Leaf, isDirectory: false)); + MigrationRecord record = ReadMigrationMarker(markerPath.Leaf); + + bool sourceChanged; + bool removeSource; + using (PinnedPath source = PinPathWithoutReparse( + sourcePath, + leafIsDirectory: false, + allowMissingLeaf: true, + leafAccess: WinAPI.GENERIC_READ | WinAPI.FILE_READ_ATTRIBUTES | WinAPI.READ_CONTROL)) + { + sourceChanged = + source.Leaf == null || + !FileIdentityAndDigestMatch(source.Leaf, record.SourceIdentity, record.SourceDigest); + removeSource = !sourceChanged; + if (removeSource && + IsLegacyPackageBrokerPolicyExplicitlyConfigured( + sourcePath, + source.Leaf, + out string configuredDiagnostic)) + { + session.Log( + $"preserving the configured legacy package broker policy during commit: {configuredDiagnostic}"); + removeSource = false; + } + if (removeSource && + !TryVerifyLegacyPolicySourceSecurity( + SecurityFromHandle(source.Leaf, isDirectory: false), + out string sourceSecurityDiagnostic)) + { + session.Log( + $"preserving the legacy package broker policy during commit: {sourceSecurityDiagnostic}"); + removeSource = false; + } + } + + VerifyPackageBrokerSecurity(SecurityFromHandle(markerPath.Leaf, isDirectory: false)); + DeleteFileByHandle(markerPath.Leaf); + + if (!removeSource) + { + if (sourceChanged) + { + session.Log( + "legacy package broker policy changed after migration; preserving the current source"); + } + return; + } + + TryDeleteLegacyPolicySource(session, sourcePath, record); + } + + internal static void EnsureSecureDirectoryTree(string programData, string target) + { + string programDataPath = Path.GetFullPath(programData); + string targetPath = Path.GetFullPath(target); + string prefix = programDataPath.TrimEnd(Path.DirectorySeparatorChar) + Path.DirectorySeparatorChar; + if (!targetPath.StartsWith(prefix, StringComparison.OrdinalIgnoreCase)) + { + throw new InvalidOperationException($"{targetPath} is outside the ProgramData directory"); + } + + string[] components = targetPath + .Substring(prefix.Length) + .Split(new[] { Path.DirectorySeparatorChar }, StringSplitOptions.RemoveEmptyEntries); + if (components.Length != 2 || + !string.Equals(components[0], "Devolutions", StringComparison.OrdinalIgnoreCase) || + !string.Equals(components[1], "PackageBroker", StringComparison.OrdinalIgnoreCase)) + { + throw new InvalidOperationException("package broker directory has an unexpected shape"); + } + + List handles = new(); + try + { + SafeFileHandle programDataHandle = OpenPathWithoutReparse( + programDataPath, + isDirectory: true, + WinAPI.FILE_READ_ATTRIBUTES | WinAPI.READ_CONTROL, + WinAPI.FILE_SHARE_READ | WinAPI.FILE_SHARE_WRITE, + allowMissing: false); + handles.Add(programDataHandle); + VerifyResolvedPath(programDataHandle, programDataPath); + VerifyTrustedDirectorySecurity(SecurityFromHandle(programDataHandle, isDirectory: true)); + + string vendorPath = Path.Combine(programDataPath, components[0]); + SafeFileHandle vendorHandle = OpenPathWithoutReparse( + vendorPath, + isDirectory: true, + WinAPI.FILE_READ_ATTRIBUTES | WinAPI.READ_CONTROL, + WinAPI.FILE_SHARE_READ | WinAPI.FILE_SHARE_WRITE, + allowMissing: true); + if (vendorHandle == null) + { + Directory.CreateDirectory(vendorPath); + vendorHandle = OpenPathWithoutReparse( + vendorPath, + isDirectory: true, + WinAPI.FILE_READ_ATTRIBUTES | WinAPI.READ_CONTROL, + WinAPI.FILE_SHARE_READ | WinAPI.FILE_SHARE_WRITE, + allowMissing: false); + } + handles.Add(vendorHandle); + VerifyResolvedPath(vendorHandle, vendorPath); + VerifyTrustedDirectorySecurity(SecurityFromHandle(vendorHandle, isDirectory: true)); + + string leafPath = Path.Combine(vendorPath, components[1]); + SafeFileHandle leafHandle = OpenPathWithoutReparse( + leafPath, + isDirectory: true, + WinAPI.FILE_READ_ATTRIBUTES | WinAPI.READ_CONTROL, + WinAPI.FILE_SHARE_READ | WinAPI.FILE_SHARE_WRITE, + allowMissing: true); + bool leafCreated = leafHandle == null; + if (leafCreated) + { + CreateDirectoryWithSecurity(leafPath, Includes.PROGRAM_DATA_PACKAGE_BROKER_SDDL); + leafHandle = OpenPathWithoutReparse( + leafPath, + isDirectory: true, + WinAPI.FILE_READ_ATTRIBUTES | WinAPI.READ_CONTROL, + WinAPI.FILE_SHARE_READ | WinAPI.FILE_SHARE_WRITE, + allowMissing: false); + } + handles.Add(leafHandle); + VerifyResolvedPath(leafHandle, leafPath); + FileSystemSecurity leafSecurity = SecurityFromHandle(leafHandle, isDirectory: true); + VerifyPackageBrokerSecurity(leafSecurity); + if (leafCreated) + { + VerifySecurityDescriptor(leafSecurity, Includes.PROGRAM_DATA_PACKAGE_BROKER_SDDL); + } + } + finally + { + foreach (SafeFileHandle handle in handles) + { + handle.Dispose(); + } + } + } + + internal static void VerifyPackageBrokerSecurity(FileSystemSecurity security) + { + VerifyDaclPresentAndNonEmpty(security, "package broker path"); + SecurityIdentifier system = new(WellKnownSidType.LocalSystemSid, null); + SecurityIdentifier administrators = new(WellKnownSidType.BuiltinAdministratorsSid, null); + SecurityIdentifier owner = (SecurityIdentifier)security.GetOwner(typeof(SecurityIdentifier)); + if (!owner.Equals(system) && !owner.Equals(administrators)) + { + throw new InvalidOperationException($"package broker path has untrusted owner {owner.Value}"); + } + if (!security.AreAccessRulesProtected) + { + throw new InvalidOperationException("package broker path DACL inheritance is not protected"); + } + + FileSystemAccessRule[] rules = security + .GetAccessRules(true, true, typeof(SecurityIdentifier)) + .Cast() + .ToArray(); + bool IsExpectedFullControl(FileSystemAccessRule rule, SecurityIdentifier sid) => + rule.IdentityReference.Equals(sid) && + rule.AccessControlType == AccessControlType.Allow && + (rule.FileSystemRights & FileSystemRights.FullControl) == FileSystemRights.FullControl; + if (rules.Length != 2 || + !rules.Any(rule => IsExpectedFullControl(rule, system)) || + !rules.Any(rule => IsExpectedFullControl(rule, administrators))) + { + throw new InvalidOperationException("package broker path DACL is not SYSTEM/Administrators-only"); + } + } + + internal static bool TryVerifyLegacyPolicySourceSecurity( + FileSystemSecurity security, + out string diagnostic) + { + try + { + VerifyLegacyPolicySourceSecurity(security); + diagnostic = null; + return true; + } + catch (InvalidOperationException error) + { + diagnostic = error.Message; + return false; + } + } + + internal static bool TryReadConfiguredPolicyPath( + string configJson, + out string configuredPath, + out string diagnostic) + { + configuredPath = null; + diagnostic = null; + try + { + if (ContainsNonStrictJsonSyntax(configJson)) + { + diagnostic = "configuration uses non-strict JSON syntax"; + return false; + } + + using (JsonTextReader syntaxReader = new(new StringReader(configJson))) + { + while (syntaxReader.Read()) + { + if (syntaxReader.TokenType == JsonToken.Comment || + syntaxReader.TokenType == JsonToken.Undefined || + ((syntaxReader.TokenType == JsonToken.String || + syntaxReader.TokenType == JsonToken.PropertyName) && + syntaxReader.QuoteChar != '"')) + { + diagnostic = "configuration uses non-strict JSON syntax"; + return false; + } + } + } + + using JsonTextReader reader = new(new StringReader(configJson)) + { + DateParseHandling = DateParseHandling.None, + SupportMultipleContent = false, + }; + JObject config = JObject.Load( + reader, + new JsonLoadSettings + { + CommentHandling = CommentHandling.Load, + DuplicatePropertyNameHandling = DuplicatePropertyNameHandling.Error, + }); + if (reader.Read()) + { + diagnostic = "configuration contains multiple JSON values"; + return false; + } + + JToken token = config["PackageBroker"]?["PolicyPath"]; + if (token == null || token.Type == JTokenType.Null) + { + return true; + } + if (token.Type != JTokenType.String || string.IsNullOrWhiteSpace(token.Value())) + { + diagnostic = "PackageBroker.PolicyPath is not a valid path string"; + return false; + } + + configuredPath = token.Value(); + if (!TryValidateConfiguredLocalPolicyPath(configuredPath, out diagnostic)) + { + return false; + } + return true; + } + catch (Exception error) when ( + error is JsonException || + error is ArgumentException) + { + diagnostic = $"configuration could not be parsed safely: {error.Message}"; + return false; + } + } + + internal static bool TryValidateConfiguredLocalPolicyPath(string path, out string diagnostic) + { + diagnostic = null; + string root; + string relative; + const string volumePrefix = @"\\?\Volume{"; + if (path.StartsWith(volumePrefix, StringComparison.OrdinalIgnoreCase)) + { + int volumeEnd = path.IndexOf(@"}\", volumePrefix.Length, StringComparison.Ordinal); + if (volumeEnd < 0 || + !Guid.TryParseExact( + path.Substring(volumePrefix.Length, volumeEnd - volumePrefix.Length), + "D", + out _)) + { + diagnostic = "PackageBroker.PolicyPath has an invalid local volume GUID"; + return false; + } + root = path.Substring(0, volumeEnd + 2); + relative = path.Substring(volumeEnd + 2); + } + else + { + if (path.Length < 3 || + !char.IsLetter(path[0]) || + path[1] != ':' || + path[2] != '\\') + { + diagnostic = "PackageBroker.PolicyPath is not a fully qualified local path"; + return false; + } + root = path.Substring(0, 3); + relative = path.Substring(3); + } + + if (string.IsNullOrEmpty(relative) || + relative.EndsWith(@"\", StringComparison.Ordinal) || + relative.Contains('/') || + relative.Contains(':') || + relative.Split('\\').Any(component => + string.IsNullOrEmpty(component) || + component == "." || + component == "..")) + { + diagnostic = "PackageBroker.PolicyPath has an unsafe local path shape"; + return false; + } + if (!string.Equals(Path.GetExtension(relative), ".json", StringComparison.OrdinalIgnoreCase)) + { + diagnostic = "PackageBroker.PolicyPath must name a JSON file"; + return false; + } + + uint driveType = WinAPI.GetDriveType(root); + if (driveType == WinAPI.DRIVE_UNKNOWN || + driveType == WinAPI.DRIVE_NO_ROOT_DIR || + driveType == WinAPI.DRIVE_REMOTE) + { + diagnostic = "PackageBroker.PolicyPath does not use an available local volume"; + return false; + } + return true; + } + + internal static bool ContainsNonStrictJsonSyntax(string json) + { + bool inString = false; + bool escaped = false; + for (int index = 0; index < json.Length; index++) + { + char current = json[index]; + if (inString) + { + if (escaped) + { + escaped = false; + } + else if (current == '\\') + { + escaped = true; + } + else if (current == '"') + { + inString = false; + } + continue; + } + + if (current == '"') + { + inString = true; + continue; + } + if (current == '/' && + index + 1 < json.Length && + (json[index + 1] == '/' || json[index + 1] == '*')) + { + return true; + } + if (current != ',') + { + continue; + } + + int next = index + 1; + while (next < json.Length && char.IsWhiteSpace(json[next])) + { + next++; + } + if (next < json.Length && (json[next] == '}' || json[next] == ']')) + { + return true; + } + } + return inString || escaped; + } + + internal static PinnedPath PinPathWithoutReparse( + string path, + bool leafIsDirectory, + bool allowMissingLeaf, + uint leafAccess) + { + string fullPath = Path.GetFullPath(path); + string root = Path.GetPathRoot(fullPath); + string parent = Path.GetDirectoryName(fullPath); + Stack ancestors = new(); + while (!string.IsNullOrEmpty(parent)) + { + ancestors.Push(parent); + if (string.Equals(parent, root, StringComparison.OrdinalIgnoreCase)) + { + break; + } + parent = Path.GetDirectoryName(parent); + } + + List handles = new(); + try + { + foreach (string ancestor in ancestors) + { + SafeFileHandle ancestorHandle = OpenPathWithoutReparse( + ancestor, + isDirectory: true, + WinAPI.FILE_READ_ATTRIBUTES, + WinAPI.FILE_SHARE_READ | WinAPI.FILE_SHARE_WRITE, + allowMissing: false); + VerifyResolvedPath(ancestorHandle, ancestor); + handles.Add(ancestorHandle); + } + + uint shareMode = (leafAccess & WinAPI.GENERIC_READ) != 0 + ? WinAPI.FILE_SHARE_READ + : WinAPI.FILE_SHARE_READ | WinAPI.FILE_SHARE_WRITE; + SafeFileHandle leaf = OpenPathWithoutReparse( + fullPath, + leafIsDirectory, + leafAccess, + shareMode, + allowMissingLeaf); + if (leaf != null) + { + VerifyResolvedPath(leaf, fullPath); + handles.Add(leaf); + } + return new PinnedPath(handles, leaf); + } + catch + { + foreach (SafeFileHandle handle in handles) + { + handle.Dispose(); + } + throw; + } + } + + internal static string FileIdentity(SafeFileHandle handle) + { + if (!WinAPI.GetFileInformationByHandle(handle, out WinAPI.ByHandleFileInformation information)) + { + throw new Win32Exception(Marshal.GetLastWin32Error(), "failed to query file identity"); + } + return string.Join( + ":", + information.VolumeSerialNumber, + information.FileIndexHigh, + information.FileIndexLow); + } + + internal static string FileContentDigest(SafeFileHandle handle) + { + using FileStream stream = OpenPinnedFileStream(handle); + using SHA256 sha256 = SHA256.Create(); + return Convert.ToBase64String(sha256.ComputeHash(stream)); + } + + internal static bool FileIdentityAndDigestMatch( + SafeFileHandle handle, + string expectedIdentity, + string expectedDigest) => + string.Equals(FileIdentity(handle), expectedIdentity, StringComparison.Ordinal) && + string.Equals(FileContentDigest(handle), expectedDigest, StringComparison.Ordinal); + + internal static void DeleteFileByHandle(SafeFileHandle handle) + { + WinAPI.FileDispositionInfo disposition = new() { DeleteFile = true }; + if (!WinAPI.SetFileInformationByHandle( + handle, + WinAPI.FileInfoByHandleClass.FileDispositionInfo, + ref disposition, + (uint)Marshal.SizeOf())) + { + throw new Win32Exception(Marshal.GetLastWin32Error(), "failed to delete the pinned file"); + } + } + + internal static bool DeleteFileIfIdentityAndDigestMatch( + SafeFileHandle handle, + string expectedIdentity, + string expectedDigest) + { + if (!FileIdentityAndDigestMatch(handle, expectedIdentity, expectedDigest)) + { + return false; + } + + DeleteFileByHandle(handle); + return true; + } + + internal static MigrationRecord ReadMigrationMarkerJson(string markerJson) + { + JObject document = JObject.Parse(markerJson); + string sourceIdentity = document.Value("SourceIdentity"); + string sourceDigest = document.Value("SourceDigest"); + string destinationIdentity = document.Value("DestinationIdentity"); + string destinationDigest = document.Value("DestinationDigest"); + if (string.IsNullOrEmpty(sourceIdentity) || + string.IsNullOrEmpty(sourceDigest) || + string.IsNullOrEmpty(destinationIdentity) || + string.IsNullOrEmpty(destinationDigest)) + { + throw new InvalidOperationException("package broker migration marker is incomplete"); + } + return new MigrationRecord(sourceIdentity, sourceDigest, destinationIdentity, destinationDigest); + } + + private static string MigrationMarkerPath(Session session) => + Path.Combine( + ProgramDataPackageBrokerDirectory, + $".legacy-policy-migration-{session.Get(AgentProperties.installId)}.marker"); + + internal static NoReplaceMoveResult MoveFileNoReplace(string source, string destination) + { + if (WinAPI.MoveFileEx(source, destination, 0)) + { + return NoReplaceMoveResult.Moved; + } + + int error = Marshal.GetLastWin32Error(); + if (error == WinAPI.ERROR_FILE_EXISTS || error == WinAPI.ERROR_ALREADY_EXISTS) + { + return NoReplaceMoveResult.DestinationExists; + } + + throw new Win32Exception(error, $"failed to move {source} to {destination} without replacement"); + } + + private static PinnedPath WriteMigrationMarker(string marker, MigrationRecord record) + { + using (FileStream markerFile = new(marker, FileMode.CreateNew, FileAccess.ReadWrite, FileShare.None)) + { + byte[] markerContent = Encoding.UTF8.GetBytes(record.ToJson()); + markerFile.Write(markerContent, 0, markerContent.Length); + markerFile.Flush(true); + } + + SetFileSecurity(marker, Includes.PROGRAM_DATA_PACKAGE_BROKER_FILE_SDDL); + PinnedPath markerPath = PinPathWithoutReparse( + marker, + leafIsDirectory: false, + allowMissingLeaf: false, + leafAccess: WinAPI.GENERIC_READ | WinAPI.DELETE | WinAPI.FILE_READ_ATTRIBUTES | WinAPI.READ_CONTROL); + try + { + VerifyPackageBrokerSecurity(SecurityFromHandle(markerPath.Leaf, isDirectory: false)); + _ = ReadMigrationMarker(markerPath.Leaf); + return markerPath; + } + catch + { + markerPath.Dispose(); + throw; + } + } + + private static MigrationRecord ReadMigrationMarker(SafeFileHandle marker) + { + using FileStream stream = OpenPinnedFileStream(marker); + using StreamReader reader = new( + stream, + new UTF8Encoding(encoderShouldEmitUTF8Identifier: false, throwOnInvalidBytes: true), + detectEncodingFromByteOrderMarks: true); + return ReadMigrationMarkerJson(reader.ReadToEnd()); + } + + private static bool IsLegacyPackageBrokerPolicyExplicitlyConfigured( + string sourcePath, + SafeFileHandle source, + out string diagnostic) + { + string configPath = Path.Combine(ProgramDataDirectory, "agent.json"); + try + { + using PinnedPath config = PinPathWithoutReparse( + configPath, + leafIsDirectory: false, + allowMissingLeaf: true, + leafAccess: WinAPI.GENERIC_READ | WinAPI.FILE_READ_ATTRIBUTES); + if (config.Leaf == null) + { + diagnostic = null; + return false; + } + + string configJson; + using (FileStream stream = OpenPinnedFileStream(config.Leaf)) + using (StreamReader reader = new( + stream, + new UTF8Encoding(encoderShouldEmitUTF8Identifier: false, throwOnInvalidBytes: true), + detectEncodingFromByteOrderMarks: false)) + { + configJson = reader.ReadToEnd(); + } + if (!TryReadConfiguredPolicyPath(configJson, out string configuredPath, out diagnostic)) + { + return true; + } + if (configuredPath == null) + { + return false; + } + + using PinnedPath configured = PinPathWithoutReparse( + configuredPath, + leafIsDirectory: false, + allowMissingLeaf: true, + leafAccess: WinAPI.FILE_READ_ATTRIBUTES); + if (configured.Leaf == null) + { + diagnostic = $"PackageBroker.PolicyPath in {configPath} could not be resolved safely"; + return true; + } + if (!string.Equals(FileIdentity(configured.Leaf), FileIdentity(source), StringComparison.Ordinal)) + { + diagnostic = null; + return false; + } + + diagnostic = $"PackageBroker.PolicyPath in {configPath} still points to {sourcePath}"; + return true; + } + catch (Exception error) + { + diagnostic = $"could not safely determine PackageBroker.PolicyPath from {configPath}: {error.Message}"; + return true; + } + } + + private static void VerifyLegacyPolicySourceSecurity(FileSystemSecurity security) + { + const FileSystemRights unsafeRights = + FileSystemRights.WriteData | + FileSystemRights.AppendData | + FileSystemRights.WriteAttributes | + FileSystemRights.WriteExtendedAttributes | + FileSystemRights.Delete | + FileSystemRights.DeleteSubdirectoriesAndFiles | + FileSystemRights.ChangePermissions | + FileSystemRights.TakeOwnership | + GenericWrite | + GenericAll; + VerifyTrustedOwnerAndNoUnsafeGrants( + security, + unsafeRights, + "legacy policy", + "unsafe write or tamper"); + } + + internal static void VerifyTrustedDirectorySecurity(FileSystemSecurity security) + { + const FileSystemRights tamperRights = + FileSystemRights.Delete | + FileSystemRights.DeleteSubdirectoriesAndFiles | + FileSystemRights.ChangePermissions | + FileSystemRights.TakeOwnership | + GenericAll; + VerifyTrustedOwnerAndNoUnsafeGrants( + security, + tamperRights, + "directory", + "path-tampering"); + } + + private static void VerifyTrustedOwnerAndNoUnsafeGrants( + FileSystemSecurity security, + FileSystemRights unsafeRights, + string subject, + string accessDescription) + { + VerifyDaclPresentAndNonEmpty(security, subject); + SecurityIdentifier system = new(WellKnownSidType.LocalSystemSid, null); + SecurityIdentifier administrators = new(WellKnownSidType.BuiltinAdministratorsSid, null); + SecurityIdentifier trustedInstaller = + (SecurityIdentifier)new NTAccount(@"NT SERVICE\TrustedInstaller").Translate(typeof(SecurityIdentifier)); + SecurityIdentifier owner = (SecurityIdentifier)security.GetOwner(typeof(SecurityIdentifier)); + if (!owner.Equals(system) && !owner.Equals(administrators) && !owner.Equals(trustedInstaller)) + { + throw new InvalidOperationException($"{subject} has untrusted owner {owner.Value}"); + } + + foreach (FileSystemAccessRule rule in security.GetAccessRules( + includeExplicit: true, + includeInherited: true, + targetType: typeof(SecurityIdentifier))) + { + if (rule.AccessControlType != AccessControlType.Allow || + (rule.PropagationFlags & PropagationFlags.InheritOnly) != 0 || + (rule.FileSystemRights & unsafeRights) == 0) + { + continue; + } + + SecurityIdentifier identity = (SecurityIdentifier)rule.IdentityReference; + if (!identity.Equals(system) && + !identity.Equals(administrators) && + !identity.Equals(trustedInstaller)) + { + throw new InvalidOperationException( + $"{subject} grants {accessDescription} rights to {identity.Value}"); + } + } + } + + private static void VerifyDaclPresentAndNonEmpty(FileSystemSecurity security, string subject) + { + RawSecurityDescriptor descriptor = + new(security.GetSecurityDescriptorBinaryForm(), 0); + if (!descriptor.ControlFlags.HasFlag(ControlFlags.DiscretionaryAclPresent) || + descriptor.DiscretionaryAcl == null) + { + throw new InvalidOperationException($"{subject} has a NULL DACL granting full control to everyone"); + } + if (descriptor.DiscretionaryAcl.Count == 0) + { + throw new InvalidOperationException($"{subject} has an empty DACL with no trusted access entries"); + } + } + + internal static void CreateDirectoryWithSecurity(string path, string sddl) + { + const uint sdRevision = 1; + if (!WinAPI.ConvertStringSecurityDescriptorToSecurityDescriptorW( + sddl, + sdRevision, + out IntPtr securityDescriptor, + out _)) + { + throw new Win32Exception( + Marshal.GetLastWin32Error(), + $"failed to create security descriptor for {path}"); + } + + try + { + WinAPI.SECURITY_ATTRIBUTES attributes = new() + { + nLength = (uint)Marshal.SizeOf(), + lpSecurityDescriptor = securityDescriptor, + bInheritHandle = false, + }; + if (!WinAPI.CreateDirectory(path, ref attributes)) + { + int error = Marshal.GetLastWin32Error(); + if (error != WinAPI.ERROR_ALREADY_EXISTS) + { + throw new Win32Exception(error, $"failed to securely create {path}"); + } + } + } + finally + { + WinAPI.LocalFree(securityDescriptor); + } + } + + private static void SetFileSecurity(string path, string sddl) + { + const uint sdRevision = 1; + if (!WinAPI.ConvertStringSecurityDescriptorToSecurityDescriptorW( + sddl, + sdRevision, + out IntPtr securityDescriptor, + out _)) + { + throw new Win32Exception(Marshal.GetLastWin32Error(), $"failed to create security descriptor for {path}"); + } + + try + { + if (!WinAPI.SetFileSecurityW(path, PackageBrokerSecurityInformation, securityDescriptor)) + { + throw new Win32Exception(Marshal.GetLastWin32Error(), $"failed to secure {path}"); + } + } + finally + { + WinAPI.LocalFree(securityDescriptor); + } + } + + internal static void VerifySecurityDescriptor(FileSystemSecurity actual, string expectedSddl) + { + if (!actual.AreAccessRulesProtected) + { + throw new InvalidOperationException("new directory DACL inheritance is not protected"); + } + + const AccessControlSections sections = + AccessControlSections.Owner | + AccessControlSections.Group | + AccessControlSections.Access; + RawSecurityDescriptor expected = new(expectedSddl); + string expectedCanonical = expected.GetSddlForm(sections); + string actualCanonical = actual.GetSecurityDescriptorSddlForm(sections); + if (!string.Equals(actualCanonical, expectedCanonical, StringComparison.Ordinal)) + { + throw new InvalidOperationException("new directory security does not match its creation descriptor"); + } + } + + private static SafeFileHandle OpenPathWithoutReparse( + string path, + bool isDirectory, + uint desiredAccess, + uint shareMode, + bool allowMissing) + { + uint flags = WinAPI.FILE_FLAG_OPEN_REPARSE_POINT; + if (isDirectory) + { + flags |= WinAPI.FILE_FLAG_BACKUP_SEMANTICS; + } + + SafeFileHandle handle = WinAPI.CreateFile( + path, + desiredAccess, + shareMode, + IntPtr.Zero, + WinAPI.OPEN_EXISTING, + flags, + IntPtr.Zero); + if (handle.IsInvalid) + { + int error = Marshal.GetLastWin32Error(); + handle.Dispose(); + if (allowMissing && + (error == WinAPI.ERROR_FILE_NOT_FOUND || error == WinAPI.ERROR_PATH_NOT_FOUND)) + { + return null; + } + throw new Win32Exception(error, $"failed to open {path} without following reparse points"); + } + + if (!WinAPI.GetFileInformationByHandle(handle, out WinAPI.ByHandleFileInformation information)) + { + int error = Marshal.GetLastWin32Error(); + handle.Dispose(); + throw new Win32Exception(error, $"failed to inspect {path}"); + } + if ((information.FileAttributes & WinAPI.FILE_ATTRIBUTE_REPARSE_POINT) != 0) + { + handle.Dispose(); + throw new InvalidOperationException($"{path} is a reparse point"); + } + + bool actualDirectory = (information.FileAttributes & WinAPI.FILE_ATTRIBUTE_DIRECTORY) != 0; + if (actualDirectory != isDirectory) + { + handle.Dispose(); + throw new InvalidOperationException($"{path} has an unexpected filesystem type"); + } + if (!isDirectory && information.NumberOfLinks != 1) + { + handle.Dispose(); + throw new InvalidOperationException($"{path} has multiple hard links"); + } + return handle; + } + + private static void VerifyResolvedPath(SafeFileHandle handle, string expectedPath) + { + StringBuilder buffer = new(512); + uint length = WinAPI.GetFinalPathNameByHandle(handle.DangerousGetHandle(), buffer, (uint)buffer.Capacity, 0); + if (length == 0) + { + throw new Win32Exception(Marshal.GetLastWin32Error(), $"failed to resolve {expectedPath}"); + } + if (length >= buffer.Capacity) + { + buffer.EnsureCapacity((int)length + 1); + length = WinAPI.GetFinalPathNameByHandle( + handle.DangerousGetHandle(), + buffer, + (uint)buffer.Capacity, + 0); + if (length == 0 || length >= buffer.Capacity) + { + throw new Win32Exception(Marshal.GetLastWin32Error(), $"failed to resolve {expectedPath}"); + } + } + + string resolved = NormalizeExtendedPath(buffer.ToString()); + string expected = Path.GetFullPath(expectedPath).TrimEnd(Path.DirectorySeparatorChar); + if (!string.Equals(resolved.TrimEnd(Path.DirectorySeparatorChar), expected, StringComparison.OrdinalIgnoreCase)) + { + throw new InvalidOperationException($"{expectedPath} resolved to unexpected path {resolved}"); + } + } + + private static string NormalizeExtendedPath(string path) + { + const string uncPrefix = @"\\?\UNC\"; + const string localPrefix = @"\\?\"; + if (path.StartsWith(uncPrefix, StringComparison.OrdinalIgnoreCase)) + { + return @"\\" + path.Substring(uncPrefix.Length); + } + return path.StartsWith(localPrefix, StringComparison.OrdinalIgnoreCase) + ? path.Substring(localPrefix.Length) + : path; + } + + private static FileSystemSecurity SecurityFromHandle(SafeFileHandle handle, bool isDirectory) + { + uint information = + WinAPI.OWNER_SECURITY_INFORMATION | + WinAPI.GROUP_SECURITY_INFORMATION | + WinAPI.DACL_SECURITY_INFORMATION; + WinAPI.GetKernelObjectSecurity(handle, information, null, 0, out uint requiredSize); + int error = Marshal.GetLastWin32Error(); + if (requiredSize == 0 || error != WinAPI.ERROR_INSUFFICIENT_BUFFER) + { + throw new Win32Exception(error, "failed to query pinned path security descriptor size"); + } + + byte[] descriptor = AllocateSecurityDescriptorBuffer(requiredSize); + if (!WinAPI.GetKernelObjectSecurity( + handle, + information, + descriptor, + (uint)descriptor.Length, + out _)) + { + throw new Win32Exception( + Marshal.GetLastWin32Error(), + "failed to query pinned path security descriptor"); + } + + FileSystemSecurity security = isDirectory ? new DirectorySecurity() : new FileSecurity(); + security.SetSecurityDescriptorBinaryForm(descriptor); + return security; + } + + private static FileStream OpenPinnedFileStream(SafeFileHandle handle) + { + SafeFileHandle borrowedHandle = new(handle.DangerousGetHandle(), ownsHandle: false); + FileStream stream = new(borrowedHandle, FileAccess.Read); + stream.Position = 0; + return stream; + } + + private static void LogLegacyYamlMigrationRequired(Session session, string destination) + { + foreach (string extension in new[] { "yaml", "yml" }) + { + string legacyYaml = Path.Combine(ProgramDataDirectory, $"package-broker-policy.{extension}"); + if (TryProbePinnedOrdinaryFile(legacyYaml, out string diagnostic)) + { + session.Log( + $"legacy YAML package broker policy remains untouched at {legacyYaml}; " + + $"validate and migrate it manually to strict JSON at {destination}"); + } + else if (diagnostic != null) + { + session.Log(diagnostic); + } + } + } + + internal static byte[] AllocateSecurityDescriptorBuffer(uint requiredSize) => + new byte[checked((int)requiredSize)]; + + internal static bool TryProbePinnedOrdinaryFile(string path, out string diagnostic) + { + diagnostic = null; + if (path.StartsWith(@"\\", StringComparison.Ordinal)) + { + diagnostic = $"refusing to inspect remote legacy policy path {path}"; + return false; + } + + try + { + using PinnedPath pinned = PinPathWithoutReparse( + path, + leafIsDirectory: false, + allowMissingLeaf: true, + leafAccess: WinAPI.FILE_READ_ATTRIBUTES); + return pinned.Leaf != null; + } + catch (Exception error) + { + diagnostic = $"could not safely inspect legacy policy path {path}: {error.Message}"; + return false; + } + } + + private static void TryDeleteTemporaryFile( + Session session, + string path, + string expectedIdentity, + string expectedDigest) + { + try + { + using PinnedPath temporary = PinTemporaryForCleanup(path, allowMissing: true); + if (temporary.Leaf == null) + { + return; + } + + VerifyPackageBrokerSecurity(SecurityFromHandle(temporary.Leaf, isDirectory: false)); + if (expectedIdentity != null && + !FileIdentityAndDigestMatch(temporary.Leaf, expectedIdentity, expectedDigest)) + { + session.Log( + $"package broker policy migration temporary path {path} was replaced; preserving the current file"); + return; + } + DeleteFileByHandle(temporary.Leaf); + } + catch (Exception error) + { + session.Log($"failed to remove package broker policy migration temporary file {path}: {error}"); + } + } + + internal static PinnedPath PinTemporaryForCleanup(string path, bool allowMissing) => + PinPathWithoutReparse( + path, + leafIsDirectory: false, + allowMissingLeaf: allowMissing, + leafAccess: WinAPI.GENERIC_READ | WinAPI.DELETE | WinAPI.FILE_READ_ATTRIBUTES | WinAPI.READ_CONTROL); + + internal static bool TryDeleteLegacyPolicySource( + Action log, + string sourcePath, + MigrationRecord record) + { + try + { + using PinnedPath source = PinPathWithoutReparse( + sourcePath, + leafIsDirectory: false, + allowMissingLeaf: true, + leafAccess: WinAPI.GENERIC_READ | WinAPI.DELETE | WinAPI.FILE_READ_ATTRIBUTES | WinAPI.READ_CONTROL); + if (source.Leaf == null) + { + log("legacy package broker policy disappeared before cleanup; preserving the migrated copy"); + return false; + } + if (!FileIdentityAndDigestMatch(source.Leaf, record.SourceIdentity, record.SourceDigest)) + { + log("legacy package broker policy changed before cleanup; preserving both copies"); + return false; + } + if (!TryVerifyLegacyPolicySourceSecurity( + SecurityFromHandle(source.Leaf, isDirectory: false), + out string sourceSecurityDiagnostic)) + { + log($"legacy package broker policy became unsafe before cleanup: {sourceSecurityDiagnostic}"); + return false; + } + + return DeleteFileIfIdentityAndDigestMatch( + source.Leaf, + record.SourceIdentity, + record.SourceDigest); + } + catch (Exception error) + { + log($"failed to remove the migrated legacy package broker policy; preserving both copies: {error}"); + return false; + } + } + + private static bool TryDeleteLegacyPolicySource( + Session session, + string sourcePath, + MigrationRecord record) => + TryDeleteLegacyPolicySource(session.Log, sourcePath, record); + + internal sealed class PinnedPath : IDisposable + { + private readonly IReadOnlyList handles; + + internal PinnedPath(IReadOnlyList handles, SafeFileHandle leaf) + { + this.handles = handles; + Leaf = leaf; + } + + internal SafeFileHandle Leaf { get; } + + public void Dispose() + { + foreach (SafeFileHandle handle in handles) + { + handle.Dispose(); + } + } + } + + internal readonly struct MigrationRecord + { + internal MigrationRecord( + string sourceIdentity, + string sourceDigest, + string destinationIdentity, + string destinationDigest) + { + SourceIdentity = sourceIdentity; + SourceDigest = sourceDigest; + DestinationIdentity = destinationIdentity; + DestinationDigest = destinationDigest; + } + + internal string SourceIdentity { get; } + internal string SourceDigest { get; } + internal string DestinationIdentity { get; } + internal string DestinationDigest { get; } + + internal string ToJson() => + new JObject + { + ["SourceIdentity"] = SourceIdentity, + ["SourceDigest"] = SourceDigest, + ["DestinationIdentity"] = DestinationIdentity, + ["DestinationDigest"] = DestinationDigest, + }.ToString(Formatting.None, Array.Empty()); + } + + internal enum NoReplaceMoveResult + { + Moved, + DestinationExists, + } +} diff --git a/package/AgentWindowsManaged/Actions/WinAPI.cs b/package/AgentWindowsManaged/Actions/WinAPI.cs index fe25d1ed5..8268e0526 100644 --- a/package/AgentWindowsManaged/Actions/WinAPI.cs +++ b/package/AgentWindowsManaged/Actions/WinAPI.cs @@ -8,25 +8,72 @@ namespace DevolutionsAgent.Actions; internal static class WinAPI { internal static uint CREATE_ALWAYS = 2; + internal const int ERROR_ALREADY_EXISTS = 183; + internal const int ERROR_FILE_EXISTS = 80; + internal const int ERROR_FILE_NOT_FOUND = 2; + internal const int ERROR_INSUFFICIENT_BUFFER = 122; + internal const int ERROR_PATH_NOT_FOUND = 3; internal static uint CREATE_NO_WINDOW = 0x08000000; internal const uint DACL_SECURITY_INFORMATION = 0x00000004; + internal const uint DELETE = 0x00010000; + internal const uint GROUP_SECURITY_INFORMATION = 0x00000002; + internal const uint OWNER_SECURITY_INFORMATION = 0x00000001; + internal const uint PROTECTED_DACL_SECURITY_INFORMATION = 0x80000000; internal const int EM_SETCUEBANNER = 0x1501; internal static uint FILE_ATTRIBUTE_NORMAL = 0x00000080; + internal const uint FILE_ATTRIBUTE_DIRECTORY = 0x00000010; + 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 static uint FILE_SHARE_READ = 0x00000001; internal static uint FILE_SHARE_WRITE = 0x00000002; + internal const uint GENERIC_READ = 0x80000000; internal static uint GENERIC_WRITE = 0x40000000; + internal const uint DRIVE_NO_ROOT_DIR = 1; + internal const uint DRIVE_REMOTE = 4; + internal const uint DRIVE_UNKNOWN = 0; + internal const uint OPEN_EXISTING = 3; + internal const uint READ_CONTROL = 0x00020000; internal static uint MOVEFILE_REPLACE_EXISTING = 0x1; internal static uint MOVEFILE_DELAY_UNTIL_REBOOT = 0x04; + [StructLayout(LayoutKind.Sequential)] + internal struct ByHandleFileInformation + { + internal uint FileAttributes; + internal System.Runtime.InteropServices.ComTypes.FILETIME CreationTime; + internal System.Runtime.InteropServices.ComTypes.FILETIME LastAccessTime; + internal System.Runtime.InteropServices.ComTypes.FILETIME LastWriteTime; + internal uint VolumeSerialNumber; + internal uint FileSizeHigh; + internal uint FileSizeLow; + internal uint NumberOfLinks; + internal uint FileIndexHigh; + internal uint FileIndexLow; + } + + internal enum FileInfoByHandleClass + { + FileDispositionInfo = 4, + } + + [StructLayout(LayoutKind.Sequential)] + internal struct FileDispositionInfo + { + [MarshalAs(UnmanagedType.U1)] + internal bool DeleteFile; + } + internal const uint SC_MANAGER_ALL_ACCESS = 0xF003F; internal const uint SC_MANAGER_CONNECT = 0x0001; @@ -218,6 +265,21 @@ internal static extern SafeFileHandle CreateFile( IntPtr hTemplateFile ); + [DllImport("advapi32", SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + internal static extern bool GetKernelObjectSecurity( + SafeFileHandle handle, + uint requestedInformation, + [Out] byte[] securityDescriptor, + uint length, + out uint lengthNeeded); + + [DllImport("kernel32", EntryPoint = "CreateDirectoryW", CharSet = CharSet.Unicode, SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + internal static extern bool CreateDirectory( + [MarshalAs(UnmanagedType.LPWStr)] string lpPathName, + ref SECURITY_ATTRIBUTES lpSecurityAttributes); + [DllImport("kernel32", EntryPoint = "CreateProcessW", CharSet = CharSet.Unicode, SetLastError = true)] internal static extern bool CreateProcess( [MarshalAs(UnmanagedType.LPWStr)] string lpApplicationName, @@ -241,6 +303,23 @@ 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 ByHandleFileInformation lpFileInformation); + + [DllImport("kernel32", EntryPoint = "GetDriveTypeW", CharSet = CharSet.Unicode)] + internal static extern uint GetDriveType([MarshalAs(UnmanagedType.LPWStr)] string rootPathName); + + [DllImport("kernel32", SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + internal static extern bool SetFileInformationByHandle( + SafeFileHandle hFile, + FileInfoByHandleClass fileInformationClass, + ref FileDispositionInfo fileInformation, + uint bufferSize); + [DllImport("Kernel32", EntryPoint = "GetFinalPathNameByHandleW", CharSet = CharSet.Auto, SetLastError = true)] internal static extern uint GetFinalPathNameByHandle( IntPtr hFile, diff --git a/package/AgentWindowsManaged/Program.cs b/package/AgentWindowsManaged/Program.cs index d2a246305..09a32b242 100644 --- a/package/AgentWindowsManaged/Program.cs +++ b/package/AgentWindowsManaged/Program.cs @@ -348,7 +348,8 @@ static void Main() Win64 = project.Platform == Platform.x64, RegistryKeyAction = RegistryKeyAction.create, Feature = Features.PSU_FEATURE, - } + }, + CreateEventLogSourceRegistryValue(project.Platform == Platform.x64), }; List projectProperties = AgentProperties.Properties.Select(x => x.ToWixSharpProperty()).ToList(); @@ -422,6 +423,18 @@ static void Main() } } + internal static RegValue CreateEventLogSourceRegistryValue(bool win64) => + new( + RegistryHive.LocalMachine, + $"SYSTEM\\CurrentControlSet\\Services\\EventLog\\Application\\{Includes.PRODUCT_NAME}", + "EventMessageFile", + $"[{AgentProperties.InstallDir}]{Includes.EXECUTABLE_NAME}") + { + AttributesDefinition = "Type=string", + Win64 = win64, + RegistryKeyAction = RegistryKeyAction.createAndRemoveOnUninstall, + }; + private static void Project_UnhandledException(ExceptionEventArgs e) { string errorMessage = diff --git a/package/AgentWindowsManaged/Resources/Includes.cs b/package/AgentWindowsManaged/Resources/Includes.cs index 5ee9e12ae..667a1747c 100644 --- a/package/AgentWindowsManaged/Resources/Includes.cs +++ b/package/AgentWindowsManaged/Resources/Includes.cs @@ -60,5 +60,18 @@ internal static class Includes /// NT AUTHORITY\SYSTEM Allow FullControl /// internal static readonly string PROGRAM_DATA_PEDM_SDDL = "O:SYG:SYD:(A;OICI;FA;;;SY)"; + + /// + /// Protected ACL for the dedicated package-broker policy directory. + /// + /// + /// This directory must not inherit the LOCAL SERVICE and Users grants required by + /// unrelated Agent features under %ProgramData%\Devolutions\Agent. + /// + internal static readonly string PROGRAM_DATA_PACKAGE_BROKER_SDDL = + "O:SYG:SYD:P(A;OICI;FA;;;SY)(A;OICI;FA;;;BA)"; + + internal static readonly string PROGRAM_DATA_PACKAGE_BROKER_FILE_SDDL = + "O:SYG:SYD:P(A;;FA;;;SY)(A;;FA;;;BA)"; } }