Repository navigation
fix(minimald): refuse an activation whose declared port something else holds (0.7.0 backport) - #2738
Closed
norrietaylor wants to merge 9 commits into
Closed
norrietaylor wants to merge 9 commits into
norrietaylor wants to merge 9 commits into
Conversation
* fix(minimal,minvmd): hold the answerer channel socket owner-only The box-zone answerer's channel socket was world-connectable: the launchd plist bound it SockPathMode 0666 and the systemd socket unit SocketMode=0666, leaving the peer-uid gate the only check on who may connect. Both units now bind it owner-only -- SockPathOwner the operator, SockPathMode 0600 on launchd; SocketUser the operator, SocketMode=0600 on systemd -- with the uid gate kept as defence in depth. An install whose socket node carries group or other bits -- one that predates the mode, or a hand that relaxed it -- now fails custody through a channel_mode fact, so the advisory re-surfaces and a step re-run re-binds the socket. The accept loop's foreign-refusal warn is rate-limited to a few lines a second, so a connect storm cannot flood the log; the refusals themselves are never limited. * fix(minimal,minimald,minvmd): name the other account on a refused channel connect A daemon whose connect the owner-only channel socket refuses now says the host's DNS service belongs to another account, not a bare Permission denied. The answerer step needs the operator's account to resolve, with no stand-in uid, and custody fails a channel socket the operator does not own. * fix(minimal,minvmd): leave another operator's DNS service alone A channel socket owned by another existing, non-root account is that operator's service: the answerer step is now unavailable with the same sentence the daemons say on a refused connect, instead of a custody failure whose step would reinstall the unit and take the service over. A root-owned socket, or one whose uid names no account, still fails custody so the step re-binds it. The minvmd tests that take the process-global foreign-refusal slots now run one at a time and start and end with every slot free, and the warn-rate test bounds the burst at two windows' worth of lines, so a slow runner that spreads the burst does not fail it. * fix(minimal): offer the step over a socket another account left behind A channel socket another account owns refuses the answerer step only when a service is installed; with none, the step's install removes the leftover socket before it loads the unit, so it is offered. --------- Co-authored-by: gominimal-aw-bot[bot] <281738952+gominimal-aw-bot[bot]@users.noreply.github.com> (cherry picked from commit 746770d)
…er (#2300) * fix(net): refuse out-of-zone names at a guest deny-subtree DNS listener A deny-all host-address box on a VM-backed guest resolved out-of-zone names at the switch gateway: the interim NET-079 carve-out admitted the gateway's port 53 whole, UDP and TCP, so a box that admits no destination could still ask a resolver beyond the machine — and DNS over TCP could retry past the verdict. The guest's deny subtree now steers its UDP-53 lookups onto a loopback listener on the answerer's port (NET-141 slice 2): deny_out admits the listener beside the reply direction, the dstnat match carries no destination address (the box asks the gateway like every other host-address box; the steer is the carve-out), and the listener itself answers REFUSED for every name outside the box zone — apex and a non-lowercase zone suffix among them — NODATA for an in-zone non-A question, and forwards the zone's A questions to the switch gateway alone. DNS over TCP is neither steered nor admitted. The guest daemon starts the listener after its classifier table loads; the daemon's own lookups are untouched (cgroup-keyed match, deny path only). The interim --gateway-resolver installer flag and every test, digest and pin that spelled the interim carve-out follow the new shape. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(net): bound and pin the guest DNS listener's forwards Connect each forward socket to the gateway so a datagram from any other sender is dropped, cap forwards in flight so a flood cannot exhaust the daemon's descriptors, and drop the needless to_string calls clippy rejects in the guest classifier integration test. * fix(net): refuse questionless and non-QUERY messages at the guest DNS listener decide() forwarded a message with no question, and an UPDATE or NOTIFY whose zone section named an in-zone A, because every per-question check passed vacuously. Answer the first REFUSED and the rest NOTIMP, so the forward path carries only standard lookups. Bind the listener directly so its log does not read as the box-zone answerer, pin the integration test's boot-log check to the exact example.com REFUSED line, and cite NET-079 (the deny-all case) rather than NET-141 at the lines this change added. --------- Co-authored-by: gominimal-aw-bot[bot] <281738952+gominimal-aw-bot[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> (cherry picked from commit 2c2f8cc)
crate::traced landed on main after the release branch was cut, so the cherry-pick of #2300 did not compile here.
v0.7.2 is tagged at the branch head, so the version check requires the next patch release.
…aken (#2352) * fix(minimald,minimal): skip a taken host port instead of failing on it later An own-ip session that declares --ingress <port>:<internal> on a host port another process already holds activated silently (exit 0), and every later exec/attach failed on the raw forwarder refusal (HTTP 500: bind: address already in use) with no usable trace at activation. The daemon now reads the OS's own EADDRINUSE words in that refusal and skips just that mapping: the box keeps the forwards it could bind, the attach completes, and each skipped port logs a warn line with the session's name. Every other refusal keeps the existing rollback-and-fail. The client learns the box's published address in the finalize reply (report_published_address, gated like report_shared_port_collisions) and probes each declared TCP host port at activation, printing a warning that names the port and the way out -- the box activates either way, never refused on it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(minimald,minimal): fail on a taken host port and warn at activation Per review: a declared host port another process holds no longer skips the forward while the name stays registered. The apply rolls back and fails as NET-121 requires, with an error that names the taken port. The daemon now probes the declared ports at finalize, when no activate hook bound them, and reports the taken ones. The client prints one warning per port, in place of its own probe. * chore(worker-iterate): address CodeRabbit findings on PR #2352 * fix(minimald,minimal): say the next step for a taken host port and skip the probe on a vm host The taken-port error reaches the person at every failed command, so it now names the port and the next step (free the port, or activate again with --ingress <other>:<internal>) and drops the forwarder's raw refusal, which the apply's warn line already logs. On a VM-backed host the forwarder binds on the host OS, so a bind in the guest cannot find a port the host holds and can warn falsely for a host_ip box. host_ports_in_use now takes the control channel and reports nothing when the host unbinds the declared forwards. The comments state the all-or-nothing rule without review history, and the CLI warning says "activate again with --ingress" in place of "declare --ingress". * chore(worker-iterate): address CodeRabbit findings on PR #2352 * fix(minimald,minimal): fail finalize on a taken declared host port The finalize probe now runs right after the name registration, before the activate-hook launch and the Active write. A declared TCP host port that another process holds fails the finalize with the named-port error. The host_ports_in_use reply field, its request flag and the CLI warning are removed, because a held port no longer reaches a successful reply. * test(minimald): pin the finalize failure on a taken host port --------- Co-authored-by: gominimal-aw-bot[bot] <281738952+gominimal-aw-bot[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: minimal-foundry[bot] <327719912+minimal-foundry[bot]@users.noreply.github.com> (cherry picked from commit 9437ba3)
…e holds (T100) (#2725) * feat(minimald): refuse activation on a declared port held elsewhere (T100) Finalize refuses a declared ingress host port that a program outside Minimal holds, or that another box holds at the 127.0.0.1 fallback once a landed range verdict keeps the box there. The refusal is a typed PortInUseRefusal that names the port and its holder, and the daemon logs one warn line with the session, port, address and holder. * feat(minimal): print the port refusal when activation is refused (T100) min session activate ends with the daemon's refusal text alone when a declared host port is already in use: the port, its holder and the --ingress remedy, in place of the RPC context around it. * fix(minimald-rpc): name an unknown port holder as another program No box in the daemon's hostname registry holding the port does not prove the holder is outside Minimal: the daemon's own proxy and a co-resident node's box also arrive as the host holder. Arch-review condition C1. * fix(minimald): bind declared ports at publish on a VM-backed host On a VM-backed host the daemon cannot probe the host's ports, so a box went Active with a held declared port and failed only at its first attach. Finalize now binds the declared forwards toward the handed switch address before the Active write. A held port fails with the same refusal and holder text as the native probe. The first attach takes the host's "already bound" answer as the box's own forward. A box that finalizes at the 127.0.0.1 fallback under a pending range verdict still yields a port another box holds there. A watcher now waits for the verdict to land, and an absent landing reports the held port, the address and the holder in the daemon log. --------- Co-authored-by: minimal-foundry[bot] <327719912+minimal-foundry[bot]@users.noreply.github.com> (cherry picked from commit 703bb7f)
…ease/0.7.0 The two cherry-picks before this one keep main's text. Main renamed and added code that release/0.7.0 does not have, so this commit adapts it: - The finalize code lives in session.rs, not box_actor/setup.rs. The refusal block, warn_port_refusal and watch_fallback_landing moved there, and registry_name is the session module's own helper, not super::helpers::registry_name. - boxes::NetworkMode, boxes::IngressPolicy and boxes::IpProto are sessions::NetworkMode, sessions::IngressPolicy and sessions::IpProto. switch::BoxGate is switch::SessionGate. - crate::traced is not on release. The fallback watcher spawns with tokio::spawn under the same root span, and the fake forwarder in the policy tests spawns with tokio::spawn. - OwnAddressReporter::registry came with NET-129's dynamic-hold tracking on main. The attach path's holder lookup needs only the accessor, so this adds the accessor alone. - DestroySessionRequest has no only_if_unfinalized field on release, and the activate cleanup there sends a plain destroy. The refusal tests assert that the refused box is listed and not Active, then destroy it as the release cleanup does, and still assert that no record is left. The user-facing refusal text is main's, unchanged.
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the
 |
Contributor
|
Approved: no findings at Nothing to do. Clobber: not checked in depth. If that was not intentional, restore the earlier change. |
This was referenced Oct 29, 2026
Closed
…vation-port-held-0.7.0 # Conflicts: # scripts/install-host-classifier.sh
norrietaylor
pushed a commit
that referenced
this pull request
Nov 4, 2026
…e holds (#2725) T100. Finalize refuses a declared ingress host port that a program outside Minimal holds, or that another box holds at the 127.0.0.1 fallback once a landed range verdict keeps the box there. The typed PortInUseRefusal names the port and its holder, the daemon logs one warn line, and min session activate ends with that refusal text alone. An unknown holder is named as another program. On a VM-backed host, finalize binds the declared forwards toward the handed switch address before the Active write, and the first attach takes the host's "already bound" answer as the box's own forward. A box that finalizes at the fallback under a pending verdict gets a watcher that reports an absent landing. Backport of #2725, folded from #2738. Adapted to the line: - The finalize code lives in session.rs, not box_actor/setup.rs, and registry_name is the session module's own helper. In the CLI, activate_refusal applies to activate_session. - boxes:: becomes sessions:: and switch::BoxGate becomes switch::SessionGate. - The fallback watcher spawns with tokio::spawn under the same root span. - OwnAddressReporter::registry comes without NET-129's dynamic-hold tracking. The attach path's holder lookup needs only the accessor. - The line's DestroySessionRequest has no only_if_unfinalized, so the refusal tests assert the refused box is listed and not Active, destroy it plainly, and assert no record stays. (cherry picked from commit 703bb7f)
Merged
norrietaylor
pushed a commit
that referenced
this pull request
Nov 5, 2026
…aken (#2352) Prerequisite for T100. A declared TCP host port that another process holds at the box's published address fails the finalize before the Active write (NET-121), with an error that names the port and the next step. The daemon probes on a native host. On a VM-backed host the forwarder binds on the host OS, so the probe is skipped and the spawn's apply reports the port. A forwarder's EADDRINUSE refusal fails the apply the same way. Backport of #2352, folded from #2738. Adapted to the line: main's box_actor/setup.rs hunk lands in session.rs, boxes::IpProto and boxes::NetworkMode become sessions::, and the fake forwarder spawns with tokio::spawn. The split from T100 carries those renames here, so this commit builds alone. (cherry picked from commit 9437ba3)
norrietaylor
pushed a commit
that referenced
this pull request
Nov 5, 2026
…e holds (#2725) T100. Finalize refuses a declared ingress host port that a program outside Minimal holds, or that another box holds at the 127.0.0.1 fallback once a landed range verdict keeps the box there. The typed PortInUseRefusal names the port and its holder, the daemon logs one warn line, and min session activate ends with that refusal text alone. An unknown holder is named as another program. On a VM-backed host, finalize binds the declared forwards toward the handed switch address before the Active write, and the first attach takes the host's "already bound" answer as the box's own forward. A box that finalizes at the fallback under a pending verdict gets a watcher that reports an absent landing. Backport of #2725, folded from #2738. Adapted to the line: - The finalize code lives in session.rs, not box_actor/setup.rs, and registry_name is the session module's own helper. In the CLI, activate_refusal applies to activate_session. - boxes:: becomes sessions:: and switch::BoxGate becomes switch::SessionGate. - The fallback watcher spawns with tokio::spawn under the same root span. - OwnAddressReporter::registry comes without NET-129's dynamic-hold tracking. The attach path's holder lookup needs only the accessor. - The line's DestroySessionRequest has no only_if_unfinalized, so the refusal tests assert the refused box is listed and not Active, destroy it plainly, and assert no record stays. (cherry picked from commit 703bb7f)
Member
Author
|
Folded into #3019, which landed on |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backports #2725 (task T100, #2703) to
release/0.7.0, together with #2352, which it builds on. If a box's declared ingress port cannot be bound on the host at activation, because another box or a host program holds it, the activation is refused before the box is reported active. The refusal names the port and what holds it, and nothing is left behind. Before this, two boxes declaring the same port both activated and one silently lost the port.Stacked on #2678; merge that first. #2678 carries the 0.7.3 version bump and the
pulldown-cmarkadvisory fix. Until it merges, this diff also shows #2678's commits.Commits
fa050c4b0cherry-picks fix(minimald): fail activation when a declared host port is already taken #2352 ("fail activation when a declared host port is already taken"). It is not on the release branch, and feat(minimald): refuse an activation whose declared port something else holds (T100) #2725 rewrites the code it adds (host_ports_in_use, the finalize probe). Itssetup.rshunk goes intocrates/minimald/src/session.rs, where this branch keeps that code.6aa05b571cherry-picks feat(minimald): refuse an activation whose declared port something else holds (T100) #2725, with the subject'sfeatchanged tofix(the release version check rejects afeatcommit). Conflicts are resolved tomain's text. In the CLI,activate_refusalis applied toactivate_session, main'sactivate_box.5cdd5e302adapts it to the release branch:boxes::types becomesessions::, andBoxGatebecomesSessionGate.tokio::spawnreplacescrate::traced, which is not on this branch.OwnAddressReporter::registry()is added as an accessor only. Main's port-holder tracking is not backported; the refusal does not need it.only_if_unfinalized, so the cleanup test proves "never reached Active" by checking the refused box's listed status instead, then still asserts no record is left.Squash-merge the three. All tests from both PRs are kept, with no assertion dropped. The refusal text is
main's, unchanged.Verification (Linux, Lima)
just clippyandjust fmt-checkare clean.activation_refused_when_declared_port_is_held_*,host_port_in_use::*,activate_refusal_names_port_and_holder).minimald: 1006 passed, 1 failed, the VM-only cgroup EACCES failuresession_host::tests::a_hook_launch_does_not_advise_unenforced_placement.minimal: 546 passed, 3 failed: theresolver::tests::finalize_install_undo_removes_*tests, which try to remove root-owned files under/etc/systemd/systemin the VM. They fail the same way on the unmodified fix(net): backport the deny-all DNS refusal and the owner-only answerer socket to 0.7.0 #2678 head; this PR does not touchresolver.rs.minimald-rpc: 65 passed.