From 0251c29abc7006cb3e8b74b133fea518ca66bb46 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Wed, 9 Sep 2026 01:22:26 -0400 Subject: [PATCH 1/5] feat(agent,agent-installer): add policy consent helper Install one protected NativeAOT helper that authenticates retained UniGetUI and Agent process identities before forwarding bounded policy replacement requests. Preserve protocol 2.0 conflict and uncertainty semantics while integrating transactional discovery, signing, and packaging. Issue: #1963 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .github/workflows/ci.yml | 30 + .github/workflows/package.yml | 7 +- ci/package-agent-windows.ps1 | 10 +- crates/now-package-broker/src/auth.rs | 83 +++ crates/now-package-broker/src/server/mod.rs | 7 +- ...DevolutionsAgentPolicyConsent.Tests.csproj | 22 + .../AgentPolicyConsent.Tests/ProtocolTests.cs | 445 ++++++++++++ package/AgentPolicyConsent/BrokerClient.cs | 250 +++++++ .../DevolutionsAgentPolicyConsent.csproj | 23 + package/AgentPolicyConsent/PeerTrust.cs | 682 ++++++++++++++++++ .../PolicyConsentContract.cs | 13 + package/AgentPolicyConsent/Program.cs | 173 +++++ package/AgentPolicyConsent/Protocol.cs | 240 ++++++ package/AgentPolicyConsent/app.manifest | 11 + .../PackageBrokerInstallerTests.cs | 47 ++ .../DevolutionsAgent.csproj | 1 + package/AgentWindowsManaged/Program.cs | 59 ++ .../AgentWindowsManaged/Resources/Includes.cs | 15 + 18 files changed, 2114 insertions(+), 4 deletions(-) create mode 100644 package/AgentPolicyConsent.Tests/DevolutionsAgentPolicyConsent.Tests.csproj create mode 100644 package/AgentPolicyConsent.Tests/ProtocolTests.cs create mode 100644 package/AgentPolicyConsent/BrokerClient.cs create mode 100644 package/AgentPolicyConsent/DevolutionsAgentPolicyConsent.csproj create mode 100644 package/AgentPolicyConsent/PeerTrust.cs create mode 100644 package/AgentPolicyConsent/PolicyConsentContract.cs create mode 100644 package/AgentPolicyConsent/Program.cs create mode 100644 package/AgentPolicyConsent/Protocol.cs create mode 100644 package/AgentPolicyConsent/app.manifest diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 40e2c2121..8f83cd4b1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -878,6 +878,9 @@ jobs: $DAgentSessionExecutable = Join-Path $TargetOutputPath "DevolutionsSession.exe" echo "dagent-session-executable=$DAgentSessionExecutable" >> $Env:GITHUB_OUTPUT + $DAgentPolicyConsentHelper = Join-Path $TargetOutputPath "DevolutionsAgentPolicyConsent.exe" + echo "dagent-policy-consent-helper=$DAgentPolicyConsentHelper" >> $Env:GITHUB_OUTPUT + $DAgentUpdaterExecutable = Join-Path $TargetOutputPath "DevolutionsAgentUpdater.exe" echo "dagent-updater-executable=$DAgentUpdaterExecutable" >> $Env:GITHUB_OUTPUT } @@ -1043,6 +1046,28 @@ jobs: DAGENT_EXECUTABLE: ${{ steps.load-variables.outputs.dagent-executable }} TARGET_OUTPUT_PATH: ${{ steps.load-variables.outputs.target-output-path }} + - name: Build NativeAOT policy consent helper + if: ${{ matrix.os == 'windows' }} + run: | + $Rid = "win-${{ matrix.arch }}" + $Output = Split-Path -Parent '${{ steps.load-variables.outputs.dagent-policy-consent-helper }}' + dotnet publish package/AgentPolicyConsent/DevolutionsAgentPolicyConsent.csproj ` + --configuration Release ` + --runtime $Rid ` + --output $Output ` + -p:Version=${{ needs.preflight.outputs.version }} + if ($LASTEXITCODE -ne 0) { + exit $LASTEXITCODE + } + $Helper = '${{ steps.load-variables.outputs.dagent-policy-consent-helper }}' + if (-Not (Test-Path -LiteralPath $Helper -PathType Leaf)) { + throw "NativeAOT policy consent helper was not produced" + } + if ((Get-Item -LiteralPath $Helper).Length -gt 8MB) { + throw "NativeAOT policy consent helper exceeds 8 MiB" + } + shell: pwsh + - name: Package if: ${{ github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository }} run: | @@ -1056,6 +1081,7 @@ jobs: $Env:DAGENT_PEDM_SHELL_EXT_DLL = "${{ steps.load-variables.outputs.dagent-pedm-shell-ext-dll }}" $Env:DAGENT_PEDM_SHELL_EXT_MSIX = "${{ steps.load-variables.outputs.dagent-pedm-shell-ext-msix }}" $Env:DAGENT_SESSION_EXECUTABLE = "${{ steps.load-variables.outputs.dagent-session-executable }}" + $Env:DAGENT_POLICY_CONSENT_HELPER = "${{ steps.load-variables.outputs.dagent-policy-consent-helper }}" $Env:DAGENT_TUN2SOCKS_EXE = "${{ steps.tun2socks.outputs.tun2socks-executable-path }}" $Env:DAGENT_WINTUN_DLL = "${{ steps.tun2socks.outputs.wintun-library-path }}" $Env:DAGENT_MULTI_PWSH_EXECUTABLE = "${{ steps.multi-pwsh.outputs.executable-path }}" @@ -1177,6 +1203,10 @@ jobs: run: dotnet test package/AgentWindowsManaged.Tests/DevolutionsAgent.Installer.Tests.csproj shell: pwsh + - name: Policy consent helper tests + run: dotnet test package/AgentPolicyConsent.Tests/DevolutionsAgentPolicyConsent.Tests.csproj -c Release + shell: pwsh + winapi-sanitizer-tests: name: Windows API sanitizer tests diff --git a/.github/workflows/package.yml b/.github/workflows/package.yml index 27ee014f9..af56e0b9f 100644 --- a/.github/workflows/package.yml +++ b/.github/workflows/package.yml @@ -322,7 +322,7 @@ jobs: run: | $IncludePattern = @(switch ('${{ matrix.project }}') { 'devolutions-gateway' { @('DevolutionsGateway.exe') } - 'devolutions-agent' { @('DevolutionsAgent.exe', 'DevolutionsAgentUpdater.exe', 'DevolutionsPedmShellExt.dll', 'DevolutionsPedmShellExt.msix', 'DevolutionsDesktopAgent.exe') } + 'devolutions-agent' { @('DevolutionsAgent.exe', 'DevolutionsAgentUpdater.exe', 'DevolutionsAgentPolicyConsent.exe', 'DevolutionsPedmShellExt.dll', 'DevolutionsPedmShellExt.msix', 'DevolutionsDesktopAgent.exe') } 'jetsocat' { @('jetsocat.exe', 'jetsocat') } }) $ExcludePattern = "*.pdb" @@ -495,6 +495,7 @@ jobs: $Env:DAGENT_PEDM_SHELL_EXT_DLL = Get-ChildItem -Path $ArchRoot -Filter 'DevolutionsPedmShellExt.dll' -File | Select-Object -First 1 $Env:DAGENT_PEDM_SHELL_EXT_MSIX = Get-ChildItem -Path $ArchRoot -Filter 'DevolutionsPedmShellExt.msix' -File | Select-Object -First 1 $Env:DAGENT_SESSION_EXECUTABLE = Get-ChildItem -Path $ArchRoot -Filter 'DevolutionsSession.exe' -File | Select-Object -First 1 + $Env:DAGENT_POLICY_CONSENT_HELPER = Get-ChildItem -Path $ArchRoot -Filter 'DevolutionsAgentPolicyConsent.exe' -File | Select-Object -First 1 $Env:DAGENT_TUN2SOCKS_EXE = Join-Path $ArchRoot 'tun2socks.exe' $Env:DAGENT_WINTUN_DLL = Join-Path $ArchRoot 'wintun.dll' $MultiPwshDirectory = Join-Path $Env:RUNNER_TEMP 'multi-pwsh' 'windows' $Arch @@ -508,6 +509,7 @@ jobs: Write-Host "DAGENT_PEDM_SHELL_EXT_DLL = ${Env:DAGENT_PEDM_SHELL_EXT_DLL}" Write-Host "DAGENT_PEDM_SHELL_EXT_MSIX = ${Env:DAGENT_PEDM_SHELL_EXT_MSIX}" Write-Host "DAGENT_SESSION_EXECUTABLE = ${Env:DAGENT_SESSION_EXECUTABLE}" + Write-Host "DAGENT_POLICY_CONSENT_HELPER = ${Env:DAGENT_POLICY_CONSENT_HELPER}" Write-Host "DAGENT_TUN2SOCKS_EXE = ${Env:DAGENT_TUN2SOCKS_EXE}" Write-Host "DAGENT_WINTUN_DLL = ${Env:DAGENT_WINTUN_DLL}" Write-Host "DAGENT_MULTI_PWSH_EXECUTABLE = ${Env:DAGENT_MULTI_PWSH_EXECUTABLE}" @@ -534,7 +536,8 @@ jobs: @((Join-Path $ArchRoot DesktopAgent), (Get-ChildItem -Path $ArchRoot -Filter 'DevolutionsPedmShellExt.dll' | Select-Object -First 1), (Get-ChildItem -Path $ArchRoot -Filter 'DevolutionsPedmShellExt.msix' | Select-Object -First 1), - (Get-ChildItem -Path $ArchRoot -Filter 'DevolutionsSession.exe' | Select-Object -First 1)) | ForEach-Object { + (Get-ChildItem -Path $ArchRoot -Filter 'DevolutionsSession.exe' | Select-Object -First 1), + (Get-ChildItem -Path $ArchRoot -Filter 'DevolutionsAgentPolicyConsent.exe' | Select-Object -First 1)) | ForEach-Object { Remove-Item $_ -Recurse -ErrorAction SilentlyContinue | Out-Null } } diff --git a/ci/package-agent-windows.ps1 b/ci/package-agent-windows.ps1 index 4a37cc512..10f0e7219 100644 --- a/ci/package-agent-windows.ps1 +++ b/ci/package-agent-windows.ps1 @@ -11,6 +11,8 @@ param( [parameter(Mandatory = $true)] [string] $SessionExe, [parameter(Mandatory = $true)] + [string] $PolicyConsentHelper, + [parameter(Mandatory = $true)] [ValidateSet('x64', 'arm64')] [string] $Architecture, [string] $Outfile @@ -98,6 +100,9 @@ function New-AgentMsi() { # The path to the devolutions-session.exe file. [string] $SessionExe, [parameter(Mandatory = $true)] + # The path to the DevolutionsAgentPolicyConsent.exe file. + [string] $PolicyConsentHelper, + [parameter(Mandatory = $true)] [ValidateSet('x64', 'arm64')] # Architecture: x64 or arm64 [string] $Architecture, @@ -120,6 +125,7 @@ function New-AgentMsi() { $PedmDll = Convert-Path -Path $PedmDll $PedmMsix = Convert-Path -Path $PedmMsix $SessionExe = Convert-Path -Path $SessionExe + $PolicyConsentHelper = Convert-Path -Path $PolicyConsentHelper if ($Outfile) { $Outfile = Convert-Path -Path $Outfile } @@ -137,6 +143,7 @@ function New-AgentMsi() { $myUpdaterExe = Set-FileNameAndCopy -Path $UpdaterExe -NewName 'DevolutionsAgentUpdater.exe' # The session is a service that gets launched on demand. $mySessionExe = Set-FileNameAndCopy -Path $SessionExe -NewName 'DevolutionsSession.exe' + $myPolicyConsentHelper = Set-FileNameAndCopy -Path $PolicyConsentHelper -NewName 'DevolutionsAgentPolicyConsent.exe' Write-Output "$repoDir\dotnet\DesktopAgent\bin\Release\net48\DevolutionsDesktopAgent.exe" @@ -145,6 +152,7 @@ function New-AgentMsi() { Set-EnvVarPath 'DAGENT_PEDM_SHELL_EXT_DLL' $myPedmDll Set-EnvVarPath 'DAGENT_PEDM_SHELL_EXT_MSIX' $myPedmMsix Set-EnvVarPath 'DAGENT_SESSION_EXECUTABLE' $mySessionExe + Set-EnvVarPath 'DAGENT_POLICY_CONSENT_HELPER' $myPolicyConsentHelper # The actual DevolutionsDesktopAgent.exe will be `\dotnet\DesktopAgent\bin\Release\net48\DevolutionsDesktopAgent.exe`. # After install, the contents of `net48` will be copied to `C:\Program Files\Devolutions\Agent\desktop\`. @@ -184,4 +192,4 @@ function New-AgentMsi() { Pop-Location } -New-AgentMsi -Generate:($Generate.IsPresent) -Exe $Exe -UpdaterExe $UpdaterExe -PedmDll $PedmDll -PedmMsix $PedmMsix -SessionExe $SessionExe -Architecture $Architecture -Outfile $Outfile +New-AgentMsi -Generate:($Generate.IsPresent) -Exe $Exe -UpdaterExe $UpdaterExe -PedmDll $PedmDll -PedmMsix $PedmMsix -SessionExe $SessionExe -PolicyConsentHelper $PolicyConsentHelper -Architecture $Architecture -Outfile $Outfile diff --git a/crates/now-package-broker/src/auth.rs b/crates/now-package-broker/src/auth.rs index d647e750c..f293a0306 100644 --- a/crates/now-package-broker/src/auth.rs +++ b/crates/now-package-broker/src/auth.rs @@ -32,6 +32,7 @@ use windows::Win32::System::Threading::{ use crate::policy_security::RetainedExecutableSecurity; const PROCESS_SYNCHRONIZE: PROCESS_ACCESS_RIGHTS = PROCESS_ACCESS_RIGHTS(0x0010_0000); +const POLICY_CONSENT_HELPER_NAME: &str = "DevolutionsAgentPolicyConsent.exe"; const PROCESS_IDENTITY_ACCESS: PROCESS_ACCESS_RIGHTS = PROCESS_ACCESS_RIGHTS( PROCESS_QUERY_INFORMATION.0 | PROCESS_QUERY_LIMITED_INFORMATION.0 | PROCESS_VM_READ.0 | PROCESS_SYNCHRONIZE.0, ); @@ -289,6 +290,20 @@ impl PipeClient { Ok(()) } + pub(crate) fn validate_policy_write(&self, skip_signature_validation: bool) -> anyhow::Result<()> { + self.validate_connection(skip_signature_validation)?; + // Dev builds cannot enforce helper identity when signature validation is explicitly disabled. + if signature_validation_skipped(skip_signature_validation) { + return Ok(()); + } + let agent = std::env::current_exe().context("failed to query Agent executable path")?; + let executable_file = self + .executable_file + .as_deref() + .context("policy consent helper executable handle is not retained")?; + Self::validate_policy_consent_helper_path(&self.executable_path, executable_file, &agent) + } + fn validate_process_instance(&self) -> anyhow::Result<()> { let Some(process) = &self.process else { return Ok(()); @@ -304,6 +319,29 @@ impl PipeClient { ) } + fn validate_policy_consent_helper_path(client: &Path, client_file: &File, agent: &Path) -> anyhow::Result<()> { + if !client + .file_name() + .is_some_and(|name| name.eq_ignore_ascii_case(POLICY_CONSENT_HELPER_NAME)) + { + bail!("policy replacement requires the Agent policy consent helper"); + } + let expected = agent + .parent() + .context("Agent executable has no installation directory")? + .join(POLICY_CONSENT_HELPER_NAME); + if !crate::policy_security::windows_paths_equal(client, &expected) { + bail!("policy consent helper is not the installed Agent helper path"); + } + let expected_id = file_id(&expected).context("failed to query installed policy consent helper identity")?; + let retained_id = + file_id_from_handle(client_file).context("failed to query retained policy consent helper identity")?; + if !same_file(&expected_id, &retained_id) { + bail!("policy consent helper does not match the installed helper"); + } + Ok(()) + } + /// Validate that the request's `effective_user` denotes the authenticated pipe client user. /// /// The name is resolved to a SID and compared against the SID captured at connect, @@ -539,6 +577,51 @@ mod tests { .expect_err("a recycled PID with a different creation time must be rejected"); } + #[test] + fn policy_consent_helper_requires_exact_agent_sibling_path() { + let current_executable = std::env::current_exe().expect("current executable"); + let current_file = open_executable_file(¤t_executable).expect("open current executable"); + let agent = Path::new(r"C:\Program Files\Devolutions\Agent\DevolutionsAgent.exe"); + assert!( + PipeClient::validate_policy_consent_helper_path( + Path::new(r"C:\Program Files\Devolutions\Agent\DevolutionsAgentPolicyConsent.exe"), + ¤t_file, + agent, + ) + .is_err(), + "path text alone must not authorize a different retained image" + ); + assert!( + PipeClient::validate_policy_consent_helper_path( + Path::new(r"C:\Users\Alice\DevolutionsAgentPolicyConsent.exe"), + ¤t_file, + agent, + ) + .is_err() + ); + assert!( + PipeClient::validate_policy_consent_helper_path( + Path::new(r"C:\Users\Alice\UniGetUI.exe"), + ¤t_file, + agent, + ) + .is_err() + ); + } + + #[test] + fn policy_consent_helper_accepts_exact_retained_sibling() { + let temp = tempfile::tempdir().expect("temp directory"); + let agent = temp.path().join("DevolutionsAgent.exe"); + let helper = temp.path().join(POLICY_CONSENT_HELPER_NAME); + std::fs::write(&agent, b"agent path anchor").expect("write Agent path anchor"); + std::fs::copy(std::env::current_exe().expect("current executable"), &helper).expect("copy helper fixture"); + let retained = open_executable_file(&helper).expect("retain helper fixture"); + + PipeClient::validate_policy_consent_helper_path(&helper, &retained, &agent) + .expect("exact retained sibling must be accepted"); + } + #[test] fn exited_process_cannot_supply_executable_identity() { let mut child = std::process::Command::new("powershell.exe") diff --git a/crates/now-package-broker/src/server/mod.rs b/crates/now-package-broker/src/server/mod.rs index 0f2a5fd6c..eea38ce7a 100644 --- a/crates/now-package-broker/src/server/mod.rs +++ b/crates/now-package-broker/src/server/mod.rs @@ -135,7 +135,12 @@ async fn authenticate_policy_management( | (&Method::PUT, "/v1/policy") ); if protected { - if let Err(error) = client.validate_connection(state.skip_signature_validation) { + let authentication = if matches!((request.method(), request.uri().path()), (&Method::PUT, "/v1/policy")) { + client.validate_policy_write(state.skip_signature_validation) + } else { + client.validate_connection(state.skip_signature_validation) + }; + if let Err(error) = authentication { if let Some(audit) = write_audit { audit.denied(crate::audit::DenialReason::AuthenticationFailed); } diff --git a/package/AgentPolicyConsent.Tests/DevolutionsAgentPolicyConsent.Tests.csproj b/package/AgentPolicyConsent.Tests/DevolutionsAgentPolicyConsent.Tests.csproj new file mode 100644 index 000000000..24bdd8852 --- /dev/null +++ b/package/AgentPolicyConsent.Tests/DevolutionsAgentPolicyConsent.Tests.csproj @@ -0,0 +1,22 @@ + + + net10.0-windows + win-x64 + true + false + enable + enable + + + + + + all + + + + + + diff --git a/package/AgentPolicyConsent.Tests/ProtocolTests.cs b/package/AgentPolicyConsent.Tests/ProtocolTests.cs new file mode 100644 index 000000000..36456f288 --- /dev/null +++ b/package/AgentPolicyConsent.Tests/ProtocolTests.cs @@ -0,0 +1,445 @@ +using System.Buffers.Binary; +using System.Text.Json; +using DevolutionsAgentPolicyConsent; +using Microsoft.Win32.SafeHandles; +using Xunit; + +namespace DevolutionsAgentPolicyConsent.Tests; + +public sealed class ProtocolTests +{ + private const string RequestId = "0123456789abcdef0123456789abcdef"; + + [Fact] + public void ArgumentsRequireExactBoundIdentity() + { + Arguments parsed = Protocol.ParseArguments( + [ + "--protocol", "2.0", + "--pipe", $"UniGetUI.PolicyElevation.{RequestId}", + "--parent-pid", "42", + "--parent-created", "638900000000000000", + "--session", "1", + ]); + + Assert.Equal(42, parsed.ParentProcessId); + Assert.Equal((uint)1, parsed.SessionId); + } + + public sealed class TrustPolicyTests + { + [Fact] + public void CurrentAndTransitionSignersAreAccepted() + { + Assert.True(PeerLease.IsAllowedSigner(PeerLease.CurrentUiSignerSpkiSha256)); + Assert.True(PeerLease.IsAllowedSigner(PeerLease.TransitionUiSignerSpkiSha256)); + } + + [Fact] + public void LookalikeSignerIsRejected() + { + Assert.False(PeerLease.IsAllowedSigner(new string('0', 64))); + } + + [Fact] + public void SignerMatchingIsCaseSensitive() + { + Assert.False(PeerLease.IsAllowedSigner(PeerLease.CurrentUiSignerSpkiSha256.ToUpperInvariant())); + } + + [Theory] + [InlineData("3.3.7")] + [InlineData("2026.2.7")] + public void ProductBindingSupportsProtocolEraAndCurrentInstallModes(string version) + { + Assert.True(PeerLease.IsSupportedUiIdentity("UniGetUI", "UniGetUI.dll", version)); + } + + [Theory] + [InlineData("Lookalike", "UniGetUI.dll", "2026.2.7")] + [InlineData("UniGetUI", "malware.exe", "2026.2.7")] + [InlineData("UniGetUI", "UniGetUI.dll", "3.3.6")] + public void ProductBindingRejectsLookalikes(string product, string originalFilename, string version) + { + Assert.False(PeerLease.IsSupportedUiIdentity(product, originalFilename, version)); + } + + [Theory] + [InlineData(41, 638900000000000000, 1)] + [InlineData(42, 638900000000000001, 1)] + [InlineData(42, 638900000000000000, 2)] + public void ProcessIdentityRejectsPidReuseAndSessionMismatch(int pid, long created, uint session) + { + Arguments expected = new("pipe", 42, 638900000000000000, 1); + Assert.False(PeerLease.MatchesProcessIdentity(expected, pid, created, session)); + } + + [Fact] + public void ProcessIdentityAcceptsExactRetainedInstance() + { + Arguments expected = new("pipe", 42, 638900000000000000, 1); + Assert.True(PeerLease.MatchesProcessIdentity(expected, 42, 638900000000000000, 1)); + } + + [Fact] + public void BrokerServerRequiresExactAgentSiblingPath() + { + Assert.True(BrokerServerLease.IsExpectedPath( + @"C:\Program Files\Devolutions\Agent\DevolutionsAgent.exe", + @"c:\PROGRAM FILES\Devolutions\Agent\DevolutionsAgent.exe")); + Assert.False(BrokerServerLease.IsExpectedPath( + @"C:\Users\Alice\DevolutionsAgent.exe", + @"C:\Program Files\Devolutions\Agent\DevolutionsAgent.exe")); + } + + [Fact] + public void AuthenticodeSignerComesFromRetainedImageHandle() + { + string path = Path.Combine(AppContext.BaseDirectory, "testhost.exe"); + using SafeFileHandle image = OpenImage(path); + using var certificate = PeerLease.VerifyAuthenticodeSigner(path, image, "test host"); + Assert.NotEmpty(certificate.RawData); + } + + [Fact] + public void ProcessImageMappingAcceptsOnlyCurrentMappedImage() + { + using SafeProcessHandle process = Native.OpenProcess( + PeerLease.ProcessQueryInformation | + PeerLease.ProcessQueryLimitedInformation | + PeerLease.Synchronize, + false, + Environment.ProcessId); + Assert.False(process.IsInvalid); + + string processPath = PeerLease.ImagePath(process); + using SafeFileHandle processImage = OpenImage(processPath); + PeerLease.VerifyImageMapping(process, processImage); + + using SafeFileHandle differentImage = + OpenImage(Path.Combine(AppContext.BaseDirectory, "DevolutionsAgentPolicyConsent.exe")); + Assert.Throws(() => PeerLease.VerifyImageMapping(process, differentImage)); + } + + [Fact] + public void BrokerServerRejectsNonSystemProcessToken() + { + using SafeProcessHandle process = Native.OpenProcess( + PeerLease.ProcessQueryLimitedInformation, + false, + Environment.ProcessId); + Assert.False(process.IsInvalid); + Assert.False(PeerLease.IsLocalSystemProcess(process)); + } + + [Fact] + public void FileIdentityRejectsDifferentImage() + { + using SafeFileHandle first = OpenImage(Path.Combine(AppContext.BaseDirectory, "testhost.exe")); + using SafeFileHandle same = OpenImage(Path.Combine(AppContext.BaseDirectory, "testhost.exe")); + using SafeFileHandle different = + OpenImage(Path.Combine(AppContext.BaseDirectory, "DevolutionsAgentPolicyConsent.exe")); + + Assert.True(PeerLease.SameFile(first, same)); + Assert.False(PeerLease.SameFile(first, different)); + } + + private static SafeFileHandle OpenImage(string path) + { + SafeFileHandle image = Native.CreateFile( + path, + PeerLease.GenericRead | PeerLease.FileExecute | PeerLease.Synchronize, + PeerLease.FileShareRead, + IntPtr.Zero, + PeerLease.OpenExisting, + 0, + IntPtr.Zero); + Assert.False(image.IsInvalid); + return image; + } + } + + [Theory] + [InlineData("--extra")] + [InlineData("--pipe")] + public void ArgumentsRejectUnknownOrDuplicateNames(string name) + { + string[] args = + [ + "--protocol", "2.0", + "--pipe", $"UniGetUI.PolicyElevation.{RequestId}", + "--parent-pid", "42", + "--parent-created", "638900000000000000", + name, "1", + ]; + + Assert.Throws(() => Protocol.ParseArguments(args)); + } + + [Fact] + public async Task FrameUsesBigEndianLengthAndRoundTrips() + { + byte[] body = [1, 2, 3, 4]; + using MemoryStream stream = new(); + await Protocol.WriteFrameAsync(stream, body, body.Length, CancellationToken.None); + Assert.Equal((uint)body.Length, BinaryPrimitives.ReadUInt32BigEndian(stream.GetBuffer())); + stream.Position = 0; + Assert.Equal(body, await Protocol.ReadFrameAsync(stream, body.Length, CancellationToken.None)); + } + + [Fact] + public async Task OversizedFrameIsRejectedBeforeBodyRead() + { + byte[] header = new byte[4]; + BinaryPrimitives.WriteUInt32BigEndian(header, 128); + using MemoryStream stream = new(header); + await Assert.ThrowsAsync( + () => Protocol.ReadFrameAsync(stream, 127, CancellationToken.None)); + } + + [Fact] + public async Task ZeroLengthAndTruncatedFramesAreRejected() + { + using MemoryStream empty = new(new byte[4]); + await Assert.ThrowsAsync( + () => Protocol.ReadFrameAsync(empty, 128, CancellationToken.None)); + + using MemoryStream truncated = new([0, 0, 0, 2, 1]); + await Assert.ThrowsAsync( + () => Protocol.ReadFrameAsync(truncated, 128, CancellationToken.None)); + } + + [Fact] + public async Task FrameReadHonorsCancellation() + { + using CancellationTokenSource cancellation = new(TimeSpan.FromMilliseconds(25)); + await Assert.ThrowsAnyAsync( + () => Protocol.ReadFrameAsync(new BlockingStream(), 128, cancellation.Token)); + } + + [Fact] + public void RequestRejectsUnknownJsonMembers() + { + string json = + $$"""{"protocolVersion":"2.0","requestId":"{{RequestId}}","operation":"Update","conflictHandling":"Reject","expectedStoreToken":"a","validationReceipt":"b","warningsAcknowledged":false,"draft":{},"command":"cmd.exe"}"""; + + Assert.Throws( + () => JsonSerializer.Deserialize(json, ProtocolJsonContext.Default.ElevationRequest)); + } + + [Fact] + public void OfficialRequestContainsOnlyPolicyReplacementFields() + { + using JsonDocument draft = JsonDocument.Parse("""{"Metadata":{"Id":"tests.policy"}}"""); + ElevationRequest request = new( + "2.0", + RequestId, + "Update", + "ConfirmOverwrite", + "token", + "receipt", + true, + draft.RootElement.Clone()); + + using JsonDocument official = JsonDocument.Parse(BrokerClient.CreateOfficialRequest(request)); + string[] names = official.RootElement.EnumerateObject().Select(property => property.Name).ToArray(); + Assert.Equal( + [ + "RequestKind", + "RequestVersion", + "ExpectedStoreToken", + "Operation", + "ConflictHandling", + "WarningsAcknowledged", + "Draft", + "ValidationReceipt", + ], names); + } + + [Fact] + public void RequestRequiresEveryMemberAndObjectDraft() + { + string missingAcknowledgement = + $$$"""{"protocolVersion":"2.0","requestId":"{{{RequestId}}}","operation":"Update","conflictHandling":"Reject","expectedStoreToken":"a","validationReceipt":"b","draft":{}}"""; + Assert.Throws( + () => JsonSerializer.Deserialize(missingAcknowledgement, ProtocolJsonContext.Default.ElevationRequest)); + + using JsonDocument draft = JsonDocument.Parse("[]"); + ElevationRequest request = new("2.0", RequestId, "Update", "Reject", "a", "b", false, draft.RootElement); + Assert.Throws(() => Protocol.ValidateRequest(request)); + } + + [Theory] + [InlineData("Delete", "Reject")] + [InlineData("Update", "Overwrite")] + public void RequestRejectsUnknownOperations(string operation, string conflictHandling) + { + using JsonDocument draft = JsonDocument.Parse("{}"); + ElevationRequest request = new( + "2.0", + RequestId, + operation, + conflictHandling, + "a", + "b", + false, + draft.RootElement); + + Assert.Throws(() => Protocol.ValidateRequest(request)); + } + + [Fact] + public void CommittedResponseContainsOnlyStoreToken() + { + ElevationResponse response = new( + "2.0", + RequestId, + "Committed", + null, + null, + "new-token", + null, + null, + null); + + Protocol.ValidateResponse(response); + string json = JsonSerializer.Serialize(response, ProtocolJsonContext.Default.ElevationResponse); + Assert.DoesNotContain("payload", json, StringComparison.OrdinalIgnoreCase); + Assert.DoesNotContain("message", json, StringComparison.OrdinalIgnoreCase); + } + + [Theory] + [InlineData("Active", "policy.id")] + [InlineData("Missing", null)] + [InlineData("Invalid", null)] + public void StaleRejectionCarriesBoundedConflictContext(string state, string? policyId) + { + ElevationResponse response = new( + "2.0", + RequestId, + "Rejected", + 409, + "StalePolicyStoreToken", + null, + "current-token", + state, + policyId); + + Protocol.ValidateResponse(response); + } + + [Fact] + public void UnknownResponseCannotClaimCommitOrConflict() + { + ElevationResponse response = new( + "2.0", + RequestId, + "Unknown", + null, + "Timeout", + "claimed-token", + null, + null, + null); + + Assert.Throws(() => Protocol.ValidateResponse(response)); + } + + [Fact] + public void ResponseRequiresExplicitNullableMembers() + { + string missingConflictFields = + $$$"""{"protocolVersion":"2.0","requestId":"{{{RequestId}}}","disposition":"Unknown","brokerStatusCode":null,"brokerErrorCode":"Timeout","committedStoreToken":null}"""; + + Assert.Throws( + () => JsonSerializer.Deserialize(missingConflictFields, ProtocolJsonContext.Default.ElevationResponse)); + } + + [Fact] + public void MaximumStaleResponseFitsExactWireBudget() + { + ElevationResponse response = new( + "2.0", + RequestId, + "Rejected", + 409, + "StalePolicyStoreToken", + null, + "T" + new string('"', 511), + "Active", + "P" + new string('"', 2047)); + + Protocol.ValidateResponse(response); + byte[] body = JsonSerializer.SerializeToUtf8Bytes( + response, + ProtocolJsonContext.Default.ElevationResponse); + Assert.InRange(body.Length, 1, Protocol.MaxResponseBodyBytes); + } + + [Fact] + public void BrokerSuccessMapsToCompactCommittedAcknowledgement() + { + ElevationResponse response = BrokerClient.ParseResponse( + RequestId, + HttpResponse( + 200, + """{"ResponseKind":"PolicyReplacementResponse","ResponseVersion":"1.0","Management":{"StoreToken":"new-token"}}""")); + + Assert.Equal("Committed", response.Disposition); + Assert.Equal("new-token", response.CommittedStoreToken); + Assert.Null(response.BrokerStatusCode); + } + + [Fact] + public void BrokerStaleErrorMapsExactConflictContext() + { + ElevationResponse response = BrokerClient.ParseResponse( + RequestId, + HttpResponse( + 409, + """{"ResponseKind":"ErrorResponse","ResponseVersion":"1.0","Code":"StalePolicyStoreToken","Management":{"StoreToken":"current-token","State":"Active","Policy":{"Metadata":{"Id":"policy.id"}}}}""")); + + Assert.Equal("Rejected", response.Disposition); + Assert.Equal(409, response.BrokerStatusCode); + Assert.Equal("current-token", response.ConflictStoreToken); + Assert.Equal("Active", response.ConflictState); + Assert.Equal("policy.id", response.ConflictPolicyId); + } + + [Fact] + public void EmptyBrokerResponseMapsToUnknown() + { + ElevationResponse response = BrokerClient.ParseResponse( + RequestId, + HttpResponse(503, string.Empty)); + + Assert.Equal("Unknown", response.Disposition); + Assert.Equal(503, response.BrokerStatusCode); + Assert.Equal("EmptyResponse", response.BrokerErrorCode); + } + + private static byte[] HttpResponse(int status, string body) => + System.Text.Encoding.UTF8.GetBytes( + $"HTTP/1.1 {status} Test\r\nContent-Length: {System.Text.Encoding.UTF8.GetByteCount(body)}\r\n\r\n{body}"); + + private sealed class BlockingStream : Stream + { + public override bool CanRead => true; + public override bool CanSeek => false; + public override bool CanWrite => false; + public override long Length => throw new NotSupportedException(); + public override long Position { get => throw new NotSupportedException(); set => throw new NotSupportedException(); } + public override void Flush() => throw new NotSupportedException(); + public override int Read(byte[] buffer, int offset, int count) => throw new NotSupportedException(); + public override long Seek(long offset, SeekOrigin origin) => throw new NotSupportedException(); + public override void SetLength(long value) => throw new NotSupportedException(); + public override void Write(byte[] buffer, int offset, int count) => throw new NotSupportedException(); + public override async ValueTask ReadAsync( + Memory buffer, + CancellationToken cancellationToken = default) + { + await Task.Delay(Timeout.InfiniteTimeSpan, cancellationToken); + return 0; + } + } +} diff --git a/package/AgentPolicyConsent/BrokerClient.cs b/package/AgentPolicyConsent/BrokerClient.cs new file mode 100644 index 000000000..bdcbd5120 --- /dev/null +++ b/package/AgentPolicyConsent/BrokerClient.cs @@ -0,0 +1,250 @@ +using System.Buffers; +using System.IO.Pipes; +using System.Text; +using System.Text.Json; + +namespace DevolutionsAgentPolicyConsent; + +internal static class BrokerClient +{ + private const string PipeName = "Devolutions.Now.PackageBroker.v1"; + private const int MaximumHeaderBytes = 64 * 1024; + private const int MaximumBrokerResponseBytes = 50_606_928; + private static readonly TimeSpan ConnectTimeout = TimeSpan.FromSeconds(5); + + internal static async Task ReplaceAsync( + ElevationRequest request, + CancellationToken cancellationToken) + { + byte[] body = CreateOfficialRequest(request); + try + { + using NamedPipeClientStream pipe = new( + ".", + PipeName, + PipeDirection.InOut, + PipeOptions.Asynchronous | PipeOptions.WriteThrough, + System.Security.Principal.TokenImpersonationLevel.Anonymous); + + using CancellationTokenSource connectTimeout = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); + connectTimeout.CancelAfter(ConnectTimeout); + await pipe.ConnectAsync(connectTimeout.Token); + using BrokerServerLease broker = BrokerServerLease.Open(pipe.SafePipeHandle); + + byte[] headers = Encoding.ASCII.GetBytes( + $"PUT /v1/policy HTTP/1.1\r\nHost: now-package-broker\r\nConnection: close\r\n" + + $"Content-Type: application/json\r\nAccept: application/json\r\nContent-Length: {body.Length}\r\n\r\n"); + await pipe.WriteAsync(headers, cancellationToken); + await pipe.WriteAsync(body, cancellationToken); + await pipe.FlushAsync(cancellationToken); + + byte[] response = await ReadBoundedAsync( + pipe, + MaximumBrokerResponseBytes + MaximumHeaderBytes, + cancellationToken); + return ParseResponse(request.RequestId, response); + } + catch (OperationCanceledException) + { + return Unknown(request.RequestId, null, "Timeout"); + } + catch (IOException) + { + return Unknown(request.RequestId, null, "BrokerUnavailable"); + } + catch (JsonException) + { + return Unknown(request.RequestId, null, "InvalidResponse"); + } + catch (BrokerResponseException error) + { + return Unknown(request.RequestId, error.StatusCode, "InvalidResponse"); + } + catch (InvalidOperationException) + { + return Unknown(request.RequestId, null, "InvalidResponse"); + } + catch (ProtocolException) + { + return Unknown(request.RequestId, null, "InvalidResponse"); + } + } + + internal static byte[] CreateOfficialRequest(ElevationRequest request) + { + ArrayBufferWriter buffer = new(); + using Utf8JsonWriter writer = new(buffer); + writer.WriteStartObject(); + writer.WriteString("RequestKind", "PolicyReplacementRequest"); + writer.WriteString("RequestVersion", "1.0"); + writer.WriteString("ExpectedStoreToken", request.ExpectedStoreToken); + writer.WriteString("Operation", request.Operation); + writer.WriteString("ConflictHandling", request.ConflictHandling); + writer.WriteBoolean("WarningsAcknowledged", request.WarningsAcknowledged); + writer.WritePropertyName("Draft"); + request.Draft.WriteTo(writer); + writer.WriteString("ValidationReceipt", request.ValidationReceipt); + writer.WriteEndObject(); + writer.Flush(); + if (buffer.WrittenCount > 16_777_216) + { + throw new ProtocolException("official policy request exceeds broker limit"); + } + return buffer.WrittenSpan.ToArray(); + } + + internal static ElevationResponse ParseResponse(string requestId, byte[] response) + { + ReadOnlySpan delimiter = "\r\n\r\n"u8; + int headerEnd = response.AsSpan().IndexOf(delimiter); + if (headerEnd < 0 || headerEnd > MaximumHeaderBytes) + { + throw new BrokerResponseException(null); + } + + string statusLine = Encoding.ASCII.GetString(response.AsSpan(0, headerEnd)).Split("\r\n", 2)[0]; + string[] statusParts = statusLine.Split(' ', 3, StringSplitOptions.RemoveEmptyEntries); + if (statusParts.Length < 2 || !int.TryParse(statusParts[1], out int status)) + { + throw new BrokerResponseException(null); + } + + ReadOnlyMemory body = response.AsMemory(headerEnd + delimiter.Length); + if (body.IsEmpty) + { + return Unknown(requestId, status, "EmptyResponse"); + } + + try + { + return ParseResponseBody(requestId, status, body); + } + catch (Exception error) when (error is JsonException or InvalidOperationException or ProtocolException) + { + throw new BrokerResponseException(status, error); + } + } + + private static ElevationResponse ParseResponseBody( + string requestId, + int status, + ReadOnlyMemory body) + { + using JsonDocument document = JsonDocument.Parse(body); + JsonElement payload = document.RootElement; + if (status is >= 200 and <= 299) + { + RequireString(payload, "ResponseKind", "PolicyReplacementResponse"); + RequireString(payload, "ResponseVersion", "1.0"); + string token = RequireNestedString(payload, "Management", "StoreToken"); + ElevationResponse committed = new( + Protocol.Version, + requestId, + "Committed", + null, + null, + token, + null, + null, + null); + Protocol.ValidateResponse(committed); + return committed; + } + + RequireString(payload, "ResponseKind", "ErrorResponse"); + RequireString(payload, "ResponseVersion", "1.0"); + string code = RequireString(payload, "Code"); + string? conflictToken = null; + string? conflictState = null; + string? conflictPolicyId = null; + if (code == "StalePolicyStoreToken") + { + JsonElement management = RequireObject(payload, "Management"); + conflictToken = RequireString(management, "StoreToken"); + conflictState = RequireString(management, "State"); + if (conflictState == "Active") + { + JsonElement policy = RequireObject(management, "Policy"); + JsonElement metadata = RequireObject(policy, "Metadata"); + conflictPolicyId = RequireString(metadata, "Id"); + } + } + ElevationResponse rejected = new( + Protocol.Version, + requestId, + "Rejected", + status, + Truncate(code, 64), + null, + conflictToken, + conflictState, + conflictPolicyId); + Protocol.ValidateResponse(rejected); + return rejected; + } + + private static void RequireString(JsonElement payload, string property, string expected) + { + if (!payload.TryGetProperty(property, out JsonElement value) || + value.ValueKind != JsonValueKind.String || + value.GetString() != expected) + { + throw new InvalidOperationException("broker response contract mismatch"); + } + } + + private static string RequireString(JsonElement payload, string property) + { + if (!payload.TryGetProperty(property, out JsonElement value) || + value.ValueKind != JsonValueKind.String || + value.GetString() is not { } result) + { + throw new InvalidOperationException("broker response contract mismatch"); + } + return result; + } + + private static string RequireNestedString(JsonElement payload, string parent, string property) => + RequireString(RequireObject(payload, parent), property); + + private static JsonElement RequireObject(JsonElement payload, string property) + { + if (!payload.TryGetProperty(property, out JsonElement value) || + value.ValueKind != JsonValueKind.Object) + { + throw new InvalidOperationException("broker response contract mismatch"); + } + return value; + } + + private static async Task ReadBoundedAsync(Stream stream, int maximum, CancellationToken cancellationToken) + { + using MemoryStream response = new(); + byte[] buffer = new byte[16 * 1024]; + while (true) + { + int read = await stream.ReadAsync(buffer, cancellationToken); + if (read == 0) + { + return response.ToArray(); + } + if (response.Length + read > maximum) + { + throw new InvalidOperationException("broker response exceeds limit"); + } + response.Write(buffer, 0, read); + } + } + + private static ElevationResponse Unknown(string requestId, int? statusCode, string errorCode) => + new(Protocol.Version, requestId, "Unknown", statusCode, errorCode, null, null, null, null); + + private static string Truncate(string value, int maximum) => + value.Length <= maximum ? value : value[..maximum]; + + private sealed class BrokerResponseException(int? statusCode, Exception? innerException = null) + : Exception("broker response contract mismatch", innerException) + { + internal int? StatusCode { get; } = statusCode; + } +} diff --git a/package/AgentPolicyConsent/DevolutionsAgentPolicyConsent.csproj b/package/AgentPolicyConsent/DevolutionsAgentPolicyConsent.csproj new file mode 100644 index 000000000..5e5e414e4 --- /dev/null +++ b/package/AgentPolicyConsent/DevolutionsAgentPolicyConsent.csproj @@ -0,0 +1,23 @@ + + + Exe + net10.0-windows + win-x64 + true + true + true + true + false + enable + enable + true + app.manifest + DevolutionsAgentPolicyConsent + DevolutionsAgentPolicyConsent + Devolutions Agent Policy Consent + Devolutions Inc. + + + + + diff --git a/package/AgentPolicyConsent/PeerTrust.cs b/package/AgentPolicyConsent/PeerTrust.cs new file mode 100644 index 000000000..846d151d1 --- /dev/null +++ b/package/AgentPolicyConsent/PeerTrust.cs @@ -0,0 +1,682 @@ +using Microsoft.Win32.SafeHandles; +using System.ComponentModel; +using System.Diagnostics; +using System.Runtime.InteropServices; +using System.Security.Cryptography; +using System.Security.Cryptography.X509Certificates; +using System.Text; + +namespace DevolutionsAgentPolicyConsent; + +internal sealed class PeerLease : IDisposable +{ + // SHA-256 digests of accepted UniGetUI signer SPKIs. Keep both keys during certificate rollover. + internal const string TransitionUiSignerSpkiSha256 = + PolicyConsentContract.TransitionUiSignerSpkiSha256; + internal const string CurrentUiSignerSpkiSha256 = + PolicyConsentContract.CurrentUiSignerSpkiSha256; + + internal const uint ProcessQueryLimitedInformation = 0x1000; + internal const uint ProcessQueryInformation = 0x0400; + internal const uint Synchronize = 0x0010_0000; + internal const uint GenericRead = 0x8000_0000; + internal const uint FileExecute = 0x20; + internal const uint FileShareRead = 0x1; + internal const uint OpenExisting = 3; + internal const int ProcessImageFileMapping = 44; + internal const uint StillActive = 259; + + private readonly SafeProcessHandle process; + private readonly SafeFileHandle image; + private readonly int processId; + private readonly long createdUtcTicks; + private readonly uint sessionId; + + private PeerLease( + SafeProcessHandle process, + SafeFileHandle image, + int processId, + long createdUtcTicks, + uint sessionId) + { + this.process = process; + this.image = image; + this.processId = processId; + this.createdUtcTicks = createdUtcTicks; + this.sessionId = sessionId; + } + + internal static PeerLease Open(Arguments arguments) + { + SafeProcessHandle process = Native.OpenProcess( + ProcessQueryInformation | ProcessQueryLimitedInformation | Synchronize, + false, + arguments.ParentProcessId); + if (process.IsInvalid) + { + throw new Win32Exception(); + } + + try + { + long created = CreationTime(process); + uint session = SessionId(arguments.ParentProcessId); + if (!MatchesProcessIdentity( + arguments, + arguments.ParentProcessId, + created, + session)) + { + throw new InvalidOperationException("parent process identity mismatch"); + } + + string path = ImagePath(process); + SafeFileHandle image = Native.CreateFile( + path, + GenericRead | FileExecute | Synchronize, + FileShareRead, + IntPtr.Zero, + OpenExisting, + 0, + IntPtr.Zero); + if (image.IsInvalid) + { + throw new Win32Exception(); + } + + try + { + VerifyImageMapping(process, image); + VerifyImageMetadata(path, image); + using X509Certificate2 signer = VerifyAuthenticodeSigner(path, image, "parent image"); + VerifySigner(signer); + VerifyImageMapping(process, image); + EnsureActive(process); + return new PeerLease(process, image, arguments.ParentProcessId, created, session); + } + catch + { + image.Dispose(); + throw; + } + } + catch + { + process.Dispose(); + throw; + } + } + + internal void VerifyConnectedServer(int serverProcessId) + { + Arguments expected = new(string.Empty, processId, createdUtcTicks, sessionId); + if (!MatchesProcessIdentity(expected, serverProcessId, CreationTime(process), SessionId(serverProcessId))) + { + throw new InvalidOperationException("connected server process identity mismatch"); + } + VerifyImageMapping(process, image); + EnsureActive(process); + } + + internal static bool IsAllowedSigner(string digest) => + FixedTimeEqualsHex(digest, CurrentUiSignerSpkiSha256) || + FixedTimeEqualsHex(digest, TransitionUiSignerSpkiSha256); + + internal static bool IsSupportedUiIdentity(string? productName, string? originalFilename, string? productVersion) => + string.Equals(productName, "UniGetUI", StringComparison.Ordinal) && + string.Equals(originalFilename, "UniGetUI.dll", StringComparison.OrdinalIgnoreCase) && + productVersion is not null && + Version.TryParse(productVersion.Split('+', '-', StringSplitOptions.TrimEntries)[0], out Version? parsed) && + parsed >= new Version(3, 3, 7); + + internal static bool MatchesProcessIdentity( + Arguments expected, + int processId, + long createdUtcTicks, + uint sessionId) => + processId == expected.ParentProcessId && + createdUtcTicks == expected.ParentCreatedUtcTicks && + sessionId == expected.SessionId; + + public void Dispose() + { + image.Dispose(); + process.Dispose(); + } + + private static void VerifyImageMetadata(string path, SafeFileHandle retainedImage) + { + VerifyPathIdentity(path, retainedImage); + if ((File.GetAttributes(path) & FileAttributes.ReparsePoint) != 0) + { + throw new InvalidOperationException("parent image is a reparse point"); + } + + FileVersionInfo version = FileVersionInfo.GetVersionInfo(path); + if (!IsSupportedUiIdentity(version.ProductName, version.OriginalFilename, version.ProductVersion)) + { + throw new InvalidOperationException("parent image product identity mismatch"); + } + VerifyPathIdentity(path, retainedImage); + } + + private static void VerifyPathIdentity(string path, SafeFileHandle retainedImage) + { + using SafeFileHandle reopened = Native.CreateFile( + path, + GenericRead | FileExecute | Synchronize, + FileShareRead, + IntPtr.Zero, + OpenExisting, + 0, + IntPtr.Zero); + if (reopened.IsInvalid) + { + throw new Win32Exception(); + } + if (!SameFile(retainedImage, reopened)) + { + throw new InvalidOperationException("parent image path no longer identifies the retained image"); + } + } + + internal static bool SameFile(SafeFileHandle left, SafeFileHandle right) + { + if (!Native.GetFileInformationByHandle(left, out Native.ByHandleFileInformation leftInfo) || + !Native.GetFileInformationByHandle(right, out Native.ByHandleFileInformation rightInfo)) + { + throw new Win32Exception(); + } + return leftInfo.VolumeSerialNumber == rightInfo.VolumeSerialNumber && + leftInfo.FileIndexHigh == rightInfo.FileIndexHigh && + leftInfo.FileIndexLow == rightInfo.FileIndexLow; + } + + internal static bool IsLocalSystemProcess(SafeProcessHandle process) + { + if (!Native.OpenProcessToken(process, 0x0008, out SafeAccessTokenHandle token)) + { + throw new Win32Exception(); + } + using (token) + { + _ = Native.GetTokenInformation(token, 1, IntPtr.Zero, 0, out uint length); + if (length == 0) + { + throw new Win32Exception(); + } + + IntPtr information = Marshal.AllocHGlobal(checked((int)length)); + try + { + if (!Native.GetTokenInformation(token, 1, information, length, out _)) + { + throw new Win32Exception(); + } + IntPtr sid = Marshal.ReadIntPtr(information); + return Native.IsWellKnownSid(sid, 22); + } + finally + { + Marshal.FreeHGlobal(information); + } + } + } + + internal static X509Certificate2 VerifyAuthenticodeSigner( + string path, + SafeFileHandle image, + string subject) + { + Guid action = new("00AAC56B-CD44-11d0-8CC2-00C04FC295EE"); + Native.WinTrustFileInfo file = new(path, image.DangerousGetHandle()); + IntPtr filePointer = Marshal.AllocHGlobal(Marshal.SizeOf()); + IntPtr dataPointer = Marshal.AllocHGlobal(Marshal.SizeOf()); + bool fileInitialized = false; + bool dataInitialized = false; + try + { + Marshal.StructureToPtr(file, filePointer, false); + fileInitialized = true; + Native.WinTrustData data = new(filePointer); + Marshal.StructureToPtr(data, dataPointer, false); + dataInitialized = true; + int status = Native.WinVerifyTrust(new IntPtr(-1), ref action, dataPointer); + if (status != 0) + { + throw new InvalidOperationException($"{subject} Authenticode validation failed (0x{status:X8})"); + } + data = Marshal.PtrToStructure(dataPointer); + IntPtr providerData = Native.WTHelperProvDataFromStateData(data.StateData); + IntPtr providerSigner = providerData == IntPtr.Zero + ? IntPtr.Zero + : Native.WTHelperGetProvSignerFromChain(providerData, 0, false, 0); + if (providerSigner == IntPtr.Zero) + { + throw new InvalidOperationException($"{subject} Authenticode signer is unavailable"); + } + + Native.CryptProviderSigner signer = Marshal.PtrToStructure(providerSigner); + if (signer.CertificateChainCount == 0 || signer.CertificateChain == IntPtr.Zero) + { + throw new InvalidOperationException($"{subject} Authenticode certificate chain is empty"); + } + Native.CryptProviderCertificate certificate = + Marshal.PtrToStructure(signer.CertificateChain); +#pragma warning disable SYSLIB0057 // WinVerifyTrust returns the certificate context for the exact retained image. + return new X509Certificate2(certificate.CertificateContext); +#pragma warning restore SYSLIB0057 + } + finally + { + if (dataInitialized) + { + Native.WinTrustData data = Marshal.PtrToStructure(dataPointer); + if (data.StateData != IntPtr.Zero) + { + data.StateAction = 2; + Marshal.StructureToPtr(data, dataPointer, true); + _ = Native.WinVerifyTrust(new IntPtr(-1), ref action, dataPointer); + } + Marshal.DestroyStructure(dataPointer); + } + if (fileInitialized) + { + Marshal.DestroyStructure(filePointer); + } + Marshal.FreeHGlobal(dataPointer); + Marshal.FreeHGlobal(filePointer); + } + } + + private static void VerifySigner(X509Certificate2 certificate) + { + byte[] subjectPublicKeyInfo; + using (RSA? rsa = certificate.GetRSAPublicKey()) + { + if (rsa is not null) + { + subjectPublicKeyInfo = rsa.ExportSubjectPublicKeyInfo(); + } + else + { + using ECDsa? ecdsa = certificate.GetECDsaPublicKey(); + subjectPublicKeyInfo = ecdsa?.ExportSubjectPublicKeyInfo() + ?? throw new InvalidOperationException("unsupported parent signer key"); + } + } + + string digest = Convert.ToHexString(SHA256.HashData(subjectPublicKeyInfo)).ToLowerInvariant(); + if (!IsAllowedSigner(digest)) + { + throw new InvalidOperationException("parent image signer is not authorized"); + } + } + + private static bool FixedTimeEqualsHex(string candidate, string expected) + { + if (candidate.Length != 64 || + expected.Length != 64 || + candidate.AsSpan().IndexOfAnyExcept("0123456789abcdef") >= 0) + { + return false; + } + try + { + return CryptographicOperations.FixedTimeEquals( + Convert.FromHexString(candidate), + Convert.FromHexString(expected)); + } + catch (FormatException) + { + return false; + } + } + + internal static void VerifyImageMapping(SafeProcessHandle process, SafeFileHandle image) + { + IntPtr fileHandle = image.DangerousGetHandle(); + int status = Native.NtQueryInformationProcess( + process.DangerousGetHandle(), + ProcessImageFileMapping, + ref fileHandle, + IntPtr.Size, + out _); + if (status != 0) + { + throw new InvalidOperationException($"parent image mapping mismatch (0x{status:X8})"); + } + } + + internal static string ImagePath(SafeProcessHandle process) + { + int capacity = 260; + while (capacity <= 32_768) + { + StringBuilder path = new(capacity); + int length = capacity; + if (Native.QueryFullProcessImageName(process, 0, path, ref length)) + { + return path.ToString(); + } + if (Marshal.GetLastWin32Error() != 122) + { + throw new Win32Exception(); + } + capacity *= 2; + } + throw new InvalidOperationException("parent image path is too long"); + } + + private static long CreationTime(SafeProcessHandle process) + { + if (!Native.GetProcessTimes(process, out long created, out _, out _, out _)) + { + throw new Win32Exception(); + } + return DateTime.FromFileTimeUtc(created).Ticks; + } + + private static uint SessionId(int processId) + { + if (!Native.ProcessIdToSessionId(processId, out uint sessionId)) + { + throw new Win32Exception(); + } + return sessionId; + } + + internal static void EnsureActive(SafeProcessHandle process) + { + if (!Native.GetExitCodeProcess(process, out uint exitCode)) + { + throw new Win32Exception(); + } + if (exitCode != StillActive) + { + throw new InvalidOperationException("parent process exited"); + } + } +} + +internal sealed class BrokerServerLease : IDisposable +{ + private const string AgentExecutableName = "DevolutionsAgent.exe"; + + private readonly SafeProcessHandle process; + private readonly SafeFileHandle image; + + private BrokerServerLease(SafeProcessHandle process, SafeFileHandle image) + { + this.process = process; + this.image = image; + } + + internal static BrokerServerLease Open(SafePipeHandle pipe) + { + if (!Native.GetNamedPipeServerProcessId(pipe, out int processId)) + { + throw new Win32Exception(); + } + + SafeProcessHandle process = Native.OpenProcess( + PeerLease.ProcessQueryLimitedInformation | PeerLease.Synchronize, + false, + processId); + if (process.IsInvalid) + { + throw new Win32Exception(); + } + + try + { + if (!PeerLease.IsLocalSystemProcess(process)) + { + throw new InvalidOperationException("broker server is not running as LocalSystem"); + } + string helperPath = Environment.ProcessPath + ?? throw new InvalidOperationException("helper executable path is unavailable"); + string expectedPath = Path.Combine( + Path.GetDirectoryName(helperPath) + ?? throw new InvalidOperationException("helper installation directory is unavailable"), + AgentExecutableName); + string serverPath = PeerLease.ImagePath(process); + if (!IsExpectedPath(serverPath, expectedPath)) + { + throw new InvalidOperationException("broker server is not the installed Agent"); + } + + SafeFileHandle image = Native.CreateFile( + expectedPath, + PeerLease.GenericRead | PeerLease.FileExecute | PeerLease.Synchronize, + PeerLease.FileShareRead, + IntPtr.Zero, + PeerLease.OpenExisting, + 0, + IntPtr.Zero); + if (image.IsInvalid) + { + throw new Win32Exception(); + } + + try + { + using X509Certificate2 _ = + PeerLease.VerifyAuthenticodeSigner(expectedPath, image, "broker server"); + PeerLease.EnsureActive(process); + return new BrokerServerLease(process, image); + } + catch + { + image.Dispose(); + throw; + } + } + catch + { + process.Dispose(); + throw; + } + } + + internal static bool IsExpectedPath(string actual, string expected) => + string.Equals( + Path.GetFullPath(actual), + Path.GetFullPath(expected), + StringComparison.OrdinalIgnoreCase); + + public void Dispose() + { + image.Dispose(); + process.Dispose(); + } +} + +internal static partial class Native +{ + [StructLayout(LayoutKind.Sequential, CharSet = CharSet.Unicode)] + internal readonly struct WinTrustFileInfo + { + internal readonly uint StructSize; + [MarshalAs(UnmanagedType.LPWStr)] + internal readonly string FilePath; + internal readonly IntPtr FileHandle; + internal readonly IntPtr KnownSubject; + + internal WinTrustFileInfo(string filePath, IntPtr fileHandle) + { + StructSize = checked((uint)Marshal.SizeOf()); + FilePath = filePath; + FileHandle = fileHandle; + KnownSubject = IntPtr.Zero; + } + } + + [StructLayout(LayoutKind.Sequential, CharSet = CharSet.Unicode)] + internal struct WinTrustData + { + internal uint StructSize; + internal IntPtr PolicyCallbackData; + internal IntPtr SipClientData; + internal uint UiChoice; + internal uint RevocationChecks; + internal uint UnionChoice; + internal IntPtr FileInfo; + internal uint StateAction; + internal IntPtr StateData; + internal IntPtr UrlReference; + internal uint ProviderFlags; + internal uint UiContext; + internal IntPtr SignatureSettings; + + internal WinTrustData(IntPtr fileInfo) + { + StructSize = checked((uint)Marshal.SizeOf()); + PolicyCallbackData = IntPtr.Zero; + SipClientData = IntPtr.Zero; + UiChoice = 2; + RevocationChecks = 0; + UnionChoice = 1; + FileInfo = fileInfo; + StateAction = 1; + StateData = IntPtr.Zero; + UrlReference = IntPtr.Zero; + ProviderFlags = 0x1000 | 0x2000; + UiContext = 0; + SignatureSettings = IntPtr.Zero; + } + } + + [StructLayout(LayoutKind.Sequential)] + internal readonly struct CryptProviderSigner + { + internal readonly uint StructSize; + internal readonly NativeFileTime VerifyAsOf; + internal readonly uint CertificateChainCount; + internal readonly IntPtr CertificateChain; + internal readonly uint SignerType; + internal readonly IntPtr SignerInfo; + internal readonly uint Error; + internal readonly uint CounterSignerCount; + internal readonly IntPtr CounterSigners; + internal readonly IntPtr ChainContext; + } + + [StructLayout(LayoutKind.Sequential)] + internal readonly struct NativeFileTime + { + internal readonly uint LowDateTime; + internal readonly uint HighDateTime; + } + + [StructLayout(LayoutKind.Sequential)] + internal readonly struct ByHandleFileInformation + { + internal readonly uint FileAttributes; + internal readonly NativeFileTime CreationTime; + internal readonly NativeFileTime LastAccessTime; + internal readonly NativeFileTime LastWriteTime; + internal readonly uint VolumeSerialNumber; + internal readonly uint FileSizeHigh; + internal readonly uint FileSizeLow; + internal readonly uint NumberOfLinks; + internal readonly uint FileIndexHigh; + internal readonly uint FileIndexLow; + } + + [StructLayout(LayoutKind.Sequential)] + internal readonly struct CryptProviderCertificate + { + internal readonly uint StructSize; + internal readonly IntPtr CertificateContext; + } + + [DllImport("kernel32.dll", SetLastError = true, CharSet = CharSet.Unicode)] + [return: MarshalAs(UnmanagedType.Bool)] + internal static extern bool QueryFullProcessImageName( + SafeProcessHandle process, + uint flags, + [Out] StringBuilder path, + ref int size); + + [LibraryImport("kernel32.dll", SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + internal static partial bool GetProcessTimes( + SafeProcessHandle process, + out long creation, + out long exit, + out long kernel, + out long user); + + [LibraryImport("kernel32.dll", SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + internal static partial bool ProcessIdToSessionId(int processId, out uint sessionId); + + [LibraryImport("kernel32.dll", SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + internal static partial bool GetExitCodeProcess(SafeProcessHandle process, out uint exitCode); + + [LibraryImport("kernel32.dll", SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + internal static partial bool GetFileInformationByHandle( + SafeFileHandle file, + out ByHandleFileInformation information); + + [LibraryImport("advapi32.dll", SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + internal static partial bool OpenProcessToken( + SafeProcessHandle process, + uint desiredAccess, + out SafeAccessTokenHandle token); + + [LibraryImport("advapi32.dll", SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + internal static partial bool GetTokenInformation( + SafeAccessTokenHandle token, + int informationClass, + IntPtr information, + uint informationLength, + out uint returnLength); + + [LibraryImport("advapi32.dll")] + [return: MarshalAs(UnmanagedType.Bool)] + internal static partial bool IsWellKnownSid(IntPtr sid, int wellKnownSidType); + + [LibraryImport("kernel32.dll", EntryPoint = "CreateFileW", SetLastError = true, StringMarshalling = StringMarshalling.Utf16)] + internal static partial SafeFileHandle CreateFile( + string fileName, + uint desiredAccess, + uint shareMode, + IntPtr securityAttributes, + uint creationDisposition, + uint flagsAndAttributes, + IntPtr templateFile); + + [LibraryImport("kernel32.dll", SetLastError = true)] + internal static partial SafeProcessHandle OpenProcess(uint desiredAccess, [MarshalAs(UnmanagedType.Bool)] bool inherit, int processId); + + [LibraryImport("ntdll.dll")] + internal static partial int NtQueryInformationProcess( + IntPtr process, + int informationClass, + ref IntPtr information, + int informationLength, + out int returnLength); + + [LibraryImport("wintrust.dll", SetLastError = true)] + internal static partial int WinVerifyTrust(IntPtr window, ref Guid action, IntPtr data); + + [LibraryImport("wintrust.dll")] + internal static partial IntPtr WTHelperProvDataFromStateData(IntPtr stateData); + + [LibraryImport("wintrust.dll")] + internal static partial IntPtr WTHelperGetProvSignerFromChain( + IntPtr providerData, + uint signerIndex, + [MarshalAs(UnmanagedType.Bool)] bool counterSigner, + uint counterSignerIndex); + + [LibraryImport("kernel32.dll", SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + internal static partial bool GetNamedPipeServerProcessId(SafePipeHandle pipe, out int processId); +} diff --git a/package/AgentPolicyConsent/PolicyConsentContract.cs b/package/AgentPolicyConsent/PolicyConsentContract.cs new file mode 100644 index 000000000..01789bbf5 --- /dev/null +++ b/package/AgentPolicyConsent/PolicyConsentContract.cs @@ -0,0 +1,13 @@ +namespace DevolutionsAgentPolicyConsent +{ + internal static class PolicyConsentContract + { + internal const string ProtocolVersion = "2.0"; + internal const string ExecutableName = "DevolutionsAgentPolicyConsent.exe"; + internal const string ProductName = "Devolutions Agent Policy Consent"; + internal const string CurrentUiSignerSpkiSha256 = + "e43ed3368eaabff61abc79eb338cba9da88a80d93b751735ff417f26afa579a8"; + internal const string TransitionUiSignerSpkiSha256 = + "99e7adb5894e242d87d32b8ad6cb5a1e0d2dd791a447bd7192c30189ef083fab"; + } +} diff --git a/package/AgentPolicyConsent/Program.cs b/package/AgentPolicyConsent/Program.cs new file mode 100644 index 000000000..2ec726a4f --- /dev/null +++ b/package/AgentPolicyConsent/Program.cs @@ -0,0 +1,173 @@ +using System.IO.Pipes; +using System.Security.Principal; +using System.Text.Json; + +namespace DevolutionsAgentPolicyConsent; + +internal static class Program +{ + private const int Success = 0; + private const int InvalidArguments = 10; + private const int ConnectionFailure = 11; + private const int PeerAuthenticationFailure = 12; + private const int ProtocolFailure = 13; + private const int UnexpectedFailure = 14; + + private static async Task Main(string[] args) + { + if (!OperatingSystem.IsWindows()) + { + return InvalidArguments; + } + + Arguments arguments; + try + { + arguments = Protocol.ParseArguments(args); + } + catch (ProtocolException) + { + return InvalidArguments; + } + + using CancellationTokenSource handshake = new(Protocol.ConnectTimeout); + PeerLease peer; + try + { + peer = await OpenPeerAsync(arguments, handshake.Token); + } + catch (OperationCanceledException) + { + return ConnectionFailure; + } + catch + { + return PeerAuthenticationFailure; + } + + using (peer) + using (NamedPipeClientStream pipe = new( + ".", + arguments.PipeName, + PipeDirection.InOut, + PipeOptions.Asynchronous | PipeOptions.WriteThrough, + TokenImpersonationLevel.Anonymous)) + { + try + { + await pipe.ConnectAsync(handshake.Token); + if (!Native.GetNamedPipeServerProcessId(pipe.SafePipeHandle, out int serverProcessId)) + { + return PeerAuthenticationFailure; + } + peer.VerifyConnectedServer(serverProcessId); + byte[] body = await Protocol.ReadFrameAsync(pipe, Protocol.MaxRequestBodyBytes, handshake.Token); + ElevationRequest request = JsonSerializer.Deserialize(body, ProtocolJsonContext.Default.ElevationRequest) + ?? throw new ProtocolException("request is null"); + Protocol.ValidateRequest(request); + peer.VerifyConnectedServer(serverProcessId); + + using CancellationTokenSource exchange = new(Protocol.ExchangeTimeout); + using CancellationTokenSource brokerCancellation = + CancellationTokenSource.CreateLinkedTokenSource(exchange.Token); + using CancellationTokenSource monitorCancellation = + CancellationTokenSource.CreateLinkedTokenSource(exchange.Token); + Task monitor = MonitorHostAsync(pipe, brokerCancellation, monitorCancellation.Token); + ElevationResponse response = await BrokerClient.ReplaceAsync(request, brokerCancellation.Token); + monitorCancellation.Cancel(); + await IgnoreCancellationAsync(monitor); + Protocol.ValidateResponse(response); + + byte[] responseBody = JsonSerializer.SerializeToUtf8Bytes( + response, + ProtocolJsonContext.Default.ElevationResponse); + using CancellationTokenSource responseWrite = new(Protocol.ResponseWriteTimeout); + await Protocol.WriteFrameAsync( + pipe, + responseBody, + Protocol.MaxResponseBodyBytes, + responseWrite.Token); + return Success; + } + catch (OperationCanceledException) + { + return ConnectionFailure; + } + catch (IOException) + { + return ConnectionFailure; + } + catch (ProtocolException) + { + return ProtocolFailure; + } + catch (JsonException) + { + return ProtocolFailure; + } + catch + { + return UnexpectedFailure; + } + } + } + + private static async Task OpenPeerAsync(Arguments arguments, CancellationToken cancellationToken) + { + Task open = Task.Run(() => PeerLease.Open(arguments)); + try + { + return await open.WaitAsync(cancellationToken); + } + catch + { + _ = open.ContinueWith( + static completed => + { + if (completed.Status == TaskStatus.RanToCompletion) + { + completed.Result.Dispose(); + } + _ = completed.Exception; + }, + CancellationToken.None, + TaskContinuationOptions.ExecuteSynchronously, + TaskScheduler.Default); + throw; + } + } + + private static async Task MonitorHostAsync( + Stream pipe, + CancellationTokenSource brokerCancellation, + CancellationToken cancellationToken) + { + byte[] unexpected = new byte[1]; + try + { + int read = await pipe.ReadAsync(unexpected, cancellationToken); + if (read is 0 or 1) + { + brokerCancellation.Cancel(); + } + } + catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) + { + } + catch (IOException) + { + brokerCancellation.Cancel(); + } + } + + private static async Task IgnoreCancellationAsync(Task task) + { + try + { + await task; + } + catch (OperationCanceledException) + { + } + } +} diff --git a/package/AgentPolicyConsent/Protocol.cs b/package/AgentPolicyConsent/Protocol.cs new file mode 100644 index 000000000..04184f3dc --- /dev/null +++ b/package/AgentPolicyConsent/Protocol.cs @@ -0,0 +1,240 @@ +using System.Buffers.Binary; +using System.Globalization; +using System.Text.Json; +using System.Text.Json.Serialization; + +namespace DevolutionsAgentPolicyConsent; + +internal static class Protocol +{ + internal const string Version = PolicyConsentContract.ProtocolVersion; + internal const string PipePrefix = "UniGetUI.PolicyElevation."; + internal const int MaxRequestBodyBytes = 16_793_054; + internal const int MaxResponseBodyBytes = 15_618; + internal static readonly TimeSpan ConnectTimeout = TimeSpan.FromSeconds(45); + internal static readonly TimeSpan ExchangeTimeout = TimeSpan.FromMinutes(2); + internal static readonly TimeSpan ResponseWriteTimeout = TimeSpan.FromSeconds(10); + + internal static Arguments ParseArguments(string[] args) + { + if (args.Length != 10) + { + throw new ProtocolException("invalid argument count"); + } + + Dictionary values = new(StringComparer.Ordinal); + for (int index = 0; index < args.Length; index += 2) + { + if (!values.TryAdd(args[index], args[index + 1])) + { + throw new ProtocolException("duplicate argument"); + } + } + + string protocol = Required(values, "--protocol"); + string pipe = Required(values, "--pipe"); + if (protocol != Version || + !pipe.StartsWith(PipePrefix, StringComparison.Ordinal) || + !IsLowerHex(pipe.AsSpan(PipePrefix.Length), 32) || + !int.TryParse( + Required(values, "--parent-pid"), + NumberStyles.None, + CultureInfo.InvariantCulture, + out int parentPid) || + parentPid <= 0 || + !long.TryParse( + Required(values, "--parent-created"), + NumberStyles.None, + CultureInfo.InvariantCulture, + out long parentCreated) || + parentCreated <= 0 || + !uint.TryParse( + Required(values, "--session"), + NumberStyles.None, + CultureInfo.InvariantCulture, + out uint session) || + values.Count != 5) + { + throw new ProtocolException("invalid argument value"); + } + + return new Arguments(pipe, parentPid, parentCreated, session); + } + + internal static void ValidateRequest(ElevationRequest request) + { + if (request.ProtocolVersion != Version || + !IsLowerHex(request.RequestId.AsSpan(), 32) || + request.Operation is not ("Update" or "ReplaceIdentity" or "Create" or "Repair") || + request.ConflictHandling is not ("Reject" or "ConfirmOverwrite") || + !IsCredential(request.ExpectedStoreToken, 512) || + !IsCredential(request.ValidationReceipt, 2048) || + request.Draft.ValueKind != JsonValueKind.Object) + { + throw new ProtocolException("invalid request"); + } + } + + internal static void ValidateResponse(ElevationResponse response) + { + if (response.ProtocolVersion != Version || + !IsLowerHex(response.RequestId.AsSpan(), 32) || + response.Disposition is not ("Committed" or "Rejected" or "Unknown") || + !IsOptionalCredential(response.BrokerErrorCode, 64)) + { + throw new ProtocolException("invalid response"); + } + + bool hasConflict = + response.ConflictStoreToken is not null || + response.ConflictState is not null || + response.ConflictPolicyId is not null; + switch (response.Disposition) + { + case "Committed" when + response.BrokerStatusCode is null && + response.BrokerErrorCode is null && + IsCredential(response.CommittedStoreToken, 512) && + !hasConflict: + return; + case "Rejected" when + response.CommittedStoreToken is null && + response.BrokerErrorCode is not null: + ValidateConflict(response, hasConflict); + return; + case "Unknown" when + response.CommittedStoreToken is null && + response.BrokerErrorCode is not null && + !hasConflict: + return; + default: + throw new ProtocolException("invalid response shape"); + } + } + + internal static async Task ReadFrameAsync(Stream stream, int maximum, CancellationToken cancellationToken) + { + byte[] header = new byte[4]; + await ReadExactlyAsync(stream, header, cancellationToken); + uint length = BinaryPrimitives.ReadUInt32BigEndian(header); + if (length == 0 || length > maximum) + { + throw new ProtocolException("invalid frame length"); + } + + byte[] body = GC.AllocateUninitializedArray(checked((int)length)); + await ReadExactlyAsync(stream, body, cancellationToken); + return body; + } + + internal static async Task WriteFrameAsync( + Stream stream, + ReadOnlyMemory body, + int maximum, + CancellationToken cancellationToken) + { + if (body.IsEmpty || body.Length > maximum) + { + throw new ProtocolException("invalid frame length"); + } + + byte[] header = new byte[4]; + BinaryPrimitives.WriteUInt32BigEndian(header, checked((uint)body.Length)); + await stream.WriteAsync(header, cancellationToken); + await stream.WriteAsync(body, cancellationToken); + await stream.FlushAsync(cancellationToken); + } + + private static async Task ReadExactlyAsync(Stream stream, Memory buffer, CancellationToken cancellationToken) + { + int offset = 0; + while (offset < buffer.Length) + { + int read = await stream.ReadAsync(buffer[offset..], cancellationToken); + if (read == 0) + { + throw new EndOfStreamException("unexpected end of elevation frame"); + } + offset += read; + } + } + + private static string Required(Dictionary values, string key) => + values.TryGetValue(key, out string? value) && value.Length != 0 + ? value + : throw new ProtocolException("missing argument"); + + private static bool IsLowerHex(ReadOnlySpan value, int length) => + value.Length == length && value.IndexOfAnyExcept("0123456789abcdef") < 0; + + internal static bool IsCredential(string? value, int maximum) => + value is not null && + value.Length is > 0 && + value.Length <= maximum && + IsAsciiAlphaNumeric(value[0]) && + value.AsSpan(1).IndexOfAnyExceptInRange('!', '~') < 0; + + private static bool IsOptionalCredential(string? value, int maximum) => + value is null || IsCredential(value, maximum); + + private static void ValidateConflict(ElevationResponse response, bool hasConflict) + { + if (response.BrokerErrorCode != "StalePolicyStoreToken") + { + if (hasConflict) + { + throw new ProtocolException("non-stale response carries conflict fields"); + } + return; + } + + if (!IsCredential(response.ConflictStoreToken, 512) || + response.ConflictState is not ("Active" or "Missing" or "Invalid") || + (response.ConflictState == "Active" + ? !IsCredential(response.ConflictPolicyId, 2048) + : response.ConflictPolicyId is not null)) + { + throw new ProtocolException("invalid stale conflict"); + } + } + + private static bool IsAsciiAlphaNumeric(char value) => + value is >= '0' and <= '9' or >= 'A' and <= 'Z' or >= 'a' and <= 'z'; +} + +internal sealed record Arguments(string PipeName, int ParentProcessId, long ParentCreatedUtcTicks, uint SessionId); + +[JsonUnmappedMemberHandling(JsonUnmappedMemberHandling.Disallow)] +internal sealed record ElevationRequest( + [property: JsonRequired] string ProtocolVersion, + [property: JsonRequired] string RequestId, + [property: JsonRequired] string Operation, + [property: JsonRequired] string ConflictHandling, + [property: JsonRequired] string ExpectedStoreToken, + [property: JsonRequired] string ValidationReceipt, + [property: JsonRequired] bool WarningsAcknowledged, + [property: JsonRequired] JsonElement Draft); + +[JsonUnmappedMemberHandling(JsonUnmappedMemberHandling.Disallow)] +internal sealed record ElevationResponse( + [property: JsonRequired] string ProtocolVersion, + [property: JsonRequired] string RequestId, + [property: JsonRequired] string Disposition, + [property: JsonRequired] int? BrokerStatusCode, + [property: JsonRequired] string? BrokerErrorCode, + [property: JsonRequired] string? CommittedStoreToken, + [property: JsonRequired] string? ConflictStoreToken, + [property: JsonRequired] string? ConflictState, + [property: JsonRequired] string? ConflictPolicyId); + +[JsonSourceGenerationOptions( + PropertyNamingPolicy = JsonKnownNamingPolicy.CamelCase, + PropertyNameCaseInsensitive = false, + UnmappedMemberHandling = JsonUnmappedMemberHandling.Disallow, + DefaultIgnoreCondition = JsonIgnoreCondition.Never, + GenerationMode = JsonSourceGenerationMode.Metadata)] +[JsonSerializable(typeof(ElevationRequest))] +[JsonSerializable(typeof(ElevationResponse))] +internal sealed partial class ProtocolJsonContext : JsonSerializerContext; + +internal sealed class ProtocolException(string message) : Exception(message); diff --git a/package/AgentPolicyConsent/app.manifest b/package/AgentPolicyConsent/app.manifest new file mode 100644 index 000000000..d303ea813 --- /dev/null +++ b/package/AgentPolicyConsent/app.manifest @@ -0,0 +1,11 @@ + + + + + + + + + + + diff --git a/package/AgentWindowsManaged.Tests/PackageBrokerInstallerTests.cs b/package/AgentWindowsManaged.Tests/PackageBrokerInstallerTests.cs index 3e7c19b01..2549938ff 100644 --- a/package/AgentWindowsManaged.Tests/PackageBrokerInstallerTests.cs +++ b/package/AgentWindowsManaged.Tests/PackageBrokerInstallerTests.cs @@ -1,5 +1,6 @@ using DevolutionsAgent; using DevolutionsAgent.Actions; +using DevolutionsAgent.Resources; using Microsoft.Deployment.WindowsInstaller; using System; using System.ComponentModel; @@ -620,6 +621,52 @@ public void MigrationActionsUseDeferredRollbackCommitSequence() Assert.Equal(Condition.NOT_BeingRemoved.ToString(), migrate.Condition.ToString()); } + [Theory] + [InlineData(false)] + [InlineData(true)] + public void PolicyConsentDiscoveryIsTransactionalAndArchitectureCorrect(bool win64) + { + RegValue value = Program.CreatePolicyConsentRegistryValue( + "ProtocolVersion", + DevolutionsAgent.Resources.Includes.POLICY_CONSENT_PROTOCOL_VERSION, + win64); + + Assert.Equal(RegistryHive.LocalMachine, value.Root); + Assert.Equal(@"Software\Devolutions\Agent\PolicyConsentHelper", value.Key); + Assert.Equal(RegistryKeyAction.createAndRemoveOnUninstall, value.RegistryKeyAction); + Assert.Equal(win64, value.Win64); + Assert.Contains(Features.AGENT_FEATURE, value.ActualFeatures); + } + + [Theory] + [InlineData(Platform.x86, false)] + [InlineData(Platform.x64, true)] + [InlineData(Platform.arm64, true)] + public void PolicyConsentDiscoveryUsesNativeRegistryView(Platform platform, bool expected) + { + Assert.Equal(expected, Program.Use64BitRegistryView(platform)); + } + + [Fact] + public void PolicyConsentDiscoveryPublishesFixedProtectedHelperIdentity() + { + (string Name, string Value)[] values = + [ + ("ProtocolVersion", DevolutionsAgent.Resources.Includes.POLICY_CONSENT_PROTOCOL_VERSION), + ("ExecutableName", DevolutionsAgent.Resources.Includes.POLICY_CONSENT_EXECUTABLE_NAME), + ("ExecutablePath", "[INSTALLDIR]DevolutionsAgentPolicyConsent.exe"), + ("ProductName", DevolutionsAgent.Resources.Includes.POLICY_CONSENT_PRODUCT_NAME), + ]; + + foreach ((string name, string expectedValue) in values) + { + RegValue value = Program.CreatePolicyConsentRegistryValue(name, expectedValue, true); + Assert.Equal(expectedValue, value.Value); + Assert.Equal(RegistryKeyAction.createAndRemoveOnUninstall, value.RegistryKeyAction); + Assert.Contains(Features.AGENT_FEATURE, value.ActualFeatures); + } + } + [Theory] [InlineData("marker inspection")] [InlineData("marker deletion")] diff --git a/package/AgentWindowsManaged/DevolutionsAgent.csproj b/package/AgentWindowsManaged/DevolutionsAgent.csproj index 527dd4bd4..6df8d0103 100644 --- a/package/AgentWindowsManaged/DevolutionsAgent.csproj +++ b/package/AgentWindowsManaged/DevolutionsAgent.csproj @@ -6,6 +6,7 @@ latest + diff --git a/package/AgentWindowsManaged/Program.cs b/package/AgentWindowsManaged/Program.cs index 09a32b242..f3173223e 100644 --- a/package/AgentWindowsManaged/Program.cs +++ b/package/AgentWindowsManaged/Program.cs @@ -77,6 +77,10 @@ private static string ResolveArtifact(string varName, string defaultPath = null) private static string DevolutionsAgentExePath => ResolveArtifact("DAGENT_EXECUTABLE", "..\\..\\target\\debug\\devolutions-agent.exe"); + private static string DevolutionsAgentPolicyConsentPath => ResolveArtifact( + "DAGENT_POLICY_CONSENT_HELPER", + "..\\AgentPolicyConsent\\bin\\Release\\net10.0-windows\\win-x64\\publish\\DevolutionsAgentPolicyConsent.exe"); + private static string DevolutionsDesktopAgentPath { // ReSharper disable once ArrangeAccessorOwnerBody @@ -123,6 +127,12 @@ private static Version DevolutionsAgentVersion } } + // The MSI version drops the "20" prefix; discovery restores the calendar-year product version. + private static Version DevolutionsAgentProductVersion => new( + DevolutionsAgentVersion.Major + 2000, + DevolutionsAgentVersion.Minor, + DevolutionsAgentVersion.Build); + private static WixSharp.Platform TargetPlatform { get @@ -298,6 +308,10 @@ static void Main() new (Features.AGENT_FEATURE, DevolutionsMultiPwshExe) { TargetFileName = "multi-pwsh.exe" + }, + new (Features.AGENT_FEATURE, DevolutionsAgentPolicyConsentPath) + { + TargetFileName = Includes.POLICY_CONSENT_EXECUTABLE_NAME } }, Dirs = new[] @@ -325,6 +339,34 @@ static void Main() Win64 = project.Platform == Platform.x64, RegistryKeyAction = RegistryKeyAction.create, }, + CreatePolicyConsentRegistryValue( + "ProtocolVersion", + Includes.POLICY_CONSENT_PROTOCOL_VERSION, + Use64BitRegistryView(project.Platform)), + CreatePolicyConsentRegistryValue( + "ExecutableName", + Includes.POLICY_CONSENT_EXECUTABLE_NAME, + Use64BitRegistryView(project.Platform)), + CreatePolicyConsentRegistryValue( + "ExecutablePath", + $"[{AgentProperties.InstallDir}]{Includes.POLICY_CONSENT_EXECUTABLE_NAME}", + Use64BitRegistryView(project.Platform)), + CreatePolicyConsentRegistryValue( + "ProductName", + Includes.POLICY_CONSENT_PRODUCT_NAME, + Use64BitRegistryView(project.Platform)), + CreatePolicyConsentRegistryValue( + "ProductVersion", + DevolutionsAgentProductVersion.ToString(), + Use64BitRegistryView(project.Platform)), + CreatePolicyConsentRegistryValue( + "CurrentUiSignerSpkiSha256", + Includes.POLICY_CONSENT_CURRENT_UI_SIGNER_SPKI_SHA256, + Use64BitRegistryView(project.Platform)), + CreatePolicyConsentRegistryValue( + "TransitionUiSignerSpkiSha256", + Includes.POLICY_CONSENT_TRANSITION_UI_SIGNER_SPKI_SHA256, + Use64BitRegistryView(project.Platform)), new (RegistryHive.LocalMachine, "SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\Run", Includes.SERVICE_NAME, $"[{AgentProperties.InstallDir}]{Includes.DESKTOP_DIRECTORY_NAME}\\{Includes.DESKTOP_EXECUTABLE_NAME}") { Win64 = project.Platform == Platform.x64, @@ -435,6 +477,23 @@ internal static RegValue CreateEventLogSourceRegistryValue(bool win64) => RegistryKeyAction = RegistryKeyAction.createAndRemoveOnUninstall, }; + internal static RegValue CreatePolicyConsentRegistryValue(string name, string value, bool win64) => + new( + RegistryHive.LocalMachine, + $"Software\\{Includes.VENDOR_NAME}\\{Includes.SHORT_NAME}\\PolicyConsentHelper", + name, + value) + { + AttributesDefinition = "Type=string", + Win64 = win64, + RegistryKeyAction = RegistryKeyAction.createAndRemoveOnUninstall, + Feature = Features.AGENT_FEATURE, + }; + + // Discovery follows the consumer's native view; legacy Agent values retain their existing layout. + internal static bool Use64BitRegistryView(Platform? platform) => + platform is Platform.x64 or Platform.arm64; + private static void Project_UnhandledException(ExceptionEventArgs e) { string errorMessage = diff --git a/package/AgentWindowsManaged/Resources/Includes.cs b/package/AgentWindowsManaged/Resources/Includes.cs index 667a1747c..bb5bab31f 100644 --- a/package/AgentWindowsManaged/Resources/Includes.cs +++ b/package/AgentWindowsManaged/Resources/Includes.cs @@ -18,6 +18,21 @@ internal static class Includes internal static readonly string EXECUTABLE_NAME = "DevolutionsAgent.exe"; + internal static readonly string POLICY_CONSENT_EXECUTABLE_NAME = + DevolutionsAgentPolicyConsent.PolicyConsentContract.ExecutableName; + + internal static readonly string POLICY_CONSENT_PRODUCT_NAME = + DevolutionsAgentPolicyConsent.PolicyConsentContract.ProductName; + + internal static readonly string POLICY_CONSENT_PROTOCOL_VERSION = + DevolutionsAgentPolicyConsent.PolicyConsentContract.ProtocolVersion; + + internal static readonly string POLICY_CONSENT_CURRENT_UI_SIGNER_SPKI_SHA256 = + DevolutionsAgentPolicyConsent.PolicyConsentContract.CurrentUiSignerSpkiSha256; + + internal static readonly string POLICY_CONSENT_TRANSITION_UI_SIGNER_SPKI_SHA256 = + DevolutionsAgentPolicyConsent.PolicyConsentContract.TransitionUiSignerSpkiSha256; + internal static readonly string DESKTOP_DIRECTORY_NAME = "desktop"; internal static readonly string DESKTOP_EXECUTABLE_NAME = "DevolutionsDesktopAgent.exe"; From 79c27fae5d5be68e5193fee8f7ca5513b50b93dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Wed, 9 Sep 2026 16:10:01 -0400 Subject: [PATCH 2/5] fix(agent): enforce signer revocation checks Require fresh whole-chain WinTrust revocation status before authorizing the UniGetUI parent or Agent broker. Bound broker trust retrieval to the existing exchange timeout and reject all pre-dispatch authentication failures. Issue: #1963 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- ci/package-agent-windows.ps1 | 2 +- .../AgentPolicyConsent.Tests/ProtocolTests.cs | 23 ++++++++++ package/AgentPolicyConsent/BrokerClient.cs | 44 ++++++++++++++++++- package/AgentPolicyConsent/PeerTrust.cs | 13 ++++-- 4 files changed, 77 insertions(+), 5 deletions(-) diff --git a/ci/package-agent-windows.ps1 b/ci/package-agent-windows.ps1 index 10f0e7219..246cfe245 100644 --- a/ci/package-agent-windows.ps1 +++ b/ci/package-agent-windows.ps1 @@ -100,7 +100,7 @@ function New-AgentMsi() { # The path to the devolutions-session.exe file. [string] $SessionExe, [parameter(Mandatory = $true)] - # The path to the DevolutionsAgentPolicyConsent.exe file. + # The path to the signed DevolutionsAgentPolicyConsent.exe file. [string] $PolicyConsentHelper, [parameter(Mandatory = $true)] [ValidateSet('x64', 'arm64')] diff --git a/package/AgentPolicyConsent.Tests/ProtocolTests.cs b/package/AgentPolicyConsent.Tests/ProtocolTests.cs index 36456f288..e5e0a21ca 100644 --- a/package/AgentPolicyConsent.Tests/ProtocolTests.cs +++ b/package/AgentPolicyConsent.Tests/ProtocolTests.cs @@ -101,6 +101,29 @@ public void AuthenticodeSignerComesFromRetainedImageHandle() Assert.NotEmpty(certificate.RawData); } + [Fact] + public void AuthenticodeRequiresFreshWholeChainRevocation() + { + Native.WinTrustData data = new(IntPtr.Zero); + + Assert.Equal(Native.WtdRevokeWholeChain, data.RevocationChecks); + Assert.Equal( + Native.WtdRevocationCheckChain | Native.WtdDisableMd2Md4, + data.ProviderFlags); + Assert.Equal(0u, data.ProviderFlags & Native.WtdCacheOnlyUrlRetrieval); + } + + [Theory] + [InlineData(0, true)] + [InlineData(unchecked((int)0x800B010C), false)] + [InlineData(unchecked((int)0x80092012), false)] + [InlineData(unchecked((int)0x80092013), false)] + [InlineData(unchecked((int)0x800B010E), false)] + public void AuthenticodeFailsClosedForIndeterminateStatus(int status, bool accepted) + { + Assert.Equal(accepted, PeerLease.IsAuthenticodeStatusAccepted(status)); + } + [Fact] public void ProcessImageMappingAcceptsOnlyCurrentMappedImage() { diff --git a/package/AgentPolicyConsent/BrokerClient.cs b/package/AgentPolicyConsent/BrokerClient.cs index bdcbd5120..4a1d5affc 100644 --- a/package/AgentPolicyConsent/BrokerClient.cs +++ b/package/AgentPolicyConsent/BrokerClient.cs @@ -29,7 +29,7 @@ internal static async Task ReplaceAsync( using CancellationTokenSource connectTimeout = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); connectTimeout.CancelAfter(ConnectTimeout); await pipe.ConnectAsync(connectTimeout.Token); - using BrokerServerLease broker = BrokerServerLease.Open(pipe.SafePipeHandle); + using BrokerServerLease broker = await OpenBrokerServerAsync(pipe, cancellationToken); byte[] headers = Encoding.ASCII.GetBytes( $"PUT /v1/policy HTTP/1.1\r\nHost: now-package-broker\r\nConnection: close\r\n" + @@ -60,6 +60,10 @@ internal static async Task ReplaceAsync( { return Unknown(request.RequestId, error.StatusCode, "InvalidResponse"); } + catch (BrokerAuthenticationException) + { + return Rejected(request.RequestId, "Unauthorized"); + } catch (InvalidOperationException) { return Unknown(request.RequestId, null, "InvalidResponse"); @@ -239,6 +243,41 @@ private static async Task ReadBoundedAsync(Stream stream, int maximum, C private static ElevationResponse Unknown(string requestId, int? statusCode, string errorCode) => new(Protocol.Version, requestId, "Unknown", statusCode, errorCode, null, null, null, null); + private static ElevationResponse Rejected(string requestId, string errorCode) => + new(Protocol.Version, requestId, "Rejected", null, errorCode, null, null, null, null); + + private static async Task OpenBrokerServerAsync( + NamedPipeClientStream pipe, + CancellationToken cancellationToken) + { + Task open = Task.Run(() => BrokerServerLease.Open(pipe.SafePipeHandle)); + try + { + return await open.WaitAsync(cancellationToken); + } + catch (Exception error) + { + DisposeLateResult(open); + throw new BrokerAuthenticationException(error); + } + } + + private static void DisposeLateResult(Task open) + { + _ = open.ContinueWith( + static completed => + { + if (completed.Status == TaskStatus.RanToCompletion) + { + completed.Result.Dispose(); + } + _ = completed.Exception; + }, + CancellationToken.None, + TaskContinuationOptions.ExecuteSynchronously, + TaskScheduler.Default); + } + private static string Truncate(string value, int maximum) => value.Length <= maximum ? value : value[..maximum]; @@ -247,4 +286,7 @@ private sealed class BrokerResponseException(int? statusCode, Exception? innerEx { internal int? StatusCode { get; } = statusCode; } + + private sealed class BrokerAuthenticationException(Exception innerException) + : Exception("broker server authentication failed", innerException); } diff --git a/package/AgentPolicyConsent/PeerTrust.cs b/package/AgentPolicyConsent/PeerTrust.cs index 846d151d1..502742540 100644 --- a/package/AgentPolicyConsent/PeerTrust.cs +++ b/package/AgentPolicyConsent/PeerTrust.cs @@ -242,7 +242,7 @@ internal static X509Certificate2 VerifyAuthenticodeSigner( Marshal.StructureToPtr(data, dataPointer, false); dataInitialized = true; int status = Native.WinVerifyTrust(new IntPtr(-1), ref action, dataPointer); - if (status != 0) + if (!IsAuthenticodeStatusAccepted(status)) { throw new InvalidOperationException($"{subject} Authenticode validation failed (0x{status:X8})"); } @@ -289,6 +289,8 @@ internal static X509Certificate2 VerifyAuthenticodeSigner( } } + internal static bool IsAuthenticodeStatusAccepted(int status) => status == 0; + private static void VerifySigner(X509Certificate2 certificate) { byte[] subjectPublicKeyInfo; @@ -494,6 +496,11 @@ public void Dispose() internal static partial class Native { + internal const uint WtdRevokeWholeChain = 1; + internal const uint WtdRevocationCheckChain = 0x0000_0040; + internal const uint WtdCacheOnlyUrlRetrieval = 0x0000_1000; + internal const uint WtdDisableMd2Md4 = 0x0000_2000; + [StructLayout(LayoutKind.Sequential, CharSet = CharSet.Unicode)] internal readonly struct WinTrustFileInfo { @@ -535,13 +542,13 @@ internal WinTrustData(IntPtr fileInfo) PolicyCallbackData = IntPtr.Zero; SipClientData = IntPtr.Zero; UiChoice = 2; - RevocationChecks = 0; + RevocationChecks = WtdRevokeWholeChain; UnionChoice = 1; FileInfo = fileInfo; StateAction = 1; StateData = IntPtr.Zero; UrlReference = IntPtr.Zero; - ProviderFlags = 0x1000 | 0x2000; + ProviderFlags = WtdRevocationCheckChain | WtdDisableMd2Md4; UiContext = 0; SignatureSettings = IntPtr.Zero; } From 9f0edd4123c6a82f42d4f043df53c0e61d2dcf03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Thu, 10 Sep 2026 00:57:16 -0400 Subject: [PATCH 3/5] fix(agent): bind broker trust to running image Authenticate the connected Agent through its retained mapped image and an allowed Devolutions signer before sending policy data. Pin the protected installation directory chain against reparse and untrusted-writer replacement, including custom install locations, and record the evidence behind both UniGetUI signer pins. Issue: #1963 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../AgentPolicyConsent.Tests/ProtocolTests.cs | 44 ++++ package/AgentPolicyConsent/PeerTrust.cs | 236 +++++++++++++++++- .../PolicyConsentContract.cs | 20 ++ 3 files changed, 288 insertions(+), 12 deletions(-) diff --git a/package/AgentPolicyConsent.Tests/ProtocolTests.cs b/package/AgentPolicyConsent.Tests/ProtocolTests.cs index e5e0a21ca..07a87266e 100644 --- a/package/AgentPolicyConsent.Tests/ProtocolTests.cs +++ b/package/AgentPolicyConsent.Tests/ProtocolTests.cs @@ -1,4 +1,5 @@ using System.Buffers.Binary; +using System.Security.AccessControl; using System.Text.Json; using DevolutionsAgentPolicyConsent; using Microsoft.Win32.SafeHandles; @@ -41,6 +42,46 @@ public void LookalikeSignerIsRejected() Assert.False(PeerLease.IsAllowedSigner(new string('0', 64))); } + [Fact] + public void AgentSignerRequiresKnownDevolutionsCertificate() + { + Assert.All( + PolicyConsentContract.DevolutionsSignerSha1Thumbprints, + thumbprint => Assert.True(PeerLease.IsAllowedDevolutionsSigner(thumbprint))); + Assert.False(PeerLease.IsAllowedDevolutionsSigner(new string('0', 40))); + } + + [Theory] + [InlineData("O:SYG:SYD:(A;;FA;;;SY)(A;;FA;;;BA)(A;;0x1200A9;;;BU)", PeerLease.FileTamperRights, true)] + [InlineData("O:BUG:SYD:(A;;FA;;;SY)(A;;FA;;;BA)", PeerLease.FileTamperRights, false)] + [InlineData("O:SYG:SYD:(A;;FA;;;SY)(A;;FA;;;BA)(A;;GW;;;BU)", PeerLease.FileTamperRights, false)] + [InlineData("O:SYG:SYD:(A;;FA;;;SY)(A;;FA;;;BA)(A;;0x6;;;AU)", PeerLease.ParentDirectoryTamperRights, false)] + [InlineData("O:SYG:SYD:(A;;FA;;;SY)(A;;FA;;;BA)(A;;0x6;;;AU)", PeerLease.AncestorDirectoryTamperRights, true)] + [InlineData("O:SYG:SYD:(A;;FA;;;SY)(A;;FA;;;BA)(A;;0x40;;;BU)", PeerLease.AncestorDirectoryTamperRights, false)] + public void ProtectedAgentPathRequiresTrustedOwnerAndWriters( + string sddl, + int tamperRights, + bool accepted) + { + RawSecurityDescriptor descriptor = new(sddl); + if (accepted) + { + PeerLease.VerifyTrustedSecurityDescriptor(descriptor, "test path", tamperRights); + } + else + { + Assert.Throws( + () => PeerLease.VerifyTrustedSecurityDescriptor(descriptor, "test path", tamperRights)); + } + } + + [Fact] + public void ProtectedAgentPathRejectsReparsePoints() + { + Assert.True(PeerLease.IsReparsePoint(PeerLease.FileAttributeReparsePoint)); + Assert.False(PeerLease.IsReparsePoint(0)); + } + [Fact] public void SignerMatchingIsCaseSensitive() { @@ -90,6 +131,9 @@ public void BrokerServerRequiresExactAgentSiblingPath() Assert.False(BrokerServerLease.IsExpectedPath( @"C:\Users\Alice\DevolutionsAgent.exe", @"C:\Program Files\Devolutions\Agent\DevolutionsAgent.exe")); + Assert.True(BrokerServerLease.IsExpectedPath( + @"D:\Managed Apps\Agent\DevolutionsAgent.exe", + @"d:\managed apps\Agent\DevolutionsAgent.exe")); } [Fact] diff --git a/package/AgentPolicyConsent/PeerTrust.cs b/package/AgentPolicyConsent/PeerTrust.cs index 502742540..9fc0eb204 100644 --- a/package/AgentPolicyConsent/PeerTrust.cs +++ b/package/AgentPolicyConsent/PeerTrust.cs @@ -2,8 +2,10 @@ using System.ComponentModel; using System.Diagnostics; using System.Runtime.InteropServices; +using System.Security.AccessControl; using System.Security.Cryptography; using System.Security.Cryptography.X509Certificates; +using System.Security.Principal; using System.Text; namespace DevolutionsAgentPolicyConsent; @@ -21,8 +23,14 @@ internal sealed class PeerLease : IDisposable internal const uint Synchronize = 0x0010_0000; internal const uint GenericRead = 0x8000_0000; internal const uint FileExecute = 0x20; + internal const uint FileReadAttributes = 0x80; + internal const uint ReadControl = 0x0002_0000; internal const uint FileShareRead = 0x1; + internal const uint FileShareWrite = 0x2; internal const uint OpenExisting = 3; + internal const uint FileAttributeReparsePoint = 0x400; + internal const uint FileFlagBackupSemantics = 0x0200_0000; + internal const uint FileFlagOpenReparsePoint = 0x0020_0000; internal const int ProcessImageFileMapping = 44; internal const uint StillActive = 259; @@ -122,6 +130,10 @@ internal static bool IsAllowedSigner(string digest) => FixedTimeEqualsHex(digest, CurrentUiSignerSpkiSha256) || FixedTimeEqualsHex(digest, TransitionUiSignerSpkiSha256); + internal static bool IsAllowedDevolutionsSigner(string thumbprint) => + PolicyConsentContract.DevolutionsSignerSha1Thumbprints.Any( + expected => FixedTimeEqualsHex(thumbprint, expected, 40)); + internal static bool IsSupportedUiIdentity(string? productName, string? originalFilename, string? productVersion) => string.Equals(productName, "UniGetUI", StringComparison.Ordinal) && string.Equals(originalFilename, "UniGetUI.dll", StringComparison.OrdinalIgnoreCase) && @@ -315,10 +327,10 @@ private static void VerifySigner(X509Certificate2 certificate) } } - private static bool FixedTimeEqualsHex(string candidate, string expected) + private static bool FixedTimeEqualsHex(string candidate, string expected, int length = 64) { - if (candidate.Length != 64 || - expected.Length != 64 || + if (candidate.Length != length || + expected.Length != length || candidate.AsSpan().IndexOfAnyExcept("0123456789abcdef") >= 0) { return false; @@ -350,6 +362,107 @@ internal static void VerifyImageMapping(SafeProcessHandle process, SafeFileHandl } } + internal static void VerifyProtectedPath(SafeFileHandle handle, string subject, int tamperRights) + { + if (!Native.GetFileInformationByHandle(handle, out Native.ByHandleFileInformation information)) + { + throw new Win32Exception(); + } + if (IsReparsePoint(information.FileAttributes)) + { + throw new InvalidOperationException($"{subject} is a reparse point"); + } + + uint error = Native.GetSecurityInfo( + handle, + 1, + 0x1 | 0x4, + out _, + out _, + out _, + out _, + out IntPtr securityDescriptor); + if (error != 0) + { + throw new Win32Exception(checked((int)error)); + } + + try + { + int length = checked((int)Native.GetSecurityDescriptorLength(securityDescriptor)); + byte[] bytes = new byte[length]; + Marshal.Copy(securityDescriptor, bytes, 0, length); + VerifyTrustedSecurityDescriptor(new RawSecurityDescriptor(bytes, 0), subject, tamperRights); + } + finally + { + _ = Native.LocalFree(securityDescriptor); + } + } + + internal static bool IsReparsePoint(uint attributes) => + (attributes & FileAttributeReparsePoint) != 0; + + internal const int FileTamperRights = + 0x0000_0002 | 0x0000_0004 | 0x0000_0010 | 0x0000_0100 | + 0x0001_0000 | 0x0004_0000 | 0x0008_0000 | 0x1000_0000 | 0x4000_0000; + internal const int ParentDirectoryTamperRights = + 0x0000_0002 | 0x0000_0004 | 0x0000_0040 | + 0x0001_0000 | 0x0004_0000 | 0x0008_0000 | 0x1000_0000 | 0x4000_0000; + internal const int AncestorDirectoryTamperRights = + 0x0000_0040 | 0x0001_0000 | 0x0004_0000 | 0x0008_0000 | 0x1000_0000; + + internal static void VerifyTrustedSecurityDescriptor( + RawSecurityDescriptor descriptor, + string subject, + int tamperRights) + { + if (descriptor.Owner is not SecurityIdentifier owner || !IsTrustedWriter(owner)) + { + throw new InvalidOperationException($"{subject} has an untrusted owner"); + } + if (!descriptor.ControlFlags.HasFlag(ControlFlags.DiscretionaryAclPresent) || + descriptor.DiscretionaryAcl is not { Count: > 0 } dacl) + { + throw new InvalidOperationException($"{subject} has no protective DACL"); + } + + foreach (GenericAce generic in dacl) + { + if (generic.AceFlags.HasFlag(AceFlags.InheritOnly) || !IsAccessAllowedAce(generic.AceType)) + { + continue; + } + if (generic is not QualifiedAce ace || ace is not KnownAce known) + { + throw new InvalidOperationException($"{subject} has an unsupported access-allowed entry"); + } + if ((known.AccessMask & tamperRights) == 0) + { + continue; + } + if (ace.SecurityIdentifier is null || !IsTrustedWriter(ace.SecurityIdentifier)) + { + throw new InvalidOperationException($"{subject} grants write access to an untrusted principal"); + } + } + } + + private static bool IsAccessAllowedAce(AceType type) => + type is AceType.AccessAllowed or + AceType.AccessAllowedCompound or + AceType.AccessAllowedObject or + AceType.AccessAllowedCallback or + AceType.AccessAllowedCallbackObject; + + private static bool IsTrustedWriter(SecurityIdentifier sid) => + sid.IsWellKnown(WellKnownSidType.LocalSystemSid) || + sid.IsWellKnown(WellKnownSidType.BuiltinAdministratorsSid) || + string.Equals( + sid.Value, + "S-1-5-80-956008885-3418522649-1831038044-1853292631-2271478464", + StringComparison.Ordinal); + internal static string ImagePath(SafeProcessHandle process) { int capacity = 260; @@ -407,11 +520,16 @@ internal sealed class BrokerServerLease : IDisposable private readonly SafeProcessHandle process; private readonly SafeFileHandle image; + private readonly List directories; - private BrokerServerLease(SafeProcessHandle process, SafeFileHandle image) + private BrokerServerLease( + SafeProcessHandle process, + SafeFileHandle image, + List directories) { this.process = process; this.image = image; + this.directories = directories; } internal static BrokerServerLease Open(SafePipeHandle pipe) @@ -422,7 +540,9 @@ internal static BrokerServerLease Open(SafePipeHandle pipe) } SafeProcessHandle process = Native.OpenProcess( - PeerLease.ProcessQueryLimitedInformation | PeerLease.Synchronize, + PeerLease.ProcessQueryInformation | + PeerLease.ProcessQueryLimitedInformation | + PeerLease.Synchronize, false, processId); if (process.IsInvalid) @@ -449,12 +569,15 @@ internal static BrokerServerLease Open(SafePipeHandle pipe) } SafeFileHandle image = Native.CreateFile( - expectedPath, - PeerLease.GenericRead | PeerLease.FileExecute | PeerLease.Synchronize, + serverPath, + PeerLease.GenericRead | + PeerLease.FileExecute | + PeerLease.ReadControl | + PeerLease.Synchronize, PeerLease.FileShareRead, IntPtr.Zero, PeerLease.OpenExisting, - 0, + PeerLease.FileFlagOpenReparsePoint, IntPtr.Zero); if (image.IsInvalid) { @@ -463,10 +586,37 @@ internal static BrokerServerLease Open(SafePipeHandle pipe) try { - using X509Certificate2 _ = - PeerLease.VerifyAuthenticodeSigner(expectedPath, image, "broker server"); - PeerLease.EnsureActive(process); - return new BrokerServerLease(process, image); + PeerLease.VerifyImageMapping(process, image); + PeerLease.VerifyProtectedPath(image, "broker server image", PeerLease.FileTamperRights); + using X509Certificate2 signer = + PeerLease.VerifyAuthenticodeSigner(serverPath, image, "broker server"); + string thumbprint = signer.GetCertHashString(HashAlgorithmName.SHA1).ToLowerInvariant(); + if (!PeerLease.IsAllowedDevolutionsSigner(thumbprint)) + { + throw new InvalidOperationException("broker server signer is not authorized"); + } + List? directories = RetainProtectedDirectories( + Path.GetDirectoryName(expectedPath) + ?? throw new InvalidOperationException("Agent installation directory is unavailable")); + try + { + PeerLease.VerifyImageMapping(process, image); + PeerLease.EnsureActive(process); + if (!Native.GetNamedPipeServerProcessId(pipe, out int confirmedProcessId) || + confirmedProcessId != processId) + { + throw new InvalidOperationException("broker server process changed during authentication"); + } + return new BrokerServerLease(process, image, directories); + } + catch + { + foreach (SafeFileHandle directory in directories) + { + directory.Dispose(); + } + throw; + } } catch { @@ -489,9 +639,54 @@ internal static bool IsExpectedPath(string actual, string expected) => public void Dispose() { + foreach (SafeFileHandle directory in directories) + { + directory.Dispose(); + } image.Dispose(); process.Dispose(); } + + private static List RetainProtectedDirectories(string installationDirectory) + { + List handles = []; + try + { + int tamperRights = PeerLease.ParentDirectoryTamperRights; + for (DirectoryInfo? directory = new(Path.GetFullPath(installationDirectory)); + directory is not null; + directory = directory.Parent) + { + SafeFileHandle handle = Native.CreateFile( + directory.FullName, + PeerLease.FileReadAttributes | PeerLease.ReadControl | PeerLease.Synchronize, + PeerLease.FileShareRead | PeerLease.FileShareWrite, + IntPtr.Zero, + PeerLease.OpenExisting, + PeerLease.FileFlagBackupSemantics | PeerLease.FileFlagOpenReparsePoint, + IntPtr.Zero); + if (handle.IsInvalid) + { + throw new Win32Exception(); + } + handles.Add(handle); + PeerLease.VerifyProtectedPath( + handle, + $"Agent installation directory '{directory.FullName}'", + tamperRights); + tamperRights = PeerLease.AncestorDirectoryTamperRights; + } + return handles; + } + catch + { + foreach (SafeFileHandle handle in handles) + { + handle.Dispose(); + } + throw; + } + } } internal static partial class Native @@ -649,6 +844,23 @@ internal static partial bool GetTokenInformation( [return: MarshalAs(UnmanagedType.Bool)] internal static partial bool IsWellKnownSid(IntPtr sid, int wellKnownSidType); + [LibraryImport("advapi32.dll", SetLastError = true)] + internal static partial uint GetSecurityInfo( + SafeFileHandle handle, + uint objectType, + uint securityInformation, + out IntPtr owner, + out IntPtr group, + out IntPtr dacl, + out IntPtr sacl, + out IntPtr securityDescriptor); + + [LibraryImport("advapi32.dll")] + internal static partial uint GetSecurityDescriptorLength(IntPtr securityDescriptor); + + [LibraryImport("kernel32.dll")] + internal static partial IntPtr LocalFree(IntPtr memory); + [LibraryImport("kernel32.dll", EntryPoint = "CreateFileW", SetLastError = true, StringMarshalling = StringMarshalling.Utf16)] internal static partial SafeFileHandle CreateFile( string fileName, diff --git a/package/AgentPolicyConsent/PolicyConsentContract.cs b/package/AgentPolicyConsent/PolicyConsentContract.cs index 01789bbf5..697d606d3 100644 --- a/package/AgentPolicyConsent/PolicyConsentContract.cs +++ b/package/AgentPolicyConsent/PolicyConsentContract.cs @@ -5,9 +5,29 @@ internal static class PolicyConsentContract internal const string ProtocolVersion = "2.0"; internal const string ExecutableName = "DevolutionsAgentPolicyConsent.exe"; internal const string ProductName = "Devolutions Agent Policy Consent"; + + // UniGetUI 2026.2.7: subject CN=Devolutions Inc, O=Devolutions Inc, C=CA; + // issuer CN=GlobalSign GCC R45 EV CodeSigning CA 2020, O=GlobalSign nv-sa, C=BE; + // serial 73D3C33603FF8BB44224F25E, SHA-1 8DB5A43BB8AFE4D2FFB92DA9007D8997A4CC4E13, + // valid 2023-10-30T17:51:18Z through 2026-10-30T17:51:18Z. internal const string CurrentUiSignerSpkiSha256 = "e43ed3368eaabff61abc79eb338cba9da88a80d93b751735ff417f26afa579a8"; + + // UniGetUI 3.3.7: subject CN="Open Source Developer, Martí Climent López", + // O=Open Source Developer, C=ES; issuer CN=Certum Code Signing 2021 CA, + // O=Asseco Data Systems S.A., C=PL; + // serial 1AC2CAA58AF100E402D9812002C08B30, SHA-1 28949703053434989162B12C101497DE35FE4E8E, + // valid 2025-06-24T18:02:38Z through 2026-06-24T18:02:37Z. + // Remove this transition pin when the minimum supported UniGetUI version postdates its last signed release. internal const string TransitionUiSignerSpkiSha256 = "99e7adb5894e242d87d32b8ad6cb5a1e0d2dd791a447bd7192c30189ef083fab"; + + // Keep synchronized with devolutions-agent-shared/src/windows/code_signing.rs. + internal static readonly string[] DevolutionsSignerSha1Thumbprints = + [ + "3f5202a9432d54293bdfe6f7e46adb0a6f8b3ba6", + "8db5a43bb8afe4d2ffb92da9007d8997a4cc4e13", + "50f753333811ff11f1920274afde3ffd4468b210", + ]; } } From e0909505561cb599def8e77fad1f1a9b0b65d3e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Thu, 10 Sep 2026 01:52:23 -0400 Subject: [PATCH 4/5] fix(agent,agent-installer): correct helper packaging Preserve timeout responses during broker authentication, accept prerelease UniGetUI versions, and suppress the helper console window. Mark ARM64 discovery registry components as 64-bit so WiX can package them beneath ProgramFiles64Folder. Issue: #1963 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- package/AgentPolicyConsent.Tests/ProtocolTests.cs | 1 + package/AgentPolicyConsent/BrokerClient.cs | 5 +++++ .../AgentPolicyConsent/DevolutionsAgentPolicyConsent.csproj | 2 +- package/AgentPolicyConsent/PeerTrust.cs | 2 +- .../AgentWindowsManaged.Tests/PackageBrokerInstallerTests.cs | 3 +++ package/AgentWindowsManaged/Program.cs | 2 +- 6 files changed, 12 insertions(+), 3 deletions(-) diff --git a/package/AgentPolicyConsent.Tests/ProtocolTests.cs b/package/AgentPolicyConsent.Tests/ProtocolTests.cs index 07a87266e..f3bdbb5ff 100644 --- a/package/AgentPolicyConsent.Tests/ProtocolTests.cs +++ b/package/AgentPolicyConsent.Tests/ProtocolTests.cs @@ -91,6 +91,7 @@ public void SignerMatchingIsCaseSensitive() [Theory] [InlineData("3.3.7")] [InlineData("2026.2.7")] + [InlineData("2026.2.7-preview")] public void ProductBindingSupportsProtocolEraAndCurrentInstallModes(string version) { Assert.True(PeerLease.IsSupportedUiIdentity("UniGetUI", "UniGetUI.dll", version)); diff --git a/package/AgentPolicyConsent/BrokerClient.cs b/package/AgentPolicyConsent/BrokerClient.cs index 4a1d5affc..1dfe80305 100644 --- a/package/AgentPolicyConsent/BrokerClient.cs +++ b/package/AgentPolicyConsent/BrokerClient.cs @@ -255,6 +255,11 @@ private static async Task OpenBrokerServerAsync( { return await open.WaitAsync(cancellationToken); } + catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) + { + DisposeLateResult(open); + throw; + } catch (Exception error) { DisposeLateResult(open); diff --git a/package/AgentPolicyConsent/DevolutionsAgentPolicyConsent.csproj b/package/AgentPolicyConsent/DevolutionsAgentPolicyConsent.csproj index 5e5e414e4..a8257d4dc 100644 --- a/package/AgentPolicyConsent/DevolutionsAgentPolicyConsent.csproj +++ b/package/AgentPolicyConsent/DevolutionsAgentPolicyConsent.csproj @@ -1,6 +1,6 @@ - Exe + WinExe net10.0-windows win-x64 true diff --git a/package/AgentPolicyConsent/PeerTrust.cs b/package/AgentPolicyConsent/PeerTrust.cs index 9fc0eb204..2948e109e 100644 --- a/package/AgentPolicyConsent/PeerTrust.cs +++ b/package/AgentPolicyConsent/PeerTrust.cs @@ -138,7 +138,7 @@ internal static bool IsSupportedUiIdentity(string? productName, string? original string.Equals(productName, "UniGetUI", StringComparison.Ordinal) && string.Equals(originalFilename, "UniGetUI.dll", StringComparison.OrdinalIgnoreCase) && productVersion is not null && - Version.TryParse(productVersion.Split('+', '-', StringSplitOptions.TrimEntries)[0], out Version? parsed) && + Version.TryParse(productVersion.Split(['+', '-'], StringSplitOptions.TrimEntries)[0], out Version? parsed) && parsed >= new Version(3, 3, 7); internal static bool MatchesProcessIdentity( diff --git a/package/AgentWindowsManaged.Tests/PackageBrokerInstallerTests.cs b/package/AgentWindowsManaged.Tests/PackageBrokerInstallerTests.cs index 2549938ff..6dc273ed7 100644 --- a/package/AgentWindowsManaged.Tests/PackageBrokerInstallerTests.cs +++ b/package/AgentWindowsManaged.Tests/PackageBrokerInstallerTests.cs @@ -635,6 +635,9 @@ public void PolicyConsentDiscoveryIsTransactionalAndArchitectureCorrect(bool win Assert.Equal(@"Software\Devolutions\Agent\PolicyConsentHelper", value.Key); Assert.Equal(RegistryKeyAction.createAndRemoveOnUninstall, value.RegistryKeyAction); Assert.Equal(win64, value.Win64); + Assert.Equal( + win64 ? "Type=string; Component:Win64=yes" : "Type=string", + value.AttributesDefinition); Assert.Contains(Features.AGENT_FEATURE, value.ActualFeatures); } diff --git a/package/AgentWindowsManaged/Program.cs b/package/AgentWindowsManaged/Program.cs index f3173223e..567150057 100644 --- a/package/AgentWindowsManaged/Program.cs +++ b/package/AgentWindowsManaged/Program.cs @@ -484,7 +484,7 @@ internal static RegValue CreatePolicyConsentRegistryValue(string name, string va name, value) { - AttributesDefinition = "Type=string", + AttributesDefinition = win64 ? "Type=string; Component:Win64=yes" : "Type=string", Win64 = win64, RegistryKeyAction = RegistryKeyAction.createAndRemoveOnUninstall, Feature = Features.AGENT_FEATURE, From 67a6419fd82938b5ca9eaf28891b345f3bafda20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20CORTIER?= Date: Thu, 10 Sep 2026 02:04:59 -0400 Subject: [PATCH 5/5] fix(agent): validate policy credentials Apply the canonical policy API safe-ASCII character set to helper credentials before a privileged request is dispatched. Issue: #1963 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../AgentPolicyConsent.Tests/ProtocolTests.cs | 18 +++++++++++++++--- package/AgentPolicyConsent/Protocol.cs | 2 +- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/package/AgentPolicyConsent.Tests/ProtocolTests.cs b/package/AgentPolicyConsent.Tests/ProtocolTests.cs index f3bdbb5ff..9b4aafaee 100644 --- a/package/AgentPolicyConsent.Tests/ProtocolTests.cs +++ b/package/AgentPolicyConsent.Tests/ProtocolTests.cs @@ -356,6 +356,18 @@ public void RequestRejectsUnknownOperations(string operation, string conflictHan Assert.Throws(() => Protocol.ValidateRequest(request)); } + [Theory] + [InlineData("a", true)] + [InlineData("A0._~:-", true)] + [InlineData("a/b", false)] + [InlineData("a b", false)] + [InlineData("a\"b", false)] + [InlineData("-token", false)] + public void CredentialsMatchOfficialPolicyApiCharacterRules(string value, bool accepted) + { + Assert.Equal(accepted, Protocol.IsCredential(value, 512)); + } + [Fact] public void CommittedResponseContainsOnlyStoreToken() { @@ -424,7 +436,7 @@ public void ResponseRequiresExplicitNullableMembers() } [Fact] - public void MaximumStaleResponseFitsExactWireBudget() + public void MaximumValidStaleResponseFitsWireBudget() { ElevationResponse response = new( "2.0", @@ -433,9 +445,9 @@ public void MaximumStaleResponseFitsExactWireBudget() 409, "StalePolicyStoreToken", null, - "T" + new string('"', 511), + "T" + new string('~', 511), "Active", - "P" + new string('"', 2047)); + "P" + new string('~', 2047)); Protocol.ValidateResponse(response); byte[] body = JsonSerializer.SerializeToUtf8Bytes( diff --git a/package/AgentPolicyConsent/Protocol.cs b/package/AgentPolicyConsent/Protocol.cs index 04184f3dc..a9057db90 100644 --- a/package/AgentPolicyConsent/Protocol.cs +++ b/package/AgentPolicyConsent/Protocol.cs @@ -172,7 +172,7 @@ value is not null && value.Length is > 0 && value.Length <= maximum && IsAsciiAlphaNumeric(value[0]) && - value.AsSpan(1).IndexOfAnyExceptInRange('!', '~') < 0; + value.AsSpan(1).IndexOfAnyExcept("ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789._~:-") < 0; private static bool IsOptionalCredential(string? value, int maximum) => value is null || IsCredential(value, maximum);