8000 docs(specs): refuse names a box's allow list does not match before forwarding upstream by norrietaylor · Pull Request #2009 · gominimal/minimal · GitHub
[go: up one dir, main page]

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
8 changes: 4 additions & 4 deletions crates/minimal/tests/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1311,7 +1311,7 @@ async fn activate_prints_no_session_id_when_composition_fails() {

/// Drives `min session activate` through the compiled binary against this
/// process's harness daemon while its user-namespace verdict is `verdict`
/// (NET-141), and returns the refusal: the `error:` block on stderr, exactly
/// (NET-143), and returns the refusal: the `error:` block on stderr, exactly
/// — the activation's own progress lines (`Applying loadouts: ...`) precede
/// it. Asserts the contract every cause shares on the way: exit 1, nothing
/// on stdout, and no session left behind. The binary is used because the
Expand Down Expand Up @@ -1385,7 +1385,7 @@ async fn refused_activation(verdict: minimald::server::UsernsRestriction) -> Str
}

/// A host whose user-namespace verdict refuses the sandbox fails the
/// activation before any session exists (NET-141): the daemon refuses the
/// activation before any session exists (NET-143): the daemon refuses the
/// create on its verdict, `min session activate` prints that refusal as its
/// error, exits 1, and leaves no session behind. The cause-specific texts
/// are the two tests below; this one holds the shape they share.
Expand Down Expand Up @@ -1454,7 +1454,7 @@ async fn activate_refusal_names_sysctl_for_max_user_namespaces_zero() {
}

/// Drives `min task run` through the compiled binary against this process's
/// harness daemon while its user-namespace verdict is `verdict` (NET-141),
/// harness daemon while its user-namespace verdict is `verdict` (NET-143),
/// and returns the refusal: the `error:` block on stderr, exactly. A task
/// run creates its ephemeral session through the same create the daemon
/// refuses, so the refusal must reach the user as the daemon's verdict —
Expand Down Expand Up @@ -1529,7 +1529,7 @@ async fn refused_task_run(verdict: minimald::server::UsernsRestriction) -> Strin
}

/// A refused host reaches `min task run` through the same create as an
/// activate, so the refusal is the machine verdict there too (NET-141):
/// activate, so the refusal is the machine verdict there too (NET-143):
/// printed verbatim, never wrapped as `CreateSession failed: ...` — a
/// wrapper that called the daemon's verdict an RPC failure would mislabel
/// it and bury the remedy's shape under a generic prefix.
Expand Down
2 changes: 1 addition & 1 deletion crates/minimald-rpc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ pub fn version_skew_message(cli: &str, daemon: &str) -> Option<String> {
})
}

