fix(agent,agent-installer): avoid leaking tunnel enrollment JWT via process cmdline (#1798)

Author: Copilot

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<String> {
 }
 
 fn parse_up_command_args(args: &[String]) -> Result<UpCommand> {
+    parse_up_command_args_with_reader(args, io::stdin().lock())
+}
+
+fn parse_up_command_args_with_reader<R: BufRead>(args: &[String], mut stdin_reader: R) -> Result<UpCommand> {
     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<UpCommand> {
     }
 
     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.

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::<EnrollmentTokenClaims>(&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"));
     }
 

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)
             }
         }
 
+        /// <summary>
+        /// Escape a single argument for the Windows command line using the
+        /// <c>CommandLineToArgvW</c> rules: internal double-quotes are escaped
+        /// as <c>\"</c>, backslash runs immediately before a quote are doubled,
+        /// and the whole value is wrapped in double quotes.
+        /// </summary>
+        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();
+        }
+
         /// <summary>
         /// Patch the freshly-written agent.json's <c>Tunnel</c> section with the operator's
         /// advertised subnets and DNS suffixes from the wizard. Keeping this out of the

testsuite/src/cli.rs

@@ -65,6 +65,24 @@ pub fn dgw_tokio_cmd() -> tokio::process::Command {
     cmd
 }
 
+static AGENT_BIN_PATH: LazyLock<std::path::PathBuf> = 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);

testsuite/src/dgw_config.rs

@@ -38,6 +38,17 @@ pub struct AiGatewayConfig {
     pub openai_api_key: Option<String>,
 }
 
+/// 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<u16>,
+}
+
 #[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<std::path::PathBuf>,
+    /// Agent tunnel (QUIC) configuration.
+    #[builder(default, setter(into))]
+    agent_tunnel: Option<AgentTunnelConfig>,
 }
 
 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}
 }}"#
         );
 

testsuite/tests/cli/agent/mod.rs

@@ -0,0 +1 @@
+mod up;