diff --git a/Makefile b/Makefile index 6a6ea142b..ce86792c5 100644 --- a/Makefile +++ b/Makefile @@ -19,8 +19,6 @@ GO_OSES := linux darwin ## Location to install dependencies to LOCALBIN ?= $(shell pwd)/bin -$(LOCALBIN): - mkdir -p $(LOCALBIN) ## Tool versions GOLANGCILINT_VERSION ?= v2.13.2 diff --git a/docs/advanced-network.md b/docs/advanced-network.md index ad5518a95..1a37f3c9e 100644 --- a/docs/advanced-network.md +++ b/docs/advanced-network.md @@ -64,7 +64,7 @@ answers. | --- | --- | --- | --- | --- | | 0 | 4 | magic | `SLKT` | `SLKT` | | 4 | 2 | version | highest it speaks (1) | the version both use; ≤ the guest's | -| 6 | 2 | offloads | offloads its tun accepts on write | offloads its tun accepts on write | +| 6 | 2 | offloads | offloads its tun accepts on write | CSUM, TSO4, TSO_ECN and USO4, always | | 8 | 2 | mtu | 0 | MTU for `tun0`; 0 keeps the current one | | 10 | 2 | flags | 0 | bit 0 `ON`: packet mode is on for this box | @@ -77,11 +77,11 @@ closing silently. A guest that does not speak the host's version closes the stream. The guest gives up on a hello that does not arrive in 5 seconds. Offload bits are the kernel's `TUN_F_*` values: `0x01` CSUM, `0x02` TSO4, -`0x04` TSO6, `0x08` TSO_ECN, `0x20` USO4, `0x40` USO6. Each side probes its +`0x04` TSO6, `0x08` TSO_ECN, `0x20` USO4, `0x40` USO6. The guest probes its kernel (USO needs Linux 6.2) and then enables, with `TUNSETOFFLOAD`, only the offloads both hellos carry; segmentation offloads are dropped when CSUM is not -common, and USO is enabled only as the USO4 and USO6 pair. The host offers -USO4 alone, so a guest never enables USO. A side never builds a frame whose GSO type the peer did not offer; +common, and USO is enabled only as the USO4 and USO6 pair. The host has no TUN +and segments in software; it offers USO4 alone, so a guest never enables USO. A side never builds a frame whose GSO type the peer did not offer; packets its kernel queued before a reconnect keep their old type, and a receiver may drop one it cannot take. diff --git a/docs/cluster.md b/docs/cluster.md index 8c0ea0e14..bde599390 100644 --- a/docs/cluster.md +++ b/docs/cluster.md @@ -74,8 +74,8 @@ A claim always enters at whatever node the client dialed: `no_redirect` set, so the target warms-or-provisions locally and a stale view can never ping-pong. The same redirect fires when the node lacks a golden for the key but gossip names a template owner, and when the node - sits at `max_claims` or `max_running`: a warm peer first, else a peer that - serves the pool. A node that would refuse a claim stops gossiping its pools + refuses admission (any [admission refusal](sandboxd-api.md#admission-refusals)): + a warm peer first, else a peer that serves the pool. A node that would refuse a claim stops gossiping its pools within a tick. A stale view can still name one; it answers 429, and the SDK then tries the next one and the origin. 3. **No candidates** — the node provisions locally: a golden clone (tens of @@ -151,7 +151,7 @@ claim placement resolves the holder. ## Querying members -`GET /v1/info` (root `api_token`) reports this node's pools plus the peer +`GET /v1/info` (root or operator token) reports this node's pools plus the peer data-plane addresses it currently sees: ```bash @@ -160,8 +160,9 @@ curl -s -H "Authorization: Bearer $TOKEN" http://node-a:7777/v1/info | jq . The [API page](sandboxd-api.md#get-v1info) describes the payload. -`peers`, served by `GET /v1/peers` (root or tenant token), is what the SDK -scatters across in `Lookup`. An empty/absent `peers` means a mesh of one. +`peers`, served by `GET /v1/peers` (root, operator or tenant token), is what the SDK +scatters across in `Lookup`; a node with `client_advertise` set answers the +client addresses. An empty/absent `peers` means a mesh of one. ## Relocating a handle diff --git a/docs/egress.md b/docs/egress.md index 97169f251..117930c3a 100644 --- a/docs/egress.md +++ b/docs/egress.md @@ -348,7 +348,9 @@ on, written to `audit.jsonl` (`op:"egress"`, `dest`, `port`, `method`, `decision`, and the secret **name** in `secret`); an intercepted `CONNECT` is recorded as a decision of its own before its inner requests. The HTTP and CONNECT doors deny a host longer than 253 bytes (the DNS limit); the audit -record clips it. A sandbox records at most 10 +record clips it. They also deny a host with a zone (`%`) or a non-ASCII byte, +which the dialer would resolve to another target than the rule matched; the +SOCKS5 door denies such a name the same way, and every door records the deny. A sandbox records at most 10 `deny` and `rate_limited` decisions per second after a burst of 100, dropping the rest, so a guest cannot flood the journals. `rate_limited` comes from [`egress_conn_rate`](deploy.md#configuration). diff --git a/docs/mcp.md b/docs/mcp.md index 7ab87a22b..134a586e5 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -42,7 +42,7 @@ in the guest, which can hold its reply up to 5 s past the cap. | `hibernate` | snapshot + stop, freeing memory while keeping id, files and processes; the next call that reaches the guest wakes it | | `promote` | publish the sandbox as the template `template_name` on its node; re-promoting replaces it | | `release` | destroy the sandbox and its files; the session forgets the id, so a second release is rejected as unknown | -| `node_info` | warm pools, promoted templates, live claims, drain state, capacity and mesh peers; needs the operator token | +| `node_info` | warm pools, promoted templates, live claims, drain state, capacity and mesh peers; needs the root or operator token | An egress-lane sandbox cannot `fork`, `checkpoint`, `hibernate` or `promote`. diff --git a/docs/sandboxd-api.md b/docs/sandboxd-api.md index 9e877dc96..1e3759db1 100644 --- a/docs/sandboxd-api.md +++ b/docs/sandboxd-api.md @@ -743,7 +743,7 @@ destroyed and the pool entry retires. Once such a pool is idle, its golden is deleted; a pool kept with `warm: 0` keeps its golden. `net`/`size` default like a claim's. Answers the fresh `GET /v1/info` payload. 400 bad key, negative warm/idle, -`warm_max` below `warm`, `idle_hibernate_seconds` on an egress pool, a +`warm_max` below `warm`, a reader pool with `warm` below 1, `idle_hibernate_seconds` on an egress pool, a negative archive duration or an `archive_after_seconds` not above the pool's `idle_hibernate_seconds`, duplicate pool, or a config-owned `egress`/`warmup`/`capture_trim`/`storage`/`replicate_seconds`/`reader`/`egress_upstream_env`/`tun` field; 401 bad api @@ -970,7 +970,7 @@ Auth: node API token; body `{"token": "", "name": "..."}` (name optional); the root `api_token` with an empty body `token` acts by id as root. Captures the sandbox's full state without stopping it and answers `200 {"checkpoint": {id, name?, sandbox_id, key, tenant?, created_at, -policy_source?, no_egress?, guest_env?}}` — `tenant` records the calling tenant, absent for root. +policy_source?, no_egress?, tun_off?, tun_profile?, egress_limit?, guest_env?}}` — `tenant` records the calling tenant, absent for root. 400 bad body or name, 401 bad api token, 404 unknown id or wrong sandbox token, 409 egress-lane sandbox, one with volumes attached (see [egress](egress.md)), a failed one, or an archived one (`POST /wake` or an exec @@ -1033,7 +1033,7 @@ part of the public API; an SDK caller has no reason to call it directly. Auth: node API token. Lists this node's checkpoints, newest first. A tenant sees only its own records; root sees everything. A checkpoint backing an archived claim is that claim's wake image, not a listable record. Answers -`200 {"checkpoints": [{id, name?, sandbox_id, key, tenant?, created_at, policy_source?, no_egress?, guest_env?}]}`, the +`200 {"checkpoints": [{id, name?, sandbox_id, key, tenant?, created_at, policy_source?, no_egress?, tun_off?, tun_profile?, egress_limit?, guest_env?}]}`, the record shape `POST /v1/sandboxes/{id}/checkpoint` returns. ## DELETE /v1/checkpoints/{id} diff --git a/docs/sdk-python.md b/docs/sdk-python.md index 476a36fb3..7b7f4d7c2 100644 --- a/docs/sdk-python.md +++ b/docs/sdk-python.md @@ -76,9 +76,10 @@ The rest of this guide is the per-method reference. client = Client("10.0.0.5:7777", api_token="...", timeout=120.0) ``` -`api_token` is the node token — a root `api_token` (full access) or a -tenant token (resource-creating verbs only; operator surfaces answer it -403). Every node of a cell shares the root token and the same tenants +`api_token` is the node token — a root `api_token` (full access), the +[`operator_token`](deploy.md#configuration) (the claim lifecycle, +`sandboxes`, `info` and `lookup`), or a tenant token (resource-creating +verbs only; operator surfaces answer it 403). Every node of a cell shares the root token and the same tenants set. `timeout` bounds every control-plane request and the data-plane dial and upgrade; a guest stream then lives until the guest ends it. Unlike the Go SDK, where a canceled context closes any call, only `run`/`exec` take a @@ -205,8 +206,8 @@ The caller mounts, flushes, and unmounts attach-only devices. See the [live volume contract](sandboxd-api.md#put-v1sandboxesidvolumes) for writer exclusion and dirty-marker limits. Each method takes `timeout=` (default: the client timeout, 120 s); a large -replacement can need longer, and a call that times out cancels the server run, -so retry the same target. +replacement can need longer, and a call that times out raises `APIError` with +status 0 and cancels the server run, so retry the same target. Responses include `volumes_want`, `volumes`, `states`, and `converging`. `VolumePhase` and `VOLUME_ATTACHED`, `VOLUME_PENDING`, and `VOLUME_DETACHING` diff --git a/docs/sdk.md b/docs/sdk.md index f54f7e1cd..9df07129b 100644 --- a/docs/sdk.md +++ b/docs/sdk.md @@ -104,9 +104,10 @@ client, err := sandbox.Connect("10.0.0.5:7777", ``` - `Connect(addr, opts...)` — `addr` is one node's address or origin. -- `WithAPIToken(token)` — the node token: a root `api_token` (full access) - or a tenant token (resource-creating verbs only; operator surfaces like - `Info` answer it 403). Every node of a cell shares the same root token +- `WithAPIToken(token)` — the node token: a root `api_token` (full access), + the [`operator_token`](deploy.md#configuration) (the claim lifecycle, the + tenant verbs, `Info` and `Lookup`), or a tenant token (resource-creating + verbs only; operator surfaces like `Info` answer it 403). Every node of a cell shares the same root token and the same tenants set. - `WithHTTPClient(client)` — replace the control-plane HTTP client when the caller needs a custom transport, proxy, or timeout. An `http.Transport`'s @@ -687,8 +688,8 @@ ctx) tears the shell down. ## Node operations -Verbs for operating a node — `Drain`, `Undrain`, `Cordon`, `Uncordon`, `SetPools`, -`SetPoolsCluster` and the tenant verbs need the root token — plus the reference +Verbs for operating a node — `Drain`, `Undrain`, `Cordon`, `Uncordon`, `SetPools` +and `SetPoolsCluster` need the root token, the tenant verbs the root or operator token — plus the reference the aggregated apiserver claims under: ```go diff --git a/e2e/cmd/androidsmoke/main.go b/e2e/cmd/androidsmoke/main.go index 2ac37a084..b6ee67aae 100644 --- a/e2e/cmd/androidsmoke/main.go +++ b/e2e/cmd/androidsmoke/main.go @@ -63,7 +63,7 @@ func run(addr, token, template string) error { if err != nil { return fmt.Errorf("checkpoint: %w", err) } - defer func() { _ = ckpt.Delete(ctx) }() + defer func() { _ = ckpt.Delete(context.WithoutCancel(ctx)) }() branch, err := ckpt.New(ctx) if err != nil { return fmt.Errorf("branch: %w", err) diff --git a/e2e/cmd/browsersmoke/main.go b/e2e/cmd/browsersmoke/main.go index f1c47e029..fb20693c7 100644 --- a/e2e/cmd/browsersmoke/main.go +++ b/e2e/cmd/browsersmoke/main.go @@ -57,7 +57,7 @@ func run(addr, token, template string) error { if err != nil { return fmt.Errorf("checkpoint: %w", err) } - defer func() { _ = ckpt.Delete(ctx) }() + defer func() { _ = ckpt.Delete(context.WithoutCancel(ctx)) }() branch, err := ckpt.New(ctx) if err != nil { return fmt.Errorf("branch: %w", err) diff --git a/mcp/tools.go b/mcp/tools.go index 33a3534a0..6ad2415c2 100644 --- a/mcp/tools.go +++ b/mcp/tools.go @@ -78,7 +78,7 @@ var ( schema(props{"sandbox_id": str(sandboxIDDesc), "template_name": str("template name to publish as")}, "sandbox_id", "template_name"), toolPromote, }, {"release", "Destroy a sandbox and free its resources; files and processes inside it are lost. This session forgets the id, so a second release of it is rejected as unknown.", schema(props{"sandbox_id": str(sandboxIDDesc)}, "sandbox_id"), toolRelease}, - {"node_info", "Report the connected node's warm pools, promoted templates, live claims, drain state, capacity, and mesh peers as JSON. Needs the operator token; a tenant token is refused.", schema(props{}), toolNodeInfo}, + {"node_info", "Report the connected node's warm pools, promoted templates, live claims, drain state, capacity, and mesh peers as JSON. Needs the root or operator token; a tenant token is refused.", schema(props{}), toolNodeInfo}, } ) diff --git a/protocol/README.md b/protocol/README.md index b3a55aabf..2c7fc756b 100644 --- a/protocol/README.md +++ b/protocol/README.md @@ -10,7 +10,8 @@ implementations of the silkd protocol: `sdk/python/tests/test_fixtures.py`. `req_*.json` are client→server frames, `resp_*.json` server→client. A frame -that the Rust or Go side cannot round-trip is a protocol drift bug. The Rust side also +that the Go side cannot round-trip, or that the Rust side cannot parse (a +request) or round-trip (a response), is a protocol drift bug. The Rust side also pins the corpus size, so adding a verb without its fixture fails CI. `enums.json` pins each wire enum's full value set (error/event/file kinds, diff --git a/sandboxd/egress/proxy.go b/sandboxd/egress/proxy.go index 4a0a8a4b9..53b980354 100644 --- a/sandboxd/egress/proxy.go +++ b/sandboxd/egress/proxy.go @@ -24,6 +24,7 @@ import ( "sync" "sync/atomic" "time" + "unicode/utf8" "golang.org/x/time/rate" ) @@ -737,20 +738,24 @@ func denied(w http.ResponseWriter, host string) { http.Error(w, fmt.Sprintf("egress denied: %s", host), http.StatusForbidden) } -// hostPort splits an authority; a bare host takes def, a port outside 1-65535, a host past 253 bytes or a malformed authority is refused rather than defaulted. +// hostPort splits an authority; a bare host takes def, a port outside 1-65535, a host past 253 bytes, a zone or non-ASCII byte, or a malformed authority is refused rather than defaulted. func hostPort(authority string, def uint16) (host string, port uint16, ok bool) { host, text, err := net.SplitHostPort(authority) if err != nil { literal, bracketed := strings.CutPrefix(authority, "[") if !bracketed { - return authority, def, authority != "" && len(authority) <= maxHostLen && !strings.ContainsAny(authority, ":[]") + return authority, def, plainHost(authority) && !strings.ContainsAny(authority, ":[]") } literal, closed := strings.CutSuffix(literal, "]") addr, parseErr := netip.ParseAddr(literal) - return literal, def, closed && parseErr == nil && addr.Is6() && len(literal) <= maxHostLen + return literal, def, closed && parseErr == nil && addr.Is6() && plainHost(literal) } n, err := strconv.ParseUint(text, 10, 16) - return host, uint16(n), err == nil && n != 0 && host != "" && len(host) <= maxHostLen + return host, uint16(n), err == nil && n != 0 && plainHost(host) +} + +func plainHost(host string) bool { + return host != "" && len(host) <= maxHostLen && !strings.ContainsFunc(host, func(r rune) bool { return r == '%' || r >= utf8.RuneSelf }) } func tcpAddrPort(a net.Addr) netip.AddrPort { diff --git a/sandboxd/egress/proxy_test.go b/sandboxd/egress/proxy_test.go index 431f7ce74..f0c86f26b 100644 --- a/sandboxd/egress/proxy_test.go +++ b/sandboxd/egress/proxy_test.go @@ -265,6 +265,23 @@ func TestADeniedForwardIs403AndAuditedAsDeny(t *testing.T) { } } +func TestANonASCIIForwardHostIsDeniedBeforeMatching(t *testing.T) { + policy := Policy{Allow: []Rule{{Host: "*.api.internal", Secret: "gh"}}} + secrets := fakeSecrets{"gh": {"Authorization", "Bearer SECRET"}} + events := make(chan Event, 4) + p := New(policy, secrets, nil, fixedDial("127.0.0.1:1"), func(ev Event) { events <- ev }, nil) + front := serveFront(t, p) + + conn := dialProxy(t, front.Listener.Addr().String(), "GET http://x.ap%C4%B0.internal/ HTTP/1.1\r\nHost: x.api.internal\r\n\r\n") + defer func() { _ = conn.Close() }() + if status := readStatus(t, bufio.NewReader(conn)); status != "HTTP/1.1 403 Forbidden" { + t.Fatalf("forward to a host that lowercases into the allow-list = %q, want 403", status) + } + if ev := recvEvent(t, events); ev.Decision != DecisionDeny || ev.Injected != "" { + t.Errorf("audit event = %+v, want deny with nothing injected", ev) + } +} + func TestConnectAllowTunnels(t *testing.T) { echo := echoServer(t) policy := Policy{Allow: []Rule{{Host: "echo.internal"}}} @@ -484,6 +501,7 @@ func TestBracketedIPv6AuthorityIsUnwrapped(t *testing.T) { {"[not-an-address]", "HTTP/1.1 403 Forbidden", DecisionDeny}, {"[2606:4700:4700::1111", "HTTP/1.1 403 Forbidden", DecisionDeny}, {"echo.internal]", "HTTP/1.1 403 Forbidden", DecisionDeny}, + {"[2606:4700:4700::1111%25.example.com]:443", "HTTP/1.1 403 Forbidden", DecisionDeny}, {"", "HTTP/1.1 403 Forbidden", DecisionDeny}, } { conn := dialProxy(t, front.Listener.Addr().String(), "CONNECT /x HTTP/1.1\r\nHost: "+tt.authority+"\r\n\r\n") diff --git a/sandboxd/egress/socks.go b/sandboxd/egress/socks.go index 506ea1001..5dc60374a 100644 --- a/sandboxd/egress/socks.go +++ b/sandboxd/egress/socks.go @@ -62,7 +62,7 @@ func (p *Proxy) serveSocks(ctx context.Context, conn net.Conn) { return } decision, intercept := p.tunnelDecision(host, port, false) - if intercept || port == 0 { + if intercept || port == 0 || !plainHost(host) { decision = DecisionDeny } ev := Event{Method: methodSOCKS, Host: host, Port: port, Decision: decision} diff --git a/sandboxd/egress/socks_test.go b/sandboxd/egress/socks_test.go index 3970816e5..2d5116a3f 100644 --- a/sandboxd/egress/socks_test.go +++ b/sandboxd/egress/socks_test.go @@ -114,6 +114,19 @@ func TestSocksRefusesWhatItCannotServe(t *testing.T) { } } +func TestSocksDeniesAndRecordsAZonedOrNonASCIIName(t *testing.T) { + for _, name := range []string{"2606:4700:4700::1111%.example.com", "x.exampl\u0130.com"} { + events := make(chan Event, 4) + _, addr := socksProxy(t, Policy{Allow: []Rule{{Host: "*.example.com"}}}, nil, fixedDial(echoServer(t)), events, nil) + if _, reply := socksExchange(t, addr, socksGreeting, socksNameRequest(name, 443)); reply[1] != socksDenied { + t.Errorf("reply code for %q = %#x, want denied", name, reply[1]) + } + if ev := recvEvent(t, events); ev.Method != methodSOCKS || ev.Host != name || ev.Decision != DecisionDeny { + t.Errorf("audit event for %q = %+v, want SOCKS5 deny", name, ev) + } + } +} + func TestSocksIPv4TargetMatchesLiteralRule(t *testing.T) { events := make(chan Event, 4) _, addr := socksProxy(t, Policy{Allow: []Rule{{Host: "127.0.0.1"}}}, nil, fixedDial(echoServer(t)), events, nil) diff --git a/sandboxd/engine/engine.go b/sandboxd/engine/engine.go index 5bbd0292f..a479dd4d8 100644 --- a/sandboxd/engine/engine.go +++ b/sandboxd/engine/engine.go @@ -339,7 +339,7 @@ func (e *Engine) DialGuestPort(ctx context.Context, vsockSocket string, port uin } if _, ok := resp.(*wire.Ready); !ok { _ = conn.Close() - return nil, fmt.Errorf("port_forward %d: %s", port, respFail(resp)) + return nil, fmt.Errorf("port_forward %d: %w", port, frameErr(resp)) } return &guestPortConn{Conn: conn, r: r}, nil } @@ -589,13 +589,6 @@ func isVMNotFound(err error) bool { return strings.Contains(err.Error(), vmNotFound) } -func respFail(resp wire.Response) string { - if errResp, ok := resp.(*wire.ErrorResp); ok { - return errResp.Error() - } - return "unexpected frame " + resp.RespType() -} - // belowFloor reports whether v is below floor; known is false for a dev build. func belowFloor(v, floor string) (below, known bool) { v = "v" + strings.TrimPrefix(v, "v") diff --git a/sandboxd/pool/reconcile.go b/sandboxd/pool/reconcile.go index 6cc51e872..3bfdd807e 100644 --- a/sandboxd/pool/reconcile.go +++ b/sandboxd/pool/reconcile.go @@ -151,18 +151,18 @@ func (m *Manager) sweepOrphanSnaps(ctx context.Context, snaps []string, snapsErr // An unreferenced hibernate snapshot is an orphan; fork and golden snapshots never span a restart if snapsErr != nil { logger.Warnf(ctx, "snapshot sweep skipped: %v", snapsErr) - } else { - orphans := slices.DeleteFunc(snaps, func(snap string) bool { - if strings.HasPrefix(snap, hibernatePrefix) { - return referenced[snap] - } - return !strings.HasPrefix(snap, forkPrefix) && !strings.HasPrefix(snap, goldenPrefix) - }) - m.runBounded(ctx, len(orphans), func(ctx context.Context, i int) { - m.dropSnap(ctx, orphans[i]) - logger.Infof(ctx, "removed orphan snapshot %s", orphans[i]) - }).Wait() + return } + orphans := slices.DeleteFunc(snaps, func(snap string) bool { + if strings.HasPrefix(snap, hibernatePrefix) { + return referenced[snap] + } + return !strings.HasPrefix(snap, forkPrefix) && !strings.HasPrefix(snap, goldenPrefix) + }) + m.runBounded(ctx, len(orphans), func(ctx context.Context, i int) { + m.dropSnap(ctx, orphans[i]) + logger.Infof(ctx, "removed orphan snapshot %s", orphans[i]) + }).Wait() } // pendingMovesLocked marks the journaled moves of loaded claims: a source keeps its claim gated until the record resolves; callers hold m.mu. diff --git a/sandboxd/pool/setpools.go b/sandboxd/pool/setpools.go index d287d9afe..21d8feeea 100644 --- a/sandboxd/pool/setpools.go +++ b/sandboxd/pool/setpools.go @@ -78,10 +78,9 @@ func (m *Manager) applyPools(ctx context.Context, desired map[types.PoolKey]conf } } for key, spec := range desired { - if p := m.pools[key]; p != nil { - continue + if m.pools[key] == nil { + m.pools[key] = newPool(spec) } - m.pools[key] = newPool(spec) } m.mu.Unlock() diff --git a/sandboxd/pool/tun.go b/sandboxd/pool/tun.go index e9b092429..e338e7ee9 100644 --- a/sandboxd/pool/tun.go +++ b/sandboxd/pool/tun.go @@ -134,15 +134,7 @@ func (m *Manager) seedTun() { if !ok { return } - held := map[string]netip.Addr{} - m.mu.Lock() - for id, sb := range m.claimed { - if sb.TunAddr.IsValid() { - held[id] = sb.TunAddr - } - } - m.mu.Unlock() - seeder.Seed(held) + seeder.Seed(m.heldTunAddrs()) } func (m *Manager) releaseTun(ctx context.Context, id string, addr netip.Addr) { @@ -187,14 +179,7 @@ func (m *Manager) reconcileTunAddrs(ctx context.Context) { return } logger := log.WithFunc("pool.reconcileTunAddrs") - held := map[string]netip.Addr{} - m.mu.Lock() - for id, sb := range m.claimed { - if sb.TunAddr.IsValid() { - held[id] = sb.TunAddr - } - } - m.mu.Unlock() + held := m.heldTunAddrs() lost, err := addrs.Confirm(ctx, held) if err != nil { logger.Warnf(ctx, "confirm overlay addresses: %v", err) @@ -230,6 +215,18 @@ func (m *Manager) reconcileTunAddrs(ctx context.Context) { } } +func (m *Manager) heldTunAddrs() map[string]netip.Addr { + held := map[string]netip.Addr{} + m.mu.Lock() + defer m.mu.Unlock() + for id, sb := range m.claimed { + if sb.TunAddr.IsValid() { + held[id] = sb.TunAddr + } + } + return held +} + // confirmAdoptedTun stamps an adopted claim's address before its row stops sparing it; an address another claim took is dropped. func (m *Manager) confirmAdoptedTun(ctx context.Context, sb *types.Sandbox) error { addrs, ok := m.tunAddrs.(tunReconciler) diff --git a/sandboxd/store/dir/dir.go b/sandboxd/store/dir/dir.go index 14f9f7f07..0a7e85ce9 100644 --- a/sandboxd/store/dir/dir.go +++ b/sandboxd/store/dir/dir.go @@ -149,21 +149,13 @@ func (d *Store) SetLabels(_ context.Context, id string, labels []byte) error { func (d *Store) Delete(_ context.Context, id string) error { final := filepath.Join(d.root, id) - entries, err := os.ReadDir(final) + err := utils.RemoveDirEntries(final, func(name string) bool { return name != store.MetaFile }) if errors.Is(err, fs.ErrNotExist) { return nil } if err != nil { return err } - for _, e := range entries { - if e.Name() == store.MetaFile { - continue - } - if err := os.RemoveAll(filepath.Join(final, e.Name())); err != nil { - return err - } - } return os.RemoveAll(final) } diff --git a/sandboxd/tunnet/packet.go b/sandboxd/tunnet/packet.go index edb71fbc6..09f44f53e 100644 --- a/sandboxd/tunnet/packet.go +++ b/sandboxd/tunnet/packet.go @@ -87,15 +87,6 @@ func ReadHello(r io.Reader) (Hello, error) { }, nil } -// FrameLen validates a frame's length prefix. -func FrameLen(head [4]byte) (int, error) { - n := int(binary.LittleEndian.Uint32(head[:])) - if n < VnetHdrLen || n > MaxFrame { - return 0, fmt.Errorf("tun frame length %d", n) - } - return n, nil -} - // Packet is the IPv4 header view of one frame body: a virtio-net header, then the packet. type Packet struct { Proto uint8 @@ -165,6 +156,15 @@ func RewriteDst(frame []byte, to netip.Addr) { rewrite(frame, dstAt, to.As4()) } +// FrameLen validates a frame's length prefix. +func FrameLen(head [4]byte) (int, error) { + n := int(binary.LittleEndian.Uint32(head[:])) + if n < VnetHdrLen || n > MaxFrame { + return 0, fmt.Errorf("tun frame length %d", n) + } + return n, nil +} + // wireCost is what a parsed frame puts on the wire once segmented: its packet count and IP bytes; false when a packet would exceed mtu. func wireCost(frame []byte, mtu int) (segs, bytes int, ok bool) { ip := frame[VnetHdrLen:] diff --git a/sdk/go/info.go b/sdk/go/info.go index adf0b7df1..7c7e7c104 100644 --- a/sdk/go/info.go +++ b/sdk/go/info.go @@ -96,7 +96,7 @@ type sandboxListResponse struct { Sandboxes []SandboxSummary `json:"sandboxes"` } -// Info reports the entry node's pools, claims, capacity state, and mesh peers. Requires the operator token. +// Info reports the entry node's pools, claims, capacity state, and mesh peers. Requires the root or operator token. func (c *Client) Info(ctx context.Context) (*NodeInfo, error) { return c.doJSONPtr[NodeInfo](ctx, http.MethodGet, c.addr, "/v1/info", nil, c.apiToken, "info") } diff --git a/sdk/go/keepalive.go b/sdk/go/keepalive.go index 3ed3d2b4f..2f21e3f2f 100644 --- a/sdk/go/keepalive.go +++ b/sdk/go/keepalive.go @@ -1,7 +1,6 @@ package sandbox import ( - "errors" "slices" "sync" "sync/atomic" @@ -120,7 +119,7 @@ type lease struct { // done ends the RPC: a terminal frame, error frames included, parks the connection; any other failure drops it. func (l *lease) done(err error) error { - if _, frame := errors.AsType[*wire.ErrorResp](err); err == nil || frame { + if _, frame := err.(*wire.ErrorResp); err == nil || frame { l.reuse() } else { l.close() diff --git a/sdk/go/keepalive_test.go b/sdk/go/keepalive_test.go index 2770f160b..e0aba6c46 100644 --- a/sdk/go/keepalive_test.go +++ b/sdk/go/keepalive_test.go @@ -10,6 +10,7 @@ import ( "testing" "time" + "github.com/cocoonstack/sandbox/protocol/wire" "github.com/cocoonstack/sandbox/sdk/go/silkd/silkdtest" ) @@ -46,6 +47,24 @@ func TestRefusedStreamOpenKeepsItsConnection(t *testing.T) { } } +func TestAWriterFailingWithAnErrorFrameDropsTheStream(t *testing.T) { + sb := testSandbox(t, newAgentServer(t, silkdtest.NewFake(t.TempDir()).ServeConn)) + if err := sb.WriteFile(t.Context(), "/f", make([]byte, 3*wire.BulkChunk), nil); err != nil { + t.Fatalf("write: %v", err) + } + refused := &wire.ErrorResp{Kind: "io", Message: "push refused"} + err := sb.ReadFileTo(t.Context(), "/f", failingWriter{refused}) + if !errors.Is(err, refused) { + t.Fatalf("read into a failing writer: %v, want the writer's error", err) + } + sb.pool.mu.Lock() + parked := len(sb.pool.idle) + sb.pool.mu.Unlock() + if parked != 0 { + t.Errorf("%d connections parked mid-stream, want the caller's failure to drop it", parked) + } +} + func TestCanceledCallSendsNothingOnAParkedConnection(t *testing.T) { sb := testSandbox(t, newAgentServer(t, silkdtest.NewFake(t.TempDir()).ServeConn)) if _, err := sb.Stat(t.Context(), "/"); err != nil { @@ -184,6 +203,10 @@ func waitUntil(t *testing.T, cond func() bool, msg string) { t.Fatal(msg) } +type failingWriter struct{ err error } + +func (w failingWriter) Write([]byte) (int, error) { return 0, w.err } + type hangUpAfterReply struct { net.Conn replied atomic.Bool diff --git a/sdk/go/tenants.go b/sdk/go/tenants.go index eedf7c662..47bb52c9d 100644 --- a/sdk/go/tenants.go +++ b/sdk/go/tenants.go @@ -53,7 +53,7 @@ type tenantUpdate struct { EgressClass string `json:"egress_class,omitempty"` } -// Tenants lists up to limit tenants named after the cursor (GET /v1/tenants); a zero limit takes the node's default page, and it requires the operator token. +// Tenants lists up to limit tenants named after the cursor (GET /v1/tenants); a zero limit takes the node's default page, and it requires the root or operator token. func (c *Client) Tenants(ctx context.Context, after string, limit int) (*TenantList, error) { q := url.Values{} if after != "" { diff --git a/sdk/openai/cocoonsandbox_openai/adapter.py b/sdk/openai/cocoonsandbox_openai/adapter.py index ca0ff7bcd..048666d17 100644 --- a/sdk/openai/cocoonsandbox_openai/adapter.py +++ b/sdk/openai/cocoonsandbox_openai/adapter.py @@ -51,6 +51,7 @@ class CocoonSandboxSession(BaseSandboxSession): def __init__(self, *, state: CocoonSandboxSessionState) -> None: self.state = state self._proxies: list[socket.socket] = [] + self._client: Client | None = None self._sb: Sandbox | None = None @classmethod @@ -121,8 +122,8 @@ async def _shutdown_backend(self) -> None: def _sandbox(self) -> Sandbox: if self._sb is None: s = self.state - client = Client(s.addr, api_token=s.api_token) - self._sb = client.attach(s.owner or s.addr, s.sandbox_id, s.sandbox_token) + self._client = Client(s.addr, api_token=s.api_token) + self._sb = self._client.attach(s.owner or s.addr, s.sandbox_id, s.sandbox_token) return self._sb def _abs(self, path: Path | str) -> Path: @@ -163,7 +164,7 @@ async def create( await asyncio.to_thread(sb.close) raise session = CocoonSandboxSession.from_state(state) - session._sb = sb + session._client, session._sb = client, sb return self._wrap_session(session) async def delete(self, session: SandboxSession) -> SandboxSession: @@ -172,6 +173,8 @@ async def delete(self, session: SandboxSession) -> SandboxSession: raise TypeError("CocoonSandboxClient.delete expects a CocoonSandboxSession") await inner._shutdown_backend() await asyncio.to_thread(inner._sandbox().close) + if inner._client is not None: + inner._client.close() return session async def resume(self, state: SandboxSessionState) -> SandboxSession: diff --git a/sdk/openai/tests/test_adapter.py b/sdk/openai/tests/test_adapter.py index 62d96a03a..eda52dd9f 100644 --- a/sdk/openai/tests/test_adapter.py +++ b/sdk/openai/tests/test_adapter.py @@ -90,6 +90,22 @@ async def go(): asyncio.run(go()) +def test_delete_closes_the_session_client(node, monkeypatch): + closed = [] + monkeypatch.setattr("cocoonsandbox.Client.close", lambda self: closed.append(self)) + + async def go(): + client = CocoonSandboxClient() + created = await client.create(options=CocoonSandboxClientOptions(addr=node)) + await client.delete(created) + resumed = await client.resume(created._inner.state) + resumed._inner._sandbox() + await client.delete(resumed) + + asyncio.run(go()) + assert len(closed) == 2 and closed[0] is not closed[1], closed + + def test_exec_maps_stdio_and_exit(node, monkeypatch): class FakeSandbox: def run(self, argv, on_stdout=None, on_stderr=None, timeout=None): diff --git a/sdk/python/cocoonsandbox/template.py b/sdk/python/cocoonsandbox/template.py index 983cff3ee..5eb6432bf 100644 --- a/sdk/python/cocoonsandbox/template.py +++ b/sdk/python/cocoonsandbox/template.py @@ -33,11 +33,9 @@ def new( """Claims the template, following placement when volumes require it.""" claim = _claim_body(self.name, self.net, self.size, ttl_seconds, volumes) claim["require_promoted"] = True - if volumes: - return self._client._claim_from(self._addr, claim, deadline=deadline) - claim["no_redirect"] = True - reply = self._client._request(self._addr, "POST", "/v1/claim", claim, "claim", deadline=deadline) - return self._client._handle_from(self._addr, reply) + if not volumes: + claim["no_redirect"] = True + return self._client._claim_from(self._addr, claim, deadline=deadline) def delete(self) -> None: query = _template_query(self.name, self.net, self.size) diff --git a/silkd/src/pty.rs b/silkd/src/pty.rs index 1692854b4..c9184bb07 100644 --- a/silkd/src/pty.rs +++ b/silkd/src/pty.rs @@ -44,8 +44,14 @@ pub async fn open( cmd.env_clear().envs(sysutil::base_env().iter().copied()); sysutil::apply_claim_env(&mut cmd); cmd.envs(&req.env); - if let Err(e) = sysutil::apply_cwd_user(&mut cmd, req.cwd.as_deref(), req.user.as_deref()) { - return proto::error_frame(out, ErrorKind::BadRequest, e).await; + let uid = match sysutil::apply_cwd_user(&mut cmd, req.cwd.as_deref(), req.user.as_deref()) { + Ok(uid) => uid, + Err(e) => return proto::error_frame(out, ErrorKind::BadRequest, e).await, + }; + if let Some(uid) = uid + && let Err(e) = std::os::unix::fs::fchown(&slave, Some(uid), None) + { + return proto::err_frame(out, &e, "chown pty slave").await; } match (slave.try_clone(), slave.try_clone()) { (Ok(in_fd), Ok(out_fd)) => { diff --git a/silkd/src/sysutil.rs b/silkd/src/sysutil.rs index a8e4e0c39..bd801ef55 100644 --- a/silkd/src/sysutil.rs +++ b/silkd/src/sysutil.rs @@ -98,23 +98,23 @@ pub fn align_proxy_env(cmd: &mut Command) { align_proxy_env_for(cmd, crate::net::routes_directly()); } -/// Sets the command's cwd and, with a user, de-escalates onto it with the user's groups before entering cwd. +/// Sets the command's cwd and, with a user, de-escalates onto it with the user's groups before entering cwd; returns that user's uid. pub fn apply_cwd_user( cmd: &mut Command, cwd: Option<&str>, user: Option<&str>, -) -> Result<(), String> { +) -> Result, String> { let Some(user) = user else { if let Some(cwd) = cwd { cmd.current_dir(cwd); } - return Ok(()); + return Ok(None); }; let (uid, gid, home) = lookup_user(user)?; de_escalate(cmd, user, uid, gid, cwd)?; cmd.env("HOME", home); cmd.env("USER", user); - Ok(()) + Ok(Some(uid)) } pub fn apply_owner(cmd: &mut Command, path: &str) { diff --git a/silkd/tests/pty_e2e.rs b/silkd/tests/pty_e2e.rs index 92f3d0155..586c1fe84 100644 --- a/silkd/tests/pty_e2e.rs +++ b/silkd/tests/pty_e2e.rs @@ -254,3 +254,33 @@ async fn pty_started_write_failure_kills_the_shell_and_unregisters_it() { ); wait_gone(pid).await; } + +#[tokio::test] +#[ignore = "needs root"] +async fn a_user_pty_shell_owns_its_tty() { + let state = Arc::new(State::new()); + let (mut cw, mut lines, handle) = connect_with( + &state, + json!({"op":"pty_open","cols":80,"rows":24,"user":"nobody"}), + ) + .await; + read_until(&mut lines, |v| v["type"] == "started").await; + send( + &mut cw, + json!({"op":"stdin","data":b64(b"echo silk-owner-$(stat -c %U \"$(tty)\")-end\n")}), + ) + .await; + let mut out = String::new(); + while !["nobody", "root"] + .iter() + .any(|u| out.contains(&format!("silk-owner-{u}-end"))) + { + let frame = read_until(&mut lines, |v| v["type"] == "stdout").await; + out.push_str(&String::from_utf8_lossy(&decode(&frame))); + } + assert!(out.contains("silk-owner-nobody-end"), "{out}"); + send(&mut cw, json!({"op":"stdin","data":b64(b"exit\n")})).await; + read_until(&mut lines, |v| v["type"] == "exit").await; + cw.shutdown().await.unwrap(); + handle.await.unwrap().unwrap(); +}