From 4a41a9c89ded3205a861d2565751ac33b3c6d649 Mon Sep 17 00:00:00 2001 From: Pedro Henrique Penna Date: Thu, 12 Nov 2026 07:08:20 -0800 Subject: [PATCH 1/2] nvx-rs: delete NVXC v1 and FEATURES Since SL4.4c, every NVX control path speaks NVXC v2, so version 1's frames, its FEATURES request and GuestFeatures were compatibility code that only the guest agents and the fake OpenVMM still served. SL4.4d (#1129) deletes them, and v2 is NVXC's only version. The protocol loses v1's magic and version, APP_FEATURES, the v1 codec (encode_app and decode_app), ExitCategory, GuestFeatures with its module, and MAX_OUTPUT_BYTES, the 1 MiB cap that only v1 applied, with the dead-code markers that SL4.4c left on them. The kind codes and error categories that v2 shares stay, since the guest loop and the fake still name v2's requests and answers with them; their gates narrow to the builds that still use them. The guest loop that nvx-init and the managed agent share decodes v2 alone: a frame of another version fails as a malformed request does, which ends the session with the data-application diagnostic. The session's per-request version, the FEATURES handler, v1's output cap and null standard input, and the BUSY answer to a v1 STOP during an execution go. The discard of an execution's output, which only the cap used, goes with nvx-init's Exec::discard. The fake OpenVMM serves v2 alone: its v1 handler, its output cap, its feature state and the fake_guest_features and fake_legacy_guest tokens go, and a frame of another version ends its session. The guest's dispatch, session and runtime test harnesses speak v2, the tests of v1 alone retire, and new tests show that the guest agent and the fake end the session of a v1 peer. tests/vm_lifecycle.rs and the live restore test speak v2's frames to nvx-init. The protocol's docs, the design of the control interface, the guest-agent guidelines, the vectors' README and the case matrix follow. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- doc/acceptance-case-matrix.md | 2 +- .../4.3-isolation-and-resource-model.md | 3 +- .../4.4-host-guest-control-interface.md | 73 ++--- doc/guest-agent-guidelines.md | 20 +- nvx-rs/README.md | 46 +-- nvx-rs/src/guest/init.rs | 4 - nvx-rs/src/guest/init/tests/exec.rs | 2 +- nvx-rs/src/guest/managed_agent/dispatch.rs | 17 +- .../src/guest/managed_agent/dispatch/tests.rs | 227 +++++-------- .../managed_agent/dispatch/tests/runtime.rs | 69 +--- .../managed_agent/dispatch/tests/streams.rs | 47 +-- nvx-rs/src/guest/managed_agent/exec.rs | 89 ++--- nvx-rs/src/guest/managed_agent/mod.rs | 26 +- nvx-rs/src/guest/managed_agent/session.rs | 140 +++----- .../src/guest/managed_agent/session/tests.rs | 112 +++---- .../managed_agent/tests/loopback/tests.rs | 109 +------ nvx-rs/src/host/restore/tests.rs | 9 +- nvx-rs/src/host/restore/tests/live_init.rs | 77 +++-- nvx-rs/src/protocol/app.rs | 148 +-------- nvx-rs/src/protocol/exec.rs | 6 - nvx-rs/src/protocol/features.rs | 145 --------- nvx-rs/src/protocol/mod.rs | 32 +- nvx-rs/src/protocol/tests.rs | 303 +----------------- nvx-rs/src/testing/fake_openvmm.rs | 210 ++++-------- nvx-rs/src/testing/fake_openvmm/tests.rs | 50 ++- nvx-rs/tests/cli.rs | 29 -- nvx-rs/tests/vectors/README.md | 5 +- nvx-rs/tests/vm_lifecycle.rs | 21 +- scripts/test-layout-allowlist.json | 3 +- 29 files changed, 465 insertions(+), 1559 deletions(-) delete mode 100644 nvx-rs/src/protocol/features.rs diff --git a/doc/acceptance-case-matrix.md b/doc/acceptance-case-matrix.md index fef690697..0d897b707 100644 --- a/doc/acceptance-case-matrix.md +++ b/doc/acceptance-case-matrix.md @@ -357,7 +357,7 @@ suite in its shards, and the public sandbox scenarios in its guest and sandbox j | `untiered-network-policy` | rewritten | On named snapshots with a NIC, which `snapshot capture --net` captures, as [Snapshots](#snapshots) says, keeping its checks of a restore's DNS, egress, forwards and listener publication. Its raw-snapshot parts retire: the raw capture and restores, the guest's repair marker, the console release, and OpenVMM's refusals of raw restores that the CLI cannot issue, in whose place the CLI must refuse a restore without `--network-profile`, and one with `--net` | | `virtio-features` | kept | | | `virtio-net` | kept | | -| `vm-lifecycle` | kept | A [cold boot](#cold-boots) through the public API: `nvx-rs/tests/vm_lifecycle.rs::a_cold_vm_is_reattached_from_saved_values_and_stopped` | +| `vm-lifecycle` | kept | A [cold boot](#cold-boots) through the public API: `nvx-rs/tests/vm_lifecycle.rs::a_cold_vm_is_reattached_from_saved_values_and_stopped`. Since SL4.4d, it pings and stops nvx-init with NVXC version 2's frames, where it sent version 1's | | `workload-identity` | kept | Since SL6.3 (#1122), it also checks that a guest given OpenVMM's workload identity without `nvx_identity=`, or with another one, fails closed | ### Cold boots diff --git a/doc/design/4.3-isolation-and-resource-model.md b/doc/design/4.3-isolation-and-resource-model.md index 888c618fa..026cf4fcd 100644 --- a/doc/design/4.3-isolation-and-resource-model.md +++ b/doc/design/4.3-isolation-and-resource-model.md @@ -15,8 +15,7 @@ the boot configuration requests it, nvx-init and the guest agents of the blockless managed root instead create an account for a host-selected identity that the image lacks, and replace an image account that uses the UID with another primary group or without a usable home. The older layered sandbox -root creates no accounts, so it does not advertise -[account creation](4.4-host-guest-control-interface.md#442-feature-negotiation). +root creates no accounts. Before the workload runs, its child clears supplementary groups and every capability set and enables `no_new_privs`, so the workload keeps none of the diff --git a/doc/design/4.4-host-guest-control-interface.md b/doc/design/4.4-host-guest-control-interface.md index 1e64cb18b..85c712e81 100644 --- a/doc/design/4.4-host-guest-control-interface.md +++ b/doc/design/4.4-host-guest-control-interface.md @@ -21,11 +21,10 @@ Table 4.2 lists the operations of the control protocol. | Operation | Purpose | | --- | --- | | `READY` | Reports readiness, and is the success reply to requests that return no other data | -| `FEATURES` | Reports which control behaviors the guest image provides | | `MAPS` | Delivers a contiguous range of host mapping-table entries | | `EXEC` | Runs one workload with a timeout, arguments, and an optional working directory and environment | | `CANCEL` | Kills the running execution that the request names | -| Shutdown | Shuts the VM down gracefully between executions | +| Shutdown | Shuts the VM down gracefully, ending a running execution first | A mapping request carries a contiguous range of the host mapping table's entries, each with its access, a source relative to the guest's mount of the @@ -40,14 +39,13 @@ 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. 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. +of the same name. In version 2 of the protocol, which NVX's host and guest +agents speak alone, 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. As +Table 4.3 shows, an execution that runs ends with an exit report, which carries +an outcome category and a status. **Table 4.3.** Exit outcomes. @@ -57,7 +55,6 @@ 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` | 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 @@ -81,38 +78,13 @@ 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 of -version 1 therefore returns a four-byte little-endian bit mask of control -features. - -**Table 4.5.** Control feature bits. - -| Bit | Feature | -| ---: | --- | -| 0 | Cancellation | -| 1 | Reserved; never set | -| 2 | Workload account creation | -| 3 | Per-execution cgroup containment | -| 4 | Per-execution environment | -| 5 | Working-directory refusal | -| 6 | Host mapping table | - -The blockless managed root provides cancellation, workload account creation, -per-execution environments, working-directory refusal, and the host mapping -table. It runs plain processes and does not announce or require cgroup -containment (bit 3). The layered sandbox root provides cancellation, -per-execution environments, and the host mapping table. Its workloads run as -an account that the image already has, so it does not create accounts. A -feature bit is added together with the behavior it names. -Hosts ignore unknown bits and trailing bytes, read an `unsupported-operation` -refusal as no features, and terminate a guest that lacks a feature they need -instead of running workloads without the policy they requested. +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. +A peer of another version is refused: the host refuses a `READY` that names +another version, and the guest agents refuse a frame of another version as a +malformed one, which ends the session. Version 1, whose `FEATURES` request +reported the control behaviors that a guest image provided, is gone. ## 4.4.3 Session Semantics @@ -120,11 +92,9 @@ 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, 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. +cancellation, the execution's standard input, or a shutdown, that arrives +during an execution is refused as `busy`. A shutdown ends the running +execution first. The control session is process-local even when guest parser state is snapshotted. Restore establishes a fresh host capability, peer identity, @@ -142,8 +112,11 @@ non-secret invariants needed before the control channel exists. Conformance vectors give the bytes that every host and guest agent must produce and accept, whatever code they share. [`nvxc-v1.txt`](../../nvx-rs/tests/vectors/nvxc-v1.txt) holds those of -version 1 of the control protocol: its frames, their execution, mapping, and -feature payloads, and records of the session that carries them. +version 1 of the control protocol, which no peer speaks any longer: its frames, +which version 2 refuses, their execution and mapping payloads, which version 2 +carries unchanged, its feature payloads, and records of the session that +carries them. Version 2's frames have no vector file: their tests pin their +bytes. [`openvmm/`](../../nvx-rs/tests/vectors/openvmm) holds the VMM's own vectors and cases of the session's records, unchanged. A vector changes only with a protocol version, which gets vector files of its own, and the bytes of an diff --git a/doc/guest-agent-guidelines.md b/doc/guest-agent-guidelines.md index f696b46cf..330095c61 100644 --- a/doc/guest-agent-guidelines.md +++ b/doc/guest-agent-guidelines.md @@ -250,8 +250,9 @@ plain-process core: root's user and group databases once, before its switch, and creates when the boot, with `nvx_workload_account=create`, or the activate request asks for it. -- **In the child**, which leads a process group of its own, with the null - device as its standard input and pipes as its standard output and error, +- **In the child**, which leads a process group of its own, with a pipe of + the input that the host streams as its standard input, which nvx-init + closes after the input's end, and pipes as its standard output and error, the execution resets its priority, sets its credentials, changes to its working directory and execs. It resets its out-of-memory score adjustment and its nice value to 0, then drops its capability bounding set and takes @@ -263,11 +264,14 @@ plain-process core: in another directory. - **End, cancellation, timeout and reset** each kill the execution's process group: nvx-init kills it when the execution's leader ends, normally or not, - when the host cancels the execution, when its timeout passes, and when a - session reset abandons it, whose output it then discards rather than report - it on the new session. PID 1 collects the status of every child, orphans - included. A process group is a cleanup scope, not containment: a process - that leaves its group escapes it, as the capture's sweep allows for. + when the host cancels the execution or shuts the VM down while it runs, + when its timeout passes, and when a session reset abandons it. None of an + abandoned execution's output reaches the new session: nvx-init reads no + more of it, and what it had already queued goes out before the reset's + acknowledgement, in the earlier epoch, whose records the broker drops. + PID 1 collects the status of every child, orphans included. A process + group is a cleanup scope, not containment: a process that leaves its group + escapes it, as the capture's sweep allows for. This is priority, not limits. PID 1 and its helpers run with an out-of-memory score adjustment of -1000 and a nice value of -10, ahead of the executions' @@ -294,7 +298,7 @@ enforces none of it. nvx-init has no workspace: it refuses a capture while an execution runs, or kills the process groups and sweeps when the host asks, as its capture -request says. Without permission to kill, it refuses the request with `EBUSY` +request says. Without permission to kill, it refuses the request as `busy` while any process but PID 1 and its registered helpers runs; with it, it ends a running execution's process group, then sweeps, killing every process but PID 1 and its helpers again and again until none remains. The checks, the diff --git a/nvx-rs/README.md b/nvx-rs/README.md index 6d64115fe..09fb22952 100644 --- a/nvx-rs/README.md +++ b/nvx-rs/README.md @@ -27,9 +27,9 @@ the nested `openvmm/` workspace. ```text nvx-rs/ ├── src/ -│ ├── protocol/ NVXS session, NVXC frames, EXEC and MAPS payloads, guest features, kernel -│ │ command-line tokens, and the guest targets that live shares may not take; -│ │ no dependencies, shared by the host and the guest +│ ├── protocol/ NVXS session, NVXC frames, EXEC and MAPS payloads, kernel command-line +│ │ tokens, and the guest targets that live shares may not take; no +│ │ dependencies, shared by the host and the guest │ ├── activation/ the activation record of a layered root, its canonical encoding, and what │ │ Ready reports about it; no dependencies, shared by the host, the guest, and │ │ every adapter's framing @@ -147,19 +147,20 @@ The codec of the control console: `NVXS` outer records, the `MAPS` payloads of the host mapping table, and the `nvx_maps=` and `nvx_workload_account=create` tokens. NVX's own control protocol, `NVXC`, is private to the crate, gated by the features of its users, `cli`, `guest` and `testing`: its application -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. 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`. +frames, and the `EXEC` payload that the host encodes and the guest agent +decodes. `protocol::app::v2` holds the codec of version 2's frames, NVXC's only +version, which carry every control path, with version 1's payloads, and stream +an execution's standard input, output and error. NVX's host, the guest loop +that nvx-init and the managed agent share, and the fake OpenVMM speak version 2 +alone, and refuse a frame of another version, such as version 1, whose frames +and `FEATURES` request are gone. The [conformance vectors](tests/vectors/README.md) of version 1, `tests/vectors/nvxc-v1.txt`, and OpenVMM's vectors and cases of the `NVXS` -session pin every record, frame, and payload independently of the codec, which -the host, the guest agent, and the fake OpenVMM share. +session pin the records and the `EXEC` and `MAPS` payloads independently of the +codec, which the host, the guest agent, and the fake OpenVMM share; version 2 +refuses the frames of version 1 that they hold, their features payloads have +no codec any longer, and version 2's tests pin its own frames. ### `nvx::protocol::cmdline` @@ -2028,7 +2029,7 @@ 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 +request reported, 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 @@ -2186,9 +2187,8 @@ with it for any other. Like OpenVMM, it refuses with status 2 a restore with with `--network-profile` alone or with `--restore-network-policy`, and a restore with `--network-ingress allow` or explicit `--host-loopback allow` without `--restore-network-policy`. Kernel -command-line tokens make it fail: `fake_exit_on_start=STATUS`, -`fake_exit_status=STATUS`, `fake_guest_features=MASK`, and -`fake_legacy_guest=1`, among others; see `src/testing/fake_openvmm.rs`. Like +command-line tokens make it fail: `fake_exit_on_start=STATUS` and +`fake_exit_status=STATUS`, among others; see `src/testing/fake_openvmm.rs`. Like OpenVMM, it opens the sandbox's block devices at the paths that its options name and holds them until it exits, after `fake_block_delay_ms=MS` if asked, and records what each held in `fake-openvmm-TOKEN-blocks.json` next to the @@ -2647,13 +2647,13 @@ 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 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 +[scenario tests](#scenario-tests) of `nvx-rs`, and checks 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 -accepts or refuses OpenVMM's vectors and cases of the `NVXS` session as they +sides of every record and payload: the codecs decode each record and each +`EXEC` and `MAPS` payload of `tests/vectors/nvxc-v1.txt` to its values and +encode them to its bytes, version 2's codec refuses its frames, the codecs +accept or refuse OpenVMM's vectors and cases of the `NVXS` session as they say, and, on Linux, the guest's listener announcements match `tests/vectors/nvxl-v1.txt`. The activation codecs decode each record and Ready activation of `tests/vectors/activation-v2.txt` to its values, encode diff --git a/nvx-rs/src/guest/init.rs b/nvx-rs/src/guest/init.rs index c64dead84..37bc326d4 100644 --- a/nvx-rs/src/guest/init.rs +++ b/nvx-rs/src/guest/init.rs @@ -886,10 +886,6 @@ impl Exec { }) } - pub(crate) fn discard(&mut self, stream: Stream) { - self.discarding[stream as usize] = true; - } - pub(crate) fn kill_failure(&mut self) -> Option> { self.failures .iter_mut() diff --git a/nvx-rs/src/guest/init/tests/exec.rs b/nvx-rs/src/guest/init/tests/exec.rs index e63ee197d..c5bb40474 100644 --- a/nvx-rs/src/guest/init/tests/exec.rs +++ b/nvx-rs/src/guest/init/tests/exec.rs @@ -299,7 +299,7 @@ fn ending_failures_are_reported_and_discarding_preserves_the_first_cause() { } let mut execution = pipe_execution(b"drop", b"keep"); execution.status_lost = false; - execution.discard(Stream::Stdout); + execution.discarding[Stream::Stdout as usize] = true; execution.cancel(); execution.end(); assert!(matches!( diff --git a/nvx-rs/src/guest/managed_agent/dispatch.rs b/nvx-rs/src/guest/managed_agent/dispatch.rs index bcc1144bb..a30488f6e 100644 --- a/nvx-rs/src/guest/managed_agent/dispatch.rs +++ b/nvx-rs/src/guest/managed_agent/dispatch.rs @@ -4,9 +4,8 @@ use crate::guest::init::Runtime; use crate::guest::mappings::{Mapper, MappingTable, SourceRoot}; use crate::guest::sys::{self, Errno, FileDescriptor}; use crate::protocol::{ - self, APP_ACTIVATE, APP_CANCEL, APP_CAPTURE, APP_EXEC, APP_FEATURES, APP_MAPS, APP_PING, - APP_READY, APP_STOP, APP_STOPPED, GuestFeatures, INVALID_REQUEST, MAPPINGS_INCOMPLETE, - UNSUPPORTED_OPERATION, + self, APP_ACTIVATE, APP_CANCEL, APP_CAPTURE, APP_EXEC, APP_MAPS, APP_PING, APP_READY, APP_STOP, + APP_STOPPED, INVALID_REQUEST, MAPPINGS_INCOMPLETE, UNSUPPORTED_OPERATION, }; use super::diagnostics; @@ -284,18 +283,6 @@ fn handle_data( APP_CANCEL => { // The targeted workload has already reported its outcome. } - APP_FEATURES => { - if request.payload.is_empty() { - session.send_app( - APP_READY, - request.request_id, - 0, - &GuestFeatures::DIRECT_AGENT.encode(), - )?; - } else { - session.send_error(request.request_id, Errno::EINVAL as i32, INVALID_REQUEST)?; - } - } APP_MAPS => match super::mappings::apply_request(system, mappings, request.payload) { Ok(()) => session.send_app(APP_READY, request.request_id, 0, &[])?, Err(MappingError::Invalid) => { diff --git a/nvx-rs/src/guest/managed_agent/dispatch/tests.rs b/nvx-rs/src/guest/managed_agent/dispatch/tests.rs index ef4010e94..cea5b7afa 100644 --- a/nvx-rs/src/guest/managed_agent/dispatch/tests.rs +++ b/nvx-rs/src/guest/managed_agent/dispatch/tests.rs @@ -6,11 +6,11 @@ use std::path::Path; use std::thread; use crate::protocol::{ - APP_ERROR, APP_EXIT, APP_STDERR, APP_STDOUT, OUTER_ACK, OUTER_CREDIT, OUTER_DATA, - OUTER_GUEST_ATTACH, OUTER_HEADER_LEN, OUTER_MAX_PAYLOAD, OUTER_RECEIVE_WINDOW, OUTER_RESET, - OUTER_WAIT, OuterHeader, ProtocolError, Workload, WorkloadEnvironment, decode_ack_payload, - decode_app, decode_credit_payload, decode_outer_header, encode_app, encode_exec_payload, - encode_outer, + APP_ERROR, APP_EXIT, APP_HEADER_LEN, APP_STDERR, APP_STDOUT, OUTER_ACK, OUTER_CREDIT, + OUTER_DATA, OUTER_GUEST_ATTACH, OUTER_HEADER_LEN, OUTER_MAX_PAYLOAD, OUTER_RECEIVE_WINDOW, + OUTER_RESET, OUTER_WAIT, OuterHeader, ProtocolError, Workload, WorkloadEnvironment, + decode_ack_payload, decode_credit_payload, decode_outer_header, encode_exec_payload, + encode_outer, v2, }; use super::super::seam::Paths; @@ -24,7 +24,11 @@ use crate::guest::sys::tests::{ use crate::guest::sys::{self, FileDescriptor}; const INSTANCE: [u8; 16] = [7; 16]; +/// The kind of `BUSY`, with which the agent refuses a request while an execution runs. +const APP_BUSY: u8 = 0x87; +/// A frame that the agent sent: its kind, request ID and status, and its payload, which follows +/// a `READY`'s protocol version. #[derive(Debug, PartialEq, Eq)] struct Response { kind: u8, @@ -120,9 +124,10 @@ impl Harness { self.responses() } - /// Sends one request, and reads the credit that returns its bytes, before any response. + /// Sends one request, of version 2, and reads the credit that returns its bytes, before any + /// response. fn send(&mut self, request_id: u64, kind: u8, payload: &[u8]) { - let frame = encode_app(kind, request_id, 0, payload); + let frame = frame(kind, request_id, payload); self.host .write_all( &encode_outer( @@ -153,13 +158,7 @@ impl Harness { (OUTER_DATA, self.guest_sequence) ); self.guest_sequence += 1; - let frame = decode_app(&frame).unwrap(); - Response { - kind: frame.kind, - request_id: frame.request_id, - status: frame.status, - payload: frame.payload.to_vec(), - } + response_of(&frame) } /// Reads responses up to the first terminal one, which it returns last. @@ -169,7 +168,7 @@ impl Harness { let response = self.next_response(); let terminal = matches!( response.kind, - APP_READY | APP_ERROR | APP_EXIT | APP_STOPPED + APP_READY | APP_ERROR | APP_EXIT | APP_STOPPED | APP_BUSY ); responses.push(response); if terminal { @@ -227,6 +226,41 @@ fn read_outer(host: &mut UnixStream) -> (OuterHeader, Vec) { mod runtime; mod streams; +/// Returns the request of version 2 of `kind`, with `request_id`, status 0 and `payload`, as the +/// protocol defines it, whatever the payload that the kind allows. +fn frame(kind: u8, request_id: u64, payload: &[u8]) -> Vec { + let length = u32::try_from(payload.len()).unwrap(); + [ + &b"NVXC\x02"[..], + &[kind, 0, 0], + &request_id.to_le_bytes(), + &0i32.to_le_bytes(), + &length.to_le_bytes(), + payload, + ] + .concat() +} + +/// Returns the response that `frame`, of version 2, carries. +fn response_of(frame: &[u8]) -> Response { + let (request_id, decoded) = v2::Frame::decode(frame).unwrap(); + let mut payload = &frame[APP_HEADER_LEN..]; + if let v2::Frame::Ready { + version, + activation, + } = decoded + { + assert_eq!(version, v2::VERSION); + payload = activation; + } + Response { + kind: frame[5], + request_id, + status: i32::from_le_bytes(frame[16..20].try_into().unwrap()), + payload: payload.to_vec(), + } +} + fn response(kind: u8, request_id: u64, status: i32, payload: &[u8]) -> Response { Response { kind, @@ -291,21 +325,6 @@ fn assert_environment_failure(mut harness: Harness, diagnostic_prefix: &[u8]) { assert_eq!(system.spawn_calls, 0); } -#[test] -fn agent_advertises_the_control_features_of_its_mode_and_stops() { - let mut harness = Harness::new(); - let response = harness.request(APP_FEATURES, &[]); - assert_eq!( - (response.kind, response.request_id, response.status), - (APP_READY, 42, 0) - ); - let features = GuestFeatures::decode(&response.payload).unwrap(); - assert_eq!(features, GuestFeatures::DIRECT_AGENT); - assert!(!features.contains(GuestFeatures::EXEC_CGROUP)); - let system = harness.stop(); - assert_eq!(system.tty_drained, 1); -} - #[test] fn fresh_session_refuses_root_requests_and_routes_capture_before_stopping() { let config = AgentConfig::for_test(); @@ -349,19 +368,14 @@ fn fresh_session_refuses_root_requests_and_routes_capture_before_stopping() { } #[test] -fn agent_refuses_feature_payloads_and_unannounced_mapping_tables() { +fn agent_refuses_unannounced_mapping_tables() { let mut harness = Harness::new(); - for (kind, payload) in [ - (APP_FEATURES, b"abc".as_slice()), - (APP_MAPS, b"\0\0\0\0\x01\0\0\0\0\0\x01\0\x02\0".as_slice()), - ] { - let response = harness.request(kind, payload); - assert_eq!( - (response.kind, response.request_id, response.status), - (APP_ERROR, 42, Errno::EINVAL as i32) - ); - assert_eq!(response.payload, INVALID_REQUEST.as_bytes()); - } + let response = harness.request(APP_MAPS, b"\0\0\0\0\x01\0\0\0\0\0\x01\0\x02\0"); + assert_eq!( + (response.kind, response.request_id, response.status), + (APP_ERROR, 42, Errno::EINVAL as i32) + ); + assert_eq!(response.payload, INVALID_REQUEST.as_bytes()); harness.stop(); } @@ -931,7 +945,7 @@ fn data_validation_diagnostics_preserve_the_c_failure_stage_and_status() { "NVX-MANAGED-ERROR: stage=data status={}\n", Errno::EPROTO as i32 ); - let frame = encode_app(APP_PING, 42, 0, &[]); + let frame = frame(APP_PING, 42, &[]); let diagnostics = Harness::new().fail(&encode_outer(OUTER_DATA, &INSTANCE, 1, 9, &frame).unwrap()); assert_eq!( @@ -968,9 +982,16 @@ fn data_validation_diagnostics_preserve_the_c_failure_stage_and_status() { #[test] fn a_request_that_fails_after_validation_reports_the_data_stage() { - let frame = encode_app(APP_STOP, 42, 0, b"x"); - let diagnostics = - Harness::new().fail(&encode_outer(OUTER_DATA, &INSTANCE, 1, 8, &frame).unwrap()); + // A restored agent's activate request passes the validation of its frame, and its record, + // which does not decode, then ends the session. + let config = AgentConfig::for_test(); + let mappings = HostMappings::announced(0); + let (capture, _) = Capture::activation_fixture(); + let system = TestSystem::new(Paths::default()); + let mut harness = Harness::spawn_with_capture(system, config, mappings, Some(capture)); + harness.reset(1, 7); + let frame = frame(APP_ACTIVATE, 42, b"x"); + let diagnostics = harness.fail(&encode_outer(OUTER_DATA, &INSTANCE, 1, 8, &frame).unwrap()); assert_eq!( diagnostics, format!( @@ -1367,15 +1388,9 @@ fn direct_endings_do_not_wait_for_an_escaped_descendant_to_close_output() -> Tes harness .host .set_read_timeout(Some(std::time::Duration::from_secs(20)))?; - for (request_id, ending, status) in [ - (42, "cancelled", 137), - (43, "timeout", 124), - (44, "output-limit", 125), - ] { + for (request_id, ending, status) in [(42, "cancelled", 137), (43, "timeout", 124)] { let exit = Gate::new(temporary.path().join(format!("exit-{request_id}")))?; - let mut output = Some(Gate::new( - temporary.path().join(format!("output-{request_id}")), - )?); + let output = Gate::new(temporary.path().join(format!("output-{request_id}")))?; let finish = Gate::new(temporary.path().join(format!("finish-{request_id}")))?; let escaped = temporary.path().join(format!("escaped-{request_id}")); // The finish gate ends only this escaped group, even after its readers have closed. @@ -1385,18 +1400,12 @@ fn direct_endings_do_not_wait_for_an_escaped_descendant_to_close_output() -> Tes "-c", "exec 3<\"$0\" 4<\"$1\" 5<\"$2\"; \ setsid sh -c 'echo $$ > \"$0\"; read -r line <&4; \ - if [ \"$1\" = output-limit ]; then \ - (read -r line <&5; kill -KILL -$$) >/dev/null 2>&1 & \ - trap \"\" PIPE; \ - for i in 1 2 3 4; do \ - (while :; do cat /dev/zero 2>/dev/null; done) & done; wait; fi; \ - read -r line <&5' \"$3\" \"$4\" & \ + read -r line <&5' \"$3\" & \ echo $$; read -r line <&3", utf8(&exit.path)?, - utf8(&output.as_ref().unwrap().path)?, + utf8(&output.path)?, utf8(&finish.path)?, utf8(&escaped)?, - ending, ] .map(str::to_owned) .into(), @@ -1420,27 +1429,17 @@ fn direct_endings_do_not_wait_for_an_escaped_descendant_to_close_output() -> Tes } if ending == "cancelled" { harness.send(request_id, APP_CANCEL, &[]); - } else if ending == "output-limit" { - drop(output.take()); } let mut responses = harness.responses(); assert_eq!( responses.pop(), Some(response(APP_EXIT, request_id, status, ending.as_bytes())) ); - let forwarded: usize = responses - .iter() - .map(|response| { - assert_eq!( - (response.kind, response.request_id, response.status), - (APP_STDOUT, request_id, 0) - ); - response.payload.len() - }) - .sum(); - assert!(forwarded + leader.payload.len() <= protocol::MAX_OUTPUT_BYTES); - if ending == "output-limit" { - assert!(forwarded > 0); + for response in responses { + assert_eq!( + (response.kind, response.request_id, response.status), + (APP_STDOUT, request_id, 0) + ); } let descendant = std::fs::read_to_string(&escaped)?.trim().parse::()?; drop(output); @@ -1552,7 +1551,7 @@ fn a_control_failure_after_a_lost_status_does_not_wait_again() -> TestResult { drop(exit); wait_for_process_exit(leader)?; for _ in 0..2 { - assert_eq!(harness.request(APP_PING, &[]).status, Errno::EBUSY as i32); + assert_eq!(harness.request(APP_PING, &[]).kind, APP_BUSY); } harness.host.write_all(&encode_outer( OUTER_WAIT, @@ -1647,12 +1646,7 @@ fn a_lost_status_keeps_its_outcome_through_a_later_cancellation_or_timeout() -> for _ in 0..2 { assert_eq!( harness.request(APP_PING, &[]), - response( - APP_ERROR, - 42, - Errno::EBUSY as i32, - protocol::BUSY.as_bytes() - ), + response(APP_BUSY, 42, 0, b"") ); } if cancelled { @@ -1978,56 +1972,6 @@ fn normal_completion_kills_the_direct_process_group() -> TestResult { Ok(()) } -#[test] -fn output_beyond_the_cap_ends_the_workload_after_the_reads_that_fit() -> TestResult { - if super::super::tests::isolate_process_test( - "NVX_DISPATCH_OUTPUT_CAP_TEST_CHILD", - "guest::managed_agent::dispatch::tests::output_beyond_the_cap_ends_the_workload_after_the_reads_that_fit", - ) { - return Ok(()); - } - let _process = super::super::tests::PROCESS_TEST_LOCK.lock()?; - let (temporary, mut harness) = direct_harness(); - // As the C agent did, the agent forwards each read of up to 32 KiB while the output - // stays within 1 MiB, and ends the workload at the first read beyond it, before the - // workload could leave its marker. The workload writes 3 MiB into a pipe that holds 1 MiB - // at most, even after uutils' `head -c` (Ubuntu 26.04's `head`) grows it to fit what it - // copies, so it can finish only after the agent has read 2 MiB. - let chunk = 32 * 1024; - let marker = temporary.path().join("survived"); - let script = format!( - "head -c {} /dev/zero; touch '{}'", - 3 * protocol::MAX_OUTPUT_BYTES, - marker.display() - ); - let mut replies = harness.request_all( - 42, - APP_EXEC, - &workload_payload(&["/bin/sh", "-c", &script], None)?, - ); - assert_eq!( - replies.pop(), - Some(response(APP_EXIT, 42, 125, b"output-limit")) - ); - let mut forwarded = 0; - for reply in &replies { - assert_eq!( - (reply.kind, reply.request_id, reply.status), - (APP_STDOUT, 42, 0) - ); - assert!((1..=chunk).contains(&reply.payload.len())); - assert!(reply.payload.iter().all(|&byte| byte == 0)); - forwarded += reply.payload.len(); - } - assert!( - protocol::MAX_OUTPUT_BYTES - chunk < forwarded && forwarded <= protocol::MAX_OUTPUT_BYTES, - "{forwarded}" - ); - assert!(harness.stop().diagnostics.is_empty()); - assert!(!marker.exists()); - Ok(()) -} - #[test] fn a_running_workload_refuses_other_requests_and_ends_when_cancelled_or_late() -> TestResult { if super::super::tests::isolate_process_test( @@ -2046,12 +1990,7 @@ fn a_running_workload_refuses_other_requests_and_ends_when_cancelled_or_late() - ); assert_eq!( harness.request_all(43, APP_PING, &[]), - vec![response( - APP_ERROR, - 43, - Errno::EBUSY as i32, - protocol::BUSY.as_bytes() - )] + vec![response(APP_BUSY, 43, 0, b"")] ); // A cancellation of another request leaves the workload running. harness.send(41, APP_CANCEL, &[]); @@ -2161,7 +2100,7 @@ fn refused_records_report_the_stage_at_which_the_c_agent_reported_them() { let outer = |record_type, instance: &[u8; 16], epoch, sequence, payload: &[u8]| { encode_outer(record_type, instance, epoch, sequence, payload).unwrap() }; - let ping = encode_app(APP_PING, 42, 0, &[]); + let ping = frame(APP_PING, 42, &[]); let mut bad_magic = outer(OUTER_DATA, &INSTANCE, 1, 8, &ping); bad_magic[0] = b'X'; let mut oversized = outer(OUTER_DATA, &INSTANCE, 1, 8, &[]); @@ -2233,7 +2172,7 @@ fn refused_records_report_the_stage_at_which_the_c_agent_reported_them() { #[test] fn data_before_the_first_reset_is_a_type_that_the_agent_does_not_take_yet() { let harness = Harness::spawn(TestSystem::new(Paths::default()), 0); - let frame = encode_app(APP_PING, 42, 0, &[]); + let frame = frame(APP_PING, 42, &[]); let diagnostics = harness.fail(&encode_outer(OUTER_DATA, &INSTANCE, 1, 0, &frame).unwrap()); assert_eq!( diagnostics, @@ -2247,7 +2186,7 @@ fn data_at_the_last_sequence_number_is_out_of_sequence() { // for the broker's next record. let mut harness = Harness::spawn(TestSystem::new(Paths::default()), 0); harness.reset(1, u64::MAX - 1); - let frame = encode_app(APP_PING, 42, 0, &[]); + let frame = frame(APP_PING, 42, &[]); let record = encode_outer(OUTER_DATA, &INSTANCE, 1, u64::MAX, &frame).unwrap(); assert_eq!( String::from_utf8(harness.fail(&record)).unwrap(), @@ -2265,7 +2204,7 @@ fn a_stream_that_ends_reports_a_read_failure_inside_a_record_or_not() { "NVX-MANAGED-ERROR: stage=outer-read status={}\n", Errno::EIO as i32 ); - let frame = encode_app(APP_PING, 42, 0, &[]); + let frame = frame(APP_PING, 42, &[]); let record = encode_outer(OUTER_DATA, &INSTANCE, 1, 8, &frame).unwrap(); for length in [0, 20, OUTER_HEADER_LEN + 1] { let diagnostics = Harness::new().close(&record[..length]); diff --git a/nvx-rs/src/guest/managed_agent/dispatch/tests/runtime.rs b/nvx-rs/src/guest/managed_agent/dispatch/tests/runtime.rs index 20a4589f8..c9fc3c48a 100644 --- a/nvx-rs/src/guest/managed_agent/dispatch/tests/runtime.rs +++ b/nvx-rs/src/guest/managed_agent/dispatch/tests/runtime.rs @@ -204,8 +204,7 @@ fn privileged_layered_runtime_helper_failure_wins_over_pending_sigterm() { fn receive(host: &mut UnixStream) -> Response { let (header, payload) = read_outer(host); assert_eq!(header.record_type, OUTER_DATA); - let frame = decode_app(&payload).unwrap(); - response(frame.kind, frame.request_id, frame.status, frame.payload) + response_of(&payload) } fn prelaunch_termination(helper_ends: bool) { @@ -224,14 +223,7 @@ fn prelaunch_termination(helper_ends: bool) { assert_eq!(read_outer(&mut host).0.record_type, OUTER_ACK); let execution = workload_payload(&["/bin/true"], Some("/")).unwrap(); host.write_all( - &encode_outer( - OUTER_DATA, - &INSTANCE, - 1, - 1, - &encode_app(APP_EXEC, 9, 0, &execution), - ) - .unwrap(), + &encode_outer(OUTER_DATA, &INSTANCE, 1, 1, &frame(APP_EXEC, 9, &execution)).unwrap(), ) .unwrap(); let Ok(Event::Data(payload)) = session.next_event() else { @@ -335,21 +327,15 @@ fn privileged_layered_runtime_delivers_a_fragment_collected_status_before_driver host.write_all(&encode_outer(OUTER_RESET, &INSTANCE, 1, 0, &[]).unwrap()) .unwrap(); assert_eq!(read_outer(&mut host).0.record_type, OUTER_ACK); - let execution = encode_app(APP_EXEC, 9, 0, &payload); + let execution = frame(APP_EXEC, 9, &payload); host.write_all(&encode_outer(OUTER_DATA, &INSTANCE, 1, 1, &execution).unwrap()) .unwrap(); assert_eq!(read_outer(&mut host).0.record_type, OUTER_CREDIT); until(|| pid_path.exists() && std::fs::metadata(&pid_path).unwrap().len() != 0); let id: i64 = std::fs::read_to_string(&pid_path).unwrap().parse().unwrap(); let leader = sys::open_pidfd(ProcessId::from_positive(id).unwrap()).unwrap(); - let ping = encode_outer( - OUTER_DATA, - &INSTANCE, - 1, - 2, - &encode_app(APP_PING, 98, 0, &[]), - ) - .unwrap(); + let ping = + encode_outer(OUTER_DATA, &INSTANCE, 1, 2, &frame(APP_PING, 98, &[])).unwrap(); fragment(&mut host, probe, &ping); std::fs::OpenOptions::new() .write(true) @@ -363,10 +349,7 @@ fn privileged_layered_runtime_delivers_a_fragment_collected_status_before_driver until(|| !Path::new(&format!("/proc/{id}")).exists()); host.write_all(&ping[1..]).unwrap(); assert_eq!(read_outer(&mut host).0.record_type, OUTER_CREDIT); - assert_eq!( - receive(&mut host), - response(APP_ERROR, 98, Errno::EBUSY as i32, b"busy") - ); + assert_eq!(receive(&mut host), response(APP_BUSY, 98, 0, b"")); let mut output = Vec::new(); loop { let frame = receive(&mut host); @@ -379,14 +362,7 @@ fn privileged_layered_runtime_delivers_a_fragment_collected_status_before_driver } assert_eq!(output, b"observed"); host.write_all( - &encode_outer( - OUTER_DATA, - &INSTANCE, - 1, - 3, - &encode_app(APP_STOP, 99, 0, &[]), - ) - .unwrap(), + &encode_outer(OUTER_DATA, &INSTANCE, 1, 3, &frame(APP_STOP, 99, &[])).unwrap(), ) .unwrap(); assert_eq!(read_outer(&mut host).0.record_type, OUTER_CREDIT); @@ -448,14 +424,8 @@ fn privileged_layered_runtime_keeps_helper_policy_and_first_error_during_failed_ .unwrap(); assert_eq!(read_outer(&mut host).0.record_type, OUTER_ACK); host.write_all( - &encode_outer( - OUTER_DATA, - &INSTANCE, - 1, - 1, - &encode_app(APP_EXEC, 9, 0, &payload), - ) - .unwrap(), + &encode_outer(OUTER_DATA, &INSTANCE, 1, 1, &frame(APP_EXEC, 9, &payload)) + .unwrap(), ) .unwrap(); assert_eq!(read_outer(&mut host).0.record_type, OUTER_CREDIT); @@ -566,7 +536,7 @@ fn only_stdout_prefix(mut traffic: &[u8]) { if traffic.len() < length { return; } - let frame = decode_app(&traffic[OUTER_HEADER_LEN..length]).unwrap(); + let frame = response_of(&traffic[OUTER_HEADER_LEN..length]); assert_eq!((frame.kind, frame.request_id), (APP_STDOUT, 9)); traffic = &traffic[length..]; } @@ -707,14 +677,8 @@ fn privileged_layered_runtime_delivers_a_write_collected_status_before_driver_ev narrow_output(&probe); reset_peer(&mut host); host.write_all( - &encode_outer( - OUTER_DATA, - &INSTANCE, - 1, - 1, - &encode_app(APP_EXEC, 9, 0, &payload), - ) - .unwrap(), + &encode_outer(OUTER_DATA, &INSTANCE, 1, 1, &frame(APP_EXEC, 9, &payload)) + .unwrap(), ) .unwrap(); assert_eq!(read_outer(&mut host).0.record_type, OUTER_CREDIT); @@ -752,14 +716,7 @@ fn privileged_layered_runtime_delivers_a_write_collected_status_before_driver_ev } assert_eq!(output, vec![0; 6000]); host.write_all( - &encode_outer( - OUTER_DATA, - &INSTANCE, - 1, - 2, - &encode_app(APP_STOP, 99, 0, &[]), - ) - .unwrap(), + &encode_outer(OUTER_DATA, &INSTANCE, 1, 2, &frame(APP_STOP, 99, &[])).unwrap(), ) .unwrap(); assert_eq!(read_outer(&mut host).0.record_type, OUTER_CREDIT); diff --git a/nvx-rs/src/guest/managed_agent/dispatch/tests/streams.rs b/nvx-rs/src/guest/managed_agent/dispatch/tests/streams.rs index 00795b901..1b6f502d6 100644 --- a/nvx-rs/src/guest/managed_agent/dispatch/tests/streams.rs +++ b/nvx-rs/src/guest/managed_agent/dispatch/tests/streams.rs @@ -1,8 +1,9 @@ -//! NVXC version 2 through the shared loop: its requests beside version 1's, with `READY`'s -//! version; one execution at a time, with `BUSY` for another, and `CANCEL` and `STOP` while it -//! runs; its standard input, which the agent acknowledges once its pipe took it, under the -//! window, until its explicit end; its output past version 1's cap; and the send queue, while -//! which is full the agent reads no output and still serves control requests. +//! NVXC version 2 through the shared loop: its requests, with `READY`'s version, and the +//! refusal of a frame of version 1; one execution at a time, with `BUSY` for another, and +//! `CANCEL` and `STOP` while it runs; its standard input, which the agent acknowledges once its +//! pipe took it, under the window, until its explicit end; its output past version 1's cap; and +//! the send queue, while which is full the agent reads no output and still serves control +//! requests. use std::os::fd::AsFd; use std::time::{Duration, Instant}; @@ -146,7 +147,7 @@ fn shell(script: &str) -> Vec { } #[test] -fn version_2_serves_ping_and_the_mapping_table_beside_version_1() { +fn version_2_serves_ping_and_the_mapping_table() { let (temporary, system) = TestSystem::with_host_export(); let export = system.paths.hostfs_dir.join("root").canonicalize().unwrap(); let source = temporary.path().canonicalize().unwrap().join("guest/src"); @@ -169,16 +170,6 @@ fn version_2_serves_ping_and_the_mapping_table_beside_version_1() { ); harness.send_v2(3, Frame::Maps(&maps)); assert_eq!(harness.next_v2(), Reply::Ready(3, Vec::new())); - // The same session still serves version 1, its features request included. - assert_eq!( - harness.request(APP_PING, &[]), - response(APP_READY, 42, 0, b"") - ); - let features = harness.request(APP_FEATURES, &[]); - assert_eq!( - GuestFeatures::decode(&features.payload), - Ok(GuestFeatures::DIRECT_AGENT) - ); let system = harness.stop_v2(4); assert_eq!( system.kernel.calls(), @@ -188,6 +179,26 @@ fn version_2_serves_ping_and_the_mapping_table_beside_version_1() { assert!(system.diagnostics.is_empty()); } +/// A request of version 1, as its conformance vector holds it, ends the session as a malformed +/// request does. +#[test] +fn a_request_of_version_1_ends_the_session() { + let frame = crate::tests::vector(crate::tests::NVXC_V1, "FRAME", "PING").bytes; + let diagnostics = + Harness::new().fail(&encode_outer(OUTER_DATA, &INSTANCE, 1, 8, &frame).unwrap()); + let line = + |stage: &str, status: i32| format!("NVX-MANAGED-ERROR: stage={stage} status={status}\n"); + assert_eq!( + diagnostics, + [ + line("data-application", 24), + line("data", Errno::EPROTO as i32) + ] + .concat() + .as_bytes() + ); +} + #[test] fn version_2_has_no_features_request() { let frame = [&b"NVXC\x02\x05\0\0"[..], &42u64.to_le_bytes(), &[0; 8]].concat(); @@ -220,7 +231,7 @@ fn version_2_carries_capture_and_activation_with_their_payloads() { Reply::Error(1, Errno::ENOTSUP as i32, b"unsupported-operation".to_vec()) ); // Outside PID 1, the capture's preparation fails, which the agent reports as version 1 - // does. + // did. harness.send_v2(2, Frame::Capture(&[0])); assert_eq!( harness.next_v2(), @@ -399,7 +410,7 @@ fn a_version_2_execution_forwards_output_past_the_old_cap() -> TestResult { } let _process = super::super::super::tests::PROCESS_TEST_LOCK.lock()?; let (_temporary, mut harness) = direct_harness(); - let total = protocol::MAX_OUTPUT_BYTES * 3 / 2; + let total = 3 * 1024 * 1024 / 2; harness.send_v2( 42, Frame::Exec(&shell(&format!("head -c {total} /dev/zero"))), diff --git a/nvx-rs/src/guest/managed_agent/exec.rs b/nvx-rs/src/guest/managed_agent/exec.rs index f047514de..f8cc787d7 100644 --- a/nvx-rs/src/guest/managed_agent/exec.rs +++ b/nvx-rs/src/guest/managed_agent/exec.rs @@ -5,7 +5,7 @@ use std::time::{Duration, Instant}; use crate::guest::init::{self, Stream}; use crate::guest::sys::{self, Errno, c_string}; use crate::protocol::{ - self, APP_ACTIVATE, APP_CANCEL, APP_CAPTURE, APP_EXIT, APP_STDERR, APP_STDOUT, APP_STOP, BUSY, + APP_ACTIVATE, APP_CANCEL, APP_CAPTURE, APP_EXIT, APP_STDERR, APP_STDOUT, APP_STOP, BUSY, CWD_FAILED, GuestExecPayload, LAUNCH_FAILED, OUTER_DATA, v2::STDIN_WINDOW, }; @@ -27,16 +27,14 @@ const ENVIRONMENT_DIAGNOSTIC: &[u8] = b"nvx-managed-agent: cannot configure work /// The agent drives its native plain-process workload in the C agent's loop: each turn polls the /// workload and the control session, serves a control record, forwards the workload's output, /// collects the statuses of the agent's children, and checks the workload's timeout. The agent -/// keeps the C agent's policy: the output cap, checked for each read of up to 32 KiB, and the -/// precedence of a cancellation, a timeout, and the cap over the workload's own status. +/// keeps the C agent's precedence of a cancellation and a timeout over the workload's own status. /// -/// An execution of NVXC version 2 has no output cap. Its workload reads a pipe, into which each -/// turn writes what the host sent in `STDIN` frames, under [`STDIN_WINDOW`], and acknowledges -/// what it wrote, and which it closes after `STDIN_END`. The session streams while it runs: its -/// records go through the send queue, which never waits for the console, and the agent reads no -/// more output while the queue is full, so the workload waits on its pipes while the agent serves -/// control requests. `STOP` cancels it, and once its `EXIT` reports that, `stop` holds the -/// `STOP`'s request ID. +/// An execution has no output cap. Its workload reads a pipe, into which each turn writes what +/// the host sent in `STDIN` frames, under [`STDIN_WINDOW`], and acknowledges what it wrote, and +/// which it closes after `STDIN_END`. The session streams while it runs: its records go through +/// the send queue, which never waits for the console, and the agent reads no more output while +/// the queue is full, so the workload waits on its pipes while the agent serves control requests. +/// `STOP` cancels it, and once its `EXIT` reports that, `stop` holds the `STOP`'s request ID. pub(crate) fn run( system: &mut impl System, session: &mut Session, @@ -68,12 +66,9 @@ pub(crate) fn run( Err(LaunchError::Other) => return session.send_error(request_id, 125, LAUNCH_FAILED), }; let started = Instant::now(); - let mut state = RunState { - v2: session.v2(), - ..RunState::default() - }; + let mut state = RunState::default(); let mut buffer = [0u8; OUTPUT_CHUNK_BYTES]; - session.stream(state.v2); + session.stream(true); while state.running(&workload) || (workload.has_open_output() && !state.ending()) { let session_ready = match poll( &workload, @@ -88,7 +83,6 @@ pub(crate) fn run( } }; if session_ready { - let stop = state.v2.then_some(&mut *stop); match handle_control(system, session, request_id, &mut state.input, stop) { Ok(ExecControl::None) => {} Ok(ExecControl::Cancel) if state.running(&workload) && !state.cancelled => { @@ -120,7 +114,6 @@ pub(crate) fn run( return Err(error); } } - session.answer_in(state.v2); } if !state.session_lost { // The space that the console freed takes the input's acknowledgement before more @@ -128,7 +121,7 @@ pub(crate) fn run( session.send_queued()?; state.input.write(&mut workload, session, request_id)?; } - let output = if state.session_lost || state.output_limited { + let output = if state.session_lost { &mut [][..] } else { &mut buffer[..] @@ -144,7 +137,7 @@ pub(crate) fn run( } } let outcome = if state.ending() { - if !state.session_lost && !state.output_limited { + if !state.session_lost { state.forward(system, session, request_id, &mut workload, &mut buffer)?; } state.forward(system, session, request_id, &mut workload, &mut [])?; @@ -197,8 +190,6 @@ pub(crate) fn run( ) } else if state.timed_out { session.send_app(APP_EXIT, request_id, 124, b"timeout") - } else if state.output_limited { - session.send_app(APP_EXIT, request_id, 125, b"output-limit") } else { match outcome { Some(status) => match (status.code(), status.signal()) { @@ -347,12 +338,6 @@ impl Workload { } } - fn discard(&mut self, stream: Stream) { - match self { - Self::Direct(workload) => workload.discard(stream), - } - } - fn wait(&mut self, session: &Session) -> std::io::Result { loop { self.collect(session)?; @@ -495,19 +480,12 @@ impl Input { /// outcome that it reports, and the workload's launch report and outcome. #[derive(Default)] struct RunState { - /// Whether the execution is of NVXC version 2, which streams the workload's input, and caps - /// no output. - v2: bool, /// The workload's standard input, in version 2. input: Input, - /// Bytes of output forwarded so far, which the 1 MiB cap bounds. - output_bytes: usize, /// Whether the host cancelled the workload while it ran. cancelled: bool, /// Whether the workload's timeout passed while it ran. timed_out: bool, - /// Whether the workload's output exceeded the cap. - output_limited: bool, /// Whether a reset detached the workload's session, after which its output is discarded. session_lost: bool, /// Whether the agent cannot collect the workload's status, as when another collector took @@ -522,7 +500,7 @@ struct RunState { impl RunState { /// Whether the agent ended the workload, fixing its outcome or abandoning its session. fn ending(&self) -> bool { - self.cancelled || self.timed_out || self.output_limited || self.session_lost + self.cancelled || self.timed_out || self.session_lost } /// Returns whether the workload may still be running: its status is neither known nor lost. @@ -533,10 +511,9 @@ impl RunState { } /// Takes the workload's events until it has no step to take, forwarding its output as NVXC - /// frames, until a read that the agent cannot forward: one beyond the cap, after which it - /// reads and drops the rest of the output, or a failure to read or send, which ends the - /// workload and the turn, as the C agent's did. A failure to collect the workload's status, - /// the execution's only one, ends the turn too. + /// frames, until a read that the agent cannot forward: a failure to read or send, which ends + /// the workload and the turn, as the C agent's did. A failure to collect the workload's + /// status, the execution's only one, ends the turn too. fn forward( &mut self, system: &mut impl System, @@ -552,7 +529,7 @@ impl RunState { if let Some((id, status)) = session.take_status() { workload.observe(id, status); } - let chunk = if session.queue_full() || (self.v2 && read) { + let chunk = if session.queue_full() || read { &mut [][..] } else { &mut *buffer @@ -567,18 +544,6 @@ impl RunState { }; match event { Event::Output(stream, bytes) => { - if !self.v2 && self.output_bytes > protocol::MAX_OUTPUT_BYTES - bytes.len() { - self.output_limited = true; - // The cap ends forwarding: read and drop the rest of both streams, - // as a lost session does, so the client receives a prefix of what the - // workload wrote, not a stream with a silent gap where a later chunk - // fit the leftover room. - workload.discard(Stream::Stdout); - workload.discard(Stream::Stderr); - workload.terminate(); - return Ok(()); - } - self.output_bytes += bytes.len(); read = true; let kind = match stream { Stream::Stdout => APP_STDOUT, @@ -691,7 +656,7 @@ fn launch( execution: &GuestExecPayload, ) -> Result { let mut request = launch::direct_request(config, execution)?; - request.input = session.v2(); + request.input = true; let workload = system.start_direct(request).map_err(LaunchError::Start)?; session.watch(workload.id()); Ok(Workload::Direct(workload)) @@ -777,16 +742,15 @@ enum ExecControl { Capture, } -/// Processes one control-console record while a workload runs: in version 2, the workload's -/// standard input, of which bytes beyond the window or after its end end the session, and, for an -/// execution of version 2, whose `stop` is given, a `STOP`, which cancels the workload and sets -/// `stop`. +/// Processes one control-console record while a workload runs: the workload's standard input, of +/// which bytes beyond the window or after its end end the session, and a `STOP`, which cancels +/// the workload and sets `stop`. fn handle_control( system: &mut impl System, session: &mut Session, exec_id: u64, input: &mut Input, - stop: Option<&mut Option>, + stop: &mut Option, ) -> Result { let payload = match session.next_event() { Ok(SessionEvent::Reset) => { @@ -794,9 +758,7 @@ fn handle_control( // earlier epoch's `STOP` is abandoned with its work. session.stream(false); session.acknowledge()?; - if let Some(stop) = stop { - *stop = None; - } + *stop = None; return Ok(ExecControl::SessionLost); } Ok(SessionEvent::Data(payload)) => payload, @@ -819,10 +781,7 @@ fn handle_control( return Err(failure.into_error()); } }; - if request.kind == APP_STOP - && session.v2() - && let Some(stop) = stop - { + if request.kind == APP_STOP { *stop = Some(request.request_id); return Ok(ExecControl::Cancel); } diff --git a/nvx-rs/src/guest/managed_agent/mod.rs b/nvx-rs/src/guest/managed_agent/mod.rs index e6055dffc..344d3c00e 100644 --- a/nvx-rs/src/guest/managed_agent/mod.rs +++ b/nvx-rs/src/guest/managed_agent/mod.rs @@ -5,17 +5,15 @@ //! With `ROOTFS` `-`, the direct agent runs as PID 1: it applies the host mapping table beneath //! its own root, `/`, launches each workload through nvx-init's plain-process core, reaps orphaned //! processes, and powers the VM off after `STOP`. It refuses any other `ROOTFS`. It refuses -//! workloads until the table is complete, and keeps the C agent's command lines, wire behavior, -//! status and error categories, 1 MiB output and receive windows, and timeout and cancellation -//! behavior. +//! workloads until the table is complete, and keeps the C agent's command lines, status and error +//! categories, 1 MiB receive window, and timeout and cancellation behavior. //! -//! It also serves NVXC version 2, whose frames carry the same requests, and in which each answer -//! of a request takes its version. An execution of version 2 runs as one of version 1 does, alone, -//! and a second `EXEC` gets `BUSY`, but its workload reads a pipe of the standard input that the -//! host streams, which the agent acknowledges as it writes it, up to its explicit end; its output -//! has no cap; and `STOP` ends it. While it runs, the agent's records wait in a send queue rather -//! than for the console, and the agent reads none of the workload's output while the queue is -//! full. +//! It serves NVXC version 2, whose frames carry the requests of version 1, which the C agent +//! spoke, and refuses a frame of any other version. An execution runs alone, and a second `EXEC` +//! gets `BUSY`; its workload reads a pipe of the standard input that the host streams, which the +//! agent acknowledges as it writes it, up to its explicit end; its output has no cap; and `STOP` +//! ends it. While it runs, the agent's records wait in a send queue rather than for the console, +//! and the agent reads none of the workload's output while the queue is full. //! //! The agent exits with 0, or with 125 when it fails. //! @@ -36,9 +34,8 @@ //! or Rust cleanup before `execve` or `_exit`. The execution reads the workload's output, //! ends it, and reports its launch and outcome, and the //! agent forwards the output as NVXC frames, serves the control session while the workload runs, -//! and reports the outcome. The agent keeps its own policy: the null device as the workload's -//! standard input and the output cap of version 1, the identity, -//! the timeout, the precedence of a cancellation, a timeout, and the cap over the workload's own +//! and reports the outcome. The agent keeps its own policy: the identity, +//! the timeout, the precedence of a cancellation and a timeout over the workload's own //! status, and the outcome that it reports for a failed launch stage. It builds the workload's //! environment in memory, as C's environment functions changed it, and passes it to `execve`, so //! that no process changes its @@ -64,8 +61,7 @@ mod diagnostics; mod dispatch; /// Environments that the agent builds for `execve`. mod environment; -/// The NVXC adapter, which runs each workload as an execution, and its output cap, cancellation, -/// and timeout. +/// The NVXC adapter, which runs each workload as an execution, and its cancellation and timeout. mod exec; /// Direct launch commands. mod launch; diff --git a/nvx-rs/src/guest/managed_agent/session.rs b/nvx-rs/src/guest/managed_agent/session.rs index aac1bbf22..d945351cf 100644 --- a/nvx-rs/src/guest/managed_agent/session.rs +++ b/nvx-rs/src/guest/managed_agent/session.rs @@ -21,11 +21,11 @@ const SEND_QUEUE_BYTES: usize = 64 * 1024; /// The agent's control session: the guest's role of the shared NVXS session on the control /// console, and the NVXC frames that its DATA records carry. /// -/// It serves requests of NVXC versions 1 and 2, which keep the codes of the kinds that both have, -/// and answers each in the version of the request that it serves. While an execution of version -/// 2 runs, the session's records go through a send queue, which never waits for the console, so -/// that the agent keeps serving control requests while the host does not read; the record that -/// it sends next without streaming waits until the console has taken the queue and itself. +/// It serves requests of NVXC version 2, which keeps the codes of the kinds that version 1 had, +/// and refuses a frame of any other version as a malformed request. While an execution runs, the +/// session's records go through a send queue, which never waits for the console, so that the +/// agent keeps serving control requests while the host does not read; the record that it sends +/// next without streaming waits until the console has taken the queue and itself. pub(crate) struct Session<'r> { /// The guest's NVXS connection on the owned control-console descriptor. connection: GuestConnection>, @@ -34,9 +34,6 @@ pub(crate) struct Session<'r> { /// Error of the connection's latest failure, which a connection that a failure ended reports /// again. last_error: Option, - /// Whether the request that the session serves is of version 2, whose frames its answers - /// take. - v2: bool, } /// Byte transport only: the existing connection still owns all record framing. @@ -205,10 +202,10 @@ pub(crate) enum Event { Data(Vec), } -/// An NVXC request that the session accepted, whose answers take its version. +/// An NVXC request that the session accepted. pub(crate) enum Request<'a> { - /// A request of version 1, or one of version 2 of a kind that version 1 has too, with the - /// code that both give it. + /// A request other than an execution's standard input, with the code that version 2 gives + /// its kind, as version 1 did. App(protocol::AppFrame<'a>), /// Version 2's standard input of the execution of this ID: a `STDIN` frame's bytes, or `None` /// for its `STDIN_END`. @@ -293,7 +290,6 @@ impl<'r> Session<'r> { }), capture: None, last_error: None, - v2: false, } } @@ -501,8 +497,7 @@ impl<'r> Session<'r> { )); } }; - // Each joined request gets its Ready in its own version. - let mut replies = vec![(request.request_id, self.v2)]; + let mut replies = vec![request.request_id]; while sys::readable_or_hung_up(&[self.descriptor().borrowed()], 0)?[0] { match self .next_event() @@ -539,7 +534,7 @@ impl<'r> Session<'r> { ) { return Err(Errno::EPROTO); } - replies.push((retry.request_id, self.v2)); + replies.push(retry.request_id); } } } @@ -549,13 +544,10 @@ impl<'r> Session<'r> { .map_err(activation_failure)?; (Some(retained), ready, replies) } - ActivationAction::Ready(ready) => { - (None, ready, vec![(request.request_id, self.v2)]) - } + ActivationAction::Ready(ready) => (None, ready, vec![request.request_id]), _ => return Err(Errno::EPROTO), }; - for (id, v2) in replies { - self.v2 = v2; + for id in replies { self.send_app(protocol::APP_READY, id, 0, &ready.encode())?; } self.connection.get_ref().deadline.set(None); @@ -674,63 +666,43 @@ impl<'r> Session<'r> { acknowledged.map_err(|error| self.error(error)) } - /// Decodes the NVXC request that a DATA payload carries, of version 1 or 2, then returns the + /// Decodes the NVXC request that a DATA payload carries, of version 2, then returns the /// payload's bytes to the receive window before the agent serves the request, as the C agent - /// did. The session answers in the request's version until it accepts another. + /// did. A frame of another version, such as version 1, is malformed. pub(crate) fn accept_request<'a>( &mut self, payload: &'a [u8], ) -> Result, DataFailure> { // A DATA payload has at most `OUTER_MAX_PAYLOAD` bytes, so its length fits either type. let status = payload.len() as i32; - let streams = payload.get(4) == Some(&v2::VERSION); - let request = if streams { - v2::Frame::decode(payload) - .ok() - .map(|(request_id, frame)| match frame { - v2::Frame::Stdin(bytes) => Request::Stdin(request_id, Some(bytes)), - v2::Frame::StdinEnd => Request::Stdin(request_id, None), - // Version 2 keeps version 1's header, and the codes of the kinds that both - // have, which its decoder checked. - _ => Request::App(protocol::AppFrame { - kind: payload[5], - request_id, - status: 0, - payload: &payload[protocol::APP_HEADER_LEN..], - }), - }) - } else { - protocol::decode_app(payload) - .ok() - .filter(|frame| frame.request_id != 0 && frame.status == 0) - .map(Request::App) - } - .ok_or_else(|| data_failure(DataFailureKind::Application, status))?; + let request = v2::Frame::decode(payload) + .ok() + .map(|(request_id, frame)| match frame { + v2::Frame::Stdin(bytes) => Request::Stdin(request_id, Some(bytes)), + v2::Frame::StdinEnd => Request::Stdin(request_id, None), + // Version 2 keeps version 1's header, and the codes of the kinds that both + // have, which its decoder checked. + _ => Request::App(protocol::AppFrame { + kind: payload[5], + request_id, + status: 0, + payload: &payload[protocol::APP_HEADER_LEN..], + }), + }) + .ok_or_else(|| data_failure(DataFailureKind::Application, status))?; let credited = self.connection.credit(payload.len() as u32); credited.map_err(|error| DataFailure { kind: DataFailureKind::Credit, status, error: self.error(error), })?; - self.v2 = streams; Ok(request) } - /// Whether the session answers in version 2. - pub(crate) fn v2(&self) -> bool { - self.v2 - } - - /// Answers in version 2, or in version 1, such as an execution's frames after a control - /// request of its own version. - pub(crate) fn answer_in(&mut self, v2: bool) { - self.v2 = v2; - } - - /// Encodes and sends one application frame, in one DATA record, in the session's version. + /// Encodes and sends one application frame of version 2, in one DATA record. /// - /// In version 2, `READY` starts with the protocol version, an `ERROR` of the `busy` category - /// is `BUSY`, and an `EXIT` names its outcome as version 1 does. + /// `READY` starts with the protocol version, an `ERROR` of the `busy` category is `BUSY`, and + /// an `EXIT` names its outcome as version 1 did. pub(crate) fn send_app( &mut self, kind: u8, @@ -738,36 +710,26 @@ impl<'r> Session<'r> { status: i32, payload: &[u8], ) -> Result<(), Errno> { - let frame = if self.v2 { - let frame = match kind { - APP_READY => v2::Frame::Ready { - version: v2::VERSION, - activation: payload, - }, - APP_STDOUT => v2::Frame::Stdout(payload), - APP_STDERR => v2::Frame::Stderr(payload), - APP_EXIT => v2::Frame::Exit { - status, - outcome: outcome(payload), - }, - APP_STOPPED => v2::Frame::Stopped, - APP_ERROR if payload == protocol::BUSY.as_bytes() => v2::Frame::Busy, - APP_ERROR => v2::Frame::Error { - status, - category: payload, - }, - _ => return Err(Errno::EINVAL), - }; - frame.encode(request_id).map_err(|_| Errno::EMSGSIZE)? - } else { - if protocol::APP_HEADER_LEN - .checked_add(payload.len()) - .is_none_or(|length| length > OUTER_MAX_PAYLOAD) - { - return Err(Errno::EMSGSIZE); - } - protocol::encode_app(kind, request_id, status, payload) + let frame = match kind { + APP_READY => v2::Frame::Ready { + version: v2::VERSION, + activation: payload, + }, + APP_STDOUT => v2::Frame::Stdout(payload), + APP_STDERR => v2::Frame::Stderr(payload), + APP_EXIT => v2::Frame::Exit { + status, + outcome: outcome(payload), + }, + APP_STOPPED => v2::Frame::Stopped, + APP_ERROR if payload == protocol::BUSY.as_bytes() => v2::Frame::Busy, + APP_ERROR => v2::Frame::Error { + status, + category: payload, + }, + _ => return Err(Errno::EINVAL), }; + let frame = frame.encode(request_id).map_err(|_| Errno::EMSGSIZE)?; let sent = self.connection.send(&frame); sent.map_err(|error| self.error(error)) } diff --git a/nvx-rs/src/guest/managed_agent/session/tests.rs b/nvx-rs/src/guest/managed_agent/session/tests.rs index 6363f81af..eb1a3c0ef 100644 --- a/nvx-rs/src/guest/managed_agent/session/tests.rs +++ b/nvx-rs/src/guest/managed_agent/session/tests.rs @@ -7,10 +7,12 @@ use super::*; use crate::guest::snapshot::{CaptureCause, CaptureFailure, CaptureStage}; use crate::protocol::{ APP_CAPTURE, APP_READY, OUTER_ACK, OUTER_CREDIT, OUTER_DATA, OUTER_GUEST_ATTACH, - OUTER_HEADER_LEN, OUTER_RESET, OuterHeader, decode_app, decode_outer_header, encode_outer, + OUTER_HEADER_LEN, OUTER_RESET, OuterHeader, decode_outer_header, encode_outer, }; const INSTANCE: [u8; 16] = [7; 16]; +/// The kind of `BUSY`, with which the agent refuses a request while an execution runs. +const APP_BUSY: u8 = 0x87; impl Session<'_> { pub(crate) fn activation_state_for_test(&self) -> Option { @@ -50,34 +52,29 @@ impl Peer { (header, payload) } + /// Reads the agent's next frame, of version 2, and checks its kind, request ID and status, and + /// its payload, which follows a `READY`'s protocol version. fn app(&mut self, kind: u8, id: u64, status: i32, payload: &[u8]) { let (header, bytes) = self.read(); assert_eq!(header.record_type, OUTER_DATA); assert_eq!(header.epoch, self.epoch); - let frame = decode_app(&bytes).unwrap(); + let (request_id, frame) = v2::Frame::decode(&bytes).unwrap(); + let mut carried = &bytes[protocol::APP_HEADER_LEN..]; + if let v2::Frame::Ready { + version, + activation, + } = frame + { + assert_eq!(version, v2::VERSION); + carried = activation; + } + let frame_status = i32::from_le_bytes(bytes[16..20].try_into().unwrap()); assert_eq!( - (frame.kind, frame.request_id, frame.status, frame.payload), + (bytes[5], request_id, frame_status, carried), (kind, id, status, payload) ); } - fn send(&mut self, kind: u8, id: u64, payload: &[u8]) { - let bytes = protocol::encode_app(kind, id, 0, payload); - self.stream - .write_all( - &encode_outer( - OUTER_DATA, - &INSTANCE, - self.epoch, - self.host_sequence, - &bytes, - ) - .unwrap(), - ) - .unwrap(); - self.host_sequence += 1; - } - fn send_v2(&mut self, id: u64, frame: v2::Frame<'_>) { let bytes = frame.encode(id).unwrap(); let record = encode_outer( @@ -138,7 +135,7 @@ fn restored() -> (Session<'static>, Peer, Vec) { } fn activate_bytes(session: &mut Session<'_>, peer: &mut Peer, id: u64, payload: &[u8]) -> Vec { - peer.send(protocol::APP_ACTIVATE, id, payload); + peer.send_v2(id, v2::Frame::Activate(payload)); let Event::Data(bytes) = session .next_event() .unwrap_or_else(|_| panic!("activate DATA")) @@ -169,7 +166,7 @@ fn a_replacement_session_joins_one_activation_and_caches_ready_without_reapplyin (1000, 1001, true) ); peer.reset(2, 4); - peer.send(protocol::APP_ACTIVATE, 62, &payload); + peer.send_v2(62, v2::Frame::Activate(&payload)); Ok("one applied space") }) .unwrap(); @@ -201,37 +198,6 @@ fn a_replacement_session_joins_one_activation_and_caches_ready_without_reapplyin ); } -#[test] -fn joined_activations_get_ready_in_their_own_versions() { - let (mut session, mut peer, payload) = restored(); - let (record, _) = protocol::decode_activate_payload(&payload).unwrap(); - let ready = crate::activation::ReadyActivation::of(&record).encode(); - let bytes = activate_bytes(&mut session, &mut peer, 61, &payload); - let Ok(Request::App(request)) = session.accept_request(&bytes) else { - panic!("valid activate"); - }; - peer.credit(); - let retained = session.activate_with(request, |_, _, _, _| { - peer.send_v2(62, v2::Frame::Activate(&payload)); - Ok("one applied space") - }); - assert_eq!(retained.unwrap(), Some("one applied space")); - peer.credit(); - peer.app(APP_READY, 61, 0, &ready); - let (_, frame) = peer.read(); - let activation = ready.as_slice(); - assert_eq!( - v2::Frame::decode(&frame).unwrap(), - ( - 62, - v2::Frame::Ready { - version: v2::VERSION, - activation - } - ) - ); -} - #[test] fn postactivation_capture_stays_refused_without_pending_cleanup() { let (mut session, mut peer, payload) = restored(); @@ -278,7 +244,7 @@ fn changed_record_or_workload_bytes_during_application_fail_without_ready() { }; peer.credit(); let result = session.activate_with(request, |_, _, _, _| { - peer.send(protocol::APP_ACTIVATE, 65, &changed); + peer.send_v2(65, v2::Frame::Activate(&changed)); Ok(()) }); assert_eq!(result.err(), Some(Errno::EPROTO)); @@ -355,7 +321,7 @@ fn a_failed_restored_activation_gives_init_the_status_of_its_class() { }; peer.credit(); let result = session.activate_with(request, |_, _, _, _| { - peer.send(protocol::APP_PING, 70, &[]); + peer.send_v2(70, v2::Frame::Ping); Ok(()) }); assert_eq!(result.err(), Some(Errno::EPROTO)); @@ -429,7 +395,8 @@ fn reconnect_and_partial_input_wait_only_until_the_original_activation_deadline( fn an_unread_output_stream_cannot_extend_the_activation_deadline() { let (mut session, _peer, _) = restored(); let deadline = short_deadline(&mut session); - let payload = vec![0; OUTER_MAX_PAYLOAD - protocol::APP_HEADER_LEN]; + // The largest payload that a `READY` carries after its protocol version. + let payload = vec![0; v2::MAX_PAYLOAD - 1]; let error = loop { if let Err(error) = session.send_app(APP_READY, 67, 0, &payload) { break error; @@ -446,7 +413,7 @@ fn queue( payload: &[u8], active: bool, ) -> bool { - peer.send(APP_CAPTURE, id, payload); + peer.send_v2(id, v2::Frame::Capture(payload)); let Event::Data(bytes) = session .next_event() .unwrap_or_else(|_| panic!("valid request DATA")) @@ -463,15 +430,16 @@ fn queue( #[test] fn capture_refuses_active_execution_without_kill_and_validates_the_existing_flag() { - for (payload, status, category) in [ - (b"\0".as_slice(), Errno::EBUSY, protocol::BUSY), - (b"\x02".as_slice(), Errno::EINVAL, protocol::INVALID_REQUEST), - (b"".as_slice(), Errno::EINVAL, protocol::INVALID_REQUEST), + let invalid = protocol::INVALID_REQUEST.as_bytes(); + for (payload, kind, status, category) in [ + (b"\0".as_slice(), APP_BUSY, 0, b"".as_slice()), + (b"\x02".as_slice(), APP_ERROR, Errno::EINVAL as i32, invalid), + (b"".as_slice(), APP_ERROR, Errno::EINVAL as i32, invalid), ] { let (mut session, mut peer) = connected(true); assert!(!queue(&mut session, &mut peer, 31, payload, true)); assert!(!session.capture_pending()); - peer.app(APP_ERROR, 31, status as i32, category.as_bytes()); + peer.app(kind, 31, status, category); } let (mut session, mut peer) = connected(true); assert!(queue(&mut session, &mut peer, 32, &[1], true)); @@ -624,19 +592,15 @@ fn restored_capture_consumes_reset_and_acknowledges_the_existing_connection() { assert!(session.rootless()); assert!(session.capture_status().is_none()); assert_eq!(session.connection.session().identity().unwrap().epoch, 2); - let bytes = protocol::encode_app(APP_CAPTURE, 38, 0, &[0]); - assert!( - !session - .queue_capture(decode_app(&bytes).unwrap(), false) - .unwrap() - ); + let request = protocol::AppFrame { + kind: APP_CAPTURE, + request_id: 38, + status: 0, + payload: &[0], + }; + assert!(!session.queue_capture(request, false).unwrap()); assert!(!session.capture_pending()); - peer.app( - APP_ERROR, - 38, - Errno::EBUSY as i32, - protocol::BUSY.as_bytes(), - ); + peer.app(APP_BUSY, 38, 0, b""); peer.stream.set_nonblocking(true).unwrap(); assert_eq!( peer.stream.read(&mut [0]).unwrap_err().kind(), 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 9667d2b28..7d6c952cc 100644 --- a/nvx-rs/src/guest/managed_agent/tests/loopback/tests.rs +++ b/nvx-rs/src/guest/managed_agent/tests/loopback/tests.rs @@ -4,8 +4,7 @@ use std::time::Instant; use crate::host::session::{ControlSession, ExecEvent, SessionError}; use crate::protocol::v2::Outcome; use crate::protocol::{ - APP_EXEC, APP_EXIT, APP_STDERR, APP_STDOUT, CAPABILITY_LEN, HostMap, Workload, - WorkloadEnvironment, decode_app, encode_app, encode_exec_payload, + CAPABILITY_LEN, HostMap, Workload, WorkloadEnvironment, encode_exec_payload, }; use super::super::AgentConfig; @@ -56,29 +55,6 @@ fn run_exec( 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( @@ -243,86 +219,3 @@ fn control_session_broker_and_real_agent_loop_back() { assert_eq!(system.kernel.moves(), [target.display().to_string()]); assert!(system.diagnostics.is_empty()); } - -/// 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() { - if super::super::isolate_process_test( - "NVX_AGENT_OUTPUT_LIMIT_CHILD", - "guest::managed_agent::tests::loopback::tests::output_limit_forwards_nothing_after_the_cap", - ) { - return; - } - let _process = super::super::PROCESS_TEST_LOCK.lock().unwrap(); - let (temporary, mut system) = TestSystem::with_direct(); - system.paths.hostfs_dir = temporary.path().join("hostfs"); - std::fs::create_dir_all(system.paths.hostfs_dir.join("root/0/source")).unwrap(); - // The workload uses the plain-process core with simulated credential calls. - system.direct_starter = Some(start_as_privileged); - let target = temporary.path().canonicalize().unwrap().join("mapped"); - let (broker_guest, agent) = UnixStream::pair().unwrap(); - let agent_descriptor = FileDescriptor::from_owned(agent.into()); - let agent_thread = thread::spawn(move || { - let mut system = system; - let config = AgentConfig { - uid: c"1000".to_owned(), - gid: c"1000".to_owned(), - user: c"workload".to_owned(), - home: c"/".to_owned(), - restored_proxy: None, - }; - let mut session = Session::new(agent_descriptor); - let mut mappings = HostMappings::announced(1); - let outcome = dispatch::run(&mut system, &mut session, &config, &mut mappings).unwrap(); - (outcome, system) - }); - let core = Arc::new(Mutex::new(BrokerCore::new(broker_guest))); - let broker = BrokerHandle { core }; - let capability = [0x5a; CAPABILITY_LEN]; - let mut session = ControlSession::attach(broker.transport(), &capability, deadline()).unwrap(); - session.ping(deadline()).unwrap(); - session - .map_host_paths( - &[HostMap { - source: "0/source".to_owned(), - target: target.to_string_lossy().into_owned(), - read_only: true, - }], - deadline(), - ) - .unwrap(); - - // 32 KiB of 'b', which is one full read of the agent, then 50 bytes of 'c', in one - // write, after 1 MiB minus `room` 'a's that the agent forwards first. - let chunk = temporary.path().join("chunk"); - let mut bytes = vec![b'b'; 32 * 1024]; - bytes.extend_from_slice(&[b'c'; 50]); - std::fs::write(&chunk, &bytes).unwrap(); - let room = 100usize; - let script = format!( - "head -c {} /dev/zero | tr '\\0' a; sleep 1; dd if={} bs=65536 count=1 2>/dev/null; sleep 10", - crate::protocol::MAX_OUTPUT_BYTES - room, - chunk.display() - ); - let workload = Workload { - argv: vec!["/bin/sh".to_owned(), "-c".to_owned(), script], - timeout_ms: 0, - cwd: None, - environment: WorkloadEnvironment::Default, - }; - // 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, (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); - assert_eq!(count(b'c'), 0, "output after a dropped chunk was forwarded"); - let tail = &stdout[stdout.len().saturating_sub(room)..]; - assert!(tail.iter().all(|&each| each == b'a' || each == b'b')); - session.stop(deadline()).unwrap(); - drop(session); - let _ = agent_thread.join().unwrap(); -} diff --git a/nvx-rs/src/host/restore/tests.rs b/nvx-rs/src/host/restore/tests.rs index 14b20f739..3e0b9d40f 100644 --- a/nvx-rs/src/host/restore/tests.rs +++ b/nvx-rs/src/host/restore/tests.rs @@ -1,14 +1,7 @@ #![cfg(any(target_os = "linux", windows))] #![cfg_attr(windows, allow(unsafe_code))] -// 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") - ) -))] +#[cfg(feature = "cli")] mod live_init; use std::io::{BufRead, BufReader, Write}; diff --git a/nvx-rs/src/host/restore/tests/live_init.rs b/nvx-rs/src/host/restore/tests/live_init.rs index 0b9aa4ec4..5d9af1746 100644 --- a/nvx-rs/src/host/restore/tests/live_init.rs +++ b/nvx-rs/src/host/restore/tests/live_init.rs @@ -16,15 +16,13 @@ use crate::host::nvxs::HostConnection; use crate::host::session; use crate::host::snapshot::SnapshotManifest; use crate::host::spawn::{Launched, VmmCommand, spawn_vm_with_mode}; +use crate::protocol::v2::{self, Frame, Outcome}; use crate::protocol::{ - APP_ERROR, APP_EXEC, APP_EXIT, APP_MAPS, APP_READY, APP_STDERR, APP_STDOUT, APP_STOP, - APP_STOPPED, CAPABILITY_LEN, HostMap, Workload, WorkloadEnvironment, decode_app, encode_app, - encode_exec_payload, encode_maps_payloads, + CAPABILITY_LEN, HostMap, Workload, WorkloadEnvironment, encode_exec_payload, + encode_maps_payloads, }; type AnyError = Box; -const ACTIVATE: u8 = 8; -const CAPTURE: u8 = 7; struct OwnedVm(Option); impl Drop for OwnedVm { @@ -92,11 +90,12 @@ fn ready( by: Instant, ) -> Result<(), AnyError> { let bytes = connection.receive(Some(by))?; - let frame = decode_app(&bytes)?; - require( - (frame.kind, frame.request_id, frame.status, frame.payload) == (APP_READY, id, 0, expected), - format!("unexpected Ready: {frame:?}"), - ) + let frame = Frame::decode(&bytes)?; + let ready = Frame::Ready { + version: v2::VERSION, + activation: expected, + }; + require(frame == (id, ready), format!("unexpected Ready: {frame:?}")) } fn workload(script: &str, environment: WorkloadEnvironment<'_>) -> Result, AnyError> { @@ -116,23 +115,25 @@ fn execute( by: Instant, ) -> Result { connection.send( - &encode_app(APP_EXEC, id, 0, &workload(script, environment)?), + &Frame::Exec(&workload(script, environment)?).encode(id)?, Some(by), )?; + // The workload reads no standard input. + connection.send(&Frame::StdinEnd.encode(id)?, Some(by))?; let mut output = Vec::new(); loop { let bytes = connection.receive(Some(by))?; - let frame = decode_app(&bytes)?; + let (request_id, frame) = Frame::decode(&bytes)?; require( - frame.request_id == id, + request_id == id, format!("wrong execution response: {frame:?}"), )?; - match frame.kind { - APP_STDOUT => output.extend_from_slice(frame.payload), - APP_STDERR => eprint!("{}", String::from_utf8_lossy(frame.payload)), - APP_EXIT => { + match frame { + Frame::Stdout(chunk) => output.extend_from_slice(chunk), + Frame::Stderr(chunk) => eprint!("{}", String::from_utf8_lossy(chunk)), + Frame::Exit { status, outcome } => { require( - frame.status == 0 && frame.payload == b"exit", + status == 0 && outcome == Outcome::Exit, format!("execution failed: {frame:?}"), )?; return Ok(String::from_utf8(output)?); @@ -410,12 +411,12 @@ fn run() -> Result<(), AnyError> { transaction: Some(transaction), } .encode(); - connection.send(&encode_app(ACTIVATE, 100, 0, &activation), Some(by))?; + connection.send(&Frame::Activate(&activation).encode(100)?, Some(by))?; ready(&mut connection, 100, &expected_ready, by)?; println!("ACTIVATION_READY_PUBLICATION={published} OWN_DNS=10.88.0.53"); if case == "changed" { activation[0] ^= 1; - connection.send(&encode_app(ACTIVATE, 101, 0, &activation), Some(by))?; + connection.send(&Frame::Activate(&activation).encode(101)?, Some(by))?; require( process .wait(Instant::now() + Duration::from_secs(15))? @@ -426,20 +427,17 @@ fn run() -> Result<(), AnyError> { println!("CHANGED_IDENTITY_REPLAY_FATAL"); } else { connection.send( - &encode_app( - APP_EXEC, - 102, - 0, - &workload("true", WorkloadEnvironment::Default)?, - ), + &Frame::Exec(&workload("true", WorkloadEnvironment::Default)?).encode(102)?, Some(by), )?; let bytes = connection.receive(Some(by))?; - let frame = decode_app(&bytes)?; + let frame = Frame::decode(&bytes)?; + let refusal = Frame::Error { + status: 11, + category: b"mappings-incomplete", + }; require( - frame.kind == APP_ERROR - && frame.status == 11 - && frame.payload == b"mappings-incomplete", + frame == (102, refusal), format!("EXEC bypassed MAPS gate: {frame:?}"), )?; let maps = [HostMap { @@ -448,7 +446,7 @@ fn run() -> Result<(), AnyError> { read_only: true, }]; for payload in encode_maps_payloads(&maps, 32768)? { - connection.send(&encode_app(APP_MAPS, 103, 0, &payload), Some(by))?; + connection.send(&Frame::Maps(&payload).encode(103)?, Some(by))?; ready(&mut connection, 103, &[], by)?; } let script = format!( @@ -525,20 +523,20 @@ printf 'REAL_READY_MAPS_EXEC_OK\n' .to_owned(); for kill in [0, 1] { connection.send( - &encode_app(CAPTURE, 106 + u64::from(kill), 0, &[kill]), + &Frame::Capture(&[kill]).encode(106 + u64::from(kill))?, Some(by), )?; let bytes = connection.receive(Some(by))?; - let frame = decode_app(&bytes)?; + let (_, frame) = Frame::decode(&bytes)?; require( - frame.kind == APP_ERROR && frame.status == 95, + matches!(frame, Frame::Error { status: 95, .. }), "activated restore accepted capture", )?; } drop(connection); let transport = session::connect(&endpoint, pid, process.reference().start_time, by)?; connection = HostConnection::attach(transport, &capability, by)?; - connection.send(&encode_app(ACTIVATE, 108, 0, &activation), Some(by))?; + connection.send(&Frame::Activate(&activation).encode(108)?, Some(by))?; ready(&mut connection, 108, &expected_ready, by)?; let replay = execute( &mut connection, @@ -559,13 +557,10 @@ printf 'RECONNECT_REPLAY_PRESERVED_UPPER_AND_MAPS\n' "retry regenerated the machine ID", )?; print!("{replay}"); - connection.send(&encode_app(APP_STOP, 110, 0, &[]), Some(by))?; + connection.send(&Frame::Stop.encode(110)?, Some(by))?; let bytes = connection.receive(Some(by))?; - let frame = decode_app(&bytes)?; - require( - frame.kind == APP_STOPPED && frame.status == 0, - "STOP not acknowledged", - )?; + let frame = Frame::decode(&bytes)?; + require(frame == (110, Frame::Stopped), "STOP not acknowledged")?; require( process .wait(Instant::now() + Duration::from_secs(30))? diff --git a/nvx-rs/src/protocol/app.rs b/nvx-rs/src/protocol/app.rs index 3c267741a..73c894f32 100644 --- a/nvx-rs/src/protocol/app.rs +++ b/nvx-rs/src/protocol/app.rs @@ -3,17 +3,15 @@ //! //! A frame is a 24-byte header (the magic, the version, the frame kind, two zero flags bytes, the //! request ID, a status, and the payload length) followed by the payload. Responses repeat the ID -//! 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. +//! of their request. `v2` holds version 2, NVXC's only version, which carries every control path +//! and streams an execution's standard input, output and error. This module holds the codes of +//! its kinds and its error categories, with which NVX's guest agents and the fake OpenVMM serve +//! it, and the payloads of the capture and activate requests. -#[cfg(any( - feature = "testing", - all(feature = "guest", target_os = "linux", target_arch = "x86_64") -))] +#[cfg(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, violation}; @@ -55,18 +53,6 @@ pub(crate) mod workload { ))] pub(crate) mod v2; -/// Magic of every application frame. -#[cfg(any( - 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 = "testing", - all(feature = "guest", target_os = "linux", target_arch = "x86_64") -))] -pub(crate) const APP_VERSION: u8 = 1; /// Length of an application frame header. #[cfg(any( feature = "cli", @@ -99,13 +85,6 @@ pub(crate) const APP_STOP: u8 = 3; all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] 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 = "testing", - all(feature = "guest", target_os = "linux", target_arch = "x86_64") -))] -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( @@ -137,12 +116,9 @@ pub(crate) const APP_STDOUT: u8 = 0x82; all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] 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 = "testing", - all(feature = "guest", target_os = "linux", target_arch = "x86_64") -))] +/// Response that ends a workload: the status is its exit status, and the payload how it ended, +/// as [`v2::Outcome`] names it. +#[cfg(all(feature = "guest", target_os = "linux", target_arch = "x86_64"))] pub(crate) const APP_EXIT: u8 = 0x84; /// Response that acknowledges [`APP_STOP`]. #[cfg(any( @@ -206,10 +182,7 @@ pub(crate) const MAPPINGS_INCOMPLETE: &str = "mappings-incomplete"; pub(crate) const MAPPING_FAILED: &str = "mapping-failed"; /// Decoded application frame. -#[cfg(any( - feature = "testing", - all(feature = "guest", target_os = "linux", target_arch = "x86_64") -))] +#[cfg(all(feature = "guest", target_os = "linux", target_arch = "x86_64"))] #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(crate) struct AppFrame<'a> { /// One of the `APP_*` frame kinds. @@ -222,52 +195,6 @@ pub(crate) struct AppFrame<'a> { pub(crate) payload: &'a [u8], } -/// Encodes one application frame. -#[cfg(any( - feature = "testing", - all(feature = "guest", target_os = "linux", target_arch = "x86_64") -))] -pub(crate) fn encode_app(kind: u8, request_id: u64, status: i32, payload: &[u8]) -> Vec { - let length = u32::try_from(payload.len()).unwrap_or(u32::MAX); - let mut frame = Vec::with_capacity(APP_HEADER_LEN + payload.len()); - frame.extend_from_slice(&APP_MAGIC); - frame.push(APP_VERSION); - frame.push(kind); - frame.extend_from_slice(&0u16.to_le_bytes()); - frame.extend_from_slice(&request_id.to_le_bytes()); - frame.extend_from_slice(&status.to_le_bytes()); - frame.extend_from_slice(&length.to_le_bytes()); - frame.extend_from_slice(payload); - frame -} - -/// Decodes and validates an application frame. -#[cfg(any( - feature = "testing", - all(feature = "guest", target_os = "linux", target_arch = "x86_64") -))] -pub(crate) fn decode_app(frame: &[u8]) -> Result, ProtocolError> { - if frame.len() < APP_HEADER_LEN { - return Err(violation( - "control endpoint returned a truncated application frame", - )); - } - let flags = u16::from_le_bytes([frame[6], frame[7]]); - let length = u32::from_le_bytes(array(&frame[20..24])) as usize; - let payload = &frame[APP_HEADER_LEN..]; - if frame[..4] != APP_MAGIC || frame[4] != APP_VERSION || flags != 0 || length != payload.len() { - return Err(violation( - "control endpoint returned an invalid application frame", - )); - } - Ok(AppFrame { - kind: frame[5], - request_id: u64::from_le_bytes(array(&frame[8..16])), - status: i32::from_le_bytes(array(&frame[16..20])), - payload, - }) -} - /// Encodes the capture request's flag as one byte, 0 or 1. #[cfg(feature = "cli")] pub(crate) fn encode_capture_payload(kill_executions: bool) -> [u8; 1] { @@ -345,60 +272,3 @@ fn check_workload_identity(workload: &WorkloadIdentity) -> Result<(), ProtocolEr } Ok(()) } - -/// Exit category reported by the guest agent, the payload of an [`APP_EXIT`] response. -#[cfg(any(feature = "cli", feature = "testing"))] -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub(crate) enum ExitCategory { - /// The workload exited; the status is its exit code. - Exit, - /// The workload's timeout elapsed and the guest agent killed it. - Timeout, - /// The workload exceeded the output limit and the guest agent killed it. - OutputLimit, - /// A signal ended the workload; the status is 128 plus the signal number. - Signal, - /// The guest agent could not supervise the workload. - Failed, - /// A cancellation request ended the workload. - Cancelled, -} - -#[cfg(any(feature = "cli", feature = "testing"))] -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), - b"timeout" => Ok(Self::Timeout), - b"output-limit" => Ok(Self::OutputLimit), - b"signal" => Ok(Self::Signal), - b"failed" => Ok(Self::Failed), - b"cancelled" => Ok(Self::Cancelled), - _ => Err(violation( - "guest agent returned an unsupported exit category", - )), - } - } - - /// Returns the payload of an [`APP_EXIT`] response that reports this category. - #[cfg(feature = "testing")] - pub(crate) const fn as_str(self) -> &'static str { - match self { - Self::Exit => "exit", - Self::Timeout => "timeout", - Self::OutputLimit => "output-limit", - Self::Signal => "signal", - Self::Failed => "failed", - Self::Cancelled => "cancelled", - } - } -} diff --git a/nvx-rs/src/protocol/exec.rs b/nvx-rs/src/protocol/exec.rs index 189b7f849..5f6e65342 100644 --- a/nvx-rs/src/protocol/exec.rs +++ b/nvx-rs/src/protocol/exec.rs @@ -29,12 +29,6 @@ pub(crate) const MAX_ARGUMENT_BYTES: usize = 4096; 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. pub(crate) const EXEC_EXTENDED: u16 = 1; diff --git a/nvx-rs/src/protocol/features.rs b/nvx-rs/src/protocol/features.rs deleted file mode 100644 index 351f004ce..000000000 --- a/nvx-rs/src/protocol/features.rs +++ /dev/null @@ -1,145 +0,0 @@ -//! 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; - -#[cfg(feature = "cli")] -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 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)] -pub(crate) struct GuestFeatures(u32); - -impl GuestFeatures { - /// What a guest agent that predates the `APP_FEATURES` request provides. - #[cfg(feature = "cli")] - pub(crate) const NONE: Self = Self(0); - /// Terminates a workload on `APP_CANCEL` and reports the `cancelled` exit category. - pub(crate) const CANCEL: Self = Self(1 << 0); - // Bit 1 announced the bind mounts that `nvx_map=` kernel command-line tokens listed. Guests - // no longer set it, so a host that still sends those tokens refuses them. - /// Creates the account that `nvx_workload_account=create` selects. Only the blockless root - /// creates accounts; the layered root's executions require an existing one. - pub(crate) const WORKLOAD_ACCOUNT: Self = Self(1 << 2); - /// Runs each workload in a cgroup of its own and kills whatever it leaves behind. - #[cfg(feature = "cli")] - pub(crate) const EXEC_CGROUP: Self = Self(1 << 3); - /// Applies the working directory and the explicit environment of an extended `EXEC` request - /// to that execution alone, and layers the environment over the default one on request. - pub(crate) const EXEC_ENVIRONMENT: Self = Self(1 << 4); - /// Starts each workload in the working directory that its `EXEC` request names, or in `/`, - /// entered with the workload's identity, and refuses the launch with the `cwd-failed` - /// category when the workload cannot enter it. - pub(crate) const EXEC_CWD: Self = Self(1 << 5); - /// Bind-mounts the host mapping table of `APP_MAPS` requests, as many entries as the - /// `nvx_maps=` kernel command-line token announces, keeps the shared export root-only, and - /// refuses workloads until every entry is mounted. - pub(crate) const HOST_MAPPING_TABLE: Self = Self(1 << 6); - /// Features announced by the direct-mode managed agent. - #[cfg(all(feature = "guest", target_os = "linux", target_arch = "x86_64"))] - pub(crate) const DIRECT_AGENT: Self = Self( - Self::CANCEL.0 - | Self::WORKLOAD_ACCOUNT.0 - | Self::EXEC_ENVIRONMENT.0 - | Self::EXEC_CWD.0 - | Self::HOST_MAPPING_TABLE.0, - ); - /// Features that the fake OpenVMM's guest announces for a layered sandbox, as the managed - /// agent's sandbox mode did. - #[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` 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 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")] - pub(crate) const LAYERED_SANDBOX: Self = - Self(Self::EXEC_ENVIRONMENT.0 | Self::HOST_MAPPING_TABLE.0); - - /// Human-readable names of every required feature, used in compatibility errors. - #[cfg(feature = "cli")] - pub(crate) const NAMES: [(Self, &'static str); 6] = [ - (Self::CANCEL, "cancellation"), - (Self::WORKLOAD_ACCOUNT, "workload accounts"), - (Self::EXEC_CGROUP, "workload containment"), - (Self::EXEC_ENVIRONMENT, "per-execution environments"), - (Self::EXEC_CWD, "working directories"), - (Self::HOST_MAPPING_TABLE, "host path mapping tables"), - ]; - - /// Returns the features whose bits are set in `bits`. - #[cfg(feature = "testing")] - pub(crate) const fn from_bits(bits: u32) -> Self { - Self(bits) - } - - /// Returns the bits of these features. - #[cfg(feature = "testing")] - pub(crate) const fn bits(self) -> u32 { - self.0 - } - - /// Decodes the payload of a features response. - /// - /// Later revisions may append fields, so trailing bytes are ignored. - #[cfg(feature = "cli")] - pub(crate) fn decode(payload: &[u8]) -> Result { - let bits = payload - .get(..4) - .ok_or_else(|| violation("the guest agent returned a truncated features response"))?; - Ok(Self(u32::from_le_bytes(array(bits)))) - } - - /// Encodes the payload of a features response, as a guest agent sends it. - #[cfg(any( - feature = "testing", - all(feature = "guest", target_os = "linux", target_arch = "x86_64") - ))] - pub(crate) const fn encode(self) -> [u8; 4] { - self.0.to_le_bytes() - } - - /// Names the features of `required` that this set lacks. - #[cfg(feature = "cli")] - pub(crate) fn missing(self, required: Self) -> Vec<&'static str> { - Self::NAMES - .iter() - .filter(|(feature, _)| required.0 & feature.0 != 0 && self.0 & feature.0 == 0) - .map(|(_, name)| *name) - .collect() - } -} - -impl BitOr for GuestFeatures { - type Output = Self; - - fn bitor(self, other: Self) -> Self { - Self(self.0 | other.0) - } -} diff --git a/nvx-rs/src/protocol/mod.rs b/nvx-rs/src/protocol/mod.rs index 39b748864..2c1c406e4 100644 --- a/nvx-rs/src/protocol/mod.rs +++ b/nvx-rs/src/protocol/mod.rs @@ -2,9 +2,9 @@ //! //! OpenVMM brokers an authenticated stream of outer records (magic `NVXS`) between one host //! client and the guest's agent. Data records carry the bytes of an application protocol. NVX's -//! own, `NVXC`, with its frames, its `EXEC` payloads and the control features that a guest image -//! advertises, is private to this crate: only NVX's programs speak it, the CLI, nvx-init, the -//! managed agent and the fake OpenVMM. All integers are little-endian. +//! own, `NVXC`, with its frames and its `EXEC` payloads, is private to this crate: only NVX's +//! programs speak it, the CLI, nvx-init, the managed agent and the fake OpenVMM. All integers are +//! little-endian. //! //! - Outer records: [`encode_outer`] and [`decode_outer_header`], and the `OUTER_*` record types. //! - `MAPS` payloads: the [`HostMap`] table that the host encodes with [`encode_maps_payloads`], @@ -29,7 +29,7 @@ //! activation failed, the time ABI's included, from which the host classifies a clone's end. //! //! The module depends on nothing outside the standard library, so the host, the guest, and test -//! doubles share it. Tests pin its wire constants as literals, and its records, frames, and +//! doubles share it. Tests pin its wire constants and frames as literals, and its records and //! payloads with the conformance vectors of the crate's `tests/vectors/`, whose bytes no codec //! produced. @@ -45,12 +45,6 @@ pub mod cmdline; ))] pub(crate) mod exec; pub mod export; -#[cfg(any( - feature = "cli", - feature = "testing", - all(feature = "guest", target_os = "linux", target_arch = "x86_64") -))] -mod features; pub mod maps; pub mod nvxs; mod outer; @@ -66,6 +60,8 @@ pub mod view; pub(crate) use app::APP_ACTIVATE; #[cfg(all(feature = "guest", target_os = "linux", target_arch = "x86_64"))] pub(crate) use app::APP_CAPTURE; +#[cfg(all(feature = "guest", target_os = "linux", target_arch = "x86_64"))] +pub(crate) use app::APP_EXIT; #[cfg(any( feature = "cli", feature = "testing", @@ -74,8 +70,6 @@ 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(feature = "testing")] -pub(crate) use app::ExitCategory; #[cfg(any( feature = "cli", feature = "testing", @@ -102,8 +96,8 @@ pub(crate) use app::workload; 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, UNSUPPORTED_OPERATION, decode_app, encode_app, + APP_CANCEL, APP_ERROR, APP_EXEC, APP_MAPS, APP_PING, APP_READY, APP_STDERR, APP_STDOUT, + APP_STOP, APP_STOPPED, UNSUPPORTED_OPERATION, }; #[cfg(any( feature = "testing", @@ -113,22 +107,12 @@ pub(crate) use app::{BUSY, CWD_FAILED, INVALID_REQUEST, LAUNCH_FAILED, MAPPINGS_ #[cfg(all(feature = "guest", target_os = "linux", target_arch = "x86_64"))] pub(crate) use app::{decode_activate_payload, decode_capture_payload}; pub use boot::{MAPS_TOKEN, parse_host_mapping_count}; -#[cfg(any( - feature = "testing", - all(feature = "guest", target_os = "linux", target_arch = "x86_64") -))] -pub(crate) use exec::MAX_OUTPUT_BYTES; #[cfg(feature = "testing")] pub(crate) use exec::decode_exec_payload; #[cfg(all(feature = "guest", target_os = "linux", target_arch = "x86_64"))] pub(crate) use exec::{GuestExecPayload, decode_guest_exec_payload}; #[cfg(feature = "cli")] pub(crate) use exec::{Workload, WorkloadEnvironment, encode_exec_payload}; -#[cfg(any( - feature = "testing", - all(feature = "guest", target_os = "linux", target_arch = "x86_64") -))] -pub(crate) use features::GuestFeatures; pub use maps::{ GuestHostMap, GuestMapsPayload, HostMap, MAP_ENTRY_HEADER_LEN, MAP_READ_ONLY, MAPS_HEADER_LEN, MAX_HOST_MAPPINGS, MAX_MAP_PATH_BYTES, MapsPayload, decode_guest_maps_payload, diff --git a/nvx-rs/src/protocol/tests.rs b/nvx-rs/src/protocol/tests.rs index dc10cc73f..628d5f93e 100644 --- a/nvx-rs/src/protocol/tests.rs +++ b/nvx-rs/src/protocol/tests.rs @@ -1,9 +1,9 @@ //! Tests of the control protocol, including its conformance vectors. //! -//! `tests/vectors/nvxc-v1.txt` holds the bytes of the records, frames, and payloads that these -//! tests pin, which come from the protocol's definition rather than from this codec. The host's -//! encoders, and the decoders that the guest agent and the fake OpenVMM share with it, must all -//! match them, so a change to the codec that every side would agree on still fails here. +//! `tests/vectors/nvxc-v1.txt` holds the bytes of the records and payloads that these tests pin, +//! which come from the protocol's definition rather than from this codec. The host's encoders, +//! and the decoders that the guest agent and the fake OpenVMM share with it, must all match them, +//! so a change to the codec that every side would agree on still fails here. #[cfg(all( feature = "testing", @@ -47,11 +47,6 @@ use crate::tests::{NVXC_V1, of_kind, vector}; all(feature = "guest", target_os = "linux", target_arch = "x86_64") ))] 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", @@ -91,36 +86,6 @@ const MAX_CWD_BYTES: usize = 4095; #[cfg(feature = "cli")] const MAX_TIMEOUT_MS: u32 = u32::MAX; -#[cfg(any( - feature = "testing", - all(feature = "guest", target_os = "linux", target_arch = "x86_64") -))] -impl GuestFeatures { - /// The features that the host once required, which the direct-mode managed agent announces. - /// Only tests read them. - #[cfg(all( - feature = "cli", - feature = "guest", - target_os = "linux", - target_arch = "x86_64" - ))] - pub(crate) const REQUIRED: Self = Self::DIRECT_AGENT; - - /// Returns whether this set holds every feature of `other`. - #[cfg(any( - all(feature = "cli", feature = "testing"), - all( - any(feature = "cli", feature = "testing"), - feature = "guest", - target_os = "linux", - target_arch = "x86_64" - ) - ))] - pub(crate) fn contains(self, other: Self) -> bool { - (self | other) == self - } -} - /// A workload that runs `argv` without a timeout in `/` with the default environment. #[cfg(feature = "cli")] fn program<'a>(argv: &[&str]) -> Workload<'a> { @@ -145,29 +110,6 @@ fn record_type(name: &str) -> RecordType { .unwrap_or_else(|| panic!("no record type is named {name}")) } -/// Returns the frame kind that a vector names. -#[cfg(any( - feature = "testing", - all(feature = "guest", target_os = "linux", target_arch = "x86_64") -))] -fn frame_kind(name: &str) -> u8 { - match name { - "PING" => APP_PING, - "EXEC" => APP_EXEC, - "STOP" => APP_STOP, - "CANCEL" => APP_CANCEL, - "FEATURES" => APP_FEATURES, - "MAPS" => APP_MAPS, - "READY" => APP_READY, - "STDOUT" => APP_STDOUT, - "STDERR" => APP_STDERR, - "EXIT" => APP_EXIT, - "STOPPED" => APP_STOPPED, - "ERROR" => APP_ERROR, - _ => panic!("no frame kind is named {name}"), - } -} - /// Returns the bytes of a vector's text as the host's text, which is UTF-8. fn utf8(bytes: &[u8]) -> String { String::from_utf8(bytes.to_vec()).unwrap() @@ -249,34 +191,6 @@ fn records_match_their_vectors() { ); } -/// Each `FRAME` vector decodes to its values, which encode to its bytes. -#[cfg(any( - feature = "testing", - all(feature = "guest", target_os = "linux", target_arch = "x86_64") -))] -#[test] -fn frames_match_their_vectors() { - let names = ["PING", "STDOUT", "EXIT", "ERROR_CWD_FAILED"]; - for vector in of_kind(NVXC_V1, "FRAME", &names) { - let name = vector.name; - let mut values = vector.values(); - let kind = frame_kind(values.word()); - let request_id: u64 = values.number(); - let status: i32 = values.number(); - let payload = values.bytes(); - drop(values); - let encoded = encode_app(kind, request_id, status, &payload); - assert_eq!(encoded, vector.bytes, "{name}"); - let frame = AppFrame { - kind, - request_id, - status, - payload: &payload, - }; - assert_eq!(decode_app(&vector.bytes).unwrap(), frame, "{name}"); - } -} - #[cfg(all( feature = "cli", feature = "guest", @@ -287,11 +201,8 @@ fn frames_match_their_vectors() { fn capture_requests_round_trip_both_kill_flags() { for (payload, kill_executions) in [([0], false), ([1], true)] { assert_eq!(app::encode_capture_payload(kill_executions), payload); - let bytes = encode_app(app::APP_CAPTURE, 42, 0, &payload); - let frame = decode_app(&bytes).unwrap(); - assert_eq!((frame.kind, frame.request_id, frame.status), (7, 42, 0)); assert_eq!( - app::decode_capture_payload(frame.payload).unwrap(), + app::decode_capture_payload(&payload).unwrap(), kill_executions ); } @@ -331,11 +242,8 @@ fn activate_requests_round_trip_records_and_workload_identity() { let payload = app::encode_activate_payload(&record, &workload).unwrap(); assert_eq!(payload[..9], [4, 3, 2, 1, 8, 7, 6, 5, flag]); assert_eq!(payload[9..], vector.bytes); - let bytes = encode_app(app::APP_ACTIVATE, 42, 0, &payload); - let frame = decode_app(&bytes).unwrap(); - assert_eq!((frame.kind, frame.request_id, frame.status), (8, 42, 0)); assert_eq!( - app::decode_activate_payload(frame.payload).unwrap(), + app::decode_activate_payload(&payload).unwrap(), (record.clone(), workload) ); } @@ -527,40 +435,6 @@ fn maps_payloads_match_their_vectors() { } } -/// Each `FEATURES` vector decodes to its bits, which encode to its bytes. -#[cfg(all(feature = "cli", feature = "testing"))] -#[test] -fn features_match_their_vectors() { - for vector in of_kind(NVXC_V1, "FEATURES", &["ALL"]) { - let mut values = vector.values(); - let features = GuestFeatures::from_bits(values.number()); - drop(values); - assert_eq!(features.encode()[..], vector.bytes, "{}", vector.name); - let decoded = GuestFeatures::decode(&vector.bytes).unwrap(); - assert_eq!(decoded, features, "{}", vector.name); - } -} - -#[cfg(all( - feature = "cli", - feature = "guest", - target_os = "linux", - target_arch = "x86_64" -))] -#[test] -fn the_host_requires_every_feature() { - assert_eq!( - GuestFeatures::CANCEL - | GuestFeatures::WORKLOAD_ACCOUNT - | GuestFeatures::EXEC_ENVIRONMENT - | GuestFeatures::EXEC_CWD - | GuestFeatures::HOST_MAPPING_TABLE, - GuestFeatures::REQUIRED - ); - assert!(!GuestFeatures::DIRECT_AGENT.contains(GuestFeatures::EXEC_CGROUP)); - assert!(!GuestFeatures::REQUIRED.contains(GuestFeatures::EXEC_CGROUP)); -} - #[cfg(all(feature = "cli", feature = "testing"))] #[test] fn exec_payload_carries_every_32_bit_timeout() { @@ -758,10 +632,6 @@ fn exec_requests_are_refused_where_the_guest_agent_refuses_them() { assert_eq!(most.argv.len(), MAX_ARGUMENTS); } -#[cfg(any( - feature = "testing", - all(feature = "guest", target_os = "linux", target_arch = "x86_64") -))] #[test] fn decoders_reject_malformed_input() { let mut header = [0u8; OUTER_HEADER_LEN]; @@ -770,104 +640,6 @@ fn decoders_reject_malformed_input() { header[4] = 1; header[40..44].copy_from_slice(&(OUTER_MAX_PAYLOAD as u32 + 1).to_le_bytes()); assert!(decode_outer_header(&header).is_err()); - - let mut frame = encode_app(APP_STDOUT, 1, 0, b"abc"); - assert_eq!(decode_app(&frame).unwrap().payload, b"abc"); - frame.push(0); - assert!(decode_app(&frame).is_err()); - assert!(decode_app(&frame[..10]).is_err()); -} - -#[cfg(all(feature = "cli", feature = "testing"))] -#[test] -fn exit_categories_match_the_guest_agent() { - for category in [ - ExitCategory::Exit, - ExitCategory::Timeout, - ExitCategory::OutputLimit, - ExitCategory::Signal, - ExitCategory::Failed, - ExitCategory::Cancelled, - ] { - assert_eq!( - ExitCategory::parse(category.as_str().as_bytes()).unwrap(), - category - ); - } - assert_eq!(ExitCategory::parse(b"exit").unwrap(), ExitCategory::Exit); - assert_eq!( - ExitCategory::parse(b"timeout").unwrap(), - ExitCategory::Timeout - ); - assert_eq!( - ExitCategory::parse(b"output-limit").unwrap(), - ExitCategory::OutputLimit - ); - assert_eq!( - ExitCategory::parse(b"signal").unwrap(), - ExitCategory::Signal - ); - assert_eq!( - ExitCategory::parse(b"failed").unwrap(), - ExitCategory::Failed - ); - assert_eq!( - ExitCategory::parse(b"cancelled").unwrap(), - ExitCategory::Cancelled - ); - assert!(ExitCategory::parse(b"canceled").is_err()); -} - -#[cfg(all( - feature = "cli", - feature = "testing", - feature = "guest", - target_os = "linux", - target_arch = "x86_64" -))] -#[test] -fn guest_features_report_what_a_guest_lacks() { - let required = GuestFeatures::REQUIRED; - let complete = GuestFeatures::decode(&required.encode()).unwrap(); - assert!(complete.missing(required).is_empty()); - assert!(complete.contains(required)); - - // Trailing bytes belong to later revisions. - let cancel_only = GuestFeatures::decode(&[1, 0, 0, 0, 0xaa]).unwrap(); - assert_eq!(cancel_only, GuestFeatures::CANCEL); - assert!(!cancel_only.contains(required)); - assert_eq!( - cancel_only.missing(required), - [ - "workload accounts", - "per-execution environments", - "working directories", - "host path mapping tables", - ] - ); - assert!(GuestFeatures::decode(&[1, 0, 0]).is_err()); - - // A guest of the previous release announced its host mappings with bit 1, which no - // longer counts. - let previous = GuestFeatures::decode(&0b11_1111u32.to_le_bytes()).unwrap(); - assert_eq!(previous.missing(required), ["host path mapping tables"]); - - // Every required feature has a name, so a refusal can say what is missing. - assert_eq!( - GuestFeatures::NONE.missing(required).len(), - required.bits().count_ones() as usize - ); -} - -#[cfg(all(feature = "cli", feature = "testing"))] -#[test] -fn guest_features_use_distinct_bits() { - let mut seen = 1u32 << 1; - for (feature, name) in GuestFeatures::NAMES { - assert_eq!(feature.bits().count_ones(), 1, "{name}"); - assert_eq!(seen & feature.bits(), 0, "{name} reuses a feature bit"); - seen |= feature.bits(); - } } fn map(source: &str, target: &str, read_only: bool) -> HostMap { @@ -1062,33 +834,20 @@ fn record_definitions_keep_their_wire_values() { assert_eq!(OUTER_CREDIT, 9); assert_eq!(OUTER_RECEIVE_WINDOW, 1024 * 1024); assert_eq!(APP_HEADER_LEN, 24); - assert_eq!(APP_VERSION, 1); assert_eq!(APP_PING, 1); assert_eq!(APP_EXEC, 2); assert_eq!(APP_STOP, 3); assert_eq!(APP_CANCEL, 4); + assert_eq!(APP_MAPS, 6); assert_eq!(APP_READY, 0x81); assert_eq!(APP_STDOUT, 0x82); assert_eq!(APP_STDERR, 0x83); - assert_eq!(APP_EXIT, 0x84); + #[cfg(all(feature = "guest", target_os = "linux", target_arch = "x86_64"))] + assert_eq!((APP_CAPTURE, APP_ACTIVATE, APP_EXIT), (7, 8, 0x84)); assert_eq!(APP_STOPPED, 0x85); assert_eq!(APP_ERROR, 0xff); } -#[cfg(all(feature = "cli", feature = "testing"))] -#[test] -fn feature_definitions_keep_their_wire_values() { - assert_eq!(APP_FEATURES, 5); - assert_eq!(APP_MAPS, 6); - assert_eq!(GuestFeatures::CANCEL.bits(), 0x01); - // Bit 1 is retired and stays unused, so hosts that still need it refuse the image. - assert_eq!(GuestFeatures::WORKLOAD_ACCOUNT.bits(), 0x04); - assert_eq!(GuestFeatures::EXEC_CGROUP.bits(), 0x08); - assert_eq!(GuestFeatures::EXEC_ENVIRONMENT.bits(), 0x10); - assert_eq!(GuestFeatures::EXEC_CWD.bits(), 0x20); - assert_eq!(GuestFeatures::HOST_MAPPING_TABLE.bits(), 0x40); -} - #[cfg(any( feature = "testing", all(feature = "guest", target_os = "linux", target_arch = "x86_64") @@ -1118,7 +877,6 @@ fn exec_extension_keeps_its_wire_values() { assert_eq!(MAX_ARGUMENTS, 64); assert_eq!(MAX_ARGUMENT_BYTES, 4096); assert_eq!(MAX_ENVIRONMENT, 256); - assert_eq!(MAX_OUTPUT_BYTES, 1024 * 1024); assert_eq!(CWD_FAILED, "cwd-failed"); assert_eq!(LAUNCH_FAILED, "launch-failed"); assert_eq!(INVALID_REQUEST, "invalid-request"); @@ -1238,49 +996,6 @@ fn host_mapping_count_splits_the_command_line_as_the_guest_init_does() { } } -#[cfg(all( - feature = "testing", - feature = "guest", - target_os = "linux", - target_arch = "x86_64" -))] -#[test] -fn both_agent_modes_announce_the_host_mapping_table() { - assert_eq!(GuestFeatures::DIRECT_AGENT.bits(), 0x75); - // The layered root creates no account, so it announces no workload accounts (bit 2). - assert_eq!(GuestFeatures::SANDBOX_AGENT.bits(), 0x51); - assert!(!GuestFeatures::SANDBOX_AGENT.contains(GuestFeatures::WORKLOAD_ACCOUNT)); - assert!(GuestFeatures::DIRECT_AGENT.contains(GuestFeatures::WORKLOAD_ACCOUNT)); - for features in [GuestFeatures::DIRECT_AGENT, GuestFeatures::SANDBOX_AGENT] { - assert!(features.contains(GuestFeatures::HOST_MAPPING_TABLE)); - } - assert_eq!( - GuestFeatures::DIRECT_AGENT.bits() & (1 << 1), - 0, - "retired bit one stays clear" - ); -} - -#[cfg(all(feature = "cli", feature = "testing"))] -#[test] -fn layered_sandboxes_need_per_execution_environments_and_mapping_tables() { - // The CLI's exec sends working directories and environments, and its start sends the - // mappings of the shares; it never cancels a workload or asks for a workload account - // (microsoft/nvx#566). - assert_eq!(GuestFeatures::LAYERED_SANDBOX.bits(), (1 << 4) | (1 << 6)); - assert!(GuestFeatures::SANDBOX_AGENT.contains(GuestFeatures::LAYERED_SANDBOX)); - assert!( - GuestFeatures::SANDBOX_AGENT - .missing(GuestFeatures::LAYERED_SANDBOX) - .is_empty() - ); - assert_eq!( - (GuestFeatures::CANCEL | GuestFeatures::WORKLOAD_ACCOUNT) - .missing(GuestFeatures::LAYERED_SANDBOX), - ["per-execution environments", "host path mapping tables"] - ); -} - #[cfg(all( feature = "cli", feature = "guest", diff --git a/nvx-rs/src/testing/fake_openvmm.rs b/nvx-rs/src/testing/fake_openvmm.rs index b488b66c8..f10a320be 100644 --- a/nvx-rs/src/testing/fake_openvmm.rs +++ b/nvx-rs/src/testing/fake_openvmm.rs @@ -32,17 +32,15 @@ //! made with `mkdir`), or that is a mapped regular file, with a diagnostic and the `cwd-failed` //! category. //! -//! Like the guest agent, the fake serves NVXC versions 1 and 2, and answers each request in its -//! version. A workload of version 1 reads no standard input, so its `cat` copies nothing, and -//! ends with `output-limit` once its output would pass 1 MiB. A workload of version 2 runs alone, -//! and another `EXEC` gets `BUSY`, as other requests do while it runs; its output has no cap; its +//! Like the guest agent, the fake serves NVXC version 2 alone: a frame of another version, such +//! as version 1, ends the session, as any malformed record does. A workload runs alone, and another +//! `EXEC` gets `BUSY`, as other requests do while it runs; its output has no cap; its //! standard input is what the host's `STDIN` frames sent, which the fake acknowledges as the //! workload reads it, until `STDIN_END`, and of which more than the window unacknowledged, or //! more after its end, ends the session; and a `STOP` while it sleeps or reads its input ends it //! with the cancelled outcome before the fake acknowledges the `STOP`, unless `fake_ignore_stop=1` //! ignores it, as between workloads; with `fake_drop_stop=1`, the fake closes the connection after -//! that `EXIT`, without acknowledging the `STOP`. Each refusal while a workload runs takes the -//! version of the request that it refuses. +//! that `EXIT`, without acknowledging the `STOP`. //! //! Host paths are mapped as OpenVMM and the guest agent map them: the command line exports //! numbered `--mount-child` directories, and files with the `file` flag, through @@ -71,10 +69,8 @@ //! `fake_boot_diagnostics=1` emits console output and a binary log before control attachment. //! `fake_refuse_attach=1` refuses attachment after the endpoint has accepted its client. //! `fake_foreground_gate=console|connect|handshake|keys` synchronizes foreground regressions. -//! `fake_guest_features=MASK` advertises the decimal feature mask `MASK` instead of the current -//! guest's, `fake_legacy_guest=1` refuses the features request like a guest agent that -//! predates it, and `fake_refuse_maps=1` fails every `MAPS` request like a guest that cannot -//! mount a mapped path. The command line is recorded in `fake-openvmm-.json` next to the +//! `fake_refuse_maps=1` fails every `MAPS` request like a guest that cannot mount a mapped path. +//! The command line is recorded in `fake-openvmm-.json` next to the //! kernel, and each connection to the control endpoint that the fake accepts adds a line to //! `fake-openvmm--sessions.log` there, so that a test can tell whether a host connected. //! @@ -88,8 +84,7 @@ //! path that its option names, the layer read-only and the scratch disk for reading and writing, //! and holds them until it exits; it reads them whole, and records what each held, with its role //! and path, in `fake-openvmm--blocks.json` next to the kernel. The -//! `fake_block_delay_ms=MS` token delays the opens. Its guest advertises the features of the -//! sandbox agent, `GuestFeatures::SANDBOX_AGENT`. With `--microvm-lifecycle managed`, it serves +//! `fake_block_delay_ms=MS` token delays the opens. With `--microvm-lifecycle managed`, it serves //! the control console as above, and maps the sandbox's shares as above too: from the children of //! the aggregate export at `/run/nvx/hostfs/root`, as many as `nvx_maps=` announces, which `MAPS` //! requests deliver; it refuses a `--mount` share and `nvx_share=` tokens. With @@ -206,13 +201,12 @@ use std::time::{Duration, Instant}; use crate::protocol::nvxs::ErrorCode; use crate::protocol::{ - APP_CANCEL, 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, BUSY, CAPABILITY_LEN, CWD_FAILED, - ExitCategory, GuestFeatures, INVALID_REQUEST, LAUNCH_FAILED, MAPPING_FAILED, - MAPPINGS_INCOMPLETE, MAPS_TOKEN, MAX_HOST_MAPPINGS, MAX_OUTPUT_BYTES, MapsPayload, OUTER_DATA, - OUTER_ERROR, OUTER_HEADER_LEN, OUTER_HOST_ATTACH, OUTER_READY, OUTER_WAIT, OuterHeader, - UNSUPPORTED_OPERATION, decode_app, decode_exec_payload, decode_maps_payload, - decode_outer_header, encode_app, encode_outer, v2, + APP_CANCEL, APP_ERROR, APP_EXEC, APP_HEADER_LEN, APP_MAPS, APP_PING, APP_READY, APP_STDERR, + APP_STDOUT, APP_STOP, APP_STOPPED, BUSY, CAPABILITY_LEN, CWD_FAILED, INVALID_REQUEST, + LAUNCH_FAILED, MAPPING_FAILED, MAPPINGS_INCOMPLETE, MAPS_TOKEN, MAX_HOST_MAPPINGS, MapsPayload, + OUTER_DATA, OUTER_ERROR, OUTER_HEADER_LEN, OUTER_HOST_ATTACH, OUTER_READY, OUTER_WAIT, + OuterHeader, UNSUPPORTED_OPERATION, decode_exec_payload, decode_maps_payload, + decode_outer_header, encode_outer, v2, }; /// Largest workload-output frame emitted by the fake guest. @@ -228,17 +222,6 @@ const IDENTITY_TOKEN: &str = "nvx_identity="; /// The UID of the workloads of a launch that names no identity: the guest's `nobody`. const DEFAULT_WORKLOAD_UID: u32 = 65534; -/// Control features of the current guest: cancellation, workload accounts, -/// per-execution environments, working directories, and host path mapping tables. -/// Bit 1 is retired. -const GUEST_FEATURES: GuestFeatures = GuestFeatures::from_bits( - GuestFeatures::CANCEL.bits() - | GuestFeatures::WORKLOAD_ACCOUNT.bits() - | GuestFeatures::EXEC_ENVIRONMENT.bits() - | GuestFeatures::EXEC_CWD.bits() - | GuestFeatures::HOST_MAPPING_TABLE.bits(), -); - /// The guest directory at which the host mounts the aggregate export. const GUEST_EXPORT: &str = "/run/nvx/hostfs/root"; @@ -433,14 +416,10 @@ struct Options { exit_delay: Duration, /// Whether cancellation requests are deliberately ignored. ignore_cancel: bool, - /// Whether the guest omits feature negotiation like an older image. - legacy_guest: bool, /// Whether host-mapping requests are deliberately refused. refuse_maps: bool, /// UID whose account and environment the fake guest models. workload_uid: u32, - /// Feature mask returned during guest feature negotiation. - guest_features: GuestFeatures, /// Host directories of the aggregate export, in child order. children: Vec, /// Number of host mappings that the kernel command line announces. @@ -1844,11 +1823,6 @@ fn parse(program: &str, arguments: &[String]) -> Result { .and_then(Path::file_name) .map(|name| name.to_string_lossy().into_owned()) .unwrap_or_default(); - let default_features = if sandbox { - GuestFeatures::SANDBOX_AGENT - } else { - GUEST_FEATURES - }; let restore_console = restore .as_ref() .and_then(|_| values.get("--virtio-console")) @@ -1909,12 +1883,8 @@ fn parse(program: &str, arguments: &[String]) -> Result { drop_stop: knob("fake_drop_stop") == Some("1"), exit_delay: millis("fake_exit_delay_ms").unwrap_or_default(), ignore_cancel: knob("fake_ignore_cancel") == Some("1"), - legacy_guest: knob("fake_legacy_guest") == Some("1"), refuse_maps: knob("fake_refuse_maps") == Some("1"), workload_uid, - guest_features: knob("fake_guest_features") - .and_then(|value| value.parse().ok()) - .map_or(default_features, GuestFeatures::from_bits), children, maps, share_targets, @@ -2570,7 +2540,7 @@ enum Flow { enum Interrupt { /// No control request is waiting. None, - /// The workload's matching cancellation request, or a `STOP` of version 2, arrived. + /// The workload's matching cancellation request, or a `STOP`, arrived. Cancel, /// The host disconnected and abandoned the workload. Disconnected, @@ -2578,9 +2548,8 @@ enum Interrupt { /// A host request that a DATA record carries. enum Request { - /// A request: whether it is of NVXC version 2, its kind, whose code version 2 shares with - /// version 1, its ID, and its payload. - App(bool, u8, u64, Vec), + /// A request: its kind, whose code version 2 shares with version 1, its ID, and its payload. + App(u8, u64, Vec), /// Version 2's standard input of the workload with this ID: a `STDIN` frame's bytes, or /// `None` for its `STDIN_END`. Stdin(u64, Option>), @@ -2670,9 +2639,6 @@ struct Session<'a, S> { ignore_cancel: bool, /// Whether stop requests are ignored for fault injection, also while a workload runs. ignore_stop: bool, - /// Whether the request that the fake serves, or the workload that runs, is of NVXC version - /// 2, whose frames its answers take. - v2: bool, /// What the host sent of the running workload's standard input in version 2 and the workload /// has not read, which the fake acknowledges as the workload reads it. stdin: Vec, @@ -2683,11 +2649,8 @@ struct Session<'a, S> { } impl Session<'_, S> { - /// Encodes and sends one sequenced guest application frame, in the session's version. + /// Encodes and sends one sequenced guest application frame, of version 2. fn send(&mut self, kind: u8, request_id: u64, status: i32, payload: &[u8]) -> io::Result<()> { - if !self.v2 { - return self.send_frame(&encode_app(kind, request_id, status, payload)); - } let frame = match kind { APP_READY => v2::Frame::Ready { version: v2::VERSION, @@ -2727,7 +2690,8 @@ impl Session<'_, S> { self.stream.write_all(&record) } - /// Reads the next host request, of either version, returning `None` at end of stream. + /// Reads the next host request, of version 2, returning `None` at end of stream. A frame of + /// another version is invalid. fn request(&mut self) -> io::Result> { let Some((header, frame)) = read_record(self.stream)? else { return Ok(None); @@ -2736,21 +2700,15 @@ impl Session<'_, S> { && header.instance_id == self.instance && header.epoch == self.epoch && header.sequence == self.host_sequence; - let request = if frame.get(4) == Some(&v2::VERSION) { - v2::Frame::decode(&frame) - .ok() - .map(|(request_id, decoded)| match decoded { - v2::Frame::Stdin(bytes) => Request::Stdin(request_id, Some(bytes.to_vec())), - v2::Frame::StdinEnd => Request::Stdin(request_id, None), - // Version 2 keeps version 1's header and the codes of the kinds that both - // have. - _ => Request::App(true, frame[5], request_id, frame[APP_HEADER_LEN..].to_vec()), - }) - } else { - decode_app(&frame).ok().map(|frame| { - Request::App(false, frame.kind, frame.request_id, frame.payload.to_vec()) - }) - }; + let request = v2::Frame::decode(&frame) + .ok() + .map(|(request_id, decoded)| match decoded { + v2::Frame::Stdin(bytes) => Request::Stdin(request_id, Some(bytes.to_vec())), + v2::Frame::StdinEnd => Request::Stdin(request_id, None), + // Version 2 keeps version 1's header and the codes of the kinds that both + // have. + _ => Request::App(frame[5], request_id, frame[APP_HEADER_LEN..].to_vec()), + }); let request = request .filter(|_| valid) .ok_or_else(|| io::Error::other("invalid data record"))?; @@ -2765,15 +2723,11 @@ impl Session<'_, S> { return Ok(Flow::Continue); }; // The standard input of a workload that ended takes no more bytes. - let Request::App(v2, kind, request_id, payload) = request else { + let Request::App(kind, request_id, payload) = request else { continue; }; - self.v2 = v2; match kind { APP_PING => self.send(APP_READY, request_id, 0, &[])?, - APP_FEATURES if !options.legacy_guest => { - self.send(APP_READY, request_id, 0, &options.guest_features.encode())?; - } APP_EXEC if guest.mapped.len() < options.maps => { self.send(APP_ERROR, request_id, 11, MAPPINGS_INCOMPLETE.as_bytes())?; } @@ -2826,23 +2780,18 @@ impl Session<'_, S> { } self.stdin.extend(bytes); } - Some(Request::App(true, APP_STOP, _, _)) if self.v2 && self.ignore_stop => {} - Some(Request::App(true, APP_STOP, id, _)) if self.v2 => { + Some(Request::App(APP_STOP, _, _)) if self.ignore_stop => {} + Some(Request::App(APP_STOP, id, _)) => { self.stop = Some(id); return Ok(Interrupt::Cancel); } - Some(Request::App(_, APP_CANCEL, id, _)) + Some(Request::App(APP_CANCEL, id, _)) if id == request_id && !self.ignore_cancel => { return Ok(Interrupt::Cancel); } - Some(Request::App(_, APP_CANCEL, _, _)) => {} - // The refusal takes the request's version, and the workload's frames their own. - Some(Request::App(v2, _, id, _)) => { - let running = std::mem::replace(&mut self.v2, v2); - self.send(APP_ERROR, id, 16, BUSY.as_bytes())?; - self.v2 = running; - } + Some(Request::App(APP_CANCEL, _, _)) => {} + Some(Request::App(_, id, _)) => self.send(APP_ERROR, id, 16, BUSY.as_bytes())?, } } Ok(Interrupt::None) @@ -2864,9 +2813,8 @@ impl Session<'_, S> { let Ok(exec) = decode_exec_payload(payload) else { return self.send_some(APP_ERROR, request_id, 22, INVALID_REQUEST.as_bytes()); }; - // A workload of version 1 has no standard input, as one that reads the null device. self.stdin.clear(); - self.stdin_ended = !self.v2; + self.stdin_ended = false; let mut environment = if exec.inherit_default_env { guest.default_environment.clone() } else { @@ -2907,13 +2855,12 @@ impl Session<'_, S> { [program, ..] => { let message = format!("{}: {program}: not found\n", self.program); self.send(APP_STDERR, request_id, 0, message.as_bytes())?; - return self.exit(request_id, 127, ExitCategory::Exit); + return self.exit(request_id, 127, v2::Outcome::Exit); } [] => return self.send_some(APP_ERROR, request_id, 22, INVALID_REQUEST.as_bytes()), }; let started = Instant::now(); let limit = (timeout_ms > 0).then(|| started + Duration::from_millis(timeout_ms.into())); - let mut output = 0usize; let mut status = 0; for command in script .split(';') @@ -2932,15 +2879,9 @@ impl Session<'_, S> { } else { APP_STDERR }; - if !self.forward(kind, request_id, &mut output, line.as_bytes())? { - return self.exit(request_id, 125, ExitCategory::OutputLimit); - } - } - "printerr" => { - if !self.forward(APP_STDERR, request_id, &mut output, argument.as_bytes())? { - return self.exit(request_id, 125, ExitCategory::OutputLimit); - } + self.send(kind, request_id, 0, line.as_bytes())?; } + "printerr" => self.send(APP_STDERR, request_id, 0, argument.as_bytes())?, "touch" => { // A workload that writes a read-write share creates host files. if let Err(error) = OpenOptions::new().create(true).append(true).open(argument) @@ -2961,37 +2902,35 @@ impl Session<'_, S> { .min(Duration::from_millis(5)), ), Interrupt::Cancel => { - return self.exit(request_id, 137, ExitCategory::Cancelled); + return self.exit(request_id, 137, v2::Outcome::Cancelled); } Interrupt::Disconnected => return Ok(None), } } if wake > end { - return self.exit(request_id, 124, ExitCategory::Timeout); + return self.exit(request_id, 124, v2::Outcome::Timeout); } } "exit" => { let status = argument.parse().unwrap_or(1); - return self.exit(request_id, status, ExitCategory::Exit); + return self.exit(request_id, status, v2::Outcome::Exit); } "signal" => { let signal: i32 = argument.parse().unwrap_or(9); - return self.exit(request_id, 128 + signal, ExitCategory::Signal); + return self.exit(request_id, 128 + signal, v2::Outcome::Signal); } "flood" => { let mut remaining: usize = argument.parse().unwrap_or(0); while remaining > 0 { let chunk = remaining.min(OUTPUT_CHUNK_BYTES); - if !self.forward(APP_STDOUT, request_id, &mut output, &vec![b'x'; chunk])? { - return self.exit(request_id, 125, ExitCategory::OutputLimit); - } + self.send(APP_STDOUT, request_id, 0, &vec![b'x'; chunk])?; remaining -= chunk; } } "cat" => loop { let chunk = self.stdin.len().min(OUTPUT_CHUNK_BYTES); if limit.is_some_and(|limit| Instant::now() >= limit) { - return self.exit(request_id, 124, ExitCategory::Timeout); + return self.exit(request_id, 124, v2::Outcome::Timeout); } else if chunk > 0 { // The workload reads these bytes now, so the fake acknowledges them. let bytes: Vec = self.stdin.drain(..chunk).collect(); @@ -3003,7 +2942,7 @@ impl Session<'_, S> { match self.interrupt(request_id)? { Interrupt::None => thread::sleep(Duration::from_millis(5)), Interrupt::Cancel => { - return self.exit(request_id, 137, ExitCategory::Cancelled); + return self.exit(request_id, 137, v2::Outcome::Cancelled); } Interrupt::Disconnected => return Ok(None), } @@ -3024,9 +2963,7 @@ impl Session<'_, S> { "env" => { for (name, value) in &environment { let line = format!("{name}={value}\n"); - if !self.forward(APP_STDOUT, request_id, &mut output, line.as_bytes())? { - return self.exit(request_id, 125, ExitCategory::OutputLimit); - } + self.send(APP_STDOUT, request_id, 0, line.as_bytes())?; } } "printenv" => { @@ -3035,16 +2972,7 @@ impl Session<'_, S> { .find(|(variable, _)| variable == argument) .map(|(_, value)| format!("{value}\n")); match value { - Some(line) => { - if !self.forward( - APP_STDOUT, - request_id, - &mut output, - line.as_bytes(), - )? { - return self.exit(request_id, 125, ExitCategory::OutputLimit); - } - } + Some(line) => self.send(APP_STDOUT, request_id, 0, line.as_bytes())?, None => status = 1, } } @@ -3056,18 +2984,18 @@ impl Session<'_, S> { let line = format!("{timeout_ms}\n"); self.send(APP_STDOUT, request_id, 0, line.as_bytes())?; } - "fail" => return self.exit(request_id, 125, ExitCategory::Failed), + "fail" => return self.exit(request_id, 125, v2::Outcome::Failed), "launchfail" => { return self.send_some(APP_ERROR, request_id, 125, LAUNCH_FAILED.as_bytes()); } _ => { let message = format!("{}: unknown command {name}\n", self.program); self.send(APP_STDERR, request_id, 0, message.as_bytes())?; - return self.exit(request_id, 127, ExitCategory::Exit); + return self.exit(request_id, 127, v2::Outcome::Exit); } } } - self.exit(request_id, status, ExitCategory::Exit) + self.exit(request_id, status, v2::Outcome::Exit) } /// Sends one frame and adapts success to the optional value expected by script execution. @@ -3081,44 +3009,15 @@ impl Session<'_, S> { self.send(kind, request_id, status, payload).map(Some) } - /// Ends workload `request_id` with `status` and `category`. + /// Ends workload `request_id` with `status` and `outcome`. fn exit( &mut self, request_id: u64, status: i32, - category: ExitCategory, + outcome: v2::Outcome, ) -> io::Result> { - if self.v2 { - let outcome = match category { - ExitCategory::Exit => v2::Outcome::Exit, - ExitCategory::Signal => v2::Outcome::Signal, - ExitCategory::Timeout => v2::Outcome::Timeout, - ExitCategory::Cancelled => v2::Outcome::Cancelled, - ExitCategory::Failed | ExitCategory::OutputLimit => v2::Outcome::Failed, - }; - return self - .send_v2(request_id, v2::Frame::Exit { status, outcome }) - .map(Some); - } - self.send_some(APP_EXIT, request_id, status, category.as_str().as_bytes()) - } - - /// Forwards workload output unless it would exceed the guest agent's output limit of version - /// 1, and returns whether it did. A workload whose output was refused ends with - /// `output-limit`. - fn forward( - &mut self, - kind: u8, - request_id: u64, - forwarded: &mut usize, - output: &[u8], - ) -> io::Result { - if !self.v2 && *forwarded + output.len() > MAX_OUTPUT_BYTES { - return Ok(false); - } - *forwarded += output.len(); - self.send(kind, request_id, 0, output)?; - Ok(true) + self.send_v2(request_id, v2::Frame::Exit { status, outcome }) + .map(Some) } } @@ -3175,7 +3074,6 @@ fn serve_client( host_sequence: 0, ignore_cancel: options.ignore_cancel, ignore_stop: options.ignore_stop, - v2: false, stdin: Vec::new(), stdin_ended: true, stop: None, diff --git a/nvx-rs/src/testing/fake_openvmm/tests.rs b/nvx-rs/src/testing/fake_openvmm/tests.rs index d03852068..43c920ef1 100644 --- a/nvx-rs/src/testing/fake_openvmm/tests.rs +++ b/nvx-rs/src/testing/fake_openvmm/tests.rs @@ -1,4 +1,6 @@ use super::*; +#[cfg(all(unix, feature = "cli"))] +use crate::tests::{NVXC_V1, vector}; #[test] fn shells_raise_shlvl_like_busybox() { @@ -20,18 +22,6 @@ fn shells_raise_shlvl_like_busybox() { assert_eq!(fresh[0], ("SHLVL".to_owned(), "1".to_owned())); } -#[cfg(all( - feature = "cli", - feature = "guest", - target_os = "linux", - target_arch = "x86_64" -))] -#[test] -fn the_current_guest_advertises_every_required_feature() { - assert_eq!(GUEST_FEATURES, GuestFeatures::REQUIRED); - assert!(GUEST_FEATURES.missing(GuestFeatures::REQUIRED).is_empty()); -} - #[test] fn mount_children_resolve_to_hosts_of_their_kind() { let temporary = tempfile::tempdir().unwrap(); @@ -1276,7 +1266,7 @@ fn version_2_workloads_read_their_input_until_its_end_under_the_window() { #[test] fn version_2_workloads_send_output_past_the_cap_of_version_1() { let mut host = Host::attach(); - let total = MAX_OUTPUT_BYTES * 3 / 2; + let total = 3 * 1024 * 1024 / 2; host.send(1, v2::Frame::Exec(&script(&format!("flood {total}")))); let mut forwarded = 0; loop { @@ -1287,25 +1277,21 @@ fn version_2_workloads_send_output_past_the_cap_of_version_1() { } } assert_eq!(forwarded, total); - // Version 1, which the same session still serves, caps it. - host.send_frame(&encode_app( - APP_EXEC, - 2, - 0, - &script(&format!("flood {total}")), - )); - let mut capped = 0; - let exit = loop { - let payload = host.read(); - let frame = decode_app(&payload).unwrap(); - match frame.kind { - APP_STDOUT => capped += frame.payload.len(), - _ => break (frame.kind, frame.status, frame.payload.to_vec()), - } - }; - assert!(capped <= MAX_OUTPUT_BYTES); - assert_eq!(exit, (APP_EXIT, 125, b"output-limit".to_vec())); - host.stop(3); + host.stop(2); +} + +/// A request of version 1, or version 1's `FEATURES` in a frame of version 2, ends the session +/// without an answer, as any malformed record does. +#[cfg(all(unix, feature = "cli"))] +#[test] +fn version_1_requests_end_the_session() { + let features = [&b"NVXC\x02\x05\0\0"[..], &1u64.to_le_bytes(), &[0; 8]].concat(); + for frame in [vector(NVXC_V1, "FRAME", "PING").bytes, features] { + let mut host = Host::attach(); + host.send_frame(&frame); + assert!(read_record(&mut host.stream).unwrap().is_none()); + assert!(!host.fake.join().unwrap()); + } } #[cfg(all(unix, feature = "cli"))] diff --git a/nvx-rs/tests/cli.rs b/nvx-rs/tests/cli.rs index 5066fd14a..e7eff0523 100644 --- a/nvx-rs/tests/cli.rs +++ b/nvx-rs/tests/cli.rs @@ -120,35 +120,6 @@ fn text(data: &[u8]) -> String { String::from_utf8_lossy(data).replace("\r\n", "\n") } -#[test] -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(); - 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!(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) - ); - } - } -} - #[test] fn a_mapping_that_the_guest_refuses_leaves_no_usable_start() { // The guest agent mounts each share as the mapping table that start sends once it is ready diff --git a/nvx-rs/tests/vectors/README.md b/nvx-rs/tests/vectors/README.md index a3271f930..a0a3f6fd0 100644 --- a/nvx-rs/tests/vectors/README.md +++ b/nvx-rs/tests/vectors/README.md @@ -9,7 +9,7 @@ yet breaks those peers. Only bytes that no codec produced catch such a change. | File | Vectors | | --- | --- | -| [`nvxc-v1.txt`](nvxc-v1.txt) | Version 1 of the control protocol: `NVXC` frames, their `EXEC`, `MAPS`, and features payloads, and `NVXS` records of NVX's host and guest | +| [`nvxc-v1.txt`](nvxc-v1.txt) | Version 1 of the control protocol, which no peer speaks any longer: `NVXC` frames, which version 2 refuses, their `EXEC` and `MAPS` payloads, which version 2 carries unchanged, their features payloads, and `NVXS` records of NVX's host and guest | | [`nvxl-v1.txt`](nvxl-v1.txt) | Version 1 of the `NVXL` listener announcement | | [`activation-v2.txt`](activation-v2.txt) | Version 2 of the activation record of a layered root, and what Ready reports about an activation | | [`snapshot-key-v1.txt`](snapshot-key-v1.txt) | Version 1 of the canonical encoding of a snapshot's compatibility key, with its identity | @@ -48,6 +48,9 @@ KIND|NAME|BYTES|VALUE|... A decoder must decode the bytes of each vector to its values, and an encoder must encode the values to exactly its bytes. A kind whose name starts with `INVALID_` has no values: every decoder must refuse the bytes of its vectors. +The frames and the features payload of `nvxc-v1.txt` have no codec any longer: +version 2's decoder refuses those frames, version 2 carries their `EXEC` and +`MAPS` payloads unchanged, and its tests pin the bytes of its own frames. A key holds too many values for one line, so the value of each vector of `snapshot-key-v1.txt` is its identity, and the file's header names the diff --git a/nvx-rs/tests/vm_lifecycle.rs b/nvx-rs/tests/vm_lifecycle.rs index 2865a123c..3578f940a 100644 --- a/nvx-rs/tests/vm_lifecycle.rs +++ b/nvx-rs/tests/vm_lifecycle.rs @@ -100,22 +100,24 @@ impl Drop for Started { } } -/// The NVXC frame of `kind` for the request `request`, with status 0 and no payload: the magic, -/// the version, 1, the kind, two zero flag bytes, the request's ID, the status, and the payload's -/// length, each integer little-endian, as `tests/vectors/nvxc-v1.txt` pins them. -fn frame(kind: u8, request: u64) -> Vec { +/// The NVXC frame of `kind` for the request `request`, with status 0 and `payload`: the magic, the +/// version, 2, the kind, two zero flag bytes, the request's ID, the status, and the payload's +/// length, each integer little-endian, as version 2 of NVXC defines them. +fn frame(kind: u8, request: u64, payload: &[u8]) -> Vec { + let length = u32::try_from(payload.len()).expect("a payload fits a frame"); [ &b"NVXC"[..], - &[1, kind, 0, 0], + &[2, kind, 0, 0], &request.to_le_bytes(), &0i32.to_le_bytes(), - &0u32.to_le_bytes(), + &length.to_le_bytes(), + payload, ] .concat() } /// Sends the request `kind` of the caller's own protocol with its guest, and checks that the guest -/// acknowledges it with `reply` by `deadline`. +/// acknowledges it with `reply` by `deadline`; a `READY` names the protocol version, 2. /// /// `nvx` carries the bytes of whatever protocol a caller speaks with its guest over the /// `HostConnection` that `vm::reattach` returns, and leaves the protocol to the caller. This @@ -130,14 +132,15 @@ fn acknowledged( deadline: Instant, ) { connection - .send(&frame(kind, request), Some(deadline)) + .send(&frame(kind, request, &[]), Some(deadline)) .expect("the guest takes the request"); let response = connection .receive(Some(deadline)) .expect("the guest answers the request"); + let version: &[u8] = if reply == READY { &[2] } else { &[] }; assert_eq!( response, - frame(reply, request), + frame(reply, request, version), "the guest acknowledges request {kind}" ); } diff --git a/scripts/test-layout-allowlist.json b/scripts/test-layout-allowlist.json index bf3399869..b62692a24 100644 --- a/scripts/test-layout-allowlist.json +++ b/scripts/test-layout-allowlist.json @@ -12,7 +12,6 @@ "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/vm.rs": 2, - "nvx-rs/src/protocol/app.rs": 1 + "nvx-rs/src/host/vm.rs": 2 } } From 675b0d9b3fa34814da799e04fa0eaafd189c54c8 Mon Sep 17 00:00:00 2001 From: Pedro Henrique Penna Date: Thu, 12 Nov 2026 07:08:33 -0800 Subject: [PATCH 2/2] error: delete the guest-features-missing code ErrorCode::GuestFeaturesMissing reported a guest image that lacked the control features of NVXC v1's FEATURES request, which SL4.4c stopped sending and the previous commit deletes. Nothing reports the code any longer, so SL4.4d (#1129) deletes it, a public API removal inside leaving code, with its row in the README's and doc/usage.md's tables of codes, and the mention of control features in ErrorKind::Unavailable. The public API snapshots lose the variant. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- doc/usage.md | 1 - nvx-rs/README.md | 3 +-- nvx-rs/public-api/linux.txt | 1 - nvx-rs/public-api/windows.txt | 1 - nvx-rs/src/error.rs | 9 ++------- 5 files changed, 3 insertions(+), 12 deletions(-) diff --git a/doc/usage.md b/doc/usage.md index a5e0a6ca7..c0e223c97 100644 --- a/doc/usage.md +++ b/doc/usage.md @@ -94,7 +94,6 @@ each code for callers of the `nvx` library. | `unsupported` | 1 | NVX refuses the request: options that it cannot combine, a value that it does not accept, or a policy that it cannot represent or enforce. The message names the option or field. | | `unavailable` | 1 | A dependency is missing or unusable, other than as a finer code says: install metadata, the hypervisor, a release, a distro layer or scratch image that the request names, or a host that NVX does not support. | | `artifact-missing` | 1 | An artifact that the command runs, such as OpenVMM, the guest kernel, or an initramfs, does not exist, or no artifact root holds them. | -| `guest-features-missing` | 1 | The guest image lacks control features that the sandbox needs, which the message names. | | `sandbox-not-provisioned` | 1 | No sandbox is provisioned at `--state-dir`, which does not exist, is not a plain directory, or holds no configuration. | | `sandbox-provisioned` | 1 | `provision` found the sandbox provisioned already, or it was provisioned again while `deprovision` removed it. | | `sandbox-not-running` | 1 | `exec` or `stop` found the VM of a provisioned sandbox not running. | diff --git a/nvx-rs/README.md b/nvx-rs/README.md index 09fb22952..2eba81a5e 100644 --- a/nvx-rs/README.md +++ b/nvx-rs/README.md @@ -2350,7 +2350,7 @@ optional cause. | `ErrorKind` | Meaning | | --- | --- | -| `Unavailable` | A missing or unusable artifact, hypervisor, or release, a guest image without a required control feature, or an unsupported host | +| `Unavailable` | A missing or unusable artifact, hypervisor, or release, or an unsupported host | | `Unsupported` | A request that NVX refuses: a value that it does not accept, such as a malformed or out-of-range one, options that it cannot combine, or a request that it cannot represent or enforce, such as a policy beyond a limit of OpenVMM or the guest agent | | `Exited` | The VM's OpenVMM process exited | | `StillRunning` | A stop could not confirm the VM process's exit, so the caller keeps its verified process | @@ -2393,7 +2393,6 @@ usage errors, which no library error has. | `argument-missing` | `Unsupported` | A usage error: a required argument, or the value of an option, is missing | | `argument-invalid` | `Unsupported` | A usage error: an argument has an invalid value, such as one that is not among its choices, not a number, not of its form, or not valid Unicode | | `artifact-missing` | `Unavailable` | An artifact that the operation runs, such as OpenVMM, the guest kernel, or an initramfs, does not exist, or no artifact root holds them | -| `guest-features-missing` | `Unavailable` | The guest image lacks control features that the operation needs, which the message names | | `sandbox-not-provisioned` | `NotFound` | No sandbox is provisioned at its state directory, which does not exist, is not a plain directory, or holds no configuration | | `sandbox-provisioned` | `Other` | The sandbox is provisioned already, or was provisioned again while a removal deleted it | | `sandbox-not-running` | `Other` | The operation needs the sandbox's VM, which is not running | diff --git a/nvx-rs/public-api/linux.txt b/nvx-rs/public-api/linux.txt index cce9ed98a..2b17bbd4e 100644 --- a/nvx-rs/public-api/linux.txt +++ b/nvx-rs/public-api/linux.txt @@ -25,7 +25,6 @@ variant nvx::ErrorCode::ExecRefused variant nvx::ErrorCode::Exited variant nvx::ErrorCode::ForeignFiles variant nvx::ErrorCode::FormatUnsupported -variant nvx::ErrorCode::GuestFeaturesMissing variant nvx::ErrorCode::Io variant nvx::ErrorCode::NotFound variant nvx::ErrorCode::Other diff --git a/nvx-rs/public-api/windows.txt b/nvx-rs/public-api/windows.txt index 25ae22fa6..6548939a6 100644 --- a/nvx-rs/public-api/windows.txt +++ b/nvx-rs/public-api/windows.txt @@ -25,7 +25,6 @@ variant nvx::ErrorCode::ExecRefused variant nvx::ErrorCode::Exited variant nvx::ErrorCode::ForeignFiles variant nvx::ErrorCode::FormatUnsupported -variant nvx::ErrorCode::GuestFeaturesMissing variant nvx::ErrorCode::Io variant nvx::ErrorCode::NotFound variant nvx::ErrorCode::Other diff --git a/nvx-rs/src/error.rs b/nvx-rs/src/error.rs index 5637f62f6..a0d5e7500 100644 --- a/nvx-rs/src/error.rs +++ b/nvx-rs/src/error.rs @@ -10,8 +10,8 @@ pub type Result = std::result::Result; #[non_exhaustive] pub enum ErrorKind { /// A runtime dependency is missing or unusable: an artifact, the hypervisor, a release or - /// guest image without the control contract or features that this crate needs, or a host - /// that NVX does not support. + /// guest image without the control contract that this crate needs, or a host that NVX does + /// not support. Unavailable, /// NVX refuses the request: a value that it does not accept, such as a malformed or /// out-of-range one, options that it cannot combine, or a request that it cannot represent or @@ -117,8 +117,6 @@ pub enum ErrorCode { /// `artifact-missing`: an artifact that the operation runs, such as OpenVMM, the guest kernel, /// or an initramfs, does not exist, or no artifact root holds them. ArtifactMissing, - /// `guest-features-missing`: the guest image lacks control features that the operation needs. - GuestFeaturesMissing, /// `sandbox-not-provisioned`: no sandbox is provisioned at its state directory, which does not /// exist, is not a plain directory, or holds no configuration. SandboxNotProvisioned, @@ -185,7 +183,6 @@ impl ErrorCode { Self::ArgumentMissing, Self::ArgumentInvalid, Self::ArtifactMissing, - Self::GuestFeaturesMissing, Self::SandboxNotProvisioned, Self::SandboxProvisioned, Self::SandboxNotRunning, @@ -221,7 +218,6 @@ impl ErrorCode { Self::ArgumentMissing => "argument-missing", Self::ArgumentInvalid => "argument-invalid", Self::ArtifactMissing => "artifact-missing", - Self::GuestFeaturesMissing => "guest-features-missing", Self::SandboxNotProvisioned => "sandbox-not-provisioned", Self::SandboxProvisioned => "sandbox-provisioned", Self::SandboxNotRunning => "sandbox-not-running", @@ -247,7 +243,6 @@ impl ErrorCode { match self { Self::Unavailable | Self::ArtifactMissing - | Self::GuestFeaturesMissing | Self::TemplateIneligible | Self::TemplateTombstoned => ErrorKind::Unavailable, Self::Unsupported