cli: check the command line that a sandbox launches (SL6.0f)

doc/run.md

@@ -1019,10 +1019,11 @@ treats as whitespace, including `0xa0`, while the workload's entrypoint and
 arguments, which nvx-init receives over the control session, may.
 Sandbox `--cmdline` uses Linux's quote and separator rules, and refuses reserved
 names even when quoted or written with hyphens instead of underscores. Validation
-uses the same trimmed text that is rendered and also checks the guest bootstrap's
-whitespace-only view, so reserved settings inside quoted values cannot override
-the generated settings. It also
-refuses a `--` or unclosed quote that hides the generated root tokens.
+reads the text that the kernel command line holds, after the generated settings,
+and also checks the guest bootstrap's whitespace-only view, so reserved settings
+inside quoted values cannot override the generated settings. It also refuses a
+`--` or unclosed quote that hides the tokens that OpenVMM appends, and double
+quotes, which the VM control console cannot represent.
 nvx-init runs the workload as a plain process in the VM's root, with no
 capabilities and `no_new_privs`, as it runs every
 [managed](#managed-lifecycle) execution. The workload always runs as the fixed

doc/usage.md

@@ -376,7 +376,7 @@ do, and their names cannot be abbreviated either.
 | `--network-proxy IPV4:TCP-PORT` | none | Make the network proxy-only: the workload reaches the HTTP or HTTPS proxy that listens on host `127.0.0.1:TCP-PORT` at `IPV4:TCP-PORT`, where `IPV4` must be the guest gateway, and nothing else. `TCP-PORT` can't be 53, where OpenVMM answers DNS at the gateway. Requires `--network-egress deny` and `--host-loopback deny`, and refuses egress rules, `--network-ingress allow`, and forwards. NVX gives the workload no proxy settings; see [Proxy-only networking](run.md#proxy-only-networking). |
 | `--host-loopback-forward PROTOCOL:HOST_PORT:GUEST_PORT` | none | Publish one TCP or UDP localhost port to the guest; repeat to add forwards. Requires `--host-loopback allow`. A UDP forward also requires egress allowed by default and no deny rule that covers the guest network, which carries its replies; OpenVMM lets the replies on a forwarded TCP connection through whatever the egress policy. |
 | `--outcome-report PATH` | none | Write a bounded local JSON outcome report: for `run`, OpenVMM's report of its VM, which the guest stopped after the workload; for `exec`, the workload's outcome. |
-| `--cmdline TEXT` | empty | Append non-sandbox kernel parameters, which `nvx-rs` reads as Linux does: split at whitespace, which includes the `0xa0` in `à`, outside double quotes, with names that match whether they spell a dash or an underscore. Quoted values, such as `a="b c"`, keep their whitespace. Parameters whose names start with `nvx_` or `virtfs_` are reserved and refused, even inside a quoted value, and so is a `--` or unclosed quote that hides the sandbox's own tokens; see [Experimental single-workload sandbox](run.md#experimental-single-workload-sandbox). The guest powers off if its kernel applies a clock parameter that the time ABI forbids, such as `tsc=unstable`. |
+| `--cmdline TEXT` | empty | Append non-sandbox kernel parameters, which `nvx-rs` reads as Linux does: split at whitespace, which includes the `0xa0` in `à`, outside double quotes, with names that match whether they spell a dash or an underscore. Parameters whose names start with `nvx_` or `virtfs_` are reserved and refused, even inside a quoted value, and so are a `--` or unclosed quote that hides the tokens that OpenVMM appends, and double quotes, which the VM control console cannot represent; see [Experimental single-workload sandbox](run.md#experimental-single-workload-sandbox). The guest powers off if its kernel applies a clock parameter that the time ABI forbids, such as `tsc=unstable`. |
 | `--dry-run` | off | For `run` only, print the generated OpenVMM microVM command without launching it. Other operations reject this option. |
 
 `run` and `start` install the release of a Git checkout that holds no

nvx-rs/src/host/cli/run.rs

@@ -54,7 +54,8 @@ use super::download::{self, Hold};
 use super::network::{NativeRules, NetworkOptions};
 use super::sandbox::{
     self, HYPERVISORS, MAX_MEMORY_MIB, MOUNT_OWNERS, copy_openvmm_outcome, error_with_cause,
-    hypervisor, path_string, path_text, sandbox_network, validate_outcome_destination,
+    export_tokens, hypervisor, path_string, path_text, sandbox_network,
+    validate_outcome_destination,
 };
 use super::session::{AttachmentCancellation, Session};
 use super::shell::format_command;
@@ -63,16 +64,17 @@ use super::{Exit, Result, check_interrupt, output};
 use crate::ErrorCode;
 use crate::activation::{MachineIdMode, View};
 use crate::host::artifacts::{ArtifactDir, Guest, require_artifact};
-use crate::host::config::{Boot, ConsoleAttachment, NicConfig, RootConfig, VmConfig};
+use crate::host::config::{
+    Boot, COMMAND_LINE_BUDGET, ConsoleAttachment, NicConfig, RootConfig, VmConfig,
+};
 use crate::host::filesystem::{
     self, ExportPlan, ExportRequest, Mapping, Owner, PathList, ReservedTargets, Target,
 };
 use crate::host::platform::{self, Transport};
 use crate::host::runtime::{ArtifactIdentity, LaunchOptions, Runtime, RuntimeIdentity};
-use crate::host::sandbox::SANDBOX_COMMAND_LINE_MAX_SIZE;
 use crate::host::sandbox::mount::{
-    MountAccess, MountKind, MountOwner, POLICY_KINDS, PolicyKind, export_command_line_fragment,
-    mount_kind, parse_mount_value, require_mount_owner_supported,
+    MountAccess, MountKind, MountOwner, POLICY_KINDS, PolicyKind, mount_kind, parse_mount_value,
+    require_mount_owner_supported,
 };
 use crate::host::sandbox::options::{SandboxOperation, validate_run_share_targets};
 use crate::host::vm::{self, VmDir};
@@ -665,9 +667,7 @@ fn cold_vm_config(
     };
     if let Some(export) = &config.filesystem {
         let line = config.kernel_command_line().map_err(error_with_cause)?;
-        if line.len() + export_command_line_fragment(export).len() + 1
-            > SANDBOX_COMMAND_LINE_MAX_SIZE
-        {
+        if line.len() + export_tokens(export).len() + 1 > COMMAND_LINE_BUDGET {
             return Err(Exit::error(
                 ErrorCode::Unsupported,
                 "the --mount shares and --cmdline exceed the 1024-byte budget of the kernel command \

nvx-rs/src/host/cli/run/tests.rs

@@ -66,11 +66,12 @@ fn cold_share_command_lines_fit_the_kernel_budget_to_the_byte() {
     };
     let rules = options.network.egress_rules().unwrap();
     let base = cold_config(&options, &rules).unwrap();
-    let fragment = export_command_line_fragment(base.filesystem.as_ref().unwrap());
-    let room = SANDBOX_COMMAND_LINE_MAX_SIZE
-        - base.kernel_command_line().unwrap().len()
-        - fragment.len()
-        - 2;
+    let fragment = export_tokens(base.filesystem.as_ref().unwrap());
+    assert_eq!(
+        fragment,
+        " virtfs_dir=/run/nvx/hostfs/root virtfs_tag=microvm virtfs_mode=rw virtfs_aggregate=1"
+    );
+    let room = COMMAND_LINE_BUDGET - base.kernel_command_line().unwrap().len() - fragment.len() - 2;
     for unicode in [false, true] {
         options.cmdline = if unicode {
             "\u{e9}".repeat(room / 2) + &"x".repeat(room % 2)
@@ -80,7 +81,7 @@ fn cold_share_command_lines_fit_the_kernel_budget_to_the_byte() {
         let fitting = cold_config(&options, &rules).unwrap();
         assert_eq!(
             fitting.kernel_command_line().unwrap().len() + fragment.len() + 1,
-            SANDBOX_COMMAND_LINE_MAX_SIZE
+            COMMAND_LINE_BUDGET
         );
         options.cmdline.push('x');
         assert!(cold_config(&options, &rules).is_err());

nvx-rs/src/host/cli/sandbox/lifecycle/start.rs

@@ -27,14 +27,14 @@ use crate::host::cli::sandbox::state::{
     Config, RecordedDistro, RecordedScratch, Source, read_config, serialize_launch,
 };
 use crate::host::cli::sandbox::{
-    Artifacts, Options, error_with_cause, path_string, refuse_own_files,
+    Artifacts, Options, error_with_cause, export_tokens, path_string, refuse_own_files,
 };
 use crate::host::cli::session::{ExecRequest, ExecResult, Session};
 use crate::host::cli::shell::format_command;
 use crate::host::cli::snapshots::Store;
 use crate::host::cli::store::StateFile;
 use crate::host::cli::{Exit, Result, hex, io_error, output, random_bytes};
-use crate::host::config::{RootConfig, UpperConfig, VmConfig};
+use crate::host::config::{COMMAND_LINE_BUDGET, RootConfig, UpperConfig, VmConfig};
 use crate::host::platform;
 use crate::host::process::KILL_TIMEOUT;
 use crate::host::report::Report;
@@ -43,10 +43,11 @@ use crate::host::restore_driver::{self, Cancel, ControlRestoreError, Workload};
 use crate::host::runtime::{ArtifactIdentity, Runtime, RuntimeIdentity};
 use crate::host::sandbox::distro::DISTRO_POLICY_REVISION;
 use crate::host::sandbox::scratch::{CLAIM_RELEASE_WAIT, CheckedScratch};
-use crate::host::sandbox::{SandboxLaunch, SandboxSettings, SandboxUpper};
 use crate::host::session::{ExecEvent, SessionError};
 use crate::host::snapshot::OPENVMM_DIR;
 use crate::host::vm::{self, Launched, Prepared, SavedVm, VmDir};
+use crate::protocol::cmdline::{self, Parameter};
+use crate::protocol::root::RootLayout;
 use crate::protocol::workload::WorkloadIdentity;
 use crate::protocol::{self, ExitCategory, HostMap, WorkloadEnvironment};
 use crate::{ErrorCode, ErrorKind};
@@ -133,17 +134,24 @@ fn start_locked(
         ),
         _ => None,
     };
-    // Keep provision's refusal of caller-owned nvx_* tokens, without launching its old plan.
-    check_command_line(&config)?;
+    let vm_config = config.vm_config()?;
+    check_command_line(&config, &vm_config)?;
     artifacts.require()?;
     let runtime = runtime_for(artifacts, &config, artifacts.launch_identity()?)?;
     let restore = source.as_ref().map(|source| Restore {
         source,
         identity: config.workload_identity(),
         forward_status,
     });
-    let config = config.vm_config()?;
-    let started = launch_locked(lock, &runtime, config, restore, scratch, timeout, vm::spawn);
+    let started = launch_locked(
+        lock,
+        &runtime,
+        vm_config,
+        restore,
+        scratch,
+        timeout,
+        vm::spawn,
+    );
     drop(distro);
     started
 }
@@ -574,8 +582,8 @@ pub(crate) fn run(
     let recorded = record(distro, scratch, provision).and_then(|config| {
         // As `start` does, refuse the user's own nvx_* tokens, and the command line that the
         // launch would refuse, such as one that holds double quotes.
-        check_command_line(&config)?;
         let vm_config = config.vm_config()?;
+        check_command_line(&config, &vm_config)?;
         vm_config.check_command_line().map_err(error_with_cause)?;
         Session::exec_payload(request)?;
         Ok((config, vm_config))
@@ -768,26 +776,65 @@ fn record(
     Ok(config)
 }
 
-/// Refuses the user's kernel command line of `config` as the plan of the sandbox profile refuses
-/// it: a NUL; a token that the profile or the `--mount` option owns, an `nvx_*` or `virtfs_*` one,
-/// as Linux or the guest's init reads it; and a line that, with the boot tokens of the root and
-/// the bootstrap tokens of the shares, exceeds its 1024-byte x86 budget, or conflicts with or hides
-/// those of the root. It builds that plan from the configuration: the one part of a start and a
-/// run that still goes through the old `host::sandbox` launch.
-fn check_command_line(config: &Config) -> Result {
-    let upper = match config.scratch() {
-        Some(scratch) => SandboxUpper::scratch_at(scratch),
-        None => SandboxUpper::RAM,
-    };
-    let settings = SandboxSettings {
-        hostname: config.hostname().to_owned(),
-        workload_identity: config.workload_identity(),
-        ..SandboxSettings::default()
-    };
-    SandboxLaunch::new(vec![config.distro()], upper, settings)?
-        .with_export(config.export().cloned())?
-        .bound_plan(config.distro_binding())?
-        .kernel_command_line(config.cmdline())?;
+/// Refuses the kernel command line that a start or a run of the sandbox of `config` launches, as
+/// `vm`, its VM's configuration, renders it: one whose user's part holds a NUL, or a token that
+/// the sandbox profile or the `--mount` option owns, an `nvx_*` or `virtfs_*` one, as Linux or
+/// the guest's init reads it; one that, with the bootstrap tokens that OpenVMM appends for the
+/// shares and a NUL, exceeds [`COMMAND_LINE_BUDGET`]; one from which the guest cannot read the
+/// boot tokens of its root, as when the user's part adds a virtio-mmio device at the scratch
+/// disk's base beside a RAM upper; and one that hides from Linux the tokens that OpenVMM appends
+/// to it, with a `--` or an unclosed quote, as Linux reads them.
+fn check_command_line(config: &Config, vm: &VmConfig) -> Result {
+    let refused = |message: &str| Exit::error(ErrorCode::Unsupported, message);
+    let user = config.cmdline().as_bytes();
+    if user.contains(&0) {
+        return Err(refused("kernel command line contains an embedded NUL"));
+    }
+    let guest = cmdline::init_tokens(user).map(|token| Parameter {
+        name: token.name,
+        value: token.value,
+    });
+    for Parameter { name, .. } in cmdline::parameters(user).chain(guest) {
+        let owner = if name.starts_with(b"nvx_") || name.starts_with(b"nvx-") {
+            "the sandbox profile"
+        } else if name.starts_with(b"virtfs_") || name.starts_with(b"virtfs-") {
+            "the sandbox --mount option"
+        } else {
+            continue;
+        };
+        let name = String::from_utf8_lossy(name);
+        return Err(refused(&format!("{name} is owned by {owner}")));
+    }
+    let line = vm.kernel_command_line().map_err(error_with_cause)?;
+    let shares = vm
+        .filesystem
+        .as_ref()
+        .map_or(0, |plan| export_tokens(plan).len());
+    if line.len() + shares + 1 > COMMAND_LINE_BUDGET {
+        return Err(refused(
+            "sandbox kernel command line exceeds its 1024-byte x86 budget; use shorter arguments \
+             and kernel parameters",
+        ));
+    }
+    RootLayout::parse(line.as_bytes()).map_err(|error| {
+        refused(&format!(
+            "sandbox kernel command line conflicts with the boot tokens of its root: {error}"
+        ))
+    })?;
+    // Linux must read the tokens that OpenVMM appends, as it reads one after the line whose name
+    // the user's part, which holds no `nvx_*` token, cannot spell.
+    let appended = format!("{line} nvx_appended");
+    if !matches!(
+        cmdline::parameters(appended.as_bytes()).last(),
+        Some(Parameter {
+            name: b"nvx_appended",
+            value: None
+        })
+    ) {
+        return Err(refused(
+            "sandbox kernel command line hides the tokens that OpenVMM appends to it",
+        ));
+    }
     Ok(())
 }
 

nvx-rs/src/host/cli/sandbox/lifecycle/start/tests.rs

@@ -4,6 +4,7 @@ use std::fs;
 use std::path::PathBuf;
 
 use super::*;
+use crate::host::filesystem::ExportPlan;
 use crate::host::paths::resolve;
 use crate::host::sandbox::distro::DistroBinding;
 use crate::host::sandbox::layer::{LayerRole, SandboxLayer};
@@ -81,6 +82,110 @@ fn run_checks_again_that_its_shares_do_not_overlap() {
     );
 }
 
+/// A start and a run check the kernel command line that they launch: the user's part, after the
+/// boot tokens of the root and of the workload, and the bootstrap tokens that OpenVMM appends for
+/// the shares fill the 1024-byte budget to the byte, whatever the upper and the shares, and the
+/// guest must read the root's boot tokens from it.
+#[test]
+fn starts_check_the_kernel_command_line_that_they_launch() {
+    let directory = tempfile::tempdir().unwrap();
+    let root = resolve(directory.path()).unwrap();
+    let path = |name: &str| root.join(name).to_str().unwrap().to_owned();
+    for (name, content) in [
+        ("distro.erofs", "distro"),
+        ("scratch.ext4", "scratch"),
+        ("settings.json", "{}"),
+        ("output.log", ""),
+    ] {
+        fs::write(path(name), content).unwrap();
+    }
+    let layer = SandboxLayer::new(
+        LayerRole::Distro,
+        &path("distro.erofs"),
+        "11111111-1111-1111-1111-111111111111",
+    );
+    let distro = RecordedDistro::new(&layer, DistroBinding::verify(&layer).unwrap()).unwrap();
+    let scratch = PathBuf::from(path("scratch.ext4"));
+    let settings = format!("/etc/settings.json,{}", path("settings.json"));
+    let output = format!("/var/log/output.log,{},rw", path("output.log"));
+    let export = |shares: &[String]| plan_mounts(&sandbox_mounts(shares, &[], None).unwrap());
+    let read_only = export(std::slice::from_ref(&settings)).unwrap();
+    let writable = export(&[settings, output]).unwrap();
+    assert_eq!(
+        export_tokens(read_only.as_ref().unwrap()),
+        " virtfs_dir=/run/nvx/hostfs/root virtfs_tag=microvm virtfs_mode=ro virtfs_aggregate=1"
+    );
+    let configure = |cmdline: &str, scratch: Option<&Path>, export: Option<&ExportPlan>| {
+        let config = ProvisionConfig {
+            hypervisor: "kvm",
+            memory_mib: 256,
+            cmdline,
+            ram_upper_percent: None,
+            hostname: DEFAULT_HOSTNAME,
+            workload_identity: DEFAULT_WORKLOAD_IDENTITY,
+            export,
+            network: None,
+        };
+        let config = record(&distro, scratch, &config).unwrap();
+        let vm = config.vm_config().unwrap();
+        (config, vm)
+    };
+    let refusal = |cmdline: &str, scratch: Option<&Path>, export: Option<&ExportPlan>| {
+        let (config, vm) = configure(cmdline, scratch, export);
+        let error = check_command_line(&config, &vm).unwrap_err();
+        assert_eq!(error.code(), Some(ErrorCode::Unsupported), "{error:?}");
+        error.message().unwrap_or_default().to_owned()
+    };
+    for upper in [None, Some(scratch.as_path())] {
+        for export in [None, read_only.as_ref(), writable.as_ref()] {
+            let (_, vm) = configure("", upper, export);
+            let fragment = export.map(export_tokens).unwrap_or_default();
+            let room =
+                COMMAND_LINE_BUDGET - vm.kernel_command_line().unwrap().len() - fragment.len() - 2;
+            let (config, vm) = configure(&"x".repeat(room), upper, export);
+            check_command_line(&config, &vm).unwrap();
+            assert_eq!(
+                vm.kernel_command_line().unwrap().len() + fragment.len() + 1,
+                COMMAND_LINE_BUDGET
+            );
+            assert_eq!(
+                refusal(&"x".repeat(room + 1), upper, export),
+                "sandbox kernel command line exceeds its 1024-byte x86 budget; use shorter \
+                 arguments and kernel parameters"
+            );
+        }
+    }
+    assert_eq!(
+        refusal("quiet\0", None, None),
+        "kernel command line contains an embedded NUL"
+    );
+    // A `--` as Linux reads it, whose whitespace includes the 0xa0 that ends `à` and a vertical
+    // tab, and an unclosed quote hide the tokens that OpenVMM appends after the user's part, also
+    // after a parameter of any name.
+    for user in [
+        "quiet -- debug",
+        "quiet \u{e0}--",
+        "quiet\u{b}--",
+        "x --",
+        "x\u{b}--",
+        "a=\"b c",
+    ] {
+        assert_eq!(
+            refusal(user, None, None),
+            "sandbox kernel command line hides the tokens that OpenVMM appends to it",
+            "{user:?}"
+        );
+    }
+    // The user's part may add a virtio-mmio device, which the guest reads as the kernel does: one
+    // at the scratch disk's base conflicts with a RAM upper.
+    assert_eq!(
+        refusal("virtio_mmio.device=0x1000@0xd0006000:22", None, None),
+        "sandbox kernel command line conflicts with the boot tokens of its root: a RAM upper \
+         takes no scratch block device, but the command line adds a virtio-mmio device at \
+         0xd0006000"
+    );
+}
+
 /// A run removes its VM directory with the staging file of an outcome report that OpenVMM did
 /// not finish writing, as one that the run forced down can leave, and keeps a directory that
 /// holds a file of another name.