diff --git a/doc/acceptance-case-matrix.md b/doc/acceptance-case-matrix.md index 52def558e..fef690697 100644 --- a/doc/acceptance-case-matrix.md +++ b/doc/acceptance-case-matrix.md @@ -341,7 +341,7 @@ suite in its shards, and the public sandbox scenarios in its guest and sandbox j | `run-console-restore` | retired | SL3.8b removes `nvx-rs run`'s restore of a raw OpenVMM snapshot and the boot console that it reattached on the terminal: `run --restore-snapshot` restores a named snapshot through `snapshot restore`'s path | | `sandbox-blocks` | kept | | | `sandbox-gpt-layer` | kept | The [cold cells](#cold-boots) of a GPT disk, over a scratch upper and over a RAM upper, through `run`, and `provision` and `start` | -| `sandbox-lifecycle` | kept | A [cold cell](#cold-boots): the EROFS layer over a scratch upper, through `provision` and `start`, which also reattach to the VM from a new process for each command | +| `sandbox-lifecycle` | kept | A [cold cell](#cold-boots): the EROFS layer over a scratch upper, through `provision` and `start`, which also reattach to the VM from a new process for each command. Since SL4.4c, its request that writes 2 MiB checks that `exec` streams all of it, where it checked NVXC version 1's 1 MiB bound | | `sandbox-proxy-only` | kept | Its proxy-only network, over the EROFS layer and a scratch upper; it is no [cold cell](#cold-boots) of its own | | `sandbox-ram-upper` | kept | A [cold cell](#cold-boots): the EROFS layer over a RAM upper, through `run`, and `provision` and `start`. Since SL3.10c, it checks that `provision` records the configuration's format 10 | | `scratch-snapshot` | kept | | diff --git a/doc/design/4.4-host-guest-control-interface.md b/doc/design/4.4-host-guest-control-interface.md index 2fab67599..1e64cb18b 100644 --- a/doc/design/4.4-host-guest-control-interface.md +++ b/doc/design/4.4-host-guest-control-interface.md @@ -40,8 +40,12 @@ and optionally an absolute working directory of at most 4,096 bytes and up to 256 `KEY=VALUE` environment entries of at most 4,096 bytes each, with distinct, non-empty keys. The environment either replaces the default environment or is layered over it, each entry replacing the default variable -of the same name. Output returns as separate standard output and standard -error streams within the credit window, at most 1 MiB of them combined for +of the same name. In version 2 of the protocol, which NVX's host speaks, the +host streams an execution's standard input, which the guest agents +acknowledge as they write it, with at most 64 KiB unacknowledged, until the +host ends it, and output returns as separate standard output and standard +error streams within the credit window, without a total limit. Version 1 +carries no standard input, and forwards at most 1 MiB of output combined for each execution. As Table 4.3 shows, an execution that runs ends with an exit report, which carries an outcome category and a status. @@ -53,7 +57,7 @@ report, which carries an outcome category and a status. | `signal` | A signal ended the workload; the status is 128 plus the signal number | | `timeout` | The workload exceeded its timeout and was killed; the status is 124 | | `cancelled` | A cancellation killed the workload; the status is 137 | -| `output-limit` | The workload's combined output exceeded 1 MiB, and the guest agents killed it; the status is 125 | +| `output-limit` | In version 1, the workload's combined output exceeded 1 MiB, and the guest agents killed it; the status is 125 | | `failed` | The guest agents could not supervise the workload; the status is 125 | Otherwise, the guest agents reply with an error, which carries one of the @@ -77,9 +81,15 @@ account for. ## 4.4.2 Feature Negotiation +Version 2 of the protocol negotiates nothing: every frame names its version, +the guest's `READY` names the version that its agents speak, and NVX's host +requires version 2, whose guest agents provide every behavior that the host +depends on. Version 1 negotiates features instead. + A successful readiness probe does not prove that an image enforces the -behaviors that a host depends on. As Table 4.5 shows, a `FEATURES` request -therefore returns a four-byte little-endian bit mask of control features. +behaviors that a host depends on. As Table 4.5 shows, a `FEATURES` request of +version 1 therefore returns a four-byte little-endian bit mask of control +features. **Table 4.5.** Control feature bits. @@ -110,10 +120,11 @@ Requests are typed, length-bounded, ordered, and associated with one VM instance and epoch. Credit limits bound queued data in both directions. Malformed, unauthenticated, stale, duplicated, or out-of-order records fail closed. Only one execution runs at a time; any request other than a -cancellation that arrives during an execution is refused as `busy`. Shutdown -is therefore graceful only between executions: a host cancels a running -execution before it requests shutdown, or else terminates the VM, which fails -the execution. +cancellation, or in version 2 the execution's standard input or a shutdown, +that arrives during an execution is refused as `busy`. In version 2, a +shutdown ends the running execution first. In version 1, shutdown is therefore +graceful only between executions: a host cancels a running execution before +it requests shutdown, or else terminates the VM, which fails the execution. The control session is process-local even when guest parser state is snapshotted. Restore establishes a fresh host capability, peer identity, diff --git a/doc/run.md b/doc/run.md index 0ba9e25f3..563eb97c2 100644 --- a/doc/run.md +++ b/doc/run.md @@ -908,11 +908,12 @@ VM directory, and prints its OpenVMM command as `>> COMMAND`. Once nvx-init is ready and has mapped the shares, the run executes its workload once over the control session, as [`sandbox exec`](#managed-lifecycle) does, with the same `--cwd`, `--environment`, `--environment-file`, -`--inherit-default-environment`, and `--exec-timeout-ms` options. It then asks +`--inherit-default-environment`, and `--exec-timeout-ms` options, and streams +its standard input, output and error as `sandbox exec` does. It then asks nvx-init to stop the VM, terminates OpenVMM if it does not exit within `--timeout` seconds, warning that it did, and removes the VM directory, and -finally prints the workload's standard output and standard error and exits -with its status. A VM that exits before nvx-init is ready, as when the guest +finally exits with the workload's status. A VM that exits before nvx-init is +ready, as when the guest refuses the workload identity, fails the run as it fails a `start`, with the category and status of the outcome report that OpenVMM published, such as `guest-exit status 125`. `--outcome-report` forwards that report, of the VM's @@ -1738,10 +1739,11 @@ rejects `--state-dir`. Environment files are limited to 1 MiB of UTF-8 JSON. Environments contain at most 256 unique, nonempty names. Each `KEY=VALUE` entry and working-directory path is limited to 4096 UTF-8 bytes; the combined execution request must also -fit the existing 64 KiB control-payload bound. A managed `exec` forwards at -most 1 MiB of combined standard output and standard error; the guest agent -kills a workload that writes more, and `exec` exits with status 125 and reports -the `output-limit` category. +fit the existing 64 KiB control-payload bound. A managed `exec` streams its +standard input to the workload, which sees it end when the command's own +input ends, and the workload's standard output and standard error back as +they arrive, without a limit on their size. Ctrl-C cancels the workload, and +`exec` exits with status 130. The `managed-exec-config` scenario of `test-microvm --backend BACKEND` is the authoritative acceptance for these public options. It invokes @@ -1795,11 +1797,10 @@ template, so it has the same artifact requirements. It checks that: is on scratch; - `/sbin/nvx-sandbox-smoke` passes its security profile checks in a sandbox provisioned with `--hostname`, after earlier requests; -- a request that writes more than the 1 MiB output bound exits with status - 125 and an `output-limit` outcome report after the host has received more - than 1 MiB less one 32 KiB read chunk and at most 1 MiB, and a request whose - `--outcome-report` path exists fails before its workload runs and leaves the - file intact; +- a request that writes 2 MiB, more than the 1 MiB that version 1 of NVXC + forwarded, streams all of it and exits with status 0 and an `exit` outcome + report, and a request whose `--outcome-report` path exists fails before its + workload runs and leaves the file intact; - `stop` ends OpenVMM and its control socket or pipe, leaves only `config.json`, `sandbox.lock`, `exec.lock`, `openvmm.log`, and a successful `outcome.json`, and leaves a cleanly unmounted scratch file system, after diff --git a/doc/usage.md b/doc/usage.md index 331feefe0..a5e0a6ca7 100644 --- a/doc/usage.md +++ b/doc/usage.md @@ -333,10 +333,13 @@ launches or is recorded; see [Sandbox networking](run.md#sandbox-networking). given. It exits with status 1 for an error, 2 for a usage error, and 130 after Ctrl-C, and otherwise with the status that the workload or OpenVMM returns. Its errors and usage errors carry their [error codes](#error-codes), as -`error[CODE]: MESSAGE` and `nvx-rs sandbox: error[CODE]: MESSAGE`; an `exec` -prints its workload's standard output and error before any error of its own, -which starts a new line. Options take their values as those of [`run`](#run) -do, and their names cannot be abbreviated either. +`error[CODE]: MESSAGE` and `nvx-rs sandbox: error[CODE]: MESSAGE`. An `exec`, +and the workload of a `run`, stream the command's standard input to the +workload until it ends, and the workload's standard output and error back as +they arrive, without a limit; a command whose input is the null device gives +the workload none. An error of the command's own follows the workload's output +on a new line, and Ctrl-C cancels the workload. Options take their values as +those of [`run`](#run) do, and their names cannot be abbreviated either. | Option | Default | Description | | --- | --- | --- | diff --git a/nvx-rs/README.md b/nvx-rs/README.md index 981d6f9f4..6d64115fe 100644 --- a/nvx-rs/README.md +++ b/nvx-rs/README.md @@ -151,9 +151,10 @@ frames, the `EXEC` payload that the host encodes and the guest agent decodes, and the control features that a guest image advertises. Beside version 1's frames, `protocol::app::v2` holds the codec of version 2's, which carry every control path, with version 1's payloads, and stream an execution's standard -input, output and error. The guest loop that nvx-init and the managed agent -share, and the fake OpenVMM, serve both versions, so `v2` follows their -features, `guest` and `testing`, until the host speaks it too. +input, output and error. NVX's host speaks version 2 alone, and the guest loop +that nvx-init and the managed agent share, and the fake OpenVMM, serve both +versions, so `v2` follows the features `cli`, `guest` and `testing`, and +version 1's frames those of their servers, `guest` and `testing`. The [conformance vectors](tests/vectors/README.md) of version 1, `tests/vectors/nvxc-v1.txt`, and OpenVMM's vectors and cases of the `NVXS` @@ -1050,7 +1051,11 @@ OpenVMM's outcome report. The Linux and Windows primitives are private to the crate, in `host::platform`, and so is the authenticated control session, which carries NVX's own control protocol, `NVXC`, one frame per DATA record of a `HostConnection`: the CLI and its restore driver use it, with the `cli` -feature. +feature. The session speaks version 2 alone, and requires the guest's `READY` +to name that version. It streams an execution's standard input, with an +explicit end, and keeps at most 64 KiB of it that the guest has not +acknowledged writing, and receives the execution's output without a total +limit; a `STOP` ends an execution that runs. `report::read` returns `Ok(None)` for a missing report, and an `Io` error for an unreadable or malformed report, retaining the underlying error. It keeps today's @@ -1905,16 +1910,17 @@ before them. as `nvx.py` quoted it with `subprocess.list2cmdline` and `shlex.join`. Unless `--dry-run` is given, it then executes the workload once through the control session, as `exec` does, with its working directory and environment - options, asks the guest to shut the VM down, waits up to `--timeout` seconds - for OpenVMM to exit before it terminates it, removes the VM directory, and - prints the workload's standard output and then its standard error, and exits - with its status. `--outcome-report` forwards OpenVMM's outcome report of the - VM's stop. + options, streaming its standard input, output and error, asks the guest to + shut the VM down, waits up to `--timeout` seconds for OpenVMM to exit before + it terminates it, removes the VM directory, and exits with the workload's + status. `--outcome-report` forwards OpenVMM's outcome report of the VM's + stop. - `provision` records the sandbox in its state directory. `start` launches OpenVMM detached with the managed lifecycle, passes it a fresh capability through its standard input, and waits until the guest answers. `exec` runs a - workload, prints its standard output and then its standard error, and exits - with its status; `--outcome-report` writes + workload, streams its own standard input to it, until that ends, and the + workload's standard output and error back as they arrive, and exits with the + workload's status; Ctrl-C cancels the workload. `--outcome-report` writes `{"operation_id": HEX, "outcome": {"category": CATEGORY, "operation": "exec", "status_code": STATUS}, "schema_version": 1}`. `stop` shuts the VM down and prints nothing, but for a warning when it terminates a VM of another @@ -2015,24 +2021,21 @@ OpenVMM that a start of such a build left behind when it was interrupted has no record, and that build asked for it to be ended by hand: end it before the switch too, since this build finds no VM in that sandbox. -#### Guest features - -After the guest answers, `start`, and `sandbox run`, ask it for its control -features and require `GuestFeatures::LAYERED_SANDBOX`, the features that their -launches use: per-execution environments (bit 4), since `exec` and -`sandbox run` send a working directory and an environment, and host path -mapping tables (bit 6), since they then send the mapping table of the -sandbox's shares, which the guest agent mounts before -it runs any workload; a mapping that it refuses fails the start, after the -guest shuts down as `stop` has it do. It needs neither cancellation (bit 0), -since the CLI never cancels a workload, and Ctrl-C ends the session, nor -workload account creation (bit 2), since a layered sandbox runs its workloads -as an account of the image. A guest image that lacks a required feature, or -that predates the request, is stopped as a failed start is, and the start -fails with -`the guest image lacks control features that layered sandboxes need (...)`, -which names the missing features. `nvx.py` does not ask; this, the checks of -the [distro binding](#distro-bindings) and of the +#### Guest protocol + +After the guest answers, `start`, and `sandbox run`, speak version 2 of `NVXC` +to it, whose `READY` names the version that the guest agent speaks, and require +that version: a guest image whose agent answers in another version, as one +that predates version 2 does, is stopped as a failed start is, and the start +fails. They ask for no control features, which only version 1's `FEATURES` +request reports, and which `start` required before: per-execution environments +and host path mapping tables, which every guest agent of version 2 provides. +`exec` and `sandbox run` send a working directory and an environment, and the +start then sends the mapping table of the sandbox's shares, which the guest +agent mounts before it runs any workload; a mapping that it refuses fails the +start, after the guest shuts down as `stop` has it do. `nvx.py` does not check +the version; this, the streams of `exec` and `sandbox run`, the checks of the +[distro binding](#distro-bindings) and of the [scratch binding](#images-by-path), and the claim of the scratch image are the intended changes of behavior. @@ -2110,8 +2113,11 @@ intended changes of behavior. launched exits, as `nvx::host::transport::connect_verified` does, and fails at once with `exited`, where `nvx.py` waited until `--timeout` passed before it reported the exit. -- `exec` refuses more than 1 MiB of workload output, which the guest agent - never forwards. +- `exec` and `sandbox run` stream the workload's standard input, output and + error while it runs, where `nvx.py` gave it no input and printed its output + once it ended. So a guest's diagnostic of a request that it refuses, such as + a working directory that the workload cannot enter, precedes the error. + Ctrl-C cancels the workload, where `nvx.py` closed the session. - `provision`, and `start` again, refuse a share whose resolved host path equals, contains, or lies inside the sandbox's state directory, image, or scratch image, or the OpenVMM, kernel, or initramfs of the artifact root, as @@ -2641,8 +2647,9 @@ tests, only the test hooks that The tests need no hypervisor. The fake-OpenVMM test runs `nvx-fake-openvmm` directly, as `nvx-rs run` runs OpenVMM on the terminal. The CLI test runs the -[scenario tests](#scenario-tests) of `nvx-rs`, and checks the guest-feature -requirement and, on Linux, Ctrl-C. The +[scenario tests](#scenario-tests) of `nvx-rs`, and checks that a start asks +the guest for no features, that `exec` and `sandbox run` stream, and, on +Linux, Ctrl-C, which also cancels an `exec`. The [conformance vectors](tests/vectors/README.md) of the wire formats cover both sides of every record, frame, and payload: the codec decodes each vector of `tests/vectors/nvxc-v1.txt` to its values and encodes them to its bytes, it diff --git a/nvx-rs/src/guest/managed_agent/tests/loopback/tests.rs b/nvx-rs/src/guest/managed_agent/tests/loopback/tests.rs index 28570ecbc..9667d2b28 100644 --- a/nvx-rs/src/guest/managed_agent/tests/loopback/tests.rs +++ b/nvx-rs/src/guest/managed_agent/tests/loopback/tests.rs @@ -2,8 +2,10 @@ use std::thread; use std::time::Instant; use crate::host::session::{ControlSession, ExecEvent, SessionError}; +use crate::protocol::v2::Outcome; use crate::protocol::{ - CAPABILITY_LEN, ExitCategory, GuestFeatures, HostMap, Workload, WorkloadEnvironment, + APP_EXEC, APP_EXIT, APP_STDERR, APP_STDOUT, CAPABILITY_LEN, HostMap, Workload, + WorkloadEnvironment, decode_app, encode_app, encode_exec_payload, }; use super::super::AgentConfig; @@ -16,24 +18,67 @@ fn deadline() -> Instant { Instant::now() + Duration::from_secs(10) } -fn run_exec( +/// Runs `workload` as the execution of `id`, with `input` as its standard input, and returns its +/// output and how it ended. +fn run_exec_with( session: &mut ControlSession, + id: u64, workload: &Workload<'_>, + input: &[u8], ) -> (Vec, Vec, ExecEvent) { - let id = session.start_exec(workload, deadline()).unwrap(); - let mut stdout = Vec::new(); - let mut stderr = Vec::new(); + session + .start_exec(id, &encode_exec_payload(workload).unwrap(), deadline()) + .unwrap(); + let (mut stdout, mut stderr, mut rest, mut open) = (Vec::new(), Vec::new(), input, true); loop { + // The window takes more of the input as the agent acknowledges what it wrote. + let sent = session.send_stdin(id, rest, deadline()).unwrap(); + rest = &rest[sent..]; + if rest.is_empty() && open { + session.close_stdin(id, deadline()).unwrap(); + open = false; + } match session.next_exec_event(id, Some(deadline())).unwrap() { ExecEvent::Stdout(bytes) => stdout.extend(bytes), ExecEvent::Stderr(bytes) => stderr.extend(bytes), - event @ ExecEvent::Exit { .. } | event @ ExecEvent::Rejected { .. } => { + ExecEvent::Acknowledged(_) => {} + event @ (ExecEvent::Exit { .. } | ExecEvent::Rejected { .. }) => { return (stdout, stderr, event); } } } } +fn run_exec( + session: &mut ControlSession, + workload: &Workload<'_>, +) -> (Vec, Vec, ExecEvent) { + run_exec_with(session, 1, workload, &[]) +} + +/// Runs `workload` over version 1's frames, which the agent still serves, and returns its output, +/// its exit category and its status. +fn run_exec_v1( + session: &mut ControlSession, + workload: &Workload<'_>, +) -> (Vec, Vec, (Vec, i32)) { + let payload = encode_exec_payload(workload).unwrap(); + let request = encode_app(APP_EXEC, 1, 0, &payload); + session.connection.send(&request, Some(deadline())).unwrap(); + let mut stdout = Vec::new(); + let mut stderr = Vec::new(); + loop { + let reply = session.connection.receive(Some(deadline())).unwrap(); + let frame = decode_app(&reply).unwrap(); + match frame.kind { + APP_STDOUT => stdout.extend(frame.payload), + APP_STDERR => stderr.extend(frame.payload), + APP_EXIT => return (stdout, stderr, (frame.payload.to_vec(), frame.status)), + kind => panic!("unexpected frame of kind {kind}"), + } + } +} + #[test] fn control_session_broker_and_real_agent_loop_back() { if super::super::isolate_process_test( @@ -71,10 +116,6 @@ fn control_session_broker_and_real_agent_loop_back() { let mut session = ControlSession::attach(broker.transport(), &capability, deadline()).unwrap(); session.ping(deadline()).unwrap(); - assert_eq!( - session.features(deadline()).unwrap(), - GuestFeatures::DIRECT_AGENT - ); session .map_host_paths( &[HostMap { @@ -100,7 +141,7 @@ fn control_session_broker_and_real_agent_loop_back() { assert_eq!( exit, ExecEvent::Exit { - category: ExitCategory::Exit, + category: Outcome::Exit, status: 7 } ); @@ -109,7 +150,7 @@ fn control_session_broker_and_real_agent_loop_back() { assert_eq!( signal, ExecEvent::Exit { - category: ExitCategory::Signal, + category: Outcome::Signal, status: 128 + sys::Signal::SIGTERM as i32 } ); @@ -117,7 +158,7 @@ fn control_session_broker_and_real_agent_loop_back() { assert_eq!( realtime_signal, ExecEvent::Exit { - category: ExitCategory::Signal, + category: Outcome::Signal, status: 128 + 40 } ); @@ -125,21 +166,40 @@ fn control_session_broker_and_real_agent_loop_back() { assert_eq!( timeout, ExecEvent::Exit { - category: ExitCategory::Timeout, + category: Outcome::Timeout, status: 124 } ); - let (_, _, limited) = run_exec(&mut session, &workload("head -c 1048577 /dev/zero", 0)); + // Version 2 caps no output: the old cap of 1 MiB passes whole. + let (stdout, _, uncapped) = run_exec(&mut session, &workload("head -c 1048577 /dev/zero", 0)); + assert_eq!(stdout, vec![0; 1024 * 1024 + 1]); + assert_eq!( + uncapped, + ExecEvent::Exit { + category: Outcome::Exit, + status: 0 + } + ); + // The workload reads the standard input that the host streams, more than the window holds, + // until its end. + let input: Vec = (0..200 * 1024).map(|index| (index % 251) as u8).collect(); + let (stdout, _, echoed) = run_exec_with(&mut session, 2, &workload("cat", 0), &input); + assert_eq!(stdout, input); assert_eq!( - limited, + echoed, ExecEvent::Exit { - category: ExitCategory::OutputLimit, - status: 125 + category: Outcome::Exit, + status: 0 } ); - let cancel_id = session - .start_exec(&workload("sleep 10", 0), deadline()) + let cancel_id = 3; + session + .start_exec( + cancel_id, + &encode_exec_payload(&workload("sleep 10", 0)).unwrap(), + deadline(), + ) .unwrap(); session.cancel_exec(cancel_id, deadline()).unwrap(); assert_eq!( @@ -147,13 +207,18 @@ fn control_session_broker_and_real_agent_loop_back() { .next_exec_event(cancel_id, Some(deadline())) .unwrap(), ExecEvent::Exit { - category: ExitCategory::Cancelled, + category: Outcome::Cancelled, status: 128 + sys::Signal::SIGKILL as i32 } ); - let reset_id = session - .start_exec(&workload("sleep 10", 0), deadline()) + let reset_id = 4; + session + .start_exec( + reset_id, + &encode_exec_payload(&workload("sleep 10", 0)).unwrap(), + deadline(), + ) .unwrap(); broker.reset(); assert!(matches!( @@ -163,6 +228,14 @@ fn control_session_broker_and_real_agent_loop_back() { drop(session); let mut session = ControlSession::attach(broker.transport(), &capability, deadline()).unwrap(); session.ping(deadline()).unwrap(); + // A stop during an execution ends it first. + session + .start_exec( + 5, + &encode_exec_payload(&workload("sleep 10", 0)).unwrap(), + deadline(), + ) + .unwrap(); session.stop(deadline()).unwrap(); drop(session); @@ -171,8 +244,8 @@ fn control_session_broker_and_real_agent_loop_back() { assert!(system.diagnostics.is_empty()); } -/// Once the output cap is hit, the agent forwards nothing more: each stream the client -/// receives stays a prefix of what the workload wrote, with no silent gap where a later +/// Once the output cap of NVXC version 1 is hit, the agent forwards nothing more: each stream the +/// client receives stays a prefix of what the workload wrote, with no silent gap where a later /// chunk fit the leftover room. #[test] fn output_limit_forwards_nothing_after_the_cap() { @@ -239,15 +312,10 @@ fn output_limit_forwards_nothing_after_the_cap() { cwd: None, environment: WorkloadEnvironment::Default, }; - let (stdout, stderr, exit) = run_exec(&mut session, &workload); + // Version 1, which caps the output, until SL4.4d deletes it. + let (stdout, stderr, exit) = run_exec_v1(&mut session, &workload); let count = |byte: u8| stdout.iter().filter(|&&each| each == byte).count(); - assert_eq!( - exit, - ExecEvent::Exit { - category: ExitCategory::OutputLimit, - status: 125 - } - ); + assert_eq!(exit, (b"output-limit".to_vec(), 125)); assert!(stderr.is_empty()); assert_eq!(count(b'a'), crate::protocol::MAX_OUTPUT_BYTES - room); assert!(count(b'b') <= room); diff --git a/nvx-rs/src/host/cli/sandbox/lifecycle/mod.rs b/nvx-rs/src/host/cli/sandbox/lifecycle/mod.rs index 65f8319d7..1b11bc969 100644 --- a/nvx-rs/src/host/cli/sandbox/lifecycle/mod.rs +++ b/nvx-rs/src/host/cli/sandbox/lifecycle/mod.rs @@ -44,7 +44,6 @@ use crate::host::runtime::{IdentityPart, LaunchOptions, Runtime, RuntimeIdentity use crate::host::sandbox::layer::SandboxLayer; use crate::host::sandbox::require_file; use crate::host::vm::SavedVm; -use crate::protocol::GuestFeatures; pub(crate) use self::directory::{create_state_directory, prepare_state_directory}; pub(crate) use self::outcome::{ @@ -81,10 +80,6 @@ pub(crate) struct ProvisionConfig<'a> { pub(crate) network: Option<&'a NetworkRecord>, } -/// The control features that the layered sandbox root must provide for the requests of -/// `nvx-rs sandbox`. -pub(crate) const REQUIRED_FEATURES: GuestFeatures = GuestFeatures::LAYERED_SANDBOX; - /// Records the launch of the distro layer `distro`, at the path at which it was verified, with /// the layer's binding, over `scratch`, the scratch image of an upper on scratch, with the options /// of `config`, in a new state directory, as `provision` does, with the binding of its scratch diff --git a/nvx-rs/src/host/cli/sandbox/lifecycle/start.rs b/nvx-rs/src/host/cli/sandbox/lifecycle/start.rs index 0ef21b4fe..e2e48b2cd 100644 --- a/nvx-rs/src/host/cli/sandbox/lifecycle/start.rs +++ b/nvx-rs/src/host/cli/sandbox/lifecycle/start.rs @@ -1,6 +1,7 @@ //! Starting a managed sandbox and executing workloads in its running VM, and running one workload //! in a VM of its own, without a state directory. +use std::cell::Cell; use std::fs; use std::io; use std::path::Path; @@ -14,8 +15,8 @@ use serde_json::Value; use super::directory::prepare_state_directory; use super::outcome::{copy_openvmm_outcome, validate_outcome_destination}; use super::{ - CONFIG_NAME, Found, ProvisionConfig, REQUIRED_FEATURES, connect, find_vm, mismatched, - require_config, running_vm, runtime_for, + CONFIG_NAME, Found, ProvisionConfig, connect, find_vm, mismatched, require_config, running_vm, + runtime_for, }; use crate::activation::{ActivationRecord, TransactionId}; use crate::host::cli::artifacts::locate; @@ -29,7 +30,7 @@ use crate::host::cli::sandbox::state::{ use crate::host::cli::sandbox::{ Artifacts, Options, error_with_cause, export_tokens, path_string, refuse_own_files, }; -use crate::host::cli::session::{ExecRequest, ExecResult, Session}; +use crate::host::cli::session::{ExecRequest, ExecResult, Session, closes}; use crate::host::cli::shell::format_command; use crate::host::cli::snapshots::Store; use crate::host::cli::store::StateFile; @@ -48,8 +49,9 @@ 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::v2::Outcome; use crate::protocol::workload::WorkloadIdentity; -use crate::protocol::{self, ExitCategory, HostMap, WorkloadEnvironment}; +use crate::protocol::{self, HostMap, WorkloadEnvironment}; use crate::{ErrorCode, ErrorKind}; /// Starts a provisioned sandbox, as `start` does. A workload of a restored sandbox that does not @@ -414,24 +416,12 @@ fn binding_error(error: crate::Error) -> Exit { } /// Waits until the guest of a VM that starts answers on `endpoint`, which `launched`, the OpenVMM -/// process of the start, serves, or until OpenVMM exits, checks that the guest provides the -/// features of [`REQUIRED_FEATURES`], and returns the session, on which the caller sends the -/// guest what it needs before the launch commits. +/// process of the start, serves, or until OpenVMM exits, and returns the session, on which the +/// caller sends the guest what it needs before the launch commits. The guest's Ready names the +/// version of NVXC that it speaks, which the session requires. fn check_ready(saved: &SavedVm, runtime: &Runtime, timeout: f64) -> Result { let mut session = Session::reattach(saved, runtime, timeout)?; session.ping(timeout)?; - let features = session.features(timeout)?; - let missing = features.missing(REQUIRED_FEATURES); - if !missing.is_empty() { - return Err(Exit::error( - ErrorCode::GuestFeaturesMissing, - format!( - "the guest image lacks control features that layered sandboxes need ({}); use \ - an NVX release or build that includes them", - missing.join(", ") - ), - )); - } Ok(session) } @@ -495,8 +485,9 @@ fn restore_guest( Ok(ExecEvent::Stderr(data)) => { output::write_stderr(&data).map_err(Exit::from_os_error)?; } + Ok(ExecEvent::Acknowledged(_)) => {} Ok(ExecEvent::Exit { - category: ExitCategory::Exit, + category: Outcome::Exit, status: 0, }) => return Ok(()), Ok(ExecEvent::Exit { status, .. }) if restore.forward_status => { @@ -533,8 +524,10 @@ fn restore_guest( ControlRestoreError::Maps(error) | ControlRestoreError::Start(error) => match error { SessionError::TimedOut => ErrorCode::TimedOut, // As for a cold start, a closed endpoint proves no exit, and gives OpenVMM its grace - // to exit by itself before the start ends it. + // to exit by itself before the start ends it. An endpoint that closes before it read + // the end of the workload's input, which follows its Start, resets the stream. SessionError::ProcessExited | SessionError::Closed => return Err(Exit::EndpointClosed), + SessionError::Io(error) if closes(error) => return Err(Exit::EndpointClosed), SessionError::Io(_) => ErrorCode::Io, _ => ErrorCode::Protocol, }, @@ -683,6 +676,10 @@ pub(crate) fn run( let dir = VmDir::open(&path).map_err(error_with_cause)?; let timeout = options.timeout; let mut cleanup = true; + // A diagnostic or an error of the run's own starts a line of its own after standard error + // that the workload's output left open. + let open_line = Cell::new(None); + let end_line = || output::end_stderr_line(open_line.take().as_slice()); let result = (|| { let mut prepared = vm::create(&runtime, &vm_config, None, &dir) .map_err(error_with_cause)? @@ -735,6 +732,7 @@ pub(crate) fn run( .expect("a ready VM has a session") .exec(request, timeout) }); + open_line.set(executed.as_ref().ok().and_then(|result| result.stderr_end)); // A VM whose control endpoint closed is shutting down already. let requested = match session.take() { Some(mut session) @@ -769,12 +767,14 @@ pub(crate) fn run( if let Some(report) = &options.outcome_report && let Err(error) = copy_openvmm_outcome(&launched.outputs.outcome, report) { + end_line(); error.report(); } // Ctrl-C during the request ends the run as interrupted, which is no failed shutdown. if let Some(Err(error)) = &requested && !matches!(error, Exit::Interrupted) { + end_line(); output::print_stderr(&format!( "warning: managed guest shutdown failed: {error:?}" )); @@ -785,16 +785,21 @@ pub(crate) fn run( return Err(exited_during_startup(report)); } } else if executed.is_ok() && stopped.forced { + end_line(); output::print_stderr("warning: OpenVMM was forcibly terminated after the run"); } executed.map(Some) })(); if cleanup && let Err(error) = remove_vm_directory(&dir) { + end_line(); output::print_stderr(&format!( "warning: cannot remove the temporary VM directory {}: {error:?}", dir.path().display() )); } + if result.is_err() { + end_line(); + } result } diff --git a/nvx-rs/src/host/cli/sandbox/lifecycle/start/tests/restore.rs b/nvx-rs/src/host/cli/sandbox/lifecycle/start/tests/restore.rs index e6f939913..f974b9867 100644 --- a/nvx-rs/src/host/cli/sandbox/lifecycle/start/tests/restore.rs +++ b/nvx-rs/src/host/cli/sandbox/lifecycle/start/tests/restore.rs @@ -31,13 +31,23 @@ use crate::host::sandbox::{DEFAULT_HOSTNAME, DEFAULT_WORKLOAD_IDENTITY}; use crate::host::snapshot::tests::RestoreFixture; use crate::host::snapshot::{DESCRIPTION_FILE, KEY_FILE, OPENVMM_DIR}; use crate::protocol::root::RootImage; +use crate::protocol::v2::{self, Frame, Outcome}; use crate::protocol::{ - APP_ACTIVATE, APP_EXEC, APP_EXIT, APP_HEADER_LEN, APP_MAPS, APP_READY, APP_STDOUT, OUTER_DATA, - OUTER_HEADER_LEN, OUTER_HOST_ATTACH, OUTER_MAX_PAYLOAD, OUTER_READY, decode_app, - decode_outer_header, encode_app, encode_exec_payload, encode_maps_payloads, encode_outer, - status, + OUTER_DATA, OUTER_HEADER_LEN, OUTER_HOST_ATTACH, OUTER_READY, decode_outer_header, + encode_exec_payload, encode_maps_payloads, encode_outer, status, }; +/// The codes of the requests of NVXC that the fake guest of the fixture's restores serves: the +/// activate message, the mapping table and the workload's Start, and the end of its standard +/// input. +const ACTIVATE: u8 = 0x08; +/// See [`ACTIVATE`]. +const MAPS: u8 = 0x06; +/// See [`ACTIVATE`]. +const EXEC: u8 = 0x02; +/// See [`ACTIVATE`]. +const STDIN_END: u8 = 0x0a; + /// The fake VMM and guest that the fixture's restores spawn in place of OpenVMM. const HELPER: &str = "host::cli::sandbox::lifecycle::start::tests::restore::restore_helper"; /// OpenVMM's restore-ready event. @@ -286,7 +296,7 @@ fn named_snapshots_restore_each_image_format_and_upper_with_their_own_policy_and fixture.start("state", config, "ready", 30.0).unwrap(); let requests = fixture.requests("state"); let kinds: Vec = requests.iter().map(|(kind, _)| *kind).collect(); - assert_eq!(kinds, [APP_ACTIVATE, APP_MAPS, APP_EXEC]); + assert_eq!(kinds, [ACTIVATE, MAPS, EXEC, STDIN_END]); let (record, identity) = activation(&requests[0].1); assert_eq!(identity, (1000, 1001, 0)); let configuration = record.configuration(); @@ -306,7 +316,7 @@ fn named_snapshots_restore_each_image_format_and_upper_with_their_own_policy_and assert_eq!(configuration.resources.cpus, 2); assert_eq!( requests[1].1, - encode_maps_payloads(&maps, OUTER_MAX_PAYLOAD - APP_HEADER_LEN).unwrap()[0] + encode_maps_payloads(&maps, v2::MAX_PAYLOAD).unwrap()[0] ); let workload = protocol::Workload { argv: WORKLOAD.map(str::to_owned).to_vec(), @@ -390,7 +400,7 @@ fn a_one_off_restore_installs_the_network_options_of_run_as_its_own_policy() { // The guest learns the DNS server that the policy lets it reach, and announces its listeners, // which the policy publishes on host loopback. let requests = fixture.requests("state"); - assert_eq!(requests[0].0, APP_ACTIVATE); + assert_eq!(requests[0].0, ACTIVATE); assert_eq!( activation(&requests[0].1).0.configuration().network, Some(Network { @@ -506,7 +516,7 @@ fn concurrent_restores_of_one_snapshot_wait_for_its_lock_until_the_first_vm_has_ assert!(spawned[1].2); assert!(fixture.exited(0)); assert!(fixture.requests("first").is_empty()); - assert_eq!(fixture.requests("second").len(), 2); + assert_eq!(fixture.requests("second").len(), 3); assert!(fixture.snapshot_free()); } @@ -655,14 +665,14 @@ fn a_later_start_restores_again_with_an_upper_in_ram_but_never_with_a_scratch_im if ram { again.unwrap(); assert_eq!(fixture.spawned().len(), 2); - assert_eq!(fixture.requests("state").len(), 4); + assert_eq!(fixture.requests("state").len(), 6); } else { // The guest of the first start may have written the image, which a restore's guest // refuses: nothing launches, and the view stays for deprovision. let error = again.unwrap_err(); assert_eq!(error.code(), Some(ErrorCode::Unsupported), "{error:?}"); assert_eq!(fixture.spawned().len(), 1); - assert_eq!(fixture.requests("state").len(), 2); + assert_eq!(fixture.requests("state").len(), 3); } assert_eq!(fixture.view("state"), 3, "{ram}"); assert!(fixture.snapshot_free()); @@ -957,7 +967,7 @@ fn read_record(stream: &mut impl Read) -> Option<(u8, Vec)> { } /// Serves one session of the host as the guest of the recipe's behavior does, and traces its -/// requests. +/// requests, and the end of the standard input of the workload that the Start runs. fn serve(stream: &mut (impl Read + Write), capability: &[u8], epoch: u64, recipe: &Value) { let Some((kind, payload)) = read_record(stream) else { return; @@ -968,26 +978,39 @@ fn serve(stream: &mut (impl Read + Write), capability: &[u8], epoch: u64, recipe return; } let trace = recipe["trace"].as_str().unwrap(); - let mut sequence = 1; - while let Some((kind, payload)) = read_record(stream) { - assert_eq!(kind, OUTER_DATA); - let request = decode_app(&payload).unwrap(); - let line = json!({"kind": request.kind, "payload": hex(request.payload)}); + // Traces the frame of a DATA record, and returns its kind, its request ID and its payload. + let request = |frame: &[u8]| { + let (request_id, _) = Frame::decode(frame).unwrap(); + let (kind, payload) = (frame[5], &frame[24..]); + let line = json!({"kind": kind, "payload": hex(payload)}); let mut file = OpenOptions::new() .create(true) .append(true) .open(trace) .unwrap(); writeln!(file, "{line}").unwrap(); + (kind, request_id, payload.to_vec()) + }; + let mut sequence = 1; + while let Some((kind, frame)) = read_record(stream) { + assert_eq!(kind, OUTER_DATA); + let (kind, request_id, payload) = request(&frame); // A guest that hangs up does once it has the workload's Start, which it never answers. - if request.kind == APP_EXEC && recipe["behavior"] == "hangup" { + if kind == EXEC && recipe["behavior"] == "hangup" { return; } - let replies = match request.kind { - APP_ACTIVATE if recipe["behavior"] == "malformed" => vec![(APP_READY, 0, vec![0])], + let ready = |activation: &[u8]| { + let frame = Frame::Ready { + version: v2::VERSION, + activation, + }; + frame.encode(request_id).unwrap() + }; + let replies = match kind { + ACTIVATE if recipe["behavior"] == "malformed" => vec![ready(&[0])], // The guest cannot assemble its root, and powers off with the status of the failure's // class, which OpenVMM reports, and then exits with, or reports unreadably. - APP_ACTIVATE + ACTIVATE if matches!( recipe["behavior"].as_str(), Some("unusable-root" | "unreadable-report") @@ -1006,24 +1029,29 @@ fn serve(stream: &mut (impl Read + Write), capability: &[u8], epoch: u64, recipe fs::write(recipe["outcome"].as_str().unwrap(), report).unwrap(); std::process::exit(code.into()); } - APP_ACTIVATE => { - let ready = ReadyActivation::of(&activation(request.payload).0); - vec![(APP_READY, 0, ready.encode())] + ACTIVATE => vec![ready( + &ReadyActivation::of(&activation(&payload).0).encode(), + )], + MAPS => vec![ready(&[])], + // The workload reads its standard input until its end, which follows the Start, + // writes its output, then exits, with status 3 if it fails, or SIGTERM ends it. + _ => { + let (_, end) = read_record(stream).unwrap(); + assert_eq!(request(&end), (STDIN_END, request_id, Vec::new())); + let (status, outcome) = match recipe["behavior"].as_str() { + Some("failing") => (3, Outcome::Exit), + Some("signaled") => (143, Outcome::Signal), + _ => (0, Outcome::Exit), + }; + [ + Frame::Stdout(b"restored\n"), + Frame::Exit { status, outcome }, + ] + .map(|frame| frame.encode(request_id).unwrap()) + .to_vec() } - APP_MAPS => vec![(APP_READY, 0, Vec::new())], - // The workload writes its output, then exits, with status 3 if it fails, or SIGTERM - // ends it. - _ => vec![ - (APP_STDOUT, 0, b"restored\n".to_vec()), - match recipe["behavior"].as_str() { - Some("failing") => (APP_EXIT, 3, b"exit".to_vec()), - Some("signaled") => (APP_EXIT, 143, b"signal".to_vec()), - _ => (APP_EXIT, 0, b"exit".to_vec()), - }, - ], }; - for (kind, status, payload) in replies { - let reply = encode_app(kind, request.request_id, status, &payload); + for reply in replies { let record = encode_outer(OUTER_DATA, &[7; 16], epoch, sequence, &reply).unwrap(); if stream.write_all(&record).is_err() { return; diff --git a/nvx-rs/src/host/cli/sandbox/mod.rs b/nvx-rs/src/host/cli/sandbox/mod.rs index c67b49bc5..52dffcc65 100644 --- a/nvx-rs/src/host/cli/sandbox/mod.rs +++ b/nvx-rs/src/host/cli/sandbox/mod.rs @@ -5,8 +5,8 @@ //! - `run`, the default, runs one workload in a sandbox of its own, without a state directory: it //! launches the VM that a `provision` and a `start` of the same options would launch, in a //! temporary VM directory, prints its OpenVMM command as `>> COMMAND`, executes the workload -//! once over NVXC, as `exec` would, with `exec`'s options, stops the VM, and exits with the -//! workload's status, after its output. A dry run prints the command and launches nothing. See +//! once over NVXC, as `exec` would, with `exec`'s options and its streams, stops the VM, and +//! exits with the workload's status. A dry run prints the command and launches nothing. See //! [`lifecycle::run`]. //! - `provision`, `start`, `exec`, `stop`, and `deprovision` manage a persistent sandbox in its //! `--state-dir`; see [`lifecycle`]. @@ -380,15 +380,12 @@ fn run_from(root: Option<&str>, options: Options, source: Option<&Source>) -> Re options.timeout, options.wait, )?; - output::write_stdout(&result.stdout) - .and_then(|()| output::write_stderr(&result.stderr)) - .map_err(Exit::from_os_error)?; if let Some(report) = &options.outcome_report { // An error of the report follows the workload's output, and starts a line of its // own even after standard error that the workload left without a line end, so // that callers find it as they find any other. lifecycle::write_exec_outcome(report, &result) - .inspect_err(|_| output::end_stderr_line(&result.stderr))?; + .inspect_err(|_| output::end_stderr_line(result.stderr_end.as_slice()))?; } Err(Exit::Status(result.returncode)) } @@ -420,9 +417,6 @@ fn run_from(root: Option<&str>, options: Options, source: Option<&Source>) -> Re else { return Ok(()); }; - output::write_stdout(&result.stdout) - .and_then(|()| output::write_stderr(&result.stderr)) - .map_err(Exit::from_os_error)?; Err(Exit::Status(result.returncode)) } (SandboxOperation::Run | SandboxOperation::Provision, None) => { diff --git a/nvx-rs/src/host/cli/session.rs b/nvx-rs/src/host/cli/session.rs index 31cd61672..da3e7395a 100644 --- a/nvx-rs/src/host/cli/session.rs +++ b/nvx-rs/src/host/cli/session.rs @@ -1,6 +1,6 @@ //! The control session of `nvx-rs sandbox`, a port of the former -//! `scripts/nvx_tools/control_session.py` on the framing, sequencing, attachment, and transports -//! of [`crate::host::session`]. +//! `scripts/nvx_tools/control_session.py` on the requests, framing, sequencing, attachment, and +//! transports of [`crate::host::session`], which speaks version 2 of NVXC. //! //! It keeps the observable behavior of the Python session: //! @@ -14,9 +14,9 @@ //! - An endpoint that closes, as OpenVMM closes it when it shuts down, ends the operation with //! [`Exit::EndpointClosed`], while a session that OpenVMM resets fails with `managed control //! session was reset`. -//! - `exec` refuses invalid requests before it sends anything, with Python's messages, buffers -//! the workload's output, and accepts the exit categories of `MANAGED_EXIT_CATEGORIES`; a -//! refusal keeps its status and its category, which the guest sends in ASCII. +//! - `exec` refuses invalid requests before it sends anything, with Python's messages, and +//! accepts the exit categories of `MANAGED_EXIT_CATEGORIES`; a refusal keeps its status and its +//! category. //! - A session whose transfer failed is closed, and refuses further requests. //! //! Unlike the Python session, it also hands the guest agent a host mapping table, as `start` @@ -24,37 +24,41 @@ //! [`Session::reattach`] verifies a saved VM and observes its transport before authentication; //! [`Session::connect_launched_cancellable`], with which a cold run waits for the endpoint of the //! OpenVMM that it launched, also stops waiting once that OpenVMM exits, where Python waited until -//! its timeout. +//! its timeout. `exec` streams, where Python buffered the workload's output: the workload reads +//! the command's standard input, until its end, and its output reaches the command's standard +//! output and error as it comes, without a total cap; and Ctrl-C cancels it, which would +//! otherwise outlive the session. //! //! The transports also wait in slices of at most 250 ms, as Python's do, so that Ctrl-C ends a -//! wait within that time, except while a request is being sent, which its timeout bounds. +//! wait within that time, except while a request is being sent, which its timeout bounds, and +//! while a workload streams, whose loop checks for Ctrl-C itself. -use std::io; +use std::io::{self, Read}; use std::sync::Arc; use std::sync::atomic::{AtomicBool, AtomicU8, Ordering}; -use std::time::Duration; +use std::sync::mpsc; +use std::thread; +use std::time::{Duration, Instant}; use super::deadline::{Deadline, RETRY_INTERVAL, sleep}; -use super::{Exit, Result, check_interrupt}; +use super::{Exit, Result, check_interrupt, output}; use crate::host::platform::{self, Transport}; use crate::host::process::Process; use crate::host::runtime::Runtime; use crate::host::sandbox::options::validate_exec_environment; -use crate::host::session::{ControlSession, SessionError}; +use crate::host::session::{ControlSession, ExecEvent, SessionError}; use crate::host::transport; use crate::host::vm::{self, SavedVm}; +use crate::protocol::v2::{self, Outcome}; use crate::protocol::{ - APP_ERROR, APP_EXEC, APP_EXIT, APP_FEATURES, APP_HEADER_LEN, APP_MAPS, APP_PING, APP_READY, - APP_STDERR, APP_STDOUT, APP_STOP, APP_STOPPED, CAPABILITY_LEN, GuestFeatures, HostMap, - MAPPING_FAILED, MAX_OUTPUT_BYTES, OUTER_MAX_PAYLOAD, UNSUPPORTED_OPERATION, Workload, - WorkloadEnvironment, encode_exec_payload, encode_maps_payloads, + CAPABILITY_LEN, HostMap, Workload, WorkloadEnvironment, encode_exec_payload, }; use crate::{ErrorCode, ErrorKind}; -/// The exit categories that `exec` accepts, `MANAGED_EXIT_CATEGORIES`. A cancellation, which this -/// session never requests, is not among them. -pub(crate) const EXIT_CATEGORIES: [&str; 5] = - ["exit", "timeout", "output-limit", "signal", "failed"]; +/// The exit categories that `exec` accepts, `MANAGED_EXIT_CATEGORIES`, but the output limit, +/// which version 2 of NVXC does not have. A cancellation, whose end this session never waits +/// for, is not among them. +pub(crate) const EXIT_CATEGORIES: [&str; 4] = ["exit", "timeout", "signal", "failed"]; /// The most arguments of a workload, `APP_MAX_ARGUMENTS`. const MAX_ARGUMENTS: usize = 64; /// The longest argument and working directory, in UTF-8 bytes. @@ -157,6 +161,9 @@ struct Probe { closed: AtomicBool, /// Whether Ctrl-C ended a wait. interrupted: AtomicBool, + /// Whether a workload streams, whose loop, rather than a wait of the transport, ends at + /// Ctrl-C, so that it can cancel the workload first. + streaming: AtomicBool, /// The foreground owner's cancellation, absent for ordinary sessions. cancellation: Option, } @@ -171,7 +178,7 @@ struct ProbedTransport { /// Returns whether `error` tells that the endpoint closed the stream, as Python's /// `_PEER_CLOSED` and `_PIPE_CLOSED_ERRORS` do. -fn closes(error: &io::Error) -> bool { +pub(super) fn closes(error: &io::Error) -> bool { matches!( error.kind(), io::ErrorKind::BrokenPipe @@ -195,7 +202,7 @@ impl Transport for ProbedTransport { self.probe.direction.store(RECEIVING, Ordering::SeqCst); let deadline = timeout.map(|timeout| std::time::Instant::now() + timeout); loop { - if platform::interrupted() { + if platform::interrupted() && !self.probe.streaming.load(Ordering::SeqCst) { return Err(self.interrupted()); } if let Some(cancellation) = &self.probe.cancellation { @@ -264,17 +271,16 @@ pub(crate) struct ExecRequest { pub(crate) inherit_default_environment: bool, } -/// How a workload ended: `ManagedExecResult`. +/// How a workload ended: `ManagedExecResult`, whose output already went to the standard streams. #[derive(Debug, Clone, PartialEq, Eq)] pub(crate) struct ExecResult { /// The workload's status. pub(crate) returncode: i32, /// One of [`EXIT_CATEGORIES`]. pub(crate) category: &'static str, - /// The workload's standard output. - pub(crate) stdout: Vec, - /// The workload's standard error. - pub(crate) stderr: Vec, + /// The last byte that the workload wrote to standard error, if any, which tells whether it + /// left a line open there. + pub(crate) stderr_end: Option, } /// An authenticated control session: `ControlSession` of the former `control_session.py`. @@ -328,9 +334,7 @@ impl Session { } })?; Ok(Self { - inner: Some(ControlSession { - connection: attached.connection, - }), + inner: Some(attached.connection.into()), probe, }) } @@ -451,54 +455,23 @@ impl Session { .ok_or_else(|| Exit::error(ErrorCode::Protocol, "managed control session is closed")) } - /// Sends a request and closes the session if the transfer fails. - fn send(&mut self, kind: u8, request_id: u64, payload: &[u8], deadline: Deadline) -> Result { + /// Runs `operation` on the open session, and closes the session if a transfer fails. + fn call( + &mut self, + operation: impl FnOnce(&mut ControlSession) -> std::result::Result, + ) -> Result { self.probe.direction.store(SENDING, Ordering::SeqCst); - let sent = self - .session()? - .send_request(kind, request_id, payload, deadline.instant()); - sent.map_err(|error| self.fail(error)) - } - - /// Reads the next response and closes the session if the transfer fails. - fn receive(&mut self, deadline: Deadline) -> Result<(u8, u64, i32, Vec)> { - self.probe.direction.store(RECEIVING, Ordering::SeqCst); - let received = self.session()?.read_response(Some(deadline.instant())); - received.map_err(|error| self.fail(error)) - } - - /// Reads the next response, which must answer `request_id`, and returns its kind, status, - /// and payload. - fn reply(&mut self, request_id: u64, deadline: Deadline) -> Result<(u8, i32, Vec)> { - let (kind, response_id, status, payload) = self.receive(deadline)?; - if response_id != request_id { - return Err(Exit::error( - ErrorCode::Protocol, - "managed guest returned a mismatched request ID", - )); - } - Ok((kind, status, payload)) - } - - /// Sends a request of `kind` without a payload and waits up to `timeout` seconds until the - /// guest agent acknowledges `what` with an empty `reply` of status 0. - fn acknowledged(&mut self, kind: u8, reply: u8, what: &str, timeout: f64) -> Result { - let request_id = request_id()?; - let deadline = Deadline::after(timeout); - self.send(kind, request_id, &[], deadline)?; - let (kind, response_id, status, payload) = self.receive(deadline)?; - if kind != reply || response_id != request_id || status != 0 || !payload.is_empty() { - return Err(Exit::error( - ErrorCode::Protocol, - format!("managed guest did not acknowledge {what}"), - )); - } - Ok(()) + let result = operation(self.session()?); + result.map_err(|error| self.fail(error)) } - /// Closes the session after a failed transfer and returns the failure. + /// Closes the session after a failed transfer, and returns the failure. A refusal of the guest + /// agent, or an answer that the protocol does not allow, fails no transfer: the session stays, + /// and its connection ends at a record that the protocol does not allow. fn fail(&mut self, error: SessionError) -> Exit { - self.inner = None; + if !matches!(error, SessionError::Protocol(_)) { + self.inner = None; + } self.failure(error) } @@ -541,27 +514,8 @@ impl Session { /// Waits until the guest agent acknowledges readiness, as `ControlSession.ping` does. pub(crate) fn ping(&mut self, timeout: f64) -> Result { - self.acknowledged(APP_PING, APP_READY, "readiness", timeout) - } - - /// Asks the guest agent which control features its image provides. An agent that predates - /// the request refuses it as an unsupported operation, so its image provides none. - pub(crate) fn features(&mut self, timeout: f64) -> Result { - let request_id = request_id()?; - let deadline = Deadline::after(timeout); - self.send(APP_FEATURES, request_id, &[], deadline)?; - let (kind, status, payload) = self.reply(request_id, deadline)?; - let invalid = || { - Exit::error( - ErrorCode::Protocol, - "managed guest returned an invalid features response", - ) - }; - match kind { - APP_READY if status == 0 => GuestFeatures::decode(&payload).map_err(|_| invalid()), - APP_ERROR if payload == UNSUPPORTED_OPERATION => Ok(GuestFeatures::NONE), - _ => Err(invalid()), - } + let deadline = Deadline::after(timeout).instant(); + self.call(|session| session.ping(deadline)) } /// Hands the guest agent the host mapping table `maps`, in order and in as many `MAPS` @@ -569,41 +523,8 @@ impl Session { /// `timeout` seconds. The agent refuses workloads until then, and refuses the rest of the /// table after an entry that it could not mount. pub(crate) fn map_host_paths(&mut self, maps: &[HostMap], timeout: f64) -> Result { - let payloads = encode_maps_payloads(maps, OUTER_MAX_PAYLOAD - APP_HEADER_LEN) - .map_err(|error| Exit::error(ErrorCode::Protocol, error.message()))?; - let deadline = Deadline::after(timeout); - for payload in payloads { - let request_id = request_id()?; - self.send(APP_MAPS, request_id, &payload, deadline)?; - let (kind, status, payload) = self.reply(request_id, deadline)?; - match kind { - APP_READY if status == 0 && payload.is_empty() => {} - APP_ERROR if payload == MAPPING_FAILED.as_bytes() => { - return Err(Exit::error( - ErrorCode::Protocol, - format!( - "managed guest could not mount a mapped host path (error {status})" - ), - )); - } - APP_ERROR => { - return Err(Exit::error( - ErrorCode::Protocol, - format!( - "managed guest did not mount the host paths: {} (status {status})", - String::from_utf8_lossy(&payload) - ), - )); - } - _ => { - return Err(Exit::error( - ErrorCode::Protocol, - "managed guest returned an invalid host mapping response", - )); - } - } - } - Ok(()) + let deadline = Deadline::after(timeout).instant(); + self.call(|session| session.map_host_paths(maps, deadline)) } /// Checks the workload of `request` and returns the payload of its exec request, as @@ -658,11 +579,48 @@ impl Session { } /// Runs a workload and returns how it ended, as `ControlSession.exec` does: the request and - /// every response must arrive within `response_timeout` seconds of each other's start. + /// every response must arrive within `response_timeout` seconds of its start. The workload + /// reads this process's standard input until its end, and its standard output and error + /// reach this process's as they come. Ctrl-C cancels it. An error that follows its output + /// starts a line of its own, as callers find errors. pub(crate) fn exec( &mut self, request: &ExecRequest, response_timeout: f64, + ) -> Result { + let mut end = None; + let write = |stderr, data: &[u8]| { + if stderr { + end = data.last().copied(); + output::write_stderr(data) + } else { + output::write_stdout(data) + } + }; + let result = self.exec_with( + request, + response_timeout, + read_stdin, + platform::interrupted, + write, + ); + if result.is_err() { + output::end_stderr_line(end.as_slice()); + } + result + } + + /// Runs a workload as [`exec`](Self::exec) does, with the chunks that `input` returns the + /// receiver of, until it disconnects, as its standard input, or a read of it fails, `write` + /// for its standard output, or its standard error if its first argument holds, and + /// `interrupted` for Ctrl-C. + fn exec_with( + &mut self, + request: &ExecRequest, + response_timeout: f64, + input: impl FnOnce() -> mpsc::Receiver>>, + interrupted: impl Fn() -> bool, + mut write: impl FnMut(bool, &[u8]) -> io::Result<()>, ) -> Result { if !(response_timeout > 0.0 && response_timeout < f64::INFINITY) { return Err(Exit::error( @@ -673,57 +631,72 @@ impl Session { let payload = Self::exec_payload(request)?; let request_id = request_id()?; let deadline = Deadline::after(response_timeout); - self.send(APP_EXEC, request_id, &payload, deadline)?; - let mut stdout = Vec::new(); - let mut stderr = Vec::new(); + self.call(|session| session.start_exec(request_id, &payload, deadline.instant()))?; + self.probe.streaming.store(true, Ordering::SeqCst); + let result = self.stream(request_id, deadline, &input(), &interrupted, &mut write); + self.probe.streaming.store(false, Ordering::SeqCst); + result + } + + /// Streams the execution of `request_id` until it ends or `deadline` passes, as + /// [`exec_with`](Self::exec_with) does, with no more than the window of its standard input + /// unacknowledged, which [`ControlSession::send_stdin`] keeps. + fn stream( + &mut self, + request_id: u64, + deadline: Deadline, + input: &mpsc::Receiver>>, + interrupted: &impl Fn() -> bool, + write: &mut impl FnMut(bool, &[u8]) -> io::Result<()>, + ) -> Result { + let (mut pending, mut open, mut stderr_end) = (Vec::new(), true, None); loop { - let (kind, status, response) = self.reply(request_id, deadline)?; - let violation = |message: &str| Exit::error(ErrorCode::Protocol, message); - match kind { - APP_STDOUT | APP_STDERR => { - // The guest agent forwards at most this much output, so more breaks the - // protocol. - if stdout.len() + stderr.len() + response.len() > MAX_OUTPUT_BYTES { - return Err(violation(&format!( - "managed guest exceeded the {MAX_OUTPUT_BYTES}-byte output limit" - ))); + if interrupted() { + self.cancel(request_id); + return Err(Exit::Interrupted); + } + if open && pending.is_empty() { + match input.try_recv() { + Ok(Ok(chunk)) => pending = chunk, + // An end of the input would hand the workload what it read as all of it. + Ok(Err(error)) => { + self.cancel(request_id); + return Err(Exit::error( + ErrorCode::Io, + format!("cannot read standard input: {error}"), + )); } - let output = if kind == APP_STDOUT { - &mut stdout - } else { - &mut stderr - }; - output.extend_from_slice(&response); - } - APP_EXIT => { - if !response.is_ascii() { - return Err(violation("managed guest returned an invalid exit category")); + Err(mpsc::TryRecvError::Empty) => {} + Err(mpsc::TryRecvError::Disconnected) => { + open = false; + self.call(|session| session.close_stdin(request_id, deadline.instant()))?; } - let category = EXIT_CATEGORIES - .iter() - .find(|category| category.as_bytes() == response) - .ok_or_else(|| { - violation("managed guest returned an unsupported exit category") - })?; + } + } + if !pending.is_empty() { + let sent = self + .call(|session| session.send_stdin(request_id, &pending, deadline.instant()))?; + pending.drain(..sent); + } + self.probe.direction.store(RECEIVING, Ordering::SeqCst); + let slice = deadline.instant().min(Instant::now() + RETRY_INTERVAL); + let written = match self.session()?.next_exec_event(request_id, Some(slice)) { + Err(SessionError::TimedOut) if !deadline.passed() => Ok(()), + Err(error) => return Err(self.fail(error)), + Ok(ExecEvent::Stdout(data)) => write(false, &data), + Ok(ExecEvent::Stderr(data)) => { + stderr_end = data.last().copied(); + write(true, &data) + } + Ok(ExecEvent::Acknowledged(_)) => Ok(()), + Ok(ExecEvent::Exit { category, status }) => { return Ok(ExecResult { returncode: status, - category, - stdout, - stderr, + category: exit_category(category)?, + stderr_end, }); } - APP_ERROR => { - // `bytes.decode("ascii", "replace")`. - let category: String = response - .iter() - .map(|&byte| { - if byte.is_ascii() { - char::from(byte) - } else { - '\u{fffd}' - } - }) - .collect(); + Ok(ExecEvent::Rejected { status, category }) => { return Err(Exit::error( ErrorCode::ExecRefused, format!( @@ -731,23 +704,81 @@ impl Session { ), )); } - _ => { - return Err(violation("managed guest returned an invalid exec response")); - } + }; + // Ctrl-C ends a write, after which the next turn cancels the workload. + if let Err(error) = written + && !interrupted() + { + return Err(Exit::from_os_error(error)); } } } + /// Asks the guest agent, within one wait slice, to end the execution of `request_id`, which + /// would otherwise outlive the session, which ends with the command. + fn cancel(&mut self, request_id: u64) { + if let Some(session) = &mut self.inner { + let _ = session.cancel_exec(request_id, Instant::now() + POLL_SLICE); + } + } + /// Asks the guest agent to shut the VM down, as `ControlSession.stop` does. pub(crate) fn stop(&mut self, timeout: f64) -> Result { - self.acknowledged(APP_STOP, APP_STOPPED, "stop", timeout) + let deadline = Deadline::after(timeout).instant(); + self.call(|session| session.stop(deadline)) } } +/// Returns the category of [`EXIT_CATEGORIES`] of a workload that ended with `outcome`, or fails +/// for a cancellation, whose end no execution of this session reports. +fn exit_category(outcome: Outcome) -> Result<&'static str> { + Ok(match outcome { + Outcome::Exit => "exit", + Outcome::Timeout => "timeout", + Outcome::Signal => "signal", + Outcome::Failed => "failed", + Outcome::Cancelled => { + return Err(Exit::error( + ErrorCode::Protocol, + "managed guest returned an unsupported exit category", + )); + } + }) +} + +/// Returns the receiver of the chunks of this process's standard input, which a thread of its own +/// reads until the input ends, or a read fails, whose error ends them, or no one receives them +/// any longer. +fn read_stdin() -> mpsc::Receiver>> { + let (sender, receiver) = mpsc::sync_channel(1); + thread::spawn(move || { + let mut input = io::stdin().lock(); + loop { + let mut chunk = vec![0; v2::MAX_PAYLOAD]; + match input.read(&mut chunk) { + Err(error) if error.kind() == io::ErrorKind::Interrupted => {} + Ok(0) => return, + Ok(count) => { + chunk.truncate(count); + if sender.send(Ok(chunk)).is_err() { + return; + } + } + Err(error) => { + let _ = sender.send(Err(error)); + return; + } + } + } + }); + receiver +} + /// Returns Python's message for a violation of the protocol that `nvx::host::session` reports. /// /// The two sessions report the same violations of records that have one fault, and their -/// messages differ only where Python folds a case into a broader one. +/// messages differ only where Python folds a case into a broader one, and where Python named the +/// guest agent `managed guest`. fn protocol_message(message: String) -> String { match message.as_str() { "control attach response carried a payload" => { @@ -760,7 +791,10 @@ fn protocol_message(message: String) -> String { "control endpoint returned a truncated application frame" => { "control endpoint returned an invalid data record".to_owned() } - _ => message, + _ => match message.strip_prefix("the guest agent ") { + Some(rest) => format!("managed guest {rest}"), + None => message, + }, } } diff --git a/nvx-rs/src/host/cli/session/tests.rs b/nvx-rs/src/host/cli/session/tests.rs index 5699bbc3a..6deb02a4d 100644 --- a/nvx-rs/src/host/cli/session/tests.rs +++ b/nvx-rs/src/host/cli/session/tests.rs @@ -12,18 +12,33 @@ use crate::host::transport::HostConnection; use crate::protocol::exec::{ EXEC_CWD_PRESENT, EXEC_ENVIRONMENT_PRESENT, EXEC_EXTENDED, EXEC_INHERIT_DEFAULT_ENV, }; +use crate::protocol::v2::{Frame, STDIN_WINDOW, VERSION}; use crate::protocol::{ - OUTER_DATA, OUTER_HEADER_LEN, OUTER_HOST_ATTACH, OUTER_READY, OUTER_RESET, decode_app, - decode_outer_header, encode_app, encode_outer, + OUTER_DATA, OUTER_HEADER_LEN, OUTER_HOST_ATTACH, OUTER_READY, OUTER_RESET, decode_outer_header, + encode_outer, }; const CAPABILITY: [u8; CAPABILITY_LEN] = [7; CAPABILITY_LEN]; const INSTANCE: [u8; 16] = [0x11; 16]; +/// The codes of the kinds of requests that version 2 shares with version 1, as its header holds +/// them, and those of an execution's standard input. +const PING: u8 = 0x01; +const EXEC: u8 = 0x02; +const CANCEL: u8 = 0x04; +const STDIN: u8 = 0x09; +const STDIN_END: u8 = 0x0a; + +/// A `READY` of the protocol version, which acknowledges a request. +const READY: Frame<'static> = Frame::Ready { + version: VERSION, + activation: &[], +}; + impl Session { fn from_connection(connection: HostConnection) -> Self { Self { - inner: Some(ControlSession { connection }), + inner: Some(connection.into()), probe: Arc::new(Probe::default()), } } @@ -36,6 +51,46 @@ impl Session { ) -> Result { Self::connect_unless_exited(endpoint, pid, capability, timeout, || false, None) } + + /// Runs `request` as `exec` does, with a standard input that stays open, as a terminal's does, + /// without Ctrl-C, and discards its output. + fn run(&mut self, request: &ExecRequest, response_timeout: f64) -> Result { + let (_input, receiver) = mpsc::channel(); + self.exec_with( + request, + response_timeout, + || receiver, + || false, + |_, _| Ok(()), + ) + } + + /// Runs `request` as `exec` does, with the standard input that `chunks` hold, until its end, + /// and without Ctrl-C, and returns how it ended and its standard output and error. + fn run_with( + &mut self, + request: &ExecRequest, + response_timeout: f64, + chunks: &[&[u8]], + ) -> (Result, Vec, Vec) { + let (input, receiver) = mpsc::channel(); + for chunk in chunks { + input.send(Ok(chunk.to_vec())).unwrap(); + } + drop(input); + let (mut stdout, mut stderr) = (Vec::new(), Vec::new()); + let result = self.exec_with( + request, + response_timeout, + || receiver, + || false, + |error, data| { + (if error { &mut stderr } else { &mut stdout }).extend_from_slice(data); + Ok(()) + }, + ); + (result, stdout, stderr) + } } /// What a [`Recording`] transport records: the direction and timeout of each transfer. @@ -163,14 +218,19 @@ impl Guest { fn request(&mut self) -> (u8, u64, Vec) { let (record_type, frame) = self.record().unwrap(); assert_eq!(record_type, OUTER_DATA); - let frame = decode_app(&frame).unwrap(); - (frame.kind, frame.request_id, frame.payload.to_vec()) + let (request_id, _) = Frame::decode(&frame).unwrap(); + (frame[5], request_id, frame[24..].to_vec()) + } + + /// Sends `frame` with `request_id`. + fn send(&mut self, request_id: u64, frame: Frame<'_>) { + self.send_raw(&frame.encode(request_id).unwrap()); } - fn send(&mut self, kind: u8, request_id: u64, status: i32, payload: &[u8]) { + /// Sends the bytes of a frame, which may break the protocol. + fn send_raw(&mut self, frame: &[u8]) { self.sequence += 1; - let frame = encode_app(kind, request_id, status, payload); - let record = encode_outer(OUTER_DATA, &INSTANCE, 1, self.sequence, &frame).unwrap(); + let record = encode_outer(OUTER_DATA, &INSTANCE, 1, self.sequence, frame).unwrap(); self.stream.write_all(&record, None).unwrap(); } } @@ -197,21 +257,21 @@ fn an_owned_connection_keeps_its_authenticated_framing_without_another_attach() guest.accept(); for expected in [17, 0] { let (kind, request_id, payload) = guest.request(); - assert_eq!(kind, APP_PING); + assert_eq!(kind, PING); assert!(payload.is_empty()); if expected != 0 { assert_eq!(request_id, expected); } - guest.send(APP_READY, request_id, 0, &[]); + guest.send(request_id, READY); } }); let until = Instant::now() + Duration::from_secs(5); let mut connection = HostConnection::attach(Box::new(host), &CAPABILITY, until).unwrap(); connection - .send(&encode_app(APP_PING, 17, 0, &[]), Some(until)) + .send(&Frame::Ping.encode(17).unwrap(), Some(until)) .unwrap(); let reply = connection.receive(Some(until)).unwrap(); - assert_eq!(decode_app(&reply).unwrap().request_id, 17); + assert_eq!(Frame::decode(&reply).unwrap(), (17, READY)); let mut session = Session::from_connection(connection); session.ping(5.0).unwrap(); worker.join().unwrap(); @@ -256,15 +316,35 @@ fn exec_payload(workload: ExecRequest) -> Vec { let (sender, receiver) = mpsc::channel(); let (mut session, worker) = session_with(move |guest| { let (kind, request_id, payload) = guest.request(); - assert_eq!(kind, APP_EXEC); + assert_eq!(kind, EXEC); sender.send(payload).unwrap(); - guest.send(APP_EXIT, request_id, 0, b"exit"); + guest.send(request_id, exit(0, Outcome::Exit)); }); - session.exec(&workload, 5.0).unwrap(); + session.run(&workload, 5.0).unwrap(); worker.join().unwrap(); receiver.recv().unwrap() } +/// Returns the `EXIT` of an execution that ended with `outcome` and `status`. +fn exit(status: i32, outcome: Outcome) -> Frame<'static> { + Frame::Exit { status, outcome } +} + +/// Returns the bytes of an `EXIT` of `request_id` and `status` whose outcome is `text`, which +/// version 2 may not know. +fn exit_text(request_id: u64, status: i32, text: &[u8]) -> Vec { + let length = u32::try_from(text.len()).unwrap(); + [ + b"NVXC".as_slice(), + &[VERSION, 0x84, 0, 0], + &request_id.to_le_bytes(), + &status.to_le_bytes(), + &length.to_le_bytes(), + text, + ] + .concat() +} + fn u16_at(payload: &[u8], offset: usize) -> u16 { u16::from_le_bytes(payload[offset..offset + 2].try_into().unwrap()) } @@ -288,7 +368,7 @@ fn values(payload: &[u8], mut offset: usize, count: usize) -> (Vec>, usi fn exec_streams_output_and_returns_bounded_status() { let (mut session, worker) = session_with(|guest| { let (kind, request_id, payload) = guest.request(); - assert_eq!(kind, APP_EXEC); + assert_eq!(kind, EXEC); // The legacy request: the timeout, the argument count, and no extension. assert_eq!( ( @@ -298,39 +378,145 @@ fn exec_streams_output_and_returns_bounded_status() { ), (5000, 3, 0) ); - guest.send(APP_STDOUT, request_id, 0, b"hello"); - guest.send(APP_STDERR, request_id, 0, b"warning"); - guest.send(APP_EXIT, request_id, 7, b"exit"); + // The command's standard input is empty, so the workload's ends at once. + assert_eq!(guest.request(), (STDIN_END, request_id, Vec::new())); + guest.send(request_id, Frame::Stdout(b"hello")); + guest.send(request_id, Frame::Stderr(b"warning")); + guest.send(request_id, exit(7, Outcome::Exit)); }); let mut workload = request(&["/bin/sh", "-c", "echo hello"]); workload.timeout_ms = 5000; - let result = session.exec(&workload, 5.0).unwrap(); + let (result, stdout, stderr) = session.run_with(&workload, 5.0, &[]); worker.join().unwrap(); assert_eq!( - result, + result.unwrap(), ExecResult { returncode: 7, category: "exit", - stdout: b"hello".to_vec(), - stderr: b"warning".to_vec(), + stderr_end: Some(b'g'), } ); + assert_eq!((stdout, stderr), (b"hello".to_vec(), b"warning".to_vec())); +} + +#[test] +fn exec_streams_output_past_the_old_cap() { + // Version 1 forwarded at most 1 MiB of output; version 2 streams it whole. + let (mut session, worker) = session_with(|guest| { + let (_, request_id, _) = guest.request(); + assert_eq!(guest.request(), (STDIN_END, request_id, Vec::new())); + let chunk = vec![b'x'; 60_000]; + for _ in 0..18 { + guest.send(request_id, Frame::Stdout(&chunk)); + } + guest.send(request_id, exit(0, Outcome::Exit)); + }); + let (result, stdout, stderr) = session.run_with(&request(&["/bin/true"]), 5.0, &[]); + worker.join().unwrap(); + assert_eq!(result.unwrap().returncode, 0); + assert_eq!(stdout, vec![b'x'; 18 * 60_000]); + assert!(stderr.is_empty()); +} + +#[test] +fn exec_streams_its_standard_input_under_the_window_until_its_end() { + let input: Vec = (0..120 * 1024).map(|index| (index % 251) as u8).collect(); + let (mut session, worker) = session_with(|guest| { + let (_, request_id, _) = guest.request(); + // The guest acknowledges nothing until the window is full, or the input ended. + let (mut received, mut outstanding) = (Vec::new(), 0); + loop { + match guest.request() { + (STDIN, id, bytes) if id == request_id => { + outstanding += bytes.len(); + assert!(outstanding <= STDIN_WINDOW as usize, "{outstanding}"); + received.extend(bytes); + } + (STDIN_END, id, bytes) if id == request_id && bytes.is_empty() => break, + request => panic!("unexpected request {request:?}"), + } + if outstanding == STDIN_WINDOW as usize { + let count = u32::try_from(outstanding).unwrap(); + guest.send(request_id, Frame::StdinAck(count)); + outstanding = 0; + } + } + if outstanding > 0 { + let count = u32::try_from(outstanding).unwrap(); + guest.send(request_id, Frame::StdinAck(count)); + } + for chunk in received.chunks(v2::MAX_PAYLOAD) { + guest.send(request_id, Frame::Stdout(chunk)); + } + guest.send(request_id, exit(0, Outcome::Exit)); + }); + let chunks: Vec<&[u8]> = input.chunks(40 * 1024).collect(); + let (result, stdout, _) = session.run_with(&request(&["/bin/cat"]), 5.0, &chunks); + worker.join().unwrap(); + assert_eq!(result.unwrap().returncode, 0); + assert_eq!(stdout, input); +} + +#[test] +fn ctrl_c_cancels_the_running_workload() { + let (mut session, worker) = session_with(|guest| { + let (_, request_id, _) = guest.request(); + guest.send(request_id, Frame::Stdout(b"started")); + // Ctrl-C after the output: the guest ends the workload, which would outlive the session. + assert_eq!(guest.request(), (CANCEL, request_id, Vec::new())); + }); + let started = AtomicBool::new(false); + let (_input, receiver) = mpsc::channel(); + let result = session.exec_with( + &request(&["/bin/sleep", "60"]), + 5.0, + || receiver, + || started.load(Ordering::SeqCst), + |_, _| { + started.store(true, Ordering::SeqCst); + Ok(()) + }, + ); + assert!(matches!(result, Err(Exit::Interrupted)), "{result:?}"); + worker.join().unwrap(); +} + +#[test] +fn a_failed_read_of_the_standard_input_cancels_the_workload() { + let (mut session, worker) = session_with(|guest| { + let (_, request_id, _) = guest.request(); + assert_eq!(guest.request(), (STDIN, request_id, b"partial".to_vec())); + // No end of the input follows, which would let the workload take what it read as all. + assert_eq!(guest.request(), (CANCEL, request_id, Vec::new())); + }); + let (input, receiver) = mpsc::channel(); + input.send(Ok(b"partial".to_vec())).unwrap(); + input.send(Err(io::Error::other("read failed"))).unwrap(); + let result = session.exec_with( + &request(&["/bin/cat"]), + 5.0, + || receiver, + || false, + |_, _| Ok(()), + ); + assert_eq!(message(result), "cannot read standard input: read failed"); + worker.join().unwrap(); } #[test] fn exec_rejects_unbounded_or_relative_arguments() { let (mut session, worker) = session_with(|_| {}); assert_eq!( - message(session.exec(&request(&["relative"]), 1.0)), + message(session.run(&request(&["relative"]), 1.0)), "managed exec entrypoint must be absolute" ); assert_eq!( - message(session.exec(&request(&["/bin/true"; 65]), 1.0)), + message(session.run(&request(&["/bin/true"; 65]), 1.0)), "managed exec requires 1 through 64 arguments" ); for argument in ["", "a\0b", &"a".repeat(4097)] { assert_eq!( - message(session.exec(&request(&["/bin/true", argument]), 1.0)), + message(session.run(&request(&["/bin/true", argument]), 1.0)), "managed exec argument is empty or exceeds 4096 bytes" ); } @@ -455,7 +641,7 @@ fn exec_rejects_invalid_execution_fields_before_sending() { let mut workload = request(&["/bin/true"]); workload.cwd = Some(cwd.to_owned()); assert_eq!( - failure(session.exec(&workload, 1.0)), + failure(session.run(&workload, 1.0)), ( ErrorCode::Unsupported, "managed exec working directory must be an absolute path of at most 4096 bytes" @@ -480,7 +666,7 @@ fn exec_rejects_invalid_execution_fields_before_sending() { .map(|entry| (*entry).to_owned()) .collect(), ); - assert_eq!(message(session.exec(&workload, 1.0)), expected); + assert_eq!(message(session.run(&workload, 1.0)), expected); } // Nothing reached the guest, which reads the end of the stream. drop(session); @@ -493,7 +679,7 @@ fn exec_rejects_invalid_response_timeout_before_sending() { let (mut session, worker, received) = silent_session(); for timeout in [0.0, -1.0, f64::NAN, f64::INFINITY] { assert_eq!( - message(session.exec(&request(&["/bin/true"]), timeout)), + message(session.run(&request(&["/bin/true"]), timeout)), "managed exec response timeout must be positive and finite" ); } @@ -504,59 +690,67 @@ fn exec_rejects_invalid_response_timeout_before_sending() { #[test] fn exec_rejects_unknown_exit_category() { - // A cancellation, which this session never requests, is not among the categories. - for category in [&b"guest-provided-detail"[..], b"cancelled"] { + // A cancellation, whose end this session never waits for, is not among the categories. + let (mut session, worker) = session_with(|guest| { + let (_, request_id, _) = guest.request(); + guest.send(request_id, exit(137, Outcome::Cancelled)); + }); + assert_eq!( + message(session.run(&request(&["/bin/true"]), 5.0)), + "managed guest returned an unsupported exit category" + ); + worker.join().unwrap(); + // Version 2 knows no other outcome. + for category in [&b"guest-provided-detail"[..], b"\xffexit", b"output-limit"] { let (mut session, worker) = session_with(move |guest| { let (_, request_id, _) = guest.request(); - guest.send(APP_EXIT, request_id, 125, category); + guest.send_raw(&exit_text(request_id, 125, category)); }); assert_eq!( - message(session.exec(&request(&["/bin/true"]), 5.0)), - "managed guest returned an unsupported exit category" + message(session.run(&request(&["/bin/true"]), 5.0)), + "the NVXC frame of version 2 is malformed" ); worker.join().unwrap(); } - let (mut session, worker) = session_with(|guest| { - let (_, request_id, _) = guest.request(); - guest.send(APP_EXIT, request_id, 0, b"\xffexit"); - }); - assert_eq!( - message(session.exec(&request(&["/bin/true"]), 5.0)), - "managed guest returned an invalid exit category" - ); - worker.join().unwrap(); } #[test] fn exec_refusal_keeps_status_category_and_diagnostic() { + let diagnostic = + b"nvx-managed-agent: cannot enter working directory /missing: No such file or directory\n"; let (mut session, worker) = session_with(|guest| { let (_, request_id, _) = guest.request(); - guest.send( - APP_STDERR, - request_id, - 0, - b"nvx-managed-agent: cannot enter working directory /missing: No such file or \ - directory\n", - ); - guest.send(APP_ERROR, request_id, 2, b"cwd-failed"); + assert_eq!(guest.request(), (STDIN_END, request_id, Vec::new())); + guest.send(request_id, Frame::Stderr(diagnostic)); + let refusal = Frame::Error { + status: 2, + category: b"cwd-failed", + }; + guest.send(request_id, refusal); }); let mut workload = request(&["/bin/true"]); workload.cwd = Some("/missing".to_owned()); + let (result, stdout, stderr) = session.run_with(&workload, 5.0, &[]); assert_eq!( - failure(session.exec(&workload, 5.0)), + failure(result), ( ErrorCode::ExecRefused, "managed guest rejected exec (status=2, category=cwd-failed)".to_owned() ) ); + assert_eq!((stdout, stderr), (Vec::new(), diagnostic.to_vec())); worker.join().unwrap(); - // A category that is not ASCII is decoded with replacement characters. + // A category that is not UTF-8 is decoded with replacement characters. let (mut session, worker) = session_with(|guest| { let (_, request_id, _) = guest.request(); - guest.send(APP_ERROR, request_id, 5, b"bad\xff"); + let refusal = Frame::Error { + status: 5, + category: b"bad\xff", + }; + guest.send(request_id, refusal); }); assert_eq!( - message(session.exec(&request(&["/bin/true"]), 5.0)), + message(session.run(&request(&["/bin/true"]), 5.0)), "managed guest rejected exec (status=5, category=bad\u{fffd})" ); worker.join().unwrap(); @@ -639,14 +833,15 @@ impl Transport for Recording { let response = if header.record_type == OUTER_HOST_ATTACH { encode_outer(OUTER_READY, &INSTANCE, 1, 0, &[]).unwrap() } else { - let request = decode_app(&data[OUTER_HEADER_LEN..]).unwrap(); - let (kind, payload) = match request.kind { - APP_PING => (APP_READY, &b""[..]), - APP_EXEC => (APP_EXIT, &b"exit"[..]), - _ => (APP_STOPPED, &b""[..]), + let request = &data[OUTER_HEADER_LEN..]; + let (request_id, _) = Frame::decode(request).unwrap(); + let response = match request[5] { + PING => READY, + EXEC => exit(0, Outcome::Exit), + _ => Frame::Stopped, }; self.sequence += 1; - let frame = encode_app(kind, request.request_id, 0, payload); + let frame = response.encode(request_id).unwrap(); encode_outer(OUTER_DATA, &INSTANCE, 1, self.sequence, &frame).unwrap() }; self.responses.extend(response); @@ -664,7 +859,7 @@ fn operations_bound_their_requests_by_their_deadlines() { let mut operations = vec![(7.0, transfers.lock().unwrap().split_off(0))]; session.ping(5.0).unwrap(); operations.push((5.0, transfers.lock().unwrap().split_off(0))); - session.exec(&request(&["/bin/true"]), 3.0).unwrap(); + session.run(&request(&["/bin/true"]), 3.0).unwrap(); operations.push((3.0, transfers.lock().unwrap().split_off(0))); session.stop(2.0).unwrap(); operations.push((2.0, transfers.lock().unwrap().split_off(0))); @@ -702,7 +897,7 @@ fn failed_transfer_closes_the_session() { ); let count = transfers.lock().unwrap().len(); assert_eq!( - self::failure(session.exec(&request(&["/bin/true"]), 5.0)), + self::failure(session.run(&request(&["/bin/true"]), 5.0)), ( ErrorCode::Protocol, "managed control session is closed".to_owned() @@ -731,7 +926,7 @@ fn request_to_a_stalled_endpoint_times_out_and_closes_the_session() { let mut session = Session::attach(Box::new(host), &CAPABILITY, 5.0, None).unwrap(); let started = Instant::now(); assert_eq!( - message(session.exec(&request(&["/bin/true"]), 0.25)), + message(session.run(&request(&["/bin/true"]), 0.25)), "managed control request timed out" ); let elapsed = started.elapsed(); @@ -1041,64 +1236,11 @@ fn a_closed_endpoint_ends_the_operation_with_its_own_error() { assert!(matches!(failure, Exit::EndpointClosed), "{failure:?}"); } -#[test] -fn output_beyond_the_guest_limit_breaks_the_protocol() { - // The guest agent forwards at most 1 MiB of output, so a guest that sends more misbehaves. - let (mut session, worker) = session_with(|guest| { - let (_, request_id, _) = guest.request(); - let chunk = vec![b'x'; 60_000]; - for _ in 0..18 { - guest.send(APP_STDOUT, request_id, 0, &chunk); - } - guest.send(APP_EXIT, request_id, 0, b"exit"); - }); - assert_eq!( - failure(session.exec(&request(&["/bin/true"]), 5.0)), - ( - ErrorCode::Protocol, - format!("managed guest exceeded the {MAX_OUTPUT_BYTES}-byte output limit") - ) - ); - // The guest may still be sending its exit record, which the host never reads, and a send - // fails once the session is gone, so the session ends after the guest. - worker.join().unwrap(); -} - -#[cfg(feature = "testing")] -#[test] -fn features_of_a_guest_that_predates_them_are_none() { - let (mut session, worker) = session_with(|guest| { - let (kind, request_id, _) = guest.request(); - assert_eq!(kind, APP_FEATURES); - guest.send(APP_ERROR, request_id, 95, UNSUPPORTED_OPERATION); - let (_, request_id, _) = guest.request(); - guest.send( - APP_READY, - request_id, - 0, - &GuestFeatures::SANDBOX_AGENT.encode(), - ); - let (_, request_id, _) = guest.request(); - guest.send(APP_ERROR, request_id, 22, b"invalid-request"); - }); - assert_eq!(session.features(5.0).unwrap(), GuestFeatures::NONE); - assert_eq!(session.features(5.0).unwrap(), GuestFeatures::SANDBOX_AGENT); - assert_eq!( - message(session.features(5.0)), - "managed guest returned an invalid features response" - ); - worker.join().unwrap(); -} - -#[cfg(any( - feature = "testing", - all(feature = "guest", target_os = "linux", target_arch = "x86_64") -))] #[test] fn protocol_violations_keep_the_messages_of_python() { let (mut session, worker) = session_with(|guest| { let (_, request_id, _) = guest.request(); - guest.send(APP_READY, request_id ^ 1, 0, &[]); + guest.send(request_id ^ 1, READY); }); assert_eq!( message(session.ping(5.0)), @@ -1107,43 +1249,36 @@ fn protocol_violations_keep_the_messages_of_python() { worker.join().unwrap(); let (mut session, worker) = session_with(|guest| { let (_, request_id, _) = guest.request(); - guest.send(APP_STDOUT, request_id ^ 1, 0, b"x"); + guest.send(request_id ^ 1, Frame::Stdout(b"x")); }); assert_eq!( - message(session.exec(&request(&["/bin/true"]), 5.0)), + message(session.run(&request(&["/bin/true"]), 5.0)), "managed guest returned a mismatched request ID" ); worker.join().unwrap(); let (mut session, worker) = session_with(|guest| { let (_, request_id, _) = guest.request(); - guest.send(APP_READY, request_id ^ 1, 0, &GuestFeatures::NONE.encode()); + guest.send(request_id, READY); }); assert_eq!( - message(session.features(5.0)), - "managed guest returned a mismatched request ID" + message(session.stop(5.0)), + "managed guest did not acknowledge stop" ); worker.join().unwrap(); + // An acknowledgement has no payload but the protocol version. let (mut session, worker) = session_with(|guest| { let (_, request_id, _) = guest.request(); - guest.send(APP_READY, request_id, 0, b""); + let ready = Frame::Ready { + version: VERSION, + activation: b"x", + }; + guest.send(request_id, ready); }); assert_eq!( - message(session.stop(5.0)), - "managed guest did not acknowledge stop" + message(session.ping(5.0)), + "managed guest did not acknowledge readiness" ); worker.join().unwrap(); - // An acknowledgement has status 0 and no payload. - for (status, payload) in [(1, &b""[..]), (0, b"x")] { - let (mut session, worker) = session_with(move |guest| { - let (_, request_id, _) = guest.request(); - guest.send(APP_READY, request_id, status, payload); - }); - assert_eq!( - message(session.ping(5.0)), - "managed guest did not acknowledge readiness" - ); - worker.join().unwrap(); - } } /// The endpoints of POSIX hosts: Unix-domain sockets that this process serves. @@ -1207,7 +1342,7 @@ mod endpoints { workload .arguments .extend(std::iter::repeat_n("a".repeat(4000), 10)); - let failure = session.exec(&workload, 5.0).unwrap_err(); + let failure = session.run(&workload, 5.0).unwrap_err(); worker.join().unwrap(); assert!(matches!(failure, Exit::EndpointClosed), "{failure:?}"); let (mut session, worker, _directory) = connected(|_| {}); @@ -1490,7 +1625,7 @@ mod endpoints { fn read_reports_a_closed_endpoint_after_its_data() { let (mut session, worker) = connected(|guest| { let (_, request_id, _) = guest.request(); - guest.send(APP_READY, request_id, 0, &[]); + guest.send(request_id, READY); let _ = guest.request(); }); session.ping(5.0).unwrap(); diff --git a/nvx-rs/src/host/cli/snapshots/capture/tests.rs b/nvx-rs/src/host/cli/snapshots/capture/tests.rs index 94c82360f..067f0673b 100644 --- a/nvx-rs/src/host/cli/snapshots/capture/tests.rs +++ b/nvx-rs/src/host/cli/snapshots/capture/tests.rs @@ -16,9 +16,10 @@ use crate::host::process::{BootId, Opened, ProcessRef}; use crate::host::snapshot::tests::{ManifestSpec, SnapshotFixture, context}; use crate::host::snapshot::{MANIFEST_NAME, Snapshot}; use crate::host::spawn::{self, VmmCommand}; +use crate::protocol::v2::{self, Frame}; use crate::protocol::{ - APP_CAPTURE, APP_ERROR, APP_READY, CAPABILITY_LEN, OUTER_DATA, OUTER_HEADER_LEN, - OUTER_HOST_ATTACH, OUTER_READY, decode_app, decode_outer_header, encode_app, encode_outer, + CAPABILITY_LEN, OUTER_DATA, OUTER_HEADER_LEN, OUTER_HOST_ATTACH, OUTER_READY, + decode_outer_header, encode_outer, }; const HELPER: &str = "host::cli::snapshots::capture::tests::capture_helper"; @@ -834,14 +835,21 @@ fn respond(stream: &mut (impl Read + Write), capability: &[u8], refused: bool) { .unwrap(); let (kind, payload) = read_record(stream); assert_eq!(kind, OUTER_DATA); - let request = decode_app(&payload).unwrap(); - assert_eq!(request.kind, APP_CAPTURE); - assert_eq!(request.payload, [0]); + // The capture request of NVXC version 2, with its flag that keeps the executions. + let (request_id, frame) = Frame::decode(&payload).unwrap(); + assert_eq!(frame, Frame::Capture(&[0])); let reply = if refused { - encode_app(APP_ERROR, request.request_id, 22, b"capture-failed") + Frame::Error { + status: 22, + category: b"capture-failed", + } } else { - encode_app(APP_READY, request.request_id, 0, &[]) + Frame::Ready { + version: v2::VERSION, + activation: &[], + } }; + let reply = reply.encode(request_id).unwrap(); stream .write_all(&encode_outer(OUTER_DATA, &[7; 16], 1, 1, &reply).unwrap()) .unwrap(); diff --git a/nvx-rs/src/host/restore/tests.rs b/nvx-rs/src/host/restore/tests.rs index 3e0b9d40f..14b20f739 100644 --- a/nvx-rs/src/host/restore/tests.rs +++ b/nvx-rs/src/host/restore/tests.rs @@ -1,7 +1,14 @@ #![cfg(any(target_os = "linux", windows))] #![cfg_attr(windows, allow(unsafe_code))] -#[cfg(feature = "cli")] +// It speaks version 1 of NVXC, which only the guest agents and the fake OpenVMM build. +#[cfg(all( + feature = "cli", + any( + feature = "testing", + all(feature = "guest", target_os = "linux", target_arch = "x86_64") + ) +))] mod live_init; use std::io::{BufRead, BufReader, Write}; diff --git a/nvx-rs/src/host/restore_driver.rs b/nvx-rs/src/host/restore_driver.rs index e35650aa0..4e1b5c97d 100644 --- a/nvx-rs/src/host/restore_driver.rs +++ b/nvx-rs/src/host/restore_driver.rs @@ -2,9 +2,10 @@ //! //! [`restore`] drives a [`RestoreMachine`] through a launched VM's restore-ready event and //! control sessions until the guest's Ready, then sends the VM's mapping table and starts its -//! workload, and returns the live [`ControlSession`]. The restore-ready client and the control -//! server must be the VM's held process. Each session carries the same activate message: the -//! activation record and the workload's identity. A lost session is replaced, in at most +//! workload, whose standard input it ends at once, and returns the live [`ControlSession`], of +//! NVXC version 2, in which the workload's events arrive. The restore-ready client and the +//! control server must be the VM's held process. Each session carries the same activate message: +//! the activation record and the workload's identity. A lost session is replaced, in at most //! [`MAX_SESSIONS`] sessions in all. //! //! Every pending accept, read and write races the step's deadline, the caller's [`Cancel`] and @@ -38,7 +39,7 @@ use super::transport::Transport; use super::vm::{Capability, Launched, VmOutputs}; use crate::ErrorKind; use crate::activation::ActivationRecord; -use crate::protocol::{self, APP_EXEC, HostMap, workload::WorkloadIdentity}; +use crate::protocol::{self, HostMap, workload::WorkloadIdentity}; /// How long a restore may take, from the driver's start to the guest's Ready. const READY_BUDGET: Duration = Duration::from_secs(60); @@ -240,9 +241,11 @@ fn start( } let payload = protocol::encode_exec_payload(&workload.exec) .map_err(|error| ControlRestoreError::Start(error.into()))?; + // The workload reads no standard input, as the null device gives none. watch .check() - .and_then(|()| session.send_request(APP_EXEC, workload.request_id, &payload, deadline)) + .and_then(|()| session.start_exec(workload.request_id, &payload, deadline)) + .and_then(|()| session.close_stdin(workload.request_id, deadline)) .map_err(|error| ControlRestoreError::Start(watch.cause(error, deadline)))?; Ok(session) } diff --git a/nvx-rs/src/host/restore_driver/tests.rs b/nvx-rs/src/host/restore_driver/tests.rs index 718a7c2be..5abe88072 100644 --- a/nvx-rs/src/host/restore_driver/tests.rs +++ b/nvx-rs/src/host/restore_driver/tests.rs @@ -11,12 +11,13 @@ use crate::activation::{ }; use crate::host::process::Opened; use crate::host::session::ExecEvent; -use crate::protocol::{ExitCategory, WorkloadEnvironment}; +use crate::protocol::WorkloadEnvironment; +use crate::protocol::v2::Outcome; /// A stand-in for OpenVMM and its guest, which serves the control endpoint, sends the /// restore-ready event, attaches the host with the capability of its standard input, and answers -/// NVXC requests as `case` asks, with `activation` in its Ready. It records each request in -/// `trace`, and the step at which it stalls in `stage`. +/// NVXC requests of version 2 as `case` asks, with `activation` in its Ready. It records each +/// request in `trace`, and the step at which it stalls in `stage`. const PEER: &str = r#" import json, os, socket, struct, sys, time case, endpoint, ready, trace, stage, activation = sys.argv[1:] @@ -78,7 +79,7 @@ def record(stream, kind, epoch, sequence, payload=b''): stream.write(struct.pack('<4sHBB16sQQI', b'NVXS', 1, kind, 0, bytes([7]) * 16, epoch, sequence, len(payload)) + payload) def reply(stream, kind, rid, status, payload, epoch, sequence): - frame = struct.pack('<4sBBHQiI', b'NVXC', 1, kind, 0, rid, status, len(payload)) + payload + frame = struct.pack('<4sBBHQiI', b'NVXC', 2, kind, 0, rid, status, len(payload)) + payload record(stream, 5, epoch, sequence, frame) server = None if case == 'connect' else bind() if case == 'stall': @@ -103,7 +104,7 @@ for session in range(1, 4): while True: header = exact(stream, 44) frame = exact(stream, struct.unpack('), /// A chunk of the workload's standard error. Stderr(Vec), + /// The guest agent wrote this many more bytes of the standard input into the workload's, + /// which leave the window. + Acknowledged(u32), /// The workload ended. Exit { /// How it ended. - category: ExitCategory, - /// Its exit status, 128 plus the signal number for [`ExitCategory::Signal`]. + category: Outcome, + /// Its exit status, 128 plus the signal number for [`Outcome::Signal`]. status: i32, }, /// The guest agent refused the workload, and nothing of it runs any longer. Rejected { /// The guest's error number. status: i32, - /// The error category, such as [`LAUNCH_FAILED`](crate::protocol::LAUNCH_FAILED), - /// truncated to 64 characters. + /// The error category, such as `launch-failed`, truncated to 64 characters. category: String, }, } -/// Authenticated control session: `NVXC` frames, each in one DATA record of the host's NVXS -/// connection to OpenVMM's broker. +/// Authenticated control session: `NVXC` frames of version 2, each in one DATA record of the +/// host's NVXS connection to OpenVMM's broker. /// /// Received bytes are buffered until a whole record is available, so a read that times out /// loses nothing and callers can poll with short deadlines. A failure that leaves the stream @@ -146,6 +149,18 @@ pub(crate) enum ExecEvent { pub(crate) struct ControlSession { /// The host's NVXS connection to OpenVMM's control endpoint. pub(crate) connection: HostConnection, + /// The bytes of the running execution's standard input that the host sent and the guest agent + /// has not acknowledged, at most [`STDIN_WINDOW`]. + unacknowledged: u32, +} + +impl From for ControlSession { + fn from(connection: HostConnection) -> Self { + Self { + connection, + unacknowledged: 0, + } + } } impl ControlSession { @@ -158,19 +173,12 @@ impl ControlSession { ) -> Result { let connection = HostConnection::attach(transport, capability, deadline) .map_err(|error| failure(error, Stage::Attach))?; - Ok(Self { connection }) + Ok(connection.into()) } /// Waits until the guest agent acknowledges readiness. - #[cfg_attr( - not(test), - expect( - dead_code, - reason = "SL4.4c's host paths ping the guest through the session" - ) - )] pub(crate) fn ping(&mut self, deadline: Instant) -> Result<(), SessionError> { - self.acknowledged(APP_PING, &[], APP_READY, "readiness", deadline) + self.acknowledged(Frame::Ping, "readiness", deadline) } /// Asks nvx-init to capture the VM, killing its executions first when requested. @@ -179,13 +187,8 @@ impl ControlSession { kill_executions: bool, deadline: Instant, ) -> Result<(), SessionError> { - self.acknowledged( - APP_CAPTURE, - &protocol::encode_capture_payload(kill_executions), - APP_READY, - "capture", - deadline, - ) + let payload = protocol::encode_capture_payload(kill_executions); + self.acknowledged(Frame::Capture(&payload), "capture", deadline) } /// Sends nvx-init's activate message, whose `payload` carries a restore's activation record @@ -196,41 +199,14 @@ impl ControlSession { deadline: Instant, ) -> Result { let request_id = request_id()?; - self.send_app(protocol::APP_ACTIVATE, request_id, payload, Some(deadline))?; - match self.read_reply(request_id, Some(deadline))? { - (APP_READY, 0, ready) => { - crate::activation::ReadyActivation::decode(&ready).map_err(|error| { - SessionError::Protocol(format!("the guest's Ready is invalid: {error}")) - }) - } - _ => Err(SessionError::Protocol( - "the guest agent did not activate the restore".to_owned(), - )), - } - } - - /// Asks the guest agent which control features its image provides. - /// - /// An agent that predates the request refuses it as an unsupported operation, which means - /// that the image provides none of them. - #[cfg_attr( - not(test), - expect( - dead_code, - reason = "no host path sends FEATURES here, and SL4.4d deletes it" - ) - )] - pub(crate) fn features(&mut self, deadline: Instant) -> Result { - let request_id = request_id()?; - self.send_app(APP_FEATURES, request_id, &[], Some(deadline))?; - let (kind, status, payload) = self.read_reply(request_id, Some(deadline))?; - match kind { - APP_READY if status == 0 => Ok(GuestFeatures::decode(&payload)?), - APP_ERROR if payload == UNSUPPORTED_OPERATION => Ok(GuestFeatures::NONE), - _ => Err(SessionError::Protocol( - "the guest agent returned an invalid features response".to_owned(), - )), - } + self.send(request_id, Frame::Activate(payload), Some(deadline))?; + let reply = self.reply(request_id, Some(deadline))?; + let ready = ready(Frame::decode(&reply)?.1)?.ok_or_else(|| { + SessionError::Protocol("the guest agent did not activate the restore".to_owned()) + })?; + crate::activation::ReadyActivation::decode(ready).map_err(|error| { + SessionError::Protocol(format!("the guest's Ready is invalid: {error}")) + }) } /// Hands the guest agent the host mapping table, in as many requests as it takes, and waits @@ -240,25 +216,26 @@ impl ControlSession { maps: &[HostMap], deadline: Instant, ) -> Result<(), SessionError> { - let payloads = protocol::encode_maps_payloads(maps, OUTER_MAX_PAYLOAD - APP_HEADER_LEN)?; - for payload in payloads { + for payload in protocol::encode_maps_payloads(maps, v2::MAX_PAYLOAD)? { let request_id = request_id()?; - self.send_app(APP_MAPS, request_id, &payload, Some(deadline))?; - let (kind, status, payload) = self.read_reply(request_id, Some(deadline))?; - match kind { - APP_READY if status == 0 && payload.is_empty() => {} - APP_ERROR if payload == protocol::MAPPING_FAILED.as_bytes() => { + self.send(request_id, Frame::Maps(&payload), Some(deadline))?; + let reply = self.reply(request_id, Some(deadline))?; + match Frame::decode(&reply)?.1 { + Frame::Error { status, category } + if category == protocol::MAPPING_FAILED.as_bytes() => + { return Err(SessionError::Protocol(format!( "the guest agent could not mount a mapped host path (error {status})" ))); } - APP_ERROR => { - let category = error_category(&payload); + Frame::Error { status, category } => { + let category = error_category(category); return Err(SessionError::Protocol(format!( "the guest agent did not mount the host paths: {category} (status \ {status})" ))); } + frame if ready(frame)?.is_some_and(<[u8]>::is_empty) => {} _ => { return Err(SessionError::Protocol( "the guest agent returned an invalid host mapping response".to_owned(), @@ -269,23 +246,43 @@ impl ControlSession { Ok(()) } - /// Starts a workload and returns the request ID that identifies its events. - #[cfg_attr( - not(test), - expect( - dead_code, - reason = "SL4.4c's sandbox exec and run start workloads through it" - ) - )] + /// Starts the workload that the `EXEC` payload `payload` describes as the execution of + /// `request_id`, which its events repeat, with none of its standard input in the window. pub(crate) fn start_exec( &mut self, - workload: &Workload<'_>, + request_id: u64, + payload: &[u8], deadline: Instant, - ) -> Result { - let payload = protocol::encode_exec_payload(workload)?; - let request_id = request_id()?; - self.send_app(APP_EXEC, request_id, &payload, Some(deadline))?; - Ok(request_id) + ) -> Result<(), SessionError> { + self.unacknowledged = 0; + self.send(request_id, Frame::Exec(payload), Some(deadline)) + } + + /// Sends the next bytes of `data` that the window takes, at most what one frame holds, as + /// standard input of the execution of `request_id`, and returns how many it sent: none + /// while the guest agent has not acknowledged [`STDIN_WINDOW`] bytes. + pub(crate) fn send_stdin( + &mut self, + request_id: u64, + data: &[u8], + deadline: Instant, + ) -> Result { + let room = (STDIN_WINDOW - self.unacknowledged) as usize; + let chunk = &data[..data.len().min(room).min(v2::MAX_PAYLOAD)]; + if !chunk.is_empty() { + self.send(request_id, Frame::Stdin(chunk), Some(deadline))?; + self.unacknowledged += chunk.len() as u32; + } + Ok(chunk.len()) + } + + /// Ends the standard input of the execution of `request_id` after the bytes that it sent. + pub(crate) fn close_stdin( + &mut self, + request_id: u64, + deadline: Instant, + ) -> Result<(), SessionError> { + self.send(request_id, Frame::StdinEnd, Some(deadline)) } /// Reads the next event of the workload started with `request_id`. @@ -294,22 +291,33 @@ impl ControlSession { request_id: u64, deadline: Option, ) -> Result { - let (kind, status, payload) = self.read_reply(request_id, deadline)?; - match kind { - APP_STDOUT => Ok(ExecEvent::Stdout(payload)), - APP_STDERR => Ok(ExecEvent::Stderr(payload)), - APP_EXIT => Ok(ExecEvent::Exit { - category: ExitCategory::parse(&payload)?, + let reply = self.reply(request_id, deadline)?; + Ok(match Frame::decode(&reply)?.1 { + Frame::Stdout(data) => ExecEvent::Stdout(data.to_vec()), + Frame::Stderr(data) => ExecEvent::Stderr(data.to_vec()), + Frame::StdinAck(count) => { + self.unacknowledged = self.unacknowledged.checked_sub(count).ok_or_else(|| { + SessionError::Protocol( + "the guest agent acknowledged standard input that it did not receive" + .to_owned(), + ) + })?; + ExecEvent::Acknowledged(count) + } + Frame::Exit { status, outcome } => ExecEvent::Exit { + category: outcome, status, - }), - APP_ERROR => Ok(ExecEvent::Rejected { + }, + Frame::Error { status, category } => ExecEvent::Rejected { status, - category: error_category(&payload), - }), - _ => Err(SessionError::Protocol( - "the guest agent returned an invalid exec response".to_owned(), - )), - } + category: error_category(category), + }, + _ => { + return Err(SessionError::Protocol( + "the guest agent returned an invalid exec response".to_owned(), + )); + } + }) } /// Asks the guest agent to terminate the workload started with `request_id`. @@ -317,128 +325,111 @@ impl ControlSession { /// The guest reports the workload's outcome as a regular exec event, with the cancelled /// category if the request arrived before the workload exited. A request for a workload that /// already finished is ignored. - #[cfg_attr( - not(test), - expect( - dead_code, - reason = "SL4.4c's sandbox exec and run cancel through it on Ctrl-C" - ) - )] pub(crate) fn cancel_exec( &mut self, request_id: u64, deadline: Instant, ) -> Result<(), SessionError> { - self.send_app(APP_CANCEL, request_id, &[], Some(deadline)) + self.send(request_id, Frame::Cancel, Some(deadline)) } - /// Asks the guest agent to shut the VM down. - #[cfg_attr( - not(test), - expect( - dead_code, - reason = "SL4.4c's sandbox exec and run stop the guest through it" - ) - )] + /// Asks the guest agent to shut the VM down, which first ends an execution that runs: the + /// frames of that execution, which come before the acknowledgement, are skipped. pub(crate) fn stop(&mut self, deadline: Instant) -> Result<(), SessionError> { - self.acknowledged(APP_STOP, &[], APP_STOPPED, "stop", deadline) - } - - /// Sends an application request of `kind` with `request_id` and `payload` in the next - /// sequenced outer record, for a caller that checks the responses itself, as the `nvx-rs` - /// CLI does with the messages of `nvx.py`. - pub(crate) fn send_request( - &mut self, - kind: u8, - request_id: u64, - payload: &[u8], - deadline: Instant, - ) -> Result<(), SessionError> { - self.send_app(kind, request_id, payload, Some(deadline)) - } - - /// Reads the next application frame, checked against the session's identity and sequence, - /// and returns its kind, request ID, status, and payload. - pub(crate) fn read_response( - &mut self, - deadline: Option, - ) -> Result<(u8, u64, i32, Vec), SessionError> { - self.read_app(deadline) + let request_id = request_id()?; + self.send(request_id, Frame::Stop, Some(deadline))?; + loop { + let record = self.receive(Some(deadline))?; + match Frame::decode(&record)? { + (id, Frame::Stopped) if id == request_id => return Ok(()), + ( + id, + Frame::Stdout(_) + | Frame::Stderr(_) + | Frame::StdinAck(_) + | Frame::Exit { .. } + | Frame::Error { .. }, + ) if id != request_id => {} + _ => { + return Err(SessionError::Protocol( + "the guest agent did not acknowledge stop".to_owned(), + )); + } + } + } } - /// Encodes and sends one application frame in the next DATA record. - fn send_app( + /// Encodes `frame` with `request_id`, and sends it in the next DATA record, which the guest + /// agent takes as one frame. + fn send( &mut self, - kind: u8, request_id: u64, - payload: &[u8], + frame: Frame<'_>, deadline: Option, ) -> Result<(), SessionError> { - let frame = protocol::encode_app(kind, request_id, 0, payload); - // The guest agent takes each DATA record as one frame, so a frame must fit one. - if frame.len() > OUTER_MAX_PAYLOAD { - return Err(SessionError::Protocol( - "control payload exceeds the outer protocol limit".to_owned(), - )); - } + let frame = frame.encode(request_id)?; self.connection .send(&frame, deadline) .map_err(|error| failure(error, Stage::Data)) } - /// Reads and decodes the next application frame, the payload of the next DATA record. - fn read_app( - &mut self, - deadline: Option, - ) -> Result<(u8, u64, i32, Vec), SessionError> { - let payload = self - .connection + /// Returns the payload of the next DATA record: the bytes of the guest agent's next frame. + fn receive(&mut self, deadline: Option) -> Result, SessionError> { + self.connection .receive(deadline) - .map_err(|error| failure(error, Stage::Data))?; - let frame = protocol::decode_app(&payload)?; - Ok(( - frame.kind, - frame.request_id, - frame.status, - frame.payload.to_vec(), - )) + .map_err(|error| failure(error, Stage::Data)) } - /// Reads the next application frame, which must answer `request_id`, and returns its kind, - /// status, and payload. - fn read_reply( + /// Returns the bytes of the next frame, which must answer `request_id`. + fn reply( &mut self, request_id: u64, deadline: Option, - ) -> Result<(u8, i32, Vec), SessionError> { - let (kind, response_id, status, payload) = self.read_app(deadline)?; - if response_id != request_id { + ) -> Result, SessionError> { + let record = self.receive(deadline)?; + if Frame::decode(&record)?.0 != request_id { return Err(SessionError::Protocol( "the guest agent returned a mismatched request ID".to_owned(), )); } - Ok((kind, status, payload)) + Ok(record) } - /// Sends a request of `kind` and waits until the guest agent acknowledges - /// `what` with an empty `reply` of status 0. + /// Sends `request` and waits until the guest agent acknowledges `what` with a `READY` of its + /// request ID that carries nothing but the protocol version. fn acknowledged( &mut self, - kind: u8, - payload: &[u8], - reply: u8, + request: Frame<'_>, what: &str, deadline: Instant, ) -> Result<(), SessionError> { let request_id = request_id()?; - self.send_app(kind, request_id, payload, Some(deadline))?; - let (kind, response_id, status, payload) = self.read_app(Some(deadline))?; - if kind != reply || response_id != request_id || status != 0 || !payload.is_empty() { - return Err(SessionError::Protocol(format!( + self.send(request_id, request, Some(deadline))?; + let reply = self.receive(Some(deadline))?; + match Frame::decode(&reply)? { + (id, frame) if id == request_id && ready(frame)?.is_some_and(<[u8]>::is_empty) => { + Ok(()) + } + _ => Err(SessionError::Protocol(format!( "the guest agent did not acknowledge {what}" - ))); + ))), } - Ok(()) + } +} + +/// Returns what a `READY` reports about an activation, once it checked that the `READY` carries +/// the protocol version, or `None` for another frame. +fn ready(frame: Frame<'_>) -> Result, SessionError> { + match frame { + Frame::Ready { + version: v2::VERSION, + activation, + } => Ok(Some(activation)), + Frame::Ready { version, .. } => Err(SessionError::Protocol(format!( + "the guest agent speaks NVXC version {version}, not version {}", + v2::VERSION + ))), + _ => Ok(None), } } diff --git a/nvx-rs/src/host/session/tests.rs b/nvx-rs/src/host/session/tests.rs index 74f63c441..a1960ff3a 100644 --- a/nvx-rs/src/host/session/tests.rs +++ b/nvx-rs/src/host/session/tests.rs @@ -6,7 +6,7 @@ use super::*; use crate::protocol::nvxs::{ErrorCode, Identity}; use crate::protocol::{ OUTER_DATA, OUTER_ERROR, OUTER_HEADER_LEN, OUTER_HOST_ATTACH, OUTER_READY, OUTER_RESET, - OUTER_WAIT, WorkloadEnvironment, decode_app, decode_outer_header, encode_app, encode_outer, + OUTER_WAIT, Workload, WorkloadEnvironment, decode_outer_header, encode_outer, }; #[cfg(any(target_os = "linux", windows))] @@ -14,6 +14,51 @@ mod managed_lifecycle; const INSTANCE: [u8; 16] = [7; 16]; +/// The codes of the kinds of requests that version 2 shares with version 1, as its header holds +/// them. +const PING: u8 = 0x01; +const EXEC: u8 = 0x02; +const STOP: u8 = 0x03; +const CANCEL: u8 = 0x04; +const MAPS: u8 = 0x06; +const CAPTURE: u8 = 0x07; +const ACTIVATE: u8 = 0x08; +const STDIN: u8 = 0x09; +const STDIN_END: u8 = 0x0a; + +/// Returns the bytes of `frame` with `request_id`. +fn encode(request_id: u64, frame: Frame<'_>) -> Vec { + frame.encode(request_id).unwrap() +} + +/// Returns a `READY` of the protocol version for `request_id`, which reports `activation`. +fn ready_frame(request_id: u64, activation: &[u8]) -> Vec { + encode( + request_id, + Frame::Ready { + version: v2::VERSION, + activation, + }, + ) +} + +/// Returns the kind, request ID and payload of a request that the host sent. +fn request(frame: &[u8]) -> (u8, u64, Vec) { + let (request_id, _) = Frame::decode(frame).unwrap(); + (frame[5], request_id, frame[24..].to_vec()) +} + +/// Returns a workload of `argv`, without a timeout, in the default environment. +fn workload(argv: &[&str]) -> Vec { + protocol::encode_exec_payload(&Workload { + argv: argv.iter().map(|&argument| argument.to_owned()).collect(), + timeout_ms: 0, + cwd: None, + environment: WorkloadEnvironment::Default, + }) + .unwrap() +} + #[test] fn the_legacy_connect_wrapper_keeps_indeterminate_process_failures_as_io() { let error = Error::io("cannot verify the VM process").with_source(io::Error::new( @@ -118,12 +163,18 @@ fn ready() -> (ControlSession, Scripted) { /// Kind and payload of each application request that a guest received. type Requests = Arc)>>>; -/// Transport of a guest that answers each application request with `answer(kind, request_id)` -/// and records the kind and payload of each request. +/// The frames with which a guest answers a request, from its kind and request ID. +type Answer = Box Vec> + Send>; + +/// The frame with which a guest answers a request, from its kind and request ID. +type Reply = fn(u8, u64) -> Vec; + +/// Transport of a guest that answers each application request with the frames that `answer` +/// returns for it, each in a record, and records the kind and payload of each request. struct Responder { input: VecDeque, next_sequence: u64, - answer: fn(u8, u64) -> Vec, + answer: Answer, requests: Requests, } @@ -142,15 +193,13 @@ impl Transport for Responder { fn write_all(&mut self, data: &[u8], _timeout: Option) -> io::Result<()> { let header = decode_outer_header(data[..OUTER_HEADER_LEN].try_into().unwrap()).unwrap(); if header.record_type == OUTER_DATA { - let request = decode_app(&data[OUTER_HEADER_LEN..]).unwrap(); - self.requests - .lock() - .unwrap() - .push((request.kind, request.payload.to_vec())); - let reply = (self.answer)(request.kind, request.request_id); - self.input - .extend(outer(OUTER_DATA, 1, self.next_sequence, &reply)); - self.next_sequence += 1; + let (kind, request_id, payload) = request(&data[OUTER_HEADER_LEN..]); + self.requests.lock().unwrap().push((kind, payload)); + for reply in (self.answer)(kind, request_id) { + self.input + .extend(outer(OUTER_DATA, 1, self.next_sequence, &reply)); + self.next_sequence += 1; + } } Ok(()) } @@ -164,6 +213,14 @@ fn attached(answer: fn(u8, u64) -> Vec) -> ControlSession { /// Attaches to a guest that answers each application request with `answer`, and returns the /// kind and payload of each request that the guest receives. fn attached_recording(answer: fn(u8, u64) -> Vec) -> (ControlSession, Requests) { + responding(Box::new(move |kind, request_id| { + vec![answer(kind, request_id)] + })) +} + +/// Attaches to a guest that answers each application request with the frames that `answer` +/// returns, and returns the kind and payload of each request that the guest receives. +fn responding(answer: Answer) -> (ControlSession, Requests) { let requests = Arc::new(Mutex::new(Vec::new())); let guest = Responder { input: outer(OUTER_READY, 1, 0, &[]).into(), @@ -299,55 +356,67 @@ fn attach_responses_that_the_protocol_does_not_allow_fail_closed() { #[test] fn exec_streams_events_in_order() { - // The request ID is random, so the scripted guest answers the frames the host sent. + // The scripted guest answers the frames of the execution that the host started, whose output + // passes the old cap of version 1, 1 MiB, in frames that each fill a record. let (mut session, wire) = ready(); - let workload = Workload { - argv: vec!["/bin/sh".to_owned(), "-c".to_owned(), "echo hi".to_owned()], - timeout_ms: 1000, - cwd: None, - environment: WorkloadEnvironment::Default, - }; - let request_id = session.start_exec(&workload, deadline()).unwrap(); + let payload = workload(&["/bin/sh", "-c", "echo hi"]); + session.start_exec(9, &payload, deadline()).unwrap(); let records = host_records(&wire.output()); let (record_type, sequence, frame) = &records[1]; assert_eq!((*record_type, *sequence), (OUTER_DATA, 0)); - let frame = decode_app(frame).unwrap(); - assert_eq!((frame.kind, frame.request_id), (APP_EXEC, request_id)); + assert_eq!(request(frame), (EXEC, 9, payload)); - wire.supply(&[ - outer( - OUTER_DATA, - 1, - 1, - &encode_app(APP_STDOUT, request_id, 0, b"hi\n"), - ), + let chunk = vec![b'x'; v2::MAX_PAYLOAD]; + let chunks = 1024 * 1024 / v2::MAX_PAYLOAD + 1; + let mut records: Vec> = (1..=chunks) + .map(|sequence| { + outer( + OUTER_DATA, + 1, + sequence as u64, + &encode(9, Frame::Stdout(&chunk)), + ) + }) + .collect(); + let sequence = chunks as u64; + records.extend([ outer( OUTER_DATA, 1, - 2, - &encode_app(APP_STDERR, request_id, 0, b"warn"), + sequence + 1, + &encode(9, Frame::Stderr(b"warn")), ), outer( OUTER_DATA, 1, - 3, - &encode_app(APP_EXIT, request_id, 3, b"exit"), + sequence + 2, + &encode( + 9, + Frame::Exit { + status: 3, + outcome: Outcome::Exit, + }, + ), ), ]); - let events: Vec = (0..3) - .map(|_| { - session - .next_exec_event(request_id, Some(deadline())) - .unwrap() - }) - .collect(); + wire.supply(&records); + let mut stdout = 0; + let events = loop { + match session.next_exec_event(9, Some(deadline())).unwrap() { + ExecEvent::Stdout(data) => stdout += data.len(), + event => { + break [event, session.next_exec_event(9, Some(deadline())).unwrap()]; + } + } + }; + assert_eq!(stdout, chunks * v2::MAX_PAYLOAD); + assert!(stdout > 1024 * 1024); assert_eq!( events, [ - ExecEvent::Stdout(b"hi\n".to_vec()), ExecEvent::Stderr(b"warn".to_vec()), ExecEvent::Exit { - category: ExitCategory::Exit, + category: Outcome::Exit, status: 3 }, ] @@ -361,42 +430,35 @@ fn exec_streams_events_in_order() { #[test] fn exec_sends_the_working_directory_and_reports_its_refusal() { let (mut session, wire) = ready(); - let workload = Workload { + let payload = protocol::encode_exec_payload(&Workload { argv: vec!["/bin/pwd".to_owned()], timeout_ms: 0, cwd: Some("/work"), environment: WorkloadEnvironment::Default, - }; - let request_id = session.start_exec(&workload, deadline()).unwrap(); + }) + .unwrap(); + session.start_exec(5, &payload, deadline()).unwrap(); let records = host_records(&wire.output()); - let frame = decode_app(&records[1].2).unwrap(); - assert_eq!((frame.kind, frame.request_id), (APP_EXEC, request_id)); - assert_eq!( - frame.payload, - protocol::encode_exec_payload(&workload).unwrap() - ); + assert_eq!(request(&records[1].2), (EXEC, 5, payload)); let diagnostic = b"nvx-managed-agent: cannot enter working directory /work: No such file\n"; wire.supply(&[ - outer( - OUTER_DATA, - 1, - 1, - &encode_app(APP_STDERR, request_id, 0, diagnostic), - ), + outer(OUTER_DATA, 1, 1, &encode(5, Frame::Stderr(diagnostic))), outer( OUTER_DATA, 1, 2, - &encode_app(APP_ERROR, request_id, 2, protocol::CWD_FAILED.as_bytes()), + &encode( + 5, + Frame::Error { + status: 2, + category: protocol::CWD_FAILED.as_bytes(), + }, + ), ), ]); let events: Vec = (0..2) - .map(|_| { - session - .next_exec_event(request_id, Some(deadline())) - .unwrap() - }) + .map(|_| session.next_exec_event(5, Some(deadline())).unwrap()) .collect(); assert_eq!( events, @@ -410,13 +472,63 @@ fn exec_sends_the_working_directory_and_reports_its_refusal() { ); } +#[test] +fn standard_input_keeps_at_most_the_window_unacknowledged() { + let (mut session, wire) = ready(); + session + .start_exec(7, &workload(&["/bin/cat"]), deadline()) + .unwrap(); + // A frame holds what a record does, and the window the rest of its 64 KiB. + let data = vec![b'i'; 100 * 1024]; + let first = session.send_stdin(7, &data, deadline()).unwrap(); + assert_eq!(first, v2::MAX_PAYLOAD); + let second = session.send_stdin(7, &data[first..], deadline()).unwrap(); + assert_eq!(first + second, STDIN_WINDOW as usize); + assert_eq!(session.send_stdin(7, &data, deadline()).unwrap(), 0); + // An acknowledgement returns its bytes to the window, and no more than it. + wire.supply(&[outer(OUTER_DATA, 1, 1, &encode(7, Frame::StdinAck(1000)))]); + assert_eq!( + session.next_exec_event(7, Some(deadline())).unwrap(), + ExecEvent::Acknowledged(1000) + ); + assert_eq!(session.send_stdin(7, &data, deadline()).unwrap(), 1000); + assert_eq!(session.send_stdin(7, &data, deadline()).unwrap(), 0); + session.close_stdin(7, deadline()).unwrap(); + let frames: Vec<_> = host_records(&wire.output())[2..] + .iter() + .map(|(_, _, frame)| request(frame)) + .collect(); + assert_eq!( + frames, + [ + (STDIN, 7, data[..first].to_vec()), + (STDIN, 7, data[..second].to_vec()), + (STDIN, 7, data[..1000].to_vec()), + (STDIN_END, 7, Vec::new()), + ] + ); + // A new execution starts with its whole window, and the guest agent acknowledges no bytes + // that the host did not send. + session + .start_exec(8, &workload(&["/bin/cat"]), deadline()) + .unwrap(); + assert_eq!(session.send_stdin(8, &data[..10], deadline()).unwrap(), 10); + wire.supply(&[outer(OUTER_DATA, 1, 2, &encode(8, Frame::StdinAck(11)))]); + assert_violation( + &session.next_exec_event(8, Some(deadline())).unwrap_err(), + "the guest agent acknowledged standard input that it did not receive", + "an acknowledgement beyond the bytes sent", + ); +} + #[test] fn records_that_the_protocol_does_not_allow_fail_closed() { - let frame = encode_app(APP_STDOUT, 1, 0, b"x"); + let frame = encode(1, Frame::Stdout(b"x")); let data = outer(OUTER_DATA, 1, 1, &frame); let oversized = u32::try_from(OUTER_MAX_PAYLOAD + 1).unwrap().to_le_bytes(); let invalid = "control endpoint returned an invalid data record"; let truncated = "control endpoint returned a truncated application frame"; + let malformed = "the NVXC frame of version 2 is malformed"; let cases = [ ( "DATA out of sequence", @@ -452,8 +564,13 @@ fn records_that_the_protocol_does_not_allow_fail_closed() { ), ( "a truncated frame", - outer(OUTER_DATA, 1, 1, &frame[..APP_HEADER_LEN - 1]), - truncated, + outer(OUTER_DATA, 1, 1, &frame[..23]), + malformed, + ), + ( + "a frame of version 1", + outer(OUTER_DATA, 1, 1, &patched(frame.clone(), 4, &[1])), + "the control peer sent a frame of another protocol than NVXC version 2", ), ( "nonzero flags", @@ -507,7 +624,11 @@ fn records_that_the_protocol_does_not_allow_fail_closed() { #[test] fn timed_out_polls_keep_partial_records() { let (mut session, wire) = ready(); - let record = outer(OUTER_DATA, 1, 1, &encode_app(APP_EXIT, 9, 0, b"cancelled")); + let exit = Frame::Exit { + status: 0, + outcome: Outcome::Cancelled, + }; + let record = outer(OUTER_DATA, 1, 1, &encode(9, exit)); let (first, second) = record.split_at(30); wire.supply(&[first.to_vec()]); assert!(matches!( @@ -519,107 +640,241 @@ fn timed_out_polls_keep_partial_records() { assert_eq!( session.next_exec_event(9, Some(deadline())).unwrap(), ExecEvent::Exit { - category: ExitCategory::Cancelled, + category: Outcome::Cancelled, status: 0 } ); let records = host_records(&wire.output()); - let frame = decode_app(&records.last().unwrap().2).unwrap(); - assert_eq!((frame.kind, frame.request_id), (APP_CANCEL, 9)); + assert_eq!(request(&records.last().unwrap().2), (CANCEL, 9, Vec::new())); } #[test] fn frames_travel_in_one_record_each() { let (mut session, wire) = ready(); - let longest = vec![0x5a; OUTER_MAX_PAYLOAD - APP_HEADER_LEN]; - session - .send_request(APP_EXEC, 1, &longest, deadline()) - .unwrap(); + let longest = vec![0x5a; v2::MAX_PAYLOAD]; + session.start_exec(1, &longest, deadline()).unwrap(); let error = session - .send_request( - APP_EXEC, - 2, - &[longest.as_slice(), &[0]].concat(), - deadline(), - ) + .start_exec(2, &[longest.as_slice(), &[0]].concat(), deadline()) .unwrap_err(); - let message = "control payload exceeds the outer protocol limit"; + let message = "an NVXC frame must fit one outer data record"; assert_violation(&error, message, "a frame longer than a record"); let records = host_records(&wire.output()); assert_eq!(records.len(), 2); let (record_type, sequence, frame) = &records[1]; assert_eq!((*record_type, *sequence), (OUTER_DATA, 0)); - let frame = decode_app(frame).unwrap(); - assert_eq!((frame.kind, frame.request_id), (APP_EXEC, 1)); - assert_eq!(frame.payload, longest); + assert_eq!(request(frame), (EXEC, 1, longest)); // The refused frame changed nothing. session.cancel_exec(1, deadline()).unwrap(); let records = host_records(&wire.output()); assert_eq!((records[2].0, records[2].1), (OUTER_DATA, 1)); } -/// The response kind that acknowledges a ping or a stop request. -fn acknowledgement(kind: u8) -> u8 { - if kind == APP_PING { - APP_READY +/// Returns the frame that acknowledges a ping or a stop request of `request_id`. +fn acknowledgement(kind: u8, request_id: u64) -> Vec { + if kind == PING { + ready_frame(request_id, &[]) } else { - APP_STOPPED + encode(request_id, Frame::Stopped) } } #[test] fn pings_and_stops_need_an_empty_acknowledgement() { - let (mut session, requests) = - attached_recording(|kind, id| encode_app(acknowledgement(kind), id, 0, &[])); + let (mut session, requests) = attached_recording(acknowledgement); session.ping(deadline()).unwrap(); session.stop(deadline()).unwrap(); assert_eq!( *requests.lock().unwrap(), - [(APP_PING, Vec::new()), (APP_STOP, Vec::new())] + [(PING, Vec::new()), (STOP, Vec::new())] ); - let answers: [fn(u8, u64) -> Vec; 4] = [ - |_, id| encode_app(APP_ERROR, id, 0, &[]), - |kind, id| encode_app(acknowledgement(kind), id ^ 1, 0, &[]), - |kind, id| encode_app(acknowledgement(kind), id, 1, &[]), - |kind, id| encode_app(acknowledgement(kind), id, 0, b"x"), + let readiness = "the guest agent did not acknowledge readiness"; + let stop = "the guest agent did not acknowledge stop"; + let malformed = "the NVXC frame of version 2 is malformed"; + // A refusal, another request's acknowledgement, one with a status, and one of the other + // request or with a payload. + let answers: [(Reply, &str, &str); 4] = [ + (|_, id| encode(id, Frame::Busy), readiness, stop), + (|kind, id| acknowledgement(kind, id ^ 1), readiness, stop), + ( + |kind, id| patched(acknowledgement(kind, id), 16, &[1]), + malformed, + malformed, + ), + ( + |kind, id| { + if kind == PING { + ready_frame(id, b"x") + } else { + ready_frame(id, &[]) + } + }, + readiness, + stop, + ), ]; - for answer in answers { + for (answer, pinged, stopped) in answers { let error = attached(answer).ping(deadline()).unwrap_err(); - assert_eq!( - error.to_string(), - "the guest agent did not acknowledge readiness" - ); + assert_violation(&error, pinged, "a ping"); let error = attached(answer).stop(deadline()).unwrap_err(); - assert_eq!( - error.to_string(), - "the guest agent did not acknowledge stop" - ); + assert_violation(&error, stopped, "a stop"); + } +} + +#[test] +fn ready_must_carry_the_protocol_version() { + let maps = [HostMap { + source: "0".to_owned(), + target: "/work".to_owned(), + read_only: true, + }]; + let answers: [(Reply, u8); 2] = [ + ( + |_, id| { + let frame = Frame::Ready { + version: 1, + activation: &[], + }; + encode(id, frame) + }, + 1, + ), + ( + |_, id| { + let frame = Frame::Ready { + version: 3, + activation: &[], + }; + encode(id, frame) + }, + 3, + ), + ]; + for (answer, version) in answers { + let message = format!("the guest agent speaks NVXC version {version}, not version 2"); + let error = attached(answer).ping(deadline()).unwrap_err(); + assert_violation(&error, &message, "a ping"); + let error = attached(answer).capture(false, deadline()).unwrap_err(); + assert_violation(&error, &message, "a capture"); + let error = attached(answer) + .map_host_paths(&maps, deadline()) + .unwrap_err(); + assert_violation(&error, &message, "a mapping table"); + let error = attached(answer) + .activate(b"activation", deadline()) + .unwrap_err(); + assert_violation(&error, &message, "an activation"); } + // A guest agent that predates version 2 answers in version 1's frames. + let error = attached(|_, id| { + [ + b"NVXC".as_slice(), + &[1, 0x81, 0, 0], + &id.to_le_bytes(), + &[0; 8], + ] + .concat() + }) + .ping(deadline()) + .unwrap_err(); + let message = "the control peer sent a frame of another protocol than NVXC version 2"; + assert_violation(&error, message, "a READY of version 1"); +} + +#[test] +fn a_stop_ends_the_execution_that_runs_and_skips_its_frames() { + // The guest agent ends the execution, whose output and end come before the acknowledgement. + let (mut session, requests) = responding(Box::new(|kind, id| match kind { + STOP => vec![ + encode(4, Frame::Stdout(b"partial")), + encode(4, Frame::StdinAck(1)), + encode( + 4, + Frame::Exit { + status: 137, + outcome: Outcome::Cancelled, + }, + ), + encode(id, Frame::Stopped), + ], + _ => Vec::new(), + })); + session + .start_exec(4, &workload(&["/bin/cat"]), deadline()) + .unwrap(); + assert_eq!(session.send_stdin(4, b"i", deadline()).unwrap(), 1); + session.stop(deadline()).unwrap(); + let kinds: Vec = requests + .lock() + .unwrap() + .iter() + .map(|(kind, _)| *kind) + .collect(); + assert_eq!(kinds, [EXEC, STDIN, STOP]); + // An execution that the guest agent refused ends with its error, which comes before the + // acknowledgement too. + let (mut session, _) = responding(Box::new(|kind, id| match kind { + STOP => vec![ + encode(5, Frame::Stderr(b"cannot enter /missing")), + encode( + 5, + Frame::Error { + status: 2, + category: b"cwd-failed", + }, + ), + encode(id, Frame::Stopped), + ], + _ => Vec::new(), + })); + session + .start_exec(5, &workload(&["/bin/pwd"]), deadline()) + .unwrap(); + session.stop(deadline()).unwrap(); + // A frame of the stop's own ID other than its acknowledgement is refused, its refusal too. + let (mut session, _) = responding(Box::new(|_, id| vec![encode(id, Frame::Stdout(b"x"))])); + assert_violation( + &session.stop(deadline()).unwrap_err(), + "the guest agent did not acknowledge stop", + "output of the stop's own ID", + ); + let (mut session, _) = responding(Box::new(|_, id| { + vec![encode( + id, + Frame::Error { + status: 1, + category: b"invalid-request", + }, + )] + })); + assert_violation( + &session.stop(deadline()).unwrap_err(), + "the guest agent did not acknowledge stop", + "a refusal of the stop", + ); } #[test] fn capture_sends_both_kill_flags_and_waits_for_acknowledgement() { let (mut session, requests) = attached_recording(|kind, id| { - assert_eq!(kind, APP_CAPTURE); + assert_eq!(kind, CAPTURE); assert_ne!(id, 0); - encode_app(APP_READY, id, 0, &[]) + ready_frame(id, &[]) }); session.capture(false, deadline()).unwrap(); session.capture(true, deadline()).unwrap(); assert_eq!( *requests.lock().unwrap(), - [(APP_CAPTURE, vec![0]), (APP_CAPTURE, vec![1])] + [(CAPTURE, vec![0]), (CAPTURE, vec![1])] ); } #[test] fn capture_refuses_invalid_acknowledgements() { - let answers: [fn(u8, u64) -> Vec; 5] = [ - |_, id| encode_app(APP_ERROR, id, 16, b"busy"), - |_, id| encode_app(APP_STOPPED, id, 0, &[]), - |_, id| encode_app(APP_READY, id ^ 1, 0, &[]), - |_, id| encode_app(APP_READY, id, 1, &[]), - |_, id| encode_app(APP_READY, id, 0, b"x"), + let answers: [fn(u8, u64) -> Vec; 4] = [ + |_, id| encode(id, Frame::Busy), + |_, id| encode(id, Frame::Stopped), + |_, id| ready_frame(id ^ 1, &[]), + |_, id| ready_frame(id, b"x"), ]; for answer in answers { let error = attached(answer).capture(false, deadline()).unwrap_err(); @@ -629,14 +884,23 @@ fn capture_refuses_invalid_acknowledgements() { "an invalid capture acknowledgement", ); } + // Version 2 has no status in a READY. + let error = attached(|_, id| patched(ready_frame(id, &[]), 16, &[1])) + .capture(false, deadline()) + .unwrap_err(); + assert_violation( + &error, + "the NVXC frame of version 2 is malformed", + "a READY with a status", + ); } #[cfg(feature = "cli")] #[test] fn activate_sends_its_payload_and_returns_the_activation_that_ready_reports() { let (mut session, requests) = attached_recording(|kind, id| { - assert_eq!(kind, protocol::APP_ACTIVATE); - encode_app(APP_READY, id, 0, &[[1].as_slice(), &[9; 16]].concat()) + assert_eq!(kind, ACTIVATE); + ready_frame(id, &[[1].as_slice(), &[9; 16]].concat()) }); let ready = session.activate(b"activation", deadline()).unwrap(); assert_eq!( @@ -645,7 +909,7 @@ fn activate_sends_its_payload_and_returns_the_activation_that_ready_reports() { ); assert_eq!( *requests.lock().unwrap(), - [(protocol::APP_ACTIVATE, b"activation".to_vec())] + [(ACTIVATE, b"activation".to_vec())] ); } @@ -653,10 +917,16 @@ fn activate_sends_its_payload_and_returns_the_activation_that_ready_reports() { #[test] fn activate_refuses_a_reply_other_than_a_valid_ready() { let answers: [fn(u8, u64) -> Vec; 4] = [ - |_, id| encode_app(APP_ERROR, id, 22, b"invalid-request"), - |_, id| encode_app(APP_READY, id ^ 1, 0, &[0]), - |_, id| encode_app(APP_READY, id, 1, &[0]), - |_, id| encode_app(APP_READY, id, 0, &[2]), + |_, id| { + let frame = Frame::Error { + status: 22, + category: b"invalid-request", + }; + encode(id, frame) + }, + |_, id| ready_frame(id ^ 1, &[0]), + |_, id| patched(ready_frame(id, &[0]), 16, &[1]), + |_, id| ready_frame(id, &[2]), ]; for answer in answers { let error = attached(answer) @@ -666,49 +936,11 @@ fn activate_refuses_a_reply_other_than_a_valid_ready() { } } -#[cfg(all(feature = "guest", target_os = "linux", target_arch = "x86_64"))] -#[test] -fn features_report_what_the_guest_provides() { - let mut session = attached(|kind, id| { - assert_eq!(kind, APP_FEATURES); - encode_app(APP_READY, id, 0, &GuestFeatures::REQUIRED.encode()) - }); - assert_eq!( - session.features(deadline()).unwrap(), - GuestFeatures::REQUIRED - ); -} - -#[test] -fn guests_that_predate_the_features_request_provide_none() { - // This is how an agent without the request refuses an unknown request kind. - let mut session = attached(|_, id| encode_app(APP_ERROR, id, 95, b"unsupported-operation")); - assert_eq!(session.features(deadline()).unwrap(), GuestFeatures::NONE); -} - -#[test] -fn malformed_features_responses_fail_closed() { - let answers: [fn(u8, u64) -> Vec; 5] = [ - |_, id| encode_app(APP_READY, id, 0, &[1, 0]), - |_, id| encode_app(APP_ERROR, id, 16, b"busy"), - |_, id| encode_app(APP_READY, id, 1, &[0; 4]), - |_, id| encode_app(APP_READY, id ^ 1, 0, &[0; 4]), - |_, id| encode_app(APP_STDOUT, id, 0, &[0; 4]), - ]; - for answer in answers { - let mut session = attached(answer); - assert!(matches!( - session.features(deadline()), - Err(SessionError::Protocol(_)) - )); - } -} - #[test] fn host_mappings_travel_in_records_that_the_guest_acknowledges() { let (mut session, requests) = attached_recording(|kind, id| { - assert_eq!(kind, APP_MAPS); - encode_app(APP_READY, id, 0, &[]) + assert_eq!(kind, MAPS); + ready_frame(id, &[]) }); let target = format!("/{}", "t".repeat(4000)); let maps: Vec = (0..40) @@ -740,7 +972,13 @@ fn refused_host_mappings_fail_the_session() { target: "/work".to_owned(), read_only: false, }]; - let mut session = attached(|_, id| encode_app(APP_ERROR, id, 2, b"mapping-failed")); + let mut session = attached(|_, id| { + let frame = Frame::Error { + status: 2, + category: b"mapping-failed", + }; + encode(id, frame) + }); let error = session.map_host_paths(&maps, deadline()).unwrap_err(); assert!( error @@ -748,11 +986,24 @@ fn refused_host_mappings_fail_the_session() { .contains("could not mount a mapped host path (error 2)"), "{error}" ); - let answers: [fn(u8, u64) -> Vec; 4] = [ - |_, id| encode_app(APP_ERROR, id, 22, b"invalid-request"), - |_, id| encode_app(APP_ERROR, id, 95, b"unsupported-operation"), - |_, id| encode_app(APP_READY, id, 0, b"x"), - |_, id| encode_app(APP_READY, id ^ 1, 0, &[]), + let answers: [fn(u8, u64) -> Vec; 5] = [ + |_, id| { + let frame = Frame::Error { + status: 22, + category: b"invalid-request", + }; + encode(id, frame) + }, + |_, id| { + let frame = Frame::Error { + status: 95, + category: b"unsupported-operation", + }; + encode(id, frame) + }, + |_, id| encode(id, Frame::Busy), + |_, id| ready_frame(id, b"x"), + |_, id| ready_frame(id ^ 1, &[]), ]; for answer in answers { let mut session = attached(answer); @@ -766,29 +1017,20 @@ fn refused_host_mappings_fail_the_session() { #[test] fn responses_to_another_request_fail_closed() { let mismatched = "the guest agent returned a mismatched request ID"; - let answer: fn(u8, u64) -> Vec = |_, id| encode_app(APP_READY, id ^ 1, 0, &[]); - let error = attached(answer).features(deadline()).unwrap_err(); - assert_eq!(error.to_string(), mismatched); let maps = [HostMap { source: "0".to_owned(), target: "/work".to_owned(), read_only: true, }]; - let error = attached(answer) + let error = attached(|_, id| ready_frame(id ^ 1, &[])) .map_host_paths(&maps, deadline()) .unwrap_err(); assert_eq!(error.to_string(), mismatched); - let mut session = attached(|_, id| encode_app(APP_STDOUT, id ^ 1, 0, b"x")); - let workload = Workload { - argv: vec!["/bin/true".to_owned()], - timeout_ms: 0, - cwd: None, - environment: WorkloadEnvironment::Default, - }; - let request_id = session.start_exec(&workload, deadline()).unwrap(); - let error = session - .next_exec_event(request_id, Some(deadline())) - .unwrap_err(); + let mut session = attached(|_, id| encode(id ^ 1, Frame::Stdout(b"x"))); + session + .start_exec(6, &workload(&["/bin/true"]), deadline()) + .unwrap(); + let error = session.next_exec_event(6, Some(deadline())).unwrap_err(); assert_eq!(error.to_string(), mismatched); } diff --git a/nvx-rs/src/host/session/tests/managed_lifecycle.rs b/nvx-rs/src/host/session/tests/managed_lifecycle.rs index c0f0c67a4..c52b1b557 100644 --- a/nvx-rs/src/host/session/tests/managed_lifecycle.rs +++ b/nvx-rs/src/host/session/tests/managed_lifecycle.rs @@ -30,16 +30,15 @@ use std::io::{self, Write}; use std::net::{SocketAddr, TcpStream}; use std::path::{Path, PathBuf}; use std::process::{Child, Command, Stdio}; +use std::sync::atomic::{AtomicU64, Ordering}; use std::sync::mpsc; use std::thread; use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH}; use crate::host::platform::process_start_time; use crate::host::session::{self, ControlSession, ExecEvent}; -use crate::protocol::{ - self, APP_ERROR, APP_MAPS, APP_READY, CAPABILITY_LEN, ExitCategory, HostMap, Workload, - WorkloadEnvironment, -}; +use crate::protocol::v2::{self, Frame}; +use crate::protocol::{self, CAPABILITY_LEN, HostMap, Workload, WorkloadEnvironment}; /// The categories with which the guest refuses a working directory that the workload cannot /// enter, a malformed request, and a workload before it applied every host mapping: NVXC's wire @@ -91,7 +90,7 @@ impl Drop for OpenVmm { #[derive(Debug, Default)] struct Outcome { /// How it exited, and its status, if it ran. - exit: Option<(ExitCategory, i32)>, + exit: Option<(v2::Outcome, i32)>, /// The guest's error number and category, if the guest refused it. refusal: Option<(i32, String)>, /// Its standard output. @@ -103,12 +102,18 @@ struct Outcome { impl Outcome { /// Returns whether the workload exited with status 0 and printed exactly `stdout`. fn printed(&self, stdout: &str) -> bool { - self.exit == Some((ExitCategory::Exit, 0)) + self.exit == Some((v2::Outcome::Exit, 0)) && self.stdout == stdout.as_bytes() && self.stderr.is_empty() } } +/// Returns a request ID that no earlier request of the test took. +fn request_id() -> u64 { + static NEXT: AtomicU64 = AtomicU64::new(1); + NEXT.fetch_add(1, Ordering::Relaxed) +} + /// The control session of the guest that OpenVMM `pid` runs. struct Guest { /// The control endpoint. @@ -133,7 +138,7 @@ impl Guest { .expect("the control endpoint accepts the capability") } - /// Runs `argv` in `session` and returns how it ended. + /// Runs `argv` in `session`, which ends its standard input at once, and returns how it ended. fn exec( &self, session: &mut ControlSession, @@ -150,8 +155,11 @@ impl Guest { }; // The longest wait bounds the whole exchange, so the workloads must end well before it. let deadline = Instant::now() + self.timeout; - let request = session - .start_exec(&workload, deadline) + let request = request_id(); + let payload = protocol::encode_exec_payload(&workload).expect("the workload fits a frame"); + session + .start_exec(request, &payload, deadline) + .and_then(|()| session.close_stdin(request, deadline)) .expect("the guest accepts the request"); let mut outcome = Outcome::default(); loop { @@ -161,6 +169,7 @@ impl Guest { { ExecEvent::Stdout(chunk) => outcome.stdout.extend(chunk), ExecEvent::Stderr(chunk) => outcome.stderr.extend(chunk), + ExecEvent::Acknowledged(count) => panic!("{argv:?} acknowledged {count} bytes"), ExecEvent::Exit { category, status } => { outcome.exit = Some((category, status)); return outcome; @@ -395,14 +404,21 @@ fn layered_mapping_session( let send = |session: &mut ControlSession, payload: &[u8], accepted| { let deadline = Instant::now() + guest.timeout; session - .send_request(APP_MAPS, 900, payload, deadline) + .send(900, Frame::Maps(payload), Some(deadline)) .unwrap(); + let reply = session.receive(Some(deadline)).unwrap(); let expected = if accepted { - (APP_READY, 900, 0, Vec::new()) + Frame::Ready { + version: v2::VERSION, + activation: &[], + } } else { - (APP_ERROR, 900, 22, INVALID_REQUEST.as_bytes().to_vec()) + Frame::Error { + status: 22, + category: INVALID_REQUEST.as_bytes(), + } }; - assert_eq!(session.read_response(Some(deadline)).unwrap(), expected); + assert_eq!(Frame::decode(&reply).unwrap(), (900, expected)); }; if count == 0 { send(&mut session, &parts[0], false); @@ -461,16 +477,16 @@ fn layered_mapping_session( ); assert!(mapped.printed("sl76b-real-export\n"), "{path}: {mapped:?}"); } - let request = session - .start_exec( - &Workload { - argv: vec!["/bin/sleep".to_owned(), "30".to_owned()], - timeout_ms: 0, - cwd: None, - environment: WorkloadEnvironment::Default, - }, - Instant::now() + guest.timeout, - ) + let request = request_id(); + let payload = protocol::encode_exec_payload(&Workload { + argv: vec!["/bin/sleep".to_owned(), "30".to_owned()], + timeout_ms: 0, + cwd: None, + environment: WorkloadEnvironment::Default, + }) + .unwrap(); + session + .start_exec(request, &payload, Instant::now() + guest.timeout) .unwrap(); session .cancel_exec(request, Instant::now() + guest.timeout) @@ -480,7 +496,7 @@ fn layered_mapping_session( .next_exec_event(request, Some(Instant::now() + guest.timeout)) .unwrap(), ExecEvent::Exit { - category: ExitCategory::Cancelled, + category: v2::Outcome::Cancelled, status: 137 } )); @@ -647,7 +663,7 @@ fn a_managed_guest_runs_request_scoped_workloads_until_it_stops() { ); assert_eq!( timed_out.exit, - Some((ExitCategory::Timeout, 124)), + Some((v2::Outcome::Timeout, 124)), "{timed_out:?}" ); let after_timeout = guest.exec( diff --git a/nvx-rs/src/protocol/app.rs b/nvx-rs/src/protocol/app.rs index 6ca904452..3c267741a 100644 --- a/nvx-rs/src/protocol/app.rs +++ b/nvx-rs/src/protocol/app.rs @@ -6,12 +6,17 @@ //! of their request. This module holds version 1, and `v2` version 2, which carries every control //! path and streams an execution's standard input, output and error. +#[cfg(any( + feature = "testing", + all(feature = "guest", target_os = "linux", target_arch = "x86_64") +))] +use super::array; #[cfg(any( feature = "cli", feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] -use super::{ProtocolError, array, violation}; +use super::{ProtocolError, violation}; #[cfg(any( feature = "cli", all(feature = "guest", target_os = "linux", target_arch = "x86_64") @@ -44,6 +49,7 @@ pub(crate) mod workload { } #[cfg(any( + feature = "cli", feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] @@ -51,14 +57,12 @@ pub(crate) mod v2; /// Magic of every application frame. #[cfg(any( - feature = "cli", feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] pub(crate) const APP_MAGIC: [u8; 4] = *b"NVXC"; /// Version of the application frame format. #[cfg(any( - feature = "cli", feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] @@ -73,28 +77,24 @@ pub(crate) const APP_HEADER_LEN: usize = 24; /// Request that the guest agent acknowledge readiness with [`APP_READY`]. #[cfg(any( - feature = "cli", feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] pub(crate) const APP_PING: u8 = 1; /// Request that the guest agent run a workload; the payload is an `EXEC` payload. #[cfg(any( - feature = "cli", feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] pub(crate) const APP_EXEC: u8 = 2; /// Request that the guest agent shut the VM down, which it acknowledges with [`APP_STOPPED`]. #[cfg(any( - feature = "cli", feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] pub(crate) const APP_STOP: u8 = 3; /// Request that the guest agent terminate the workload of the request with the frame's ID. #[cfg(any( - feature = "cli", feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] @@ -102,7 +102,6 @@ pub(crate) const APP_CANCEL: u8 = 4; /// Request for the guest's control features, which the guest agent answers with [`APP_READY`] /// and a [`GuestFeatures`](super::GuestFeatures) payload. #[cfg(any( - feature = "cli", feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] @@ -110,40 +109,30 @@ pub(crate) const APP_FEATURES: u8 = 5; /// Request that the guest agent mount entries of the host mapping table; the payload is a `MAPS` /// payload. #[cfg(any( - feature = "cli", feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] pub(crate) const APP_MAPS: u8 = 6; /// Request that nvx-init capture the VM after optionally killing its executions. -#[cfg(any( - feature = "cli", - all(feature = "guest", target_os = "linux", target_arch = "x86_64") -))] +#[cfg(all(feature = "guest", target_os = "linux", target_arch = "x86_64"))] pub(crate) const APP_CAPTURE: u8 = 7; /// Request that nvx-init activate a restored VM with its record and workload identity. -#[cfg(any( - feature = "cli", - all(feature = "guest", target_os = "linux", target_arch = "x86_64") -))] +#[cfg(all(feature = "guest", target_os = "linux", target_arch = "x86_64"))] pub(crate) const APP_ACTIVATE: u8 = 8; /// Response that acknowledges a request. #[cfg(any( - feature = "cli", feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] pub(crate) const APP_READY: u8 = 0x81; /// Response that carries a chunk of a workload's standard output. #[cfg(any( - feature = "cli", feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] pub(crate) const APP_STDOUT: u8 = 0x82; /// Response that carries a chunk of a workload's standard error. #[cfg(any( - feature = "cli", feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] @@ -151,14 +140,12 @@ pub(crate) const APP_STDERR: u8 = 0x83; /// Response that ends a workload: the status is its exit status, and the payload its /// [`ExitCategory`]. #[cfg(any( - feature = "cli", feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] pub(crate) const APP_EXIT: u8 = 0x84; /// Response that acknowledges [`APP_STOP`]. #[cfg(any( - feature = "cli", feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] @@ -166,7 +153,6 @@ pub(crate) const APP_STOPPED: u8 = 0x85; /// Response that refuses a request: the status is an error number, and the payload an error /// category such as [`LAUNCH_FAILED`]. #[cfg(any( - feature = "cli", feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] @@ -174,7 +160,6 @@ pub(crate) const APP_ERROR: u8 = 0xff; /// Error category with which a guest agent refuses a request kind that it does not know. #[cfg(any( - feature = "cli", feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] @@ -222,7 +207,6 @@ pub(crate) const MAPPING_FAILED: &str = "mapping-failed"; /// Decoded application frame. #[cfg(any( - feature = "cli", feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] @@ -240,7 +224,6 @@ pub(crate) struct AppFrame<'a> { /// Encodes one application frame. #[cfg(any( - feature = "cli", feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] @@ -260,7 +243,6 @@ pub(crate) fn encode_app(kind: u8, request_id: u64, status: i32, payload: &[u8]) /// Decodes and validates an application frame. #[cfg(any( - feature = "cli", feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] @@ -386,6 +368,13 @@ pub(crate) enum ExitCategory { impl ExitCategory { /// Parses the payload of an [`APP_EXIT`] response. #[cfg(feature = "cli")] + #[cfg_attr( + not(all(test, feature = "testing")), + expect( + dead_code, + reason = "NVX's host speaks NVXC version 2 since SL4.4c, and SL4.4d deletes version 1" + ) + )] pub(crate) fn parse(payload: &[u8]) -> Result { match payload { b"exit" => Ok(Self::Exit), diff --git a/nvx-rs/src/protocol/exec.rs b/nvx-rs/src/protocol/exec.rs index 2795f2cf9..189b7f849 100644 --- a/nvx-rs/src/protocol/exec.rs +++ b/nvx-rs/src/protocol/exec.rs @@ -30,6 +30,10 @@ pub(crate) const MAX_GUEST_CWD_BYTES: usize = MAX_ARGUMENT_BYTES; /// Largest number of workload environment entries. pub(crate) const MAX_ENVIRONMENT: usize = 256; /// Largest combined stdout and stderr volume the guest agent forwards for one execution. +#[cfg(any( + feature = "testing", + all(feature = "guest", target_os = "linux", target_arch = "x86_64") +))] pub(crate) const MAX_OUTPUT_BYTES: usize = 1024 * 1024; /// Extension field of a payload that carries flags, a working directory, and an environment. diff --git a/nvx-rs/src/protocol/features.rs b/nvx-rs/src/protocol/features.rs index 1c8e95e9f..351f004ce 100644 --- a/nvx-rs/src/protocol/features.rs +++ b/nvx-rs/src/protocol/features.rs @@ -1,4 +1,17 @@ -//! The control features that a guest image advertises in response to `APP_FEATURES`. +//! The control features that a guest image advertises in response to `APP_FEATURES`, which only +//! version 1 of NVXC has. +//! +//! NVX's host asked for them before it spoke version 2, whose Ready names the protocol version +//! instead. The guest agents and the fake OpenVMM still advertise them to version 1's `FEATURES` +//! until SL4.4d deletes them, and with them the host's side, which no host path uses any longer: +//! only some builds of the tests use it, so no expectation of dead code holds in every build. +#![cfg_attr( + feature = "cli", + allow( + dead_code, + reason = "NVX's host sends no FEATURES since SL4.4c, and SL4.4d deletes them" + ) +)] use std::ops::BitOr; @@ -8,8 +21,8 @@ use super::{ProtocolError, array, violation}; /// Control features of a guest image, advertised in the response to `APP_FEATURES`. /// /// The Rust guest agent uses these shared definitions and also speaks for the features of its -/// image's init script. A host drives a sandbox only when its guest provides every feature that -/// the host depends on, such as [`LAYERED_SANDBOX`](Self::LAYERED_SANDBOX) for `nvx-rs sandbox`; +/// image's init script. A host drove a sandbox only when its guest provided every feature that +/// the host depended on, such as [`LAYERED_SANDBOX`](Self::LAYERED_SANDBOX) for `nvx-rs sandbox`; /// an older image would silently ignore the policy or the request that depends on the missing /// feature. #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -54,15 +67,15 @@ impl GuestFeatures { #[cfg(feature = "testing")] pub(crate) const SANDBOX_AGENT: Self = Self(Self::CANCEL.0 | Self::EXEC_ENVIRONMENT.0 | Self::HOST_MAPPING_TABLE.0); - /// The features that `nvx-rs sandbox` depends on in the layered sandbox root: + /// The features that `nvx-rs sandbox` depended on in the layered sandbox root while it spoke + /// version 1: /// [`EXEC_ENVIRONMENT`](Self::EXEC_ENVIRONMENT), because `exec` sends a working directory and /// an environment in an extended `EXEC`, and /// [`HOST_MAPPING_TABLE`](Self::HOST_MAPPING_TABLE), because `start` sends the mappings of /// the sandbox's shares as `MAPS` requests. /// - /// It holds no [`CANCEL`](Self::CANCEL), because the CLI sends no `APP_CANCEL`: as `nvx.py` - /// did, Ctrl-C ends an `exec` by closing the session, and the guest agent ends the workload - /// when OpenVMM resets the session that it ran for. It holds no + /// It holds no [`CANCEL`](Self::CANCEL), because the CLI sent no `APP_CANCEL` before it spoke + /// version 2: as `nvx.py` did, Ctrl-C ended an `exec` by closing the session. It holds no /// [`WORKLOAD_ACCOUNT`](Self::WORKLOAD_ACCOUNT), because no sandbox launch requests account /// creation, which the layered root neither performs nor announces (microsoft/nvx#566). #[cfg(feature = "cli")] diff --git a/nvx-rs/src/protocol/mod.rs b/nvx-rs/src/protocol/mod.rs index e9ae7e0f7..39b748864 100644 --- a/nvx-rs/src/protocol/mod.rs +++ b/nvx-rs/src/protocol/mod.rs @@ -62,15 +62,9 @@ pub mod status; mod tests; pub mod view; -#[cfg(any( - feature = "cli", - all(feature = "guest", target_os = "linux", target_arch = "x86_64") -))] +#[cfg(all(feature = "guest", target_os = "linux", target_arch = "x86_64"))] pub(crate) use app::APP_ACTIVATE; -#[cfg(any( - feature = "cli", - all(feature = "guest", target_os = "linux", target_arch = "x86_64") -))] +#[cfg(all(feature = "guest", target_os = "linux", target_arch = "x86_64"))] pub(crate) use app::APP_CAPTURE; #[cfg(any( feature = "cli", @@ -80,13 +74,20 @@ pub(crate) use app::APP_CAPTURE; pub(crate) use app::APP_HEADER_LEN; #[cfg(all(feature = "guest", target_os = "linux", target_arch = "x86_64"))] pub(crate) use app::AppFrame; -#[cfg(any(feature = "cli", feature = "testing"))] +#[cfg(feature = "testing")] pub(crate) use app::ExitCategory; +#[cfg(any( + feature = "cli", + feature = "testing", + all(feature = "guest", target_os = "linux", target_arch = "x86_64") +))] +pub(crate) use app::MAPPING_FAILED; #[cfg(feature = "cli")] pub(crate) use app::encode_activate_payload; #[cfg(feature = "cli")] pub(crate) use app::encode_capture_payload; #[cfg(any( + feature = "cli", feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] @@ -97,14 +98,12 @@ pub(crate) use app::v2; ))] pub(crate) use app::workload; #[cfg(any( - feature = "cli", feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] pub(crate) use app::{ APP_CANCEL, APP_ERROR, APP_EXEC, APP_EXIT, APP_FEATURES, APP_MAPS, APP_PING, APP_READY, - APP_STDERR, APP_STDOUT, APP_STOP, APP_STOPPED, MAPPING_FAILED, UNSUPPORTED_OPERATION, - decode_app, encode_app, + APP_STDERR, APP_STDOUT, APP_STOP, APP_STOPPED, UNSUPPORTED_OPERATION, decode_app, encode_app, }; #[cfg(any( feature = "testing", @@ -115,7 +114,6 @@ pub(crate) use app::{BUSY, CWD_FAILED, INVALID_REQUEST, LAUNCH_FAILED, MAPPINGS_ pub(crate) use app::{decode_activate_payload, decode_capture_payload}; pub use boot::{MAPS_TOKEN, parse_host_mapping_count}; #[cfg(any( - feature = "cli", feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] @@ -127,7 +125,6 @@ pub(crate) use exec::{GuestExecPayload, decode_guest_exec_payload}; #[cfg(feature = "cli")] pub(crate) use exec::{Workload, WorkloadEnvironment, encode_exec_payload}; #[cfg(any( - feature = "cli", feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] diff --git a/nvx-rs/src/protocol/tests.rs b/nvx-rs/src/protocol/tests.rs index 215eba5f5..dc10cc73f 100644 --- a/nvx-rs/src/protocol/tests.rs +++ b/nvx-rs/src/protocol/tests.rs @@ -46,7 +46,12 @@ use crate::tests::{NVXC_V1, of_kind, vector}; feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] -use super::app::{APP_HEADER_LEN, APP_VERSION, AppFrame}; +use super::app::APP_HEADER_LEN; +#[cfg(any( + feature = "testing", + all(feature = "guest", target_os = "linux", target_arch = "x86_64") +))] +use super::app::{APP_VERSION, AppFrame}; #[cfg(all( feature = "cli", feature = "testing", @@ -87,7 +92,6 @@ const MAX_CWD_BYTES: usize = 4095; const MAX_TIMEOUT_MS: u32 = u32::MAX; #[cfg(any( - feature = "cli", feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] @@ -143,7 +147,6 @@ fn record_type(name: &str) -> RecordType { /// Returns the frame kind that a vector names. #[cfg(any( - feature = "cli", feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] @@ -248,7 +251,6 @@ fn records_match_their_vectors() { /// Each `FRAME` vector decodes to its values, which encode to its bytes. #[cfg(any( - feature = "cli", feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] @@ -757,7 +759,6 @@ fn exec_requests_are_refused_where_the_guest_agent_refuses_them() { } #[cfg(any( - feature = "cli", feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] @@ -1046,7 +1047,6 @@ fn maps_decoders_refuse_the_invalid_vectors() { // them yet breaks compatibility with released peers. These tests pin each value literally. #[cfg(any( - feature = "cli", feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] diff --git a/nvx-rs/tests/cli.rs b/nvx-rs/tests/cli.rs index bd3c65dc6..5066fd14a 100644 --- a/nvx-rs/tests/cli.rs +++ b/nvx-rs/tests/cli.rs @@ -121,62 +121,31 @@ fn text(data: &[u8]) -> String { } #[test] -fn start_requires_the_guest_features_of_layered_sandboxes() { +fn starts_ask_the_guest_for_no_features() { + // NVXC version 2 has no FEATURES: a guest that refuses that request of version 1, or that + // advertises none of the features, starts, runs workloads and stops all the same. let root = Root::new(); - // A guest that lacks per-execution environments or host path mapping tables, or that - // predates feature negotiation, is stopped, and its start fails like any other. - for (state, command_line, missing) in [ - ( - "cancel-only", - "fake_guest_features=1", - "per-execution environments, host path mapping tables", - ), - ( - "legacy", - "fake_legacy_guest=1", - "per-execution environments, host path mapping tables", - ), - ( - "environments-only", - "fake_guest_features=16", - "host path mapping tables", - ), + for (state, command_line) in [ + ("legacy", "fake_legacy_guest=1"), + ("featureless", "fake_guest_features=0"), ] { let output = run(&root, &provision(&root, state, command_line)); assert!(output.status.success(), "{}", text(&output.stderr)); let output = run(&root, &lifecycle("start", &root, state)); - assert_eq!(output.status.code(), Some(1)); - assert_eq!( - text(&output.stderr), - format!( - "error[guest-features-missing]: the guest image lacks control features that \ - layered sandboxes need ({missing}); use an NVX release or build that includes \ - them\n" - ) - ); - let mut names: Vec = fs::read_dir(root.path.join(state)) - .unwrap() - .map(|entry| entry.unwrap().file_name().into_string().unwrap()) - .collect(); - names.sort(); - assert_eq!(names, ["config.json", "openvmm.log", "sandbox.lock"]); - let output = run(&root, &lifecycle("deprovision", &root, state)); - assert!(output.status.success(), "{}", text(&output.stderr)); - } - // Per-execution environments and host path mapping tables are all that the CLI needs: - // neither cancellation nor workload accounts. - let output = run( - &root, - &provision(&root, "environments", "fake_guest_features=80"), - ); - assert!(output.status.success(), "{}", text(&output.stderr)); - for operation in ["start", "stop", "deprovision"] { - let output = run(&root, &lifecycle(operation, &root, "environments")); - assert!( - output.status.success(), - "{operation}: {}", - text(&output.stderr) - ); + assert!(output.status.success(), "{state}: {}", text(&output.stderr)); + let mut exec = lifecycle("exec", &root, state); + exec.extend(["--entrypoint", "/bin/echo", "--arg", "ran"].map(str::to_owned)); + let output = run(&root, &exec); + assert!(output.status.success(), "{state}: {}", text(&output.stderr)); + assert_eq!(text(&output.stdout), "ran\n"); + for operation in ["stop", "deprovision"] { + let output = run(&root, &lifecycle(operation, &root, state)); + assert!( + output.status.success(), + "{state} {operation}: {}", + text(&output.stderr) + ); + } } } @@ -3004,6 +2973,64 @@ mod interrupts { } } + #[test] + fn interrupting_an_exec_cancels_the_workload_that_streams() { + use std::io::{BufRead, BufReader, Write}; + + let root = Root::new(); + for arguments in [ + provision(&root, "state", ""), + lifecycle("start", &root, "state"), + ] { + let output = run(&root, &arguments); + assert!(output.status.success(), "{}", text(&output.stderr)); + } + let mut arguments = lifecycle("exec", &root, "state"); + arguments + .extend(["--entrypoint", "/bin/sh", "--arg=-c", "--arg", "cat"].map(str::to_owned)); + let mut child = Command::new(NVX_RS) + .arg("--root") + .arg(&root.path) + .args(&arguments) + .current_dir(&root.path) + .env_remove("NVX_OPENVMM") + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .unwrap(); + // The workload echoes its input as it reads it, which stays open. + let mut input = child.stdin.take().unwrap(); + input.write_all(b"streamed\n").unwrap(); + let mut echoed = String::new(); + BufReader::new(child.stdout.take().unwrap()) + .read_line(&mut echoed) + .unwrap(); + assert_eq!(echoed, "streamed\n"); + let interrupted = Instant::now(); + let pid = nix::unistd::Pid::from_raw(i32::try_from(child.id()).unwrap()); + nix::sys::signal::kill(pid, nix::sys::signal::Signal::SIGINT).unwrap(); + let output = child.wait_with_output().unwrap(); + assert_eq!(output.status.code(), Some(130), "{}", text(&output.stderr)); + assert_eq!(text(&output.stderr), "Interrupted\n"); + assert!(interrupted.elapsed() < Duration::from_secs(10)); + drop(input); + // The guest ended the workload, so the sandbox runs another, and stops. + let mut echo = lifecycle("exec", &root, "state"); + echo.extend(["--entrypoint", "/bin/echo", "--arg", "again"].map(str::to_owned)); + let output = run(&root, &echo); + assert!(output.status.success(), "{}", text(&output.stderr)); + assert_eq!(text(&output.stdout), "again\n"); + for operation in ["stop", "deprovision"] { + let output = run(&root, &lifecycle(operation, &root, "state")); + assert!( + output.status.success(), + "{operation}: {}", + text(&output.stderr) + ); + } + } + #[test] fn interrupting_a_provision_that_waits_for_a_download_provisions_nothing() { use std::io::{BufRead, BufReader, Read}; @@ -3088,8 +3115,8 @@ mod interrupts { #[test] fn interrupting_an_exec_ends_its_write_to_a_full_pipe() { - // The CLI writes a workload's output once the workload ends. Like Python's, the write - // ends at Ctrl-C, also while it waits for a pipe's reader that does not read. + // The CLI writes a workload's output as it comes. Like Python's, the write ends at + // Ctrl-C, also while it waits for a pipe's reader that does not read. let root = Root::new(); for arguments in [ provision(&root, "state", ""), diff --git a/nvx-rs/tests/cli/harness.rs b/nvx-rs/tests/cli/harness.rs index 1470a1cf5..e86fb5ecb 100644 --- a/nvx-rs/tests/cli/harness.rs +++ b/nvx-rs/tests/cli/harness.rs @@ -381,6 +381,36 @@ impl Root { Outcome::new(arguments, output) } + /// Runs `nvx-rs --root ROOT ARGUMENTS` in the root, as [`nvx_rs`](Self::nvx_rs) does, but with + /// `input` as its standard input, which then ends. + pub(crate) fn nvx_rs_with_input>( + &self, + arguments: &[S], + input: &[u8], + ) -> Outcome { + let arguments: Vec = arguments + .iter() + .map(|argument| self.expand(argument.as_ref())) + .collect(); + let mut child = self + .command(&arguments) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .unwrap(); + let mut stdin = child.stdin.take().unwrap(); + let input = input.to_vec(); + // A command that ends before it reads its whole input breaks the pipe, which the run's + // outcome then shows. + let writer = std::thread::spawn(move || { + let _ = std::io::Write::write_all(&mut stdin, &input); + }); + let output = child.wait_with_output().unwrap(); + writer.join().unwrap(); + Outcome::new(arguments, output) + } + /// Runs `nvx-rs --root ROOT ARGUMENTS` in the root, as [`nvx_rs`](Self::nvx_rs) does, but /// with the umask `umask`, rather than this process's, which the tests running in parallel /// share. diff --git a/nvx-rs/tests/cli/lifecycle.rs b/nvx-rs/tests/cli/lifecycle.rs index 07bfee820..c052ea250 100644 --- a/nvx-rs/tests/cli/lifecycle.rs +++ b/nvx-rs/tests/cli/lifecycle.rs @@ -787,7 +787,7 @@ struct Execution { status: i32, /// What the workload wrote to standard output. stdout: String, - /// What the workload wrote to standard error. + /// What the workload wrote to standard error, or the guest agent before it refused it. stderr: &'static str, /// The code of the error with which `exec` refuses the workload, if it does, and what the /// error names. @@ -900,6 +900,15 @@ fn executions() -> Vec { "", ); missing.stderr = "openvmm: /bin/missing: not found\n"; + // The guest agent's diagnostic streams before its refusal. + let mut cwd_refused = refused( + "missing working directory", + &["--cwd", "/missing"], + "exec-refused", + vec!["cwd-failed"], + ); + cwd_refused.stderr = + "nvx-managed-agent: cannot enter working directory /missing: No such file or directory\n"; let mut existing = refused( "existing outcome report", &["--outcome-report", "{root/reports/existing.json}"], @@ -988,12 +997,7 @@ fn executions() -> Vec { "", ), missing, - refused( - "missing working directory", - &["--cwd", "/missing"], - "exec-refused", - vec!["cwd-failed"], - ), + cwd_refused, refused( "relative working directory", &["--cwd", "relative"], @@ -1052,7 +1056,7 @@ fn managed_execs_exit_with_the_workload_status() { } let before = root.snapshot(); let started = Instant::now(); - let outcome = root.nvx_rs(&sandbox("exec", "{root/state}", &case.options)); + let mut outcome = root.nvx_rs(&sandbox("exec", "{root/state}", &case.options)); eprintln!("case: {}", case.name); match &case.refusal { Some((code, fragments)) => { @@ -1061,6 +1065,14 @@ fn managed_execs_exit_with_the_workload_status() { .map(|fragment| root.expand(fragment)) .collect(); let fragments: Vec<&str> = fragments.iter().map(String::as_str).collect(); + // What the guest wrote before it refused the workload streams before the error. + let streamed = case.stderr.as_bytes(); + assert!( + outcome.stderr.starts_with(streamed), + "{}", + outcome.describe() + ); + outcome.stderr.drain(..streamed.len()); outcome.assert_error(code, &fragments); // A refusal changes nothing, but the fake OpenVMM's log of the connections to its // control endpoint, and the workload did not run. @@ -1098,6 +1110,51 @@ fn managed_execs_exit_with_the_workload_status() { .assert_silent_success(); } +#[test] +fn managed_execs_stream_their_standard_input_and_output() { + let root = Root::new(); + root.nvx_rs(&provision("{root/state}", &[])) + .assert_silent_success(); + root.nvx_rs(&sandbox::<&str>("start", "{root/state}", &[])) + .assert_silent_success(); + // The workload reads more input than the window of 64 KiB holds, until its end, and echoes + // it as it reads it. + let input: Vec = (0..200 * 1024).map(|index| (index % 251) as u8).collect(); + let cat = [ + "--entrypoint", + "/bin/sh", + "--arg=-c", + "--arg", + "cat; echoerr ended", + ]; + let echoed = root.nvx_rs_with_input(&sandbox("exec", "{root/state}", &cat), &input); + assert_eq!(echoed.status, Some(0), "{}", echoed.describe()); + assert!(echoed.stdout == input, "{}", echoed.describe()); + assert_eq!(echoed.stderr(), "ended\n"); + // Output passes the old cap of 1 MiB whole. + let flood = root.nvx_rs(&sandbox( + "exec", + "{root/state}", + &[ + "--entrypoint", + "/bin/sh", + "--arg=-c", + "--arg", + "flood 2000000", + ], + )); + assert_eq!(flood.status, Some(0), "{}", flood.describe()); + assert!( + flood.stdout == vec![b'x'; 2_000_000], + "{}", + flood.describe() + ); + root.nvx_rs(&sandbox::<&str>("stop", "{root/state}", &[])) + .assert_silent_success(); + root.nvx_rs(&sandbox::<&str>("deprovision", "{root/state}", &[])) + .assert_silent_success(); +} + /// Runs the lifecycle operation `operation` of the sandbox in `{root/STATE}` with `options`, and /// returns how it ended. fn operate(root: &Root, operation: &str, state: &str, options: &[&str]) -> Outcome { diff --git a/nvx-rs/tests/cli/sandbox_run.rs b/nvx-rs/tests/cli/sandbox_run.rs index 8ed207373..53b2c535d 100644 --- a/nvx-rs/tests/cli/sandbox_run.rs +++ b/nvx-rs/tests/cli/sandbox_run.rs @@ -464,6 +464,25 @@ fn a_run_whose_guest_exits_during_startup_fails_as_a_start_does() { ); } +/// A run streams its workload's standard input, which the command's ends, and its output. +#[test] +fn a_run_streams_its_standard_input_and_output() { + let root = Root::new(); + let before = root.snapshot(); + let outcome = root.nvx_rs_with_input( + &sandbox_run(&[ + "--entrypoint", + "/bin/sh", + "--arg=-c", + "--arg", + "cat; echoerr ended", + ]), + b"line one\nline two\n", + ); + assert_ran(&root, &before, &outcome, 0, "line one\nline two\n"); + assert_eq!(outcome.stderr(), "ended\n", "{}", outcome.describe()); +} + #[test] fn a_run_whose_workload_the_guest_refuses_stops_its_vm() { let root = Root::new(); @@ -477,11 +496,13 @@ fn a_run_whose_workload_the_guest_refuses_stops_its_vm() { "{root/reports/run.json}", ])); assert_ran(&root, &before, &outcome, 1, ""); + // The guest agent's diagnostic streams before the refusal, which the run reports. assert_eq!( outcome.stderr(), format!( - "error[exec-refused]: managed guest rejected exec (status=2, category=cwd-failed)\ - {LINE_END}" + "nvx-managed-agent: cannot enter working directory /missing: No such file or \ + directory\nerror[exec-refused]: managed guest rejected exec (status=2, \ + category=cwd-failed){LINE_END}" ) ); assert_outcome_report( @@ -557,6 +578,52 @@ fn a_run_forces_down_a_vm_that_does_not_stop() { ); } +/// A run's own warnings start a line of their own after standard error that its workload left +/// without a line end, which the run otherwise leaves as it is. +#[test] +fn a_runs_warnings_start_lines_of_their_own_after_its_workloads_output() { + let root = Root::new(); + let before = root.snapshot(); + let outcome = root.nvx_rs(&sandbox_run(&[ + "--entrypoint", + "/bin/sh", + "--arg=-c", + "--arg", + "printerr partial", + "--timeout", + "1", + "--cmdline", + "fake_ignore_stop=1", + ])); + assert_ran(&root, &before, &outcome, 0, ""); + let stderr = outcome.stderr(); + assert!( + stderr.starts_with(&format!( + "partial{LINE_END}warning: managed guest shutdown failed: " + )), + "{}", + outcome.describe() + ); + assert!( + stderr.ends_with(&format!( + "{LINE_END}warning: OpenVMM was forcibly terminated after the run{LINE_END}" + )), + "{}", + outcome.describe() + ); + let root = Root::new(); + let before = root.snapshot(); + let outcome = root.nvx_rs(&sandbox_run(&[ + "--entrypoint", + "/bin/sh", + "--arg=-c", + "--arg", + "printerr partial", + ])); + assert_ran(&root, &before, &outcome, 0, ""); + assert_eq!(outcome.stderr(), "partial", "{}", outcome.describe()); +} + /// A run forwards an outcome report that the caller can read whatever its umask, even one that /// clears the owner's own permissions, and whose group's and others' permissions the caller's /// umask narrows: OpenVMM starts with the caller's umask without the owner's bits, and the run diff --git a/scripts/nvx_tools/sandbox_lifecycle_tests.py b/scripts/nvx_tools/sandbox_lifecycle_tests.py index ffc35e348..7db444167 100644 --- a/scripts/nvx_tools/sandbox_lifecycle_tests.py +++ b/scripts/nvx_tools/sandbox_lifecycle_tests.py @@ -51,12 +51,9 @@ MEMORY_MIB = 256 # An identity that the Ubuntu layer's user database lacks. UNAVAILABLE_IDENTITY = "4242:4242" -# The guest agent forwards at most this much output for one exec request, and -# reads it in chunks of at most OUTPUT_CHUNK_BYTES. It drops the chunk that -# would exceed the bound, so more than OUTPUT_LIMIT_BYTES - OUTPUT_CHUNK_BYTES -# bytes reach the host. -OUTPUT_LIMIT_BYTES = 1024 * 1024 -OUTPUT_CHUNK_BYTES = 32 * 1024 +# More output than the 1 MiB that version 1 of NVXC forwarded for one exec +# request, which version 2 streams whole. +STREAMED_OUTPUT_BYTES = 2 * 1024 * 1024 STATE_PATH = "/tmp/nvx lifecycle state" STATE_ARGUMENTS = (" leading and trailing ", "tab\tseparated", "line one\nline two") STATE_CONTENT = "".join(f"{argument}\n" for argument in STATE_ARGUMENTS).encode() @@ -831,31 +828,28 @@ def check_security_profile(self) -> None: ) def check_bounded_results(self) -> None: - # The agent stops forwarding output at its bound, and the outcome + # The host receives all of the workload's output, and the outcome # report carries only the operation, category, and status. - report = self.root / "output-limit-outcome.json" + report = self.root / "streamed-output-outcome.json" result = self.exec( "/usr/bin/head", "--arg=-c", - f"--arg={2 * OUTPUT_LIMIT_BYTES}", + f"--arg={STREAMED_OUTPUT_BYTES}", "--arg=/dev/zero", "--outcome-report", str(report), - expected=125, ) if ( - not OUTPUT_LIMIT_BYTES - OUTPUT_CHUNK_BYTES - < len(result.stdout) - <= OUTPUT_LIMIT_BYTES + len(result.stdout) != STREAMED_OUTPUT_BYTES or result.stdout.strip(b"\0") or result.stderr ): raise RuntimeError( - "the output limit forwarded " + "the streamed output forwarded " f"{len(result.stdout)} stdout and {len(result.stderr)} stderr bytes" ) - read_exec_outcome(report, category="output-limit", status_code=125) - self.copy_evidence(report, "output-limit-outcome.json") + read_exec_outcome(report, category="exit", status_code=0) + self.copy_evidence(report, "streamed-output-outcome.json") # An existing report path fails the request before the workload runs. existing = self.root / "existing-outcome.json" diff --git a/scripts/test-layout-allowlist.json b/scripts/test-layout-allowlist.json index 6ba886ddc..bf3399869 100644 --- a/scripts/test-layout-allowlist.json +++ b/scripts/test-layout-allowlist.json @@ -12,7 +12,7 @@ "nvx-rs/src/host/platform/windows/mod.rs": 1, "nvx-rs/src/host/platform/windows/process.rs": 2, "nvx-rs/src/host/platform/windows/spawn.rs": 1, - "nvx-rs/src/host/session.rs": 5, - "nvx-rs/src/host/vm.rs": 2 + "nvx-rs/src/host/vm.rs": 2, + "nvx-rs/src/protocol/app.rs": 1 } } diff --git a/scripts/test_microvm_tests.py b/scripts/test_microvm_tests.py index ef0df03ac..f48e54f8d 100644 --- a/scripts/test_microvm_tests.py +++ b/scripts/test_microvm_tests.py @@ -1311,12 +1311,14 @@ def _exec( + b"NVX-UBUNTU-SANDBOX-OK uid=65534 gid=65534\n" ) elif entrypoint == "/usr/bin/head": - size = sandbox_lifecycle_tests.OUTPUT_LIMIT_BYTES - if "unbounded-output" in self.faults: - size = int(arguments[1]) + size = int(arguments[1]) + if "capped-output" in self.faults: + # The guest killed the workload at version 1's bound. + returncode, category = 125, "output-limit" + size = 1024 * 1024 elif "short-output" in self.faults: - size -= sandbox_lifecycle_tests.OUTPUT_CHUNK_BYTES - returncode, category, stdout = 125, "output-limit", bytes(size) + size -= 32 * 1024 + stdout = bytes(size) elif entrypoint != "/bin/true": raise AssertionError(f"unexpected workload {entrypoint}") if report is not None: @@ -1640,7 +1642,7 @@ def test_acceptance_drives_the_complete_public_lifecycle(self): "sandbox-lifecycle-identity-outcome.json", "sandbox-lifecycle-runtime.json", "sandbox-lifecycle-probe.log", - "sandbox-lifecycle-output-limit-outcome.json", + "sandbox-lifecycle-streamed-output-outcome.json", "sandbox-lifecycle-outcome.json", "sandbox-lifecycle-openvmm.log", }, @@ -1713,8 +1715,8 @@ def test_acceptance_detects_lifecycle_regressions(self): "duplicate-start": "start returned 0, expected 1", "exec-loses-state": "the state-reading request returned status 1", "probe-fails": "exec returned 1, expected 0", - "unbounded-output": "the output limit forwarded 2097152 stdout", - "short-output": "the output limit forwarded 1015808 stdout", + "capped-output": "exec returned 125, expected 0", + "short-output": "the streamed output forwarded 2064384 stdout", "overwrite-report": "exec returned 0, expected 1", "stop-leaks-capability": "after stop: the state directory holds", "stop-leaks-process": "OpenVMM still runs after stop",