Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 55 additions & 28 deletions .github/workflows/run-nvx-microvm-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -516,7 +516,7 @@ jobs:

with open(sys.argv[1], encoding="utf-8") as stream:
config = json.load(stream)
if config["format"] != 3 or config["mount"]["owner"] != "caller":
if config["format"] != 7 or config["export"]["owner"] != "caller":
sys.exit(f"managed sandbox did not persist caller ownership: {config}")
EOF
python3 scripts/nvx.py sandbox start --state-dir "${state}"
Expand Down Expand Up @@ -627,26 +627,33 @@ jobs:
--workload-user 65534:65534
python3 - "${state}/config.json" <<'EOF'
import json
import os
import sys

with open(sys.argv[1], encoding="utf-8") as stream:
config = json.load(stream)
shares = [
(
mount["guest_target"],
mount["access"],
mount["denied_paths"],
mount["kind"],
# The export plan binds each share's root, in the order of the
# targets' depth, then the targets.
export = config["export"]
shares = []
for bind in export["binds"]:
child = export["children"][bind["child"]]
shares.append(
(
bind["target"],
bind["source"],
"ro" if bind["readOnly"] else "rw",
[os.path.relpath(path, child["root"]) for path in child["denied"]],
"file" if child.get("file") else "directory",
)
)
for mount in config["mounts"]
]
expected = [
("/workspace", "rw", ["nvx-denied"], "directory"),
("/opt/hostedtoolcache", "ro", ["nvx-denied"], "directory"),
("/config/settings.json", "ro", [], "file"),
("/results/output.txt", "rw", [], "file"),
("/workspace", "", "rw", ["nvx-denied"], "directory"),
("/config/settings.json", "", "ro", [], "file"),
("/opt/hostedtoolcache", "", "ro", ["nvx-denied"], "directory"),
("/results/output.txt", "", "rw", [], "file"),
]
if config["format"] != 6 or "mount" in config or shares != expected:
if config["format"] != 7 or "mounts" in config or shares != expected:
sys.exit(f"managed sandbox did not persist every share: {config}")
EOF
python3 scripts/nvx.py sandbox start --state-dir "${state}"
Expand Down Expand Up @@ -753,20 +760,26 @@ jobs:
--workload-user 65534:65534
python3 - "${state}/config.json" <<'EOF'
import json
import os
import sys

with open(sys.argv[1], encoding="utf-8") as stream:
config = json.load(stream)
shares = [
(
mount["guest_target"],
mount["access"],
mount["denied_paths"],
mount["allowed_paths"],
mount["writable_paths"],
# The export plan holds the policy paths resolved inside the share.
export = config["export"]
shares = []
for bind in export["binds"]:
child = export["children"][bind["child"]]
shares.append(
(
bind["target"],
"ro" if bind["readOnly"] else "rw",
*(
[os.path.relpath(path, child["root"]) for path in child[name]]
for name in ("denied", "allowed", "write")
),
)
)
for mount in config["mounts"]
]
expected = [
(
"/workspace",
Expand All @@ -776,7 +789,7 @@ jobs:
["nvx-writable"],
),
]
if config["format"] != 5 or "mount" in config or shares != expected:
if config["format"] != 7 or "mounts" in config or shares != expected:
sys.exit(f"managed sandbox did not persist the access policy: {config}")
EOF
python3 scripts/nvx.py sandbox start --state-dir "${state}"
Expand Down Expand Up @@ -1091,8 +1104,17 @@ jobs:
Initialize-Shares
Invoke-Nvx sandbox provision --state-dir $state --hypervisor "${{ inputs.backend }}" --layer $layer --scratch $scratch @shareOptions --workload-user 65534:65534
$config = Get-Content -Raw (Join-Path $state "config.json") | ConvertFrom-Json
$shares = ($config.mounts | ForEach-Object { "$($_.guest_target),$($_.access),$($_.kind)" }) -join " "
if ($config.format -ne 6 -or $shares -ne "/workspace,rw,directory /opt/hostedtoolcache,ro,directory /config/settings.json,ro,file /results/output.txt,rw,file") {
# The export plan binds each share's root, in the order of the
# targets' depth, then the targets.
$shares = ($config.export.binds | ForEach-Object {
$bind = $_
$child = $config.export.children[$bind.child]
$mode = if ($bind.readOnly) { "ro" } else { "rw" }
$kind = if ($child.file) { "file" } else { "directory" }
$denied = ($child.denied | ForEach-Object { $_.Substring($child.root.Length + 1).Replace("\", "/") }) -join ";"
"$($bind.target),$($bind.source),$mode,$denied,$kind"
}) -join " "
if ($config.format -ne 7 -or $shares -ne "/workspace,,rw,nvx-denied,directory /config/settings.json,,ro,,file /opt/hostedtoolcache,,ro,nvx-denied,directory /results/output.txt,,rw,,file") {
throw "managed sandbox did not persist every share: $($config | ConvertTo-Json -Compress -Depth 5)"
}
try {
Expand Down Expand Up @@ -1197,8 +1219,13 @@ jobs:
Initialize-Share
Invoke-Nvx sandbox provision --state-dir $state --hypervisor "${{ inputs.backend }}" --layer $layer --scratch $scratch @shareOptions --workload-user 65534:65534
$config = Get-Content -Raw (Join-Path $state "config.json") | ConvertFrom-Json
$mount = $config.mounts[0]
if ($config.format -ne 5 -or $config.mounts.Count -ne 1 -or $mount.access -ne "rw" -or ($mount.denied_paths -join ",") -ne "nvx-denied" -or ($mount.allowed_paths -join ",") -ne "nvx-denied/nvx-allowed" -or ($mount.writable_paths -join ",") -ne "nvx-writable") {
# The export plan holds the policy paths resolved inside the share.
$child = @($config.export.children)[0]
$bind = @($config.export.binds)[0]
function Get-Relative($paths) {
($paths | ForEach-Object { $_.Substring($child.root.Length + 1).Replace("\", "/") }) -join ","
}
if ($config.format -ne 7 -or @($config.export.children).Count -ne 1 -or @($config.export.binds).Count -ne 1 -or $bind.target -ne "/workspace" -or $bind.readOnly -or (Get-Relative $child.denied) -ne "nvx-denied" -or (Get-Relative $child.allowed) -ne "nvx-denied/nvx-allowed" -or (Get-Relative $child.write) -ne "nvx-writable") {
throw "managed sandbox did not persist the access policy: $($config | ConvertTo-Json -Compress -Depth 5)"
}
try {
Expand Down
5 changes: 4 additions & 1 deletion aci_edge_sandboxes/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -853,7 +853,10 @@ translation, for example to turn a host working directory into a
or singles out, such as a read-write path inside a read-only one, must not
contain colons, backslashes, links, or whitespace other than spaces, and no
name in it may begin or end with a space.
- Mapped paths must exist and be directories or regular files. A path listed
- Mapped paths must exist and be directories or regular files. On Windows, a
mapped or denied path that only a verbatim `\\?\` path names, such as that
of a directory whose name ends in a period or a space, is refused, since the
path without the prefix would name another object, or none. A path listed
both read-only and read-write is mapped read-only. A read-only path,
directory or file, must not resolve to a path inside a read-write
directory, but a read-write path may lie inside a read-only directory.
Expand Down
29 changes: 15 additions & 14 deletions aci_edge_sandboxes/src/openvmm/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ use std::thread;
use std::time::Duration;

use nvx::Artifacts;
use nvx::host::filesystem::{self, ExportPlan, ExportRequest, PathList};
use nvx::host::filesystem::{self, ExportPlan, ExportRequest, Mapping, PathList, ReservedTargets};
use nvx::host::instance::{self, Execution, Found, LockedInstance, WorkloadOutcome};
use nvx::host::launch::{LaunchSpec, WorkloadIdentity};
use nvx::host::report::Report;
Expand Down Expand Up @@ -325,21 +325,22 @@ fn egress_rule(rule: &NetworkRule) -> network::EgressRule {
}
}

/// Plans the export of the contract's filesystem section, under the contract's field names.
/// Plans the export of the contract's filesystem section, under the contract's field names, at
/// the guest paths of the host-path convention in the blockless root.
fn export_plan(policy: &FilesystemPolicy) -> Result<Option<ExportPlan>> {
let list = |name, paths| PathList { name, paths };
let mappings: Vec<Mapping<'_>> = Mapping::convention(
list("filesystem.readonlyPaths", &policy.readonly_paths),
true,
)
.chain(Mapping::convention(
list("filesystem.readwritePaths", &policy.readwrite_paths),
false,
))
.collect();
Ok(filesystem::plan(&ExportRequest {
read_only: PathList {
name: "filesystem.readonlyPaths",
paths: &policy.readonly_paths,
},
read_write: PathList {
name: "filesystem.readwritePaths",
paths: &policy.readwrite_paths,
},
denied: PathList {
name: "filesystem.deniedPaths",
paths: &policy.denied_paths,
},
denied: list("filesystem.deniedPaths", &policy.denied_paths),
..ExportRequest::new(&mappings, ReservedTargets::BLOCKLESS_ROOT)
})?)
}

Expand Down
26 changes: 17 additions & 9 deletions aci_edge_sandboxes/src/openvmm/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,11 @@ use crate::model::{HostLoopbackForward, NetworkPolicy};
/// Version of `sandbox.json`, the configuration that this crate records, which holds the
/// sandbox's [`ExportPlan`] in the form of `nvx`.
///
/// Version 1 described sandboxes assembled from image layers and a scratch disk, and version 2
/// Version 1 described sandboxes assembled from image layers and a scratch disk, version 2
/// exported the mapped host paths as one directory whose bind mounts the kernel command line
/// listed.
pub(crate) const STATE_FORMAT: u32 = 3;
/// listed, and version 3 recorded plans whose exported directories could be hidden but for their
/// allowed paths.
pub(crate) const STATE_FORMAT: u32 = 4;
/// Backend key recorded in every sandbox.
pub(crate) const BACKEND_KEY: &str = "openvmm";

Expand Down Expand Up @@ -340,18 +341,26 @@ mod tests {
"binds": [{ "source": "", "target": "/host/work", "readOnly": true }],
});
unsupported(&legacy);
// Version 3 recorded whether OpenVMM hid each exported directory.
legacy["format"] = 3.into();
legacy["filesystem"] = serde_json::json!({
"children": [{ "root": "/host/work", "writable": false, "hidden": false,
"denied": [], "deniedIdentities": [], "allowed": [], "write": [] }],
"binds": [{ "child": 0, "source": "", "target": "/host/work", "readOnly": true }],
});
unsupported(&legacy);
}

#[test]
fn records_of_the_current_format_keep_loading() {
// A record that the crate wrote before NVX moved to `nvx-rs`, with a file and a
// directory child, pinned identities, and every optional field.
// A record with a file and a directory child, pinned identities, and every optional
// field.
let root = tempfile::tempdir().unwrap();
let store = StateStore::open(root.path()).unwrap();
let id = SandboxId::generate().unwrap();
store.create(&id, &record()).unwrap();
let written = serde_json::json!({
"format": 3,
"format": 4,
"backend": "openvmm",
"network": {
"egress": {
Expand All @@ -364,9 +373,8 @@ mod tests {
"filesystem": {
"children": [
{ "root": "/host/work/config.json", "file": true, "writable": false,
"hidden": false, "denied": [], "deniedIdentities": [], "allowed": [],
"write": [] },
{ "root": "/host/work/out", "writable": true, "hidden": false,
"denied": [], "deniedIdentities": [], "allowed": [], "write": [] },
{ "root": "/host/work/out", "writable": true,
"denied": ["/host/work/out/secret"],
"deniedIdentities": [{ "device": 1, "index": 2 }], "allowed": [],
"write": [] }
Expand Down
4 changes: 2 additions & 2 deletions aci_edge_sandboxes/tests/openvmm_fake.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1995,8 +1995,8 @@ fn recorded_read_only_paths_inside_read_write_directories_are_refused_at_start()
..FilesystemPolicy::default()
});
let sandbox_id = nvx.provision(&request).unwrap().sandbox_id;
// An earlier crate recorded a read-only src inside the read-write work like this nesting of
// read-write paths, but with a read-only bind, which only the guest kept read-only.
// A record with a read-only bind of src inside the read-write work, which only the guest
// would keep read-only, as nothing that the planner makes holds.
let mut record = state_json(&fixture, &sandbox_id, "sandbox.json");
assert_eq!(
record["filesystem"]["binds"][1]["source"], "src",
Expand Down
38 changes: 22 additions & 16 deletions doc/run.md
Original file line number Diff line number Diff line change
Expand Up @@ -807,8 +807,8 @@ mode applies to the whole file.

A relative `--mount-deny`, `--mount-allow`, or `--mount-write` path is
resolved inside its share's host directory. With one share, every such path
applies to it. With several, each applies to the `--mount` before it and must
name a path inside that share's directory. The guest targets and the host
applies to it. With several, each applies to the `--mount` before it. Either
way, it must name a path inside its share's directory. The guest targets and the host
paths of the shares must not equal or contain one another, because one
share could otherwise hide another or reach its files under a different access
mode, so a shared file may lie beside a shared directory, but not inside it.
Expand Down Expand Up @@ -853,18 +853,22 @@ without its shares.
An `rw` share supports the symbolic links that package managers and
language toolchains create; see
[virtio-fs host mapping](#virtio-fs-host-mapping) for their semantics. The
existing OpenVMM file-identity policy applies to each share. A managed
sandbox stores each share's absolute host path, kind, mode, denied, allowed,
and writable paths and the ownership mode in its configuration, and reattaches
every share on each `start`, which refuses a host path that no longer holds a
directory or regular file as recorded. A configuration with several shares
uses format 4, which earlier NVX releases reject rather than start without the
other shares.
A configuration with allowed or writable paths uses format 5, which earlier
NVX releases reject rather than start a share without its access policy.
A configuration with a shared file uses format 6, which earlier NVX releases
reject rather than start without the file. A host path with commas keeps the
configuration's format, because earlier NVX releases already refuse it.
existing OpenVMM file-identity policy applies to each share.
`run` and `provision` plan the shares once, with the planner that every NVX
caller uses: it resolves each host path and access-policy path, which must
exist, refuses what OpenVMM or the guest would refuse, and records the file
identity of each shared or denied path that the workload could move, those
inside a writable part of a share. On Windows, it refuses a path that only a
verbatim `\\?\` path names, such as that of a directory whose name ends in a
period or a space, which the path without the prefix would not name. The
shares' children follow their resolved host paths. A managed sandbox stores
the plan in its configuration, in format 7, and each `start` checks it again,
as `run` does once it has verified the distro layer: it refuses a host path
that no longer holds a directory or regular file as recorded, a recorded path
whose file identity changed, and shares whose host paths came to overlap, such
as through a hard link or a bind mount. NVX no longer reads configurations of
formats 2 to 6, which recorded the shares as requested; deprovision such a
sandbox and provision it again.

#### Access policy

Expand All @@ -889,8 +893,10 @@ or link reaches the share:
writable path to a read-only file fails with `EXDEV` (`Invalid cross-device
link`), so the link cannot make the file writable.

Each path must exist inside the share's host directory, must not cross a
symbolic link or junction, and is attributed to a share like `--mount-deny`.
Each path must exist inside the share's host directory, and is attributed to
a share like `--mount-deny`. `sandbox` resolves it once, when it plans the
share, so it may cross a symbolic link or junction to a path inside the same
directory; `run` passes it to OpenVMM, which refuses a path that crosses one.
Its names may contain spaces, but not begin or end with one, nor contain
other whitespace, colons, or backslashes.
Allowed paths follow the share's write policy: without `--mount-write`, an
Expand Down
Loading
Loading