diff --git a/devolutions-agent/src/main.rs b/devolutions-agent/src/main.rs index 71ca21580..b2ed0acbd 100644 --- a/devolutions-agent/src/main.rs +++ b/devolutions-agent/src/main.rs @@ -36,6 +36,7 @@ extern crate tracing; mod service; use std::env; +use std::io::{self, BufRead}; use std::sync::mpsc; use anyhow::{Context as _, Result, bail}; @@ -143,6 +144,10 @@ fn parse_advertise_subnets(value: &str) -> Vec { } fn parse_up_command_args(args: &[String]) -> Result { + parse_up_command_args_with_reader(args, io::stdin().lock()) +} + +fn parse_up_command_args_with_reader(args: &[String], mut stdin_reader: R) -> Result { let mut gateway_url = None; let mut enrollment_token = None; let mut agent_name = None; @@ -168,6 +173,21 @@ fn parse_up_command_args(args: &[String]) -> Result { } if let Some(enrollment_string) = enrollment_string { + // A single hyphen means "read the enrollment string from stdin". + let enrollment_string = if enrollment_string == "-" { + let mut line = String::new(); + stdin_reader + .read_line(&mut line) + .context("failed to read enrollment string from stdin")?; + let trimmed = line.trim().to_owned(); + if trimmed.is_empty() { + bail!("enrollment string read from stdin is empty"); + } + trimmed + } else { + enrollment_string + }; + let claims = parse_enrollment_jwt(&enrollment_string)?; // The JWT itself is the Bearer token; the Gateway verifies the signature. diff --git a/devolutions-gateway/src/api/tunnel.rs b/devolutions-gateway/src/api/tunnel.rs index fee203c58..0d8656bda 100644 --- a/devolutions-gateway/src/api/tunnel.rs +++ b/devolutions-gateway/src/api/tunnel.rs @@ -44,6 +44,43 @@ fn validate_enrollment_jwt(token: &str, provisioner_key: &picky::key::PublicKey) ) } +#[deprecated = "make sure this is never used without a deliberate action"] +mod unsafe_debug { + // Any function in this module should only be used at development stage when deliberately + // enabling debugging options. + + use picky::jose::jws::RawJws; + use picky::jose::jwt::{self, JwtSig}; + + use crate::token::{AccessScope, EnrollmentTokenClaims}; + + /// Dangerous enrollment token validation procedure. + /// + /// Like [`validate_enrollment_jwt`], but skips signature and `exp`/`nbf` checks. + /// + /// Skips signature verification and `exp`/`nbf` checks. Only the scope + /// (`AgentEnroll` or `Wildcard`) is still enforced, so test tokens still + /// have to carry the right intent. + pub(super) fn dangerous_validate_enrollment_jwt(token: &str) -> bool { + warn!( + "**DEBUG OPTION** Using dangerous enrollment token validation for testing purposes. Make sure this is not happening in production!" + ); + + let Ok(jwt) = RawJws::decode(token).map(RawJws::discard_signature).map(JwtSig::from) else { + return false; + }; + + let Ok(validated) = jwt.validate::(&jwt::NO_CHECK_VALIDATOR) else { + return false; + }; + + matches!( + validated.state.claims.scope, + AccessScope::AgentEnroll | AccessScope::Wildcard + ) + } +} + #[derive(Deserialize)] pub struct EnrollRequest { /// Agent-generated UUID (the agent owns its identity). @@ -122,7 +159,14 @@ async fn enroll_agent( .as_ref() .ok_or_else(|| HttpError::not_found().msg("agent enrollment is not configured"))?; - if !validate_enrollment_jwt(provided_token, &conf.provisioner_public_key) { + let token_is_valid = if conf.debug.disable_token_validation { + #[allow(deprecated, reason = "properly gated by disable_token_validation debug option")] + unsafe_debug::dangerous_validate_enrollment_jwt(provided_token) + } else { + validate_enrollment_jwt(provided_token, &conf.provisioner_public_key) + }; + + if !token_is_valid { return Err(HttpError::forbidden().msg("invalid enrollment token")); } diff --git a/package/AgentWindowsManaged/Actions/CustomActions.cs b/package/AgentWindowsManaged/Actions/CustomActions.cs index a3d7f19f2..ebd15bdd0 100644 --- a/package/AgentWindowsManaged/Actions/CustomActions.cs +++ b/package/AgentWindowsManaged/Actions/CustomActions.cs @@ -366,10 +366,13 @@ ActionResult Fail(string msg) // else (advertise subnets, advertise domains) is patched into agent.json *after* // enrollment, so we don't accumulate parallel CLI surfaces for what is ultimately // configuration data. - string arguments = $"up --enrollment-string \"{enrollmentString}\""; + // + // The JWT is passed via stdin (sentinel `-`) to avoid exposing it in the process + // command line (visible to any local process via WMI / Process Explorer / ETW). + string arguments = "up --enrollment-string -"; if (resolvedName.Length != 0) { - arguments += $" --name \"{resolvedName}\""; + arguments += $" --name {EscapeArg(resolvedName)}"; } string Redact(string s) => s.Replace(enrollmentString, "***"); @@ -380,11 +383,17 @@ ActionResult Fail(string msg) UseShellExecute = false, RedirectStandardOutput = true, RedirectStandardError = true, + RedirectStandardInput = true, CreateNoWindow = true, WorkingDirectory = ProgramDataDirectory, }; using Process process = Process.Start(startInfo); + + // Write the JWT to stdin and close it so the child sees EOF. + process.StandardInput.Write(enrollmentString); + process.StandardInput.Close(); + if (!process.WaitForExit(60_000)) { try @@ -447,6 +456,51 @@ private static bool JwtHasAgentName(string jwt) } } + /// + /// Escape a single argument for the Windows command line using the + /// CommandLineToArgvW rules: internal double-quotes are escaped + /// as \", backslash runs immediately before a quote are doubled, + /// and the whole value is wrapped in double quotes. + /// + private static string EscapeArg(string arg) + { + StringBuilder sb = new(); + sb.Append('"'); + + for (int i = 0; i < arg.Length; i++) + { + int backslashes = 0; + while (i < arg.Length && arg[i] == '\\') + { + backslashes++; + i++; + } + + if (i == arg.Length) + { + // Trailing backslashes must be doubled because the + // closing quote follows immediately. + sb.Append('\\', backslashes * 2); + break; + } + else if (arg[i] == '"') + { + // Backslashes before a double-quote must be doubled, + // plus one extra to escape the quote itself. + sb.Append('\\', backslashes * 2 + 1); + sb.Append('"'); + } + else + { + sb.Append('\\', backslashes); + sb.Append(arg[i]); + } + } + + sb.Append('"'); + return sb.ToString(); + } + /// /// Patch the freshly-written agent.json's Tunnel section with the operator's /// advertised subnets and DNS suffixes from the wizard. Keeping this out of the diff --git a/testsuite/src/cli.rs b/testsuite/src/cli.rs index a18c5fa34..cfd818347 100644 --- a/testsuite/src/cli.rs +++ b/testsuite/src/cli.rs @@ -65,6 +65,24 @@ pub fn dgw_tokio_cmd() -> tokio::process::Command { cmd } +static AGENT_BIN_PATH: LazyLock = LazyLock::new(|| { + escargot::CargoBuild::new() + .manifest_path("../devolutions-agent/Cargo.toml") + .bin("devolutions-agent") + .current_release() + .current_target() + .run() + .expect("build Devolutions Agent") + .path() + .to_path_buf() +}); + +pub fn agent_assert_cmd() -> assert_cmd::Command { + let mut cmd = assert_cmd::Command::new(&*AGENT_BIN_PATH); + cmd.env("RUST_BACKTRACE", "0"); + cmd +} + pub fn assert_stderr_eq(output: &assert_cmd::assert::Assert, expected: expect_test::Expect) { let stderr = std::str::from_utf8(&output.get_output().stderr).unwrap(); expected.assert_eq(stderr); diff --git a/testsuite/src/dgw_config.rs b/testsuite/src/dgw_config.rs index 74cf16b04..038d5dd18 100644 --- a/testsuite/src/dgw_config.rs +++ b/testsuite/src/dgw_config.rs @@ -38,6 +38,17 @@ pub struct AiGatewayConfig { pub openai_api_key: Option, } +/// Configuration for the agent tunnel feature in tests. +#[derive(Clone, TypedBuilder)] +pub struct AgentTunnelConfig { + /// Whether the agent tunnel is enabled. + #[builder(default = true)] + pub enabled: bool, + /// UDP port for the QUIC listener. + #[builder(default, setter(into))] + pub listen_port: Option, +} + #[derive(TypedBuilder)] pub struct DgwConfig { #[builder(default, setter(into))] @@ -60,6 +71,9 @@ pub struct DgwConfig { /// Pass a path that does not yet exist to test behaviour before the folder is created. #[builder(default, setter(into))] recording_path: Option, + /// Agent tunnel (QUIC) configuration. + #[builder(default, setter(into))] + agent_tunnel: Option, } fn find_unused_port() -> u16 { @@ -92,6 +106,7 @@ impl DgwConfigHandle { ai_gateway, enable_unstable, recording_path, + agent_tunnel, } = config; let tempdir = tempfile::tempdir().context("create tempdir")?; @@ -155,6 +170,20 @@ impl DgwConfigHandle { String::new() }; + let agent_tunnel_json = if let Some(at_config) = agent_tunnel { + let listen_port = at_config.listen_port.unwrap_or_else(find_unused_port); + format!( + r#", + "AgentTunnel": {{ + "Enabled": {}, + "ListenPort": {listen_port} + }}"#, + at_config.enabled + ) + } else { + String::new() + }; + let config = format!( r#"{{ "ProvisionerPublicKeyData": {{ @@ -174,7 +203,7 @@ impl DgwConfigHandle { "__debug__": {{ "disable_token_validation": {disable_token_validation}, "enable_unstable": {enable_unstable} - }}{ai_gateway_json}{recording_path_json} + }}{ai_gateway_json}{recording_path_json}{agent_tunnel_json} }}"# ); diff --git a/testsuite/tests/cli/agent/mod.rs b/testsuite/tests/cli/agent/mod.rs new file mode 100644 index 000000000..786e3668c --- /dev/null +++ b/testsuite/tests/cli/agent/mod.rs @@ -0,0 +1 @@ +mod up; diff --git a/testsuite/tests/cli/agent/up.rs b/testsuite/tests/cli/agent/up.rs new file mode 100644 index 000000000..a9ee50454 --- /dev/null +++ b/testsuite/tests/cli/agent/up.rs @@ -0,0 +1,127 @@ +//! E2E tests for `devolutions-agent up` enrollment, +//! focusing on the `--enrollment-string -` stdin path. + +use base64::Engine as _; +use testsuite::cli::agent_assert_cmd; + +/// Build a JWT with the given payload. The header and signature are placeholders — +/// the agent does not verify them; only the Gateway does. +fn make_jwt(payload: serde_json::Value) -> String { + let header = serde_json::json!({ "alg": "RS256", "typ": "JWT" }); + let b64 = base64::engine::general_purpose::URL_SAFE_NO_PAD; + format!( + "{}.{}.{}", + b64.encode(header.to_string()), + b64.encode(payload.to_string()), + b64.encode("signature-placeholder"), + ) +} + +fn sample_jwt(jet_gw_url: &str) -> String { + make_jwt(serde_json::json!({ + "scope": "gateway.agent.enroll", + "exp": 1_999_999_999i64, + "jti": "00000000-0000-0000-0000-000000000000", + "jet_gw_url": jet_gw_url, + "jet_agent_name": "site-a-agent", + })) +} + +/// `up --enrollment-string -` reads the JWT from stdin. The enrollment fails (no +/// real gateway), but the fact that it gets past argument parsing proves stdin +/// reading works end-to-end. +#[test] +fn up_enrollment_string_from_stdin() { + let jwt = sample_jwt("https://gateway.example.com:7171"); + + let output = agent_assert_cmd() + .args(["up", "--enrollment-string", "-"]) + .write_stdin(jwt) + .assert() + .failure(); + + let stderr = std::str::from_utf8(&output.get_output().stderr).unwrap(); + assert!( + !stderr.contains("Invalid up arguments"), + "argument parsing should succeed; stderr was: {stderr}" + ); + assert!( + stderr.contains("Bootstrap failed"), + "should fail at enrollment, not parsing; stderr was: {stderr}" + ); +} + +/// `up --enrollment-string -` with empty stdin must report an error about an +/// empty enrollment string. +#[test] +fn up_enrollment_string_stdin_empty_is_error() { + let output = agent_assert_cmd() + .args(["up", "--enrollment-string", "-"]) + .write_stdin("") + .assert() + .failure(); + + let stderr = std::str::from_utf8(&output.get_output().stderr).unwrap(); + assert!( + stderr.contains("empty"), + "error should mention empty enrollment string; stderr was: {stderr}" + ); +} + +/// Enrollment against a real Gateway with token validation disabled. +/// +/// Starts a Gateway with agent tunnel enabled and token validation off, +/// builds a sample JWT pointing at the real Gateway URL, and runs +/// `devolutions-agent up` via stdin. The enrollment should succeed. +#[tokio::test] +async fn up_enrollment_against_real_gateway() { + use anyhow::Context as _; + use testsuite::cli::{agent_assert_cmd, dgw_tokio_cmd, wait_for_tcp_port}; + use testsuite::dgw_config::{AgentTunnelConfig, DgwConfig}; + + let config_handle = DgwConfig::builder() + .disable_token_validation(true) + .agent_tunnel(AgentTunnelConfig::builder().build()) + .enable_unstable(true) + .build() + .init() + .expect("init gateway config"); + + // Start a real Gateway. + let mut gateway = dgw_tokio_cmd() + .env("DGATEWAY_CONFIG_PATH", config_handle.config_dir()) + .kill_on_drop(true) + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .spawn() + .context("start gateway") + .expect("spawn gateway"); + + wait_for_tcp_port(config_handle.http_port()) + .await + .expect("gateway HTTP port ready"); + + // Build a sample JWT pointing at the real Gateway URL. + // Token validation is disabled, so the signature is not checked. + let jwt = sample_jwt(&format!("http://127.0.0.1:{}", config_handle.http_port())); + + // Run the agent with --enrollment-string - (stdin). + // Set DAGENT_CONFIG_PATH so certs are written to a temp directory. + let agent_data_dir = tempfile::tempdir().expect("create agent temp dir"); + + let output = agent_assert_cmd() + .env("DAGENT_CONFIG_PATH", agent_data_dir.path()) + .args(["up", "--enrollment-string", "-"]) + .write_stdin(jwt) + .timeout(std::time::Duration::from_secs(30)) + .assert() + .success(); + + let stderr = std::str::from_utf8(&output.get_output().stderr).unwrap(); + assert!( + !stderr.contains("Bootstrap failed"), + "enrollment should succeed; stderr was: {stderr}" + ); + + gateway.kill().await.ok(); +} diff --git a/testsuite/tests/cli/mod.rs b/testsuite/tests/cli/mod.rs index c895a483c..43da9ed3a 100644 --- a/testsuite/tests/cli/mod.rs +++ b/testsuite/tests/cli/mod.rs @@ -1,2 +1,3 @@ +mod agent; mod dgw; mod jetsocat;