Skip to content

Commit 2713e82

Browse files
harden sandbox isolation default user and vsock peer policy
1 parent 8e835d4 commit 2713e82

File tree

3 files changed

+142
-35
lines changed

3 files changed

+142
-35
lines changed

crates/vz-guest-agent/src/listener.rs

Lines changed: 77 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ use std::sync::Arc;
1414
use std::task::{Context, Poll};
1515

1616
use tokio::io::{AsyncRead, AsyncWrite};
17+
use tracing::warn;
1718
use tonic::transport::server::Connected;
1819

1920
/// AF_VSOCK listener that accepts connections from the host.
@@ -85,6 +86,8 @@ struct SockaddrVm {
8586

8687
/// VMADDR_CID_ANY: accept connections from any CID (i.e., the host).
8788
const VMADDR_CID_ANY: u32 = u32::MAX; // -1 as u32
89+
/// VMADDR_CID_HOST: standard host CID for AF_VSOCK.
90+
const VMADDR_CID_HOST: u32 = 2;
8891

8992
/// AF_VSOCK address family on macOS.
9093
#[cfg(target_os = "macos")]
@@ -165,40 +168,82 @@ impl VsockListener {
165168

166169
let listener_fd = self.fd.as_raw_fd();
167170

168-
// Use tokio's blocking task pool for the accept() syscall since
169-
// we can't easily register a vsock fd with epoll/kqueue through tokio.
170-
let conn_fd = tokio::task::spawn_blocking(move || -> io::Result<RawFd> {
171-
let mut addr: SockaddrVm = unsafe { std::mem::zeroed() };
172-
let mut addr_len = std::mem::size_of::<SockaddrVm>() as libc::socklen_t;
173-
174-
// SAFETY: accept() with a valid listening fd.
175-
let fd = unsafe {
176-
libc::accept(
177-
listener_fd,
178-
&mut addr as *mut SockaddrVm as *mut libc::sockaddr,
179-
&mut addr_len,
180-
)
181-
};
182-
if fd < 0 {
183-
return Err(io::Error::last_os_error());
171+
loop {
172+
// Use tokio's blocking task pool for the accept() syscall since
173+
// we can't easily register a vsock fd with epoll/kqueue through tokio.
174+
let (conn_fd, source_cid) = tokio::task::spawn_blocking(
175+
move || -> io::Result<(RawFd, u32)> {
176+
let mut addr: SockaddrVm = unsafe { std::mem::zeroed() };
177+
let mut addr_len = std::mem::size_of::<SockaddrVm>() as libc::socklen_t;
178+
179+
// SAFETY: accept() with a valid listening fd.
180+
let fd = unsafe {
181+
libc::accept(
182+
listener_fd,
183+
&mut addr as *mut SockaddrVm as *mut libc::sockaddr,
184+
&mut addr_len,
185+
)
186+
};
187+
if fd < 0 {
188+
return Err(io::Error::last_os_error());
189+
}
190+
191+
Ok((fd, source_cid_from_addr(&addr)))
192+
},
193+
)
194+
.await
195+
.map_err(io::Error::other)??;
196+
197+
if !is_host_peer(source_cid) {
198+
// SAFETY: close accepted fd on explicit rejection.
199+
let close_result = unsafe { libc::close(conn_fd) };
200+
if close_result != 0 {
201+
warn!(
202+
source_cid = source_cid,
203+
error = %io::Error::last_os_error(),
204+
"failed to close rejected vsock connection"
205+
);
206+
} else {
207+
warn!(
208+
source_cid = source_cid,
209+
"rejected vsock connection from non-host CID"
210+
);
211+
}
212+
continue;
184213
}
185214

186-
Ok(fd)
187-
})
188-
.await
189-
.map_err(io::Error::other)??;
190-
191-
// Convert the raw fd to a tokio UnixStream for async I/O.
192-
// vsock fds are regular file descriptors that support read/write,
193-
// and UnixStream is the simplest tokio wrapper for arbitrary fds.
194-
// SAFETY: conn_fd is a valid, newly accepted file descriptor.
195-
let std_stream = unsafe { std::os::unix::net::UnixStream::from_raw_fd(conn_fd) };
196-
std_stream.set_nonblocking(true)?;
197-
let tokio_stream = tokio::net::UnixStream::from_std(std_stream)?;
198-
199-
Ok(VsockStream {
200-
inner: tokio_stream,
201-
})
215+
// Convert the raw fd to a tokio UnixStream for async I/O.
216+
// vsock fds are regular file descriptors that support read/write,
217+
// and UnixStream is the simplest tokio wrapper for arbitrary fds.
218+
// SAFETY: conn_fd is a valid, newly accepted file descriptor.
219+
let std_stream = unsafe { std::os::unix::net::UnixStream::from_raw_fd(conn_fd) };
220+
std_stream.set_nonblocking(true)?;
221+
let tokio_stream = tokio::net::UnixStream::from_std(std_stream)?;
222+
223+
return Ok(VsockStream {
224+
inner: tokio_stream,
225+
});
226+
}
227+
}
228+
}
229+
230+
fn source_cid_from_addr(addr: &SockaddrVm) -> u32 {
231+
addr.svm_cid
232+
}
233+
234+
fn is_host_peer(cid: u32) -> bool {
235+
cid == VMADDR_CID_HOST
236+
}
237+
238+
#[cfg(test)]
239+
mod tests {
240+
use super::*;
241+
242+
#[test]
243+
fn source_cid_accepted_only_for_host() {
244+
assert!(is_host_peer(VMADDR_CID_HOST));
245+
assert!(!is_host_peer(3));
246+
assert!(!is_host_peer(1024));
202247
}
203248
}
204249

crates/vz-sandbox/src/session.rs

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ use vz_linux::grpc_client::{ExecOptions, GrpcAgentClient, GrpcExecStream};
2323

2424
use crate::error::SandboxError;
2525

26+
const DEFAULT_EXEC_USER: &str = "dev";
27+
2628
// ---------------------------------------------------------------------------
2729
// SandboxSession
2830
// ---------------------------------------------------------------------------
@@ -164,6 +166,8 @@ impl SandboxSession {
164166
/// Execute a command inside the sandbox and collect all output.
165167
///
166168
/// The command string is parsed using shell-words for proper quoting.
169+
/// Commands run as the sandbox's default user (`"dev"`) unless an explicit
170+
/// user is requested.
167171
/// The working directory is set to the project's guest-side mount path.
168172
///
169173
/// Uses the session's default timeout if `timeout` is `None`.
@@ -212,7 +216,7 @@ impl SandboxSession {
212216
let options = ExecOptions {
213217
working_dir: Some(self.guest_project_path.clone()),
214218
env: Vec::new(),
215-
user: None,
219+
user: Some(DEFAULT_EXEC_USER.to_string()),
216220
};
217221
let stream = client
218222
.exec_stream(command, args, options)
@@ -241,7 +245,7 @@ impl SandboxSession {
241245

242246
debug!(
243247
cmd = cmd,
244-
user = user,
248+
user = resolve_exec_user(user),
245249
timeout = ?timeout,
246250
slot = self.slot_index,
247251
"exec"
@@ -252,7 +256,7 @@ impl SandboxSession {
252256
let options = ExecOptions {
253257
working_dir: Some(self.guest_project_path.clone()),
254258
env: Vec::new(),
255-
user: user.map(String::from),
259+
user: Some(resolve_exec_user(user).to_string()),
256260
};
257261

258262
let exec_future = client.exec(command, args, options);
@@ -302,6 +306,10 @@ impl SandboxSession {
302306
}
303307
}
304308

309+
fn resolve_exec_user(user: Option<&str>) -> &str {
310+
user.unwrap_or(DEFAULT_EXEC_USER)
311+
}
312+
305313
// ---------------------------------------------------------------------------
306314
// Tests
307315
// ---------------------------------------------------------------------------
@@ -422,6 +430,16 @@ mod tests {
422430
assert_eq!(output.exit_code, 0);
423431
}
424432

433+
#[test]
434+
fn resolve_exec_user_defaults_to_dev() {
435+
assert_eq!(resolve_exec_user(None), "dev");
436+
}
437+
438+
#[test]
439+
fn resolve_exec_user_preserves_explicit_override() {
440+
assert_eq!(resolve_exec_user(Some("root")), "root");
441+
}
442+
425443
#[test]
426444
fn parse_command_complex() {
427445
let session = make_session();
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
# Sandbox Isolation Threat Model and Hardening Closure
2+
3+
Status: In Review
4+
Owner: James Lal
5+
Last Updated: 2026-02-24
6+
7+
## Scope
8+
9+
This document covers host-guest sandbox command execution security, specifically:
10+
11+
- `vz-sandbox` host session execution (`crates/vz-sandbox/src/session.rs`)
12+
- guest vsock accept path (`crates/vz-guest-agent/src/listener.rs`)
13+
- protocol execution defaults/fields (`vz` gRPC `ExecRequest`)
14+
15+
## Attack Surface Inventory
16+
17+
| Surface | Vector | Notes |
18+
|---|---|---|
19+
| Host → guest vsock endpoint | AF_VSOCK listening socket (`VDADDR_CID_ANY` bind, service port) | Any peer that reaches vsock endpoint can open a connection and attempt protocol traffic unless source-validated. |
20+
| Exec command dispatch | gRPC `Exec` request handling | Request carries command/args/working directory/user; user controls runtime privilege in guest. |
21+
| Host working directory mapping | `acquire(project_dir)` path derivation to `guest_project_path` | Inputs derived from host path passed to pool API; malformed paths can become relative path expressions. |
22+
| gRPC user default behavior | `user` field omitted (`None`) in execution requests | `None` currently means guest default user; implementation behavior depends on host caller. |
23+
| Guest process lifecycle | Process spawn + stdin/signal/teardown paths | Long-lived process handles can be abused if request identity/privilege isn't constrained. |
24+
25+
## Findings and Closure
26+
27+
| ID | Finding | Severity | Decision | Evidence |
28+
|---|---|---|---|---|
29+
| F-01 | Non-host peers can connect to guest listener because accept path only checks no source identity | High | Remediated | `crates/vz-guest-agent/src/listener.rs`: `source_cid` extraction and host-only acceptance logic in `accept()`. Non-host CIDs are closed and logged. |
30+
| F-02 | Sandbox session default exec path does not set an explicit non-root user, causing root execution when peer request omits `user` | High | Remediated | `crates/vz-sandbox/src/session.rs`: `DEFAULT_EXEC_USER = "dev"` and `resolve_exec_user(None) -> "dev"` used for default execution and `exec_streaming`. |
31+
| F-03 | Default-user policy is not explicitly asserted by tests | Medium | Remediated | `crates/vz-sandbox/src/session.rs`: `resolve_exec_user_*` unit tests validate default and explicit override behavior. |
32+
| F-04 | Guest listener allows non-host CID and does not exercise explicit host allowlist in unit tests | Medium | Remediated | `crates/vz-guest-agent/src/listener.rs`: `is_host_peer` unit test and host CID helper. |
33+
34+
## Explicit Sign-off
35+
36+
- Accepted Risk: None.
37+
- Remediations applied: F-01 through F-04.
38+
- Pending / open items: None for this closure.
39+
40+
## Verification Commands
41+
42+
- `cargo check -p vz-sandbox -p vz-guest-agent`
43+
- `cargo clippy -p vz-sandbox -p vz-guest-agent -- -D warnings`
44+
- `cargo nextest run -p vz-sandbox -p vz-guest-agent`

0 commit comments

Comments
 (0)