/// How a user-namespace refusal opens (NET-141): the daemon formats a
/// How a user-namespace refusal opens (NET-143): the daemon formats a
/// create refused on the host's user-namespace verdict as this lead, the
/// cause (`sandbox::UsernsRestriction::cause`) closing the parenthesis,
/// and the cause's remedy for the daemon on a second line. Clients print
Expand Down
4 changes: 2 additions & 2 deletions crates/minimald/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1223,7 +1223,7 @@ async fn async_main(egress: &mut GuestEgress) -> Result<(), MainError> {
std::env::var(EGRESS_DENY_ALL_OPT_OUT_ENV).ok().as_deref(),
),
// Live: the create gate probes this process on every create
// (NET-141). The preflight below switches it off under
// (NET-143). The preflight below switches it off under
// `MINIMALD_USERNS_GATE=off`.
user_namespace_gate: minimald::server::UsernsGate::Live,
};
Expand Down Expand Up @@ -1254,7 +1254,7 @@ async fn async_main(egress: &mut GuestEgress) -> Result<(), MainError> {
// this process's own privileges and AppArmor label are what the kernel
// will check. On a restricted host (stock Ubuntu 24.04+ with an
// unconfined daemon) that denial otherwise surfaces only when the first
// attach dies writing /proc/self/uid_map. The create gate (NET-141)
// attach dies writing /proc/self/uid_map. The create gate (NET-143)
// refuses every session with the cause and the remedy before allocating
// anything, re-probing on each create so a remedy applied to the running
// host takes effect without a restart; this start-up read only warns
Expand Down
4 changes: 2 additions & 2 deletions crates/minimald/src/rpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ async fn serve_create_session(
return Ok(Errorable::Err { error: message });
}

// The user-namespace gate (NET-141): a host that refuses the
// The user-namespace gate (NET-143): a host that refuses the
// unprivileged user namespace every session sandbox starts by
// unsharing would otherwise mint a session whose first attach
// dies writing /proc/self/uid_map, with the cause in this log
Expand Down Expand Up @@ -547,7 +547,7 @@ async fn serve_create_session(
/// carries.
const HOST_IP_ENFORCEMENT_ATTR: &str = "host_ip_enforcement";

/// The refusal a host's user-namespace restriction yields (NET-141): the
/// The refusal a host's user-namespace restriction yields (NET-143): the
/// wire crate's lead — what the client recognises the refusal by — around
/// the cause in the words the person reads, then that cause's own remedy
/// for the daemon on its own line: the install step and the restart it
Expand Down
2 changes: 1 addition & 1 deletion crates/minimald/src/rpc/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2806,7 +2806,7 @@ fn advisory_over_a_probe_cause_names_the_refusal_not_a_blanket_unenforced() {
);
}

/// The user-namespace gate (NET-141): a host whose verdict refuses the
/// The user-namespace gate (NET-143): a host whose verdict refuses the
/// sandbox refuses the create itself, and the reply carries the verdict
/// — the cause in words and that cause's own remedy — with nothing
/// allocated for a caller to tear down. The gate is this server's own
Expand Down
6 changes: 3 additions & 3 deletions crates/minimald/src/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ pub struct Config {
#[serde(default)]
pub deny_all_opt_out: bool,
/// How the create gate learns the host's verdict on the unprivileged
/// user namespace every session sandbox starts by unsharing (NET-141).
/// user namespace every session sandbox starts by unsharing (NET-143).
/// Never persisted or handed in: the daemon binary decides it at start
/// (live, or off under `MINIMALD_USERNS_GATE=off`), and the test harness
/// fixes it per server.
Expand All @@ -304,7 +304,7 @@ pub fn this_daemon_path() -> String {
/// dependency of their own.
pub use sandbox::UsernsRestriction;

/// Where the create gate's user-namespace verdict comes from (NET-141).
/// Where the create gate's user-namespace verdict comes from (NET-143).
#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)]
pub enum UsernsGate {
/// Probe this process on every create — a few `/proc` reads — so a
Expand Down Expand Up @@ -802,7 +802,7 @@ impl ServerStateHandle {
}

/// The host's user-namespace verdict the create gate decides on
/// (NET-141), per [`Config::user_namespace_gate`]: probed live from this
/// (NET-143), per [`Config::user_namespace_gate`]: probed live from this
/// process on each call, so nothing is cached across creates; the lock
/// is held for the copy of the gate alone, never across the probe.
pub(crate) async fn user_namespace_verdict(&self) -> Option<UsernsRestriction> {
Expand Down
6 changes: 3 additions & 3 deletions crates/sandbox/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2960,7 +2960,7 @@ fn shell_word(path: &str) -> String {
/// a loss of isolation.
///
/// A `Some` is not advisory: `minimald` gates every session create on it
/// (NET-141), refusing the create with this cause and its remedy, so a false
/// (NET-143), refusing the create with this cause and its remedy, so a false
/// `Some` — a host whose kernel would in fact allow the unshare — refuses
/// every session on that host. Three facts are read, each live from `/proc`
/// on every call, so a remedy applied to a running host takes effect at the
Expand Down Expand Up @@ -3801,7 +3801,7 @@ mod tests {
/// The daemon's AppArmor remedy names the install step for the
/// installed daemon and, on its own line, the loader with `--path` for
/// this daemon's own path, so a source-built daemon is covered too
/// (NET-141).
/// (NET-143).
#[test]
fn daemon_apparmor_remedy_names_finalize_install_and_this_binary() {
let remedy = UsernsRestriction::ApparmorUnconfined.remedy(RemedyTarget::Daemon {
Expand All @@ -3827,7 +3827,7 @@ mod tests {

/// The mip AppArmor remedy is a command the operator pastes, so a data
/// dir or binary path with a space stays one shell word, while a plain
/// path is shown bare (NET-141).
/// path is shown bare (NET-143).
#[test]
fn mip_apparmor_remedy_quotes_paths_with_spaces() {
let remedy = UsernsRestriction::ApparmorUnconfined.remedy(RemedyTarget::Mip {
Expand Down
24 changes: 23 additions & 1 deletion docs/specs/18-spec-box-networking/18-spec-box-networking.md
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ included, with every refusal logged (NET-001 to NET-004).
verify: cargo nextest run -p minimald run_box_ends_when_its_run_ends
<!-- S1b-2c; event-driven within the WHILE; `min task run` creates a session for the run, execs the task into it, and today destroys it from the client; the destroy moves to the daemon side of the exec's exit so a lost client strands no session; the abandoned-launch reap covers only un-finalized sessions and does not reach this case -->

- **NET-141** WHEN an activation targets a host whose user-namespace verdict refuses the sandbox THE SYSTEM SHALL fail the activation before the session is created, name the cause, exit non-zero, and leave no session behind.
- **NET-143** WHEN an activation targets a host whose user-namespace verdict refuses the sandbox THE SYSTEM SHALL fail the activation before the session is created, name the cause, exit non-zero, and leave no session behind.
tier: T0
verify: cargo nextest run -p minimal activate_refuses_unconfinable_sandbox_before_session_creation
verify: cargo nextest run -p minimald create_reply_carries_user_namespace_verdict
Expand Down Expand Up @@ -484,6 +484,12 @@ included, with every refusal logged (NET-001 to NET-004).
verify: cargo nextest run -p minimald dns_pinned_admission_window
<!-- S8b/AC1; prose 42; event-driven -->

- **NET-141** WHEN the resolver Minimal owns for a box receives a query for a name that its `egress.allow_dns_hosts` does not match, as NET-066 matches names, and that is not in the box zone (NET-072, including `host.min.internal`), THE SYSTEM SHALL answer REFUSED and forward the query to no resolver beyond it.
tier: T1
verify: cargo nextest run -p minimald unmatched_name_refused_without_upstream_forward
property: for every name, every allow-pattern set, and every zone, the resolver forwards the query beyond the one Minimal owns for the box only if the name matches a pattern or is in the zone, and it never forwards a name that matches none and is not in the zone
Comment on lines +487 to +490
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LOW · correctness · confidence 0.60 · in NET-141

NET-141 states a general rule and a property over every allow-pattern set. Its verify: test unmatched_name_refused_without_upstream_forward in crates/minimald/src/net/dns_gate.rs drives only a deny-all relay (deny_all_egress()). The annotation admits that a partial allow list is still forwarded. So the requirement is false for partial lists and the verify line passes without proving it. Add a partial-list case to the test, or scope the requirement to deny-all until the gate refuses for every list.

What the reviewer checked

dns_gate.rs:2687 test uses spawn_test_relay(&deny_all_egress()); annotation: 'NET-141 holds only for deny-all boxes'

Hand this to an agent
Act on a code-review finding.

File:       docs/specs/18-spec-box-networking/18-spec-box-networking.md
Lines:      487-490
Category:   correctness
Severity:   low (reviewer confidence 0.60)

Finding:
NET-141 states a general rule and a property over every allow-pattern set. Its `verify:` test `unmatched_name_refused_without_upstream_forward` in `crates/minimald/src/net/dns_gate.rs` drives only a deny-all relay (`deny_all_egress()`). The annotation admits that a partial allow list is still forwarded. So the requirement is false for partial lists and the verify line passes without proving it. Add a partial-list case to the test, or scope the requirement to deny-all until the gate refuses for every list.

First verify this against the code yourself. It is model output and
may be wrong. If it is wrong, or the tradeoff was deliberate, say so
and change nothing.

👍 useful · 👎 noise — react to tell the reviewer. Replying works too, and says more.

<!-- design §5.3 ("Non-matching names are refused"); event-driven; the complement of NET-066, decided by the same matcher, so admission and refusal never disagree on a name: case-insensitive, a trailing dot ignored, the same wildcard rule. "Beyond the one Minimal owns" covers every onward hop: the gateway resolver, the host's resolvers, and the Egress Gateway when enrolled. The refusal is REFUSED (rcode 5), answered at once: never NXDOMAIN, which asserts the name does not exist and is negatively cached for the whole name (negative answers are governed by NET-125 and NET-128), and never a timeout. The refusal comes before any onward query, because a forwarded query carries data out in the queried name even when the connection that follows is dropped (NET-062). Names steered to the Box Egress Proxy need no exemption: the credentialed set stays inside the allow list, so they always match. Scope: the requirement binds queries that reach the resolver Minimal owns for the box. For an own-address box, on a native host and inside a VM-backed host alike, that is the relay's DNS gate: the box's `resolv.conf` names the switch gateway, every one of its frames passes the relay (NET-081), and the gate intercepts every standard query to that resolver before it reaches the switch, which is where the refusal is made. For a host-address box inside a VM-backed host it is the node's DNS layer once NET-003's sub-bullet holds; until that routing lands, a host-address box's queries go to the switch gateway without passing a per-box gate, and NET-141 binds nothing for it. A native host-address box shares the host's network namespace, so if its egress rules allow the host's own resolver it can query that resolver directly, and no Minimal resolver sees the query; that is the reduced tier the design concedes for host-address boxes, and closing it needs a native routing rule this spec does not yet state. On a VM-backed host the refusal follows NET-066's two legs: the deciding copy is host-side, in minvmd's DNS gate outside the VM escape boundary, and the in-VM gate is the precision copy, so the VM-host proof must show the refusal at the host-side leg. Residual: minvmd's DNS table today observes a box's queries and pins the replies but cannot refuse a query, so until it can, the refusal on a VM-backed host holds only below the escape boundary. The T1 property is the proof that no onward query is made. The session e2e has two legs for names outside the allow list. The deny-all box's leg asserts a fast resolution failure: its lookup fails inside 5000 ms, and its connection fails at resolution. The disallowed-name leg of the box with a partial allow list asserts only that the connection does not complete. It prints whether the name was refused fast or timed out. When the refusal covers a partial allow list, that leg must assert a fast resolution failure too. Neither leg observes whether an onward query occurs. Known gap: the relay's DNS gate refuses only for a deny-all box, and the verify test covers only that case. For a box with a partial allow list, the relay still forwards a name that matches no pattern. Until the gate refuses for every allow list and the test covers a partial list, NET-141 holds only for deny-all boxes. -->

- **NET-067** IF an allowed name resolves into the box's `egress.deny_subnets` or the infrastructure deny set THEN THE SYSTEM SHALL refuse the connection.
tier: T2
verify: cargo nextest run -p minimald denied_range_resolution_refused
Expand Down Expand Up @@ -1287,6 +1293,14 @@ What a native co-resident host cannot offer is an enforcement point outside its
escape boundary ([design §7.4][design]); its egress rules hold only while the
daemon does, and the attribute that makes that gap visible to policy is EHE's.

The invariant that no unmatched name travels beyond the resolver Minimal owns
for a box (NET-141) has two stated limits. A native host-address box shares the
host's network namespace. If its egress rules allow the host's own resolver,
it can query that resolver directly, and no Minimal resolver sees the query.
The design concedes this reduced tier for host-address boxes. On a VM-backed
host, the host-side DNS table cannot yet refuse a query. Until it can, the
refusal holds only inside the VM, below the escape boundary.
Comment on lines +1297 to +1302
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LOW · correctness · confidence 0.50 · in NET-141

The text says the host-side DNS table cannot yet refuse a query. But crates/minvmd/src/net/dns_pins.rs (deny_all_refusal) already decides deny-all box queries host-side. It drops them before the switch. The residual is true for partial allow lists only. State that difference, or the paragraph and the invariant misdescribe the deny-all case.

What the reviewer checked

dns_pins.rs:1299 'The host-side deny-all DNS gate ... decided outside the VM for own-address rows'

Hand this to an agent
Act on a code-review finding.

File:       docs/specs/18-spec-box-networking/18-spec-box-networking.md
Lines:      1296-1301
Category:   correctness
Severity:   low (reviewer confidence 0.50)

Finding:
The text says the host-side DNS table cannot yet refuse a query. But `crates/minvmd/src/net/dns_pins.rs` (`deny_all_refusal`) already decides deny-all box queries host-side. It drops them before the switch. The residual is true for partial allow lists only. State that difference, or the paragraph and the invariant misdescribe the deny-all case.

First verify this against the code yourself. It is model output and
may be wrong. If it is wrong, or the tradeoff was deliberate, say so
and change nothing.

👍 useful · 👎 noise — react to tell the reviewer. Replying works too, and says more.


## Security considerations

- **Invariant:** THE SYSTEM SHALL admit no connection to a box, and no
Expand Down Expand Up @@ -1324,6 +1338,14 @@ daemon does, and the attribute that makes that gap visible to policy is EHE's.
set.
enforced by: the rebinding intersection
covered by: NET-066, NET-067
- **Invariant:** THE SYSTEM SHALL carry no name that a box's allow list does
not match, outside the box zone, beyond the resolver Minimal owns for the
box B6FD , for every query that reaches that resolver.
enforced by: the refusal at the resolver Minimal owns for the box. On a
VM-backed host the in-VM gate makes it, until minvmd's host-side DNS gate
can refuse a query itself. That residual and the native host-address box's
bypass are the two limits stated in the design reasoning.
covered by: NET-141
Comment thread
minimal-foundry[bot] marked this conversation as resolved.
Resolved
Comment on lines +1341 to +1348
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LOW · security · confidence 0.55 · in NET-141

The new invariant says no unmatched name leaves the box's resolver, with no qualifier. NET-141's own comment says the relay's DNS gate refuses only for deny-all boxes. A box with a partial allow list still forwards unmatched names upstream, and the verify test covers only deny-all. The invariant is therefore false today for the common case, and a reader will assume the exfiltration channel is closed. Add the partial-allow-list gap to the invariant's enforced by text, next to the two stated limits.

What the reviewer checked

NET-141 comment: 'Known gap: the relay's DNS gate refuses only for a deny-all box ... Until the gate refuses for every allow list and the test covers a partial list, NET-141 holds only for deny-all boxes.'

Hand this to an agent
Act on a code-review finding.

File:       docs/specs/18-spec-box-networking/18-spec-box-networking.md
Lines:      1340-1347
Category:   security
Severity:   low (reviewer confidence 0.55)

Finding:
The new invariant says no unmatched name leaves the box's resolver, with no qualifier. NET-141's own comment says the relay's DNS gate refuses only for deny-all boxes. A box with a partial allow list still forwards unmatched names upstream, and the verify test covers only deny-all. The invariant is therefore false today for the common case, and a reader will assume the exfiltration channel is closed. Add the partial-allow-list gap to the invariant's `enforced by` text, next to the two stated limits.

First verify this against the code yourself. It is model output and
may be wrong. If it is wrong, or the tradeoff was deliberate, say so
and change nothing.

👍 useful · 👎 noise — react to tell the reviewer. Replying works too, and says more.

- **Invariant:** THE SYSTEM SHALL publish a dynamically requested port only
under a recorded allow decision.
enforced by: the local daemon evaluates the same request shape as the
Expand Down
Loading
0