From c8f3b056465009608f829ab61a28f15c95d9a960 Mon Sep 17 00:00:00 2001 From: Vladyslav Nikonov Date: Tue, 11 Aug 2026 14:31:03 +0300 Subject: [PATCH 1/8] refactor(agent): extract package broker into dedicated crate Move the package broker subsystem out of the devolutions-agent binary into a new crates/now-package-broker crate, keeping the agent manifest lean and giving the broker-only dependency set (axum, hyper-util, notify, now-policy, ...) its own home. The crate compiles to an empty library on non-Windows platforms. Extract code_signing into devolutions-agent-shared so it is shared between the updater and the broker, which previously reached back into the agent crate for it. The agent forwards the development-only dev-skip-broker-signature feature to the broker crate. Issue: DGW-417 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .gitattributes | 4 +- Cargo.lock | 54 +++-- crates/devolutions-agent-shared/Cargo.toml | 4 + .../src/windows}/code_signing.rs | 18 +- .../src/windows/mod.rs | 1 + crates/now-package-broker/Cargo.toml | 71 +++++++ .../samples/corporate-allowlist.policy.json | 0 .../samples/deny-risky-options.policy.json | 0 .../winget-unknown-install.request.json | 0 .../winget-vscode-install.request.json | 0 .../winget-vscode-skiphash.request.json | 0 .../samples/scenarios/baseline.scenarios.json | 0 .../now-package-broker/src}/auth.rs | 3 +- .../src}/command_builder/bun.rs | 0 .../src}/command_builder/cargo.rs | 0 .../src}/command_builder/chocolatey.rs | 0 .../src}/command_builder/dotnet.rs | 0 .../src}/command_builder/mod.rs | 0 .../src}/command_builder/npm.rs | 0 .../src}/command_builder/pip.rs | 0 .../src}/command_builder/powershell.rs | 0 .../src}/command_builder/scoop.rs | 0 .../src}/command_builder/vcpkg.rs | 0 .../src}/command_builder/winget.rs | 0 .../src}/evaluator/constraints.rs | 0 .../src}/evaluator/matching.rs | 0 .../now-package-broker/src}/evaluator/mod.rs | 0 .../src}/evaluator/tests.rs | 0 .../src}/evaluator/version.rs | 0 .../src}/evaluator/wildcard.rs | 0 .../now-package-broker/src}/event_channel.rs | 0 .../now-package-broker/src}/executor/mod.rs | 2 +- .../src}/executor/output.rs | 0 .../src}/executor/windows/mod.rs | 8 +- .../src}/executor/windows/privileges.rs | 0 .../src}/executor/windows/process.rs | 10 +- .../src}/executor/windows/token.rs | 0 crates/now-package-broker/src/lib.rs | 34 +++ .../src}/operation_tracker.rs | 2 +- crates/now-package-broker/src/pipe.rs | 176 ++++++++++++++++ .../now-package-broker/src}/policy_loader.rs | 2 +- .../src}/policy_security.rs | 0 .../now-package-broker/src}/policy_watcher.rs | 2 +- .../now-package-broker/src}/scenario_tests.rs | 2 +- .../src}/server/connection.rs | 0 .../src}/server/execution.rs | 6 +- .../now-package-broker/src}/server/mod.rs | 14 +- .../src}/server/responses.rs | 4 +- .../now-package-broker/src}/task.rs | 14 +- devolutions-agent/Cargo.toml | 23 +- devolutions-agent/src/broker/mod.rs | 20 -- devolutions-agent/src/broker/pipe.rs | 197 ------------------ devolutions-agent/src/lib.rs | 4 - devolutions-agent/src/service.rs | 8 +- devolutions-agent/src/updater/package.rs | 4 +- 55 files changed, 379 insertions(+), 308 deletions(-) rename {devolutions-agent/src => crates/devolutions-agent-shared/src/windows}/code_signing.rs (79%) create mode 100644 crates/now-package-broker/Cargo.toml rename {devolutions-agent/src/broker => crates/now-package-broker/src}/assets/samples/corporate-allowlist.policy.json (100%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/assets/samples/deny-risky-options.policy.json (100%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/assets/samples/requests/winget-unknown-install.request.json (100%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/assets/samples/requests/winget-vscode-install.request.json (100%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/assets/samples/requests/winget-vscode-skiphash.request.json (100%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/assets/samples/scenarios/baseline.scenarios.json (100%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/auth.rs (99%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/command_builder/bun.rs (100%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/command_builder/cargo.rs (100%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/command_builder/chocolatey.rs (100%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/command_builder/dotnet.rs (100%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/command_builder/mod.rs (100%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/command_builder/npm.rs (100%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/command_builder/pip.rs (100%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/command_builder/powershell.rs (100%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/command_builder/scoop.rs (100%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/command_builder/vcpkg.rs (100%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/command_builder/winget.rs (100%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/evaluator/constraints.rs (100%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/evaluator/matching.rs (100%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/evaluator/mod.rs (100%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/evaluator/tests.rs (100%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/evaluator/version.rs (100%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/evaluator/wildcard.rs (100%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/event_channel.rs (100%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/executor/mod.rs (99%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/executor/output.rs (100%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/executor/windows/mod.rs (99%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/executor/windows/privileges.rs (100%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/executor/windows/process.rs (98%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/executor/windows/token.rs (100%) create mode 100644 crates/now-package-broker/src/lib.rs rename {devolutions-agent/src/broker => crates/now-package-broker/src}/operation_tracker.rs (99%) create mode 100644 crates/now-package-broker/src/pipe.rs rename {devolutions-agent/src/broker => crates/now-package-broker/src}/policy_loader.rs (99%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/policy_security.rs (100%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/policy_watcher.rs (99%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/scenario_tests.rs (98%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/server/connection.rs (100%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/server/execution.rs (90%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/server/mod.rs (98%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/server/responses.rs (98%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/task.rs (93%) delete mode 100644 devolutions-agent/src/broker/mod.rs delete mode 100644 devolutions-agent/src/broker/pipe.rs diff --git a/.gitattributes b/.gitattributes index ea0a53f72..7c78e6099 100644 --- a/.gitattributes +++ b/.gitattributes @@ -6,7 +6,7 @@ *.tsx text eol=lf *.json text eol=lf *.ps1 text eol=lf -*.sh text eol=lf +*.sh text eol=lf *.sln text eol=crlf *.md text eol=lf *.mustache text eol=lf @@ -25,4 +25,4 @@ devolutions-gateway/openapi/ts-angular-client/api/** linguist-generated merge=bi devolutions-gateway/openapi/ts-angular-client/model/** linguist-generated merge=binary # Sample assets produce huge LoC counts; exclude them from language statistics. -devolutions-agent/src/broker/assets/** linguist-generated +crates/now-package-broker/src/assets/** linguist-generated diff --git a/Cargo.lock b/Cargo.lock index 783656038..150686b3a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1688,13 +1688,11 @@ dependencies = [ "anyhow", "async-trait", "aws-lc-rs", - "axum 0.8.9", "backoff", "base64 0.23.0", "bytes 1.12.1", "camino", "ceviche", - "chrono", "ctrlc", "devolutions-agent-shared", "devolutions-gateway-task", @@ -1706,15 +1704,10 @@ dependencies = [ "hex", "hostname 0.4.2", "http-client-proxy", - "hyper 1.10.1", - "hyper-util", "ipnetwork", "ironrdp", - "notify 7.0.0", "notify-debouncer-mini", - "now-policy", - "now-policy-api", - "now-policy-server-template", + "now-package-broker", "parking_lot", "prost 0.13.5", "prost-types", @@ -1722,15 +1715,12 @@ dependencies = [ "quinn", "rand 0.8.7", "rcgen", - "regex", "reqwest", "rustls 0.23.42", "rustls-pemfile 2.2.0", "rustls-pki-types", - "semver", "serde", "serde_json", - "serde_yaml", "sha2 0.10.9", "tap", "tempfile", @@ -1739,14 +1729,11 @@ dependencies = [ "tokio 1.52.3", "tokio-rustls", "tokio-stream", - "tokio-util", "tonic 0.12.3", "tonic-build", - "tower-service", "tracing", "url", "uuid", - "widestring 1.2.1", "win-api-wrappers", "windows 0.61.3", "x509-parser", @@ -1756,14 +1743,18 @@ dependencies = [ name = "devolutions-agent-shared" version = "0.0.0" dependencies = [ + "anyhow", "camino", "cfg-if", + "hex", "serde", "serde_json", "tempfile", "thiserror 2.0.18", "tracing", "uuid", + "win-api-wrappers", + "windows 0.61.3", "windows-registry 0.5.3", "windows-result 0.3.4", ] @@ -4752,6 +4743,41 @@ dependencies = [ "bitflags 2.13.0", ] +[[package]] +name = "now-package-broker" +version = "0.0.0" +dependencies = [ + "anyhow", + "async-trait", + "axum 0.8.9", + "chrono", + "devolutions-agent-shared", + "devolutions-gateway-task", + "hex", + "hyper 1.10.1", + "hyper-util", + "notify 7.0.0", + "now-policy", + "now-policy-api", + "now-policy-server-template", + "parking_lot", + "regex", + "semver", + "serde", + "serde_json", + "serde_yaml", + "sha2 0.10.9", + "tempfile", + "tokio 1.52.3", + "tokio-util", + "tower-service", + "tracing", + "uuid", + "widestring 1.2.1", + "win-api-wrappers", + "windows 0.61.3", +] + [[package]] name = "now-policy" version = "0.2.0" diff --git a/crates/devolutions-agent-shared/Cargo.toml b/crates/devolutions-agent-shared/Cargo.toml index 2b2714b81..cb482c9a9 100644 --- a/crates/devolutions-agent-shared/Cargo.toml +++ b/crates/devolutions-agent-shared/Cargo.toml @@ -19,6 +19,10 @@ thiserror = "2" tracing = "0.1" [target.'cfg(windows)'.dependencies] +anyhow = "1" +hex = "0.4" +win-api-wrappers = { path = "../win-api-wrappers" } +windows = { version = "0.61", features = ["Win32_Foundation", "Win32_Security_Cryptography"] } windows-registry = "0.5" # Required for `windows-registry` error type. (`Error` is not reexported from `windows-registry`). windows-result = "0.3" diff --git a/devolutions-agent/src/code_signing.rs b/crates/devolutions-agent-shared/src/windows/code_signing.rs similarity index 79% rename from devolutions-agent/src/code_signing.rs rename to crates/devolutions-agent-shared/src/windows/code_signing.rs index 0edd2badb..4d58e5356 100644 --- a/devolutions-agent/src/code_signing.rs +++ b/crates/devolutions-agent-shared/src/windows/code_signing.rs @@ -6,13 +6,13 @@ use anyhow::{Context as _, bail}; use win_api_wrappers::security::crypt::{AuthenticodeSignatureStatus, authenticode_status}; /// List of allowed thumbprints for Devolutions code signing certificates. -pub(crate) const DEVOLUTIONS_CERT_THUMBPRINTS: &[&str] = &[ +pub const DEVOLUTIONS_CERT_THUMBPRINTS: &[&str] = &[ "3f5202a9432d54293bdfe6f7e46adb0a6f8b3ba6", "8db5a43bb8afe4d2ffb92da9007d8997a4cc4e13", "50f753333811ff11f1920274afde3ffd4468b210", ]; -pub(crate) fn certificate_sha1_thumbprint(cert_der: &[u8]) -> anyhow::Result<[u8; 20]> { +pub fn certificate_sha1_thumbprint(cert_der: &[u8]) -> anyhow::Result<[u8; 20]> { use windows::Win32::Security::Cryptography::{CALG_SHA1, CryptHashCertificate}; let mut thumbprint = [0u8; 20]; @@ -39,7 +39,7 @@ pub(crate) fn certificate_sha1_thumbprint(cert_der: &[u8]) -> anyhow::Result<[u8 Ok(thumbprint) } -pub(crate) fn is_devolutions_certificate_thumbprint(calculated_thumbprint: &[u8; 20]) -> bool { +pub fn is_devolutions_certificate_thumbprint(calculated_thumbprint: &[u8; 20]) -> bool { DEVOLUTIONS_CERT_THUMBPRINTS.iter().any(|thumbprint| { let mut thumbprint_bytes = [0u8; 20]; hex::decode_to_slice(thumbprint, &mut thumbprint_bytes) @@ -49,32 +49,32 @@ pub(crate) fn is_devolutions_certificate_thumbprint(calculated_thumbprint: &[u8; }) } -pub(crate) fn validate_devolutions_authenticode_signature(path: &Path) -> anyhow::Result { +pub fn validate_devolutions_authenticode_signature(path: &Path) -> anyhow::Result { let wintrust_result = authenticode_status(path).with_context(|| { format!( - "failed to read authenticode signature for client executable '{}'", + "failed to read authenticode signature for executable '{}'", path.display() ) })?; if !matches!(wintrust_result.status, AuthenticodeSignatureStatus::Valid) { - bail!("client executable signature is not valid: {:?}", wintrust_result.status); + bail!("executable signature is not valid: {:?}", wintrust_result.status); } let signer = wintrust_result .provider .as_ref() .and_then(|provider| provider.signers.first()) - .context("client executable signature has no signer")?; + .context("executable signature has no signer")?; let signing_cert = signer .cert_chain .first() - .context("client executable signature has no signing certificate")?; + .context("executable signature has no signing certificate")?; let thumbprint = certificate_sha1_thumbprint(&signing_cert.cert.encoded)?; if !is_devolutions_certificate_thumbprint(&thumbprint) { bail!( - "client executable is signed with an unexpected certificate thumbprint: {}", + "executable is signed with an unexpected certificate thumbprint: {}", hex::encode(thumbprint) ); } diff --git a/crates/devolutions-agent-shared/src/windows/mod.rs b/crates/devolutions-agent-shared/src/windows/mod.rs index 576d7bfed..436a8d657 100644 --- a/crates/devolutions-agent-shared/src/windows/mod.rs +++ b/crates/devolutions-agent-shared/src/windows/mod.rs @@ -1,5 +1,6 @@ mod reversed_hex_uuid; +pub mod code_signing; pub mod registry; use uuid::{Uuid, uuid}; diff --git a/crates/now-package-broker/Cargo.toml b/crates/now-package-broker/Cargo.toml new file mode 100644 index 000000000..379c5024a --- /dev/null +++ b/crates/now-package-broker/Cargo.toml @@ -0,0 +1,71 @@ +[package] +name = "now-package-broker" +version = "0.0.0" +edition = "2024" +license = "MIT/Apache-2.0" +authors = ["Devolutions Inc. "] +description = "Package broker for the Devolutions Agent" +publish = false + +[features] +default = [] +# Development-only feature allowing broker client signature validation to be skipped. +# Must never be enabled for shipped builds: without it, broker client signature validation is +# unconditionally enforced regardless of the configuration file contents. +dev-skip-broker-signature = [] + +[lints] +workspace = true + +# The broker is only functional on Windows; all dependencies are Windows-only +# so the crate compiles to an empty library on other platforms. +[target.'cfg(windows)'.dependencies] +anyhow = "1" +async-trait = "0.1" +axum = { version = "0.8", default-features = false, features = ["http1", "json", "tokio", "original-uri", "matched-path"] } +chrono = { version = "0.4", features = ["serde"] } +devolutions-agent-shared = { path = "../devolutions-agent-shared" } +devolutions-gateway-task = { path = "../devolutions-gateway-task" } +hex = "0.4" +hyper = { version = "1", features = ["http1", "server"] } +hyper-util = { version = "0.1", features = ["tokio", "server", "server-auto", "service"] } +notify = { version = "7", default-features = false } +now-policy = "0.2" +now-policy-api = { version = "0.3", features = ["policy-compat"] } +now-policy-server-template = { version = "0.3", features = ["policy-compat"] } +parking_lot = "0.12" +regex = "1" +semver = "1" +serde_json = "1" +sha2 = "0.10" +tokio = { version = "1.52", features = ["net", "io-util", "rt", "macros", "parking_lot", "fs", "sync", "time"] } +tokio-util = "0.7" +tower-service = "0.3" +tracing = "0.1" +uuid = { version = "1.23", features = ["v4"] } +widestring = "1.2" +win-api-wrappers = { path = "../win-api-wrappers" } + +[target.'cfg(windows)'.dependencies.windows] +version = "0.61" +features = [ + "Win32_Foundation", + "Win32_Security", + "Win32_Security_Authorization", + "Win32_Storage_FileSystem", + "Win32_System_ApplicationInstallationAndServicing", + "Win32_System_Com", + "Win32_System_Console", + "Win32_System_IO", + "Win32_System_Ioctl", + "Win32_System_Pipes", + "Win32_System_Threading", + "Win32_UI_Shell", + "Win32_UI_WindowsAndMessaging", +] + +[target.'cfg(windows)'.dev-dependencies] +serde = { version = "1", features = ["derive"] } +serde_yaml = "0.9" +tempfile = "3" +tokio = { version = "1.52", features = ["rt-multi-thread"] } diff --git a/devolutions-agent/src/broker/assets/samples/corporate-allowlist.policy.json b/crates/now-package-broker/src/assets/samples/corporate-allowlist.policy.json similarity index 100% rename from devolutions-agent/src/broker/assets/samples/corporate-allowlist.policy.json rename to crates/now-package-broker/src/assets/samples/corporate-allowlist.policy.json diff --git a/devolutions-agent/src/broker/assets/samples/deny-risky-options.policy.json b/crates/now-package-broker/src/assets/samples/deny-risky-options.policy.json similarity index 100% rename from devolutions-agent/src/broker/assets/samples/deny-risky-options.policy.json rename to crates/now-package-broker/src/assets/samples/deny-risky-options.policy.json diff --git a/devolutions-agent/src/broker/assets/samples/requests/winget-unknown-install.request.json b/crates/now-package-broker/src/assets/samples/requests/winget-unknown-install.request.json similarity index 100% rename from devolutions-agent/src/broker/assets/samples/requests/winget-unknown-install.request.json rename to crates/now-package-broker/src/assets/samples/requests/winget-unknown-install.request.json diff --git a/devolutions-agent/src/broker/assets/samples/requests/winget-vscode-install.request.json b/crates/now-package-broker/src/assets/samples/requests/winget-vscode-install.request.json similarity index 100% rename from devolutions-agent/src/broker/assets/samples/requests/winget-vscode-install.request.json rename to crates/now-package-broker/src/assets/samples/requests/winget-vscode-install.request.json diff --git a/devolutions-agent/src/broker/assets/samples/requests/winget-vscode-skiphash.request.json b/crates/now-package-broker/src/assets/samples/requests/winget-vscode-skiphash.request.json similarity index 100% rename from devolutions-agent/src/broker/assets/samples/requests/winget-vscode-skiphash.request.json rename to crates/now-package-broker/src/assets/samples/requests/winget-vscode-skiphash.request.json diff --git a/devolutions-agent/src/broker/assets/samples/scenarios/baseline.scenarios.json b/crates/now-package-broker/src/assets/samples/scenarios/baseline.scenarios.json similarity index 100% rename from devolutions-agent/src/broker/assets/samples/scenarios/baseline.scenarios.json rename to crates/now-package-broker/src/assets/samples/scenarios/baseline.scenarios.json diff --git a/devolutions-agent/src/broker/auth.rs b/crates/now-package-broker/src/auth.rs similarity index 99% rename from devolutions-agent/src/broker/auth.rs rename to crates/now-package-broker/src/auth.rs index 009c90de3..bc770d149 100644 --- a/devolutions-agent/src/broker/auth.rs +++ b/crates/now-package-broker/src/auth.rs @@ -3,6 +3,7 @@ use std::path::{Path, PathBuf}; use anyhow::{Context as _, bail}; +use devolutions_agent_shared::windows::code_signing::validate_devolutions_authenticode_signature; use now_policy_api::{CancelRequest, ClientContext, PackageRequest, StatusRequest}; use tokio::net::windows::named_pipe::NamedPipeServer; use tracing::{debug, warn}; @@ -14,8 +15,6 @@ use windows::Win32::Security::TOKEN_QUERY; use windows::Win32::Storage::FileSystem::FILE_ID_INFO; use windows::Win32::System::Threading::PROCESS_QUERY_LIMITED_INFORMATION; -use crate::code_signing::validate_devolutions_authenticode_signature; - #[derive(Clone, Debug)] pub(crate) struct PipeClient { process_id: u32, diff --git a/devolutions-agent/src/broker/command_builder/bun.rs b/crates/now-package-broker/src/command_builder/bun.rs similarity index 100% rename from devolutions-agent/src/broker/command_builder/bun.rs rename to crates/now-package-broker/src/command_builder/bun.rs diff --git a/devolutions-agent/src/broker/command_builder/cargo.rs b/crates/now-package-broker/src/command_builder/cargo.rs similarity index 100% rename from devolutions-agent/src/broker/command_builder/cargo.rs rename to crates/now-package-broker/src/command_builder/cargo.rs diff --git a/devolutions-agent/src/broker/command_builder/chocolatey.rs b/crates/now-package-broker/src/command_builder/chocolatey.rs similarity index 100% rename from devolutions-agent/src/broker/command_builder/chocolatey.rs rename to crates/now-package-broker/src/command_builder/chocolatey.rs diff --git a/devolutions-agent/src/broker/command_builder/dotnet.rs b/crates/now-package-broker/src/command_builder/dotnet.rs similarity index 100% rename from devolutions-agent/src/broker/command_builder/dotnet.rs rename to crates/now-package-broker/src/command_builder/dotnet.rs diff --git a/devolutions-agent/src/broker/command_builder/mod.rs b/crates/now-package-broker/src/command_builder/mod.rs similarity index 100% rename from devolutions-agent/src/broker/command_builder/mod.rs rename to crates/now-package-broker/src/command_builder/mod.rs diff --git a/devolutions-agent/src/broker/command_builder/npm.rs b/crates/now-package-broker/src/command_builder/npm.rs similarity index 100% rename from devolutions-agent/src/broker/command_builder/npm.rs rename to crates/now-package-broker/src/command_builder/npm.rs diff --git a/devolutions-agent/src/broker/command_builder/pip.rs b/crates/now-package-broker/src/command_builder/pip.rs similarity index 100% rename from devolutions-agent/src/broker/command_builder/pip.rs rename to crates/now-package-broker/src/command_builder/pip.rs diff --git a/devolutions-agent/src/broker/command_builder/powershell.rs b/crates/now-package-broker/src/command_builder/powershell.rs similarity index 100% rename from devolutions-agent/src/broker/command_builder/powershell.rs rename to crates/now-package-broker/src/command_builder/powershell.rs diff --git a/devolutions-agent/src/broker/command_builder/scoop.rs b/crates/now-package-broker/src/command_builder/scoop.rs similarity index 100% rename from devolutions-agent/src/broker/command_builder/scoop.rs rename to crates/now-package-broker/src/command_builder/scoop.rs diff --git a/devolutions-agent/src/broker/command_builder/vcpkg.rs b/crates/now-package-broker/src/command_builder/vcpkg.rs similarity index 100% rename from devolutions-agent/src/broker/command_builder/vcpkg.rs rename to crates/now-package-broker/src/command_builder/vcpkg.rs diff --git a/devolutions-agent/src/broker/command_builder/winget.rs b/crates/now-package-broker/src/command_builder/winget.rs similarity index 100% rename from devolutions-agent/src/broker/command_builder/winget.rs rename to crates/now-package-broker/src/command_builder/winget.rs diff --git a/devolutions-agent/src/broker/evaluator/constraints.rs b/crates/now-package-broker/src/evaluator/constraints.rs similarity index 100% rename from devolutions-agent/src/broker/evaluator/constraints.rs rename to crates/now-package-broker/src/evaluator/constraints.rs diff --git a/devolutions-agent/src/broker/evaluator/matching.rs b/crates/now-package-broker/src/evaluator/matching.rs similarity index 100% rename from devolutions-agent/src/broker/evaluator/matching.rs rename to crates/now-package-broker/src/evaluator/matching.rs diff --git a/devolutions-agent/src/broker/evaluator/mod.rs b/crates/now-package-broker/src/evaluator/mod.rs similarity index 100% rename from devolutions-agent/src/broker/evaluator/mod.rs rename to crates/now-package-broker/src/evaluator/mod.rs diff --git a/devolutions-agent/src/broker/evaluator/tests.rs b/crates/now-package-broker/src/evaluator/tests.rs similarity index 100% rename from devolutions-agent/src/broker/evaluator/tests.rs rename to crates/now-package-broker/src/evaluator/tests.rs diff --git a/devolutions-agent/src/broker/evaluator/version.rs b/crates/now-package-broker/src/evaluator/version.rs similarity index 100% rename from devolutions-agent/src/broker/evaluator/version.rs rename to crates/now-package-broker/src/evaluator/version.rs diff --git a/devolutions-agent/src/broker/evaluator/wildcard.rs b/crates/now-package-broker/src/evaluator/wildcard.rs similarity index 100% rename from devolutions-agent/src/broker/evaluator/wildcard.rs rename to crates/now-package-broker/src/evaluator/wildcard.rs diff --git a/devolutions-agent/src/broker/event_channel.rs b/crates/now-package-broker/src/event_channel.rs similarity index 100% rename from devolutions-agent/src/broker/event_channel.rs rename to crates/now-package-broker/src/event_channel.rs diff --git a/devolutions-agent/src/broker/executor/mod.rs b/crates/now-package-broker/src/executor/mod.rs similarity index 99% rename from devolutions-agent/src/broker/executor/mod.rs rename to crates/now-package-broker/src/executor/mod.rs index 61f0287b6..88519475b 100644 --- a/devolutions-agent/src/broker/executor/mod.rs +++ b/crates/now-package-broker/src/executor/mod.rs @@ -9,7 +9,7 @@ use tokio_util::sync::CancellationToken; use tracing::info; use win_api_wrappers::identity::sid::Sid; -use crate::broker::event_channel::OperationEventSink; +use crate::event_channel::OperationEventSink; mod output; diff --git a/devolutions-agent/src/broker/executor/output.rs b/crates/now-package-broker/src/executor/output.rs similarity index 100% rename from devolutions-agent/src/broker/executor/output.rs rename to crates/now-package-broker/src/executor/output.rs diff --git a/devolutions-agent/src/broker/executor/windows/mod.rs b/crates/now-package-broker/src/executor/windows/mod.rs similarity index 99% rename from devolutions-agent/src/broker/executor/windows/mod.rs rename to crates/now-package-broker/src/executor/windows/mod.rs index 1f77ee71a..a1e9e48a8 100644 --- a/devolutions-agent/src/broker/executor/windows/mod.rs +++ b/crates/now-package-broker/src/executor/windows/mod.rs @@ -22,7 +22,7 @@ use super::{ BROKER_SUPPORTED_MANAGERS, CommandExecutor, ExecutionContext, ExecutionOutput, OperationCanceled, ProcessStartedCallback, is_canceled_error, }; -use crate::broker::policy_security; +use crate::policy_security; mod privileges; mod process; @@ -161,9 +161,7 @@ fn manager_is_available(manager: ManagerName, user_env: &HashMap .is_ok(), ManagerName::Bun => resolve_bun_executable("bun", user_env).is_ok(), ManagerName::Cargo => resolve_cargo_executable(user_env).is_ok(), - ManagerName::Dotnet => { - Path::new(&crate::broker::command_builder::dotnet::trusted_dotnet_executable()).is_file() - } + ManagerName::Dotnet => Path::new(&crate::command_builder::dotnet::trusted_dotnet_executable()).is_file(), ManagerName::Pip => resolve_python_executable("python.exe", user_env).is_ok(), // npm runs through the user PATH `npm` shim inside the trusted Windows PowerShell wrapper, // so both the shim and the host must exist. @@ -1355,7 +1353,7 @@ mod tests { prepare_chocolatey_script_in_with_default_install_root, prepare_main_command_in, prepare_shell_command_in, reject_unsupported_vcpkg_elevation, resolve_trusted_chocolatey_executable, resolve_winget_executable, }; - use crate::broker::executor::{CommandExecutor as _, ExecutionContext}; + use crate::executor::{CommandExecutor as _, ExecutionContext}; fn grant(permissions: u32, sid: Sid) -> ExplicitAccess { ExplicitAccess { diff --git a/devolutions-agent/src/broker/executor/windows/privileges.rs b/crates/now-package-broker/src/executor/windows/privileges.rs similarity index 100% rename from devolutions-agent/src/broker/executor/windows/privileges.rs rename to crates/now-package-broker/src/executor/windows/privileges.rs diff --git a/devolutions-agent/src/broker/executor/windows/process.rs b/crates/now-package-broker/src/executor/windows/process.rs similarity index 98% rename from devolutions-agent/src/broker/executor/windows/process.rs rename to crates/now-package-broker/src/executor/windows/process.rs index c73ac1975..dc6276e19 100644 --- a/devolutions-agent/src/broker/executor/windows/process.rs +++ b/crates/now-package-broker/src/executor/windows/process.rs @@ -19,12 +19,12 @@ use windows::Win32::System::Threading::{ }; use windows::Win32::UI::WindowsAndMessaging::SW_HIDE; -use crate::broker::event_channel::{OperationEventSink, OutputStream}; -use crate::broker::executor::{ +use crate::event_channel::{OperationEventSink, OutputStream}; +use crate::executor::{ ExecutionOutput, MAX_CAPTURED_OUTPUT_BYTES, OperationCanceled, ProcessStartedCallback, tail_utf8, }; -use crate::broker::operation_tracker::OperationTracker; -use crate::broker::policy_security; +use crate::operation_tracker::OperationTracker; +use crate::policy_security; /// How long a canceled process is given to exit after the graceful console /// ctrl event before it is forcefully terminated. @@ -66,7 +66,7 @@ impl<'a> OutputCapture<'a> { /// stdout and stderr are redirected into two separate pipes; raw chunks are forwarded to /// `event_sink` (when present) as they arrive, and a tail-truncated combined copy is /// returned in [`ExecutionOutput`] (limited to -/// [`crate::broker::executor::MAX_CAPTURED_OUTPUT_BYTES`]); otherwise no output is captured. +/// [`crate::executor::MAX_CAPTURED_OUTPUT_BYTES`]); otherwise no output is captured. /// /// Returns the process exit code and (when captured) its output. /// diff --git a/devolutions-agent/src/broker/executor/windows/token.rs b/crates/now-package-broker/src/executor/windows/token.rs similarity index 100% rename from devolutions-agent/src/broker/executor/windows/token.rs rename to crates/now-package-broker/src/executor/windows/token.rs diff --git a/crates/now-package-broker/src/lib.rs b/crates/now-package-broker/src/lib.rs new file mode 100644 index 000000000..b55c994db --- /dev/null +++ b/crates/now-package-broker/src/lib.rs @@ -0,0 +1,34 @@ +//! Package broker for the Devolutions Agent. +//! +//! Provides policy evaluation and command execution for package operations, +//! communicating over a Windows named pipe using HTTP/1.1. +//! +//! The broker is only functional on Windows; on other platforms this crate is empty. + +#[cfg(windows)] +mod auth; +#[cfg(windows)] +pub mod command_builder; +#[cfg(windows)] +pub mod evaluator; +#[cfg(windows)] +pub mod event_channel; +#[cfg(windows)] +pub mod executor; +#[cfg(windows)] +pub mod operation_tracker; +#[cfg(windows)] +pub mod pipe; +#[cfg(windows)] +pub mod policy_loader; +#[cfg(windows)] +mod policy_security; +#[cfg(windows)] +pub mod policy_watcher; +#[cfg(windows)] +pub mod server; +#[cfg(windows)] +pub mod task; + +#[cfg(all(test, windows))] +mod scenario_tests; diff --git a/devolutions-agent/src/broker/operation_tracker.rs b/crates/now-package-broker/src/operation_tracker.rs similarity index 99% rename from devolutions-agent/src/broker/operation_tracker.rs rename to crates/now-package-broker/src/operation_tracker.rs index 10d62ad78..3850e0d4f 100644 --- a/devolutions-agent/src/broker/operation_tracker.rs +++ b/crates/now-package-broker/src/operation_tracker.rs @@ -14,7 +14,7 @@ use now_policy_api::{EventChannel, OperationStatus, PackageRequest, ResourceId}; use sha2::{Digest as _, Sha256}; use tokio_util::sync::CancellationToken; -use crate::broker::event_channel::OperationEventSink; +use crate::event_channel::OperationEventSink; /// How long completed/failed operation results are retained for status queries. const RESULT_RETENTION: Duration = Duration::from_secs(5 * 60); // 5 minutes. diff --git a/crates/now-package-broker/src/pipe.rs b/crates/now-package-broker/src/pipe.rs new file mode 100644 index 000000000..1cb6d6ca7 --- /dev/null +++ b/crates/now-package-broker/src/pipe.rs @@ -0,0 +1,176 @@ +//! Named pipe transport for Windows. +//! +//! Creates a named pipe server with appropriate ACLs and accepts connections, +//! forwarding them to the HTTP server. + +use std::sync::Arc; + +use anyhow::Context as _; +use tokio::net::windows::named_pipe::{NamedPipeServer, ServerOptions}; +use tokio::sync::Semaphore; +use tokio_util::sync::CancellationToken; +use tracing::{error, info, warn}; +use win_api_wrappers::identity::sid::Sid; +use win_api_wrappers::security::acl::{Acl, ExplicitAccess, InheritableAcl, InheritableAclKind, Trustee}; +use win_api_wrappers::security::attributes::SecurityAttributesInit; +use windows::Win32::Foundation::GENERIC_ALL; +use windows::Win32::Security; +use windows::Win32::Security::Authorization::SET_ACCESS; +use windows::Win32::Storage::FileSystem::{FILE_GENERIC_READ, FILE_GENERIC_WRITE}; + +use crate::auth::PipeClient; +use crate::server::{BrokerState, build_router_for_client, serve_connection}; + +/// Default pipe name for the package broker. +pub const DEFAULT_PIPE_NAME: &str = r"\\.\pipe\Devolutions.Now.PackageBroker.v1"; + +/// Maximum number of concurrently served pipe connections. +/// +/// Connection setup performs unauthenticated work (client process identity lookups) +/// before any signature gate, so a connection flood could otherwise trigger unbounded +/// work and task spawning. While all slots are taken, no pipe instance is listening and +/// further clients fail to connect until a slot frees up. +const MAX_CONCURRENT_CONNECTIONS: usize = 16; + +/// Deadline for serving a single pipe connection, from accept to response completion. +/// +/// Each connection serves exactly one HTTP request (`keep_alive` is disabled) and all +/// endpoints respond without blocking on package operations (execution is asynchronous, +/// tracked via the operation tracker), so a healthy exchange completes well within this +/// deadline. Without it, idle clients holding their connection open without sending a +/// request would each pin a connection slot indefinitely and could exhaust the pool. +const CONNECTION_DEADLINE: std::time::Duration = std::time::Duration::from_secs(30); + +/// Start the named pipe server and accept connections until shutdown. +pub async fn run_pipe_server(state: Arc, shutdown: CancellationToken) -> anyhow::Result<()> { + let pipe_name = state.pipe_name.clone(); + info!(%pipe_name, "Starting named pipe server"); + + let connection_permits = Arc::new(Semaphore::new(MAX_CONCURRENT_CONNECTIONS)); + + let mut first_instance = true; + loop { + // Wait for a free connection slot before exposing a new pipe instance, + // bounding the number of concurrently served connections. + let permit = tokio::select! { + permit = Arc::clone(&connection_permits).acquire_owned() => { + permit.expect("the semaphore is never closed") + } + _ = shutdown.cancelled() => { + info!("Pipe server shutting down"); + return Ok(()); + } + }; + + // Create a new pipe instance for each connection. + let server = create_pipe_instance(&pipe_name, first_instance)?; + first_instance = false; + + tokio::select! { + result = server.connect() => { + match result { + Ok(()) => { + let state = Arc::clone(&state); + tokio::spawn(async move { + // The permit is held for the lifetime of the connection task. + let _permit = permit; + + let serve = async move { + // Capture the client identity off the accept loop so a slow + // lookup cannot stall accepting other connections. + let client = match PipeClient::from_connected_pipe(&server) { + Ok(client) => client, + Err(error) => { + warn!(%error, "Rejected named pipe client"); + return; + } + }; + info!("Client connected to named pipe"); + let router = build_router_for_client(state, client); + serve_connection(server, router).await; + info!("Client disconnected from named pipe"); + }; + + // Enforce a deadline so idle or slow clients cannot pin + // a connection slot indefinitely. + if tokio::time::timeout(CONNECTION_DEADLINE, serve).await.is_err() { + warn!("Closed named pipe connection: deadline exceeded"); + } + }); + } + Err(error) => { + error!(%error, "Failed to accept pipe connection"); + } + } + } + _ = shutdown.cancelled() => { + info!("Pipe server shutting down"); + return Ok(()); + } + } + } +} + +fn create_pipe_instance(pipe_name: &str, first_instance: bool) -> anyhow::Result { + let security_attributes = build_pipe_security_attributes().context("failed to build pipe security attributes")?; + + // SAFETY: `create_with_security_attributes_raw` requires a pointer to a valid + // `SECURITY_ATTRIBUTES` that stays alive for the duration of the call. The pointer + // comes from `security_attributes` (a `win_api_wrappers::security::SecurityAttributes`), + // a local binding that owns the structure and its security descriptor and is dropped + // only at the end of this function, well after the call returns. `CreateNamedPipeW` + // copies the descriptor at creation, so the pointer is not retained afterwards. + let server = unsafe { + ServerOptions::new() + .first_pipe_instance(first_instance) + .create_with_security_attributes_raw(pipe_name, security_attributes.as_mut_ptr().cast()) + }?; + + Ok(server) +} + +/// Build a security descriptor that grants: +/// - SYSTEM: full control +/// - Administrators: full control +/// - BUILTIN\Users: read + write (allows interactive users to connect) +fn build_pipe_security_attributes() -> anyhow::Result { + let system_sid = Sid::from_well_known(Security::WinLocalSystemSid, None).context("failed to create SYSTEM SID")?; + let admins_sid = Sid::from_well_known(Security::WinBuiltinAdministratorsSid, None) + .context("failed to create Administrators SID")?; + let users_sid = Sid::from_well_known(Security::WinBuiltinUsersSid, None).context("failed to create Users SID")?; + + let entries = [ + ExplicitAccess { + access_permissions: GENERIC_ALL.0, + access_mode: SET_ACCESS, + inheritance: Security::ACE_FLAGS(0), + trustee: Trustee::Sid(system_sid), + }, + ExplicitAccess { + access_permissions: GENERIC_ALL.0, + access_mode: SET_ACCESS, + inheritance: Security::ACE_FLAGS(0), + trustee: Trustee::Sid(admins_sid), + }, + ExplicitAccess { + access_permissions: FILE_GENERIC_READ.0 | FILE_GENERIC_WRITE.0, + access_mode: SET_ACCESS, + inheritance: Security::ACE_FLAGS(0), + trustee: Trustee::Sid(users_sid), + }, + ]; + + let empty_acl = Acl::new().context("failed to create empty ACL")?; + let dacl = empty_acl.set_entries(&entries).context("failed to set ACL entries")?; + + let attrs = SecurityAttributesInit { + dacl: Some(InheritableAcl { + kind: InheritableAclKind::Protected, + acl: dacl, + }), + ..Default::default() + } + .init(); + + Ok(attrs) +} diff --git a/devolutions-agent/src/broker/policy_loader.rs b/crates/now-package-broker/src/policy_loader.rs similarity index 99% rename from devolutions-agent/src/broker/policy_loader.rs rename to crates/now-package-broker/src/policy_loader.rs index 60ec86a4b..3d44cb9dd 100644 --- a/devolutions-agent/src/broker/policy_loader.rs +++ b/crates/now-package-broker/src/policy_loader.rs @@ -11,7 +11,7 @@ use now_policy::PolicyDocument; use now_policy::schema::{parse_policy_json, parse_policy_yaml}; use tracing::info; -use crate::broker::policy_security; +use crate::policy_security; /// Default policy directory. pub fn default_policy_dir() -> PathBuf { diff --git a/devolutions-agent/src/broker/policy_security.rs b/crates/now-package-broker/src/policy_security.rs similarity index 100% rename from devolutions-agent/src/broker/policy_security.rs rename to crates/now-package-broker/src/policy_security.rs diff --git a/devolutions-agent/src/broker/policy_watcher.rs b/crates/now-package-broker/src/policy_watcher.rs similarity index 99% rename from devolutions-agent/src/broker/policy_watcher.rs rename to crates/now-package-broker/src/policy_watcher.rs index fe0d0d676..a30bda5e3 100644 --- a/devolutions-agent/src/broker/policy_watcher.rs +++ b/crates/now-package-broker/src/policy_watcher.rs @@ -14,7 +14,7 @@ use tokio::sync::watch; use tokio_util::sync::CancellationToken; use tracing::{error, info, warn}; -use crate::broker::policy_loader; +use crate::policy_loader; /// State of the policy: either loaded and active, or unavailable. #[derive(Debug, Clone)] diff --git a/devolutions-agent/src/broker/scenario_tests.rs b/crates/now-package-broker/src/scenario_tests.rs similarity index 98% rename from devolutions-agent/src/broker/scenario_tests.rs rename to crates/now-package-broker/src/scenario_tests.rs index e2ebab6a0..c0c16e4b8 100644 --- a/devolutions-agent/src/broker/scenario_tests.rs +++ b/crates/now-package-broker/src/scenario_tests.rs @@ -15,7 +15,7 @@ use super::evaluator; /// Local samples directory bundled inside the crate. fn samples_dir() -> PathBuf { - PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("src/broker/assets/samples") + PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("src/assets/samples") } // ─── Scenario file structures ──────────────────────────────────────────────── diff --git a/devolutions-agent/src/broker/server/connection.rs b/crates/now-package-broker/src/server/connection.rs similarity index 100% rename from devolutions-agent/src/broker/server/connection.rs rename to crates/now-package-broker/src/server/connection.rs diff --git a/devolutions-agent/src/broker/server/execution.rs b/crates/now-package-broker/src/server/execution.rs similarity index 90% rename from devolutions-agent/src/broker/server/execution.rs rename to crates/now-package-broker/src/server/execution.rs index ff141be7b..911e7bc6f 100644 --- a/devolutions-agent/src/broker/server/execution.rs +++ b/crates/now-package-broker/src/server/execution.rs @@ -5,8 +5,8 @@ use std::sync::Arc; use now_policy_api::ResourceId; use tracing::{error, info}; -use crate::broker::executor::{CommandExecutor, ExecutionContext, ProcessStartedCallback, is_canceled_error}; -use crate::broker::operation_tracker::OperationTracker; +use crate::executor::{CommandExecutor, ExecutionContext, ProcessStartedCallback, is_canceled_error}; +use crate::operation_tracker::OperationTracker; pub(super) fn spawn_execution( executor: Arc, @@ -30,7 +30,7 @@ pub(super) fn spawn_execution( } else { #[allow(clippy::cast_sign_loss)] let unsigned = output.exit_code as u32; - match crate::broker::executor::describe_exit_code(output.exit_code) { + match crate::executor::describe_exit_code(output.exit_code) { Some(description) => format!( "{exe_name} exited with code {} (0x{unsigned:08X}): {description}", output.exit_code diff --git a/devolutions-agent/src/broker/server/mod.rs b/crates/now-package-broker/src/server/mod.rs similarity index 98% rename from devolutions-agent/src/broker/server/mod.rs rename to crates/now-package-broker/src/server/mod.rs index 7baf78852..6f1e65b20 100644 --- a/devolutions-agent/src/broker/server/mod.rs +++ b/crates/now-package-broker/src/server/mod.rs @@ -18,11 +18,11 @@ use now_policy_server_template::{MAX_REQUEST_BODY_BYTES, PackageBrokerServer, Sh use tracing::{info, trace, warn}; use win_api_wrappers::identity::sid::Sid; -use crate::broker::auth::PipeClient; -use crate::broker::command_builder::build_command; -use crate::broker::evaluator; -use crate::broker::executor::{CommandExecutor, ExecutionContext}; -use crate::broker::operation_tracker::OperationTracker; +use crate::auth::PipeClient; +use crate::command_builder::build_command; +use crate::evaluator; +use crate::executor::{CommandExecutor, ExecutionContext}; +use crate::operation_tracker::OperationTracker; mod connection; mod execution; @@ -283,7 +283,7 @@ impl BrokerState { #[cfg(windows)] { let operation_key = operation_id.to_string(); - match crate::broker::event_channel::open_operation_channel(&operation_key, user_sid) { + match crate::event_channel::open_operation_channel(&operation_key, user_sid) { Ok((sink, descriptor)) => { self.tracker .set_event_channel(&operation_key, sink.clone(), descriptor.clone()); @@ -529,7 +529,7 @@ mod tests { use now_policy_api as api; use super::*; - use crate::broker::executor::{ExecutionOutput, OperationCanceled, ProcessStartedCallback}; + use crate::executor::{ExecutionOutput, OperationCanceled, ProcessStartedCallback}; struct NoopExecutor; diff --git a/devolutions-agent/src/broker/server/responses.rs b/crates/now-package-broker/src/server/responses.rs similarity index 98% rename from devolutions-agent/src/broker/server/responses.rs rename to crates/now-package-broker/src/server/responses.rs index 74b27a7b1..3899f8759 100644 --- a/devolutions-agent/src/broker/server/responses.rs +++ b/crates/now-package-broker/src/server/responses.rs @@ -8,7 +8,7 @@ use now_policy_api::{ RuleId, Scope, ServerContext, Transport, }; -use crate::broker::operation_tracker::OperationTracker; +use crate::operation_tracker::OperationTracker; pub(super) fn api_version() -> ApiVersion { API_VERSION_STR.into() @@ -24,7 +24,7 @@ pub(super) fn server_context() -> ServerContext { /// Capability descriptions for every manager the broker can drive. /// /// This is the full supported set; the server filters it down to the managers actually -/// available for the requesting user (see [`crate::broker::server::BrokerState`]). +/// available for the requesting user (see [`crate::server::BrokerState`]). pub(super) fn supported_manager_capabilities() -> Vec { vec![ ManagerCapability { diff --git a/devolutions-agent/src/broker/task.rs b/crates/now-package-broker/src/task.rs similarity index 93% rename from devolutions-agent/src/broker/task.rs rename to crates/now-package-broker/src/task.rs index 100009266..490ee5ab5 100644 --- a/devolutions-agent/src/broker/task.rs +++ b/crates/now-package-broker/src/task.rs @@ -8,11 +8,11 @@ use devolutions_gateway_task::{ShutdownSignal, Task}; use tokio_util::sync::CancellationToken; use tracing::{info, warn}; -use crate::broker::executor::{self, CommandExecutor}; -use crate::broker::pipe::DEFAULT_PIPE_NAME; -use crate::broker::policy_loader; -use crate::broker::policy_watcher::{PolicyState, PolicyWatcher}; -use crate::broker::server::BrokerState; +use crate::executor::{self, CommandExecutor}; +use crate::pipe::DEFAULT_PIPE_NAME; +use crate::policy_loader; +use crate::policy_watcher::{PolicyState, PolicyWatcher}; +use crate::server::BrokerState; /// Configuration for the broker task. #[derive(Debug, Clone)] @@ -102,7 +102,7 @@ impl Task for BrokerTask { policy: RwLock::new(initial_policy), executor, pipe_name: self.config.pipe_name.clone(), - tracker: crate::broker::operation_tracker::OperationTracker::new(), + tracker: crate::operation_tracker::OperationTracker::new(), skip_signature_validation: self.config.skip_signature_validation, manager_probe_cache: Default::default(), }); @@ -153,7 +153,7 @@ impl Task for BrokerTask { let server_shutdown = shutdown.clone(); let server_handle = tokio::spawn({ let state = Arc::clone(&state); - async move { crate::broker::pipe::run_pipe_server(state, server_shutdown).await } + async move { crate::pipe::run_pipe_server(state, server_shutdown).await } }); // Wait for agent shutdown signal. diff --git a/devolutions-agent/Cargo.toml b/devolutions-agent/Cargo.toml index ef72213e4..2be961bb6 100644 --- a/devolutions-agent/Cargo.toml +++ b/devolutions-agent/Cargo.toml @@ -13,7 +13,7 @@ default = [] # Development-only feature allowing the SkipBrokerSignatureValidation debug option to take effect. # Must never be enabled for shipped builds: without it, broker client signature validation is # unconditionally enforced regardless of the configuration file contents. -dev-skip-broker-signature = [] +dev-skip-broker-signature = ["now-package-broker/dev-skip-broker-signature"] [lints] workspace = true @@ -23,12 +23,10 @@ agent-tunnel-proto = { path = "../crates/agent-tunnel-proto" } anyhow = "1" backoff = "0.4" async-trait = "0.1" -axum = { version = "0.8", default-features = false, features = ["http1", "json", "tokio", "original-uri", "matched-path"] } base64 = "0.23" bytes = "1" camino = { version = "1.1", features = ["serde1"] } ceviche = "0.7" -chrono = { version = "0.4", features = ["serde"] } ctrlc = "3.5" devolutions-agent-shared = { path = "../crates/devolutions-agent-shared" } devolutions-gateway-task = { path = "../crates/devolutions-gateway-task" } @@ -36,21 +34,14 @@ devolutions-log = { path = "../crates/devolutions-log" } futures = "0.3" hex = "0.4" hostname = "0.4" -hyper = { version = "1", features = ["http1", "server"] } -hyper-util = { version = "0.1", features = ["tokio", "server", "server-auto", "service"] } http-client-proxy = { path = "../crates/http-client-proxy" } ipnetwork = "0.20" -notify = { version = "7", default-features = false, features = ["macos_kqueue"] } -now-policy = "0.2" -now-policy-api = { version = "0.3", features = ["policy-compat"] } -now-policy-server-template = { version = "0.3", features = ["policy-compat"] } parking_lot = "0.12" prost = "0.13" prost-types = "0.13" quinn = "0.11" rand = "0.8" # FIXME(@CBenoit): maybe we don't need this crate rcgen = { version = "0.13", features = ["pem"] } -regex = "1" reqwest = { version = "0.12", default-features = false, features = ["rustls-tls-native-roots", "http2", "socks", "json"] } rustls = { version = "0.23", default-features = false, features = ["std", "ring"] } rustls-pemfile = "2.2" @@ -58,15 +49,11 @@ rustls-pki-types = "1" sha2 = "0.10" serde_json = "1" serde = { version = "1", features = ["derive"] } -serde_yaml = "0.9" -semver = "1" tap = "1.0" tempfile = "3" tokio-rustls = { version = "0.26", default-features = false, features = ["logging", "tls12", "ring"] } tokio-stream = "0.1" -tokio-util = "0.7" tonic = { version = "0.12", features = ["transport"] } -tower-service = "0.3" tracing = "0.1" url = { version = "2.5", features = ["serde"] } x509-parser = "0.16" @@ -101,10 +88,10 @@ aws-lc-rs = "1.15" time = { version = "0.3", features = ["local-offset", "macros", "parsing"] } devolutions-pedm = { path = "../crates/devolutions-pedm" } notify-debouncer-mini = "0.6" +now-package-broker = { path = "../crates/now-package-broker" } reqwest = { version = "0.12", default-features = false, features = ["rustls-tls-native-roots", "http2", "socks"] } thiserror = "2" uuid = { version = "1.17", features = ["v4"] } -widestring = "1.2" win-api-wrappers = { path = "../crates/win-api-wrappers" } [target.'cfg(windows)'.dependencies.windows] @@ -118,13 +105,7 @@ features = [ "Win32_Security_Cryptography", "Win32_Security_Authorization", "Win32_System_ApplicationInstallationAndServicing", - "Win32_System_Ioctl", - "Win32_System_IO", - "Win32_System_Pipes", "Win32_System_RemoteDesktop", - "Win32_System_Com", - "Win32_System_Console", - "Win32_UI_Shell", ] [target.'cfg(windows)'.build-dependencies] diff --git a/devolutions-agent/src/broker/mod.rs b/devolutions-agent/src/broker/mod.rs deleted file mode 100644 index a551c0a0d..000000000 --- a/devolutions-agent/src/broker/mod.rs +++ /dev/null @@ -1,20 +0,0 @@ -//! Package broker module. -//! -//! Provides policy evaluation and command execution for package operations, -//! communicating over a Windows named pipe using HTTP/1.1. - -pub(crate) mod auth; -pub mod command_builder; -pub mod evaluator; -pub mod event_channel; -pub mod executor; -pub mod operation_tracker; -pub mod pipe; -pub mod policy_loader; -pub(crate) mod policy_security; -pub mod policy_watcher; -pub mod server; -pub mod task; - -#[cfg(test)] -mod scenario_tests; diff --git a/devolutions-agent/src/broker/pipe.rs b/devolutions-agent/src/broker/pipe.rs deleted file mode 100644 index 7a4d9730a..000000000 --- a/devolutions-agent/src/broker/pipe.rs +++ /dev/null @@ -1,197 +0,0 @@ -//! Named pipe transport for Windows. -//! -//! Creates a named pipe server with appropriate ACLs and accepts connections, -//! forwarding them to the HTTP server. - -#[cfg(windows)] -mod windows_pipe { - use std::sync::Arc; - - use anyhow::Context as _; - use tokio::net::windows::named_pipe::{NamedPipeServer, ServerOptions}; - use tokio::sync::Semaphore; - use tokio_util::sync::CancellationToken; - use tracing::{error, info, warn}; - use win_api_wrappers::identity::sid::Sid; - use win_api_wrappers::security::acl::{Acl, ExplicitAccess, InheritableAcl, InheritableAclKind, Trustee}; - use win_api_wrappers::security::attributes::SecurityAttributesInit; - use windows::Win32::Foundation::GENERIC_ALL; - use windows::Win32::Security; - use windows::Win32::Security::Authorization::SET_ACCESS; - use windows::Win32::Storage::FileSystem::{FILE_GENERIC_READ, FILE_GENERIC_WRITE}; - - use crate::broker::auth::PipeClient; - use crate::broker::server::{BrokerState, build_router_for_client, serve_connection}; - - /// Default pipe name for the package broker. - pub const DEFAULT_PIPE_NAME: &str = r"\\.\pipe\Devolutions.Now.PackageBroker.v1"; - - /// Maximum number of concurrently served pipe connections. - /// - /// Connection setup performs unauthenticated work (client process identity lookups) - /// before any signature gate, so a connection flood could otherwise trigger unbounded - /// work and task spawning. While all slots are taken, no pipe instance is listening and - /// further clients fail to connect until a slot frees up. - const MAX_CONCURRENT_CONNECTIONS: usize = 16; - - /// Deadline for serving a single pipe connection, from accept to response completion. - /// - /// Each connection serves exactly one HTTP request (`keep_alive` is disabled) and all - /// endpoints respond without blocking on package operations (execution is asynchronous, - /// tracked via the operation tracker), so a healthy exchange completes well within this - /// deadline. Without it, idle clients holding their connection open without sending a - /// request would each pin a connection slot indefinitely and could exhaust the pool. - const CONNECTION_DEADLINE: std::time::Duration = std::time::Duration::from_secs(30); - - /// Start the named pipe server and accept connections until shutdown. - pub async fn run_pipe_server(state: Arc, shutdown: CancellationToken) -> anyhow::Result<()> { - let pipe_name = state.pipe_name.clone(); - info!(%pipe_name, "Starting named pipe server"); - - let connection_permits = Arc::new(Semaphore::new(MAX_CONCURRENT_CONNECTIONS)); - - let mut first_instance = true; - loop { - // Wait for a free connection slot before exposing a new pipe instance, - // bounding the number of concurrently served connections. - let permit = tokio::select! { - permit = Arc::clone(&connection_permits).acquire_owned() => { - permit.expect("the semaphore is never closed") - } - _ = shutdown.cancelled() => { - info!("Pipe server shutting down"); - return Ok(()); - } - }; - - // Create a new pipe instance for each connection. - let server = create_pipe_instance(&pipe_name, first_instance)?; - first_instance = false; - - tokio::select! { - result = server.connect() => { - match result { - Ok(()) => { - let state = Arc::clone(&state); - tokio::spawn(async move { - // The permit is held for the lifetime of the connection task. - let _permit = permit; - - let serve = async move { - // Capture the client identity off the accept loop so a slow - // lookup cannot stall accepting other connections. - let client = match PipeClient::from_connected_pipe(&server) { - Ok(client) => client, - Err(error) => { - warn!(%error, "Rejected named pipe client"); - return; - } - }; - info!("Client connected to named pipe"); - let router = build_router_for_client(state, client); - serve_connection(server, router).await; - info!("Client disconnected from named pipe"); - }; - - // Enforce a deadline so idle or slow clients cannot pin - // a connection slot indefinitely. - if tokio::time::timeout(CONNECTION_DEADLINE, serve).await.is_err() { - warn!("Closed named pipe connection: deadline exceeded"); - } - }); - } - Err(error) => { - error!(%error, "Failed to accept pipe connection"); - } - } - } - _ = shutdown.cancelled() => { - info!("Pipe server shutting down"); - return Ok(()); - } - } - } - } - - fn create_pipe_instance(pipe_name: &str, first_instance: bool) -> anyhow::Result { - let security_attributes = - build_pipe_security_attributes().context("failed to build pipe security attributes")?; - - // SAFETY: `create_with_security_attributes_raw` requires a pointer to a valid - // `SECURITY_ATTRIBUTES` that stays alive for the duration of the call. The pointer - // comes from `security_attributes` (a `win_api_wrappers::security::SecurityAttributes`), - // a local binding that owns the structure and its security descriptor and is dropped - // only at the end of this function, well after the call returns. `CreateNamedPipeW` - // copies the descriptor at creation, so the pointer is not retained afterwards. - let server = unsafe { - ServerOptions::new() - .first_pipe_instance(first_instance) - .create_with_security_attributes_raw(pipe_name, security_attributes.as_mut_ptr().cast()) - }?; - - Ok(server) - } - - /// Build a security descriptor that grants: - /// - SYSTEM: full control - /// - Administrators: full control - /// - BUILTIN\Users: read + write (allows interactive users to connect) - fn build_pipe_security_attributes() -> anyhow::Result { - let system_sid = - Sid::from_well_known(Security::WinLocalSystemSid, None).context("failed to create SYSTEM SID")?; - let admins_sid = Sid::from_well_known(Security::WinBuiltinAdministratorsSid, None) - .context("failed to create Administrators SID")?; - let users_sid = - Sid::from_well_known(Security::WinBuiltinUsersSid, None).context("failed to create Users SID")?; - - let entries = [ - ExplicitAccess { - access_permissions: GENERIC_ALL.0, - access_mode: SET_ACCESS, - inheritance: Security::ACE_FLAGS(0), - trustee: Trustee::Sid(system_sid), - }, - ExplicitAccess { - access_permissions: GENERIC_ALL.0, - access_mode: SET_ACCESS, - inheritance: Security::ACE_FLAGS(0), - trustee: Trustee::Sid(admins_sid), - }, - ExplicitAccess { - access_permissions: FILE_GENERIC_READ.0 | FILE_GENERIC_WRITE.0, - access_mode: SET_ACCESS, - inheritance: Security::ACE_FLAGS(0), - trustee: Trustee::Sid(users_sid), - }, - ]; - - let empty_acl = Acl::new().context("failed to create empty ACL")?; - let dacl = empty_acl.set_entries(&entries).context("failed to set ACL entries")?; - - let attrs = SecurityAttributesInit { - dacl: Some(InheritableAcl { - kind: InheritableAclKind::Protected, - acl: dacl, - }), - ..Default::default() - } - .init(); - - Ok(attrs) - } -} - -#[cfg(windows)] -pub use windows_pipe::*; - -/// Fallback for non-Windows (pipe transport not supported). -#[cfg(not(windows))] -pub const DEFAULT_PIPE_NAME: &str = "not-supported-on-this-platform"; - -#[cfg(not(windows))] -pub async fn run_pipe_server( - _state: std::sync::Arc, - _shutdown: tokio_util::sync::CancellationToken, -) -> anyhow::Result<()> { - anyhow::bail!("named pipe transport is only supported on Windows") -} diff --git a/devolutions-agent/src/lib.rs b/devolutions-agent/src/lib.rs index d0d05e994..b508f1889 100644 --- a/devolutions-agent/src/lib.rs +++ b/devolutions-agent/src/lib.rs @@ -5,10 +5,6 @@ use ctrlc as _; #[macro_use] extern crate tracing; -#[cfg(windows)] -pub mod broker; -#[cfg(windows)] -pub(crate) mod code_signing; pub mod config; pub mod domain_detect; pub mod enrollment; diff --git a/devolutions-agent/src/service.rs b/devolutions-agent/src/service.rs index 8b04d740b..631d793b4 100644 --- a/devolutions-agent/src/service.rs +++ b/devolutions-agent/src/service.rs @@ -2,10 +2,6 @@ use std::time::Duration; use anyhow::Context; use devolutions_agent::AgentServiceEvent; -#[cfg(windows)] -use devolutions_agent::broker::pipe::DEFAULT_PIPE_NAME; -#[cfg(windows)] -use devolutions_agent::broker::task::{BrokerTask, BrokerTaskConfig}; use devolutions_agent::config::ConfHandle; use devolutions_agent::log::AgentLog; use devolutions_agent::psu_agent::PsuAgentTask; @@ -19,6 +15,10 @@ use devolutions_gateway_task::{ChildTask, ShutdownHandle, ShutdownSignal}; use devolutions_log::{self, LogDeleterTask, LoggerGuard}; #[cfg(windows)] use devolutions_pedm::PedmTask; +#[cfg(windows)] +use now_package_broker::pipe::DEFAULT_PIPE_NAME; +#[cfg(windows)] +use now_package_broker::task::{BrokerTask, BrokerTaskConfig}; use tokio::runtime::{self, Runtime}; use tokio::sync::mpsc; diff --git a/devolutions-agent/src/updater/package.rs b/devolutions-agent/src/updater/package.rs index 3289c82d2..f0e99bbda 100644 --- a/devolutions-agent/src/updater/package.rs +++ b/devolutions-agent/src/updater/package.rs @@ -3,10 +3,12 @@ use std::ops::DerefMut; use camino::{Utf8Path, Utf8PathBuf}; +use devolutions_agent_shared::windows::code_signing::{ + certificate_sha1_thumbprint, is_devolutions_certificate_thumbprint, +}; use uuid::Uuid; use win_api_wrappers::utils::WideString; -use crate::code_signing::{certificate_sha1_thumbprint, is_devolutions_certificate_thumbprint}; use crate::updater::io::remove_file_on_reboot; use crate::updater::{AGENT_UPDATE_IN_PROGRESS, Product, UpdaterCtx, UpdaterError}; From 1dd9b44f99fabba24928361b890162ceb2d62368 Mon Sep 17 00:00:00 2001 From: Krista House Date: Tue, 11 Aug 2026 23:57:31 -0400 Subject: [PATCH 2/8] fix(webapp): restore hostname suggestion contrast (#1929) The hostname suggestion list did not set its own text colour, so options were painted with whatever colour the PrimeNG theme supplied. When that colour was close to the panel background the suggestions were invisible until the pointer moved over them. The styles meant to prevent this had been written against PrimeNG 18 class names and stopped matching anything when the app moved to PrimeNG 20. They are now ported to the current class names and the suggestion list is themed like every other dropdown, so options stay readable in both light and dark themes. Issue: DGW-337 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../assets/css/override/_p-autocomplete.scss | 224 +++--------------- .../form/web-client-form.component.html | 4 - .../src/client/app/styles/dvl.preset.ts | 67 ++++++ 3 files changed, 105 insertions(+), 190 deletions(-) diff --git a/webapp/apps/gateway-ui/src/assets/css/override/_p-autocomplete.scss b/webapp/apps/gateway-ui/src/assets/css/override/_p-autocomplete.scss index 255506922..4dae3cd93 100644 --- a/webapp/apps/gateway-ui/src/assets/css/override/_p-autocomplete.scss +++ b/webapp/apps/gateway-ui/src/assets/css/override/_p-autocomplete.scss @@ -9,7 +9,9 @@ width: 100%; } - &.p-autocomplete-dd { + /* The dropdown button floats over the input, so the input keeps its + rounded corners instead of butting up against the button. */ + &:has(.p-autocomplete-dropdown) { .p-autocomplete-input { padding-right: 2em; border-top-right-radius: $borderRadius; @@ -21,18 +23,12 @@ color: $input-txt-border-focus-color; } } - - &.p-autocomplete-multiple { - .p-autocomplete-multiple-container { - padding-right: 2em; - } - } } .p-autocomplete-dropdown { width: 2em; background: transparent !important; - color: var(--p-select-dropdown-color); + color: var(--p-autocomplete-dropdown-color); border: 0 none; position: absolute; right: 0; @@ -44,185 +40,63 @@ color: $input-txt-border-focus-color; } } - - &.p-autocomplete-multiple { - .p-autocomplete-multiple-container:not(.p-disabled).p-focus + .p-autocomplete-dropdown { - .pi { - color: $input-txt-border-focus-color; - } - } - - .p-autocomplete-multiple-container { - padding: .429em; - - &:not(.p-disabled):hover { - border-color: $input-txt-border-focus-color; - } - - &:not(.p-disabled).p-focus { - border-color: $input-txt-border-focus-color; - } - - .p-autocomplete-input-token { - padding: 0 .25em 0 0; - vertical-align: top; - - input { - border: none; - color: $default-text-color; - font-family: $fontFamily; - font-size: $fontSize; - padding: 0; - margin: 0; - } - } - - .p-autocomplete-token { - font-size: $fontSize; - padding-right: 8px; - line-height: 24px; - background: var(--input-chips-bg-color); - color: var(--input-chips-txt-color); - margin-right: 4px; - border-radius: 4px; - max-height: 24px; - - & > div { - display: flex; - align-items: center; - padding: 3px; - } - } - } - } } -.p-autocomplete-panel { - background-color: var(--main-body-bg); +.p-autocomplete-overlay { + background: var(--main-body-bg); color: $dropdown-item-txt-color; - border: 1px solid $dropdown-overlay-border-color; + border: 1px solid $input-txt-border-focus-color; box-shadow: $defaultBoxShadow; margin-top: -1px; /* hack to simulate a thin line */ border-radius: 0 0 $borderRadius $borderRadius; @include custom-scrollbar(); - .p-autocomplete-items { - padding: 0; - - .p-autocomplete-empty-message { - align-items: center; - display: flex; - height: 36px; - justify-content: flex-start; - padding-left: 10px; - } - - .p-autocomplete-item { - align-items: center; - background: var(--main-body-bg); - display: flex; - margin: 0; - padding: $listItemPadding; - color: $default-text-color; - @include border-radius(0); - - .user-or-group-container { - display: flex; - align-items: center; - padding: 5px 0; - - label { - @include text-ellipsis(); - width: 100%; - margin-left: 10px; - line-height: 1.6; - color: $grid-txt-color; - } - } - } - - .p-autocomplete-item:hover, .p-autocomplete-item.p-highlight { - background-color: $dropdown-user-group-selected-bg-hover-color; - color: $default-text-color; - } - } -} - -.p-fluid { - .p-autocomplete { - &.p-autocomplete-multiple.p-autocomplete-dd { - .p-autocomplete-multiple-container { - width: 100%; - } - } + .p-autocomplete-list-container { + border-radius: 0 0 $borderRadius $borderRadius; - &.p-autocomplete-dd { - .p-inputtext { - width: 100%; - } - } + @include custom-scrollbar(); } } -.p-autocomplete.p-autocomplete-multiple { - display: block; - - .p-inputtext { - - input:hover, - input:active, - input:focus, - input:active:focus { - border: none !important; - transition: none !important; - box-shadow: none !important; - } - } - - .p-autocomplete-token { - margin: 2px; - - @include transition(ease-in-out background-color .15s, ease-in-out color .15s); +.p-autocomplete-list { + background: var(--main-body-bg); + color: $dropdown-item-txt-color; + padding: 0; - > div { - margin-right: 5px; - } + .p-autocomplete-option { + align-items: center; + background: var(--main-body-bg); + color: $dropdown-item-txt-color; + display: flex; + margin: 0; + padding: $listItemPadding; - .p-autocomplete-token-icon { - font-size: 16px; - right: 3px; - vertical-align: middle; - color: var(--input-chips-icon-color); + @include border-radius(0); - @include transition(ease-in-out color .15s); + &.p-autocomplete-option-selected { + font-weight: 600; + background-color: $dropdown-item-selected-bg-color; + color: $dropdown-item-selected-txt-color; } - &:hover:not(:disabled) { - background-color: var(--input-chips-bg-hover-color); - color: var(--input-chips-txt-hover-color); - - .p-autocomplete-token-icon { - color: var(--input-chips-icon-hover-color); - } + &[data-p-focused='true'], + &.p-focus { + background-color: $dropdown-item-selected-bg-hover-color; + color: $dropdown-item-txt-color; } - &:active:not(:disabled) { - background-color: var(--input-chips-bg-pressed-color); - color: var(--input-chips-txt-color); - - .p-autocomplete-token-icon { - color: var(--input-chips-icon-color); - } + &:not(.p-autocomplete-option-selected):not(.p-disabled):hover { + background-color: $dropdown-item-selected-bg-hover-color; } } - .p-autocomplete-multiple-container { - padding: 5px 0 5px 8px; - width: 100%; - max-height: 160px; - - @include custom-scrollbar(); + .p-autocomplete-empty-message { + align-items: center; + display: flex; + height: 36px; + justify-content: flex-start; + padding-left: 10px; } } @@ -232,29 +106,7 @@ border-bottom-left-radius: 0 !important; } - .p-autocomplete-multiple-container { - border-bottom-left-radius: 0 !important; - border-bottom-right-radius: 0 !important; - } - .p-autocomplete-dropdown { border-color: $default-btn-bg-pressed-color; } } - -.p-autocomplete-option-selected { - background: $dropdown-item-selected-bg-color; - color: $dropdown-item-selected-txt-color; -} - -.p-autocomplete-list { - background: var(--main-body-bg); - color: $dropdown-item-txt-color; -} - -.p-autocomplete-overlay { - background: var(--main-body-bg); - color: $dropdown-item-txt-color; - border-radius: 0 0 $borderRadius $borderRadius; - border: 1px solid $input-txt-border-focus-color; -} \ No newline at end of file diff --git a/webapp/apps/gateway-ui/src/client/app/modules/web-client/form/web-client-form.component.html b/webapp/apps/gateway-ui/src/client/app/modules/web-client/form/web-client-form.component.html index 747260805..9dbeafc86 100644 --- a/webapp/apps/gateway-ui/src/client/app/modules/web-client/form/web-client-form.component.html +++ b/webapp/apps/gateway-ui/src/client/app/modules/web-client/form/web-client-form.component.html @@ -54,15 +54,11 @@ diff --git a/webapp/apps/gateway-ui/src/client/app/styles/dvl.preset.ts b/webapp/apps/gateway-ui/src/client/app/styles/dvl.preset.ts index f840ccc73..accb86cee 100644 --- a/webapp/apps/gateway-ui/src/client/app/styles/dvl.preset.ts +++ b/webapp/apps/gateway-ui/src/client/app/styles/dvl.preset.ts @@ -409,6 +409,73 @@ const DvlPreset = definePreset(Aura, { }, }, + autocomplete: { + root: { + background: 'var(--bg-100)', + disabledBackground: 'var(--bg-300)', + color: 'var(--text-400)', + disabledColor: 'var(--text-100)', + borderColor: 'var(--border-200)', + hoverBorderColor: 'var(--accent-brand-400)', + focusBorderColor: 'var(--accent-brand-300)', + invalidBorderColor: 'var(--accent-danger-400)', + borderRadius: '4px', + paddingX: '0.5rem', + paddingY: '0.375rem', + shadow: 'none', + transitionDuration: '0.2s', + placeholderColor: 'var(--text-100)', + }, + + dropdown: { + width: '2.5rem', + color: 'var(--text-200)', + hoverColor: 'var(--text-300)', + activeColor: 'var(--text-400)', + borderColor: 'var(--border-200)', + borderRadius: '4px', + }, + + overlay: { + background: 'var(--bg-200)', + color: 'var(--text-300)', + borderColor: 'var(--accent-brand-300)', + borderRadius: '4px', + shadow: '0 4px 6px var(--alt-600)', + }, + + list: { + padding: '0', + gap: '0', + }, + + option: { + color: 'var(--text-300)', + focusBackground: 'var(--border-contrast-brand-100)', + focusColor: 'var(--text-300)', + selectedBackground: 'var(--border-contrast-brand-200)', + selectedColor: 'var(--text-300)', + selectedFocusBackground: 'var(--border-contrast-brand-200)', + selectedFocusColor: 'var(--text-300)', + padding: '0.5rem 0.75rem', + borderRadius: '0', + }, + + optionGroup: { + background: 'var(--bg-200)', + color: 'var(--text-300)', + padding: '0.5rem 0.75rem', + }, + + chip: { + borderRadius: '4px', + }, + + emptyMessage: { + padding: '0.5rem 0.75rem', + }, + }, + // // CARD / DIALOG // From 3bca19162af1fd95c35c73cf65c0f1d824fdaf79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20B=C3=A9dard?= Date: Tue, 11 Aug 2026 23:58:35 -0400 Subject: [PATCH 3/8] ci: warm the Artifactory cache for every published npm package (#1919) The step was hardcoded to gateway-client, so the four other packages published by this workflow were never pulled into npm-remote-cache and consumers got a 403 on their first install. --- .github/workflows/publish-libraries.yml | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/.github/workflows/publish-libraries.yml b/.github/workflows/publish-libraries.yml index c267366f6..2679d4ac3 100644 --- a/.github/workflows/publish-libraries.yml +++ b/.github/workflows/publish-libraries.yml @@ -240,7 +240,23 @@ jobs: shell: pwsh - name: Update Artifactory Cache - run: gh workflow run update-artifactory-cache.yml --repo Devolutions/scheduled-tasks --field package_name="gateway-client" + run: | + Set-PSDebug -Trace 1 + + $Files = Get-ChildItem -Recurse npm-packages/*.tgz + + foreach ($File in $Files) { + $Manifest = (tar -xzOf "$File" package/package.json | Out-String) | ConvertFrom-Json + $PackageName = $Manifest.name -Replace '^@devolutions/', '' + + Write-Host "Updating Artifactory cache for $($Manifest.name)..." + gh workflow run update-artifactory-cache.yml --repo Devolutions/scheduled-tasks --field package_name="$PackageName" + + if ($LastExitCode -Ne 0) { + throw "Failed to trigger the Artifactory cache update for $($Manifest.name)" + } + } + shell: pwsh env: GH_TOKEN: ${{ secrets.DEVOLUTIONSBOT_WRITE_TOKEN }} From 1c06391a0ce6a673861cc7776e97a97731fa99f2 Mon Sep 17 00:00:00 2001 From: "irvingouj@Devolutions" Date: Wed, 12 Aug 2026 11:52:11 -0400 Subject: [PATCH 4/8] feat(dgw): support VMConnect through RDCleanPath (#1372) Support the explicit VMConnect RDCleanPath shape from Devolutions/IronRDP#1505. Generic PCBs keep the ordinary X.224-first path. VMConnect requests carry a Unicode payload with no X.224; Gateway encodes the binary PCB, writes it before TLS (bounded by the MS-RDPEPS 10s deadline), then leaves CredSSP and X.224 to the client. Credential injection is rejected for this ordering. Depends-on: Devolutions/IronRDP#1505 Issue: Devolutions/IronRDP#1505 --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- devolutions-gateway/src/rd_clean_path.rs | 272 +++++++++++++++++++++-- devolutions-gateway/src/upstream.rs | 5 +- 2 files changed, 254 insertions(+), 23 deletions(-) diff --git a/devolutions-gateway/src/rd_clean_path.rs b/devolutions-gateway/src/rd_clean_path.rs index d36ebabe4..ca7376bd8 100644 --- a/devolutions-gateway/src/rd_clean_path.rs +++ b/devolutions-gateway/src/rd_clean_path.rs @@ -1,15 +1,20 @@ use std::io::{self, ErrorKind}; use std::net::SocketAddr; use std::sync::Arc; +use std::time::Duration; use anyhow::Context as _; use ironrdp_pdu::nego; use ironrdp_rdcleanpath::RDCleanPathPdu; +use ironrdp_rdcleanpath::der::asn1::OctetString; use tap::prelude::*; use thiserror::Error; use tokio::io::{AsyncRead, AsyncReadExt as _, AsyncWrite, AsyncWriteExt as _}; use tracing::field; +/// MS-RDPEPS upper bound for transmitting the complete Preconnection Blob after TCP connect. +const PCB_TRANSMIT_DEADLINE: Duration = Duration::from_secs(10); + use crate::config::Conf; use crate::credential_injection_kdc::{CredentialInjectionKdc, CredentialService}; use crate::proxy::Proxy; @@ -229,14 +234,87 @@ struct ConnectedRdpServer { tls_stream: tokio_rustls::client::TlsStream, server_addr: SocketAddr, selected_target: TargetAddr, - x224_rsp: Vec, + x224_rsp: Option>, +} + +/// Explicit VMConnect request: no X.224 and a non-empty Unicode PCB V2 payload. +/// +/// Matches IronRDP `RDCleanPathMessage::VmConnectRequest` / `new_vmconnect_request`. +fn is_vmconnect_request(cleanpath_pdu: &RDCleanPathPdu) -> bool { + cleanpath_pdu.x224_connection_pdu.is_none() + && cleanpath_pdu + .preconnection_blob + .as_ref() + .is_some_and(|pcb| !pcb.trim().is_empty()) +} + +/// Encode the Hyper-V PCB V2 that the proxy writes before TLS. +/// +/// `payload` is the opaque Unicode string from RDCleanPath (`GUID` or `GUID;EnhancedMode=1`). +/// +/// Encoded locally rather than via `ironrdp-pdu` 0.9.0: that crates.io release counts `cchPCB` +/// with `chars().count()`, which under-counts non-BMP code points (UTF-16 surrogates). IronRDP +/// master fixed this to `encode_utf16().count()` but has not published a crates.io bump yet, and +/// its MSRV is ahead of Gateway. Match the fixed wire shape here so opaque Unicode payloads stay +/// well-formed. +fn encode_vmconnect_pcb_v2(payload: String) -> anyhow::Result> { + // PCB V2 layout (little-endian): + // cbSize u32 | flags u32 | version u32 | id u32 | cchPCB u16 | wszPCB UTF-16LE + NUL + const FIXED_PART_SIZE: usize = + 4 /* cbSize */ + 4 /* flags */ + 4 /* version */ + 4 /* id */; + const VERSION_V2: u32 = 2; + + let utf16: Vec = payload.encode_utf16().chain(core::iter::once(0)).collect(); + let cch_pcb = u16::try_from(utf16.len()).context("VMConnect PCB payload too long")?; + let utf16_byte_len = utf16.len().checked_mul(2).context("VMConnect PCB payload too long")?; + let total_size = FIXED_PART_SIZE + .checked_add(2 /* cchPCB */) + .and_then(|n| n.checked_add(utf16_byte_len)) + .context("VMConnect PCB payload too long")?; + let cb_size = u32::try_from(total_size).context("VMConnect PCB payload too long")?; + + let mut out = Vec::with_capacity(total_size); + out.extend_from_slice(&cb_size.to_le_bytes()); + out.extend_from_slice(&0u32.to_le_bytes()); // flags + out.extend_from_slice(&VERSION_V2.to_le_bytes()); + out.extend_from_slice(&0u32.to_le_bytes()); // id + out.extend_from_slice(&cch_pcb.to_le_bytes()); + for unit in utf16 { + out.extend_from_slice(&unit.to_le_bytes()); + } + debug_assert_eq!(out.len(), total_size); + Ok(out) +} + +/// Cert-chain-only success response after VMConnect PCB + TLS (no X.224). +/// +/// Wire-compatible with IronRDP `RDCleanPathMessage::VmConnectResponse`. +fn build_vmconnect_response( + server_addr: String, + x509_chain: impl IntoIterator>, +) -> anyhow::Result { + Ok(RDCleanPathPdu { + version: ironrdp_rdcleanpath::VERSION_1, + server_cert_chain: Some( + x509_chain + .into_iter() + .map(OctetString::new) + .collect::>() + .context("build VMConnect RDCleanPath cert chain")?, + ), + server_addr: Some(server_addr), + ..RDCleanPathPdu::default() + }) } -/// Establish a connection to the RDP server: route (agent/direct) → connect → X224 → TLS. +/// Establish a connection to the RDP server and perform the requested front sequence. /// /// The routing pipeline (explicit agent → subnet/domain match → direct) is shared with /// the WebSocket forwarders in [`crate::upstream`]; here we just do the RDP-specific -/// PCB + X224 + TLS upgrade on top of whatever leg that returns. +/// ordinary PCB + X224 + TLS or VMConnect PCB + TLS upgrade on top of whatever leg that returns. +/// +/// VMConnect is detected from the existing VERSION_1 fields: X.224 absent and a non-empty +/// `preconnection_blob` (Unicode PCB V2 payload). No IronRDP message-helper dependency. async fn connect_rdp_server( claims: &AssociationTokenClaims, cleanpath_pdu: RDCleanPathPdu, @@ -267,24 +345,64 @@ async fn connect_rdp_server( debug!(%selected_target, "Connected to destination server"); tracing::Span::current().record("target", selected_target.to_string()); - // Send preconnection blob if applicable. - if let Some(pcb) = cleanpath_pdu.preconnection_blob { - server_stream.write_all(pcb.as_bytes()).await?; - } - - // Send X224 connection request. - let x224_req = cleanpath_pdu - .x224_connection_pdu - .context("request is missing X224 connection PDU") - .map_err(CleanPathError::BadRequest)?; - server_stream.write_all(x224_req.as_bytes()).await?; - - trace!("Receiving X224 response"); + // MS-RDPEPS: complete the PCB write within 10s of TCP connect. Bound the front write(s) + // from this point so a stalled tunnel/target cannot hold the PCB open indefinitely. + let front_deadline = tokio::time::Instant::now() + PCB_TRANSMIT_DEADLINE; + + let x224_rsp = if is_vmconnect_request(&cleanpath_pdu) { + // Client sent Unicode PCB payload only; proxy encodes binary PCB V2 and skips X.224. + let pcb_payload = cleanpath_pdu + .preconnection_blob + .context("VMConnect request missing preconnection_blob") + .map_err(CleanPathError::BadRequest)?; + let pcb = encode_vmconnect_pcb_v2(pcb_payload).map_err(CleanPathError::BadRequest)?; + debug!(pcb_len = pcb.len(), "Writing encoded VMConnect PCB before TLS"); + tokio::time::timeout_at(front_deadline, async { + server_stream.write_all(&pcb).await?; + // Ensure the Hyper-V listener sees the PCB before ClientHello is queued + // (especially on agent-tunnel legs that may buffer). + server_stream.flush().await + }) + .await + .map_err(|_| { + CleanPathError::Io(io::Error::new( + ErrorKind::TimedOut, + "timed out writing VMConnect preconnection blob", + )) + })??; + None + } else { + // Ordinary: optional legacy complete PCB bytes, then X.224 CR/CC, then TLS. + tokio::time::timeout_at(front_deadline, async { + if let Some(pcb) = cleanpath_pdu.preconnection_blob { + server_stream.write_all(pcb.as_bytes()).await?; + } - let x224_rsp = read_x224_response(&mut server_stream) + let x224_req = cleanpath_pdu + .x224_connection_pdu + .context("request is missing X224 connection PDU") + .map_err(CleanPathError::BadRequest)?; + server_stream.write_all(x224_req.as_bytes()).await?; + server_stream.flush().await?; + Ok::<_, CleanPathError>(()) + }) .await - .with_context(|| format!("read X224 response from {selected_target}")) - .map_err(CleanPathError::BadRequest)?; + .map_err(|_| { + CleanPathError::Io(io::Error::new( + ErrorKind::TimedOut, + "timed out writing RDCleanPath front sequence", + )) + })??; + + trace!("Receiving X224 response"); + + Some( + read_x224_response(&mut server_stream) + .await + .with_context(|| format!("read X224 response from {selected_target}")) + .map_err(CleanPathError::BadRequest)?, + ) + }; trace!("Establishing TLS connection with server"); @@ -371,6 +489,7 @@ async fn handle_with_credential_injection( } = connect_rdp_server(&claims, cleanpath_pdu, agent_tunnel_handle.as_ref()) .await .context("RDCleanPath connection failed")?; + let x224_rsp = x224_rsp.context("RDCleanPath credential injection requires X.224")?; // Retrieve the Gateway TLS public key that must be used for client-proxy CredSSP later on. let gateway_cert_chain_handle = tokio::spawn(crate::tls::get_cert_chain_for_acceptor_cached( @@ -544,6 +663,14 @@ pub async fn handle( && let Some(entry) = credentials.get(jti) && entry.mapping.is_some() { + // VMConnect needs pre-X.224 CredSSP against the Hyper-V host cert on the client. + // Proxy CredSSP MITM is X.224-first and is not supported for this ordering. + if is_vmconnect_request(&cleanpath_pdu) { + let response = RDCleanPathPdu::new_http_error(400); + send_clean_path_response(&mut client_stream, &response).await?; + anyhow::bail!("credential injection is not supported for VMConnect RDCleanPath"); + } + let credential_injection_kdc = credentials.kdc_for(jti)?; anyhow::ensure!(token == credential_injection_kdc.raw_token(), "token mismatch"); debug!( @@ -616,8 +743,14 @@ pub async fn handle( trace!("Sending RDCleanPath response"); - let rdcleanpath_rsp = RDCleanPathPdu::new_response(server_addr.to_string(), x224_rsp, x509_chain) - .context("build RDCleanPath response")?; + // Ordinary responses include X.224 CC. VMConnect responses are cert-chain only; the client + // runs CredSSP then X.224 on the upgraded path. + let rdcleanpath_rsp = if let Some(x224_rsp) = x224_rsp { + RDCleanPathPdu::new_response(server_addr.to_string(), x224_rsp, x509_chain) + .context("build RDCleanPath response")? + } else { + build_vmconnect_response(server_addr.to_string(), x509_chain).context("build VMConnect RDCleanPath response")? + }; send_clean_path_response(&mut client_stream, &rdcleanpath_rsp).await?; @@ -816,3 +949,100 @@ impl From<&io::Error> for WsaError { } } } + +#[cfg(test)] +mod tests { + use super::*; + + fn empty_x224() -> OctetString { + OctetString::new(vec![0x03, 0x00, 0x00, 0x13]).expect("static X.224 bytes") + } + + #[test] + fn detects_vmconnect_when_pcb_payload_present_without_x224() { + let pdu = RDCleanPathPdu { + version: ironrdp_rdcleanpath::VERSION_1, + destination: Some("10.10.0.3:2179".to_owned()), + proxy_auth: Some("token".to_owned()), + preconnection_blob: Some("21c82e1f-2368-43d5-9cb6-a7c99c449bba;EnhancedMode=1".to_owned()), + ..RDCleanPathPdu::default() + }; + assert!(is_vmconnect_request(&pdu)); + } + + #[test] + fn ordinary_request_with_x224_is_not_vmconnect() { + let pdu = RDCleanPathPdu { + version: ironrdp_rdcleanpath::VERSION_1, + destination: Some("10.10.0.3:3389".to_owned()), + proxy_auth: Some("token".to_owned()), + preconnection_blob: Some("legacy-pcb-bytes".to_owned()), + x224_connection_pdu: Some(empty_x224()), + ..RDCleanPathPdu::default() + }; + assert!(!is_vmconnect_request(&pdu)); + } + + #[test] + fn empty_or_whitespace_pcb_without_x224_is_not_vmconnect() { + for pcb in [None, Some(String::new()), Some(" ".to_owned())] { + let pdu = RDCleanPathPdu { + version: ironrdp_rdcleanpath::VERSION_1, + destination: Some("10.10.0.3:2179".to_owned()), + proxy_auth: Some("token".to_owned()), + preconnection_blob: pcb, + ..RDCleanPathPdu::default() + }; + assert!(!is_vmconnect_request(&pdu)); + } + } + + #[test] + fn encodes_enhanced_pcb_v2_matching_lab_size() { + // Lab GUID with EnhancedMode; IronRDP observed 122-byte PCB on the wire. + let payload = "21c82e1f-2368-43d5-9cb6-a7c99c449bba;EnhancedMode=1".to_owned(); + let bytes = encode_vmconnect_pcb_v2(payload.clone()).expect("encode"); + assert_eq!(bytes.len(), 122); + + let decoded: ironrdp_pdu::pcb::PreconnectionBlob = ironrdp_core::decode(&bytes).expect("decode round-trip"); + assert_eq!(decoded.id, 0); + assert_eq!(decoded.version, ironrdp_pdu::pcb::PcbVersion::V2); + assert_eq!(decoded.v2_payload.as_deref(), Some(payload.as_str())); + } + + #[test] + fn encodes_basic_pcb_v2_matching_lab_size() { + let payload = "21c82e1f-2368-43d5-9cb6-a7c99c449bba".to_owned(); + let bytes = encode_vmconnect_pcb_v2(payload).expect("encode"); + assert_eq!(bytes.len(), 92); + } + + #[test] + fn encodes_non_bmp_payload_with_utf16_code_unit_cch() { + // U+1F600 needs a UTF-16 surrogate pair (2 code units, 1 scalar). + // crates.io ironrdp-pdu 0.9.0 would set cchPCB = chars+NUL = 5; correct is 6. + let payload = "vm-\u{1F600}".to_owned(); + assert_eq!(payload.chars().count(), 4); + assert_eq!(payload.encode_utf16().count(), 5); + + let bytes = encode_vmconnect_pcb_v2(payload.clone()).expect("encode"); + let cch = u16::from_le_bytes([bytes[16], bytes[17]]); + assert_eq!(cch, 6, "cchPCB must count UTF-16 code units including NUL"); + assert_eq!(bytes.len(), 16 + 2 + usize::from(cch) * 2); + + let decoded: ironrdp_pdu::pcb::PreconnectionBlob = ironrdp_core::decode(&bytes).expect("decode round-trip"); + assert_eq!(decoded.v2_payload.as_deref(), Some(payload.as_str())); + } + + #[test] + fn vmconnect_response_has_cert_chain_without_x224() { + let rsp = build_vmconnect_response("10.10.0.3:2179".to_owned(), [vec![0xDE, 0xAD], vec![0xBE, 0xEF]]) + .expect("build response"); + + assert_eq!(rsp.version, ironrdp_rdcleanpath::VERSION_1); + assert_eq!(rsp.server_addr.as_deref(), Some("10.10.0.3:2179")); + assert!(rsp.x224_connection_pdu.is_none()); + assert_eq!(rsp.server_cert_chain.as_ref().map(|c| c.len()).unwrap_or(0), 2); + assert!(rsp.error.is_none()); + } +} diff --git a/devolutions-gateway/src/upstream.rs b/devolutions-gateway/src/upstream.rs index 3716d5d33..c6f5d9ad0 100644 --- a/devolutions-gateway/src/upstream.rs +++ b/devolutions-gateway/src/upstream.rs @@ -10,8 +10,9 @@ //! 2. On the first successful connection, optionally wrap in client TLS. //! //! The two consumer patterns differ only in whether they want the TLS wrap -//! applied here (fwd.rs) or manage their own TLS upgrade (rd_clean_path.rs does -//! X224 first, then TLS). Both share `UpstreamLeg` and [`connect_upstream`]. +//! applied here (fwd.rs) or manage their own TLS upgrade in `rd_clean_path.rs` +//! (ordinary: optional PCB + X.224 then TLS; VMConnect: PCB then TLS, no X.224 +//! on the proxy). Both share `UpstreamLeg` and [`connect_upstream`]. use std::net::SocketAddr; use std::pin::Pin; From 91992cadd505aee8515dadb4954c9b5fb43c5d22 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 13 Aug 2026 12:59:57 +0900 Subject: [PATCH 5/8] build(deps): bump openssl-probe from 0.1.6 to 0.2.1 (#1926) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bumps [openssl-probe](https://github.com/rustls/openssl-probe) from 0.1.6 to 0.2.1.
Release notes

Sourced from openssl-probe's releases.

0.2.1

  • Support for OpenHarmony.
  • Corrections to crate metadata.

What's Changed

0.2.0 is the first release after openssl-probe maintenance has been handed over to the rustls team. Thanks to @​alexcrichton for creating and maintaining it for the past 9 years. We're happy to address any feedback you have for this crate.

Breaking changes

  • ProbeResult::cert_dir is now a Vec<PathBuf> rather than an Option<PathBuf>, allowing the library to yield multiple suggestions for directories which may contain certificate files.
  • Rather than using a single list of locations for certificate files and certificate directories, openssl-probe now uses much shorter per-platform lists. This should make the API faster and make it less likely to accidentally pick up locations that are unidiomatic for the platform.
  • Removed deprecated API

What's Changed

Commits
  • 9181752 Prepare 0.2.1
  • 2a23322 docs: clarify lib description, update README
  • 5e18d53 feat: add openharmony platform preset certs folder
  • df769f4 Update repo URL in Cargo metadata
  • cc52ac7 ci: check cargo-deny (and fix up SPDX metadata)
  • 4cfa095 ci: check semver compatibility
  • 04e7058 ci: check clippy
  • fbce324 ci: check code formatting
  • 11fba1b ci: setup duplicate workflow cancellation
  • a44b6f1 ci: restrict workflow permissions
  • Additional commits viewable in compare view

[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=openssl-probe&package-manager=cargo&previous-version=0.1.6&new-version=0.2.1)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) ---
Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- Cargo.lock | 12 +++--------- jetsocat/Cargo.toml | 2 +- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 783656038..662cfb151 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3761,7 +3761,7 @@ dependencies = [ "mcp-proxy", "native-tls", "openssl", - "openssl-probe 0.1.6", + "openssl-probe", "proxy-http", "proxy-socks", "proxy-types", @@ -4484,7 +4484,7 @@ dependencies = [ "libc", "log", "openssl", - "openssl-probe 0.2.1", + "openssl-probe", "openssl-sys", "schannel", "security-framework", @@ -5011,12 +5011,6 @@ dependencies = [ "syn 2.0.118", ] -[[package]] -name = "openssl-probe" -version = "0.1.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d05e27ee213611ffe7d6348b942e8f942b37114c00cc03cec254295a4a17852e" - [[package]] name = "openssl-probe" version = "0.2.1" @@ -6569,7 +6563,7 @@ version = "0.8.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "dab5152771c58876a2146916e53e35057e1a4dfa2b9df0f0305b07f611fdea4d" dependencies = [ - "openssl-probe 0.2.1", + "openssl-probe", "rustls-pki-types", "schannel", "security-framework", diff --git a/jetsocat/Cargo.toml b/jetsocat/Cargo.toml index 9501bfa26..3e7daa802 100644 --- a/jetsocat/Cargo.toml +++ b/jetsocat/Cargo.toml @@ -64,7 +64,7 @@ tinyjson = "2.5" # Small JSON library; used to avoid including a bigger one like native-tls = { version = "0.2", optional = true } # Same dependency as tokio-tungstenite rustls = { version = "0.23", optional = true, default-features = false, features = ["tls12", "std", "ring", "logging"] } # Same dependency as tokio-tungstenite rustls-native-certs = { version = "0.8", optional = true } # Same dependency as tokio-tungstenite -openssl-probe = "0.1" # Same dependency as rustls-native-certs +openssl-probe = "0.2" # Same dependency as rustls-native-certs rustls-pemfile = "2.2" # Same dependency as rustls-native-certs base64 = "0.23" # Same dependency as rustls-pemfile From 1a4616e46df130510fdc87628202990069e2ebd8 Mon Sep 17 00:00:00 2001 From: Vladyslav Nikonov Date: Thu, 13 Aug 2026 20:15:59 +0300 Subject: [PATCH 6/8] ci: install PsExec directly with checksum verification (#1933) The pstools Chocolatey package downloads PSTools.zip from Microsoft at install time and pins a checksum that breaks whenever Microsoft updates the zip; the latest package version currently fails this way and breaks the PEDM simulator CI job, while older versions skip verification entirely. Download PSTools.zip directly from Microsoft and verify it against a checksum pinned in the workflow, so installs stay integrity-checked and the hash is bumped deliberately when Microsoft publishes a new PSTools. --------- Co-authored-by: Vladyslav Nikonov Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .github/workflows/ci.yml | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7f64dde9d..113ebd31d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1197,7 +1197,24 @@ jobs: - name: Prepare runner run: | rustup toolchain install nightly - choco install pstools --yes + + # The pstools Chocolatey package is unreliable: it downloads PSTools.zip from + # Microsoft at install time, and its pinned checksum breaks whenever Microsoft + # updates the zip. Download it directly and verify against our own checksum + # instead; bump the hash deliberately when Microsoft publishes a new PSTools. + - name: Install PsExec + shell: pwsh + run: | + $expectedHash = '4F49964CC9CBAC2B5D87BDC8F9526012E9C4B243D8B7D0C0BB51F254A721CA2E' + $zipPath = Join-Path $env:RUNNER_TEMP 'PSTools.zip' + $toolsDir = Join-Path $env:RUNNER_TEMP 'PSTools' + Invoke-WebRequest -Uri 'https://download.sysinternals.com/files/PSTools.zip' -OutFile $zipPath + $actualHash = (Get-FileHash -Path $zipPath -Algorithm SHA256).Hash + if ($actualHash -ne $expectedHash) { + throw "PSTools.zip checksum mismatch: expected $expectedHash, got $actualHash" + } + Expand-Archive -Path $zipPath -DestinationPath $toolsDir + Add-Content -Path $env:GITHUB_PATH -Value $toolsDir # The Docker service for Windows containers may not be started yet at this point. # Starting it explicitly avoids flaky failures when the daemon is not ready. From 6bfe09d15a6c7dc94e8040d86c5e02073fd5f4a5 Mon Sep 17 00:00:00 2001 From: Vladyslav Nikonov Date: Thu, 13 Aug 2026 21:00:46 +0300 Subject: [PATCH 7/8] build(agent): update now-policy-api to 0.3.1 (#1928) --- Cargo.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 662cfb151..7adf897e5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4770,9 +4770,9 @@ dependencies = [ [[package]] name = "now-policy-api" -version = "0.3.0" +version = "0.3.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "11808c712bda38e8cee0ec0a6902bbfe63583f475608ee6bcf8ffd848473ab06" +checksum = "fa0817fd85c0a6b0173e2b837fa2b369be684c93c11fed1b5182284021411839" dependencies = [ "chrono", "derive_more", From dc9e8f7eb5fccd10659f7692c40e7a2673f1e7ec Mon Sep 17 00:00:00 2001 From: Vladyslav Nikonov Date: Tue, 11 Aug 2026 14:31:03 +0300 Subject: [PATCH 8/8] refactor(agent): extract package broker into dedicated crate Move the package broker subsystem out of the devolutions-agent binary into a new crates/now-package-broker crate, keeping the agent manifest lean and giving the broker-only dependency set (axum, hyper-util, notify, now-policy, ...) its own home. The crate compiles to an empty library on non-Windows platforms. Extract code_signing into devolutions-agent-shared so it is shared between the updater and the broker, which previously reached back into the agent crate for it. The agent forwards the development-only dev-skip-broker-signature feature to the broker crate. Issue: DGW-417 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .gitattributes | 4 +- Cargo.lock | 54 +++-- crates/devolutions-agent-shared/Cargo.toml | 4 + .../src/windows}/code_signing.rs | 18 +- .../src/windows/mod.rs | 1 + crates/now-package-broker/Cargo.toml | 71 +++++++ .../samples/corporate-allowlist.policy.json | 0 .../samples/deny-risky-options.policy.json | 0 .../winget-unknown-install.request.json | 0 .../winget-vscode-install.request.json | 0 .../winget-vscode-skiphash.request.json | 0 .../samples/scenarios/baseline.scenarios.json | 0 .../now-package-broker/src}/auth.rs | 3 +- .../src}/command_builder/bun.rs | 0 .../src}/command_builder/cargo.rs | 0 .../src}/command_builder/chocolatey.rs | 0 .../src}/command_builder/dotnet.rs | 0 .../src}/command_builder/mod.rs | 0 .../src}/command_builder/npm.rs | 0 .../src}/command_builder/pip.rs | 0 .../src}/command_builder/powershell.rs | 0 .../src}/command_builder/scoop.rs | 0 .../src}/command_builder/vcpkg.rs | 0 .../src}/command_builder/winget.rs | 0 .../src}/evaluator/constraints.rs | 0 .../src}/evaluator/matching.rs | 0 .../now-package-broker/src}/evaluator/mod.rs | 0 .../src}/evaluator/tests.rs | 0 .../src}/evaluator/version.rs | 0 .../src}/evaluator/wildcard.rs | 0 .../now-package-broker/src}/event_channel.rs | 0 .../now-package-broker/src}/executor/mod.rs | 2 +- .../src}/executor/output.rs | 0 .../src}/executor/windows/mod.rs | 8 +- .../src}/executor/windows/privileges.rs | 0 .../src}/executor/windows/process.rs | 10 +- .../src}/executor/windows/token.rs | 0 crates/now-package-broker/src/lib.rs | 34 +++ .../src}/operation_tracker.rs | 2 +- crates/now-package-broker/src/pipe.rs | 176 ++++++++++++++++ .../now-package-broker/src}/policy_loader.rs | 2 +- .../src}/policy_security.rs | 0 .../now-package-broker/src}/policy_watcher.rs | 2 +- .../now-package-broker/src}/scenario_tests.rs | 2 +- .../src}/server/connection.rs | 0 .../src}/server/execution.rs | 6 +- .../now-package-broker/src}/server/mod.rs | 14 +- .../src}/server/responses.rs | 4 +- .../now-package-broker/src}/task.rs | 14 +- devolutions-agent/Cargo.toml | 23 +- devolutions-agent/src/broker/mod.rs | 20 -- devolutions-agent/src/broker/pipe.rs | 197 ------------------ devolutions-agent/src/lib.rs | 4 - devolutions-agent/src/service.rs | 8 +- devolutions-agent/src/updater/package.rs | 4 +- 55 files changed, 379 insertions(+), 308 deletions(-) rename {devolutions-agent/src => crates/devolutions-agent-shared/src/windows}/code_signing.rs (79%) create mode 100644 crates/now-package-broker/Cargo.toml rename {devolutions-agent/src/broker => crates/now-package-broker/src}/assets/samples/corporate-allowlist.policy.json (100%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/assets/samples/deny-risky-options.policy.json (100%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/assets/samples/requests/winget-unknown-install.request.json (100%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/assets/samples/requests/winget-vscode-install.request.json (100%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/assets/samples/requests/winget-vscode-skiphash.request.json (100%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/assets/samples/scenarios/baseline.scenarios.json (100%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/auth.rs (99%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/command_builder/bun.rs (100%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/command_builder/cargo.rs (100%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/command_builder/chocolatey.rs (100%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/command_builder/dotnet.rs (100%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/command_builder/mod.rs (100%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/command_builder/npm.rs (100%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/command_builder/pip.rs (100%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/command_builder/powershell.rs (100%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/command_builder/scoop.rs (100%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/command_builder/vcpkg.rs (100%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/command_builder/winget.rs (100%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/evaluator/constraints.rs (100%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/evaluator/matching.rs (100%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/evaluator/mod.rs (100%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/evaluator/tests.rs (100%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/evaluator/version.rs (100%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/evaluator/wildcard.rs (100%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/event_channel.rs (100%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/executor/mod.rs (99%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/executor/output.rs (100%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/executor/windows/mod.rs (99%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/executor/windows/privileges.rs (100%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/executor/windows/process.rs (98%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/executor/windows/token.rs (100%) create mode 100644 crates/now-package-broker/src/lib.rs rename {devolutions-agent/src/broker => crates/now-package-broker/src}/operation_tracker.rs (99%) create mode 100644 crates/now-package-broker/src/pipe.rs rename {devolutions-agent/src/broker => crates/now-package-broker/src}/policy_loader.rs (99%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/policy_security.rs (100%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/policy_watcher.rs (99%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/scenario_tests.rs (98%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/server/connection.rs (100%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/server/execution.rs (90%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/server/mod.rs (98%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/server/responses.rs (98%) rename {devolutions-agent/src/broker => crates/now-package-broker/src}/task.rs (93%) delete mode 100644 devolutions-agent/src/broker/mod.rs delete mode 100644 devolutions-agent/src/broker/pipe.rs diff --git a/.gitattributes b/.gitattributes index ea0a53f72..7c78e6099 100644 --- a/.gitattributes +++ b/.gitattributes @@ -6,7 +6,7 @@ *.tsx text eol=lf *.json text eol=lf *.ps1 text eol=lf -*.sh text eol=lf +*.sh text eol=lf *.sln text eol=crlf *.md text eol=lf *.mustache text eol=lf @@ -25,4 +25,4 @@ devolutions-gateway/openapi/ts-angular-client/api/** linguist-generated merge=bi devolutions-gateway/openapi/ts-angular-client/model/** linguist-generated merge=binary # Sample assets produce huge LoC counts; exclude them from language statistics. -devolutions-agent/src/broker/assets/** linguist-generated +crates/now-package-broker/src/assets/** linguist-generated diff --git a/Cargo.lock b/Cargo.lock index 7adf897e5..874292004 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1688,13 +1688,11 @@ dependencies = [ "anyhow", "async-trait", "aws-lc-rs", - "axum 0.8.9", "backoff", "base64 0.23.0", "bytes 1.12.1", "camino", "ceviche", - "chrono", "ctrlc", "devolutions-agent-shared", "devolutions-gateway-task", @@ -1706,15 +1704,10 @@ dependencies = [ "hex", "hostname 0.4.2", "http-client-proxy", - "hyper 1.10.1", - "hyper-util", "ipnetwork", "ironrdp", - "notify 7.0.0", "notify-debouncer-mini", - "now-policy", - "now-policy-api", - "now-policy-server-template", + "now-package-broker", "parking_lot", "prost 0.13.5", "prost-types", @@ -1722,15 +1715,12 @@ dependencies = [ "quinn", "rand 0.8.7", "rcgen", - "regex", "reqwest", "rustls 0.23.42", "rustls-pemfile 2.2.0", "rustls-pki-types", - "semver", "serde", "serde_json", - "serde_yaml", "sha2 0.10.9", "tap", "tempfile", @@ -1739,14 +1729,11 @@ dependencies = [ "tokio 1.52.3", "tokio-rustls", "tokio-stream", - "tokio-util", "tonic 0.12.3", "tonic-build", - "tower-service", "tracing", "url", "uuid", - "widestring 1.2.1", "win-api-wrappers", "windows 0.61.3", "x509-parser", @@ -1756,14 +1743,18 @@ dependencies = [ name = "devolutions-agent-shared" version = "0.0.0" dependencies = [ + "anyhow", "camino", "cfg-if", + "hex", "serde", "serde_json", "tempfile", "thiserror 2.0.18", "tracing", "uuid", + "win-api-wrappers", + "windows 0.61.3", "windows-registry 0.5.3", "windows-result 0.3.4", ] @@ -4752,6 +4743,41 @@ dependencies = [ "bitflags 2.13.0", ] +[[package]] +name = "now-package-broker" +version = "0.0.0" +dependencies = [ + "anyhow", + "async-trait", + "axum 0.8.9", + "chrono", + "devolutions-agent-shared", + "devolutions-gateway-task", + "hex", + "hyper 1.10.1", + "hyper-util", + "notify 7.0.0", + "now-policy", + "now-policy-api", + "now-policy-server-template", + "parking_lot", + "regex", + "semver", + "serde", + "serde_json", + "serde_yaml", + "sha2 0.10.9", + "tempfile", + "tokio 1.52.3", + "tokio-util", + "tower-service", + "tracing", + "uuid", + "widestring 1.2.1", + "win-api-wrappers", + "windows 0.61.3", +] + [[package]] name = "now-policy" version = "0.2.0" diff --git a/crates/devolutions-agent-shared/Cargo.toml b/crates/devolutions-agent-shared/Cargo.toml index 2b2714b81..cb482c9a9 100644 --- a/crates/devolutions-agent-shared/Cargo.toml +++ b/crates/devolutions-agent-shared/Cargo.toml @@ -19,6 +19,10 @@ thiserror = "2" tracing = "0.1" [target.'cfg(windows)'.dependencies] +anyhow = "1" +hex = "0.4" +win-api-wrappers = { path = "../win-api-wrappers" } +windows = { version = "0.61", features = ["Win32_Foundation", "Win32_Security_Cryptography"] } windows-registry = "0.5" # Required for `windows-registry` error type. (`Error` is not reexported from `windows-registry`). windows-result = "0.3" diff --git a/devolutions-agent/src/code_signing.rs b/crates/devolutions-agent-shared/src/windows/code_signing.rs similarity index 79% rename from devolutions-agent/src/code_signing.rs rename to crates/devolutions-agent-shared/src/windows/code_signing.rs index 0edd2badb..4d58e5356 100644 --- a/devolutions-agent/src/code_signing.rs +++ b/crates/devolutions-agent-shared/src/windows/code_signing.rs @@ -6,13 +6,13 @@ use anyhow::{Context as _, bail}; use win_api_wrappers::security::crypt::{AuthenticodeSignatureStatus, authenticode_status}; /// List of allowed thumbprints for Devolutions code signing certificates. -pub(crate) const DEVOLUTIONS_CERT_THUMBPRINTS: &[&str] = &[ +pub const DEVOLUTIONS_CERT_THUMBPRINTS: &[&str] = &[ "3f5202a9432d54293bdfe6f7e46adb0a6f8b3ba6", "8db5a43bb8afe4d2ffb92da9007d8997a4cc4e13", "50f753333811ff11f1920274afde3ffd4468b210", ]; -pub(crate) fn certificate_sha1_thumbprint(cert_der: &[u8]) -> anyhow::Result<[u8; 20]> { +pub fn certificate_sha1_thumbprint(cert_der: &[u8]) -> anyhow::Result<[u8; 20]> { use windows::Win32::Security::Cryptography::{CALG_SHA1, CryptHashCertificate}; let mut thumbprint = [0u8; 20]; @@ -39,7 +39,7 @@ pub(crate) fn certificate_sha1_thumbprint(cert_der: &[u8]) -> anyhow::Result<[u8 Ok(thumbprint) } -pub(crate) fn is_devolutions_certificate_thumbprint(calculated_thumbprint: &[u8; 20]) -> bool { +pub fn is_devolutions_certificate_thumbprint(calculated_thumbprint: &[u8; 20]) -> bool { DEVOLUTIONS_CERT_THUMBPRINTS.iter().any(|thumbprint| { let mut thumbprint_bytes = [0u8; 20]; hex::decode_to_slice(thumbprint, &mut thumbprint_bytes) @@ -49,32 +49,32 @@ pub(crate) fn is_devolutions_certificate_thumbprint(calculated_thumbprint: &[u8; }) } -pub(crate) fn validate_devolutions_authenticode_signature(path: &Path) -> anyhow::Result { +pub fn validate_devolutions_authenticode_signature(path: &Path) -> anyhow::Result { let wintrust_result = authenticode_status(path).with_context(|| { format!( - "failed to read authenticode signature for client executable '{}'", + "failed to read authenticode signature for executable '{}'", path.display() ) })?; if !matches!(wintrust_result.status, AuthenticodeSignatureStatus::Valid) { - bail!("client executable signature is not valid: {:?}", wintrust_result.status); + bail!("executable signature is not valid: {:?}", wintrust_result.status); } let signer = wintrust_result .provider .as_ref() .and_then(|provider| provider.signers.first()) - .context("client executable signature has no signer")?; + .context("executable signature has no signer")?; let signing_cert = signer .cert_chain .first() - .context("client executable signature has no signing certificate")?; + .context("executable signature has no signing certificate")?; let thumbprint = certificate_sha1_thumbprint(&signing_cert.cert.encoded)?; if !is_devolutions_certificate_thumbprint(&thumbprint) { bail!( - "client executable is signed with an unexpected certificate thumbprint: {}", + "executable is signed with an unexpected certificate thumbprint: {}", hex::encode(thumbprint) ); } diff --git a/crates/devolutions-agent-shared/src/windows/mod.rs b/crates/devolutions-agent-shared/src/windows/mod.rs index 576d7bfed..436a8d657 100644 --- a/crates/devolutions-agent-shared/src/windows/mod.rs +++ b/crates/devolutions-agent-shared/src/windows/mod.rs @@ -1,5 +1,6 @@ mod reversed_hex_uuid; +pub mod code_signing; pub mod registry; use uuid::{Uuid, uuid}; diff --git a/crates/now-package-broker/Cargo.toml b/crates/now-package-broker/Cargo.toml new file mode 100644 index 000000000..379c5024a --- /dev/null +++ b/crates/now-package-broker/Cargo.toml @@ -0,0 +1,71 @@ +[package] +name = "now-package-broker" +version = "0.0.0" +edition = "2024" +license = "MIT/Apache-2.0" +authors = ["Devolutions Inc. "] +description = "Package broker for the Devolutions Agent" +publish = false + +[features] +default = [] +# Development-only feature allowing broker client signature validation to be skipped. +# Must never be enabled for shipped builds: without it, broker client signature validation is +# unconditionally enforced regardless of the configuration file contents. +dev-skip-broker-signature = [] + +[lints] +workspace = true + +# The broker is only functional on Windows; all dependencies are Windows-only +# so the crate compiles to an empty library on other platforms. +[target.'cfg(windows)'.dependencies] +anyhow = "1" +async-trait = "0.1" +axum = { version = "0.8", default-features = false, features = ["http1", "json", "tokio", "original-uri", "matched-path"] } +chrono = { version = "0.4", features = ["serde"] } +devolutions-agent-shared = { path = "../devolutions-agent-shared" } +devolutions-gateway-task = { path = "../devolutions-gateway-task" } +hex = "0.4" +hyper = { version = "1", features = ["http1", "server"] } +hyper-util = { version = "0.1", features = ["tokio", "server", "server-auto", "service"] } +notify = { version = "7", default-features = false } +now-policy = "0.2" +now-policy-api = { version = "0.3", features = ["policy-compat"] } +now-policy-server-template = { version = "0.3", features = ["policy-compat"] } +parking_lot = "0.12" +regex = "1" +semver = "1" +serde_json = "1" +sha2 = "0.10" +tokio = { version = "1.52", features = ["net", "io-util", "rt", "macros", "parking_lot", "fs", "sync", "time"] } +tokio-util = "0.7" +tower-service = "0.3" +tracing = "0.1" +uuid = { version = "1.23", features = ["v4"] } +widestring = "1.2" +win-api-wrappers = { path = "../win-api-wrappers" } + +[target.'cfg(windows)'.dependencies.windows] +version = "0.61" +features = [ + "Win32_Foundation", + "Win32_Security", + "Win32_Security_Authorization", + "Win32_Storage_FileSystem", + "Win32_System_ApplicationInstallationAndServicing", + "Win32_System_Com", + "Win32_System_Console", + "Win32_System_IO", + "Win32_System_Ioctl", + "Win32_System_Pipes", + "Win32_System_Threading", + "Win32_UI_Shell", + "Win32_UI_WindowsAndMessaging", +] + +[target.'cfg(windows)'.dev-dependencies] +serde = { version = "1", features = ["derive"] } +serde_yaml = "0.9" +tempfile = "3" +tokio = { version = "1.52", features = ["rt-multi-thread"] } diff --git a/devolutions-agent/src/broker/assets/samples/corporate-allowlist.policy.json b/crates/now-package-broker/src/assets/samples/corporate-allowlist.policy.json similarity index 100% rename from devolutions-agent/src/broker/assets/samples/corporate-allowlist.policy.json rename to crates/now-package-broker/src/assets/samples/corporate-allowlist.policy.json diff --git a/devolutions-agent/src/broker/assets/samples/deny-risky-options.policy.json b/crates/now-package-broker/src/assets/samples/deny-risky-options.policy.json similarity index 100% rename from devolutions-agent/src/broker/assets/samples/deny-risky-options.policy.json rename to crates/now-package-broker/src/assets/samples/deny-risky-options.policy.json diff --git a/devolutions-agent/src/broker/assets/samples/requests/winget-unknown-install.request.json b/crates/now-package-broker/src/assets/samples/requests/winget-unknown-install.request.json similarity index 100% rename from devolutions-agent/src/broker/assets/samples/requests/winget-unknown-install.request.json rename to crates/now-package-broker/src/assets/samples/requests/winget-unknown-install.request.json diff --git a/devolutions-agent/src/broker/assets/samples/requests/winget-vscode-install.request.json b/crates/now-package-broker/src/assets/samples/requests/winget-vscode-install.request.json similarity index 100% rename from devolutions-agent/src/broker/assets/samples/requests/winget-vscode-install.request.json rename to crates/now-package-broker/src/assets/samples/requests/winget-vscode-install.request.json diff --git a/devolutions-agent/src/broker/assets/samples/requests/winget-vscode-skiphash.request.json b/crates/now-package-broker/src/assets/samples/requests/winget-vscode-skiphash.request.json similarity index 100% rename from devolutions-agent/src/broker/assets/samples/requests/winget-vscode-skiphash.request.json rename to crates/now-package-broker/src/assets/samples/requests/winget-vscode-skiphash.request.json diff --git a/devolutions-agent/src/broker/assets/samples/scenarios/baseline.scenarios.json b/crates/now-package-broker/src/assets/samples/scenarios/baseline.scenarios.json similarity index 100% rename from devolutions-agent/src/broker/assets/samples/scenarios/baseline.scenarios.json rename to crates/now-package-broker/src/assets/samples/scenarios/baseline.scenarios.json diff --git a/devolutions-agent/src/broker/auth.rs b/crates/now-package-broker/src/auth.rs similarity index 99% rename from devolutions-agent/src/broker/auth.rs rename to crates/now-package-broker/src/auth.rs index 009c90de3..bc770d149 100644 --- a/devolutions-agent/src/broker/auth.rs +++ b/crates/now-package-broker/src/auth.rs @@ -3,6 +3,7 @@ use std::path::{Path, PathBuf}; use anyhow::{Context as _, bail}; +use devolutions_agent_shared::windows::code_signing::validate_devolutions_authenticode_signature; use now_policy_api::{CancelRequest, ClientContext, PackageRequest, StatusRequest}; use tokio::net::windows::named_pipe::NamedPipeServer; use tracing::{debug, warn}; @@ -14,8 +15,6 @@ use windows::Win32::Security::TOKEN_QUERY; use windows::Win32::Storage::FileSystem::FILE_ID_INFO; use windows::Win32::System::Threading::PROCESS_QUERY_LIMITED_INFORMATION; -use crate::code_signing::validate_devolutions_authenticode_signature; - #[derive(Clone, Debug)] pub(crate) struct PipeClient { process_id: u32, diff --git a/devolutions-agent/src/broker/command_builder/bun.rs b/crates/now-package-broker/src/command_builder/bun.rs similarity index 100% rename from devolutions-agent/src/broker/command_builder/bun.rs rename to crates/now-package-broker/src/command_builder/bun.rs diff --git a/devolutions-agent/src/broker/command_builder/cargo.rs b/crates/now-package-broker/src/command_builder/cargo.rs similarity index 100% rename from devolutions-agent/src/broker/command_builder/cargo.rs rename to crates/now-package-broker/src/command_builder/cargo.rs diff --git a/devolutions-agent/src/broker/command_builder/chocolatey.rs b/crates/now-package-broker/src/command_builder/chocolatey.rs similarity index 100% rename from devolutions-agent/src/broker/command_builder/chocolatey.rs rename to crates/now-package-broker/src/command_builder/chocolatey.rs diff --git a/devolutions-agent/src/broker/command_builder/dotnet.rs b/crates/now-package-broker/src/command_builder/dotnet.rs similarity index 100% rename from devolutions-agent/src/broker/command_builder/dotnet.rs rename to crates/now-package-broker/src/command_builder/dotnet.rs diff --git a/devolutions-agent/src/broker/command_builder/mod.rs b/crates/now-package-broker/src/command_builder/mod.rs similarity index 100% rename from devolutions-agent/src/broker/command_builder/mod.rs rename to crates/now-package-broker/src/command_builder/mod.rs diff --git a/devolutions-agent/src/broker/command_builder/npm.rs b/crates/now-package-broker/src/command_builder/npm.rs similarity index 100% rename from devolutions-agent/src/broker/command_builder/npm.rs rename to crates/now-package-broker/src/command_builder/npm.rs diff --git a/devolutions-agent/src/broker/command_builder/pip.rs b/crates/now-package-broker/src/command_builder/pip.rs similarity index 100% rename from devolutions-agent/src/broker/command_builder/pip.rs rename to crates/now-package-broker/src/command_builder/pip.rs diff --git a/devolutions-agent/src/broker/command_builder/powershell.rs b/crates/now-package-broker/src/command_builder/powershell.rs similarity index 100% rename from devolutions-agent/src/broker/command_builder/powershell.rs rename to crates/now-package-broker/src/command_builder/powershell.rs diff --git a/devolutions-agent/src/broker/command_builder/scoop.rs b/crates/now-package-broker/src/command_builder/scoop.rs similarity index 100% rename from devolutions-agent/src/broker/command_builder/scoop.rs rename to crates/now-package-broker/src/command_builder/scoop.rs diff --git a/devolutions-agent/src/broker/command_builder/vcpkg.rs b/crates/now-package-broker/src/command_builder/vcpkg.rs similarity index 100% rename from devolutions-agent/src/broker/command_builder/vcpkg.rs rename to crates/now-package-broker/src/command_builder/vcpkg.rs diff --git a/devolutions-agent/src/broker/command_builder/winget.rs b/crates/now-package-broker/src/command_builder/winget.rs similarity index 100% rename from devolutions-agent/src/broker/command_builder/winget.rs rename to crates/now-package-broker/src/command_builder/winget.rs diff --git a/devolutions-agent/src/broker/evaluator/constraints.rs b/crates/now-package-broker/src/evaluator/constraints.rs similarity index 100% rename from devolutions-agent/src/broker/evaluator/constraints.rs rename to crates/now-package-broker/src/evaluator/constraints.rs diff --git a/devolutions-agent/src/broker/evaluator/matching.rs b/crates/now-package-broker/src/evaluator/matching.rs similarity index 100% rename from devolutions-agent/src/broker/evaluator/matching.rs rename to crates/now-package-broker/src/evaluator/matching.rs diff --git a/devolutions-agent/src/broker/evaluator/mod.rs b/crates/now-package-broker/src/evaluator/mod.rs similarity index 100% rename from devolutions-agent/src/broker/evaluator/mod.rs rename to crates/now-package-broker/src/evaluator/mod.rs diff --git a/devolutions-agent/src/broker/evaluator/tests.rs b/crates/now-package-broker/src/evaluator/tests.rs similarity index 100% rename from devolutions-agent/src/broker/evaluator/tests.rs rename to crates/now-package-broker/src/evaluator/tests.rs diff --git a/devolutions-agent/src/broker/evaluator/version.rs b/crates/now-package-broker/src/evaluator/version.rs similarity index 100% rename from devolutions-agent/src/broker/evaluator/version.rs rename to crates/now-package-broker/src/evaluator/version.rs diff --git a/devolutions-agent/src/broker/evaluator/wildcard.rs b/crates/now-package-broker/src/evaluator/wildcard.rs similarity index 100% rename from devolutions-agent/src/broker/evaluator/wildcard.rs rename to crates/now-package-broker/src/evaluator/wildcard.rs diff --git a/devolutions-agent/src/broker/event_channel.rs b/crates/now-package-broker/src/event_channel.rs similarity index 100% rename from devolutions-agent/src/broker/event_channel.rs rename to crates/now-package-broker/src/event_channel.rs diff --git a/devolutions-agent/src/broker/executor/mod.rs b/crates/now-package-broker/src/executor/mod.rs similarity index 99% rename from devolutions-agent/src/broker/executor/mod.rs rename to crates/now-package-broker/src/executor/mod.rs index 61f0287b6..88519475b 100644 --- a/devolutions-agent/src/broker/executor/mod.rs +++ b/crates/now-package-broker/src/executor/mod.rs @@ -9,7 +9,7 @@ use tokio_util::sync::CancellationToken; use tracing::info; use win_api_wrappers::identity::sid::Sid; -use crate::broker::event_channel::OperationEventSink; +use crate::event_channel::OperationEventSink; mod output; diff --git a/devolutions-agent/src/broker/executor/output.rs b/crates/now-package-broker/src/executor/output.rs similarity index 100% rename from devolutions-agent/src/broker/executor/output.rs rename to crates/now-package-broker/src/executor/output.rs diff --git a/devolutions-agent/src/broker/executor/windows/mod.rs b/crates/now-package-broker/src/executor/windows/mod.rs similarity index 99% rename from devolutions-agent/src/broker/executor/windows/mod.rs rename to crates/now-package-broker/src/executor/windows/mod.rs index 1f77ee71a..a1e9e48a8 100644 --- a/devolutions-agent/src/broker/executor/windows/mod.rs +++ b/crates/now-package-broker/src/executor/windows/mod.rs @@ -22,7 +22,7 @@ use super::{ BROKER_SUPPORTED_MANAGERS, CommandExecutor, ExecutionContext, ExecutionOutput, OperationCanceled, ProcessStartedCallback, is_canceled_error, }; -use crate::broker::policy_security; +use crate::policy_security; mod privileges; mod process; @@ -161,9 +161,7 @@ fn manager_is_available(manager: ManagerName, user_env: &HashMap .is_ok(), ManagerName::Bun => resolve_bun_executable("bun", user_env).is_ok(), ManagerName::Cargo => resolve_cargo_executable(user_env).is_ok(), - ManagerName::Dotnet => { - Path::new(&crate::broker::command_builder::dotnet::trusted_dotnet_executable()).is_file() - } + ManagerName::Dotnet => Path::new(&crate::command_builder::dotnet::trusted_dotnet_executable()).is_file(), ManagerName::Pip => resolve_python_executable("python.exe", user_env).is_ok(), // npm runs through the user PATH `npm` shim inside the trusted Windows PowerShell wrapper, // so both the shim and the host must exist. @@ -1355,7 +1353,7 @@ mod tests { prepare_chocolatey_script_in_with_default_install_root, prepare_main_command_in, prepare_shell_command_in, reject_unsupported_vcpkg_elevation, resolve_trusted_chocolatey_executable, resolve_winget_executable, }; - use crate::broker::executor::{CommandExecutor as _, ExecutionContext}; + use crate::executor::{CommandExecutor as _, ExecutionContext}; fn grant(permissions: u32, sid: Sid) -> ExplicitAccess { ExplicitAccess { diff --git a/devolutions-agent/src/broker/executor/windows/privileges.rs b/crates/now-package-broker/src/executor/windows/privileges.rs similarity index 100% rename from devolutions-agent/src/broker/executor/windows/privileges.rs rename to crates/now-package-broker/src/executor/windows/privileges.rs diff --git a/devolutions-agent/src/broker/executor/windows/process.rs b/crates/now-package-broker/src/executor/windows/process.rs similarity index 98% rename from devolutions-agent/src/broker/executor/windows/process.rs rename to crates/now-package-broker/src/executor/windows/process.rs index c73ac1975..dc6276e19 100644 --- a/devolutions-agent/src/broker/executor/windows/process.rs +++ b/crates/now-package-broker/src/executor/windows/process.rs @@ -19,12 +19,12 @@ use windows::Win32::System::Threading::{ }; use windows::Win32::UI::WindowsAndMessaging::SW_HIDE; -use crate::broker::event_channel::{OperationEventSink, OutputStream}; -use crate::broker::executor::{ +use crate::event_channel::{OperationEventSink, OutputStream}; +use crate::executor::{ ExecutionOutput, MAX_CAPTURED_OUTPUT_BYTES, OperationCanceled, ProcessStartedCallback, tail_utf8, }; -use crate::broker::operation_tracker::OperationTracker; -use crate::broker::policy_security; +use crate::operation_tracker::OperationTracker; +use crate::policy_security; /// How long a canceled process is given to exit after the graceful console /// ctrl event before it is forcefully terminated. @@ -66,7 +66,7 @@ impl<'a> OutputCapture<'a> { /// stdout and stderr are redirected into two separate pipes; raw chunks are forwarded to /// `event_sink` (when present) as they arrive, and a tail-truncated combined copy is /// returned in [`ExecutionOutput`] (limited to -/// [`crate::broker::executor::MAX_CAPTURED_OUTPUT_BYTES`]); otherwise no output is captured. +/// [`crate::executor::MAX_CAPTURED_OUTPUT_BYTES`]); otherwise no output is captured. /// /// Returns the process exit code and (when captured) its output. /// diff --git a/devolutions-agent/src/broker/executor/windows/token.rs b/crates/now-package-broker/src/executor/windows/token.rs similarity index 100% rename from devolutions-agent/src/broker/executor/windows/token.rs rename to crates/now-package-broker/src/executor/windows/token.rs diff --git a/crates/now-package-broker/src/lib.rs b/crates/now-package-broker/src/lib.rs new file mode 100644 index 000000000..b55c994db --- /dev/null +++ b/crates/now-package-broker/src/lib.rs @@ -0,0 +1,34 @@ +//! Package broker for the Devolutions Agent. +//! +//! Provides policy evaluation and command execution for package operations, +//! communicating over a Windows named pipe using HTTP/1.1. +//! +//! The broker is only functional on Windows; on other platforms this crate is empty. + +#[cfg(windows)] +mod auth; +#[cfg(windows)] +pub mod command_builder; +#[cfg(windows)] +pub mod evaluator; +#[cfg(windows)] +pub mod event_channel; +#[cfg(windows)] +pub mod executor; +#[cfg(windows)] +pub mod operation_tracker; +#[cfg(windows)] +pub mod pipe; +#[cfg(windows)] +pub mod policy_loader; +#[cfg(windows)] +mod policy_security; +#[cfg(windows)] +pub mod policy_watcher; +#[cfg(windows)] +pub mod server; +#[cfg(windows)] +pub mod task; + +#[cfg(all(test, windows))] +mod scenario_tests; diff --git a/devolutions-agent/src/broker/operation_tracker.rs b/crates/now-package-broker/src/operation_tracker.rs similarity index 99% rename from devolutions-agent/src/broker/operation_tracker.rs rename to crates/now-package-broker/src/operation_tracker.rs index 10d62ad78..3850e0d4f 100644 --- a/devolutions-agent/src/broker/operation_tracker.rs +++ b/crates/now-package-broker/src/operation_tracker.rs @@ -14,7 +14,7 @@ use now_policy_api::{EventChannel, OperationStatus, PackageRequest, ResourceId}; use sha2::{Digest as _, Sha256}; use tokio_util::sync::CancellationToken; -use crate::broker::event_channel::OperationEventSink; +use crate::event_channel::OperationEventSink; /// How long completed/failed operation results are retained for status queries. const RESULT_RETENTION: Duration = Duration::from_secs(5 * 60); // 5 minutes. diff --git a/crates/now-package-broker/src/pipe.rs b/crates/now-package-broker/src/pipe.rs new file mode 100644 index 000000000..1cb6d6ca7 --- /dev/null +++ b/crates/now-package-broker/src/pipe.rs @@ -0,0 +1,176 @@ +//! Named pipe transport for Windows. +//! +//! Creates a named pipe server with appropriate ACLs and accepts connections, +//! forwarding them to the HTTP server. + +use std::sync::Arc; + +use anyhow::Context as _; +use tokio::net::windows::named_pipe::{NamedPipeServer, ServerOptions}; +use tokio::sync::Semaphore; +use tokio_util::sync::CancellationToken; +use tracing::{error, info, warn}; +use win_api_wrappers::identity::sid::Sid; +use win_api_wrappers::security::acl::{Acl, ExplicitAccess, InheritableAcl, InheritableAclKind, Trustee}; +use win_api_wrappers::security::attributes::SecurityAttributesInit; +use windows::Win32::Foundation::GENERIC_ALL; +use windows::Win32::Security; +use windows::Win32::Security::Authorization::SET_ACCESS; +use windows::Win32::Storage::FileSystem::{FILE_GENERIC_READ, FILE_GENERIC_WRITE}; + +use crate::auth::PipeClient; +use crate::server::{BrokerState, build_router_for_client, serve_connection}; + +/// Default pipe name for the package broker. +pub const DEFAULT_PIPE_NAME: &str = r"\\.\pipe\Devolutions.Now.PackageBroker.v1"; + +/// Maximum number of concurrently served pipe connections. +/// +/// Connection setup performs unauthenticated work (client process identity lookups) +/// before any signature gate, so a connection flood could otherwise trigger unbounded +/// work and task spawning. While all slots are taken, no pipe instance is listening and +/// further clients fail to connect until a slot frees up. +const MAX_CONCURRENT_CONNECTIONS: usize = 16; + +/// Deadline for serving a single pipe connection, from accept to response completion. +/// +/// Each connection serves exactly one HTTP request (`keep_alive` is disabled) and all +/// endpoints respond without blocking on package operations (execution is asynchronous, +/// tracked via the operation tracker), so a healthy exchange completes well within this +/// deadline. Without it, idle clients holding their connection open without sending a +/// request would each pin a connection slot indefinitely and could exhaust the pool. +const CONNECTION_DEADLINE: std::time::Duration = std::time::Duration::from_secs(30); + +/// Start the named pipe server and accept connections until shutdown. +pub async fn run_pipe_server(state: Arc, shutdown: CancellationToken) -> anyhow::Result<()> { + let pipe_name = state.pipe_name.clone(); + info!(%pipe_name, "Starting named pipe server"); + + let connection_permits = Arc::new(Semaphore::new(MAX_CONCURRENT_CONNECTIONS)); + + let mut first_instance = true; + loop { + // Wait for a free connection slot before exposing a new pipe instance, + // bounding the number of concurrently served connections. + let permit = tokio::select! { + permit = Arc::clone(&connection_permits).acquire_owned() => { + permit.expect("the semaphore is never closed") + } + _ = shutdown.cancelled() => { + info!("Pipe server shutting down"); + return Ok(()); + } + }; + + // Create a new pipe instance for each connection. + let server = create_pipe_instance(&pipe_name, first_instance)?; + first_instance = false; + + tokio::select! { + result = server.connect() => { + match result { + Ok(()) => { + let state = Arc::clone(&state); + tokio::spawn(async move { + // The permit is held for the lifetime of the connection task. + let _permit = permit; + + let serve = async move { + // Capture the client identity off the accept loop so a slow + // lookup cannot stall accepting other connections. + let client = match PipeClient::from_connected_pipe(&server) { + Ok(client) => client, + Err(error) => { + warn!(%error, "Rejected named pipe client"); + return; + } + }; + info!("Client connected to named pipe"); + let router = build_router_for_client(state, client); + serve_connection(server, router).await; + info!("Client disconnected from named pipe"); + }; + + // Enforce a deadline so idle or slow clients cannot pin + // a connection slot indefinitely. + if tokio::time::timeout(CONNECTION_DEADLINE, serve).await.is_err() { + warn!("Closed named pipe connection: deadline exceeded"); + } + }); + } + Err(error) => { + error!(%error, "Failed to accept pipe connection"); + } + } + } + _ = shutdown.cancelled() => { + info!("Pipe server shutting down"); + return Ok(()); + } + } + } +} + +fn create_pipe_instance(pipe_name: &str, first_instance: bool) -> anyhow::Result { + let security_attributes = build_pipe_security_attributes().context("failed to build pipe security attributes")?; + + // SAFETY: `create_with_security_attributes_raw` requires a pointer to a valid + // `SECURITY_ATTRIBUTES` that stays alive for the duration of the call. The pointer + // comes from `security_attributes` (a `win_api_wrappers::security::SecurityAttributes`), + // a local binding that owns the structure and its security descriptor and is dropped + // only at the end of this function, well after the call returns. `CreateNamedPipeW` + // copies the descriptor at creation, so the pointer is not retained afterwards. + let server = unsafe { + ServerOptions::new() + .first_pipe_instance(first_instance) + .create_with_security_attributes_raw(pipe_name, security_attributes.as_mut_ptr().cast()) + }?; + + Ok(server) +} + +/// Build a security descriptor that grants: +/// - SYSTEM: full control +/// - Administrators: full control +/// - BUILTIN\Users: read + write (allows interactive users to connect) +fn build_pipe_security_attributes() -> anyhow::Result { + let system_sid = Sid::from_well_known(Security::WinLocalSystemSid, None).context("failed to create SYSTEM SID")?; + let admins_sid = Sid::from_well_known(Security::WinBuiltinAdministratorsSid, None) + .context("failed to create Administrators SID")?; + let users_sid = Sid::from_well_known(Security::WinBuiltinUsersSid, None).context("failed to create Users SID")?; + + let entries = [ + ExplicitAccess { + access_permissions: GENERIC_ALL.0, + access_mode: SET_ACCESS, + inheritance: Security::ACE_FLAGS(0), + trustee: Trustee::Sid(system_sid), + }, + ExplicitAccess { + access_permissions: GENERIC_ALL.0, + access_mode: SET_ACCESS, + inheritance: Security::ACE_FLAGS(0), + trustee: Trustee::Sid(admins_sid), + }, + ExplicitAccess { + access_permissions: FILE_GENERIC_READ.0 | FILE_GENERIC_WRITE.0, + access_mode: SET_ACCESS, + inheritance: Security::ACE_FLAGS(0), + trustee: Trustee::Sid(users_sid), + }, + ]; + + let empty_acl = Acl::new().context("failed to create empty ACL")?; + let dacl = empty_acl.set_entries(&entries).context("failed to set ACL entries")?; + + let attrs = SecurityAttributesInit { + dacl: Some(InheritableAcl { + kind: InheritableAclKind::Protected, + acl: dacl, + }), + ..Default::default() + } + .init(); + + Ok(attrs) +} diff --git a/devolutions-agent/src/broker/policy_loader.rs b/crates/now-package-broker/src/policy_loader.rs similarity index 99% rename from devolutions-agent/src/broker/policy_loader.rs rename to crates/now-package-broker/src/policy_loader.rs index 60ec86a4b..3d44cb9dd 100644 --- a/devolutions-agent/src/broker/policy_loader.rs +++ b/crates/now-package-broker/src/policy_loader.rs @@ -11,7 +11,7 @@ use now_policy::PolicyDocument; use now_policy::schema::{parse_policy_json, parse_policy_yaml}; use tracing::info; -use crate::broker::policy_security; +use crate::policy_security; /// Default policy directory. pub fn default_policy_dir() -> PathBuf { diff --git a/devolutions-agent/src/broker/policy_security.rs b/crates/now-package-broker/src/policy_security.rs similarity index 100% rename from devolutions-agent/src/broker/policy_security.rs rename to crates/now-package-broker/src/policy_security.rs diff --git a/devolutions-agent/src/broker/policy_watcher.rs b/crates/now-package-broker/src/policy_watcher.rs similarity index 99% rename from devolutions-agent/src/broker/policy_watcher.rs rename to crates/now-package-broker/src/policy_watcher.rs index fe0d0d676..a30bda5e3 100644 --- a/devolutions-agent/src/broker/policy_watcher.rs +++ b/crates/now-package-broker/src/policy_watcher.rs @@ -14,7 +14,7 @@ use tokio::sync::watch; use tokio_util::sync::CancellationToken; use tracing::{error, info, warn}; -use crate::broker::policy_loader; +use crate::policy_loader; /// State of the policy: either loaded and active, or unavailable. #[derive(Debug, Clone)] diff --git a/devolutions-agent/src/broker/scenario_tests.rs b/crates/now-package-broker/src/scenario_tests.rs similarity index 98% rename from devolutions-agent/src/broker/scenario_tests.rs rename to crates/now-package-broker/src/scenario_tests.rs index e2ebab6a0..c0c16e4b8 100644 --- a/devolutions-agent/src/broker/scenario_tests.rs +++ b/crates/now-package-broker/src/scenario_tests.rs @@ -15,7 +15,7 @@ use super::evaluator; /// Local samples directory bundled inside the crate. fn samples_dir() -> PathBuf { - PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("src/broker/assets/samples") + PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("src/assets/samples") } // ─── Scenario file structures ──────────────────────────────────────────────── diff --git a/devolutions-agent/src/broker/server/connection.rs b/crates/now-package-broker/src/server/connection.rs similarity index 100% rename from devolutions-agent/src/broker/server/connection.rs rename to crates/now-package-broker/src/server/connection.rs diff --git a/devolutions-agent/src/broker/server/execution.rs b/crates/now-package-broker/src/server/execution.rs similarity index 90% rename from devolutions-agent/src/broker/server/execution.rs rename to crates/now-package-broker/src/server/execution.rs index ff141be7b..911e7bc6f 100644 --- a/devolutions-agent/src/broker/server/execution.rs +++ b/crates/now-package-broker/src/server/execution.rs @@ -5,8 +5,8 @@ use std::sync::Arc; use now_policy_api::ResourceId; use tracing::{error, info}; -use crate::broker::executor::{CommandExecutor, ExecutionContext, ProcessStartedCallback, is_canceled_error}; -use crate::broker::operation_tracker::OperationTracker; +use crate::executor::{CommandExecutor, ExecutionContext, ProcessStartedCallback, is_canceled_error}; +use crate::operation_tracker::OperationTracker; pub(super) fn spawn_execution( executor: Arc, @@ -30,7 +30,7 @@ pub(super) fn spawn_execution( } else { #[allow(clippy::cast_sign_loss)] let unsigned = output.exit_code as u32; - match crate::broker::executor::describe_exit_code(output.exit_code) { + match crate::executor::describe_exit_code(output.exit_code) { Some(description) => format!( "{exe_name} exited with code {} (0x{unsigned:08X}): {description}", output.exit_code diff --git a/devolutions-agent/src/broker/server/mod.rs b/crates/now-package-broker/src/server/mod.rs similarity index 98% rename from devolutions-agent/src/broker/server/mod.rs rename to crates/now-package-broker/src/server/mod.rs index 7baf78852..6f1e65b20 100644 --- a/devolutions-agent/src/broker/server/mod.rs +++ b/crates/now-package-broker/src/server/mod.rs @@ -18,11 +18,11 @@ use now_policy_server_template::{MAX_REQUEST_BODY_BYTES, PackageBrokerServer, Sh use tracing::{info, trace, warn}; use win_api_wrappers::identity::sid::Sid; -use crate::broker::auth::PipeClient; -use crate::broker::command_builder::build_command; -use crate::broker::evaluator; -use crate::broker::executor::{CommandExecutor, ExecutionContext}; -use crate::broker::operation_tracker::OperationTracker; +use crate::auth::PipeClient; +use crate::command_builder::build_command; +use crate::evaluator; +use crate::executor::{CommandExecutor, ExecutionContext}; +use crate::operation_tracker::OperationTracker; mod connection; mod execution; @@ -283,7 +283,7 @@ impl BrokerState { #[cfg(windows)] { let operation_key = operation_id.to_string(); - match crate::broker::event_channel::open_operation_channel(&operation_key, user_sid) { + match crate::event_channel::open_operation_channel(&operation_key, user_sid) { Ok((sink, descriptor)) => { self.tracker .set_event_channel(&operation_key, sink.clone(), descriptor.clone()); @@ -529,7 +529,7 @@ mod tests { use now_policy_api as api; use super::*; - use crate::broker::executor::{ExecutionOutput, OperationCanceled, ProcessStartedCallback}; + use crate::executor::{ExecutionOutput, OperationCanceled, ProcessStartedCallback}; struct NoopExecutor; diff --git a/devolutions-agent/src/broker/server/responses.rs b/crates/now-package-broker/src/server/responses.rs similarity index 98% rename from devolutions-agent/src/broker/server/responses.rs rename to crates/now-package-broker/src/server/responses.rs index 74b27a7b1..3899f8759 100644 --- a/devolutions-agent/src/broker/server/responses.rs +++ b/crates/now-package-broker/src/server/responses.rs @@ -8,7 +8,7 @@ use now_policy_api::{ RuleId, Scope, ServerContext, Transport, }; -use crate::broker::operation_tracker::OperationTracker; +use crate::operation_tracker::OperationTracker; pub(super) fn api_version() -> ApiVersion { API_VERSION_STR.into() @@ -24,7 +24,7 @@ pub(super) fn server_context() -> ServerContext { /// Capability descriptions for every manager the broker can drive. /// /// This is the full supported set; the server filters it down to the managers actually -/// available for the requesting user (see [`crate::broker::server::BrokerState`]). +/// available for the requesting user (see [`crate::server::BrokerState`]). pub(super) fn supported_manager_capabilities() -> Vec { vec![ ManagerCapability { diff --git a/devolutions-agent/src/broker/task.rs b/crates/now-package-broker/src/task.rs similarity index 93% rename from devolutions-agent/src/broker/task.rs rename to crates/now-package-broker/src/task.rs index 100009266..490ee5ab5 100644 --- a/devolutions-agent/src/broker/task.rs +++ b/crates/now-package-broker/src/task.rs @@ -8,11 +8,11 @@ use devolutions_gateway_task::{ShutdownSignal, Task}; use tokio_util::sync::CancellationToken; use tracing::{info, warn}; -use crate::broker::executor::{self, CommandExecutor}; -use crate::broker::pipe::DEFAULT_PIPE_NAME; -use crate::broker::policy_loader; -use crate::broker::policy_watcher::{PolicyState, PolicyWatcher}; -use crate::broker::server::BrokerState; +use crate::executor::{self, CommandExecutor}; +use crate::pipe::DEFAULT_PIPE_NAME; +use crate::policy_loader; +use crate::policy_watcher::{PolicyState, PolicyWatcher}; +use crate::server::BrokerState; /// Configuration for the broker task. #[derive(Debug, Clone)] @@ -102,7 +102,7 @@ impl Task for BrokerTask { policy: RwLock::new(initial_policy), executor, pipe_name: self.config.pipe_name.clone(), - tracker: crate::broker::operation_tracker::OperationTracker::new(), + tracker: crate::operation_tracker::OperationTracker::new(), skip_signature_validation: self.config.skip_signature_validation, manager_probe_cache: Default::default(), }); @@ -153,7 +153,7 @@ impl Task for BrokerTask { let server_shutdown = shutdown.clone(); let server_handle = tokio::spawn({ let state = Arc::clone(&state); - async move { crate::broker::pipe::run_pipe_server(state, server_shutdown).await } + async move { crate::pipe::run_pipe_server(state, server_shutdown).await } }); // Wait for agent shutdown signal. diff --git a/devolutions-agent/Cargo.toml b/devolutions-agent/Cargo.toml index ef72213e4..2be961bb6 100644 --- a/devolutions-agent/Cargo.toml +++ b/devolutions-agent/Cargo.toml @@ -13,7 +13,7 @@ default = [] # Development-only feature allowing the SkipBrokerSignatureValidation debug option to take effect. # Must never be enabled for shipped builds: without it, broker client signature validation is # unconditionally enforced regardless of the configuration file contents. -dev-skip-broker-signature = [] +dev-skip-broker-signature = ["now-package-broker/dev-skip-broker-signature"] [lints] workspace = true @@ -23,12 +23,10 @@ agent-tunnel-proto = { path = "../crates/agent-tunnel-proto" } anyhow = "1" backoff = "0.4" async-trait = "0.1" -axum = { version = "0.8", default-features = false, features = ["http1", "json", "tokio", "original-uri", "matched-path"] } base64 = "0.23" bytes = "1" camino = { version = "1.1", features = ["serde1"] } ceviche = "0.7" -chrono = { version = "0.4", features = ["serde"] } ctrlc = "3.5" devolutions-agent-shared = { path = "../crates/devolutions-agent-shared" } devolutions-gateway-task = { path = "../crates/devolutions-gateway-task" } @@ -36,21 +34,14 @@ devolutions-log = { path = "../crates/devolutions-log" } futures = "0.3" hex = "0.4" hostname = "0.4" -hyper = { version = "1", features = ["http1", "server"] } -hyper-util = { version = "0.1", features = ["tokio", "server", "server-auto", "service"] } http-client-proxy = { path = "../crates/http-client-proxy" } ipnetwork = "0.20" -notify = { version = "7", default-features = false, features = ["macos_kqueue"] } -now-policy = "0.2" -now-policy-api = { version = "0.3", features = ["policy-compat"] } -now-policy-server-template = { version = "0.3", features = ["policy-compat"] } parking_lot = "0.12" prost = "0.13" prost-types = "0.13" quinn = "0.11" rand = "0.8" # FIXME(@CBenoit): maybe we don't need this crate rcgen = { version = "0.13", features = ["pem"] } -regex = "1" reqwest = { version = "0.12", default-features = false, features = ["rustls-tls-native-roots", "http2", "socks", "json"] } rustls = { version = "0.23", default-features = false, features = ["std", "ring"] } rustls-pemfile = "2.2" @@ -58,15 +49,11 @@ rustls-pki-types = "1" sha2 = "0.10" serde_json = "1" serde = { version = "1", features = ["derive"] } -serde_yaml = "0.9" -semver = "1" tap = "1.0" tempfile = "3" tokio-rustls = { version = "0.26", default-features = false, features = ["logging", "tls12", "ring"] } tokio-stream = "0.1" -tokio-util = "0.7" tonic = { version = "0.12", features = ["transport"] } -tower-service = "0.3" tracing = "0.1" url = { version = "2.5", features = ["serde"] } x509-parser = "0.16" @@ -101,10 +88,10 @@ aws-lc-rs = "1.15" time = { version = "0.3", features = ["local-offset", "macros", "parsing"] } devolutions-pedm = { path = "../crates/devolutions-pedm" } notify-debouncer-mini = "0.6" +now-package-broker = { path = "../crates/now-package-broker" } reqwest = { version = "0.12", default-features = false, features = ["rustls-tls-native-roots", "http2", "socks"] } thiserror = "2" uuid = { version = "1.17", features = ["v4"] } -widestring = "1.2" win-api-wrappers = { path = "../crates/win-api-wrappers" } [target.'cfg(windows)'.dependencies.windows] @@ -118,13 +105,7 @@ features = [ "Win32_Security_Cryptography", "Win32_Security_Authorization", "Win32_System_ApplicationInstallationAndServicing", - "Win32_System_Ioctl", - "Win32_System_IO", - "Win32_System_Pipes", "Win32_System_RemoteDesktop", - "Win32_System_Com", - "Win32_System_Console", - "Win32_UI_Shell", ] [target.'cfg(windows)'.build-dependencies] diff --git a/devolutions-agent/src/broker/mod.rs b/devolutions-agent/src/broker/mod.rs deleted file mode 100644 index a551c0a0d..000000000 --- a/devolutions-agent/src/broker/mod.rs +++ /dev/null @@ -1,20 +0,0 @@ -//! Package broker module. -//! -//! Provides policy evaluation and command execution for package operations, -//! communicating over a Windows named pipe using HTTP/1.1. - -pub(crate) mod auth; -pub mod command_builder; -pub mod evaluator; -pub mod event_channel; -pub mod executor; -pub mod operation_tracker; -pub mod pipe; -pub mod policy_loader; -pub(crate) mod policy_security; -pub mod policy_watcher; -pub mod server; -pub mod task; - -#[cfg(test)] -mod scenario_tests; diff --git a/devolutions-agent/src/broker/pipe.rs b/devolutions-agent/src/broker/pipe.rs deleted file mode 100644 index 7a4d9730a..000000000 --- a/devolutions-agent/src/broker/pipe.rs +++ /dev/null @@ -1,197 +0,0 @@ -//! Named pipe transport for Windows. -//! -//! Creates a named pipe server with appropriate ACLs and accepts connections, -//! forwarding them to the HTTP server. - -#[cfg(windows)] -mod windows_pipe { - use std::sync::Arc; - - use anyhow::Context as _; - use tokio::net::windows::named_pipe::{NamedPipeServer, ServerOptions}; - use tokio::sync::Semaphore; - use tokio_util::sync::CancellationToken; - use tracing::{error, info, warn}; - use win_api_wrappers::identity::sid::Sid; - use win_api_wrappers::security::acl::{Acl, ExplicitAccess, InheritableAcl, InheritableAclKind, Trustee}; - use win_api_wrappers::security::attributes::SecurityAttributesInit; - use windows::Win32::Foundation::GENERIC_ALL; - use windows::Win32::Security; - use windows::Win32::Security::Authorization::SET_ACCESS; - use windows::Win32::Storage::FileSystem::{FILE_GENERIC_READ, FILE_GENERIC_WRITE}; - - use crate::broker::auth::PipeClient; - use crate::broker::server::{BrokerState, build_router_for_client, serve_connection}; - - /// Default pipe name for the package broker. - pub const DEFAULT_PIPE_NAME: &str = r"\\.\pipe\Devolutions.Now.PackageBroker.v1"; - - /// Maximum number of concurrently served pipe connections. - /// - /// Connection setup performs unauthenticated work (client process identity lookups) - /// before any signature gate, so a connection flood could otherwise trigger unbounded - /// work and task spawning. While all slots are taken, no pipe instance is listening and - /// further clients fail to connect until a slot frees up. - const MAX_CONCURRENT_CONNECTIONS: usize = 16; - - /// Deadline for serving a single pipe connection, from accept to response completion. - /// - /// Each connection serves exactly one HTTP request (`keep_alive` is disabled) and all - /// endpoints respond without blocking on package operations (execution is asynchronous, - /// tracked via the operation tracker), so a healthy exchange completes well within this - /// deadline. Without it, idle clients holding their connection open without sending a - /// request would each pin a connection slot indefinitely and could exhaust the pool. - const CONNECTION_DEADLINE: std::time::Duration = std::time::Duration::from_secs(30); - - /// Start the named pipe server and accept connections until shutdown. - pub async fn run_pipe_server(state: Arc, shutdown: CancellationToken) -> anyhow::Result<()> { - let pipe_name = state.pipe_name.clone(); - info!(%pipe_name, "Starting named pipe server"); - - let connection_permits = Arc::new(Semaphore::new(MAX_CONCURRENT_CONNECTIONS)); - - let mut first_instance = true; - loop { - // Wait for a free connection slot before exposing a new pipe instance, - // bounding the number of concurrently served connections. - let permit = tokio::select! { - permit = Arc::clone(&connection_permits).acquire_owned() => { - permit.expect("the semaphore is never closed") - } - _ = shutdown.cancelled() => { - info!("Pipe server shutting down"); - return Ok(()); - } - }; - - // Create a new pipe instance for each connection. - let server = create_pipe_instance(&pipe_name, first_instance)?; - first_instance = false; - - tokio::select! { - result = server.connect() => { - match result { - Ok(()) => { - let state = Arc::clone(&state); - tokio::spawn(async move { - // The permit is held for the lifetime of the connection task. - let _permit = permit; - - let serve = async move { - // Capture the client identity off the accept loop so a slow - // lookup cannot stall accepting other connections. - let client = match PipeClient::from_connected_pipe(&server) { - Ok(client) => client, - Err(error) => { - warn!(%error, "Rejected named pipe client"); - return; - } - }; - info!("Client connected to named pipe"); - let router = build_router_for_client(state, client); - serve_connection(server, router).await; - info!("Client disconnected from named pipe"); - }; - - // Enforce a deadline so idle or slow clients cannot pin - // a connection slot indefinitely. - if tokio::time::timeout(CONNECTION_DEADLINE, serve).await.is_err() { - warn!("Closed named pipe connection: deadline exceeded"); - } - }); - } - Err(error) => { - error!(%error, "Failed to accept pipe connection"); - } - } - } - _ = shutdown.cancelled() => { - info!("Pipe server shutting down"); - return Ok(()); - } - } - } - } - - fn create_pipe_instance(pipe_name: &str, first_instance: bool) -> anyhow::Result { - let security_attributes = - build_pipe_security_attributes().context("failed to build pipe security attributes")?; - - // SAFETY: `create_with_security_attributes_raw` requires a pointer to a valid - // `SECURITY_ATTRIBUTES` that stays alive for the duration of the call. The pointer - // comes from `security_attributes` (a `win_api_wrappers::security::SecurityAttributes`), - // a local binding that owns the structure and its security descriptor and is dropped - // only at the end of this function, well after the call returns. `CreateNamedPipeW` - // copies the descriptor at creation, so the pointer is not retained afterwards. - let server = unsafe { - ServerOptions::new() - .first_pipe_instance(first_instance) - .create_with_security_attributes_raw(pipe_name, security_attributes.as_mut_ptr().cast()) - }?; - - Ok(server) - } - - /// Build a security descriptor that grants: - /// - SYSTEM: full control - /// - Administrators: full control - /// - BUILTIN\Users: read + write (allows interactive users to connect) - fn build_pipe_security_attributes() -> anyhow::Result { - let system_sid = - Sid::from_well_known(Security::WinLocalSystemSid, None).context("failed to create SYSTEM SID")?; - let admins_sid = Sid::from_well_known(Security::WinBuiltinAdministratorsSid, None) - .context("failed to create Administrators SID")?; - let users_sid = - Sid::from_well_known(Security::WinBuiltinUsersSid, None).context("failed to create Users SID")?; - - let entries = [ - ExplicitAccess { - access_permissions: GENERIC_ALL.0, - access_mode: SET_ACCESS, - inheritance: Security::ACE_FLAGS(0), - trustee: Trustee::Sid(system_sid), - }, - ExplicitAccess { - access_permissions: GENERIC_ALL.0, - access_mode: SET_ACCESS, - inheritance: Security::ACE_FLAGS(0), - trustee: Trustee::Sid(admins_sid), - }, - ExplicitAccess { - access_permissions: FILE_GENERIC_READ.0 | FILE_GENERIC_WRITE.0, - access_mode: SET_ACCESS, - inheritance: Security::ACE_FLAGS(0), - trustee: Trustee::Sid(users_sid), - }, - ]; - - let empty_acl = Acl::new().context("failed to create empty ACL")?; - let dacl = empty_acl.set_entries(&entries).context("failed to set ACL entries")?; - - let attrs = SecurityAttributesInit { - dacl: Some(InheritableAcl { - kind: InheritableAclKind::Protected, - acl: dacl, - }), - ..Default::default() - } - .init(); - - Ok(attrs) - } -} - -#[cfg(windows)] -pub use windows_pipe::*; - -/// Fallback for non-Windows (pipe transport not supported). -#[cfg(not(windows))] -pub const DEFAULT_PIPE_NAME: &str = "not-supported-on-this-platform"; - -#[cfg(not(windows))] -pub async fn run_pipe_server( - _state: std::sync::Arc, - _shutdown: tokio_util::sync::CancellationToken, -) -> anyhow::Result<()> { - anyhow::bail!("named pipe transport is only supported on Windows") -} diff --git a/devolutions-agent/src/lib.rs b/devolutions-agent/src/lib.rs index d0d05e994..b508f1889 100644 --- a/devolutions-agent/src/lib.rs +++ b/devolutions-agent/src/lib.rs @@ -5,10 +5,6 @@ use ctrlc as _; #[macro_use] extern crate tracing; -#[cfg(windows)] -pub mod broker; -#[cfg(windows)] -pub(crate) mod code_signing; pub mod config; pub mod domain_detect; pub mod enrollment; diff --git a/devolutions-agent/src/service.rs b/devolutions-agent/src/service.rs index 8b04d740b..631d793b4 100644 --- a/devolutions-agent/src/service.rs +++ b/devolutions-agent/src/service.rs @@ -2,10 +2,6 @@ use std::time::Duration; use anyhow::Context; use devolutions_agent::AgentServiceEvent; -#[cfg(windows)] -use devolutions_agent::broker::pipe::DEFAULT_PIPE_NAME; -#[cfg(windows)] -use devolutions_agent::broker::task::{BrokerTask, BrokerTaskConfig}; use devolutions_agent::config::ConfHandle; use devolutions_agent::log::AgentLog; use devolutions_agent::psu_agent::PsuAgentTask; @@ -19,6 +15,10 @@ use devolutions_gateway_task::{ChildTask, ShutdownHandle, ShutdownSignal}; use devolutions_log::{self, LogDeleterTask, LoggerGuard}; #[cfg(windows)] use devolutions_pedm::PedmTask; +#[cfg(windows)] +use now_package_broker::pipe::DEFAULT_PIPE_NAME; +#[cfg(windows)] +use now_package_broker::task::{BrokerTask, BrokerTaskConfig}; use tokio::runtime::{self, Runtime}; use tokio::sync::mpsc; diff --git a/devolutions-agent/src/updater/package.rs b/devolutions-agent/src/updater/package.rs index 3289c82d2..f0e99bbda 100644 --- a/devolutions-agent/src/updater/package.rs +++ b/devolutions-agent/src/updater/package.rs @@ -3,10 +3,12 @@ use std::ops::DerefMut; use camino::{Utf8Path, Utf8PathBuf}; +use devolutions_agent_shared::windows::code_signing::{ + certificate_sha1_thumbprint, is_devolutions_certificate_thumbprint, +}; use uuid::Uuid; use win_api_wrappers::utils::WideString; -use crate::code_signing::{certificate_sha1_thumbprint, is_devolutions_certificate_thumbprint}; use crate::updater::io::remove_file_on_reboot; use crate::updater::{AGENT_UPDATE_IN_PROGRESS, Product, UpdaterCtx, UpdaterError};