diff --git a/README.md b/README.md index 31cf39d0..3946485e 100644 --- a/README.md +++ b/README.md @@ -72,7 +72,7 @@ performance) — source in `desktop`, and `android` - `scripts/` — `boot-bench.sh` (boot phase timing), `bench.sh` (the published benchmark procedure), `sandboxd-e2e.sh` (bare-metal e2e, below), plus the - `archive`/`egress`/`intercept` e2e drivers + `archive`/`egress`/`intercept`/`socks` e2e drivers ## Build & test diff --git a/docs/deploy.md b/docs/deploy.md index f3817d05..2ef508b8 100644 --- a/docs/deploy.md +++ b/docs/deploy.md @@ -45,6 +45,18 @@ volume claim has finalized, do not roll a node back to an older sandboxd until all volume claims are gone; the older daemon cannot preserve the volume claims' admission holds and marker bookkeeping recorded in `claims.json`. +The guest images and sandboxd move together on the egress lane: sandboxd tells +a NIC-bearing guest how its traffic leaves through `/etc/silkd-lane`, and a +guest image from another release reads a different file or none, which on a +locked bridge lane leaves its execs routing into the lock. Roll the images +with the daemon. A config that uses the `socks5` or `ports` policy keys does +not load on an older sandboxd (unknown keys are rejected), so a rollback takes +the config with it. The SOCKS5 door is opt-in and the pool policy owns it: a +pool that relied on the door binding for any bare host rule adds +`"socks5": true` to its policy or loses the door silently, since its config +still loads; a tenant policy's flag counts only on a claim outside any +configured pool, where it is the whole policy. + ## Configuration sandboxd reads one JSON file (`-config`, default diff --git a/docs/desktop.md b/docs/desktop.md index 6dd0ee46..1918020b 100644 --- a/docs/desktop.md +++ b/docs/desktop.md @@ -24,8 +24,7 @@ up a few seconds later — poll `GET /screenshot` until it returns 200. with an [egress policy](egress.md) when tasks visit the OSWorld mocked websites or the real web; the session's browsers reach the web through the relay. Thunderbird's mail policy is pinned to the SOCKS5 door, so a pool that - runs mail tasks sets `"socks5": true` in its policy, and so does the tenant - policy when the claim carries one. `net=egress` on a + runs mail tasks sets `"socks5": true` in its policy. `net=egress` on a guarded bridge works the same way: the session's proxy setup waits for the host's lane verdict in `/etc/silkd-lane` and follows it, so the locked NIC costs the desktop nothing (a CNI-backed lane is told `direct` and routes). @@ -79,8 +78,13 @@ Configure it with `SANDBOXD_ADDR`, `SANDBOXD_TOKEN`, `COCOON_TEMPLATE` needs a browser the tasks do not pkill and rebind. It takes the caller's profile and debugging port. - `guest-proxy.service` points the session's browsers and `osworld-server`'s - commands at silkd's loopback relay on a guest with no NIC, and on a guest - whose NIC the host reports as `relay` in `/etc/silkd-lane`; it waits up to - 30 s for that verdict before the session starts. A NIC the host calls - `direct`, or a host that writes nothing, leaves the unit idle. + commands at silkd's loopback relay on a guest with no device-backed NIC, and + on a guest whose NIC the host reports as `relay` in `/etc/silkd-lane`; on a + NIC-bearing guest it holds the session until that verdict lands, up to 300 s, + longer than the host's own probe and write timeouts, so a verdict that never + comes means a host that already gave the boot up or an old sandboxd that + writes none: the unit then fails visibly and the session routes directly. A + verdict that changes under a running desktop (a node moved from CNI networks + to a bridge after the claim) reaches silkd's execs at once but the session + only through a re-claim. - x86_64 only. diff --git a/docs/egress.md b/docs/egress.md index 881c633d..5c7cd81c 100644 --- a/docs/egress.md +++ b/docs/egress.md @@ -48,14 +48,16 @@ through the same code: a rule with `methods` that does not name `CONNECT` denies it, and a rule with a `secret` opens it without injecting anything, on both ports. The one difference is `intercept`: on 3128 such a rule terminates the TLS and filters the requests inside, on 1080 there is no HTTP to filter, so -the tunnel is refused. The door is opt-in: the policy sets `"socks5": true` -and some rule of it can admit a tunnel, otherwise the host side stays unwired -and the guest's dial is refused like the HTTP one with no policy. Both sides of -a pool/tenant pair must opt in, each checked on its own, so disjoint host sets -still bind a listener that denies everything. A policy that opts in with rules -that all carry `methods` without `CONNECT`, or all `intercept`, is rejected at -load. A policy that does not opt in pays nothing for the door on the claim -path. Audit lines carry `"method":"SOCKS5"`. +the tunnel is refused. The door is opt-in: the pool policy sets +`"socks5": true`, otherwise the host side stays unwired and the guest's dial +is refused like the HTTP one with no policy. The tenant policy's rules gate +every tunnel through the door as they gate `CONNECT`, so the tenant does not +opt in separately; its own `socks5` counts only on a claim outside any +configured pool, where the tenant policy is the whole policy. A policy that +opts in with rules that all carry `methods` without `CONNECT`, or all +`intercept`, is rejected at load. A pool whose policy does not opt in pays +nothing for the door on the claim path, whatever its tenants' policies say. +Audit lines carry `"method":"SOCKS5"`. ```sh curl --socks5-hostname 127.0.0.1:1080 imaps://imap.example.com/ # allowed: {"socks5": true, "allow": [{"host": "imap.example.com"}]} @@ -163,7 +165,7 @@ file. "egress": { "allow": [{ "host": "api.github.com", "secret": "gh" }] } } ], "tenants": [ - { "name": "acme", "token": "…", "egress": { "socks5": true, "allow": [{ "host": "api.github.com" }] } } + { "name": "acme", "token": "…", "egress": { "allow": [{ "host": "api.github.com" }] } } ] } ``` diff --git a/docs/silkd.md b/docs/silkd.md index 9e3ae81d..4ea54b92 100644 --- a/docs/silkd.md +++ b/docs/silkd.md @@ -66,8 +66,10 @@ lane. `SILKD_NET=none|egress` overrides the probe for tests and operators. On every lane silkd also binds `127.0.0.1:3128` and relays each connection to the host's [guarded-egress](egress.md) proxy over vsock (`CID2:2049`), and `127.0.0.1:1080` to its SOCKS5 door (`CID2:2050`) for clients whose protocol -has no HTTP-proxy form; when the host wired no policy the per-connection dial -is refused, so the ports are inert. +has no HTTP-proxy form; when the host wired no policy, or the door-owning +policy (the pool's, or the tenant's on a claim outside any pool) did not opt +into the SOCKS5 door, the per-connection dial is refused, so the ports are +inert. Where nothing routes directly — the no-network lane, or a NIC whose host verdict in `/etc/silkd-lane` reads `relay` because it is nft-locked — silkd forwards the image-baked proxy variables (`http_proxy` and friends) into every diff --git a/e2e/cmd/desktopsmoke/main.go b/e2e/cmd/desktopsmoke/main.go index a021376b..766e6359 100644 --- a/e2e/cmd/desktopsmoke/main.go +++ b/e2e/cmd/desktopsmoke/main.go @@ -145,19 +145,19 @@ func clickAndReadCursor(ctx context.Context, sb *sandbox.Sandbox) error { // proxyReachesSession proves the session's environment carries the relay and the relay reaches an allowed origin. func proxyReachesSession(ctx context.Context, sb *sandbox.Sandbox, probe string) error { - script := `test -n "$http_proxy" && curl -sS -m 20 -o /dev/null -w '%{http_code}' ` + probe - out, err := execute(ctx, sb, []string{"sh", "-c", script}, 30) + script := `systemctl is-active guest-proxy.service >&2 || exit 1; [ -n "$http_proxy" ] || { echo http_proxy unset in the session >&2; exit 1; }; exec curl -sS -m 20 -o /dev/null -w '%{http_code}' "$1"` + out, err := execute(ctx, sb, []string{"sh", "-c", script, "sh", probe}, 30) if err != nil { return fmt.Errorf("proxy leg: %w", err) } if strings.TrimSpace(out) != "200" { return fmt.Errorf("proxy leg: fetch through the session's proxy returned %q, want 200", strings.TrimSpace(out)) } - fmt.Println(" proxy: the session's environment carries the relay and an allowed origin answers 200") + fmt.Println(" proxy: guest-proxy.service active, the session's environment carries the relay, an allowed origin answers 200") return nil } -// execute runs one command through osworld-server and returns its output; a non-zero exit is an error. +// execute runs one command through osworld-server and returns its output. func execute(ctx context.Context, sb *sandbox.Sandbox, command []string, timeout int) (string, error) { req, _ := json.Marshal(map[string]any{"command": command, "shell": false, "timeout": timeout}) body, err := harness.HTTPOverPort(ctx, sb, serverPort, "POST", "/execute", req) diff --git a/e2e/cmd/sockssmoke/main.go b/e2e/cmd/sockssmoke/main.go index 11a006a6..3a1c5d63 100644 --- a/e2e/cmd/sockssmoke/main.go +++ b/e2e/cmd/sockssmoke/main.go @@ -23,7 +23,8 @@ const ( socksProxy = "127.0.0.1:1080" httpProxy = "http://127.0.0.1:3128" - curlLoginDenied = "exit=67" + curlLoginDenied = "exit=67" + curlProxyRefused = "exit=97" ) func main() { @@ -78,8 +79,10 @@ func run(addr, token, template, echo, getOnly, imap string) error { return fmt.Errorf("IMAPS over SOCKS5 did not reach the login step: %q", strings.TrimSpace(out)) } fmt.Println(" IMAPS handshake rides the tunnel; the server refused the probe login") - if code := curl(ctx, sb, "--socks5-hostname", socksProxy, "imap://"+imap+":143/"); code != "000" { - return fmt.Errorf("IMAP on the unlisted port 143 returned %q, want a refused tunnel", code) + out, _ = sb.Exec(ctx, "sh", "-c", fmt.Sprintf( + "curl -sS -m 30 --socks5-hostname %s imap://%s:143/ 2>&1; echo exit=$?", socksProxy, imap)) + if !strings.Contains(out, curlProxyRefused) { + return fmt.Errorf("IMAP on the unlisted port 143 did not fail at the SOCKS5 handshake: %q", strings.TrimSpace(out)) } fmt.Println(" IMAP on the unlisted port refused on 1080") } diff --git a/os-image/desktop/24.04/Dockerfile b/os-image/desktop/24.04/Dockerfile index db8f3960..1eb9820a 100644 --- a/os-image/desktop/24.04/Dockerfile +++ b/os-image/desktop/24.04/Dockerfile @@ -151,19 +151,19 @@ RUN apt-get update && apt-get install -y --no-install-recommends \ 'host='"'"'127.0.0.1'"'"'' \ 'port=1080' \ > /usr/local/share/guest-proxy.dconf \ - # a NIC alone says nothing about routing: the host writes /etc/silkd-lane once silkd answers, so a NIC-bearing guest waits for the verdict + # a NIC alone says nothing about routing: the host writes /etc/silkd-lane once silkd answers; the wait outlives the host's + # probe (90 s) and write (120 s) timeouts, so a guest still waiting belongs to a boot the host already gave up on && printf '%s\n' \ '#!/bin/sh' \ 'set -e' \ ': > /run/guest-proxy.env' \ 'nic=0' \ - 'for n in /sys/class/net/*; do case ${n##*/} in lo|sit0) ;; *) nic=1;; esac; done' \ + 'for n in /sys/class/net/*; do if [ -e "$n/device" ]; then nic=1; fi; done' \ 'if [ $nic = 1 ]; then' \ ' i=0' \ - ' while [ ! -e /etc/silkd-lane ] && [ $i -lt 300 ]; do sleep 0.1; i=$((i+1)); done' \ - ' if [ ! -e /etc/silkd-lane ]; then echo "guest-proxy: no lane verdict from the host; routing directly" >&2; exit 0; fi' \ - ' read -r lane < /etc/silkd-lane' \ - ' [ "$lane" = relay ] || exit 0' \ + ' while [ ! -e /etc/silkd-lane ] && [ $i -lt 3000 ]; do sleep 0.1; i=$((i+1)); done' \ + ' if [ ! -e /etc/silkd-lane ]; then echo "guest-proxy: no lane verdict from the host after 300 s; routing directly" >&2; exit 1; fi' \ + ' [ "$(cat /etc/silkd-lane)" = relay ] || exit 0' \ 'fi' \ 'cat /usr/local/share/guest-proxy.env > /run/guest-proxy.env' \ 'cat /usr/local/share/guest-proxy.dconf > /etc/dconf/db/local.d/01-proxy' \ diff --git a/os-image/desktop/README.md b/os-image/desktop/README.md index 4dfe522f..5d81d931 100644 --- a/os-image/desktop/README.md +++ b/os-image/desktop/README.md @@ -6,13 +6,15 @@ plus a GNOME session (Ubuntu session, dock, Yaru) on an Xvfb `:1` display at ([xlang-ai/osworld-server](https://github.com/xlang-ai/osworld-server), pinned commit) on guest loopback `5000`, the OSWorld Chrome CDP bridge on loopback `9222`, and the OSWorld app set: Google Chrome, LibreOffice, GIMP, -VLC. The intended pool shape is `size: 2xlarge` (8 CPU / 16G); the session -idles at ~0.5 GB anonymous memory with gnome-shell around 290 MB RSS. +VLC, Thunderbird, VS Code, Zotero, Obsidian, Shotcut, FreeCAD and WPS Office. +The intended pool shape is `size: 2xlarge` (8 CPU / 16G); the session idles +at ~0.5 GB anonymous memory with gnome-shell around 290 MB RSS. An OSWorld harness or the Sai driver claims a sandbox and reaches the guest server through the silkd port relay (`DialPort`/`ProxyPort` 5000, 9222), so the `none` lane works for local tasks; tasks that visit the OSWorld mocked -websites need the `egress` lane. +websites or the web add an egress policy, on the `none` lane or a guarded +bridge `egress` lane alike (see `docs/desktop.md`). ## Guest contract @@ -27,6 +29,12 @@ websites need the `egress` lane. `cdp-bridge` unit forwards loopback `9222` to Chrome's `1337`. - dconf defaults turn on `toolkit-accessibility`, and turn off animations, idle, screen lock, and the welcome dialog. +- `guest-proxy.service` writes the session's proxy environment, the dconf + proxy entry and the apt proxy when the guest has no device-backed NIC, or + when the host's verdict in `/etc/silkd-lane` reads `relay`; on a NIC-bearing + guest it holds the session for that verdict up to 300 s and fails visibly + without it. Thunderbird's policy pins mail to the SOCKS5 door on `1080`, so + a pool that runs mail tasks opts its policy into `socks5`. ## Build diff --git a/sandboxd/egress/intercept.go b/sandboxd/egress/intercept.go index 69da19ea..707368d2 100644 --- a/sandboxd/egress/intercept.go +++ b/sandboxd/egress/intercept.go @@ -5,6 +5,7 @@ import ( "io" "net" "net/http" + "strconv" "strings" "sync" "sync/atomic" @@ -53,7 +54,7 @@ func (p *Proxy) serveIntercept(w http.ResponseWriter, r *http.Request, host stri _ = client.SetDeadline(time.Time{}) ln := &singleConnListener{conn: tlsConn, done: make(chan struct{})} srv := &http.Server{ - Handler: &interceptHandler{proxy: p, host: host, port: port, authority: r.Host}, + Handler: &interceptHandler{proxy: p, host: host, port: port, authority: net.JoinHostPort(host, strconv.Itoa(int(port)))}, ReadHeaderTimeout: interceptTimeout, } _ = srv.Serve(ln) @@ -93,7 +94,7 @@ func (h *interceptHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { out.URL.Scheme = "https" out.URL.Host = h.authority // keep the guest's Host when it names the CONNECT host: SigV4 signing breaks on a rewrite. - if !strings.EqualFold(hostOnly(r.Host), h.host) { + if inner, _, _ := hostPort(r.Host, 0); !strings.EqualFold(inner, h.host) { out.Host = h.authority } }) diff --git a/sandboxd/egress/policy.go b/sandboxd/egress/policy.go index c6360fcc..edb73fc1 100644 --- a/sandboxd/egress/policy.go +++ b/sandboxd/egress/policy.go @@ -64,7 +64,7 @@ func (p *Policy) Intercepts() bool { return p != nil && slices.ContainsFunc(p.Allow, func(r Rule) bool { return r.Intercept }) } -// Validate rejects an empty or bare-wildcard host, a zero or repeated port, and socks5 without a tunnel rule; an empty allow-list denies everything. +// Validate rejects a policy the proxy could never honor; an empty allow-list is valid and denies everything. func (p Policy) Validate() error { for i, r := range p.Allow { switch r.Host { @@ -131,9 +131,9 @@ func (p Policy) EvalInner(host, method string, port uint16) (Rule, Decision) { return Rule{}, DecisionDeny } -// ServesSocks reports whether the SOCKS5 door is bound: opted in, with some rule that can admit a tunnel. +// ServesSocks reports whether the policy opted into the SOCKS5 door; Validate holds the rule that makes the door usable. func (p Policy) ServesSocks() bool { - return p.Socks5 && p.admitsTunnel() + return p.Socks5 } func (p Policy) admitsTunnel() bool { @@ -190,7 +190,7 @@ func (c composite) EvalInner(host, method string, port uint16) (Rule, Decision) return rule, DecisionAllow } -// ServesSocks is conservative: two host sets that never meet still bind a door that denies everything. +// ServesSocks is the pool's call: the tenant's rules already gate every tunnel through Eval. func (c composite) ServesSocks() bool { - return c.pool.ServesSocks() && c.tenant.ServesSocks() + return c.pool.ServesSocks() } diff --git a/sandboxd/egress/policy_test.go b/sandboxd/egress/policy_test.go index fe5363e5..9d6835db 100644 --- a/sandboxd/egress/policy_test.go +++ b/sandboxd/egress/policy_test.go @@ -98,10 +98,10 @@ func TestCompositeEvalInnerIntersectsTenant(t *testing.T) { func TestWildcardApexIsNotMatched(t *testing.T) { policy := Policy{Allow: []Rule{{Host: "*.example.com"}}} if _, d := policy.Eval("example.com", "GET", 443); d != DecisionDeny { - t.Errorf("apex example.com matched *.example.com, want deny") + t.Error("apex example.com matched *.example.com, want deny") } if _, d := policy.Eval("a.example.com", "GET", 443); d != DecisionAllow { - t.Errorf("a.example.com did not match *.example.com, want allow") + t.Error("a.example.com did not match *.example.com, want allow") } } @@ -131,37 +131,16 @@ func TestPolicyValidate(t *testing.T) { } func TestPolicyServesSocks(t *testing.T) { - tests := []struct { - name string - policy Policy - want bool - }{ - {"opted in, no rules", Policy{Socks5: true}, false}, - {"opted in, bare host", Policy{Socks5: true, Allow: []Rule{{Host: "a.internal"}}}, true}, - {"bare host without opting in", Policy{Allow: []Rule{{Host: "a.internal"}}}, false}, - {"opted in, methods without CONNECT", Policy{Socks5: true, Allow: []Rule{{Host: "a.internal", Methods: []string{"GET"}}}}, false}, - {"opted in, methods with CONNECT", Policy{Socks5: true, Allow: []Rule{{Host: "a.internal", Methods: []string{"connect"}}}}, true}, - {"opted in, intercept only", Policy{Socks5: true, Allow: []Rule{{Host: "a.internal", Intercept: true}}}, false}, - {"opted in, secret", Policy{Socks5: true, Allow: []Rule{{Host: "a.internal", Secret: "s"}}}, true}, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if got := tt.policy.ServesSocks(); got != tt.want { - t.Errorf("ServesSocks() = %v, want %v", got, tt.want) - } - }) - } - bare := Policy{Socks5: true, Allow: []Rule{{Host: "a.internal"}}} - getOnly := Policy{Socks5: true, Allow: []Rule{{Host: "a.internal", Methods: []string{"GET"}}}} + opted := Policy{Socks5: true, Allow: []Rule{{Host: "a.internal"}}} silent := Policy{Allow: []Rule{{Host: "a.internal"}}} - if Compose(bare, getOnly).ServesSocks() { - t.Error("composite ServesSocks() = true with a GET-only tenant, want false") + if !opted.ServesSocks() || silent.ServesSocks() { + t.Errorf("ServesSocks() opted=%v silent=%v, want true/false", opted.ServesSocks(), silent.ServesSocks()) } - if Compose(bare, silent).ServesSocks() { - t.Error("composite ServesSocks() = true with a tenant that did not opt in, want false") + if !Compose(opted, silent).ServesSocks() { + t.Error("composite ServesSocks() = false with an opted-in pool and a silent tenant; the pool owns the door") } - if !Compose(bare, bare).ServesSocks() { - t.Error("composite ServesSocks() = false with two opted-in bare policies, want true") + if Compose(silent, opted).ServesSocks() { + t.Error("composite ServesSocks() = true with a silent pool; a tenant cannot open the door alone") } } diff --git a/sandboxd/egress/proxy.go b/sandboxd/egress/proxy.go index 9d73514f..e3856b97 100644 --- a/sandboxd/egress/proxy.go +++ b/sandboxd/egress/proxy.go @@ -8,6 +8,7 @@ import ( "maps" "net" "net/http" + "net/netip" "net/textproto" "slices" "strconv" @@ -134,9 +135,13 @@ func (p *Proxy) untrack(conn net.Conn) { p.connMu.Unlock() } -// serveConnect gates an HTTPS/opaque tunnel by host and port. func (p *Proxy) serveConnect(w http.ResponseWriter, r *http.Request) { - host, port := hostPort(r.Host, 443) + host, port, ok := hostPort(r.Host, 443) + if !ok { + p.record(Event{Method: r.Method, Host: host, Decision: DecisionDeny}) + denied(w, host) + return + } decision, intercept := p.tunnelDecision(host, port) if intercept { p.serveIntercept(w, r, host, port) @@ -147,7 +152,7 @@ func (p *Proxy) serveConnect(w http.ResponseWriter, r *http.Request) { denied(w, host) return } - upstream, err := p.dial(r.Context(), "tcp", r.Host) + upstream, err := p.dial(r.Context(), "tcp", net.JoinHostPort(host, strconv.Itoa(int(port)))) if err != nil { http.Error(w, "egress: upstream unreachable", http.StatusBadGateway) return @@ -189,7 +194,12 @@ func (p *Proxy) serveForward(w http.ResponseWriter, r *http.Request) { if r.URL.Scheme == "https" { def = 443 } - host, port := hostPort(r.URL.Host, def) + host, port, ok := hostPort(r.URL.Host, def) + if !ok { + p.record(Event{Method: r.Method, Host: host, Decision: DecisionDeny}) + denied(w, host) + return + } rule, decision := p.policy.Eval(host, r.Method, port) p.relay(w, r, Event{Method: r.Method, Host: host, Port: port, Decision: decision}, rule, p.tr, nil) } @@ -276,22 +286,18 @@ func denied(w http.ResponseWriter, host string) { http.Error(w, fmt.Sprintf("egress denied: %s", host), http.StatusForbidden) } -// hostOnly strips a port from an authority, tolerating a bare host and IPv6 literals. -func hostOnly(authority string) string { - if host, _, err := net.SplitHostPort(authority); err == nil { - return host - } - return authority -} - -// hostPort splits an authority into host and port; a bare host or an unparsable port takes def. -func hostPort(authority string, def uint16) (string, uint16) { - host, portText, err := net.SplitHostPort(authority) +// hostPort splits an authority; a bare host takes def, a port outside 1-65535 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 { - return authority, def - } - if port, err := strconv.ParseUint(portText, 10, 16); err == nil { - return host, uint16(port) + literal, bracketed := strings.CutPrefix(authority, "[") + if !bracketed { + return authority, def, authority != "" && !strings.ContainsAny(authority, ":[]") + } + literal, closed := strings.CutSuffix(literal, "]") + addr, parseErr := netip.ParseAddr(literal) + return literal, def, closed && parseErr == nil && addr.Is6() } - return host, def + n, err := strconv.ParseUint(text, 10, 16) + return host, uint16(n), err == nil && n != 0 } diff --git a/sandboxd/egress/proxy_test.go b/sandboxd/egress/proxy_test.go index 3f720da7..183ce1a3 100644 --- a/sandboxd/egress/proxy_test.go +++ b/sandboxd/egress/proxy_test.go @@ -275,6 +275,108 @@ func TestPortRuleGatesConnectAndForward(t *testing.T) { } } +func TestUnparsablePortIsDeniedNotDefaulted(t *testing.T) { + policy := Policy{Allow: []Rule{{Host: "echo.internal", Ports: []uint16{443}}}} + events := make(chan Event, 4) + p := New("sb_1", "acme", policy, nil, nil, fixedDial(echoServer(t)), func(ev Event) { events <- ev }) + front := httptest.NewServer(p) + defer front.Close() + + conn := dialConnect(t, front.Listener.Addr().String(), "echo.internal:99999") + defer func() { _ = conn.Close() }() + if status := readStatus(t, bufio.NewReader(conn)); status != "HTTP/1.1 403 Forbidden" { + t.Errorf("CONNECT with an out-of-range port = %q, want 403 Forbidden", status) + } + if ev := recvEvent(t, events); ev.Port != 0 || ev.Decision != DecisionDeny { + t.Errorf("audit event = %+v, want deny with no port, not the 443 default", ev) + } + + req, err := http.NewRequestWithContext(t.Context(), http.MethodGet, "http://echo.internal:99999/x", nil) + if err != nil { + t.Fatalf("build request: %v", err) + } + resp, err := proxyClient(t, front.URL).Do(req) + if err != nil { + t.Fatalf("proxied GET: %v", err) + } + _ = resp.Body.Close() + if resp.StatusCode != http.StatusForbidden { + t.Errorf("forward GET with an out-of-range port = %d, want 403", resp.StatusCode) + } + if ev := recvEvent(t, events); ev.Port != 0 || ev.Decision != DecisionDeny { + t.Errorf("audit event = %+v, want deny with no port", ev) + } + + raw, err := net.Dial("tcp", front.Listener.Addr().String()) + if err != nil { + t.Fatalf("dial proxy: %v", err) + } + defer func() { _ = raw.Close() }() + if _, err = io.WriteString(raw, "CONNECT /x HTTP/1.1\r\nHost: echo.internal:+8443\r\n\r\n"); err != nil { + t.Fatalf("write path-form CONNECT: %v", err) + } + if status := readStatus(t, bufio.NewReader(raw)); status != "HTTP/1.1 403 Forbidden" { + t.Errorf("path-form CONNECT with a signed port = %q, want 403 (the dialer would have accepted +8443)", status) + } + if ev := recvEvent(t, events); ev.Port != 0 || ev.Decision != DecisionDeny { + t.Errorf("audit event = %+v, want deny with no port", ev) + } + + malformed, err := net.Dial("tcp", front.Listener.Addr().String()) + if err != nil { + t.Fatalf("dial proxy: %v", err) + } + defer func() { _ = malformed.Close() }() + if _, err = io.WriteString(malformed, "CONNECT /x HTTP/1.1\r\nHost: echo.internal:443:8443\r\n\r\n"); err != nil { + t.Fatalf("write malformed CONNECT: %v", err) + } + if status := readStatus(t, bufio.NewReader(malformed)); status != "HTTP/1.1 403 Forbidden" { + t.Errorf("path-form CONNECT with two ports = %q, want 403, not a bare-host default", status) + } + if ev := recvEvent(t, events); ev.Port != 0 || ev.Decision != DecisionDeny { + t.Errorf("audit event = %+v, want deny with no port", ev) + } +} + +func TestBracketedIPv6AuthorityIsUnwrapped(t *testing.T) { + policy := Policy{Allow: []Rule{{Host: "*"}}} + events := make(chan Event, 4) + p := New("sb_1", "acme", policy, nil, nil, fixedDial(echoServer(t)), func(ev Event) { events <- ev }) + front := httptest.NewServer(p) + defer front.Close() + + for _, tc := range []struct { + authority string + status string + decision Decision + }{ + {"[2606:4700:4700::1111]", "HTTP/1.1 200 Connection Established", DecisionAllow}, + {"[2606:4700:4700::1111]:443", "HTTP/1.1 200 Connection Established", DecisionAllow}, + {"[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}, + {"", "HTTP/1.1 403 Forbidden", DecisionDeny}, + } { + conn, err := net.Dial("tcp", front.Listener.Addr().String()) + if err != nil { + t.Fatalf("dial proxy: %v", err) + } + if _, err = io.WriteString(conn, "CONNECT /x HTTP/1.1\r\nHost: "+tc.authority+"\r\n\r\n"); err != nil { + t.Fatalf("write CONNECT: %v", err) + } + if status := readStatus(t, bufio.NewReader(conn)); status != tc.status { + t.Errorf("CONNECT with Host %s = %q, want %q", tc.authority, status, tc.status) + } + _ = conn.Close() + ev := recvEvent(t, events) + allowed := ev.Decision == DecisionAllow && ev.Host == "2606:4700:4700::1111" && ev.Port == 443 + denied := ev.Decision == DecisionDeny && ev.Port == 0 + if (tc.decision == DecisionAllow) != allowed || (tc.decision == DecisionDeny) != denied { + t.Errorf("audit event for %s = %+v, want %v (allow on the bare address and 443, or deny with no port)", tc.authority, ev, tc.decision) + } + } +} + type fakeSecrets map[string][2]string func (f fakeSecrets) Header(name string) (string, string, bool) { diff --git a/sandboxd/egress/socks.go b/sandboxd/egress/socks.go index af1131ba..07368ed2 100644 --- a/sandboxd/egress/socks.go +++ b/sandboxd/egress/socks.go @@ -59,7 +59,7 @@ func (p *Proxy) serveSocks(ctx context.Context, conn net.Conn) { return } decision, intercept := p.tunnelDecision(host, port) - if intercept { + if intercept || port == 0 { decision = DecisionDeny } p.record(Event{Method: methodSOCKS, Host: host, Port: port, Decision: decision}) diff --git a/sandboxd/egress/socks_test.go b/sandboxd/egress/socks_test.go index 930caffe..b99a75ef 100644 --- a/sandboxd/egress/socks_test.go +++ b/sandboxd/egress/socks_test.go @@ -165,6 +165,19 @@ func TestSocksPortRuleGatesTheTunnel(t *testing.T) { } } +func TestSocksPortZeroIsDeniedEvenByABareRule(t *testing.T) { + events := make(chan Event, 4) + _, addr := socksProxy(t, Policy{Allow: []Rule{{Host: "mail.internal"}}}, nil, fixedDial(echoServer(t)), events) + conn, reply := socksExchange(t, addr, socksGreeting, socksNameRequest("mail.internal", 0)) + defer func() { _ = conn.Close() }() + if reply[1] != socksDenied { + t.Fatalf("port 0 reply = %#x, want denied", reply[1]) + } + if ev := recvEvent(t, events); ev.Decision != DecisionDeny { + t.Errorf("audit event = %+v, want deny", ev) + } +} + func socksProxy(t *testing.T, policy Policy, ca *CA, dial DialFunc, events chan Event) (*Proxy, string) { t.Helper() var audit func(Event) diff --git a/sandboxd/engine/lane.go b/sandboxd/engine/lane.go index d395c322..90ff1729 100644 --- a/sandboxd/engine/lane.go +++ b/sandboxd/engine/lane.go @@ -3,10 +3,10 @@ package engine import "context" const ( - laneFile = "/etc/silkd-lane" - LaneRelay Lane = "relay" LaneDirect Lane = "direct" + + laneFile = "/etc/silkd-lane" ) // Lane is the host's verdict on how a guest's traffic leaves: through the relay when its NIC is nft-locked, directly when the NIC routes. diff --git a/sandboxd/pool/egress.go b/sandboxd/pool/egress.go index 482ee59a..70e2eb43 100644 --- a/sandboxd/pool/egress.go +++ b/sandboxd/pool/egress.go @@ -94,7 +94,6 @@ func (m *Manager) lockEgressNIC(ctx context.Context, sb *types.Sandbox) error { return nil } -// markLane tells the guest how its traffic leaves; the nft lock is invisible from inside. func (m *Manager) markLane(ctx context.Context, key types.PoolKey, sock string) error { lane := m.laneOf(key) if lane == "" { diff --git a/sandboxd/pool/egress_test.go b/sandboxd/pool/egress_test.go index c2f76ad5..f49b701c 100644 --- a/sandboxd/pool/egress_test.go +++ b/sandboxd/pool/egress_test.go @@ -79,7 +79,6 @@ func TestArmEgressBindsSocksOnlyWhenOptedIn(t *testing.T) { }{ {"opted in with a bare host rule", &egress.Policy{Socks5: true, Allow: []egress.Rule{{Host: "example.com"}}}, true}, {"bare host rule without opting in", &egress.Policy{Allow: []egress.Rule{{Host: "example.com"}}}, false}, - {"opted in with a GET-only rule", &egress.Policy{Socks5: true, Allow: []egress.Rule{{Host: "example.com", Methods: []string{"GET"}}}}, false}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -116,6 +115,7 @@ func TestLaneVerdictReachesGuestOnColdBoots(t *testing.T) { {"golden build on an unlocked egress lane", egKey, false, false, []string{"direct"}}, {"golden build on the none lane", testKey, true, false, nil}, {"cold provision on a locked egress lane", egKey, true, true, []string{"relay"}}, + {"cold provision on an unlocked egress lane", egKey, false, true, []string{"direct"}}, {"cold provision on the none lane", testKey, true, true, nil}, } for _, tt := range tests { diff --git a/sandboxd/pool/reconcile.go b/sandboxd/pool/reconcile.go index e338d691..fd44cf4f 100644 --- a/sandboxd/pool/reconcile.go +++ b/sandboxd/pool/reconcile.go @@ -71,17 +71,17 @@ func (m *Manager) Reconcile(ctx context.Context) error { m.mu.Unlock() // a crash mid-export leaves a *.tmp staging dir nothing in this life reuses tmps, _ := filepath.Glob(filepath.Join(m.goldensDir(), "*.tmp")) + logger := log.WithFunc("pool.Reconcile") if err := m.ckpts.SweepStaging(); err != nil { - log.WithFunc("pool.Reconcile").Error(ctx, err, "sweep checkpoint staging") + logger.Error(ctx, err, "sweep checkpoint staging") } if err := m.tpls.SweepStaging(); err != nil { - log.WithFunc("pool.Reconcile").Error(ctx, err, "sweep template staging") + logger.Error(ctx, err, "sweep template staging") } for _, tmp := range tmps { _ = os.RemoveAll(tmp) } - logger := log.WithFunc("pool.Reconcile") m.reclaimOrphanArchiveCks(ctx, claims) m.retryArchiveDeletes(ctx) removed := m.sweepStaleVMs(ctx, live, owned) diff --git a/sandboxd/pool/refill.go b/sandboxd/pool/refill.go index 523a5103..e61b32d8 100644 --- a/sandboxd/pool/refill.go +++ b/sandboxd/pool/refill.go @@ -117,17 +117,17 @@ func (m *Manager) refillOne(ctx context.Context, p *pool, golden string) { parked := now.Before(m.atCapacityUntil) m.mu.Unlock() if err != nil { + logger := log.WithFunc("pool.refillOne") switch { case ctx.Err() != nil: case entered: - log.WithFunc("pool.refillOne").Errorf(ctx, err, - "node is at capacity; parking refill for %s", capacityBackoff) + logger.Errorf(ctx, err, "node is at capacity; parking refill for %s", capacityBackoff) case parked: default: hash := p.hash - log.WithFunc("pool.refillOne").Errorf(ctx, err, "refill %s", hash) + logger.Errorf(ctx, err, "refill %s", hash) if backedOff { - log.WithFunc("pool.refillOne").Warnf(ctx, + logger.Warnf(ctx, "refill %s failed %d times running; pausing refill for %s", hash, fails, wait.Round(time.Millisecond)) } diff --git a/sandboxd/pool/telemetry.go b/sandboxd/pool/telemetry.go index e8f8006e..4ddccd33 100644 --- a/sandboxd/pool/telemetry.go +++ b/sandboxd/pool/telemetry.go @@ -61,7 +61,6 @@ type auditFrame struct { Secret string `json:"secret,omitempty"` //nolint:gosec // the secret's ref name, never its value } -// Counters snapshots the monotonic telemetry counters. func (m *Manager) Counters() Counters { c := &m.counters return Counters{ diff --git a/silkd/src/exec.rs b/silkd/src/exec.rs index 0c06f7c0..5a70a2c4 100644 --- a/silkd/src/exec.rs +++ b/silkd/src/exec.rs @@ -4,7 +4,7 @@ use std::process::Stdio; use std::sync::{Arc, OnceLock}; use std::time::Duration; -use tokio::io::{AsyncWrite, AsyncWriteExt}; +use tokio::io::{AsyncRead, AsyncReadExt, AsyncWrite, AsyncWriteExt}; use tokio::process::Command; use tokio::sync::mpsc; use tokio::time::timeout; @@ -41,7 +41,7 @@ where cmd.current_dir(cwd); } cmd.env_clear(); - cmd.envs(sysutil::base_env()); + cmd.envs(sysutil::base_env().iter().copied()); cmd.envs(&req.env); if let Some(ref user) = req.user && let Err(e) = sysutil::apply_user(&mut cmd, user) @@ -167,9 +167,8 @@ async fn pump_out( async fn copy_stream(mut r: R, proc: &Proc, stderr: bool, fg: Option<&mpsc::Sender>) where - R: tokio::io::AsyncRead + Unpin, + R: AsyncRead + Unpin, { - use tokio::io::AsyncReadExt; let make: fn(Vec) -> Chunk = if stderr { Chunk::Stderr } else { Chunk::Stdout }; let mut buf = [0u8; crate::proto::READ_CHUNK]; loop { diff --git a/silkd/src/net.rs b/silkd/src/net.rs index a813e47e..58ce1edf 100644 --- a/silkd/src/net.rs +++ b/silkd/src/net.rs @@ -3,12 +3,13 @@ use std::fs::File; use std::io::Read; use std::sync::LazyLock; -use std::sync::atomic::{AtomicI8, Ordering}; +use std::sync::atomic::{AtomicBool, AtomicI8, Ordering}; /// File the host writes with its lane verdict, `relay` or `direct`; on the root filesystem so a guest reboot keeps it. const LANE_FILE: &str = "/etc/silkd-lane"; static LANE_OVERRIDE: AtomicI8 = AtomicI8::new(-1); +static LANE_RELAY: AtomicBool = AtomicBool::new(false); /// Reports whether the guest can reach a network; only a `device`-backed interface counts, since the kernel auto-creates virtual tunnels. pub fn has_egress() -> bool { @@ -34,7 +35,18 @@ pub fn has_egress() -> bool { /// Reports whether execs route directly: a NIC the host did not mark as relayed. pub fn routes_directly() -> bool { - has_egress() && !lane_is_relay(LANE_FILE) + if !has_egress() { + return false; + } + // relay latches: the host never unlocks a lane, but a late lock can land after the first exec. + if LANE_RELAY.load(Ordering::Relaxed) { + return false; + } + let relay = lane_is_relay(LANE_FILE); + if relay { + LANE_RELAY.store(true, Ordering::Relaxed); + } + !relay } /// Lane override for tests: set_var would race every concurrent getenv. diff --git a/silkd/src/pty.rs b/silkd/src/pty.rs index f2c406b6..cbab7f0c 100644 --- a/silkd/src/pty.rs +++ b/silkd/src/pty.rs @@ -43,7 +43,9 @@ pub async fn open( if let Some(dir) = &req.cwd { cmd.current_dir(dir); } - cmd.env_clear().envs(sysutil::base_env()).envs(&req.env); + cmd.env_clear() + .envs(sysutil::base_env().iter().copied()) + .envs(&req.env); if let Some(user) = &req.user && let Err(e) = sysutil::apply_user(&mut cmd, user) { diff --git a/silkd/src/session.rs b/silkd/src/session.rs index b2ac70e5..4c296198 100644 --- a/silkd/src/session.rs +++ b/silkd/src/session.rs @@ -50,7 +50,7 @@ impl Table { } let mut child = Command::new("bash") .env_clear() - .envs(sysutil::base_env()) + .envs(sysutil::base_env().iter().copied()) .stdin(Stdio::piped()) .stdout(Stdio::piped()) // null, not piped: the init line `exec 2>&1` repoints fd 2 at the stdout pipe. diff --git a/silkd/src/sysutil.rs b/silkd/src/sysutil.rs index 64b3b053..b0d62a6c 100644 --- a/silkd/src/sysutil.rs +++ b/silkd/src/sysutil.rs @@ -63,8 +63,16 @@ pub fn signal_pid(pid: u32, sig: i32) { } /// The environment every exec starts from; the proxy snapshot rides only where nothing routes directly. -pub fn base_env() -> Vec<(&'static str, &'static str)> { - compose_env(crate::net::routes_directly(), proxy_vars()) +pub fn base_env() -> &'static [(&'static str, &'static str)] { + static DIRECT: LazyLock> = + LazyLock::new(|| compose_env(true, proxy_vars())); + static RELAYED: LazyLock> = + LazyLock::new(|| compose_env(false, proxy_vars())); + if crate::net::routes_directly() { + &DIRECT + } else { + &RELAYED + } } /// Applies base_env's lane rule to an env-inheriting child: a routed NIC drops the proxy variables. diff --git a/silkd/src/watch.rs b/silkd/src/watch.rs index 52429804..a7ae2151 100644 --- a/silkd/src/watch.rs +++ b/silkd/src/watch.rs @@ -95,9 +95,14 @@ fn to_frames(res: notify::Result, out: &mut Vec) { N::Remove(_) => EventKind::Deleted, _ => return, }; - out.extend(event.paths.into_iter().map(|p| Response::Event { - kind, - path: p.to_string_lossy().into_owned(), + out.extend(event.paths.into_iter().map(|p| { + Response::Event { + kind, + path: p + .into_os_string() + .into_string() + .unwrap_or_else(|os| os.to_string_lossy().into_owned()), + } })); }