From d346d1dc6d6565e60eeddc7ef303c59ebc0e6466 Mon Sep 17 00:00:00 2001 From: CMGS Date: Sun, 4 Oct 2026 02:18:24 +0800 Subject: [PATCH 01/11] egress: an "inject" rule intercepts only the claims that inject on the host, and a credential can fill a query or body placeholder (#308) A pool rule with intercept "inject" terminates TLS only for a claim holding a credential for the host and stays a plain rule for every other claim, so a control plane can bind per-user keys to any host the operator allows without breaking HTTP/2, WebSocket or pinned clients for the claims that hold none. Such a rule cannot carry a secret. inject.query swaps a placeholder query value byte-exactly; body: true also covers a form or JSON body of known length up to 64 KiB. A value a response header echoes back reaches the guest as the placeholder. The egress usage event names the injected credentials. --- docs/egress.md | 60 +++++++-- docs/sandboxd-api.md | 10 +- docs/security.md | 3 +- sandboxd/config/config_test.go | 28 +++-- sandboxd/egress/bench_test.go | 33 +++-- sandboxd/egress/intercept_test.go | 203 ++++++++++++++++++++++-------- sandboxd/egress/policy.go | 66 ++++++++-- sandboxd/egress/policy_test.go | 61 +++++++-- sandboxd/egress/proxy.go | 141 ++++++++++++++++++--- sandboxd/egress/proxy_test.go | 2 +- sandboxd/egress/socks_test.go | 2 +- sandboxd/pool/env.go | 4 +- sandboxd/pool/env_test.go | 21 +++- sandboxd/pool/intercept_test.go | 2 +- sandboxd/pool/journal.go | 1 + sandboxd/pool/telemetry.go | 2 +- sandboxd/pool/telemetry_test.go | 18 +++ sandboxd/pool/view.go | 2 +- sandboxd/types/env_test.go | 15 +++ sandboxd/types/types.go | 19 ++- sdk/go/options.go | 6 +- 21 files changed, 554 insertions(+), 145 deletions(-) diff --git a/docs/egress.md b/docs/egress.md index 2e452baf..6cbdab08 100644 --- a/docs/egress.md +++ b/docs/egress.md @@ -242,11 +242,16 @@ woken sandbox binds at arm time. injection needs `intercept`; an absolute-form `https://` request on the forward door needs none — the proxy dials the origin over verified TLS itself, checks the method, and injects. -- `intercept`: terminate a matched HTTPS CONNECT so the request is filtered by - method and the secret injected (see below). Only a pool rule may set it. +- `intercept`: `true` terminates a matched HTTPS CONNECT so the request is + filtered by method and the secret injected (see below). `"inject"` terminates + it only for a claim with a [credential](#claim-credentials) for that host; for + every other claim the rule is a plain one, so its tunnels splice and HTTP/2, + WebSocket and pinned clients keep working. An `"inject"` rule cannot carry a + `secret`. Only a pool rule may set `intercept`. - No policy on a claim ⇒ no egress at all (the proxy is not started). -Each decision is metered as an `egress` usage event and, when `audit_log` is +Each decision is metered as an `egress` usage event, which names the injected +secrets in `secret` as the audit line does, and, when `audit_log` is 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. @@ -317,23 +322,55 @@ request while a browser on the same host gets nothing: git config --global http.https://github.com/.extraHeader "Authorization: Basic @GH_GIT@" ``` +A key that a client sends as a query parameter takes `query` instead of +`header`, and a placeholder is required. On an intercepted request, each +`=` pair of the URL gets the value, URL-escaped; the +placeholder may arrive percent-encoded, and every other byte of the query +stays as sent. With `body: true`, the same parameter of an +`application/x-www-form-urlencoded` body, and every JSON string equal to the +placeholder in an `application/json` body, are replaced too, when the body has a +known length of at most 64 KiB; any other body streams unchanged: + +```jsonc +"FB": { "value": "EAAB…", "guest": false, + "inject": { "hosts": ["graph.facebook.com"], "query": "access_token", "body": true, "placeholder": "@FB@" } } +``` + +```sh +curl "https://graph.facebook.com/me?access_token=@FB@" +curl https://graph.facebook.com/me/feed -d access_token=@FB@ -d message=hi +``` + +Pair the credential with an `"inject"` pool rule, for example +`{ "host": "*", "intercept": "inject" }`, and only the claims that hold a +credential for a host have it intercepted: the operator decides which hosts may +be intercepted, the claim's entries decide which of them are. The decision is +taken per CONNECT, so a write that adds or removes an entry applies to the +guest's next connection; a tunnel the guest keeps open stays as it was. + The placeholder is not a secret: it stays in the guest and in `GET /env`. The entry rides the claim request or any `/env` write, and rotation is the same `PATCH`, accepted on a hibernated or archived claim without waking it. - **Envelope.** The operator still decides which hosts can carry a - credential: every host must be covered by an `intercept: true` rule of the - claim's pool and allowed by its tenant class, or the write answers 400 and + credential: every host must be covered by an `intercept` rule (`true` or + `"inject"`) of the claim's pool and allowed by its tenant class, or the write answers 400 and nothing is stored. A credential never widens egress and never rides a plaintext forward request. - **Precedence.** The pool rule's own `secret` is set first and wins: a credential naming that header for a host the rule covers is a 400. Among credentials for the same header, the first by entry name whose placeholder (if any) the request carries wins. An empty value injects nothing. +- **Echo.** On a request that carried a claim credential, an echo of its value + in a response header (a redirect's `Location` repeating the query, say) + reaches the guest as the placeholder, or blanked when there is none. + Response bodies stream unchanged. - **Shape.** `hosts` holds 1 to 8 lowercase names or `*.suffix` patterns - (no bare `*`, no port); `header` is any header name except `Host`, - `Content-Length` and the hop-by-hop ones; `placeholder`, when set, is at most - 256 bytes of header value without control characters or surrounding space. + (no bare `*`, no port); exactly one of `header` and `query`; `header` is any + header name except `Host`, `Content-Length` and the hop-by-hop ones; `query` + needs a `placeholder`, and `body` needs `query`; `placeholder`, when set, is + at most 256 bytes of header value without control characters or surrounding + space. `GET /env` returns `inject` with the value blanked. - **Reload.** A reload that drops the covering intercept rule stops that injection on the next request and logs which claims lost which hosts. @@ -389,8 +426,8 @@ usage-journal append per connection, which is why it is opt-in. ## HTTPS interception -A rule with `intercept: true` makes the proxy terminate that host's TLS with a -leaf it signs, so it sees the request's method and path and can inject the +A rule with `intercept: true`, or `"inject"` for a claim with a credential for +the host, makes the proxy terminate that host's TLS with a leaf it signs, so it sees the request's method and path and can inject the secret into HTTPS — the same guarantees plaintext already has. Upstream is re-originated and verified against the host's real root store; the proxy never trusts an unverified origin. A guest that caches TLS sessions resumes them on @@ -501,5 +538,6 @@ Swapping the intermediate while old-root guests are alive breaks interception for them, with no re-claim path to fix it. Limitations: interception is HTTP/1.1 only and breaks clients that pin -certificates, so scope it to hosts you control. A host that speaks a non-HTTP +certificates, so scope it to hosts you control, or use `"inject"` so only the +claims that inject on a host are affected. A host that speaks a non-HTTP protocol over TLS must not be given an intercept rule. diff --git a/docs/sandboxd-api.md b/docs/sandboxd-api.md index d697d5ae..0da88c99 100644 --- a/docs/sandboxd-api.md +++ b/docs/sandboxd-api.md @@ -73,8 +73,9 @@ Auth: `Authorization: Bearer ` (when configured). host-only value in the guest. `guest` (default `true`) delivers the entry into the guest; a `guest: false` entry never enters it and only feeds the node's egress [secrets](egress.md#claim-env-and-secrets), or, with `inject` - (`{"hosts": [...], "header": "...", "placeholder": "..."}`, `placeholder` - optional), becomes a + (`{"hosts": [...], "header": "...", "placeholder": "..."}` with `placeholder` + optional, or `{"hosts": [...], "query": "...", "body": true, "placeholder": "..."}` + with `body` optional), becomes a [claim credential](egress.md#claim-credentials) the proxy sends to those hosts. A bad entry answers 400, and an `inject` host the claim's egress does not intercept answers 400 with the claim's VM released; see @@ -730,7 +731,7 @@ values: {"env": {"MODE": {"value": "prod"}, "GW_KEY": {"value": "", "guest": false}}} ``` -An entry with `inject` comes back with its hosts, header and placeholder, value blanked. +An entry with `inject` comes back with its hosts, header or query, body flag and placeholder, value blanked. An `inject` host the claim's pool does not intercept, or its tenant class does not allow, and a header the pool rule's own secret already sets on that host, answer 400 and store nothing. @@ -788,7 +789,8 @@ Always on: every lifecycle transition appends one JSONL event to "id": "sb_…", "vm": "sbx-…"}` plus `key` (the pool key's stable hash, claim events), `tenant` (the owning tenant, on claim, egress, archive, unarchive and archive_delete), `children` (fork) and `ref` (the promoted -template / checkpoint id, the egress host, or a `vmm_failed` reason). An egress event routed through an +template / checkpoint id, the egress host, or a `vmm_failed` reason). An egress event that injected credentials names +them in `secret` (`gh,claim:`, never a value). An egress event routed through an [upstream proxy](egress.md#upstream-proxies) carries `upstream` (its `host:port`), and with `egress_usage_bytes` on, `egress_bytes` events add the connection's payload `tx`/`rx` bytes. A volume claim also carries `volumes`, the applied catalog names, and — omitted when empty — `volumes_rw`, diff --git a/docs/security.md b/docs/security.md index 14aa984b..9ca087f9 100644 --- a/docs/security.md +++ b/docs/security.md @@ -134,7 +134,8 @@ Facts to plan around, stated so the boundary is honest: cocoonstack/cocoon#83). Compensate with dedicated sandbox nodes and a minimal host. - **HTTPS interception is HTTP/1.1 only and breaks certificate-pinning - clients** — scope `intercept` rules to hosts you control + clients** — scope `intercept` rules to hosts you control, or use + `"inject"` so only claims holding a credential for a host are intercepted ([egress](egress.md#https-interception)). - **The audit and usage journals are local JSONL** with size rotation — no tamper-evidence. Ship them off-node if integrity against a host diff --git a/sandboxd/config/config_test.go b/sandboxd/config/config_test.go index 54387ee2..3b3a82a8 100644 --- a/sandboxd/config/config_test.go +++ b/sandboxd/config/config_test.go @@ -96,7 +96,7 @@ func TestClusterDigestCoversTheEgressLayerInAnyOrder(t *testing.T) { Secrets: []egress.SecretSpec{{Name: "gh", Header: "Authorization", ValueEnv: "GH_A"}, {Name: "gw", Header: "X-Key"}}, EgressInternalAllow: []string{"10.0.0.0/8", "172.16.0.0/12"}, EgressUpstream: &EgressUpstreamConfig{ClaimEnv: "EGRESS_UPSTREAM", Allow: []string{"a.example", "b.example"}}, - Pools: []PoolSpec{{PoolKey: key, Warm: 2, Egress: &egress.Policy{Allow: []egress.Rule{{Host: "api.example.com", Intercept: true}}}}}, + Pools: []PoolSpec{{PoolKey: key, Warm: 2, Egress: &egress.Policy{Allow: []egress.Rule{{Host: "api.example.com", Intercept: egress.InterceptAlways}}}}}, } } d := base().ClusterDigest("fp", nil) @@ -394,21 +394,25 @@ func TestLoadAcceptsNoneLanePolicyOnCNI(t *testing.T) { } func TestLoadAcceptsPoolIntercept(t *testing.T) { - path := writeConfig(t, `{"egress_ca":{"root_cert":"/x/root.crt","intermediate_cert":"/x/n.crt","intermediate_key":"/x/n.key"}, - "pools":[{"template":"rt:24.04","net":"none","size":"small","egress":{"allow":[{"host":"api.github.com","intercept":true}]}}]}`) - cfg, err := Load(path) - if err != nil { - t.Fatalf("Load: %v", err) - } - if pol := cfg.Pools[0].Egress; pol == nil || len(pol.Allow) != 1 || !pol.Allow[0].Intercept { - t.Errorf("intercept rule not parsed: %+v", cfg.Pools[0].Egress) + for mode, want := range map[string]egress.InterceptMode{"true": egress.InterceptAlways, `"inject"`: egress.InterceptInject} { + path := writeConfig(t, `{"egress_ca":{"root_cert":"/x/root.crt","intermediate_cert":"/x/n.crt","intermediate_key":"/x/n.key"}, + "pools":[{"template":"rt:24.04","net":"none","size":"small","egress":{"allow":[{"host":"api.github.com","intercept":`+mode+`}]}}]}`) + cfg, err := Load(path) + if err != nil { + t.Fatalf("Load intercept %s: %v", mode, err) + } + if pol := cfg.Pools[0].Egress; pol == nil || len(pol.Allow) != 1 || pol.Allow[0].Intercept != want { + t.Errorf("intercept %s rule not parsed: %+v", mode, cfg.Pools[0].Egress) + } } } func TestLoadRejectsInterceptWithoutCA(t *testing.T) { - path := writeConfig(t, `{"pools":[{"template":"rt:24.04","net":"none","size":"small","egress":{"allow":[{"host":"x","intercept":true}]}}]}`) - if _, err := Load(path); err == nil { - t.Error("Load accepted an intercept pool without egress_ca; want rejection") + for _, mode := range []string{"true", `"inject"`} { + path := writeConfig(t, `{"pools":[{"template":"rt:24.04","net":"none","size":"small","egress":{"allow":[{"host":"x","intercept":`+mode+`}]}}]}`) + if _, err := Load(path); err == nil { + t.Errorf("Load accepted an intercept %s pool without egress_ca; want rejection", mode) + } } } diff --git a/sandboxd/egress/bench_test.go b/sandboxd/egress/bench_test.go index a1c9d45f..ce318fb5 100644 --- a/sandboxd/egress/bench_test.go +++ b/sandboxd/egress/bench_test.go @@ -20,7 +20,7 @@ func BenchmarkSignLeaf(b *testing.B) { } func BenchmarkInterceptedRequest(b *testing.B) { - addr, roots := benchFront(b, true) + addr, roots := benchFront(b, InterceptAlways) tc := connectTLS(b, addr, "example.com:443", "example.com", roots) defer func() { _ = tc.Close() }() br := bufio.NewReader(tc) @@ -30,7 +30,7 @@ func BenchmarkInterceptedRequest(b *testing.B) { } func BenchmarkSplicedRequest(b *testing.B) { - addr, roots := benchFront(b, false) + addr, roots := benchFront(b, InterceptOff) tc := connectTLS(b, addr, "example.com:443", "example.com", roots) defer func() { _ = tc.Close() }() br := bufio.NewReader(tc) @@ -40,7 +40,7 @@ func BenchmarkSplicedRequest(b *testing.B) { } func BenchmarkInterceptedHandshake(b *testing.B) { - addr, roots := benchFront(b, true) + addr, roots := benchFront(b, InterceptAlways) for b.Loop() { tc := connectTLS(b, addr, "example.com:443", "example.com", roots) _ = tc.Close() @@ -50,7 +50,7 @@ func BenchmarkInterceptedHandshake(b *testing.B) { func BenchmarkInterceptedConnection(b *testing.B) { for _, arm := range []string{"fresh", "session-cache"} { b.Run(arm, func(b *testing.B) { - addr, roots := benchFront(b, true) + addr, roots := benchFront(b, InterceptAlways) var cache tls.ClientSessionCache if arm == "session-cache" { cache = tls.NewLRUClientSessionCache(8) @@ -70,14 +70,33 @@ func BenchmarkInterceptedConnection(b *testing.B) { } func BenchmarkSplicedHandshake(b *testing.B) { - addr, roots := benchFront(b, false) + addr, roots := benchFront(b, InterceptOff) for b.Loop() { tc := connectTLS(b, addr, "example.com:443", "example.com", roots) _ = tc.Close() } } -func benchFront(b *testing.B, intercept bool) (proxyAddr string, roots *x509.CertPool) { +func BenchmarkTunnelDecision(b *testing.B) { + ca := testCAOnly(b) + creds := map[string][]Credential{"api.example.com": {{Name: "KEY", Header: "X-Key", Value: "k"}}} + for _, arm := range []struct { + name string + mode InterceptMode + }{{"plain", InterceptOff}, {"inject", InterceptInject}, {"always", InterceptAlways}} { + p := New(Policy{Allow: []Rule{{Host: "*", Intercept: arm.mode}}}, credSecrets{creds: creds}, ca, fixedDial(""), nil, nil) + for _, host := range []string{"other.example.com", "api.example.com"} { + b.Run(arm.name+"/"+host, func(b *testing.B) { + b.ReportAllocs() + for b.Loop() { + _, _ = p.tunnelDecision(host, 443) + } + }) + } + } +} + +func benchFront(b *testing.B, intercept InterceptMode) (proxyAddr string, roots *x509.CertPool) { b.Helper() upstream := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { _, _ = io.WriteString(w, "hello") @@ -86,7 +105,7 @@ func benchFront(b *testing.B, intercept bool) (proxyAddr string, roots *x509.Cer rule := Rule{Host: "example.com", Intercept: intercept} var ca *CA roots = x509.NewCertPool() - if intercept { + if intercept == InterceptAlways { ca = testCAOnly(b) if !roots.AppendCertsFromPEM(ca.CertPEM()) { b.Fatal("append cluster root") diff --git a/sandboxd/egress/intercept_test.go b/sandboxd/egress/intercept_test.go index 2f5a8ae8..859afe63 100644 --- a/sandboxd/egress/intercept_test.go +++ b/sandboxd/egress/intercept_test.go @@ -62,7 +62,7 @@ func TestInterceptLeafCacheBounded(t *testing.T) { func TestInterceptInjectsSecretIntoHTTPS(t *testing.T) { events := make(chan Event, 4) - p, guestRoots, upstream := interceptProxy(t, Rule{Host: "example.com", Methods: []string{"GET"}, Secret: "gh", Intercept: true}, events) + p, guestRoots, upstream := interceptProxy(t, Rule{Host: "example.com", Methods: []string{"GET"}, Secret: "gh", Intercept: InterceptAlways}, events) p.pools.Load().mitm.TLSClientConfig.RootCAs = trustUpstream(upstream) front := httptest.NewServer(p) defer front.Close() @@ -91,7 +91,7 @@ func TestInterceptInjectsSecretIntoHTTPS(t *testing.T) { } func TestInterceptKeepsAClientHelloSentBeforeThe200(t *testing.T) { - p, guestRoots, upstream := interceptProxy(t, Rule{Host: "example.com", Methods: []string{"GET"}, Intercept: true}, nil) + p, guestRoots, upstream := interceptProxy(t, Rule{Host: "example.com", Methods: []string{"GET"}, Intercept: InterceptAlways}, nil) p.pools.Load().mitm.TLSClientConfig.RootCAs = trustUpstream(upstream) front := httptest.NewServer(p) defer front.Close() @@ -116,7 +116,7 @@ func TestInterceptKeepsAClientHelloSentBeforeThe200(t *testing.T) { func TestInterceptBindsInnerRequestToInterceptRule(t *testing.T) { policy := Policy{Allow: []Rule{ {Host: "*.example.com"}, - {Host: "api.example.com", Secret: "gh", Intercept: true}, + {Host: "api.example.com", Secret: "gh", Intercept: InterceptAlways}, }} p, guestRoots, upstream := interceptProxyPolicy(t, policy, nil) p.pools.Load().mitm.TLSClientConfig.RootCAs = trustUpstream(upstream) @@ -136,7 +136,7 @@ func TestInterceptBindsInnerRequestToInterceptRule(t *testing.T) { func TestInterceptCaptureEnforcesRuleMethod(t *testing.T) { policy := Policy{Allow: []Rule{ {Host: "*.example.com"}, - {Host: "api.example.com", Methods: []string{"GET"}, Intercept: true}, + {Host: "api.example.com", Methods: []string{"GET"}, Intercept: InterceptAlways}, }} p, guestRoots, upstream := interceptProxyPolicy(t, policy, nil) p.pools.Load().mitm.TLSClientConfig.RootCAs = trustUpstream(upstream) @@ -155,8 +155,8 @@ func TestInterceptCaptureEnforcesRuleMethod(t *testing.T) { func TestInterceptReachesLaterInterceptRuleByMethod(t *testing.T) { policy := Policy{Allow: []Rule{ - {Host: "*.example.com", Methods: []string{"GET"}, Intercept: true}, - {Host: "api.example.com", Methods: []string{"POST"}, Intercept: true}, + {Host: "*.example.com", Methods: []string{"GET"}, Intercept: InterceptAlways}, + {Host: "api.example.com", Methods: []string{"POST"}, Intercept: InterceptAlways}, }} p, guestRoots, upstream := interceptProxyPolicy(t, policy, nil) p.pools.Load().mitm.TLSClientConfig.RootCAs = trustUpstream(upstream) @@ -174,7 +174,7 @@ func TestInterceptReachesLaterInterceptRuleByMethod(t *testing.T) { } func TestInterceptForcesAuthorityOnForeignHost(t *testing.T) { - p, guestRoots, upstream := interceptProxy(t, Rule{Host: "example.com", Intercept: true}, nil) + p, guestRoots, upstream := interceptProxy(t, Rule{Host: "example.com", Intercept: InterceptAlways}, nil) p.pools.Load().mitm.TLSClientConfig.RootCAs = trustUpstream(upstream) front := httptest.NewServer(p) defer front.Close() @@ -190,7 +190,7 @@ func TestInterceptForcesAuthorityOnForeignHost(t *testing.T) { } func TestCloseRevokesInterceptedSession(t *testing.T) { - p, guestRoots, upstream := interceptProxy(t, Rule{Host: "example.com", Intercept: true}, nil) + p, guestRoots, upstream := interceptProxy(t, Rule{Host: "example.com", Intercept: InterceptAlways}, nil) p.pools.Load().mitm.TLSClientConfig.RootCAs = trustUpstream(upstream) front := httptest.NewServer(p) defer front.Close() @@ -216,7 +216,7 @@ func TestCloseRevokesInterceptedSession(t *testing.T) { } func TestInterceptFiltersByMethod(t *testing.T) { - p, guestRoots, upstream := interceptProxy(t, Rule{Host: "example.com", Methods: []string{"GET"}, Intercept: true}, nil) + p, guestRoots, upstream := interceptProxy(t, Rule{Host: "example.com", Methods: []string{"GET"}, Intercept: InterceptAlways}, nil) p.pools.Load().mitm.TLSClientConfig.RootCAs = trustUpstream(upstream) front := httptest.NewServer(p) defer front.Close() @@ -232,7 +232,7 @@ func TestInterceptFiltersByMethod(t *testing.T) { } func TestInterceptRejectsUntrustedUpstream(t *testing.T) { - p, guestRoots, _ := interceptProxy(t, Rule{Host: "example.com", Intercept: true}, nil) + p, guestRoots, _ := interceptProxy(t, Rule{Host: "example.com", Intercept: InterceptAlways}, nil) front := httptest.NewServer(p) defer front.Close() @@ -254,36 +254,23 @@ func TestInterceptInjectsClaimCredentialsBelowThePoolSecret(t *testing.T) { {Name: "EMPTY", Header: "X-Empty", Value: ""}, {Name: "LATER", Header: "X-Api-Key", Value: "k2"}, }}} - p, guestRoots, upstream := interceptProxySecrets(t, Policy{Allow: []Rule{{Host: "example.com", Secret: "gh", Intercept: true}}}, secrets, events) + p, guestRoots, upstream := interceptProxySecrets(t, Policy{Allow: []Rule{{Host: "example.com", Secret: "gh", Intercept: InterceptAlways}}}, secrets, events) p.pools.Load().mitm.TLSClientConfig.RootCAs = trustUpstream(upstream) front := httptest.NewServer(p) defer front.Close() tc := connectTLS(t, front.Listener.Addr().String(), "Example.com:443", "example.com", guestRoots) defer func() { _ = tc.Close() }() - req, err := http.NewRequestWithContext(t.Context(), http.MethodGet, "https://example.com/x", nil) - if err != nil { - t.Fatalf("build request: %v", err) - } - req.Header.Set("X-Api-Key", "placeholder") - req.Header.Set("X-Empty", "guest") - if err = req.Write(tc); err != nil { - t.Fatalf("write request: %v", err) - } - resp, err := http.ReadResponse(bufio.NewReader(tc), req) - if err != nil { - t.Fatalf("read response: %v", err) - } - defer func() { _ = resp.Body.Close() }() + header, seen := echoTLS(t, tc, http.MethodGet, "/echo", "", "", map[string]string{"X-Api-Key": "placeholder", "X-Empty": "guest"}) - if got := resp.Header.Get("X-Key-Seen"); got != "k1" { - t.Errorf("upstream saw X-Api-Key %q, want the first claim credential in order", got) + if seen["key"] != "k1" { + t.Errorf("upstream saw X-Api-Key %q, want the first claim credential in order", seen["key"]) } - if got := resp.Header.Get("X-Auth-Seen"); got != "Bearer SECRET" { + if got := header.Get("X-Auth-Seen"); got != "Bearer SECRET" { t.Errorf("upstream saw Authorization %q, want the pool secret kept over the claim's", got) } - if got := resp.Header.Get("X-Empty-Seen"); got != "guest" { - t.Errorf("upstream saw X-Empty %q, want the guest's value kept for an empty credential", got) + if seen["empty"] != "guest" { + t.Errorf("upstream saw X-Empty %q, want the guest's value kept for an empty credential", seen["empty"]) } _ = recvEvent(t, events) if ev := recvEvent(t, events); ev.Injected != "gh,claim:API" { @@ -295,31 +282,111 @@ func TestInterceptInjectsAPlaceholderCredentialOnlyWhereTheGuestSentIt(t *testin secrets := credSecrets{creds: map[string][]Credential{"example.com": { {Name: "GIT", Header: "X-Api-Key", Value: "real", Placeholder: "@GIT@"}, }}} - p, guestRoots, upstream := interceptProxySecrets(t, Policy{Allow: []Rule{{Host: "example.com", Intercept: true}}}, secrets, nil) + p, guestRoots, upstream := interceptProxySecrets(t, Policy{Allow: []Rule{{Host: "example.com", Intercept: InterceptAlways}}}, secrets, nil) p.pools.Load().mitm.TLSClientConfig.RootCAs = trustUpstream(upstream) front := httptest.NewServer(p) defer front.Close() for sent, want := range map[string]string{"@GIT@": "real", "": "", "@OTHER@": "@OTHER@"} { tc := connectTLS(t, front.Listener.Addr().String(), "example.com:443", "example.com", guestRoots) - req, err := http.NewRequestWithContext(t.Context(), http.MethodGet, "https://example.com/x", nil) - if err != nil { - t.Fatalf("build request: %v", err) - } + headers := map[string]string{} if sent != "" { - req.Header.Set("X-Api-Key", sent) + headers["X-Api-Key"] = sent } - if err = req.Write(tc); err != nil { - t.Fatalf("write request: %v", err) + header, seen := echoTLS(t, tc, http.MethodGet, "/echo", "", "", headers) + _ = tc.Close() + if seen["key"] != want { + t.Errorf("guest sent %q: upstream saw %q, want %q", sent, seen["key"], want) } - resp, err := http.ReadResponse(bufio.NewReader(tc), req) - if err != nil { - t.Fatalf("read response: %v", err) + if echo := header.Get("X-Key-Echo"); echo != sent { + t.Errorf("guest sent %q: the response echo reached it as %q, want what it sent", sent, echo) } - _ = resp.Body.Close() - _ = tc.Close() - if got := resp.Header.Get("X-Key-Seen"); got != want { - t.Errorf("guest sent %q: upstream saw %q, want %q", sent, got, want) + } +} + +func TestAnInjectRuleInterceptsOnlyForAClaimWithACredentialForTheHost(t *testing.T) { + secrets := credSecrets{creds: map[string][]Credential{"example.com": {{Name: "KEY", Header: "X-Api-Key", Value: "real"}}}} + p, guestRoots, upstream := interceptProxySecrets(t, Policy{Allow: []Rule{{Host: "*", Intercept: InterceptInject}}}, secrets, nil) + p.pools.Load().mitm.TLSClientConfig.RootCAs = trustUpstream(upstream) + front := httptest.NewServer(p) + defer front.Close() + + tc := connectTLS(t, front.Listener.Addr().String(), "Example.com:443", "example.com", guestRoots) + _, seen := echoTLS(t, tc, http.MethodGet, "/echo", "", "", nil) + _ = tc.Close() + if seen["key"] != "real" { + t.Errorf("a host with a credential: upstream saw X-Api-Key %q, want it intercepted and injected", seen["key"]) + } + tc = connectTLS(t, front.Listener.Addr().String(), "other.test:443", "example.com", trustUpstream(upstream)) + _, seen = echoTLS(t, tc, http.MethodGet, "/echo", "", "", map[string]string{"X-Api-Key": "guest"}) + _ = tc.Close() + if seen["key"] != "guest" { + t.Errorf("a host without a credential: upstream saw X-Api-Key %q, want the guest's own value over a splice", seen["key"]) + } + if _, intercept := New(Policy{Allow: []Rule{{Host: "*", Intercept: InterceptInject}}}, nil, p.ca, fixedDial(""), nil, nil).tunnelDecision("example.com", 443); intercept { + t.Error("a claim with no secrets intercepted on an inject rule") + } + if d, intercept := p.tunnelDecision("plain.test", 80); d != DecisionAllow || intercept { + t.Errorf("an inject rule without a credential: decision %v intercept %t, want a plain allow", d, intercept) + } +} + +func TestInterceptSubstitutesAQueryCredentialAndScrubsItsEcho(t *testing.T) { + secrets := credSecrets{creds: map[string][]Credential{"example.com": {{Name: "SERP", Query: "api_key", Value: "real key/+", Placeholder: "@SERP@"}}}} + events := make(chan Event, 8) + p, guestRoots, upstream := interceptProxySecrets(t, Policy{Allow: []Rule{{Host: "example.com", Intercept: InterceptAlways}}}, secrets, events) + p.pools.Load().mitm.TLSClientConfig.RootCAs = trustUpstream(upstream) + front := httptest.NewServer(p) + defer front.Close() + + for _, tc := range []struct { + query, want, injected string + }{ + {"b=2&api_key=%40SERP%40&a=1", "b=2&api_key=real+key%2F%2B&a=1", "claim:SERP"}, + {"api_key=@SERP@", "api_key=real+key%2F%2B", "claim:SERP"}, + {"api_key=other&x=%40SERP%40", "api_key=other&x=%40SERP%40", ""}, + {"", "", ""}, + } { + conn := connectTLS(t, front.Listener.Addr().String(), "example.com:443", "example.com", guestRoots) + header, seen := echoTLS(t, conn, http.MethodGet, "/echo?"+tc.query, "", "", nil) + _ = conn.Close() + if seen["query"] != tc.want { + t.Errorf("query %q reached the upstream as %q, want %q", tc.query, seen["query"], tc.want) + } + if loc := header.Get("Location"); strings.Contains(loc, "real") { + t.Errorf("query %q: the guest received Location %q holding the value", tc.query, loc) + } + _ = recvEvent(t, events) + if ev := recvEvent(t, events); ev.Injected != tc.injected { + t.Errorf("query %q: audit Injected %q, want %q", tc.query, ev.Injected, tc.injected) + } + } +} + +func TestInterceptSubstitutesABodyCredentialInASmallFormOrJSONBody(t *testing.T) { + secrets := credSecrets{creds: map[string][]Credential{"example.com": { + {Name: "FB", Query: "access_token", Body: true, Value: `real "tok"&`, Placeholder: "@FB@"}, + {Name: "URLONLY", Query: "key", Value: "other", Placeholder: "@URL@"}, + }}} + p, guestRoots, upstream := interceptProxySecrets(t, Policy{Allow: []Rule{{Host: "example.com", Intercept: InterceptAlways}}}, secrets, nil) + p.pools.Load().mitm.TLSClientConfig.RootCAs = trustUpstream(upstream) + front := httptest.NewServer(p) + defer front.Close() + + big := "access_token=%40FB%40&pad=" + strings.Repeat("x", maxInjectBody) + for _, tc := range []struct { + name, ctype, body, want string + }{ + {"form", "application/x-www-form-urlencoded", "a=1&access_token=%40FB%40&key=%40URL%40", "a=1&access_token=real+%22tok%22%26&key=%40URL%40"}, + {"json", "application/json; charset=utf-8", `{"access_token":"@FB@","n":"@FBX@"}`, `{"access_token":"real \"tok\"&","n":"@FBX@"}`}, + {"other type", "text/plain", "access_token=%40FB%40", "access_token=%40FB%40"}, + {"oversized", "application/x-www-form-urlencoded", big, big}, + } { + conn := connectTLS(t, front.Listener.Addr().String(), "example.com:443", "example.com", guestRoots) + _, seen := echoTLS(t, conn, http.MethodPost, "/echo", tc.ctype, tc.body, nil) + _ = conn.Close() + if seen["body"] != tc.want || seen["length"] != fmt.Sprint(len(tc.want)) { + t.Errorf("%s: upstream saw body %.80q length %s, want %.80q length %d", tc.name, seen["body"], seen["length"], tc.want, len(tc.want)) } } } @@ -330,7 +397,7 @@ func TestInterceptStreamsAnEventStreamAsItArrives(t *testing.T) { defer upstream.Close() defer close(release) ca, _ := testCA(t) - p := New(Policy{Allow: []Rule{{Host: "example.com", Intercept: true}}}, nil, ca, fixedDial(upstream.Listener.Addr().String()), nil, nil) + p := New(Policy{Allow: []Rule{{Host: "example.com", Intercept: InterceptAlways}}}, nil, ca, fixedDial(upstream.Listener.Addr().String()), nil, nil) p.pools.Load().mitm.TLSClientConfig.RootCAs = trustUpstream(upstream) guestRoots := x509.NewCertPool() guestRoots.AppendCertsFromPEM(ca.CertPEM()) @@ -348,7 +415,7 @@ func TestInterceptStreamsAnEventStreamAsItArrives(t *testing.T) { } func TestInterceptResumesTheGuestsTLSSession(t *testing.T) { - p, guestRoots, upstream := interceptProxy(t, Rule{Host: "example.com", Intercept: true}, nil) + p, guestRoots, upstream := interceptProxy(t, Rule{Host: "example.com", Intercept: InterceptAlways}, nil) p.pools.Load().mitm.TLSClientConfig.RootCAs = trustUpstream(upstream) front := httptest.NewServer(p) defer front.Close() @@ -378,10 +445,15 @@ func interceptProxySecrets(t *testing.T, policy Policy, secrets Secrets, events t.Helper() upstream := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("X-Auth-Seen", r.Header.Get("Authorization")) - w.Header().Set("X-Key-Seen", r.Header.Get("X-Api-Key")) - w.Header().Set("X-Empty-Seen", r.Header.Get("X-Empty")) w.Header().Set("X-Host-Seen", r.Host) - _, _ = io.WriteString(w, "hello") + if r.URL.Path != "/echo" { + _, _ = io.WriteString(w, "hello") + return + } + body, _ := io.ReadAll(r.Body) + w.Header().Set("Location", "/next?"+r.URL.RawQuery) + w.Header().Set("X-Key-Echo", r.Header.Get("X-Api-Key")) + _, _ = fmt.Fprintf(w, "key=%s\nempty=%s\nquery=%s\nlength=%d\nbody=%s", r.Header.Get("X-Api-Key"), r.Header.Get("X-Empty"), r.URL.RawQuery, r.ContentLength, body) })) t.Cleanup(upstream.Close) @@ -470,6 +542,37 @@ func roundTripTLSHost(t *testing.T, tc *tls.Conn, method, host string) *http.Res return resp } +// echoTLS sends one request on tc to the upstream's /echo, which reports what it saw in the body. +func echoTLS(t *testing.T, tc *tls.Conn, method, target, ctype, body string, headers map[string]string) (http.Header, map[string]string) { + t.Helper() + req, err := http.NewRequestWithContext(t.Context(), method, "https://example.com"+target, strings.NewReader(body)) + if err != nil { + t.Fatalf("build request: %v", err) + } + if ctype != "" { + req.Header.Set("Content-Type", ctype) + } + for k, v := range headers { + req.Header.Set(k, v) + } + if err = req.Write(tc); err != nil { + t.Fatalf("write request: %v", err) + } + resp, err := http.ReadResponse(bufio.NewReader(tc), req) + if err != nil { + t.Fatalf("read response: %v", err) + } + raw, _ := io.ReadAll(resp.Body) + _ = resp.Body.Close() + seen := map[string]string{} + for line := range strings.SplitSeq(string(raw), "\n") { + if k, v, ok := strings.Cut(line, "="); ok && seen[k] == "" { + seen[k] = v + } + } + return resp.Header, seen +} + func readPreamble(tb testing.TB, conn net.Conn) string { tb.Helper() var sb strings.Builder diff --git a/sandboxd/egress/policy.go b/sandboxd/egress/policy.go index 465fa11c..06b721cd 100644 --- a/sandboxd/egress/policy.go +++ b/sandboxd/egress/policy.go @@ -2,6 +2,7 @@ package egress import ( + "encoding/json/jsontext" "fmt" "net/http" "slices" @@ -11,18 +12,54 @@ import ( const ( DecisionDeny Decision = iota DecisionAllow + + InterceptOff InterceptMode = 0 + InterceptAlways InterceptMode = 1 + InterceptInject InterceptMode = 2 ) // Decision is the policy verdict for one request. type Decision int +// InterceptMode is a rule's "intercept": false, true, or "inject" for only the claims that inject on the host. +type InterceptMode uint8 + +func (m InterceptMode) MarshalJSONTo(enc *jsontext.Encoder) error { + switch m { + case InterceptAlways: + return enc.WriteToken(jsontext.True) + case InterceptInject: + return enc.WriteToken(jsontext.String("inject")) + default: + return enc.WriteToken(jsontext.False) + } +} + +func (m *InterceptMode) UnmarshalJSONFrom(dec *jsontext.Decoder) error { + tok, err := dec.ReadToken() + if err != nil { + return err + } + switch { + case tok.Kind() == 'f': + *m = InterceptOff + case tok.Kind() == 't': + *m = InterceptAlways + case tok.Kind() == '"' && tok.String() == "inject": + *m = InterceptInject + default: + return fmt.Errorf("intercept must be true, false or \"inject\", got %s", tok) + } + return nil +} + // Rule allows requests matching Host (exact, "*." suffix, or "*"), Methods and Ports; empty means any. type Rule struct { - Host string `json:"host"` - Methods []string `json:"methods,omitempty"` - Ports []uint16 `json:"ports,omitempty"` - Secret string `json:"secret,omitempty"` - Intercept bool `json:"intercept,omitzero"` + Host string `json:"host"` + Methods []string `json:"methods,omitempty"` + Ports []uint16 `json:"ports,omitempty"` + Secret string `json:"secret,omitempty"` + Intercept InterceptMode `json:"intercept,omitzero"` } // Covers reports whether the rule matches every host the pattern, exact or "*." suffix, matches. @@ -66,9 +103,9 @@ type Policy struct { Socks5 bool `json:"socks5,omitzero"` // opts the SOCKS5 door in; a rule must still admit CONNECT } -// Intercepts reports whether any rule terminates HTTPS; nil is false. +// Intercepts reports whether any rule may terminate HTTPS; nil is false. func (p *Policy) Intercepts() bool { - return p != nil && slices.ContainsFunc(p.Allow, func(r Rule) bool { return r.Intercept }) + return p != nil && slices.ContainsFunc(p.Allow, func(r Rule) bool { return r.Intercept != InterceptOff }) } // Validate rejects a policy the proxy could never honor; an empty allow-list is valid and denies everything. @@ -80,6 +117,9 @@ func (p Policy) Validate() error { case "*.": return fmt.Errorf("allow[%d]: host %q needs a domain after the wildcard", i, r.Host) } + if r.Intercept == InterceptInject && r.Secret != "" { + return fmt.Errorf("allow[%d]: an intercept \"inject\" rule splices for claims without a credential, so it must not carry a secret", i) + } for j, port := range r.Ports { if port == 0 { return fmt.Errorf("allow[%d]: port 0 is not a destination", i) @@ -95,11 +135,11 @@ func (p Policy) Validate() error { return nil } -// Eval returns the first matching non-intercept rule; matching one here would leak its secret. +// Eval returns the first matching rule that may carry a request unintercepted; an always-intercept rule here would leak its secret. func (p Policy) Eval(host, method string, port uint16) (Rule, Decision) { host = strings.ToLower(host) for _, r := range p.Allow { - if !r.Intercept && r.matches(host, method, port) { + if r.Intercept != InterceptAlways && r.matches(host, method, port) { return r, DecisionAllow } } @@ -113,7 +153,7 @@ func (p Policy) EvalHost(host string, port uint16) (Rule, Decision) { for i, r := range p.Allow { switch { case !r.matchHost(host) || !r.matchPort(port): - case r.Intercept: + case r.Intercept != InterceptOff: return r, DecisionAllow case first < 0: first = i @@ -129,7 +169,7 @@ func (p Policy) EvalHost(host string, port uint16) (Rule, Decision) { func (p Policy) EvalInner(host, method string, port uint16) (Rule, Decision) { host = strings.ToLower(host) for _, r := range p.Allow { - if r.Intercept && r.matches(host, method, port) { + if r.Intercept != InterceptOff && r.matches(host, method, port) { return r, DecisionAllow } } @@ -143,11 +183,11 @@ func (p Policy) ServesSocks() bool { // InterceptsHost reports whether an intercept rule covers the host pattern, so a claim credential for it can be injected. func (p Policy) InterceptsHost(pattern string) bool { - return slices.ContainsFunc(p.Allow, func(r Rule) bool { return r.Intercept && r.Covers(pattern) }) + return slices.ContainsFunc(p.Allow, func(r Rule) bool { return r.Intercept != InterceptOff && r.Covers(pattern) }) } func (p Policy) admitsTunnel() bool { - return slices.ContainsFunc(p.Allow, func(r Rule) bool { return !r.Intercept && r.matchMethod(http.MethodConnect) }) + return slices.ContainsFunc(p.Allow, func(r Rule) bool { return r.Intercept != InterceptAlways && r.matchMethod(http.MethodConnect) }) } // Evaluator is what the proxy consults per request. diff --git a/sandboxd/egress/policy_test.go b/sandboxd/egress/policy_test.go index 2d6af474..43252343 100644 --- a/sandboxd/egress/policy_test.go +++ b/sandboxd/egress/policy_test.go @@ -1,6 +1,9 @@ package egress -import "testing" +import ( + "encoding/json/v2" + "testing" +) func TestPolicyEval(t *testing.T) { policy := Policy{Allow: []Rule{ @@ -38,7 +41,7 @@ func TestPolicyEval(t *testing.T) { func TestEvalSkipsInterceptRules(t *testing.T) { p := Policy{Allow: []Rule{ - {Host: "api.github.com", Secret: "gh", Intercept: true}, + {Host: "api.github.com", Secret: "gh", Intercept: InterceptAlways}, {Host: "plain.github.com"}, }} @@ -51,18 +54,47 @@ func TestEvalSkipsInterceptRules(t *testing.T) { if _, d := p.EvalHost("api.github.com", 443); d != DecisionAllow { t.Error("EvalHost must still allow the intercepted CONNECT path") } + inject := Policy{Allow: []Rule{{Host: "*", Intercept: InterceptInject}}} + if _, d := inject.Eval("any.test", "CONNECT", 443); d != DecisionAllow { + t.Error("Eval skipped an inject rule, which splices or forwards for a claim without a credential") + } +} + +func TestInterceptModeJSON(t *testing.T) { + var p Policy + if err := json.Unmarshal([]byte(`{"allow":[{"host":"a"},{"host":"b","intercept":false},{"host":"c","intercept":true},{"host":"d","intercept":"inject"}]}`), &p); err != nil { + t.Fatalf("Unmarshal: %v", err) + } + want := []InterceptMode{InterceptOff, InterceptOff, InterceptAlways, InterceptInject} + for i, r := range p.Allow { + if r.Intercept != want[i] { + t.Errorf("rule %s intercept = %d, want %d", r.Host, r.Intercept, want[i]) + } + } + out, err := json.Marshal(p) + if err != nil { + t.Fatalf("Marshal: %v", err) + } + if got := string(out); got != `{"allow":[{"host":"a"},{"host":"b"},{"host":"c","intercept":true},{"host":"d","intercept":"inject"}]}` { + t.Errorf("Marshal = %s", got) + } + for _, bad := range []string{`"always"`, `1`, `null`} { + if err := json.Unmarshal([]byte(`{"allow":[{"host":"a","intercept":`+bad+`}]}`), &p); err == nil { + t.Errorf("intercept %s decoded", bad) + } + } } func TestEvalHostPrefersInterceptRule(t *testing.T) { p := Policy{Allow: []Rule{ {Host: "api.github.com", Methods: []string{"GET"}}, - {Host: "api.github.com", Intercept: true}, + {Host: "api.github.com", Intercept: InterceptAlways}, }} rule, d := p.EvalHost("api.github.com", 443) - if d != DecisionAllow || !rule.Intercept { + if d != DecisionAllow || rule.Intercept != InterceptAlways { t.Errorf("EvalHost = %+v/%v, want the intercept rule over the earlier plain match", rule, d) } - if rule, d := p.EvalHost("other.com", 443); d != DecisionDeny || rule.Intercept { + if rule, d := p.EvalHost("other.com", 443); d != DecisionDeny || rule.Intercept != InterceptOff { t.Errorf("EvalHost(other.com) = %+v/%v, want deny", rule, d) } } @@ -70,8 +102,8 @@ func TestEvalHostPrefersInterceptRule(t *testing.T) { func TestEvalInnerMatchesOnlyInterceptRulesByMethod(t *testing.T) { p := Policy{Allow: []Rule{ {Host: "*.example.com"}, - {Host: "api.example.com", Methods: []string{"GET"}, Secret: "gh", Intercept: true}, - {Host: "api.example.com", Methods: []string{"POST"}, Intercept: true}, + {Host: "api.example.com", Methods: []string{"GET"}, Secret: "gh", Intercept: InterceptAlways}, + {Host: "api.example.com", Methods: []string{"POST"}, Intercept: InterceptAlways}, }} if rule, d := p.EvalInner("api.example.com", "GET", 443); d != DecisionAllow || rule.Secret != "gh" { t.Errorf("EvalInner GET = %+v/%v, want the GET intercept rule with secret gh", rule, d) @@ -85,7 +117,7 @@ func TestEvalInnerMatchesOnlyInterceptRulesByMethod(t *testing.T) { } func TestCompositeEvalInnerIntersectsTenant(t *testing.T) { - pool := Policy{Allow: []Rule{{Host: "api.example.com", Secret: "gh", Intercept: true}}} + pool := Policy{Allow: []Rule{{Host: "api.example.com", Secret: "gh", Intercept: InterceptAlways}}} ev := Compose(pool, Policy{Allow: []Rule{{Host: "api.example.com", Methods: []string{"GET"}}}}) if rule, d := ev.EvalInner("api.example.com", "GET", 443); d != DecisionAllow || rule.Secret != "gh" { t.Errorf("EvalInner GET = %+v/%v, want allow with the pool secret", rule, d) @@ -97,8 +129,8 @@ func TestCompositeEvalInnerIntersectsTenant(t *testing.T) { func TestInterceptsHostCoversPatterns(t *testing.T) { pool := Policy{Allow: []Rule{ - {Host: "api.example.com", Intercept: true}, - {Host: "*.Corp.Test", Intercept: true}, + {Host: "api.example.com", Intercept: InterceptAlways}, + {Host: "*.Corp.Test", Intercept: InterceptAlways}, {Host: "plain.example.com"}, }} for pattern, want := range map[string]bool{ @@ -114,7 +146,7 @@ func TestInterceptsHostCoversPatterns(t *testing.T) { t.Errorf("pool InterceptsHost(%q) = %v, want %v", pattern, got, want) } } - if !(Policy{Allow: []Rule{{Host: "*", Intercept: true}}}).InterceptsHost("*.anything.test") { + if !(Policy{Allow: []Rule{{Host: "*", Intercept: InterceptAlways}}}).InterceptsHost("*.anything.test") { t.Error("a bare * intercept rule does not cover a wildcard pattern") } composed := Compose(pool, Policy{Allow: []Rule{{Host: "*.corp.test"}}}) @@ -148,6 +180,9 @@ func TestPolicyValidate(t *testing.T) { {"ports", Policy{Allow: []Rule{{Host: "api.github.com", Ports: []uint16{443, 993}}}}, false}, {"port 0", Policy{Allow: []Rule{{Host: "api.github.com", Ports: []uint16{0}}}}, true}, {"repeated port", Policy{Allow: []Rule{{Host: "api.github.com", Ports: []uint16{443, 443}}}}, true}, + {"inject rule with a secret", Policy{Allow: []Rule{{Host: "*", Secret: "gh", Intercept: InterceptInject}}}, true}, + {"socks5 with only an inject rule", Policy{Socks5: true, Allow: []Rule{{Host: "*", Intercept: InterceptInject}}}, false}, + {"socks5 with only an intercept rule", Policy{Socks5: true, Allow: []Rule{{Host: "*", Intercept: InterceptAlways}}}, true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -175,7 +210,7 @@ func TestPolicyServesSocks(t *testing.T) { func TestRulePorts(t *testing.T) { policy := Policy{Allow: []Rule{ {Host: "db.internal", Ports: []uint16{5432}}, - {Host: "mail.internal", Ports: []uint16{993, 465}, Intercept: true}, + {Host: "mail.internal", Ports: []uint16{993, 465}, Intercept: InterceptAlways}, {Host: "*.open.internal"}, }} tests := []struct { @@ -195,7 +230,7 @@ func TestRulePorts(t *testing.T) { } }) } - if rule, d := policy.EvalHost("mail.internal", 993); d != DecisionAllow || !rule.Intercept { + if rule, d := policy.EvalHost("mail.internal", 993); d != DecisionAllow || rule.Intercept != InterceptAlways { t.Errorf("EvalHost(mail.internal, 993) = %+v/%v, want the intercept rule", rule, d) } if _, d := policy.EvalHost("mail.internal", 143); d != DecisionDeny { diff --git a/sandboxd/egress/proxy.go b/sandboxd/egress/proxy.go index fc2ac8cc..0c839b12 100644 --- a/sandboxd/egress/proxy.go +++ b/sandboxd/egress/proxy.go @@ -2,10 +2,12 @@ package egress import ( "bufio" + "bytes" "cmp" "context" "crypto/rand" "crypto/tls" + "encoding/json/jsontext" "fmt" "io" "maps" @@ -14,6 +16,7 @@ import ( "net/http/httptrace" "net/netip" "net/textproto" + "net/url" "slices" "strconv" "strings" @@ -27,7 +30,8 @@ import ( const ( idleConnTimeout = 90 * time.Second // maxIdleConns bounds the upstream pool per sandbox; a guest walking many hosts cannot park a descriptor per host. - maxIdleConns = 64 + maxIdleConns = 64 + maxInjectBody = 64 << 10 ) var ( @@ -46,10 +50,12 @@ type TransferFunc func(ev Event, sent, received int64) // AuditFunc receives every egress decision the proxy takes. type AuditFunc func(Event) -// Credential is a claim-supplied header the proxy sets on an intercepted request; a set Placeholder must be the guest's value for that header. +// Credential is a claim-supplied value the proxy sets on an intercepted request as Header, or in place of the Placeholder value of Query (and with Body in the request body); a set Placeholder must be what the guest sent. type Credential struct { Name string Header string + Query string + Body bool Value string Placeholder string } @@ -275,7 +281,10 @@ func (p *Proxy) serveConnect(w http.ResponseWriter, r *http.Request) { func (p *Proxy) tunnelDecision(host string, port uint16) (decision Decision, intercept bool) { // host-gate interception: the tunnel's CONNECT verb is not the request method. if p.ca != nil { - if rule, d := p.policy.EvalHost(host, port); d == DecisionAllow && rule.Intercept { + switch rule, d := p.policy.EvalHost(host, port); { + case d != DecisionAllow: + case rule.Intercept == InterceptAlways, + rule.Intercept == InterceptInject && p.secrets != nil && len(p.secrets.Credentials(strings.ToLower(host))) > 0: return DecisionAllow, true } } @@ -325,7 +334,8 @@ func (p *Proxy) relay(w http.ResponseWriter, r *http.Request, ev Event, rule Rul out.RequestURI = "" out.Close = false // the guest's Connection: close is its own; the upstream pool keeps the conn stripHop(out.Header) - ev.Injected = p.inject(rule, out.Header, ev.Host, mitm) + var used []Credential + ev.Injected, used = p.inject(rule, out, ev.Host, mitm) var body *countingBody if p.transfer != nil && out.Body != nil && out.Body != http.NoBody { body = &countingBody{ReadCloser: out.Body} @@ -352,6 +362,7 @@ func (p *Proxy) relay(w http.ResponseWriter, r *http.Request, ev Event, rule Rul defer func() { _ = resp.Body.Close() }() maps.Copy(w.Header(), resp.Header) stripHop(w.Header()) + scrub(w.Header(), used) w.WriteHeader(resp.StatusCode) if streamed(resp) { received = copyFlushing(w, resp.Body) @@ -397,30 +408,49 @@ func (p *Proxy) reportTransfer(ev Event, sent, received int64) { } } -// inject sets the rule's secret, then on an intercepted request each claim credential whose header is still unset and whose placeholder, if any, the guest sent. -func (p *Proxy) inject(rule Rule, h http.Header, host string, mitm bool) string { +// inject sets the rule's secret, then on an intercepted request each claim credential where the guest asked for it; it returns the names set and the claim credentials used. +func (p *Proxy) inject(rule Rule, out *http.Request, host string, mitm bool) (string, []Credential) { if p.secrets == nil { - return "" + return "", nil } var names, set []string if rule.Secret != "" { if header, value, ok := p.secrets.Header(rule.Secret); ok && value != "" { - h.Set(header, value) + out.Header.Set(header, value) names, set = append(names, rule.Secret), append(set, header) } } if !mitm { - return strings.Join(names, ",") + return strings.Join(names, ","), nil } + var used, body []Credential for _, c := range p.secrets.Credentials(strings.ToLower(host)) { - if c.Value == "" || (c.Placeholder != "" && h.Get(c.Header) != c.Placeholder) || - slices.ContainsFunc(set, func(s string) bool { return strings.EqualFold(s, c.Header) }) { - continue + switch { + case c.Value == "": + case c.Query != "": + if c.Body { + body = append(body, c) + } + if q, ok := replaceParam(out.URL.RawQuery, c.Query, c.Placeholder, url.QueryEscape(c.Value)); ok { + out.URL.RawQuery = q + used = append(used, c) + } + case c.Placeholder != "" && out.Header.Get(c.Header) != c.Placeholder, + slices.ContainsFunc(set, func(s string) bool { return strings.EqualFold(s, c.Header) }): + default: + out.Header.Set(c.Header, c.Value) + set, used = append(set, c.Header), append(used, c) + } + } + for _, c := range substituteBody(out, body) { + if !slices.ContainsFunc(used, func(u Credential) bool { return u.Name == c.Name }) { + used = append(used, c) } - h.Set(c.Header, c.Value) - names, set = append(names, "claim:"+c.Name), append(set, c.Header) } - return strings.Join(names, ",") + for _, c := range used { + names = append(names, "claim:"+c.Name) + } + return strings.Join(names, ","), used } func (p *Proxy) record(ev Event) { @@ -488,8 +518,85 @@ func spliceable(c net.Conn) net.Conn { } func streamed(resp *http.Response) bool { - mediaType, _, _ := strings.Cut(resp.Header.Get("Content-Type"), ";") - return resp.ContentLength < 0 || strings.EqualFold(strings.TrimSpace(mediaType), "text/event-stream") + return resp.ContentLength < 0 || mediaType(resp.Header) == "text/event-stream" +} + +func mediaType(h http.Header) string { + mt, _, _ := strings.Cut(h.Get("Content-Type"), ";") + return strings.ToLower(strings.TrimSpace(mt)) +} + +// replaceParam swaps the value of each name=placeholder pair of a query or form string for escaped, leaving every other byte as sent. +func replaceParam(raw, name, placeholder, escaped string) (string, bool) { + pairs := strings.Split(raw, "&") + found := false + for i, pair := range pairs { + k, v, _ := strings.Cut(pair, "=") + if uk, err := url.QueryUnescape(k); err != nil || uk != name { + continue + } + if uv, err := url.QueryUnescape(v); err == nil && uv == placeholder { + pairs[i], found = k+"="+escaped, true + } + } + if !found { + return raw, false + } + return strings.Join(pairs, "&"), true +} + +// substituteBody swaps each credential's placeholder in a form or JSON body of known size up to maxInjectBody and returns the credentials it set. +func substituteBody(out *http.Request, creds []Credential) []Credential { + if len(creds) == 0 || out.ContentLength <= 0 || out.ContentLength > maxInjectBody { + return nil + } + mt := mediaType(out.Header) + form := mt == "application/x-www-form-urlencoded" + if !form && mt != "application/json" { + return nil + } + b, err := io.ReadAll(out.Body) + if err != nil { + out.Body = io.NopCloser(io.MultiReader(bytes.NewReader(b), out.Body)) + return nil + } + var used []Credential + for _, c := range creds { + if form { + s, ok := replaceParam(string(b), c.Query, c.Placeholder, url.QueryEscape(c.Value)) + if ok { + b, used = []byte(s), append(used, c) + } + continue + } + ph, _ := jsontext.AppendQuote(nil, c.Placeholder) + if bytes.Contains(b, ph) { + value, _ := jsontext.AppendQuote(nil, c.Value) + b, used = bytes.ReplaceAll(b, ph, value), append(used, c) + } + } + out.Body, out.ContentLength = io.NopCloser(bytes.NewReader(b)), int64(len(b)) + return used +} + +// scrub turns an echo of a used credential's value in the response headers back into its placeholder, so a redirect cannot hand the guest the value. +func scrub(h http.Header, used []Credential) { + if len(used) == 0 { + return + } + pairs := make([]string, 0, 4*len(used)) + for _, c := range used { + pairs = append(pairs, c.Value, c.Placeholder) + if c.Query != "" { + pairs = append(pairs, url.QueryEscape(c.Value), url.QueryEscape(c.Placeholder)) + } + } + r := strings.NewReplacer(pairs...) + for _, vs := range h { + for i, v := range vs { + vs[i] = r.Replace(v) + } + } } func copyFlushing(w http.ResponseWriter, body io.Reader) int64 { diff --git a/sandboxd/egress/proxy_test.go b/sandboxd/egress/proxy_test.go index 8b8b56fe..3d7568b7 100644 --- a/sandboxd/egress/proxy_test.go +++ b/sandboxd/egress/proxy_test.go @@ -155,7 +155,7 @@ func TestForwardNeverInjectsInterceptSecret(t *testing.T) { })) defer upstream.Close() - policy := Policy{Allow: []Rule{{Host: "api.internal", Secret: "gh", Intercept: true}}} + policy := Policy{Allow: []Rule{{Host: "api.internal", Secret: "gh", Intercept: InterceptAlways}}} secrets := fakeSecrets{"gh": {"Authorization", "Bearer SECRET"}} p := New(policy, secrets, nil, fixedDial(upstream.Listener.Addr().String()), nil, nil) front := httptest.NewServer(p) diff --git a/sandboxd/egress/socks_test.go b/sandboxd/egress/socks_test.go index 2e0fad94..1e0bdd00 100644 --- a/sandboxd/egress/socks_test.go +++ b/sandboxd/egress/socks_test.go @@ -47,7 +47,7 @@ func TestSocksDecisionMirrorsConnect(t *testing.T) { {"bare host", Policy{Allow: []Rule{{Host: "echo.internal"}}}, nil, false}, {"methods-restricted rule", Policy{Allow: []Rule{{Host: "echo.internal", Methods: []string{"GET"}}}}, nil, false}, {"explicit CONNECT method", Policy{Allow: []Rule{{Host: "echo.internal", Methods: []string{"CONNECT"}}}}, nil, false}, - {"intercept rule is the one exception", Policy{Allow: []Rule{{Host: "echo.internal", Intercept: true}}}, ca, true}, + {"intercept rule is the one exception", Policy{Allow: []Rule{{Host: "echo.internal", Intercept: InterceptAlways}}}, ca, true}, {"secret rule tunnels without injection", Policy{Allow: []Rule{{Host: "echo.internal", Secret: "s"}}}, nil, false}, {"unknown host", Policy{Allow: []Rule{{Host: "other.internal"}}}, nil, false}, } diff --git a/sandboxd/pool/env.go b/sandboxd/pool/env.go index 02f78ca7..55496fcb 100644 --- a/sandboxd/pool/env.go +++ b/sandboxd/pool/env.go @@ -136,7 +136,7 @@ func (m *Manager) deliverEnv(ctx context.Context, sbs []*types.Sandbox, inherite // checkEnv admits an env this node can serve: its upstream entry, and inject headers the proxy may set. func (m *Manager) checkEnv(env types.Env) error { for name, v := range env { - if v.Inject == nil { + if v.Inject == nil || v.Inject.Header == "" { continue } if err := egress.CheckInjectHeader(v.Inject.Header); err != nil { @@ -188,7 +188,7 @@ func (c claimSecrets) Header(name string) (header, value string, ok bool) { func (c claimSecrets) Credentials(host string) []egress.Credential { var out []egress.Credential for name, v := range c.sb.Injections(host) { - out = append(out, egress.Credential{Name: name, Header: v.Inject.Header, Value: v.Value, Placeholder: v.Inject.Placeholder}) + out = append(out, egress.Credential{Name: name, Header: v.Inject.Header, Query: v.Inject.Query, Body: v.Inject.Body, Value: v.Value, Placeholder: v.Inject.Placeholder}) } return out } diff --git a/sandboxd/pool/env_test.go b/sandboxd/pool/env_test.go index 6076bfb9..a83cf183 100644 --- a/sandboxd/pool/env_test.go +++ b/sandboxd/pool/env_test.go @@ -451,7 +451,7 @@ func TestPatchEnvMergesAndRepairsTheGuest(t *testing.T) { } func TestAnInjectNeedsAHostThePoolIntercepts(t *testing.T) { - pol := &egress.Policy{Allow: []egress.Rule{{Host: "api.github.com", Secret: "gh", Intercept: true}, {Host: "*.corp.test", Intercept: true}, {Host: "plain.test"}}} + pol := &egress.Policy{Allow: []egress.Rule{{Host: "api.github.com", Secret: "gh", Intercept: egress.InterceptAlways}, {Host: "*.corp.test", Intercept: egress.InterceptAlways}, {Host: "plain.test"}}} eng := newFakeEngine() eng.sockRoot = sockRoot(t) m := egressManager(t, eng, config.PoolSpec{PoolKey: testKey, Warm: 1, Egress: pol}) @@ -478,7 +478,7 @@ func TestAnInjectNeedsAHostThePoolIntercepts(t *testing.T) { } func TestAnInjectNeedsTheTenantClassToAllowTheHost(t *testing.T) { - pol := &egress.Policy{Allow: []egress.Rule{{Host: "api.github.com", Intercept: true}, {Host: "*.corp.test", Intercept: true}}} + pol := &egress.Policy{Allow: []egress.Rule{{Host: "api.github.com", Intercept: egress.InterceptAlways}, {Host: "*.corp.test", Intercept: egress.InterceptAlways}}} eng := newFakeEngine() eng.sockRoot = sockRoot(t) m := egressManager(t, eng, config.PoolSpec{PoolKey: testKey, Warm: 1, Egress: pol}) @@ -548,7 +548,7 @@ func TestInjectsSurviveARestartAndAReloadThatDropsTheInterceptStopsThem(t *testi eng := newFakeEngine() eng.sockRoot = sockRoot(t) dataDir := t.TempDir() - pool := config.PoolSpec{PoolKey: testKey, Warm: 1, Egress: &egress.Policy{Allow: []egress.Rule{{Host: "api.github.com", Intercept: true}}}} + pool := config.PoolSpec{PoolKey: testKey, Warm: 1, Egress: &egress.Policy{Allow: []egress.Rule{{Host: "api.github.com", Intercept: egress.InterceptAlways}}}} m := egressManagerAt(t, eng, dataDir, pool) env := types.Env{"KEY": {Value: "k", Guest: hostOnly, Inject: &types.EnvInject{Hosts: []string{"api.github.com"}, Header: "X-Api-Key"}}} sb, err := m.ClaimProvision(t.Context(), testKey, ClaimOptions{Env: env}) @@ -589,6 +589,21 @@ func TestAPlaceholderRidesTheEnvWriteIntoTheCredential(t *testing.T) { } } +func TestAQueryInjectOnAnInjectRuleRidesIntoTheCredential(t *testing.T) { + eng := newFakeEngine() + eng.sockRoot = sockRoot(t) + m := egressManager(t, eng, config.PoolSpec{PoolKey: testKey, Warm: 1, Egress: &egress.Policy{Allow: []egress.Rule{{Host: "*", Intercept: egress.InterceptInject}}}}) + sb := mustClaim(t, m, testKey) + patch := types.EnvPatch{"FB": {Value: "k", Guest: hostOnly, Inject: &types.EnvInject{Hosts: []string{"graph.example.com"}, Query: "access_token", Body: true, Placeholder: "@FB@"}}} + if err := m.PatchEnv(t.Context(), sb.ID, patch, ""); err != nil { + t.Fatalf("a query inject on an inject rule: %v", err) + } + creds := (claimSecrets{m: m, sb: sb}).Credentials("graph.example.com") + if len(creds) != 1 || creds[0].Query != "access_token" || !creds[0].Body || creds[0].Header != "" { + t.Errorf("credentials %+v, want one query credential with body", creds) + } +} + func TestALostInterceptRefusesOnlyAChangedInject(t *testing.T) { eng := newFakeEngine() eng.sockRoot = sockRoot(t) diff --git a/sandboxd/pool/intercept_test.go b/sandboxd/pool/intercept_test.go index ae6cf3bf..9ee8a4ab 100644 --- a/sandboxd/pool/intercept_test.go +++ b/sandboxd/pool/intercept_test.go @@ -139,5 +139,5 @@ func TestInterceptPoolAllowsPromote(t *testing.T) { } func interceptPolicy() *egress.Policy { - return &egress.Policy{Allow: []egress.Rule{{Host: "api.github.com", Secret: "gh", Intercept: true}}} + return &egress.Policy{Allow: []egress.Rule{{Host: "api.github.com", Secret: "gh", Intercept: egress.InterceptAlways}}} } diff --git a/sandboxd/pool/journal.go b/sandboxd/pool/journal.go index 0cabbed2..b4ba6d69 100644 --- a/sandboxd/pool/journal.go +++ b/sandboxd/pool/journal.go @@ -25,6 +25,7 @@ type usageEvent struct { Children []string `json:"children,omitempty"` Reference string `json:"ref,omitempty"` Upstream string `json:"upstream,omitempty"` + Secret string `json:"secret,omitempty"` Sent int64 `json:"tx,omitzero"` Received int64 `json:"rx,omitzero"` } diff --git a/sandboxd/pool/telemetry.go b/sandboxd/pool/telemetry.go index 3980d8a1..31404611 100644 --- a/sandboxd/pool/telemetry.go +++ b/sandboxd/pool/telemetry.go @@ -134,7 +134,7 @@ func (m *Manager) recordEgress(ctx context.Context, id, tenant string, ev egress decision = "allow" } m.recordAudit(ctx, id, auditFrame{Op: "egress", Dest: ev.Host, Port: ev.Port, Method: ev.Method, Decision: decision, Secret: ev.Injected, Upstream: ev.Upstream}) - m.recordUsage(ctx, usageEvent{Event: "egress", ID: id, Tenant: tenant, Reference: ev.Host, Upstream: ev.Upstream}) + m.recordUsage(ctx, usageEvent{Event: "egress", ID: id, Tenant: tenant, Reference: ev.Host, Upstream: ev.Upstream, Secret: ev.Injected}) } // recordUsage appends one billing event; failures are logged, never propagated. diff --git a/sandboxd/pool/telemetry_test.go b/sandboxd/pool/telemetry_test.go index ca6922bc..a0be0be8 100644 --- a/sandboxd/pool/telemetry_test.go +++ b/sandboxd/pool/telemetry_test.go @@ -4,6 +4,7 @@ import ( "encoding/json/v2" "errors" "maps" + "net/http" "os" "path/filepath" "slices" @@ -12,6 +13,7 @@ import ( "time" "github.com/cocoonstack/sandbox/sandboxd/config" + "github.com/cocoonstack/sandbox/sandboxd/egress" "github.com/cocoonstack/sandbox/sandboxd/types" ) @@ -325,6 +327,22 @@ func TestClaimMetadataSurvivesARestart(t *testing.T) { } } +func TestEgressUsageEventNamesTheInjectedCredentials(t *testing.T) { + m := newTestManager(t, newFakeEngine(), config.PoolSpec{PoolKey: testKey}) + m.recordEgress(t.Context(), "sb_v", "acme", egress.Event{Method: http.MethodGet, Host: "graph.example.com", Port: 443, Decision: egress.DecisionAllow, Injected: "claim:FB"}) + raw, err := os.ReadFile(filepath.Join(m.dataDir, "usage.jsonl")) + if err != nil { + t.Fatalf("read journal: %v", err) + } + var ev usageEvent + if err := json.Unmarshal(raw, &ev); err != nil { + t.Fatalf("decode %q: %v", raw, err) + } + if ev.Event != "egress" || ev.Reference != "graph.example.com" || ev.Secret != "claim:FB" { + t.Errorf("usage event %+v, want egress to graph.example.com naming claim:FB", ev) + } +} + func usageEventsOf(t *testing.T, m *Manager, id string) string { t.Helper() raw, err := os.ReadFile(filepath.Join(m.dataDir, "usage.jsonl")) diff --git a/sandboxd/pool/view.go b/sandboxd/pool/view.go index 9e630200..a50ab498 100644 --- a/sandboxd/pool/view.go +++ b/sandboxd/pool/view.go @@ -102,7 +102,7 @@ func (v *configView) secretSets(pool *egress.Policy, host, header string) bool { return false } return slices.ContainsFunc(pool.Allow, func(r egress.Rule) bool { - if !r.Intercept || r.Secret == "" || !r.Covers(host) { + if r.Intercept == egress.InterceptOff || r.Secret == "" || !r.Covers(host) { return false } h, _, known := v.secrets.Header(r.Secret) diff --git a/sandboxd/types/env_test.go b/sandboxd/types/env_test.go index 8dfacd77..d0567f85 100644 --- a/sandboxd/types/env_test.go +++ b/sandboxd/types/env_test.go @@ -101,6 +101,10 @@ func TestEnvInjectValidates(t *testing.T) { placeholder := func(p string) Env { return Env{"K": {Value: "v", Guest: new(false), Inject: &EnvInject{Hosts: []string{"a.test"}, Header: "Authorization", Placeholder: p}}} } + shaped := func(i EnvInject) Env { + i.Hosts = []string{"a.test"} + return Env{"K": {Value: "v", Guest: new(false), Inject: &i}} + } tests := []struct { name string env Env @@ -118,6 +122,12 @@ func TestEnvInjectValidates(t *testing.T) { {"placeholder with a newline", placeholder("@A@\r\nX-Evil: 1"), "inject placeholder"}, {"placeholder with surrounding space", placeholder(" @A@"), "inject placeholder"}, {"placeholder too long", placeholder(strings.Repeat("p", 257)), "inject placeholder"}, + {"query", shaped(EnvInject{Query: "api_key", Placeholder: "@K@"}), ""}, + {"query with body", shaped(EnvInject{Query: "access_token", Body: true, Placeholder: "@K@"}), ""}, + {"neither header nor query", shaped(EnvInject{Placeholder: "@K@"}), "exactly one of header and query"}, + {"header and query", shaped(EnvInject{Header: "X-Key", Query: "key", Placeholder: "@K@"}), "exactly one of header and query"}, + {"query without a placeholder", shaped(EnvInject{Query: "key"}), "query needs a placeholder"}, + {"body on a header", shaped(EnvInject{Header: "X-Key", Body: true}), "body needs query"}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -142,6 +152,11 @@ func TestEnvInjectMatchesAndCompares(t *testing.T) { if a.Equal(b) || a.Equal(c) || !a.Equal(Env{"K": {Value: "v", Guest: new(false), Inject: inj}}) || !a.SameGuest(b) { t.Error("Equal must see an inject or placeholder change, and SameGuest must not") } + q := Env{"K": {Value: "v", Guest: new(false), Inject: &EnvInject{Hosts: inj.Hosts, Query: "key", Placeholder: "@K@"}}} + qb := Env{"K": {Value: "v", Guest: new(false), Inject: &EnvInject{Hosts: inj.Hosts, Query: "key", Body: true, Placeholder: "@K@"}}} + if c.Equal(q) || q.Equal(qb) { + t.Error("Equal must see a header-to-query or body change") + } if fresh := c.InjectsChanged(a); len(fresh) != 1 { t.Errorf("InjectsChanged = %v, want the entry whose placeholder changed", fresh) } diff --git a/sandboxd/types/types.go b/sandboxd/types/types.go index 93cbd0c1..9782b6ca 100644 --- a/sandboxd/types/types.go +++ b/sandboxd/types/types.go @@ -220,10 +220,12 @@ func (md Metadata) encodedSize() int { return size } -// EnvInject sends a host-only entry's value as Header on intercepted requests to Hosts, each exact or "*." suffix; a set Placeholder limits it to requests whose Header carries exactly that value. +// EnvInject sends a host-only entry's value on intercepted requests to Hosts, exact or "*." suffix, as Header or as the Query parameter (with Body also in a small form or JSON body), only where the guest sent a set Placeholder. type EnvInject struct { Hosts []string `json:"hosts"` - Header string `json:"header"` + Header string `json:"header,omitempty"` + Query string `json:"query,omitempty"` + Body bool `json:"body,omitzero"` Placeholder string `json:"placeholder,omitempty"` } @@ -241,8 +243,15 @@ func (i *EnvInject) validate(name string) error { if len(i.Hosts) == 0 || len(i.Hosts) > maxInjectHosts { return fmt.Errorf("env %s: inject needs 1 to %d hosts, got %d", name, maxInjectHosts, len(i.Hosts)) } - if !httpguts.ValidHeaderFieldName(i.Header) { + switch { + case (i.Header == "") == (i.Query == ""): + return fmt.Errorf("env %s: inject needs exactly one of header and query", name) + case i.Header != "" && !httpguts.ValidHeaderFieldName(i.Header): return fmt.Errorf("env %s: inject header %q is not a valid header name", name, i.Header) + case i.Query != "" && i.Placeholder == "": + return fmt.Errorf("env %s: inject query needs a placeholder, so a value is never added to a request", name) + case i.Body && i.Query == "": + return fmt.Errorf("env %s: inject body needs query", name) } for _, h := range i.Hosts { if len(h) > 253 || !injectHostRe.MatchString(h) { @@ -256,7 +265,7 @@ func (i *EnvInject) validate(name string) error { } func (i *EnvInject) size() int { - n := len(i.Header) + len(i.Placeholder) + n := len(i.Header) + len(i.Query) + len(i.Placeholder) for _, h := range i.Hosts { n += len(h) } @@ -267,7 +276,7 @@ func (i *EnvInject) equal(other *EnvInject) bool { if i == nil || other == nil { return i == other } - return i.Header == other.Header && i.Placeholder == other.Placeholder && slices.Equal(i.Hosts, other.Hosts) + return i.Header == other.Header && i.Query == other.Query && i.Body == other.Body && i.Placeholder == other.Placeholder && slices.Equal(i.Hosts, other.Hosts) } // EnvVar is one entry of a claim's environment; a nil Guest means the entry reaches the guest. diff --git a/sdk/go/options.go b/sdk/go/options.go index 840f00b6..ff4ec9bf 100644 --- a/sdk/go/options.go +++ b/sdk/go/options.go @@ -47,10 +47,12 @@ type VolumeInfo struct { Writable bool `json:"writable,omitzero"` } -// EnvInject makes the node's egress proxy send a host-only entry's value as Header on intercepted requests to Hosts, each exact or "*." suffix; a set Placeholder limits it to requests whose Header carries exactly that value. +// EnvInject makes the node's egress proxy send a host-only entry's value on intercepted requests to Hosts, exact or "*." suffix, as Header or as the Query parameter (with Body also in a small form or JSON body), only where the guest sent a set Placeholder. type EnvInject struct { Hosts []string `json:"hosts"` - Header string `json:"header"` + Header string `json:"header,omitempty"` + Query string `json:"query,omitempty"` + Body bool `json:"body,omitzero"` Placeholder string `json:"placeholder,omitempty"` } From f1bee3c8d1b6c251d4104580616370c9e4dd4345 Mon Sep 17 00:00:00 2001 From: CMGS Date: Sun, 4 Oct 2026 02:44:26 +0800 Subject: [PATCH 02/11] egress: an always rule outranks an inject rule, query credentials scrub their echoes, and a JSON body fills only the named member Review of the first commit: an earlier inject rule shadowed a later always rule, dropping its secret and method gate; a JSON body filled any string equal to the placeholder, which a guest could store and read back; Graph-style paging links echoed a query key in the response body; an empty credential still intercepted; and the SOCKS door refused a host the claim injects on. The credentials leg on bare metal now covers the inject pool. --- docs/egress.md | 20 ++-- e2e/cmd/credsmoke/main.go | 113 ++++++++++++++++- sandboxd/egress/bench_test.go | 2 +- sandboxd/egress/intercept_test.go | 108 ++++++++++++++--- sandboxd/egress/policy.go | 24 ++-- sandboxd/egress/proxy.go | 193 ++++++++++++++++++++++++------ sandboxd/egress/socks.go | 2 +- sandboxd/types/types.go | 2 +- scripts/credentials-e2e.sh | 23 +++- sdk/go/options.go | 2 +- 10 files changed, 413 insertions(+), 76 deletions(-) diff --git a/docs/egress.md b/docs/egress.md index 6cbdab08..1a6be2ef 100644 --- a/docs/egress.md +++ b/docs/egress.md @@ -327,9 +327,10 @@ A key that a client sends as a query parameter takes `query` instead of `=` pair of the URL gets the value, URL-escaped; the placeholder may arrive percent-encoded, and every other byte of the query stays as sent. With `body: true`, the same parameter of an -`application/x-www-form-urlencoded` body, and every JSON string equal to the -placeholder in an `application/json` body, are replaced too, when the body has a -known length of at most 64 KiB; any other body streams unchanged: +`application/x-www-form-urlencoded` body, and every string member of that name, +at any depth, of an `application/json` body, are filled too when they carry the +placeholder and the body has a known length of at most 64 KiB; any other body +streams unchanged: ```jsonc "FB": { "value": "EAAB…", "guest": false, @@ -361,10 +362,15 @@ same `PATCH`, accepted on a hibernated or archived claim without waking it. credential naming that header for a host the rule covers is a 400. Among credentials for the same header, the first by entry name whose placeholder (if any) the request carries wins. An empty value injects nothing. -- **Echo.** On a request that carried a claim credential, an echo of its value - in a response header (a redirect's `Location` repeating the query, say) - reaches the guest as the placeholder, or blanked when there is none. - Response bodies stream unchanged. +- **Echo.** Origins reflect the request URL: a redirect's `Location`, a + paging link, an echoed query. On a request that carried a `query` + credential, its value, as sent, URL-escaped or JSON-escaped, reaches the + guest as the placeholder in every response header and in the response body. + The proxy drops the guest's `Accept-Encoding` on such a request so it reads + the body uncompressed, and streams it holding back only a tail that may begin + a match, so event streams stay live. A value the origin transforms otherwise + is beyond the scrub. `header` credentials, which origins do not reflect, are + not scrubbed and add no work to the response. - **Shape.** `hosts` holds 1 to 8 lowercase names or `*.suffix` patterns (no bare `*`, no port); exactly one of `header` and `query`; `header` is any header name except `Host`, `Content-Length` and the hop-by-hop ones; `query` diff --git a/e2e/cmd/credsmoke/main.go b/e2e/cmd/credsmoke/main.go index c8683353..98ff8dad 100644 --- a/e2e/cmd/credsmoke/main.go +++ b/e2e/cmd/credsmoke/main.go @@ -1,6 +1,7 @@ // credsmoke is the claim-credential acceptance: claims on one intercept pool each // send their own env-injected header to an HTTPS echo through the proxy, the guest // never holds the value, and rotation, refusal, clearing and forks behave as the API says. +// With -inject-size it also drives a pool whose only rule is {"host":"*","intercept":"inject"}. package main import ( @@ -22,6 +23,7 @@ const ( proxy = "http://127.0.0.1:3128" header = "X-Api-Key" entry = "API_KEY" + serpPH = "@SERP@" ) func main() { @@ -32,11 +34,16 @@ func main() { secret := flag.String("secret", "", "the pool secret's value the origin should also observe") reattach := flag.String("reattach", "", "id:token:value of a claim to recheck after a restart") sse := flag.String("sse", "", "plain HTTP event stream that sends one event, then holds") + injectSize := flag.String("inject-size", "", "size of the template's pool whose only rule is intercept \"inject\"; empty skips that leg") + other := flag.String("other", "example.com", "a second HTTPS host the inject pool must splice") flag.Parse() var err error - if *reattach != "" { + switch { + case *reattach != "": err = recheck(*addr, *token, *echo, *reattach) - } else { + case *injectSize != "": + err = conditional(*addr, *token, *template, sandbox.Size(*injectSize), *echo, *other) + default: err = run(*addr, *token, *template, *echo, *secret, *sse) } if err != nil { @@ -195,6 +202,88 @@ func recheck(addr, token, echo, reattach string) error { return nil } +func conditional(addr, token, template string, size sandbox.Size, echo, other string) error { + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute) + defer cancel() + claim := func() (*sandbox.Sandbox, error) { + _, sb, err := harness.Claim(ctx, addr, token, template, sandbox.WithNetwork(sandbox.NetNone), sandbox.WithSize(size)) + return sb, err + } + vault, err := claim() + if err != nil { + return err + } + defer func() { _ = vault.Close() }() + bare, err := claim() + if err != nil { + return err + } + defer func() { _ = bare.Close() }() + fmt.Printf(" claimed %s (vault) and %s (bare) on the inject pool\n", vault.ID, bare.ID) + for _, host := range []string{echo, other} { + if err = expectTunnel(ctx, vault, host, false); err != nil { + return fmt.Errorf("before any credential: %w", err) + } + } + fmt.Println(" with no credential both hosts splice: origin certificates, HTTP/2") + + stamp := strconv.FormatInt(time.Now().UnixNano(), 10) + serp, fb := "serp "+stamp+"/+", "fb-"+stamp + if err = vault.PatchEnv(ctx, map[string]*sandbox.EnvVar{ + "TS": {Value: "2016-10-10", Guest: new(false), Inject: &sandbox.EnvInject{Hosts: []string{echo}, Query: "timestamp", Placeholder: "@TS@"}}, + "SERP": {Value: serp, Guest: new(false), Inject: &sandbox.EnvInject{Hosts: []string{echo}, Query: "api_key", Placeholder: serpPH}}, + "FB": {Value: fb, Guest: new(false), Inject: &sandbox.EnvInject{Hosts: []string{echo}, Query: "access_token", Body: true, Placeholder: "@FB@"}}, + }); err != nil { + return fmt.Errorf("add credentials: %w", err) + } + for _, tc := range []struct { + sb *sandbox.Sandbox + host string + intercepted bool + }{{vault, echo, true}, {vault, other, false}, {bare, echo, false}} { + if err = expectTunnel(ctx, tc.sb, tc.host, tc.intercepted); err != nil { + return fmt.Errorf("after the credentials: %w", err) + } + } + fmt.Println(" the claim with credentials is intercepted on their host only; the other claim still splices it") + + form, doc := "access_token="+fb+"&m=hi", `{"access_token":"`+fb+`"}` + for _, tc := range []struct { + name, args, want, not string + }{ + {"query", fmt.Sprintf("'https://%s/time/valid?timestamp=%%40TS%%40'", echo), `"valid":true`, "2016"}, + {"other query value", fmt.Sprintf("'https://%s/time/valid?timestamp=bogus'", echo), `"valid":false`, "2016"}, + {"echoed query", fmt.Sprintf("'https://%s/get?b=2&api_key=%%40SERP%%40&a=1'", echo), serpPH, stamp}, + {"form body", fmt.Sprintf("https://%s/post -d access_token=@FB@ -d m=hi", echo), fmt.Sprintf(`"content-length":"%d"`, len(form)), fb}, + {"json body", fmt.Sprintf(`https://%s/post -H 'Content-Type: application/json' -d '{"access_token":"@FB@"}'`, echo), fmt.Sprintf(`"content-length":"%d"`, len(doc)), fb}, + {"response header echo", fmt.Sprintf("-D - -o /dev/null 'https://%s/response-headers?api_key=%%40SERP%%40'", echo), serpPH, stamp}, + } { + out, execErr := vault.Exec(ctx, "sh", "-c", fmt.Sprintf("curl -sS -x %s %s", proxy, tc.args)) + if execErr != nil { + return fmt.Errorf("%s exec: %w (%s)", tc.name, execErr, out) + } + if !strings.Contains(out, tc.want) || strings.Contains(out, tc.not) { + return fmt.Errorf("%s: guest saw %q, want %q and not %q", tc.name, out, tc.want, tc.not) + } + } + fmt.Println(" query and form/JSON body placeholders filled at the origin; every echo of a value reached the guest as its placeholder") + + leak, err := vault.Exec(ctx, "sh", "-c", fmt.Sprintf("grep -rlsF %q /proc/[0-9]*/environ /run /etc 2>/dev/null; env | grep -cF %q; true", fb, fb)) + if err != nil || strings.TrimSpace(leak) != "0" { + return fmt.Errorf("the credential reached the guest: %q %v", leak, err) + } + if err = vault.SetEnv(ctx, map[string]sandbox.EnvVar{}); err != nil { + return fmt.Errorf("clear: %w", err) + } + if err = expectTunnel(ctx, vault, echo, false); err != nil { + return fmt.Errorf("after a clear: %w", err) + } + fmt.Println(" value absent from the guest; a cleared env splices the next connection") + fmt.Printf("VAULT %s|%s\n", serp, fb) + fmt.Println("CREDSMOKE INJECT PASS") + return nil +} + func credential(echo, value string) map[string]sandbox.EnvVar { return map[string]sandbox.EnvVar{entry: {Value: value, Guest: new(false), Inject: &sandbox.EnvInject{Hosts: []string{echo}, Header: header}}} } @@ -220,6 +309,26 @@ func expectPlaceholder(ctx context.Context, sb *sandbox.Sandbox, echo, value str return nil } +// expectTunnel reads the issuer and HTTP version the guest gets from host; an intercepted tunnel shows the e2e CA and HTTP/1.1. +func expectTunnel(ctx context.Context, sb *sandbox.Sandbox, host string, intercepted bool) error { + out, err := sb.Exec(ctx, "sh", "-c", fmt.Sprintf("curl -sSv -o /dev/null -w 'version=%%{http_version}\\n' -x %s https://%s/ 2>&1", proxy, host)) + if err != nil { + return fmt.Errorf("%s exec: %w (%s)", host, err, out) + } + var issuer string + for line := range strings.SplitSeq(out, "\n") { + if _, v, ok := strings.Cut(line, "issuer:"); ok && issuer == "" { + issuer = v + } + } + mitm := strings.Contains(issuer, "e2e") + h2 := strings.Contains(out, "version=2") + if issuer == "" || mitm != intercepted || h2 == intercepted { + return fmt.Errorf("%s on %s: issuer %q, HTTP/2 %t, want intercepted %t:\n%s", host, sb.ID, issuer, h2, intercepted, out) + } + return nil +} + func expectHeader(ctx context.Context, sb *sandbox.Sandbox, echo, want, not, secret string) error { out, err := sb.Exec(ctx, "sh", "-c", fmt.Sprintf("curl -sS -x %s -H '%s: placeholder' https://%s/headers", proxy, header, echo)) if err != nil { diff --git a/sandboxd/egress/bench_test.go b/sandboxd/egress/bench_test.go index ce318fb5..ae80cd1f 100644 --- a/sandboxd/egress/bench_test.go +++ b/sandboxd/egress/bench_test.go @@ -89,7 +89,7 @@ func BenchmarkTunnelDecision(b *testing.B) { b.Run(arm.name+"/"+host, func(b *testing.B) { b.ReportAllocs() for b.Loop() { - _, _ = p.tunnelDecision(host, 443) + _, _ = p.tunnelDecision(host, 443, true) } }) } diff --git a/sandboxd/egress/intercept_test.go b/sandboxd/egress/intercept_test.go index 859afe63..2bc34259 100644 --- a/sandboxd/egress/intercept_test.go +++ b/sandboxd/egress/intercept_test.go @@ -2,8 +2,10 @@ package egress import ( "bufio" + "compress/gzip" "crypto/tls" "crypto/x509" + "encoding/hex" "fmt" "io" "net" @@ -293,14 +295,12 @@ func TestInterceptInjectsAPlaceholderCredentialOnlyWhereTheGuestSentIt(t *testin if sent != "" { headers["X-Api-Key"] = sent } - header, seen := echoTLS(t, tc, http.MethodGet, "/echo", "", "", headers) + _, seen := echoTLS(t, tc, http.MethodGet, "/echo", "", "", headers) _ = tc.Close() if seen["key"] != want { t.Errorf("guest sent %q: upstream saw %q, want %q", sent, seen["key"], want) } - if echo := header.Get("X-Key-Echo"); echo != sent { - t.Errorf("guest sent %q: the response echo reached it as %q, want what it sent", sent, echo) - } + } } @@ -323,10 +323,10 @@ func TestAnInjectRuleInterceptsOnlyForAClaimWithACredentialForTheHost(t *testing if seen["key"] != "guest" { t.Errorf("a host without a credential: upstream saw X-Api-Key %q, want the guest's own value over a splice", seen["key"]) } - if _, intercept := New(Policy{Allow: []Rule{{Host: "*", Intercept: InterceptInject}}}, nil, p.ca, fixedDial(""), nil, nil).tunnelDecision("example.com", 443); intercept { + if _, intercept := New(Policy{Allow: []Rule{{Host: "*", Intercept: InterceptInject}}}, nil, p.ca, fixedDial(""), nil, nil).tunnelDecision("example.com", 443, true); intercept { t.Error("a claim with no secrets intercepted on an inject rule") } - if d, intercept := p.tunnelDecision("plain.test", 80); d != DecisionAllow || intercept { + if d, intercept := p.tunnelDecision("plain.test", 80, true); d != DecisionAllow || intercept { t.Errorf("an inject rule without a credential: decision %v intercept %t, want a plain allow", d, intercept) } } @@ -343,18 +343,19 @@ func TestInterceptSubstitutesAQueryCredentialAndScrubsItsEcho(t *testing.T) { query, want, injected string }{ {"b=2&api_key=%40SERP%40&a=1", "b=2&api_key=real+key%2F%2B&a=1", "claim:SERP"}, + {"gzip=1&api_key=%40SERP%40", "gzip=1&api_key=real+key%2F%2B", "claim:SERP"}, {"api_key=@SERP@", "api_key=real+key%2F%2B", "claim:SERP"}, {"api_key=other&x=%40SERP%40", "api_key=other&x=%40SERP%40", ""}, {"", "", ""}, } { conn := connectTLS(t, front.Listener.Addr().String(), "example.com:443", "example.com", guestRoots) - header, seen := echoTLS(t, conn, http.MethodGet, "/echo?"+tc.query, "", "", nil) + header, seen := echoTLS(t, conn, http.MethodGet, "/echo?"+tc.query, "", "", map[string]string{"Accept-Encoding": "gzip"}) _ = conn.Close() if seen["query"] != tc.want { t.Errorf("query %q reached the upstream as %q, want %q", tc.query, seen["query"], tc.want) } - if loc := header.Get("Location"); strings.Contains(loc, "real") { - t.Errorf("query %q: the guest received Location %q holding the value", tc.query, loc) + if loc := header.Get("Location"); strings.Contains(loc, "real") || strings.Contains(seen["rawquery"], "real") { + t.Errorf("query %q: the guest received Location %q and body query %q, one holding the value", tc.query, loc, seen["rawquery"]) } _ = recvEvent(t, events) if ev := recvEvent(t, events); ev.Injected != tc.injected { @@ -378,7 +379,7 @@ func TestInterceptSubstitutesABodyCredentialInASmallFormOrJSONBody(t *testing.T) name, ctype, body, want string }{ {"form", "application/x-www-form-urlencoded", "a=1&access_token=%40FB%40&key=%40URL%40", "a=1&access_token=real+%22tok%22%26&key=%40URL%40"}, - {"json", "application/json; charset=utf-8", `{"access_token":"@FB@","n":"@FBX@"}`, `{"access_token":"real \"tok\"&","n":"@FBX@"}`}, + {"json", "application/json; charset=utf-8", `{"access_token":"@FB@", "message":"@FB@","n":{"access_token": "@FB@"},"a":["@FB@"]}`, `{"access_token":"real \"tok\"&", "message":"@FB@","n":{"access_token": "real \"tok\"&"},"a":["@FB@"]}`}, {"other type", "text/plain", "access_token=%40FB%40", "access_token=%40FB%40"}, {"oversized", "application/x-www-form-urlencoded", big, big}, } { @@ -391,6 +392,65 @@ func TestInterceptSubstitutesABodyCredentialInASmallFormOrJSONBody(t *testing.T) } } +func TestAnAlwaysRuleKeepsItsHostUnderAnEarlierInjectRule(t *testing.T) { + ca, _ := testCA(t) + policy := Policy{Allow: []Rule{{Host: "*", Intercept: InterceptInject}, {Host: "api.github.com", Methods: []string{"GET"}, Secret: "gh", Intercept: InterceptAlways}}} + creds := credSecrets{fakeSecrets: fakeSecrets{"gh": {"Authorization", "Bearer SECRET"}}, creds: map[string][]Credential{ + "api.github.com": {{Name: "K", Header: "X-Key", Value: "k"}}, + "empty.test": {{Name: "E", Header: "X-Key", Value: ""}}, + "graph.test": {{Name: "G", Query: "access_token", Value: "g", Placeholder: "@G@"}}, + }} + bare := New(policy, fakeSecrets{}, ca, fixedDial(""), nil, nil) + if _, intercept := bare.tunnelDecision("api.github.com", 443, true); !intercept { + t.Error("a claim without a credential spliced a host an always rule intercepts") + } + if rule, d := policy.EvalInner("api.github.com", http.MethodGet, 443); d != DecisionAllow || rule.Secret != "gh" { + t.Errorf("EvalInner GET = %+v/%v, want the always rule and its secret", rule, d) + } + if _, d := policy.EvalInner("api.github.com", http.MethodPost, 443); d != DecisionDeny { + t.Error("an earlier inject rule rescued a method the always rule refuses") + } + if rule, d := policy.EvalInner("graph.test", http.MethodPost, 443); d != DecisionAllow || rule.Intercept != InterceptInject { + t.Errorf("EvalInner on an inject-only host = %+v/%v, want the inject rule", rule, d) + } + p := New(policy, creds, ca, fixedDial(""), nil, nil) + if _, intercept := p.tunnelDecision("empty.test", 443, true); intercept { + t.Error("a credential with an empty value intercepted its host") + } + if _, intercept := p.tunnelDecision("graph.test", 443, true); !intercept { + t.Error("a credential's host was not intercepted on the proxy door") + } + if d, intercept := p.tunnelDecision("graph.test", 443, false); d != DecisionAllow || intercept { + t.Errorf("the SOCKS door on an inject rule: decision %v intercept %t, want a splice", d, intercept) + } +} + +func TestScrubReaderReplacesAcrossChunksAndHoldsOnlyAPossibleMatch(t *testing.T) { + olds, news := [][]byte{[]byte("SECRET")}, [][]byte{[]byte("@S@")} + for _, tc := range []struct { + name string + chunks []string + first string + all string + }{ + {"split match", []string{"a SEC", "RET b"}, "a ", "a @S@ b"}, + {"event tail", []string{"data: x\n\n", "data: SECRET\n\n"}, "data: x\n\n", "data: x\n\ndata: @S@\n\n"}, + {"prefix at the end", []string{"x SECR"}, "x ", "x SECR"}, + } { + src := &chunkReader{chunks: tc.chunks} + s := &scrubReader{src: src, olds: olds, news: news, chunk: make([]byte, 64)} + buf := make([]byte, 64) + n, _ := s.Read(buf) + if got := string(buf[:n]); got != tc.first { + t.Errorf("%s: first read %q, want %q", tc.name, got, tc.first) + } + rest, _ := io.ReadAll(s) + if got := string(buf[:n]) + string(rest); got != tc.all { + t.Errorf("%s: scrubbed %q, want %q", tc.name, got, tc.all) + } + } +} + func TestInterceptStreamsAnEventStreamAsItArrives(t *testing.T) { release := make(chan struct{}) upstream := httptest.NewTLSServer(sseHandler(release)) @@ -451,9 +511,15 @@ func interceptProxySecrets(t *testing.T, policy Policy, secrets Secrets, events return } body, _ := io.ReadAll(r.Body) + var out io.Writer = w + if strings.Contains(r.Header.Get("Accept-Encoding"), "gzip") { + w.Header().Set("Content-Encoding", "gzip") + gz := gzip.NewWriter(w) + defer func() { _ = gz.Close() }() + out = gz + } w.Header().Set("Location", "/next?"+r.URL.RawQuery) - w.Header().Set("X-Key-Echo", r.Header.Get("X-Api-Key")) - _, _ = fmt.Fprintf(w, "key=%s\nempty=%s\nquery=%s\nlength=%d\nbody=%s", r.Header.Get("X-Api-Key"), r.Header.Get("X-Empty"), r.URL.RawQuery, r.ContentLength, body) + _, _ = fmt.Fprintf(out, "key=%x\nempty=%x\nquery=%x\nbody=%x\nlength=%d\nrawquery=%s", r.Header.Get("X-Api-Key"), r.Header.Get("X-Empty"), r.URL.RawQuery, body, r.ContentLength, r.URL.RawQuery) })) t.Cleanup(upstream.Close) @@ -542,7 +608,6 @@ func roundTripTLSHost(t *testing.T, tc *tls.Conn, method, host string) *http.Res return resp } -// echoTLS sends one request on tc to the upstream's /echo, which reports what it saw in the body. func echoTLS(t *testing.T, tc *tls.Conn, method, target, ctype, body string, headers map[string]string) (http.Header, map[string]string) { t.Helper() req, err := http.NewRequestWithContext(t.Context(), method, "https://example.com"+target, strings.NewReader(body)) @@ -566,13 +631,26 @@ func echoTLS(t *testing.T, tc *tls.Conn, method, target, ctype, body string, hea _ = resp.Body.Close() seen := map[string]string{} for line := range strings.SplitSeq(string(raw), "\n") { - if k, v, ok := strings.Cut(line, "="); ok && seen[k] == "" { - seen[k] = v + k, v, _ := strings.Cut(line, "=") + if b, err := hex.DecodeString(v); err == nil && k != "length" { + v = string(b) } + seen[k] = v } return resp.Header, seen } +type chunkReader struct{ chunks []string } + +func (c *chunkReader) Read(b []byte) (int, error) { + if len(c.chunks) == 0 { + return 0, io.EOF + } + n := copy(b, c.chunks[0]) + c.chunks = c.chunks[1:] + return n, nil +} + func readPreamble(tb testing.TB, conn net.Conn) string { tb.Helper() var sb strings.Builder diff --git a/sandboxd/egress/policy.go b/sandboxd/egress/policy.go index 06b721cd..38deac6b 100644 --- a/sandboxd/egress/policy.go +++ b/sandboxd/egress/policy.go @@ -146,30 +146,36 @@ func (p Policy) Eval(host, method string, port uint16) (Rule, Decision) { return Rule{}, DecisionDeny } -// EvalHost matches by host and port only, preferring an intercept rule over an earlier plain match. +// EvalHost matches by host and port only, preferring an always-intercept rule, then an inject rule, over an earlier plain match. func (p Policy) EvalHost(host string, port uint16) (Rule, Decision) { host = strings.ToLower(host) - first := -1 + best := -1 for i, r := range p.Allow { switch { case !r.matchHost(host) || !r.matchPort(port): - case r.Intercept != InterceptOff: + case r.Intercept == InterceptAlways: return r, DecisionAllow - case first < 0: - first = i + case best < 0, r.Intercept == InterceptInject && p.Allow[best].Intercept == InterceptOff: + best = i } } - if first >= 0 { - return p.Allow[first], DecisionAllow + if best >= 0 { + return p.Allow[best], DecisionAllow } return Rule{}, DecisionDeny } -// EvalInner matches only intercept rules, so a plain rule cannot shadow or rescue one. +// EvalInner matches only intercept rules, so a plain rule cannot shadow or rescue one; an always-intercept rule for the host shadows every inject rule. func (p Policy) EvalInner(host, method string, port uint16) (Rule, Decision) { host = strings.ToLower(host) + mode := InterceptInject + if slices.ContainsFunc(p.Allow, func(r Rule) bool { + return r.Intercept == InterceptAlways && r.matchHost(host) && r.matchPort(port) + }) { + mode = InterceptAlways + } for _, r := range p.Allow { - if r.Intercept != InterceptOff && r.matches(host, method, port) { + if r.Intercept == mode && r.matches(host, method, port) { return r, DecisionAllow } } diff --git a/sandboxd/egress/proxy.go b/sandboxd/egress/proxy.go index 0c839b12..b79a491b 100644 --- a/sandboxd/egress/proxy.go +++ b/sandboxd/egress/proxy.go @@ -32,6 +32,7 @@ const ( // maxIdleConns bounds the upstream pool per sandbox; a guest walking many hosts cannot park a descriptor per host. maxIdleConns = 64 maxInjectBody = 64 << 10 + scrubChunk = 8 << 10 ) var ( @@ -50,7 +51,7 @@ type TransferFunc func(ev Event, sent, received int64) // AuditFunc receives every egress decision the proxy takes. type AuditFunc func(Event) -// Credential is a claim-supplied value the proxy sets on an intercepted request as Header, or in place of the Placeholder value of Query (and with Body in the request body); a set Placeholder must be what the guest sent. +// Credential is a claim-supplied value the proxy sets on an intercepted request where the guest sent its Placeholder. type Credential struct { Name string Header string @@ -232,7 +233,7 @@ func (p *Proxy) serveConnect(w http.ResponseWriter, r *http.Request) { denied(w, host) return } - decision, intercept := p.tunnelDecision(host, port) + decision, intercept := p.tunnelDecision(host, port, true) ev, route := p.routeEvent(Event{Method: r.Method, Host: host, Port: port, Decision: decision}) if intercept { p.record(ev) @@ -278,13 +279,13 @@ func (p *Proxy) serveConnect(w http.ResponseWriter, r *http.Request) { p.reportTransfer(ev, early+sent, received) } -func (p *Proxy) tunnelDecision(host string, port uint16) (decision Decision, intercept bool) { +// tunnelDecision gates a tunnel by host; injects is false on a door that never injects, where an inject rule is a plain one. +func (p *Proxy) tunnelDecision(host string, port uint16, injects bool) (decision Decision, intercept bool) { // host-gate interception: the tunnel's CONNECT verb is not the request method. if p.ca != nil { switch rule, d := p.policy.EvalHost(host, port); { case d != DecisionAllow: - case rule.Intercept == InterceptAlways, - rule.Intercept == InterceptInject && p.secrets != nil && len(p.secrets.Credentials(strings.ToLower(host))) > 0: + case rule.Intercept == InterceptAlways, rule.Intercept == InterceptInject && injects && p.holdsCredential(host): return DecisionAllow, true } } @@ -292,6 +293,10 @@ func (p *Proxy) tunnelDecision(host string, port uint16) (decision Decision, int return decision, false } +func (p *Proxy) holdsCredential(host string) bool { + return p.secrets != nil && slices.ContainsFunc(p.secrets.Credentials(strings.ToLower(host)), func(c Credential) bool { return c.Value != "" }) +} + func (p *Proxy) serveForward(w http.ResponseWriter, r *http.Request) { if !r.URL.IsAbs() { http.Error(w, "egress: proxy requires an absolute-form request URI", http.StatusBadRequest) @@ -336,6 +341,11 @@ func (p *Proxy) relay(w http.ResponseWriter, r *http.Request, ev Event, rule Rul stripHop(out.Header) var used []Credential ev.Injected, used = p.inject(rule, out, ev.Host, mitm) + olds, news := echoes(used) + if len(olds) > 0 { + // the transport then negotiates and decodes compression itself, so the body reaches the scrub as plain bytes. + out.Header.Del("Accept-Encoding") + } var body *countingBody if p.transfer != nil && out.Body != nil && out.Body != http.NoBody { body = &countingBody{ReadCloser: out.Body} @@ -362,13 +372,18 @@ func (p *Proxy) relay(w http.ResponseWriter, r *http.Request, ev Event, rule Rul defer func() { _ = resp.Body.Close() }() maps.Copy(w.Header(), resp.Header) stripHop(w.Header()) - scrub(w.Header(), used) + var src io.Reader = resp.Body + if len(olds) > 0 { + scrubHeader(w.Header(), olds, news) + w.Header().Del("Content-Length") + src = &scrubReader{src: resp.Body, olds: olds, news: news, chunk: make([]byte, scrubChunk)} + } w.WriteHeader(resp.StatusCode) if streamed(resp) { - received = copyFlushing(w, resp.Body) + received = copyFlushing(w, src) return } - received, _ = io.Copy(w, resp.Body) + received, _ = io.Copy(w, src) } // transport returns the pool for route; the direct route keeps the base transports. @@ -408,7 +423,7 @@ func (p *Proxy) reportTransfer(ev Event, sent, received int64) { } } -// inject sets the rule's secret, then on an intercepted request each claim credential where the guest asked for it; it returns the names set and the claim credentials used. +// inject sets the rule's secret, then on an intercepted request the claim credentials the guest asked for, and returns the names set and the credentials used. func (p *Proxy) inject(rule Rule, out *http.Request, host string, mitm bool) (string, []Credential) { if p.secrets == nil { return "", nil @@ -431,7 +446,7 @@ func (p *Proxy) inject(rule Rule, out *http.Request, host string, mitm bool) (st if c.Body { body = append(body, c) } - if q, ok := replaceParam(out.URL.RawQuery, c.Query, c.Placeholder, url.QueryEscape(c.Value)); ok { + if q, ok := replaceParam(out.URL.RawQuery, c.Query, c.Placeholder, c.Value); ok { out.URL.RawQuery = q used = append(used, c) } @@ -471,6 +486,68 @@ func (c *countingBody) Read(b []byte) (int, error) { return n, err } +// scrubReader replaces each of olds with its news as a response body streams; it holds back only a tail that may begin a match. +type scrubReader struct { + src io.Reader + olds, news [][]byte + chunk []byte + pending []byte + out []byte + off int + err error +} + +func (s *scrubReader) Read(b []byte) (int, error) { + for s.off == len(s.out) && s.err == nil { + s.out, s.off = s.out[:0], 0 + n, err := s.src.Read(s.chunk) + s.pending, s.err = append(s.pending, s.chunk[:n]...), err + s.scan() + } + if s.off == len(s.out) { + return 0, s.err + } + n := copy(b, s.out[s.off:]) + s.off += n + return n, nil +} + +func (s *scrubReader) scan() { + buf, i := s.pending, 0 + for { + at, k := -1, 0 + for j, old := range s.olds { + if idx := bytes.Index(buf[i:], old); idx >= 0 && (at < 0 || idx < at) { + at, k = idx, j + } + } + if at < 0 { + break + } + s.out = append(append(s.out, buf[i:i+at]...), s.news[k]...) + i += at + len(s.olds[k]) + } + hold := 0 + if s.err == nil { + hold = s.partial(buf[i:]) + } + s.out = append(s.out, buf[i:len(buf)-hold]...) + s.pending = s.pending[:copy(s.pending, buf[len(buf)-hold:])] +} + +func (s *scrubReader) partial(tail []byte) int { + longest := 0 + for _, old := range s.olds { + longest = max(longest, len(old)) + } + for k := min(len(tail), longest-1); k > 0; k-- { + if slices.ContainsFunc(s.olds, func(old []byte) bool { return len(old) > k && bytes.HasPrefix(old, tail[len(tail)-k:]) }) { + return k + } + } + return 0 +} + func stripHop(h http.Header) { // Connection may name additional hop headers this hop must consume. for _, v := range h.Values("Connection") { @@ -526,26 +603,62 @@ func mediaType(h http.Header) string { return strings.ToLower(strings.TrimSpace(mt)) } -// replaceParam swaps the value of each name=placeholder pair of a query or form string for escaped, leaving every other byte as sent. -func replaceParam(raw, name, placeholder, escaped string) (string, bool) { +// replaceParam swaps the value of each name=placeholder pair of a query or form string for the escaped value, leaving every other byte as sent. +func replaceParam(raw, name, placeholder, value string) (string, bool) { + if raw == "" { + return raw, false + } pairs := strings.Split(raw, "&") - found := false + escaped := "" for i, pair := range pairs { k, v, _ := strings.Cut(pair, "=") if uk, err := url.QueryUnescape(k); err != nil || uk != name { continue } if uv, err := url.QueryUnescape(v); err == nil && uv == placeholder { - pairs[i], found = k+"="+escaped, true + escaped = cmp.Or(escaped, url.QueryEscape(value)) + pairs[i] = k + "=" + escaped } } - if !found { + if escaped == "" { return raw, false } return strings.Join(pairs, "&"), true } -// substituteBody swaps each credential's placeholder in a form or JSON body of known size up to maxInjectBody and returns the credentials it set. +// replaceMember swaps each string member named name whose value is placeholder, at any depth, leaving every other byte as sent. +func replaceMember(b []byte, name, placeholder, value string) ([]byte, bool) { + dec := jsontext.NewDecoder(bytes.NewReader(b)) + var ends []int64 + var lens []int + for { + kind, n := dec.StackIndex(dec.StackDepth()) + if kind != '{' || n%2 == 0 || dec.PeekKind() != '"' { + if _, err := dec.ReadToken(); err != nil { + break + } + continue + } + raw, err := dec.ReadValue() + if err != nil { + break + } + if v, err := jsontext.AppendUnquote(nil, raw); err == nil && string(v) == placeholder && dec.StackPointer().LastToken() == name { + ends, lens = append(ends, dec.InputOffset()), append(lens, len(raw)) + } + } + if len(ends) == 0 { + return b, false + } + quoted, _ := jsontext.AppendQuote(nil, value) + out, at := make([]byte, 0, len(b)+len(ends)*len(quoted)), int64(0) + for i, end := range ends { + out = append(append(out, b[at:end-int64(lens[i])]...), quoted...) + at = end + } + return append(out, b[at:]...), true +} + func substituteBody(out *http.Request, creds []Credential) []Credential { if len(creds) == 0 || out.ContentLength <= 0 || out.ContentLength > maxInjectBody { return nil @@ -555,41 +668,49 @@ func substituteBody(out *http.Request, creds []Credential) []Credential { if !form && mt != "application/json" { return nil } - b, err := io.ReadAll(out.Body) - if err != nil { - out.Body = io.NopCloser(io.MultiReader(bytes.NewReader(b), out.Body)) + b := make([]byte, out.ContentLength) + if n, err := io.ReadFull(out.Body, b); err != nil { + out.Body = io.NopCloser(io.MultiReader(bytes.NewReader(b[:n]), out.Body)) return nil } var used []Credential for _, c := range creds { if form { - s, ok := replaceParam(string(b), c.Query, c.Placeholder, url.QueryEscape(c.Value)) - if ok { + if s, ok := replaceParam(string(b), c.Query, c.Placeholder, c.Value); ok { b, used = []byte(s), append(used, c) } - continue - } - ph, _ := jsontext.AppendQuote(nil, c.Placeholder) - if bytes.Contains(b, ph) { - value, _ := jsontext.AppendQuote(nil, c.Value) - b, used = bytes.ReplaceAll(b, ph, value), append(used, c) + } else if nb, ok := replaceMember(b, c.Query, c.Placeholder, c.Value); ok { + b, used = nb, append(used, c) } } out.Body, out.ContentLength = io.NopCloser(bytes.NewReader(b)), int64(len(b)) return used } -// scrub turns an echo of a used credential's value in the response headers back into its placeholder, so a redirect cannot hand the guest the value. -func scrub(h http.Header, used []Credential) { - if len(used) == 0 { - return - } - pairs := make([]string, 0, 4*len(used)) +// echoes pairs each used query credential's value, as sent, URL-escaped and JSON-escaped, with its placeholder in the same form, so a response that reflects the URL cannot hand the guest the value. +func echoes(used []Credential) (olds, news [][]byte) { for _, c := range used { - pairs = append(pairs, c.Value, c.Placeholder) - if c.Query != "" { - pairs = append(pairs, url.QueryEscape(c.Value), url.QueryEscape(c.Placeholder)) + if c.Query == "" { + continue } + pairs := [][2]string{{c.Value, c.Placeholder}, {url.QueryEscape(c.Value), url.QueryEscape(c.Placeholder)}} + if v, err := jsontext.AppendQuote(nil, c.Value); err == nil { + ph, _ := jsontext.AppendQuote(nil, c.Placeholder) + pairs = append(pairs, [2]string{string(v[1 : len(v)-1]), string(ph[1 : len(ph)-1])}) + } + for _, p := range pairs { + if !slices.ContainsFunc(olds, func(o []byte) bool { return string(o) == p[0] }) { + olds, news = append(olds, []byte(p[0])), append(news, []byte(p[1])) + } + } + } + return olds, news +} + +func scrubHeader(h http.Header, olds, news [][]byte) { + pairs := make([]string, 0, 2*len(olds)) + for i := range olds { + pairs = append(pairs, string(olds[i]), string(news[i])) } r := strings.NewReplacer(pairs...) for _, vs := range h { diff --git a/sandboxd/egress/socks.go b/sandboxd/egress/socks.go index b5c7b9bc..e71543fd 100644 --- a/sandboxd/egress/socks.go +++ b/sandboxd/egress/socks.go @@ -60,7 +60,7 @@ func (p *Proxy) serveSocks(ctx context.Context, conn net.Conn) { if !ok { return } - decision, intercept := p.tunnelDecision(host, port) + decision, intercept := p.tunnelDecision(host, port, false) if intercept || port == 0 { decision = DecisionDeny } diff --git a/sandboxd/types/types.go b/sandboxd/types/types.go index 9782b6ca..226a1f73 100644 --- a/sandboxd/types/types.go +++ b/sandboxd/types/types.go @@ -220,7 +220,7 @@ func (md Metadata) encodedSize() int { return size } -// EnvInject sends a host-only entry's value on intercepted requests to Hosts, exact or "*." suffix, as Header or as the Query parameter (with Body also in a small form or JSON body), only where the guest sent a set Placeholder. +// EnvInject sends a host-only entry's value to Hosts, exact or "*." suffix, as Header or as the Query parameter, where the guest sent a set Placeholder. type EnvInject struct { Hosts []string `json:"hosts"` Header string `json:"header,omitempty"` diff --git a/scripts/credentials-e2e.sh b/scripts/credentials-e2e.sh index eaf14822..51bd8e15 100755 --- a/scripts/credentials-e2e.sh +++ b/scripts/credentials-e2e.sh @@ -2,7 +2,9 @@ # Bare-metal acceptance for claim credentials: claims on one intercept pool each # send their own env-injected header to an HTTPS echo, the guest never holds the # value, an open event stream reaches the guest event by event, and a credential -# survives a sandboxd restart. Runs a dedicated sandboxd on $ADDR. +# survives a sandboxd restart; a pool whose only rule is intercept "inject" +# intercepts only the claims that hold a credential for a host and fills query +# and body placeholders. Runs a dedicated sandboxd on $ADDR. # Needs outbound to $ECHO and a $TEMPLATE whose silkd binds the egress relay. set -euo pipefail @@ -10,6 +12,7 @@ TEMPLATE=${TEMPLATE:-rt:24.04} ADDR=${ADDR:-127.0.0.1:7882} TOKEN=${TOKEN:-e2e} ECHO=${ECHO:-postman-echo.com} +OTHER=${OTHER:-example.com} SSE_PORT=${SSE_PORT:-18998} REPO=$(cd "$(dirname "$0")/.." && pwd) PROBE=${EGRESS_PROBE_TOKEN:-probe-$(date +%s)} @@ -81,7 +84,9 @@ cat >"$DATA/config.json" </dev/null | - jq -e '.pools[0].warm >= 1' >/dev/null 2>&1 && break + jq -e 'all(.pools[]; .warm >= 1)' >/dev/null 2>&1 && break [[ $i == 300 ]] && { echo "pool never became warm"; exit 1; } sleep 1 done @@ -115,6 +120,18 @@ if grep -q "$value" "$DATA/state/audit.jsonl" "$DATA/state/usage.jsonl" 2>/dev/n fi grep -q "$value" "$DATA/state/claims.json" || { echo "claims.json lost the credential"; exit 1; } +echo "== inject pool: conditional interception, query and body placeholders" +"$DATA/credsmoke" -addr "$ADDR" -token "$TOKEN" -template "$TEMPLATE" -echo "$ECHO" -other "$OTHER" \ + -inject-size medium | tee "$DATA/inject.log" +grep -q '"ev":"egress".*"secret":"claim:SERP"' "$DATA/state/usage.jsonl" || { echo "usage names no claim:SERP"; exit 1; } +vault=$(awk '/^VAULT / {sub(/^VAULT /, ""); print}' "$DATA/inject.log") +for v in "${vault%%|*}" "${vault##*|}"; do + if grep '"op":"egress"' "$DATA/state/audit.jsonl" | grep -qF "$v" || grep -qF "$v" "$DATA/state/usage.jsonl"; then + echo "an inject value reached an egress audit record or the usage journal" + exit 1 + fi +done + echo "== restart sandboxd, recheck the surviving claim" kill "$DAEMON_PID" wait "$DAEMON_PID" 2>/dev/null || true diff --git a/sdk/go/options.go b/sdk/go/options.go index ff4ec9bf..20b75401 100644 --- a/sdk/go/options.go +++ b/sdk/go/options.go @@ -47,7 +47,7 @@ type VolumeInfo struct { Writable bool `json:"writable,omitzero"` } -// EnvInject makes the node's egress proxy send a host-only entry's value on intercepted requests to Hosts, exact or "*." suffix, as Header or as the Query parameter (with Body also in a small form or JSON body), only where the guest sent a set Placeholder. +// EnvInject makes the node's egress proxy send a host-only entry's value to Hosts, exact or "*." suffix, as Header or as the Query parameter, where the guest sent a set Placeholder. type EnvInject struct { Hosts []string `json:"hosts"` Header string `json:"header,omitempty"` From 4c3a0125c223d3eda4955b5d0cb1e631108938a0 Mon Sep 17 00:00:00 2001 From: CMGS Date: Sun, 4 Oct 2026 02:59:07 +0800 Subject: [PATCH 03/11] egress: a scrubbed query value cannot be split by a range, its path and \/ escapings are scrubbed, and a JSON body fills only top-level members A guest could fetch a reflected value in pieces with Range, each piece too short to match; the proxy now drops Range and If-Range with Accept-Encoding on a request that carried a query credential. encodeURIComponent-style and PHP/Graph-style JSON echoes escaped past the scrub. Filling a nested JSON member let the guest choose where the value lands. --- docs/egress.md | 23 +++++++++------ sandboxd/egress/intercept_test.go | 48 ++++++++++++++++++++++++++++++- sandboxd/egress/proxy.go | 22 +++++++++----- 3 files changed, 76 insertions(+), 17 deletions(-) diff --git a/docs/egress.md b/docs/egress.md index 1a6be2ef..cb8f9e5e 100644 --- a/docs/egress.md +++ b/docs/egress.md @@ -327,8 +327,8 @@ A key that a client sends as a query parameter takes `query` instead of `=` pair of the URL gets the value, URL-escaped; the placeholder may arrive percent-encoded, and every other byte of the query stays as sent. With `body: true`, the same parameter of an -`application/x-www-form-urlencoded` body, and every string member of that name, -at any depth, of an `application/json` body, are filled too when they carry the +`application/x-www-form-urlencoded` body, and a top-level string member of that +name in an `application/json` body, are filled too when they carry the placeholder and the body has a known length of at most 64 KiB; any other body streams unchanged: @@ -357,19 +357,24 @@ same `PATCH`, accepted on a hibernated or archived claim without waking it. credential: every host must be covered by an `intercept` rule (`true` or `"inject"`) of the claim's pool and allowed by its tenant class, or the write answers 400 and nothing is stored. A credential never widens egress and never rides a - plaintext forward request. + plaintext forward request. Keep a credential's `hosts` to the vendor's own + names: under a `*` rule, a pattern such as `*.amazonaws.com` or + `*.vercel.app` also matches subdomains a guest can own. - **Precedence.** The pool rule's own `secret` is set first and wins: a credential naming that header for a host the rule covers is a 400. Among credentials for the same header, the first by entry name whose placeholder (if any) the request carries wins. An empty value injects nothing. - **Echo.** Origins reflect the request URL: a redirect's `Location`, a paging link, an echoed query. On a request that carried a `query` - credential, its value, as sent, URL-escaped or JSON-escaped, reaches the - guest as the placeholder in every response header and in the response body. - The proxy drops the guest's `Accept-Encoding` on such a request so it reads - the body uncompressed, and streams it holding back only a tail that may begin - a match, so event streams stay live. A value the origin transforms otherwise - is beyond the scrub. `header` credentials, which origins do not reflect, are + credential, its value, as sent, query- or path-escaped, or JSON-escaped + (with or without `\/`), reaches the guest as the placeholder in every + response header and in the response body. The proxy drops the guest's + `Accept-Encoding`, `Range` and `If-Range` on such a request, so it reads the + whole body uncompressed, and streams it holding back only a tail that may + begin a match, so event streams stay live. A value the origin transforms + otherwise is beyond the scrub, and so is a value the origin stored earlier + and returns on a later request that carries no credential: the scrub covers + only the response to the request that sent the value. `header` credentials, which origins do not reflect, are not scrubbed and add no work to the response. - **Shape.** `hosts` holds 1 to 8 lowercase names or `*.suffix` patterns (no bare `*`, no port); exactly one of `header` and `query`; `header` is any diff --git a/sandboxd/egress/intercept_test.go b/sandboxd/egress/intercept_test.go index 2bc34259..3f768a0d 100644 --- a/sandboxd/egress/intercept_test.go +++ b/sandboxd/egress/intercept_test.go @@ -11,6 +11,7 @@ import ( "net" "net/http" "net/http/httptest" + "net/url" "strings" "testing" "time" @@ -379,7 +380,7 @@ func TestInterceptSubstitutesABodyCredentialInASmallFormOrJSONBody(t *testing.T) name, ctype, body, want string }{ {"form", "application/x-www-form-urlencoded", "a=1&access_token=%40FB%40&key=%40URL%40", "a=1&access_token=real+%22tok%22%26&key=%40URL%40"}, - {"json", "application/json; charset=utf-8", `{"access_token":"@FB@", "message":"@FB@","n":{"access_token": "@FB@"},"a":["@FB@"]}`, `{"access_token":"real \"tok\"&", "message":"@FB@","n":{"access_token": "real \"tok\"&"},"a":["@FB@"]}`}, + {"json", "application/json; charset=utf-8", `{"access_token":"@FB@", "message":"@FB@","n":{"access_token": "@FB@"},"a":["@FB@"]}`, `{"access_token":"real \"tok\"&", "message":"@FB@","n":{"access_token": "@FB@"},"a":["@FB@"]}`}, {"other type", "text/plain", "access_token=%40FB%40", "access_token=%40FB%40"}, {"oversized", "application/x-www-form-urlencoded", big, big}, } { @@ -425,6 +426,47 @@ func TestAnAlwaysRuleKeepsItsHostUnderAnEarlierInjectRule(t *testing.T) { } } +func TestARangeCannotSplitAScrubbedQueryValue(t *testing.T) { + secrets := credSecrets{creds: map[string][]Credential{"example.com": {{Name: "K", Query: "api_key", Value: "SECRETVALUE0123456789", Placeholder: "@K@"}}}} + p, guestRoots, upstream := interceptProxySecrets(t, Policy{Allow: []Rule{{Host: "*", Intercept: InterceptInject}}}, secrets, nil) + p.pools.Load().mitm.TLSClientConfig.RootCAs = trustUpstream(upstream) + front := httptest.NewServer(p) + defer front.Close() + + for _, r := range []string{"bytes=0-9", "bytes=10-20", "bytes=0-3,10-13"} { + conn := connectTLS(t, front.Listener.Addr().String(), "example.com:443", "example.com", guestRoots) + req, err := http.NewRequestWithContext(t.Context(), http.MethodGet, "https://example.com/range?api_key=%40K%40", nil) + if err != nil { + t.Fatalf("build request: %v", err) + } + req.Header.Set("Range", r) + if err = req.Write(conn); err != nil { + t.Fatalf("write request: %v", err) + } + resp, err := http.ReadResponse(bufio.NewReader(conn), req) + if err != nil { + t.Fatalf("read response: %v", err) + } + body, _ := io.ReadAll(resp.Body) + _ = resp.Body.Close() + _ = conn.Close() + if resp.StatusCode != http.StatusOK || string(body) != "@K@" { + t.Errorf("Range %s: guest got %d %q, want the whole value scrubbed to its placeholder", r, resp.StatusCode, body) + } + } +} + +func TestEchoesCoverTheCommonEscapings(t *testing.T) { + value := "ab cd/ef 0 { - // the transport then negotiates and decodes compression itself, so the body reaches the scrub as plain bytes. - out.Header.Del("Accept-Encoding") + // the transport then decodes compression itself, and no range splits a value, so the scrub sees every echo whole. + for _, h := range []string{"Accept-Encoding", "Range", "If-Range"} { + out.Header.Del(h) + } } var body *countingBody if p.transfer != nil && out.Body != nil && out.Body != http.NoBody { @@ -626,14 +628,14 @@ func replaceParam(raw, name, placeholder, value string) (string, bool) { return strings.Join(pairs, "&"), true } -// replaceMember swaps each string member named name whose value is placeholder, at any depth, leaving every other byte as sent. +// replaceMember swaps each top-level string member named name whose value is placeholder, leaving every other byte as sent. func replaceMember(b []byte, name, placeholder, value string) ([]byte, bool) { dec := jsontext.NewDecoder(bytes.NewReader(b)) var ends []int64 var lens []int for { kind, n := dec.StackIndex(dec.StackDepth()) - if kind != '{' || n%2 == 0 || dec.PeekKind() != '"' { + if dec.StackDepth() != 1 || kind != '{' || n%2 == 0 || dec.PeekKind() != '"' { if _, err := dec.ReadToken(); err != nil { break } @@ -687,16 +689,22 @@ func substituteBody(out *http.Request, creds []Credential) []Credential { return used } -// echoes pairs each used query credential's value, as sent, URL-escaped and JSON-escaped, with its placeholder in the same form, so a response that reflects the URL cannot hand the guest the value. +// echoes pairs each used query credential's value, as sent, URL-escaped and JSON-escaped in their common forms, with its placeholder in the same form, so a response that reflects the URL cannot hand the guest the value. func echoes(used []Credential) (olds, news [][]byte) { for _, c := range used { if c.Query == "" { continue } - pairs := [][2]string{{c.Value, c.Placeholder}, {url.QueryEscape(c.Value), url.QueryEscape(c.Placeholder)}} + pairs := [][2]string{ + {c.Value, c.Placeholder}, + {url.QueryEscape(c.Value), url.QueryEscape(c.Placeholder)}, + {url.PathEscape(c.Value), url.PathEscape(c.Placeholder)}, + } if v, err := jsontext.AppendQuote(nil, c.Value); err == nil { ph, _ := jsontext.AppendQuote(nil, c.Placeholder) - pairs = append(pairs, [2]string{string(v[1 : len(v)-1]), string(ph[1 : len(ph)-1])}) + v, ph = v[1:len(v)-1], ph[1:len(ph)-1] + pairs = append(pairs, [2]string{string(v), string(ph)}, + [2]string{strings.ReplaceAll(string(v), "/", `\/`), strings.ReplaceAll(string(ph), "/", `\/`)}) } for _, p := range pairs { if !slices.ContainsFunc(olds, func(o []byte) bool { return string(o) == p[0] }) { From 539c2d4037dce29aeaf7954fbcbf7d5dbb111a55 Mon Sep 17 00:00:00 2001 From: CMGS Date: Sun, 4 Oct 2026 11:03:19 +0800 Subject: [PATCH 04/11] egress: a credential fills only its first placeholder in a query, form or JSON body A guest repeating the placeholder made the proxy expand every copy into the full value before sending: 3600 pairs in a 64 KiB form with an 8 KiB credential allocated about 29 MB on the host per request. Filling the first match bounds the growth by the value length. --- sandboxd/egress/intercept_test.go | 12 ++++++++++++ sandboxd/egress/proxy.go | 8 +++++--- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/sandboxd/egress/intercept_test.go b/sandboxd/egress/intercept_test.go index 3f768a0d..1a277da7 100644 --- a/sandboxd/egress/intercept_test.go +++ b/sandboxd/egress/intercept_test.go @@ -426,6 +426,18 @@ func TestAnAlwaysRuleKeepsItsHostUnderAnEarlierInjectRule(t *testing.T) { } } +func TestACredentialFillsOnlyTheFirstMatchingParameter(t *testing.T) { + value := strings.Repeat("v", 8<<10) + form := strings.Repeat("access_token=%40FB%40&", 3600) + if got, _ := replaceParam(form, "access_token", "@FB@", value); len(got) != len(form)-len("%40FB%40")+len(value) { + t.Errorf("3600 repeated pairs grew the form to %d bytes, want one fill on %d", len(got), len(form)) + } + doc := `{"access_token":"@FB@","access_token":"@FB@"}` + if got, _ := replaceMember([]byte(doc), "access_token", "@FB@", "v"); string(got) != `{"access_token":"v","access_token":"@FB@"}` { + t.Errorf("a repeated member filled as %s, want only the first", got) + } +} + func TestARangeCannotSplitAScrubbedQueryValue(t *testing.T) { secrets := credSecrets{creds: map[string][]Credential{"example.com": {{Name: "K", Query: "api_key", Value: "SECRETVALUE0123456789", Placeholder: "@K@"}}}} p, guestRoots, upstream := interceptProxySecrets(t, Policy{Allow: []Rule{{Host: "*", Intercept: InterceptInject}}}, secrets, nil) diff --git a/sandboxd/egress/proxy.go b/sandboxd/egress/proxy.go index 1d51cb93..beb02e12 100644 --- a/sandboxd/egress/proxy.go +++ b/sandboxd/egress/proxy.go @@ -605,7 +605,7 @@ func mediaType(h http.Header) string { return strings.ToLower(strings.TrimSpace(mt)) } -// replaceParam swaps the value of each name=placeholder pair of a query or form string for the escaped value, leaving every other byte as sent. +// replaceParam swaps the value of the first name=placeholder pair of a query or form string for the escaped value, leaving every other byte as sent. func replaceParam(raw, name, placeholder, value string) (string, bool) { if raw == "" { return raw, false @@ -618,8 +618,9 @@ func replaceParam(raw, name, placeholder, value string) (string, bool) { continue } if uv, err := url.QueryUnescape(v); err == nil && uv == placeholder { - escaped = cmp.Or(escaped, url.QueryEscape(value)) + escaped = url.QueryEscape(value) pairs[i] = k + "=" + escaped + break } } if escaped == "" { @@ -628,7 +629,7 @@ func replaceParam(raw, name, placeholder, value string) (string, bool) { return strings.Join(pairs, "&"), true } -// replaceMember swaps each top-level string member named name whose value is placeholder, leaving every other byte as sent. +// replaceMember swaps the first top-level string member named name whose value is placeholder, leaving every other byte as sent. func replaceMember(b []byte, name, placeholder, value string) ([]byte, bool) { dec := jsontext.NewDecoder(bytes.NewReader(b)) var ends []int64 @@ -647,6 +648,7 @@ func replaceMember(b []byte, name, placeholder, value string) ([]byte, bool) { } if v, err := jsontext.AppendUnquote(nil, raw); err == nil && string(v) == placeholder && dec.StackPointer().LastToken() == name { ends, lens = append(ends, dec.InputOffset()), append(lens, len(raw)) + break } } if len(ends) == 0 { From 97add225292cc809820ea47f8aa7f296bdf72826 Mon Sep 17 00:00:00 2001 From: CMGS Date: Sun, 4 Oct 2026 11:04:29 +0800 Subject: [PATCH 05/11] egress: one scrubber per request with pooled buffers, ranked intercept modes, and a flat inject view The scrub built a strings.Replacer and its escaped forms per response, copied every body byte three times through an 8 KiB chunk, and searched for a held-back tail in quadratic time; it now precomputes the forms and a first-byte table once, reads straight into a pooled 16 KiB half buffer, and checks a tail only where an old can start. A/B, six interleaved runs: a query-credential request -49%, a form or JSON fill -42%/-38%, a 64 KiB scrubbed page -52% and 24.8 KiB to 160 B, an event stream -65%. The claim inject view drops a map lookup per entry (64 entries, no match: 1004 to 580 ns per CONNECT). Intercept modes are ordered by rank, so EvalHost picks the highest and EvalInner reuses it; query and JSON fills skip a document that never names the parameter. --- docs/egress.md | 13 +- e2e/cmd/credsmoke/main.go | 47 ++-- sandboxd/egress/bench_test.go | 82 ++++++- sandboxd/egress/intercept_test.go | 303 +++++++++++++----------- sandboxd/egress/policy.go | 48 ++-- sandboxd/egress/policy_test.go | 14 +- sandboxd/egress/proxy.go | 372 +++++++++++++++--------------- sandboxd/pool/env_test.go | 61 ++--- sandboxd/pool/telemetry_test.go | 2 +- sandboxd/pool/view.go | 2 +- sandboxd/types/types.go | 17 +- scripts/credentials-e2e.sh | 5 +- 12 files changed, 541 insertions(+), 425 deletions(-) diff --git a/docs/egress.md b/docs/egress.md index cb8f9e5e..cb037656 100644 --- a/docs/egress.md +++ b/docs/egress.md @@ -323,14 +323,15 @@ git config --global http.https://github.com/.extraHeader "Authorization: Basic @ ``` A key that a client sends as a query parameter takes `query` instead of -`header`, and a placeholder is required. On an intercepted request, each +`header`, and a placeholder is required. On an intercepted request, the first `=` pair of the URL gets the value, URL-escaped; the placeholder may arrive percent-encoded, and every other byte of the query -stays as sent. With `body: true`, the same parameter of an -`application/x-www-form-urlencoded` body, and a top-level string member of that -name in an `application/json` body, are filled too when they carry the -placeholder and the body has a known length of at most 64 KiB; any other body -streams unchanged: +stays as sent. With `body: true` and no placeholder in the URL, the first such +pair of an `application/x-www-form-urlencoded` body, or the first top-level +string member of that name holding the placeholder in an `application/json` +body, is filled instead, when the body has a known length of at most 64 KiB; +any other body streams unchanged. One fill per credential bounds what a +request can grow by: ```jsonc "FB": { "value": "EAAB…", "guest": false, diff --git a/e2e/cmd/credsmoke/main.go b/e2e/cmd/credsmoke/main.go index 98ff8dad..d13d3ad0 100644 --- a/e2e/cmd/credsmoke/main.go +++ b/e2e/cmd/credsmoke/main.go @@ -1,7 +1,6 @@ // credsmoke is the claim-credential acceptance: claims on one intercept pool each // send their own env-injected header to an HTTPS echo through the proxy, the guest // never holds the value, and rotation, refusal, clearing and forks behave as the API says. -// With -inject-size it also drives a pool whose only rule is {"host":"*","intercept":"inject"}. package main import ( @@ -10,6 +9,7 @@ import ( "flag" "fmt" "net/http" + "net/url" "os" "strconv" "strings" @@ -23,7 +23,9 @@ const ( proxy = "http://127.0.0.1:3128" header = "X-Api-Key" entry = "API_KEY" + tsPH = "@TS@" serpPH = "@SERP@" + fbPH = "@FB@" ) func main() { @@ -230,40 +232,41 @@ func conditional(addr, token, template string, size sandbox.Size, echo, other st stamp := strconv.FormatInt(time.Now().UnixNano(), 10) serp, fb := "serp "+stamp+"/+", "fb-"+stamp if err = vault.PatchEnv(ctx, map[string]*sandbox.EnvVar{ - "TS": {Value: "2016-10-10", Guest: new(false), Inject: &sandbox.EnvInject{Hosts: []string{echo}, Query: "timestamp", Placeholder: "@TS@"}}, + "TS": {Value: "2016-10-10", Guest: new(false), Inject: &sandbox.EnvInject{Hosts: []string{echo}, Query: "timestamp", Placeholder: tsPH}}, "SERP": {Value: serp, Guest: new(false), Inject: &sandbox.EnvInject{Hosts: []string{echo}, Query: "api_key", Placeholder: serpPH}}, - "FB": {Value: fb, Guest: new(false), Inject: &sandbox.EnvInject{Hosts: []string{echo}, Query: "access_token", Body: true, Placeholder: "@FB@"}}, + "FB": {Value: fb, Guest: new(false), Inject: &sandbox.EnvInject{Hosts: []string{echo}, Query: "access_token", Body: true, Placeholder: fbPH}}, }); err != nil { return fmt.Errorf("add credentials: %w", err) } - for _, tc := range []struct { + for _, tt := range []struct { sb *sandbox.Sandbox host string intercepted bool }{{vault, echo, true}, {vault, other, false}, {bare, echo, false}} { - if err = expectTunnel(ctx, tc.sb, tc.host, tc.intercepted); err != nil { + if err = expectTunnel(ctx, tt.sb, tt.host, tt.intercepted); err != nil { return fmt.Errorf("after the credentials: %w", err) } } fmt.Println(" the claim with credentials is intercepted on their host only; the other claim still splices it") + base := "https://" + echo form, doc := "access_token="+fb+"&m=hi", `{"access_token":"`+fb+`"}` - for _, tc := range []struct { + for _, tt := range []struct { name, args, want, not string }{ - {"query", fmt.Sprintf("'https://%s/time/valid?timestamp=%%40TS%%40'", echo), `"valid":true`, "2016"}, - {"other query value", fmt.Sprintf("'https://%s/time/valid?timestamp=bogus'", echo), `"valid":false`, "2016"}, - {"echoed query", fmt.Sprintf("'https://%s/get?b=2&api_key=%%40SERP%%40&a=1'", echo), serpPH, stamp}, - {"form body", fmt.Sprintf("https://%s/post -d access_token=@FB@ -d m=hi", echo), fmt.Sprintf(`"content-length":"%d"`, len(form)), fb}, - {"json body", fmt.Sprintf(`https://%s/post -H 'Content-Type: application/json' -d '{"access_token":"@FB@"}'`, echo), fmt.Sprintf(`"content-length":"%d"`, len(doc)), fb}, - {"response header echo", fmt.Sprintf("-D - -o /dev/null 'https://%s/response-headers?api_key=%%40SERP%%40'", echo), serpPH, stamp}, + {"query", "'" + base + "/time/valid?timestamp=" + url.QueryEscape(tsPH) + "'", `"valid":true`, "2016"}, + {"other query value", "'" + base + "/time/valid?timestamp=bogus'", `"valid":false`, "2016"}, + {"echoed query", "'" + base + "/get?b=2&api_key=" + url.QueryEscape(serpPH) + "&a=1'", serpPH, stamp}, + {"form body", base + "/post -d access_token=" + fbPH + " -d m=hi", fmt.Sprintf(`"content-length":"%d"`, len(form)), fb}, + {"json body", base + `/post -H 'Content-Type: application/json' -d '{"access_token":"` + fbPH + `"}'`, fmt.Sprintf(`"content-length":"%d"`, len(doc)), fb}, + {"response header echo", "-D - -o /dev/null '" + base + "/response-headers?api_key=" + url.QueryEscape(serpPH) + "'", serpPH, stamp}, } { - out, execErr := vault.Exec(ctx, "sh", "-c", fmt.Sprintf("curl -sS -x %s %s", proxy, tc.args)) + out, execErr := vault.Exec(ctx, "sh", "-c", fmt.Sprintf("curl -sS -x %s %s", proxy, tt.args)) if execErr != nil { - return fmt.Errorf("%s exec: %w (%s)", tc.name, execErr, out) + return fmt.Errorf("%s exec: %w (%s)", tt.name, execErr, out) } - if !strings.Contains(out, tc.want) || strings.Contains(out, tc.not) { - return fmt.Errorf("%s: guest saw %q, want %q and not %q", tc.name, out, tc.want, tc.not) + if !strings.Contains(out, tt.want) || strings.Contains(out, tt.not) { + return fmt.Errorf("%s: guest saw %q, want %q and not %q", tt.name, out, tt.want, tt.not) } } fmt.Println(" query and form/JSON body placeholders filled at the origin; every echo of a value reached the guest as its placeholder") @@ -279,7 +282,7 @@ func conditional(addr, token, template string, size sandbox.Size, echo, other st return fmt.Errorf("after a clear: %w", err) } fmt.Println(" value absent from the guest; a cleared env splices the next connection") - fmt.Printf("VAULT %s|%s\n", serp, fb) + fmt.Printf("VAULT %s\nVAULT %s\n", serp, fb) fmt.Println("CREDSMOKE INJECT PASS") return nil } @@ -315,15 +318,11 @@ func expectTunnel(ctx context.Context, sb *sandbox.Sandbox, host string, interce if err != nil { return fmt.Errorf("%s exec: %w (%s)", host, err, out) } - var issuer string - for line := range strings.SplitSeq(out, "\n") { - if _, v, ok := strings.Cut(line, "issuer:"); ok && issuer == "" { - issuer = v - } - } + _, rest, found := strings.Cut(out, "issuer:") + issuer, _, _ := strings.Cut(rest, "\n") mitm := strings.Contains(issuer, "e2e") h2 := strings.Contains(out, "version=2") - if issuer == "" || mitm != intercepted || h2 == intercepted { + if !found || mitm != intercepted || h2 == intercepted { return fmt.Errorf("%s on %s: issuer %q, HTTP/2 %t, want intercepted %t:\n%s", host, sb.ID, issuer, h2, intercepted, out) } return nil diff --git a/sandboxd/egress/bench_test.go b/sandboxd/egress/bench_test.go index ae80cd1f..93071e1e 100644 --- a/sandboxd/egress/bench_test.go +++ b/sandboxd/egress/bench_test.go @@ -2,11 +2,13 @@ package egress import ( "bufio" + "bytes" "crypto/tls" "crypto/x509" "io" "net/http" "net/http/httptest" + "strings" "testing" ) @@ -96,16 +98,90 @@ func BenchmarkTunnelDecision(b *testing.B) { } } -func benchFront(b *testing.B, intercept InterceptMode) (proxyAddr string, roots *x509.CertPool) { +func BenchmarkInjectAndScrubHeaders(b *testing.B) { + key := "EAABsbCS1iHgBAKZBZCx9ZAZBe7ZAZCYqZBfZC0ZD" + base, err := http.NewRequestWithContext(b.Context(), http.MethodPost, "https://example.com/v19.0/me", nil) + if err != nil { + b.Fatalf("build request: %v", err) + } + for _, arm := range []struct { + name, query, ctype, body string + cred Credential + }{ + {"no-credential", "a=1", "", "", Credential{}}, + {"header", "a=1", "", "", Credential{Name: "K", Header: "X-Key", Value: key, Placeholder: "@K@"}}, + {"query", "a=1&access_token=%40K%40", "", "", Credential{Name: "K", Query: "access_token", Value: key, Placeholder: "@K@"}}, + {"form-body", "", "application/x-www-form-urlencoded", "m=hi&access_token=%40K%40", Credential{Name: "K", Query: "access_token", Body: true, Value: key, Placeholder: "@K@"}}, + {"json-body", "", "application/json", `{"m":"hi","access_token":"@K@"}`, Credential{Name: "K", Query: "access_token", Body: true, Value: key, Placeholder: "@K@"}}, + } { + creds := map[string][]Credential{} + if arm.cred.Name != "" { + creds["example.com"] = []Credential{arm.cred} + } + p := New(Policy{}, credSecrets{creds: creds}, nil, fixedDial(""), nil, nil) + b.Run(arm.name, func(b *testing.B) { + b.ReportAllocs() + for b.Loop() { + out := base.Clone(b.Context()) + out.URL.RawQuery = arm.query + if arm.ctype != "" { + out.Header.Set("Content-Type", arm.ctype) + out.Body, out.ContentLength = io.NopCloser(strings.NewReader(arm.body)), int64(len(arm.body)) + } + if _, sc := p.inject(Rule{}, out, "example.com", true); sc != nil { + sc.header(http.Header{"Location": {"/next?access_token=" + key}, "Content-Type": {"application/json"}, "Date": {"x"}}) + } + } + }) + } +} + +func BenchmarkScrubBody(b *testing.B) { + key := "EAABsbCS1iHgBAKZBZCx9ZAZBe7ZAZCYqZBfZC0ZD" + sc := newScrubber([]Credential{{Name: "K", Query: "access_token", Value: key, Placeholder: "@K@"}}) + page := bytes.Repeat([]byte(`{"id":"1234567890","message":"hello world","created_time":"2026-10-04T00:00:00+0000"},`), 64<<10/88) + page = append(page, `"paging":{"next":"https:\/\/graph.facebook.com\/v19.0\/me\/feed?access_token=`+key+`&after=x"}`...) + event := []byte("data: {\"choices\":[{\"delta\":{\"content\":\"hello there\"}}]}\n\n") + for _, arm := range []struct { + name string + src func() io.Reader + }{ + {"64KiB-page", func() io.Reader { return bytes.NewReader(page) }}, + {"sse-256-events", func() io.Reader { + events := make([]io.Reader, 256) + for i := range events { + events[i] = bytes.NewReader(event) + } + return io.MultiReader(events...) + }}, + } { + b.Run(arm.name+"/plain", func(b *testing.B) { + b.ReportAllocs() + for b.Loop() { + _, _ = io.Copy(io.Discard, arm.src()) + } + }) + b.Run(arm.name+"/scrubbed", func(b *testing.B) { + b.ReportAllocs() + for b.Loop() { + buf := copyBufs.Get().(*[]byte) + _, _ = io.Copy(io.Discard, newScrubReader(arm.src(), sc, *buf)) + copyBufs.Put(buf) + } + }) + } +} + +func benchFront(b *testing.B, mode InterceptMode) (proxyAddr string, roots *x509.CertPool) { b.Helper() upstream := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { _, _ = io.WriteString(w, "hello") })) b.Cleanup(upstream.Close) - rule := Rule{Host: "example.com", Intercept: intercept} + rule := Rule{Host: "example.com", Intercept: mode} var ca *CA roots = x509.NewCertPool() - if intercept == InterceptAlways { + if mode == InterceptAlways { ca = testCAOnly(b) if !roots.AppendCertsFromPEM(ca.CertPEM()) { b.Fatal("append cluster root") diff --git a/sandboxd/egress/intercept_test.go b/sandboxd/egress/intercept_test.go index 1a277da7..015ca886 100644 --- a/sandboxd/egress/intercept_test.go +++ b/sandboxd/egress/intercept_test.go @@ -6,6 +6,7 @@ import ( "crypto/tls" "crypto/x509" "encoding/hex" + "encoding/json/v2" "fmt" "io" "net" @@ -264,16 +265,20 @@ func TestInterceptInjectsClaimCredentialsBelowThePoolSecret(t *testing.T) { tc := connectTLS(t, front.Listener.Addr().String(), "Example.com:443", "example.com", guestRoots) defer func() { _ = tc.Close() }() - header, seen := echoTLS(t, tc, http.MethodGet, "/echo", "", "", map[string]string{"X-Api-Key": "placeholder", "X-Empty": "guest"}) + req := newRequest(t, http.MethodGet, "/echo", "") + req.Header.Set("X-Api-Key", "placeholder") + req.Header.Set("X-Empty", "guest") + r := send(t, tc, req) + seen, _ := r.seen(t) - if seen["key"] != "k1" { - t.Errorf("upstream saw X-Api-Key %q, want the first claim credential in order", seen["key"]) + if seen.Key != "k1" { + t.Errorf("upstream saw X-Api-Key %q, want the first claim credential in order", seen.Key) } - if got := header.Get("X-Auth-Seen"); got != "Bearer SECRET" { + if got := r.header.Get("X-Auth-Seen"); got != "Bearer SECRET" { t.Errorf("upstream saw Authorization %q, want the pool secret kept over the claim's", got) } - if seen["empty"] != "guest" { - t.Errorf("upstream saw X-Empty %q, want the guest's value kept for an empty credential", seen["empty"]) + if seen.Empty != "guest" { + t.Errorf("upstream saw X-Empty %q, want the guest's value kept for an empty credential", seen.Empty) } _ = recvEvent(t, events) if ev := recvEvent(t, events); ev.Injected != "gh,claim:API" { @@ -292,16 +297,15 @@ func TestInterceptInjectsAPlaceholderCredentialOnlyWhereTheGuestSentIt(t *testin for sent, want := range map[string]string{"@GIT@": "real", "": "", "@OTHER@": "@OTHER@"} { tc := connectTLS(t, front.Listener.Addr().String(), "example.com:443", "example.com", guestRoots) - headers := map[string]string{} + req := newRequest(t, http.MethodGet, "/echo", "") if sent != "" { - headers["X-Api-Key"] = sent + req.Header.Set("X-Api-Key", sent) } - _, seen := echoTLS(t, tc, http.MethodGet, "/echo", "", "", headers) + seen, _ := send(t, tc, req).seen(t) _ = tc.Close() - if seen["key"] != want { - t.Errorf("guest sent %q: upstream saw %q, want %q", sent, seen["key"], want) + if seen.Key != want { + t.Errorf("guest sent %q: upstream saw %q, want %q", sent, seen.Key, want) } - } } @@ -312,23 +316,71 @@ func TestAnInjectRuleInterceptsOnlyForAClaimWithACredentialForTheHost(t *testing front := httptest.NewServer(p) defer front.Close() - tc := connectTLS(t, front.Listener.Addr().String(), "Example.com:443", "example.com", guestRoots) - _, seen := echoTLS(t, tc, http.MethodGet, "/echo", "", "", nil) - _ = tc.Close() - if seen["key"] != "real" { - t.Errorf("a host with a credential: upstream saw X-Api-Key %q, want it intercepted and injected", seen["key"]) + for _, tt := range []struct { + target string + roots *x509.CertPool + sent, key string + }{ + {"Example.com:443", guestRoots, "", "real"}, + {"other.test:443", trustUpstream(upstream), "guest", "guest"}, + } { + tc := connectTLS(t, front.Listener.Addr().String(), tt.target, "example.com", tt.roots) + req := newRequest(t, http.MethodGet, "/echo", "") + if tt.sent != "" { + req.Header.Set("X-Api-Key", tt.sent) + } + seen, _ := send(t, tc, req).seen(t) + _ = tc.Close() + if seen.Key != tt.key { + t.Errorf("CONNECT %s: upstream saw X-Api-Key %q, want %q", tt.target, seen.Key, tt.key) + } } - tc = connectTLS(t, front.Listener.Addr().String(), "other.test:443", "example.com", trustUpstream(upstream)) - _, seen = echoTLS(t, tc, http.MethodGet, "/echo", "", "", map[string]string{"X-Api-Key": "guest"}) - _ = tc.Close() - if seen["key"] != "guest" { - t.Errorf("a host without a credential: upstream saw X-Api-Key %q, want the guest's own value over a splice", seen["key"]) +} + +func TestTunnelDecisionOnAnInjectRule(t *testing.T) { + ca, _ := testCA(t) + creds := credSecrets{creds: map[string][]Credential{ + "empty.test": {{Name: "E", Header: "X-Key", Value: ""}}, + "graph.test": {{Name: "G", Query: "access_token", Value: "g", Placeholder: "@G@"}}, + }} + for _, tt := range []struct { + name string + secrets Secrets + host string + injects bool + intercept bool + }{ + {"a claim with no secrets", nil, "graph.test", true, false}, + {"an empty credential", creds, "empty.test", true, false}, + {"a host without a credential", creds, "plain.test", true, false}, + {"a credential on the proxy door", creds, "graph.test", true, true}, + {"a credential on the SOCKS door", creds, "graph.test", false, false}, + } { + p := New(Policy{Allow: []Rule{{Host: "*", Intercept: InterceptInject}}}, tt.secrets, ca, fixedDial(""), nil, nil) + if d, intercept := p.tunnelDecision(tt.host, 443, tt.injects); d != DecisionAllow || intercept != tt.intercept { + t.Errorf("%s: decision %v intercept %t, want an allow with intercept %t", tt.name, d, intercept, tt.intercept) + } } - if _, intercept := New(Policy{Allow: []Rule{{Host: "*", Intercept: InterceptInject}}}, nil, p.ca, fixedDial(""), nil, nil).tunnelDecision("example.com", 443, true); intercept { - t.Error("a claim with no secrets intercepted on an inject rule") +} + +func TestAnAlwaysRuleOutranksAnEarlierInjectRule(t *testing.T) { + ca, _ := testCA(t) + policy := Policy{Allow: []Rule{{Host: "*", Intercept: InterceptInject}, {Host: "api.github.com", Methods: []string{"GET"}, Secret: "gh", Intercept: InterceptAlways}}} + if _, intercept := New(policy, fakeSecrets{}, ca, fixedDial(""), nil, nil).tunnelDecision("api.github.com", 443, true); !intercept { + t.Error("a claim without a credential spliced a host an always rule intercepts") } - if d, intercept := p.tunnelDecision("plain.test", 80, true); d != DecisionAllow || intercept { - t.Errorf("an inject rule without a credential: decision %v intercept %t, want a plain allow", d, intercept) + for _, tt := range []struct { + host, method string + decision Decision + mode InterceptMode + }{ + {"api.github.com", http.MethodGet, DecisionAllow, InterceptAlways}, + {"api.github.com", http.MethodPost, DecisionDeny, InterceptOff}, + {"graph.test", http.MethodPost, DecisionAllow, InterceptInject}, + } { + if rule, d := policy.EvalInner(tt.host, tt.method, 443); d != tt.decision || rule.Intercept != tt.mode { + t.Errorf("EvalInner %s %s = %+v/%v, want %v with mode %d", tt.method, tt.host, rule, d, tt.decision, tt.mode) + } } } @@ -340,27 +392,31 @@ func TestInterceptSubstitutesAQueryCredentialAndScrubsItsEcho(t *testing.T) { front := httptest.NewServer(p) defer front.Close() - for _, tc := range []struct { + for _, tt := range []struct { query, want, injected string }{ {"b=2&api_key=%40SERP%40&a=1", "b=2&api_key=real+key%2F%2B&a=1", "claim:SERP"}, - {"gzip=1&api_key=%40SERP%40", "gzip=1&api_key=real+key%2F%2B", "claim:SERP"}, - {"api_key=@SERP@", "api_key=real+key%2F%2B", "claim:SERP"}, + {"api_key=@SERP@&api_key=%40SERP%40", "api_key=real+key%2F%2B&api_key=%40SERP%40", "claim:SERP"}, {"api_key=other&x=%40SERP%40", "api_key=other&x=%40SERP%40", ""}, {"", "", ""}, } { - conn := connectTLS(t, front.Listener.Addr().String(), "example.com:443", "example.com", guestRoots) - header, seen := echoTLS(t, conn, http.MethodGet, "/echo?"+tc.query, "", "", map[string]string{"Accept-Encoding": "gzip"}) - _ = conn.Close() - if seen["query"] != tc.want { - t.Errorf("query %q reached the upstream as %q, want %q", tc.query, seen["query"], tc.want) + tc := connectTLS(t, front.Listener.Addr().String(), "example.com:443", "example.com", guestRoots) + req := newRequest(t, http.MethodGet, "/echo?"+tt.query, "") + if tt.injected != "" { + req.Header.Set("Accept-Encoding", "gzip") } - if loc := header.Get("Location"); strings.Contains(loc, "real") || strings.Contains(seen["rawquery"], "real") { - t.Errorf("query %q: the guest received Location %q and body query %q, one holding the value", tc.query, loc, seen["rawquery"]) + r := send(t, tc, req) + _ = tc.Close() + seen, reflected := r.seen(t) + if seen.Query != tt.want { + t.Errorf("query %q reached the upstream as %q, want %q", tt.query, seen.Query, tt.want) + } + if loc := r.header.Get("Location"); strings.Contains(loc, "real") || strings.Contains(reflected, "real") { + t.Errorf("query %q: the guest received Location %q and body %q, one holding the value", tt.query, loc, reflected) } _ = recvEvent(t, events) - if ev := recvEvent(t, events); ev.Injected != tc.injected { - t.Errorf("query %q: audit Injected %q, want %q", tc.query, ev.Injected, tc.injected) + if ev := recvEvent(t, events); ev.Injected != tt.injected { + t.Errorf("query %q: audit Injected %q, want %q", tt.query, ev.Injected, tt.injected) } } } @@ -376,64 +432,33 @@ func TestInterceptSubstitutesABodyCredentialInASmallFormOrJSONBody(t *testing.T) defer front.Close() big := "access_token=%40FB%40&pad=" + strings.Repeat("x", maxInjectBody) - for _, tc := range []struct { + for _, tt := range []struct { name, ctype, body, want string }{ - {"form", "application/x-www-form-urlencoded", "a=1&access_token=%40FB%40&key=%40URL%40", "a=1&access_token=real+%22tok%22%26&key=%40URL%40"}, - {"json", "application/json; charset=utf-8", `{"access_token":"@FB@", "message":"@FB@","n":{"access_token": "@FB@"},"a":["@FB@"]}`, `{"access_token":"real \"tok\"&", "message":"@FB@","n":{"access_token": "@FB@"},"a":["@FB@"]}`}, + {"form", "application/x-www-form-urlencoded", "a=1&access_token=%40FB%40&key=%40URL%40&access_token=%40FB%40", "a=1&access_token=real+%22tok%22%26&key=%40URL%40&access_token=%40FB%40"}, + {"json", "application/json; charset=utf-8", `{"message":"@FB@", "access_token":"@FB@","n":{"access_token": "@FB@"},"a":["@FB@"]}`, `{"message":"@FB@", "access_token":"real \"tok\"&","n":{"access_token": "@FB@"},"a":["@FB@"]}`}, {"other type", "text/plain", "access_token=%40FB%40", "access_token=%40FB%40"}, {"oversized", "application/x-www-form-urlencoded", big, big}, } { - conn := connectTLS(t, front.Listener.Addr().String(), "example.com:443", "example.com", guestRoots) - _, seen := echoTLS(t, conn, http.MethodPost, "/echo", tc.ctype, tc.body, nil) - _ = conn.Close() - if seen["body"] != tc.want || seen["length"] != fmt.Sprint(len(tc.want)) { - t.Errorf("%s: upstream saw body %.80q length %s, want %.80q length %d", tc.name, seen["body"], seen["length"], tc.want, len(tc.want)) + tc := connectTLS(t, front.Listener.Addr().String(), "example.com:443", "example.com", guestRoots) + req := newRequest(t, http.MethodPost, "/echo", tt.body) + req.Header.Set("Content-Type", tt.ctype) + seen, _ := send(t, tc, req).seen(t) + _ = tc.Close() + if seen.Body != tt.want || seen.Length != int64(len(tt.want)) { + t.Errorf("%s: upstream saw body %.80q length %d, want %.80q length %d", tt.name, seen.Body, seen.Length, tt.want, len(tt.want)) } } } -func TestAnAlwaysRuleKeepsItsHostUnderAnEarlierInjectRule(t *testing.T) { - ca, _ := testCA(t) - policy := Policy{Allow: []Rule{{Host: "*", Intercept: InterceptInject}, {Host: "api.github.com", Methods: []string{"GET"}, Secret: "gh", Intercept: InterceptAlways}}} - creds := credSecrets{fakeSecrets: fakeSecrets{"gh": {"Authorization", "Bearer SECRET"}}, creds: map[string][]Credential{ - "api.github.com": {{Name: "K", Header: "X-Key", Value: "k"}}, - "empty.test": {{Name: "E", Header: "X-Key", Value: ""}}, - "graph.test": {{Name: "G", Query: "access_token", Value: "g", Placeholder: "@G@"}}, - }} - bare := New(policy, fakeSecrets{}, ca, fixedDial(""), nil, nil) - if _, intercept := bare.tunnelDecision("api.github.com", 443, true); !intercept { - t.Error("a claim without a credential spliced a host an always rule intercepts") - } - if rule, d := policy.EvalInner("api.github.com", http.MethodGet, 443); d != DecisionAllow || rule.Secret != "gh" { - t.Errorf("EvalInner GET = %+v/%v, want the always rule and its secret", rule, d) - } - if _, d := policy.EvalInner("api.github.com", http.MethodPost, 443); d != DecisionDeny { - t.Error("an earlier inject rule rescued a method the always rule refuses") - } - if rule, d := policy.EvalInner("graph.test", http.MethodPost, 443); d != DecisionAllow || rule.Intercept != InterceptInject { - t.Errorf("EvalInner on an inject-only host = %+v/%v, want the inject rule", rule, d) - } - p := New(policy, creds, ca, fixedDial(""), nil, nil) - if _, intercept := p.tunnelDecision("empty.test", 443, true); intercept { - t.Error("a credential with an empty value intercepted its host") - } - if _, intercept := p.tunnelDecision("graph.test", 443, true); !intercept { - t.Error("a credential's host was not intercepted on the proxy door") - } - if d, intercept := p.tunnelDecision("graph.test", 443, false); d != DecisionAllow || intercept { - t.Errorf("the SOCKS door on an inject rule: decision %v intercept %t, want a splice", d, intercept) - } -} - func TestACredentialFillsOnlyTheFirstMatchingParameter(t *testing.T) { value := strings.Repeat("v", 8<<10) form := strings.Repeat("access_token=%40FB%40&", 3600) - if got, _ := replaceParam(form, "access_token", "@FB@", value); len(got) != len(form)-len("%40FB%40")+len(value) { + if got := replaceParam(form, "access_token", "@FB@", value); len(got) != len(form)-len("%40FB%40")+len(value) { t.Errorf("3600 repeated pairs grew the form to %d bytes, want one fill on %d", len(got), len(form)) } doc := `{"access_token":"@FB@","access_token":"@FB@"}` - if got, _ := replaceMember([]byte(doc), "access_token", "@FB@", "v"); string(got) != `{"access_token":"v","access_token":"@FB@"}` { + if got := replaceMember(doc, "access_token", "@FB@", "v"); got != `{"access_token":"v","access_token":"@FB@"}` { t.Errorf("a repeated member filled as %s, want only the first", got) } } @@ -445,43 +470,33 @@ func TestARangeCannotSplitAScrubbedQueryValue(t *testing.T) { front := httptest.NewServer(p) defer front.Close() - for _, r := range []string{"bytes=0-9", "bytes=10-20", "bytes=0-3,10-13"} { - conn := connectTLS(t, front.Listener.Addr().String(), "example.com:443", "example.com", guestRoots) - req, err := http.NewRequestWithContext(t.Context(), http.MethodGet, "https://example.com/range?api_key=%40K%40", nil) - if err != nil { - t.Fatalf("build request: %v", err) - } - req.Header.Set("Range", r) - if err = req.Write(conn); err != nil { - t.Fatalf("write request: %v", err) - } - resp, err := http.ReadResponse(bufio.NewReader(conn), req) - if err != nil { - t.Fatalf("read response: %v", err) - } - body, _ := io.ReadAll(resp.Body) - _ = resp.Body.Close() - _ = conn.Close() - if resp.StatusCode != http.StatusOK || string(body) != "@K@" { - t.Errorf("Range %s: guest got %d %q, want the whole value scrubbed to its placeholder", r, resp.StatusCode, body) + for _, byteRange := range []string{"bytes=0-9", "bytes=10-20", "bytes=0-3,10-13"} { + tc := connectTLS(t, front.Listener.Addr().String(), "example.com:443", "example.com", guestRoots) + req := newRequest(t, http.MethodGet, "/range?api_key=%40K%40", "") + req.Header.Set("Range", byteRange) + r := send(t, tc, req) + _ = tc.Close() + if r.status != http.StatusOK || r.body != "@K@" { + t.Errorf("Range %s: guest got %d %q, want the whole value scrubbed to its placeholder", byteRange, r.status, r.body) } } } -func TestEchoesCoverTheCommonEscapings(t *testing.T) { +func TestAScrubberCoversTheCommonEscapings(t *testing.T) { value := "ab cd/ef p.Allow[best].Intercept) { best = i } } - if best >= 0 { - return p.Allow[best], DecisionAllow + if best < 0 { + return Rule{}, DecisionDeny } - return Rule{}, DecisionDeny + return p.Allow[best], DecisionAllow } -// EvalInner matches only intercept rules, so a plain rule cannot shadow or rescue one; an always-intercept rule for the host shadows every inject rule. +// EvalInner matches only rules of the mode EvalHost picks for the host, so a plain rule cannot shadow or rescue an intercept rule and an always rule shadows an inject rule. func (p Policy) EvalInner(host, method string, port uint16) (Rule, Decision) { - host = strings.ToLower(host) - mode := InterceptInject - if slices.ContainsFunc(p.Allow, func(r Rule) bool { - return r.Intercept == InterceptAlways && r.matchHost(host) && r.matchPort(port) - }) { - mode = InterceptAlways + top, d := p.EvalHost(host, port) + if d == DecisionDeny || top.Intercept == InterceptOff { + return Rule{}, DecisionDeny } + host = strings.ToLower(host) for _, r := range p.Allow { - if r.Intercept == mode && r.matches(host, method, port) { + if r.Intercept == top.Intercept && r.matches(host, method, port) { return r, DecisionAllow } } diff --git a/sandboxd/egress/policy_test.go b/sandboxd/egress/policy_test.go index 43252343..02c55720 100644 --- a/sandboxd/egress/policy_test.go +++ b/sandboxd/egress/policy_test.go @@ -60,23 +60,23 @@ func TestEvalSkipsInterceptRules(t *testing.T) { } } -func TestInterceptModeJSON(t *testing.T) { +func TestInterceptModeRoundTripsFalseTrueAndInjectAndRefusesOthers(t *testing.T) { var p Policy if err := json.Unmarshal([]byte(`{"allow":[{"host":"a"},{"host":"b","intercept":false},{"host":"c","intercept":true},{"host":"d","intercept":"inject"}]}`), &p); err != nil { t.Fatalf("Unmarshal: %v", err) } - want := []InterceptMode{InterceptOff, InterceptOff, InterceptAlways, InterceptInject} - for i, r := range p.Allow { - if r.Intercept != want[i] { - t.Errorf("rule %s intercept = %d, want %d", r.Host, r.Intercept, want[i]) + for i, mode := range []InterceptMode{InterceptOff, InterceptOff, InterceptAlways, InterceptInject} { + if p.Allow[i].Intercept != mode { + t.Errorf("rule %s intercept = %d, want %d", p.Allow[i].Host, p.Allow[i].Intercept, mode) } } out, err := json.Marshal(p) if err != nil { t.Fatalf("Marshal: %v", err) } - if got := string(out); got != `{"allow":[{"host":"a"},{"host":"b"},{"host":"c","intercept":true},{"host":"d","intercept":"inject"}]}` { - t.Errorf("Marshal = %s", got) + want := `{"allow":[{"host":"a"},{"host":"b"},{"host":"c","intercept":true},{"host":"d","intercept":"inject"}]}` + if got := string(out); got != want { + t.Errorf("Marshal = %s, want %s", got, want) } for _, bad := range []string{`"always"`, `1`, `null`} { if err := json.Unmarshal([]byte(`{"allow":[{"host":"a","intercept":`+bad+`}]}`), &p); err == nil { diff --git a/sandboxd/egress/proxy.go b/sandboxd/egress/proxy.go index beb02e12..b26b277e 100644 --- a/sandboxd/egress/proxy.go +++ b/sandboxd/egress/proxy.go @@ -32,7 +32,7 @@ const ( // maxIdleConns bounds the upstream pool per sandbox; a guest walking many hosts cannot park a descriptor per host. maxIdleConns = 64 maxInjectBody = 64 << 10 - scrubChunk = 8 << 10 + scrubChunk = 16 << 10 ) var ( @@ -279,7 +279,7 @@ func (p *Proxy) serveConnect(w http.ResponseWriter, r *http.Request) { p.reportTransfer(ev, early+sent, received) } -// tunnelDecision gates a tunnel by host; injects is false on a door that never injects, where an inject rule is a plain one. +// tunnelDecision treats an inject rule as a plain one when injects is false, on a door that never injects. func (p *Proxy) tunnelDecision(host string, port uint16, injects bool) (decision Decision, intercept bool) { // host-gate interception: the tunnel's CONNECT verb is not the request method. if p.ca != nil { @@ -339,10 +339,9 @@ func (p *Proxy) relay(w http.ResponseWriter, r *http.Request, ev Event, rule Rul out.RequestURI = "" out.Close = false // the guest's Connection: close is its own; the upstream pool keeps the conn stripHop(out.Header) - var used []Credential - ev.Injected, used = p.inject(rule, out, ev.Host, mitm) - olds, news := echoes(used) - if len(olds) > 0 { + var sc *scrubber + ev.Injected, sc = p.inject(rule, out, ev.Host, mitm) + if sc != nil { // the transport then decodes compression itself, and no range splits a value, so the scrub sees every echo whole. for _, h := range []string{"Accept-Encoding", "Range", "If-Range"} { out.Header.Del(h) @@ -375,10 +374,12 @@ func (p *Proxy) relay(w http.ResponseWriter, r *http.Request, ev Event, rule Rul maps.Copy(w.Header(), resp.Header) stripHop(w.Header()) var src io.Reader = resp.Body - if len(olds) > 0 { - scrubHeader(w.Header(), olds, news) + if sc != nil { + sc.header(w.Header()) w.Header().Del("Content-Length") - src = &scrubReader{src: resp.Body, olds: olds, news: news, chunk: make([]byte, scrubChunk)} + buf := copyBufs.Get().(*[]byte) + defer copyBufs.Put(buf) + src = newScrubReader(resp.Body, sc, *buf) } w.WriteHeader(resp.StatusCode) if streamed(resp) { @@ -425,8 +426,8 @@ func (p *Proxy) reportTransfer(ev Event, sent, received int64) { } } -// inject sets the rule's secret, then on an intercepted request the claim credentials the guest asked for, and returns the names set and the credentials used. -func (p *Proxy) inject(rule Rule, out *http.Request, host string, mitm bool) (string, []Credential) { +// inject sets the rule's secret, then on an intercepted request the claim credentials the guest asked for, and returns a scrubber for the query credentials it filled. +func (p *Proxy) inject(rule Rule, out *http.Request, host string, mitm bool) (string, *scrubber) { if p.secrets == nil { return "", nil } @@ -440,34 +441,31 @@ func (p *Proxy) inject(rule Rule, out *http.Request, host string, mitm bool) (st if !mitm { return strings.Join(names, ","), nil } - var used, body []Credential + var filled, body []Credential for _, c := range p.secrets.Credentials(strings.ToLower(host)) { - switch { - case c.Value == "": - case c.Query != "": - if c.Body { + if c.Value == "" { + continue + } + if c.Query != "" { + if q := replaceParam(out.URL.RawQuery, c.Query, c.Placeholder, c.Value); q != out.URL.RawQuery { + out.URL.RawQuery, filled = q, append(filled, c) + } else if c.Body { body = append(body, c) } - if q, ok := replaceParam(out.URL.RawQuery, c.Query, c.Placeholder, c.Value); ok { - out.URL.RawQuery = q - used = append(used, c) - } - case c.Placeholder != "" && out.Header.Get(c.Header) != c.Placeholder, - slices.ContainsFunc(set, func(s string) bool { return strings.EqualFold(s, c.Header) }): - default: - out.Header.Set(c.Header, c.Value) - set, used = append(set, c.Header), append(used, c) + continue } - } - for _, c := range substituteBody(out, body) { - if !slices.ContainsFunc(used, func(u Credential) bool { return u.Name == c.Name }) { - used = append(used, c) + if (c.Placeholder != "" && out.Header.Get(c.Header) != c.Placeholder) || + slices.ContainsFunc(set, func(s string) bool { return strings.EqualFold(s, c.Header) }) { + continue } + out.Header.Set(c.Header, c.Value) + names, set = append(names, "claim:"+c.Name), append(set, c.Header) } - for _, c := range used { + filled = append(filled, substituteBody(out, body)...) + for _, c := range filled { names = append(names, "claim:"+c.Name) } - return strings.Join(names, ","), used + return strings.Join(names, ","), newScrubber(filled) } func (p *Proxy) record(ev Event) { @@ -488,37 +486,79 @@ func (c *countingBody) Read(b []byte) (int, error) { return n, err } -// scrubReader replaces each of olds with its news as a response body streams; it holds back only a tail that may begin a match. +// scrubber turns each filled query credential's value, in the escapings a reflected URL carries, back into its placeholder. +type scrubber struct { + olds []string + oldRaw [][]byte + news []string + first [256]bool + longest int +} + +func newScrubber(filled []Credential) *scrubber { + if len(filled) == 0 { + return nil + } + sc := &scrubber{} + for _, c := range filled { + values, placeholders := escapings(c.Value), escapings(c.Placeholder) + for i, old := range values { + if !slices.Contains(sc.olds, old) { + sc.olds, sc.oldRaw, sc.news = append(sc.olds, old), append(sc.oldRaw, []byte(old)), append(sc.news, placeholders[i]) + sc.first[old[0]], sc.longest = true, max(sc.longest, len(old)) + } + } + } + return sc +} + +func (sc *scrubber) header(h http.Header) { + for _, vs := range h { + for i, v := range vs { + for j, old := range sc.olds { + if strings.Contains(v, old) { + v = strings.ReplaceAll(v, old, sc.news[j]) + } + } + vs[i] = v + } + } +} + +// scrubReader streams a response body through a scrubber, holding back only a tail that may begin a match. type scrubReader struct { - src io.Reader - olds, news [][]byte - chunk []byte - pending []byte - out []byte - off int - err error + src io.Reader + sc *scrubber + pending []byte + out bytes.Buffer + err error +} + +// newScrubReader splits buf, at least two chunks long, between the bytes read and the bytes scrubbed. +func newScrubReader(src io.Reader, sc *scrubber, buf []byte) *scrubReader { + return &scrubReader{src: src, sc: sc, pending: buf[:0:scrubChunk], out: *bytes.NewBuffer(buf[scrubChunk:scrubChunk])} } func (s *scrubReader) Read(b []byte) (int, error) { - for s.off == len(s.out) && s.err == nil { - s.out, s.off = s.out[:0], 0 - n, err := s.src.Read(s.chunk) - s.pending, s.err = append(s.pending, s.chunk[:n]...), err + for s.out.Len() == 0 && s.err == nil { + if len(s.pending) == cap(s.pending) { + s.pending = slices.Grow(s.pending, scrubChunk) + } + n, err := s.src.Read(s.pending[len(s.pending):cap(s.pending)]) + s.pending, s.err = s.pending[:len(s.pending)+n], err s.scan() } - if s.off == len(s.out) { + if s.out.Len() == 0 { return 0, s.err } - n := copy(b, s.out[s.off:]) - s.off += n - return n, nil + return s.out.Read(b) } func (s *scrubReader) scan() { buf, i := s.pending, 0 for { at, k := -1, 0 - for j, old := range s.olds { + for j, old := range s.sc.oldRaw { if idx := bytes.Index(buf[i:], old); idx >= 0 && (at < 0 || idx < at) { at, k = idx, j } @@ -526,24 +566,23 @@ func (s *scrubReader) scan() { if at < 0 { break } - s.out = append(append(s.out, buf[i:i+at]...), s.news[k]...) - i += at + len(s.olds[k]) + s.out.Write(buf[i : i+at]) + s.out.WriteString(s.sc.news[k]) + i += at + len(s.sc.oldRaw[k]) } hold := 0 if s.err == nil { hold = s.partial(buf[i:]) } - s.out = append(s.out, buf[i:len(buf)-hold]...) + s.out.Write(buf[i : len(buf)-hold]) s.pending = s.pending[:copy(s.pending, buf[len(buf)-hold:])] } func (s *scrubReader) partial(tail []byte) int { - longest := 0 - for _, old := range s.olds { - longest = max(longest, len(old)) - } - for k := min(len(tail), longest-1); k > 0; k-- { - if slices.ContainsFunc(s.olds, func(old []byte) bool { return len(old) > k && bytes.HasPrefix(old, tail[len(tail)-k:]) }) { + for k := min(len(tail), s.sc.longest-1); k > 0; k-- { + if s.sc.first[tail[len(tail)-k]] && slices.ContainsFunc(s.sc.oldRaw, func(old []byte) bool { + return len(old) > k && bytes.HasPrefix(old, tail[len(tail)-k:]) + }) { return k } } @@ -605,131 +644,6 @@ func mediaType(h http.Header) string { return strings.ToLower(strings.TrimSpace(mt)) } -// replaceParam swaps the value of the first name=placeholder pair of a query or form string for the escaped value, leaving every other byte as sent. -func replaceParam(raw, name, placeholder, value string) (string, bool) { - if raw == "" { - return raw, false - } - pairs := strings.Split(raw, "&") - escaped := "" - for i, pair := range pairs { - k, v, _ := strings.Cut(pair, "=") - if uk, err := url.QueryUnescape(k); err != nil || uk != name { - continue - } - if uv, err := url.QueryUnescape(v); err == nil && uv == placeholder { - escaped = url.QueryEscape(value) - pairs[i] = k + "=" + escaped - break - } - } - if escaped == "" { - return raw, false - } - return strings.Join(pairs, "&"), true -} - -// replaceMember swaps the first top-level string member named name whose value is placeholder, leaving every other byte as sent. -func replaceMember(b []byte, name, placeholder, value string) ([]byte, bool) { - dec := jsontext.NewDecoder(bytes.NewReader(b)) - var ends []int64 - var lens []int - for { - kind, n := dec.StackIndex(dec.StackDepth()) - if dec.StackDepth() != 1 || kind != '{' || n%2 == 0 || dec.PeekKind() != '"' { - if _, err := dec.ReadToken(); err != nil { - break - } - continue - } - raw, err := dec.ReadValue() - if err != nil { - break - } - if v, err := jsontext.AppendUnquote(nil, raw); err == nil && string(v) == placeholder && dec.StackPointer().LastToken() == name { - ends, lens = append(ends, dec.InputOffset()), append(lens, len(raw)) - break - } - } - if len(ends) == 0 { - return b, false - } - quoted, _ := jsontext.AppendQuote(nil, value) - out, at := make([]byte, 0, len(b)+len(ends)*len(quoted)), int64(0) - for i, end := range ends { - out = append(append(out, b[at:end-int64(lens[i])]...), quoted...) - at = end - } - return append(out, b[at:]...), true -} - -func substituteBody(out *http.Request, creds []Credential) []Credential { - if len(creds) == 0 || out.ContentLength <= 0 || out.ContentLength > maxInjectBody { - return nil - } - mt := mediaType(out.Header) - form := mt == "application/x-www-form-urlencoded" - if !form && mt != "application/json" { - return nil - } - b := make([]byte, out.ContentLength) - if n, err := io.ReadFull(out.Body, b); err != nil { - out.Body = io.NopCloser(io.MultiReader(bytes.NewReader(b[:n]), out.Body)) - return nil - } - var used []Credential - for _, c := range creds { - if form { - if s, ok := replaceParam(string(b), c.Query, c.Placeholder, c.Value); ok { - b, used = []byte(s), append(used, c) - } - } else if nb, ok := replaceMember(b, c.Query, c.Placeholder, c.Value); ok { - b, used = nb, append(used, c) - } - } - out.Body, out.ContentLength = io.NopCloser(bytes.NewReader(b)), int64(len(b)) - return used -} - -// echoes pairs each used query credential's value, as sent, URL-escaped and JSON-escaped in their common forms, with its placeholder in the same form, so a response that reflects the URL cannot hand the guest the value. -func echoes(used []Credential) (olds, news [][]byte) { - for _, c := range used { - if c.Query == "" { - continue - } - pairs := [][2]string{ - {c.Value, c.Placeholder}, - {url.QueryEscape(c.Value), url.QueryEscape(c.Placeholder)}, - {url.PathEscape(c.Value), url.PathEscape(c.Placeholder)}, - } - if v, err := jsontext.AppendQuote(nil, c.Value); err == nil { - ph, _ := jsontext.AppendQuote(nil, c.Placeholder) - v, ph = v[1:len(v)-1], ph[1:len(ph)-1] - pairs = append(pairs, [2]string{string(v), string(ph)}, - [2]string{strings.ReplaceAll(string(v), "/", `\/`), strings.ReplaceAll(string(ph), "/", `\/`)}) - } - for _, p := range pairs { - if !slices.ContainsFunc(olds, func(o []byte) bool { return string(o) == p[0] }) { - olds, news = append(olds, []byte(p[0])), append(news, []byte(p[1])) - } - } - } - return olds, news -} - -func scrubHeader(h http.Header, olds, news [][]byte) { - pairs := make([]string, 0, 2*len(olds)) - for i := range olds { - pairs = append(pairs, string(olds[i]), string(news[i])) - } - r := strings.NewReplacer(pairs...) - for _, vs := range h { - for i, v := range vs { - vs[i] = r.Replace(v) - } - } -} - func copyFlushing(w http.ResponseWriter, body io.Reader) int64 { rc := http.NewResponseController(w) if rc.Flush() != nil { @@ -773,3 +687,97 @@ func hostPort(authority string, def uint16) (host string, port uint16, ok bool) n, err := strconv.ParseUint(text, 10, 16) return host, uint16(n), err == nil && n != 0 } + +// replaceParam fills the first name=placeholder pair of a query or form string, leaving every other byte as sent. +func replaceParam(raw, name, placeholder, value string) string { + if !strings.Contains(raw, name) { + return raw + } + for rest, at := raw, 0; rest != ""; { + pair, next, _ := strings.Cut(rest, "&") + k, v, _ := strings.Cut(pair, "=") + if uk, err := url.QueryUnescape(k); err == nil && uk == name { + if uv, err := url.QueryUnescape(v); err == nil && uv == placeholder { + return raw[:at] + k + "=" + url.QueryEscape(value) + raw[at+len(pair):] + } + } + at, rest = at+len(pair)+1, next + } + return raw +} + +// replaceMember fills the first top-level string member name holding placeholder, leaving every other byte as sent. +func replaceMember(doc, name, placeholder, value string) string { + if !strings.Contains(doc, name) { + return doc + } + dec := jsontext.NewDecoder(strings.NewReader(doc)) + if tok, err := dec.ReadToken(); err != nil || tok.Kind() != '{' { + return doc + } + for dec.PeekKind() == '"' { + key, err := dec.ReadToken() + if err != nil { + return doc + } + if key.String() != name || dec.PeekKind() != '"' { + if dec.SkipValue() != nil { + return doc + } + continue + } + raw, err := dec.ReadValue() + if err != nil { + return doc + } + if v, unquoteErr := jsontext.AppendUnquote(nil, raw); unquoteErr != nil || string(v) != placeholder { + continue + } + quoted, err := jsontext.AppendQuote(nil, value) + if err != nil { + return doc + } + end := int(dec.InputOffset()) + return doc[:end-len(raw)] + string(quoted) + doc[end:] + } + return doc +} + +func substituteBody(out *http.Request, creds []Credential) []Credential { + if len(creds) == 0 || out.ContentLength <= 0 || out.ContentLength > maxInjectBody { + return nil + } + replace := replaceMember + switch mediaType(out.Header) { + case "application/x-www-form-urlencoded": + replace = replaceParam + case "application/json": + default: + return nil + } + b := make([]byte, out.ContentLength) + if n, err := io.ReadFull(out.Body, b); err != nil { + out.Body = io.NopCloser(io.MultiReader(bytes.NewReader(b[:n]), out.Body)) + return nil + } + body := string(b) + var filled []Credential + for _, c := range creds { + if next := replace(body, c.Query, c.Placeholder, c.Value); next != body { + body, filled = next, append(filled, c) + } + } + out.Body, out.ContentLength = io.NopCloser(strings.NewReader(body)), int64(len(body)) + return filled +} + +// escapings lists s as sent, query- and path-escaped, and JSON-escaped with and without \/; newScrubber pairs a value's list with its placeholder's by index. +func escapings(s string) [5]string { + j := s + if strings.ContainsAny(s, "\"\\\t") { + if q, err := jsontext.AppendQuote(nil, s); err == nil { + j = string(q[1 : len(q)-1]) + } + } + return [5]string{s, url.QueryEscape(s), url.PathEscape(s), j, strings.ReplaceAll(j, "/", `\/`)} +} diff --git a/sandboxd/pool/env_test.go b/sandboxd/pool/env_test.go index a83cf183..9c9edf6d 100644 --- a/sandboxd/pool/env_test.go +++ b/sandboxd/pool/env_test.go @@ -575,32 +575,29 @@ func TestInjectsSurviveARestartAndAReloadThatDropsTheInterceptStopsThem(t *testi } } -func TestAPlaceholderRidesTheEnvWriteIntoTheCredential(t *testing.T) { - eng := newFakeEngine() - eng.sockRoot = sockRoot(t) - m := egressManager(t, eng, config.PoolSpec{PoolKey: testKey, Warm: 1, Egress: interceptPolicy()}) - sb := mustClaim(t, m, testKey) - patch := types.EnvPatch{"KEY": {Value: "k", Guest: hostOnly, Inject: &types.EnvInject{Hosts: []string{"api.github.com"}, Header: "X-Api-Key", Placeholder: "@KEY@"}}} - if err := m.PatchEnv(t.Context(), sb.ID, patch, ""); err != nil { - t.Fatalf("inject with a placeholder: %v", err) - } - if creds := (claimSecrets{m: m, sb: sb}).Credentials("api.github.com"); len(creds) != 1 || creds[0].Placeholder != "@KEY@" { - t.Errorf("credentials %+v, want one carrying the placeholder", creds) - } -} - -func TestAQueryInjectOnAnInjectRuleRidesIntoTheCredential(t *testing.T) { - eng := newFakeEngine() - eng.sockRoot = sockRoot(t) - m := egressManager(t, eng, config.PoolSpec{PoolKey: testKey, Warm: 1, Egress: &egress.Policy{Allow: []egress.Rule{{Host: "*", Intercept: egress.InterceptInject}}}}) - sb := mustClaim(t, m, testKey) - patch := types.EnvPatch{"FB": {Value: "k", Guest: hostOnly, Inject: &types.EnvInject{Hosts: []string{"graph.example.com"}, Query: "access_token", Body: true, Placeholder: "@FB@"}}} - if err := m.PatchEnv(t.Context(), sb.ID, patch, ""); err != nil { - t.Fatalf("a query inject on an inject rule: %v", err) - } - creds := (claimSecrets{m: m, sb: sb}).Credentials("graph.example.com") - if len(creds) != 1 || creds[0].Query != "access_token" || !creds[0].Body || creds[0].Header != "" { - t.Errorf("credentials %+v, want one query credential with body", creds) +func TestAnInjectShapeRidesTheEnvWriteIntoTheCredential(t *testing.T) { + for _, tt := range []struct { + name string + policy *egress.Policy + host string + inject types.EnvInject + }{ + {"header with a placeholder", interceptPolicy(), "api.github.com", types.EnvInject{Header: "X-Api-Key", Placeholder: "@KEY@"}}, + {"query with body on an inject rule", &egress.Policy{Allow: []egress.Rule{{Host: "*", Intercept: egress.InterceptInject}}}, "graph.example.com", types.EnvInject{Query: "access_token", Body: true, Placeholder: "@FB@"}}, + } { + eng := newFakeEngine() + eng.sockRoot = sockRoot(t) + m := egressManager(t, eng, config.PoolSpec{PoolKey: testKey, Warm: 1, Egress: tt.policy}) + sb := mustClaim(t, m, testKey) + inject := tt.inject + inject.Hosts = []string{tt.host} + if err := m.PatchEnv(t.Context(), sb.ID, types.EnvPatch{"KEY": {Value: "k", Guest: hostOnly, Inject: &inject}}, ""); err != nil { + t.Fatalf("%s: %v", tt.name, err) + } + creds := (claimSecrets{m: m, sb: sb}).Credentials(tt.host) + if len(creds) != 1 || creds[0].Header != inject.Header || creds[0].Query != inject.Query || creds[0].Body != inject.Body || creds[0].Placeholder != inject.Placeholder { + t.Errorf("%s: credentials %+v, want one carrying %+v", tt.name, creds, inject) + } } } @@ -659,6 +656,18 @@ func BenchmarkClaimSecretsHeader(b *testing.B) { _ = claimSecrets{m: m, sb: sb}.Credentials("api.example.com") } }) + many := types.Env{} + for i := range 64 { + many[fmt.Sprintf("KEY_%d", i)] = types.EnvVar{Value: "k", Guest: hostOnly, Inject: &types.EnvInject{Hosts: []string{fmt.Sprintf("api%d.example.com", i), "*.vendor.example"}, Header: "X-Key"}} + } + sb = &types.Sandbox{} + sb.SetEnv(many) + b.Run("64-injects/miss", func(b *testing.B) { + b.ReportAllocs() + for b.Loop() { + _ = claimSecrets{m: m, sb: sb}.Credentials("other.example.com") + } + }) } func injectPatch(header string, hosts ...string) types.EnvPatch { diff --git a/sandboxd/pool/telemetry_test.go b/sandboxd/pool/telemetry_test.go index a0be0be8..e48d8df0 100644 --- a/sandboxd/pool/telemetry_test.go +++ b/sandboxd/pool/telemetry_test.go @@ -335,7 +335,7 @@ func TestEgressUsageEventNamesTheInjectedCredentials(t *testing.T) { t.Fatalf("read journal: %v", err) } var ev usageEvent - if err := json.Unmarshal(raw, &ev); err != nil { + if err = json.Unmarshal(raw, &ev); err != nil { t.Fatalf("decode %q: %v", raw, err) } if ev.Event != "egress" || ev.Reference != "graph.example.com" || ev.Secret != "claim:FB" { diff --git a/sandboxd/pool/view.go b/sandboxd/pool/view.go index a50ab498..1adfc1c7 100644 --- a/sandboxd/pool/view.go +++ b/sandboxd/pool/view.go @@ -89,7 +89,7 @@ func (v *configView) injectGap(eval egress.Evaluator, ok bool, sb *types.Sandbox for _, h := range inj.Hosts { if !ok || !eval.InterceptsHost(h) { gap = append(gap, fmt.Sprintf("%s: %s is not intercepted", name, h)) - } else if v.secretSets(pool, h, inj.Header) { + } else if inj.Header != "" && v.secretSets(pool, h, inj.Header) { gap = append(gap, fmt.Sprintf("%s: the pool's secret already sets %s on %s", name, inj.Header, h)) } } diff --git a/sandboxd/types/types.go b/sandboxd/types/types.go index 226a1f73..9970323e 100644 --- a/sandboxd/types/types.go +++ b/sandboxd/types/types.go @@ -512,10 +512,15 @@ func (k PoolKey) Validate() error { return nil } -// envView is an immutable env with its inject entries named in order. +// envView is an immutable env with its inject entries in name order. type envView struct { env Env - injects []string + injects []namedVar +} + +type namedVar struct { + name string + v EnvVar } // Sandbox is the node-local record of one pooled or claimed VM. @@ -621,10 +626,10 @@ func (s *Sandbox) SetEnv(e Env) { view := &envView{env: e} for name, v := range e { if v.Inject != nil && !v.InGuest() { - view.injects = append(view.injects, name) + view.injects = append(view.injects, namedVar{name, v}) } } - slices.Sort(view.injects) + slices.SortFunc(view.injects, func(a, b namedVar) int { return strings.Compare(a.name, b.name) }) s.live.Store(view) } @@ -643,8 +648,8 @@ func (s *Sandbox) Injections(host string) iter.Seq2[string, EnvVar] { if view == nil { return } - for _, name := range view.injects { - if v := view.env[name]; v.Inject.Matches(host) && !yield(name, v) { + for _, e := range view.injects { + if e.v.Inject.Matches(host) && !yield(e.name, e.v) { return } } diff --git a/scripts/credentials-e2e.sh b/scripts/credentials-e2e.sh index 51bd8e15..d615401b 100755 --- a/scripts/credentials-e2e.sh +++ b/scripts/credentials-e2e.sh @@ -124,13 +124,12 @@ echo "== inject pool: conditional interception, query and body placeholders" "$DATA/credsmoke" -addr "$ADDR" -token "$TOKEN" -template "$TEMPLATE" -echo "$ECHO" -other "$OTHER" \ -inject-size medium | tee "$DATA/inject.log" grep -q '"ev":"egress".*"secret":"claim:SERP"' "$DATA/state/usage.jsonl" || { echo "usage names no claim:SERP"; exit 1; } -vault=$(awk '/^VAULT / {sub(/^VAULT /, ""); print}' "$DATA/inject.log") -for v in "${vault%%|*}" "${vault##*|}"; do +while IFS= read -r v; do if grep '"op":"egress"' "$DATA/state/audit.jsonl" | grep -qF "$v" || grep -qF "$v" "$DATA/state/usage.jsonl"; then echo "an inject value reached an egress audit record or the usage journal" exit 1 fi -done +done < <(sed -n 's/^VAULT //p' "$DATA/inject.log") echo "== restart sandboxd, recheck the surviving claim" kill "$DAEMON_PID" From b19fe5f67c738c10ce6f63528e13a5e8f6d4eb26 Mon Sep 17 00:00:00 2001 From: CMGS Date: Sun, 4 Oct 2026 11:19:05 +0800 Subject: [PATCH 06/11] egress: a scrubbed value that contains another matches first, and an encoded parameter name is filled Echo forms were tried in credential-name order, so a filled app id or app secret matched inside an app token (id|secret) and the rest of the token reached the guest; the forms now try longest first. The name pre-check compared the decoded name with raw query bytes and skipped a name a client percent-encodes, such as user[token]. --- sandboxd/egress/intercept_test.go | 23 ++++++++++++++++++++ sandboxd/egress/proxy.go | 36 ++++++++++++++++--------------- 2 files changed, 42 insertions(+), 17 deletions(-) diff --git a/sandboxd/egress/intercept_test.go b/sandboxd/egress/intercept_test.go index 015ca886..fb96256d 100644 --- a/sandboxd/egress/intercept_test.go +++ b/sandboxd/egress/intercept_test.go @@ -463,6 +463,29 @@ func TestACredentialFillsOnlyTheFirstMatchingParameter(t *testing.T) { } } +func TestAValueThatContainsAnotherIsScrubbedWhole(t *testing.T) { + sc := newScrubber([]Credential{ + {Name: "FB_APP_ID", Query: "app_id", Value: "1234567890", Placeholder: "@ID@"}, + {Name: "FB_SECRET", Query: "secret", Value: "s3cr3tAppSecret", Placeholder: "@SEC@"}, + {Name: "FB_TOKEN", Query: "access_token", Value: "1234567890|s3cr3tAppSecret", Placeholder: "@TOK@"}, + }) + echo := "access_token=1234567890%7Cs3cr3tAppSecret&raw=1234567890|s3cr3tAppSecret" + h := http.Header{"Location": {"/next?" + echo}} + sc.header(h) + body, _ := io.ReadAll(&scrubReader{src: strings.NewReader(echo), sc: sc}) + for _, got := range []string{h.Get("Location"), string(body)} { + if strings.Contains(got, "s3cr3t") || strings.Contains(got, "@ID@") { + t.Errorf("scrubbed %q, want the token replaced whole", got) + } + } +} + +func TestAnEncodedParameterNameIsFilled(t *testing.T) { + if got := replaceParam("a=1&user%5Btoken%5D=%40K%40", "user[token]", "@K@", "v"); got != "a=1&user%5Btoken%5D=v" { + t.Errorf("an encoded name filled as %q, want its value replaced", got) + } +} + func TestARangeCannotSplitAScrubbedQueryValue(t *testing.T) { secrets := credSecrets{creds: map[string][]Credential{"example.com": {{Name: "K", Query: "api_key", Value: "SECRETVALUE0123456789", Placeholder: "@K@"}}}} p, guestRoots, upstream := interceptProxySecrets(t, Policy{Allow: []Rule{{Host: "*", Intercept: InterceptInject}}}, secrets, nil) diff --git a/sandboxd/egress/proxy.go b/sandboxd/egress/proxy.go index b26b277e..d90e6f4f 100644 --- a/sandboxd/egress/proxy.go +++ b/sandboxd/egress/proxy.go @@ -486,11 +486,15 @@ func (c *countingBody) Read(b []byte) (int, error) { return n, err } +type echo struct { + old string + raw []byte + placeholder string +} + // scrubber turns each filled query credential's value, in the escapings a reflected URL carries, back into its placeholder. type scrubber struct { - olds []string - oldRaw [][]byte - news []string + echoes []echo first [256]bool longest int } @@ -503,21 +507,22 @@ func newScrubber(filled []Credential) *scrubber { for _, c := range filled { values, placeholders := escapings(c.Value), escapings(c.Placeholder) for i, old := range values { - if !slices.Contains(sc.olds, old) { - sc.olds, sc.oldRaw, sc.news = append(sc.olds, old), append(sc.oldRaw, []byte(old)), append(sc.news, placeholders[i]) + if !slices.ContainsFunc(sc.echoes, func(e echo) bool { return e.old == old }) { + sc.echoes = append(sc.echoes, echo{old: old, raw: []byte(old), placeholder: placeholders[i]}) sc.first[old[0]], sc.longest = true, max(sc.longest, len(old)) } } } + slices.SortStableFunc(sc.echoes, func(a, b echo) int { return cmp.Compare(len(b.old), len(a.old)) }) return sc } func (sc *scrubber) header(h http.Header) { for _, vs := range h { for i, v := range vs { - for j, old := range sc.olds { - if strings.Contains(v, old) { - v = strings.ReplaceAll(v, old, sc.news[j]) + for _, e := range sc.echoes { + if strings.Contains(v, e.old) { + v = strings.ReplaceAll(v, e.old, e.placeholder) } } vs[i] = v @@ -558,8 +563,8 @@ func (s *scrubReader) scan() { buf, i := s.pending, 0 for { at, k := -1, 0 - for j, old := range s.sc.oldRaw { - if idx := bytes.Index(buf[i:], old); idx >= 0 && (at < 0 || idx < at) { + for j, e := range s.sc.echoes { + if idx := bytes.Index(buf[i:], e.raw); idx >= 0 && (at < 0 || idx < at) { at, k = idx, j } } @@ -567,8 +572,8 @@ func (s *scrubReader) scan() { break } s.out.Write(buf[i : i+at]) - s.out.WriteString(s.sc.news[k]) - i += at + len(s.sc.oldRaw[k]) + s.out.WriteString(s.sc.echoes[k].placeholder) + i += at + len(s.sc.echoes[k].raw) } hold := 0 if s.err == nil { @@ -580,8 +585,8 @@ func (s *scrubReader) scan() { func (s *scrubReader) partial(tail []byte) int { for k := min(len(tail), s.sc.longest-1); k > 0; k-- { - if s.sc.first[tail[len(tail)-k]] && slices.ContainsFunc(s.sc.oldRaw, func(old []byte) bool { - return len(old) > k && bytes.HasPrefix(old, tail[len(tail)-k:]) + if s.sc.first[tail[len(tail)-k]] && slices.ContainsFunc(s.sc.echoes, func(e echo) bool { + return len(e.raw) > k && bytes.HasPrefix(e.raw, tail[len(tail)-k:]) }) { return k } @@ -690,9 +695,6 @@ func hostPort(authority string, def uint16) (host string, port uint16, ok bool) // replaceParam fills the first name=placeholder pair of a query or form string, leaving every other byte as sent. func replaceParam(raw, name, placeholder, value string) string { - if !strings.Contains(raw, name) { - return raw - } for rest, at := raw, 0; rest != ""; { pair, next, _ := strings.Cut(rest, "&") k, v, _ := strings.Cut(pair, "=") From 3e30cda16543281a043684588e0f0414bf531490 Mon Sep 17 00:00:00 2001 From: CMGS Date: Sun, 4 Oct 2026 11:35:34 +0800 Subject: [PATCH 07/11] egress: a contained value ending a read waits for the longer one, and an escaped JSON name is filled Longest-first only ordered whole matches inside one read: when a read ended right after an app id that also starts the app token, the id was replaced at once and the token's tail streamed out in the next read. A match is now held while a longer echo may still complete from it. The JSON pre-check compared the decoded name with raw bytes and skipped a key written with escapes; it now skips only a body with no backslash. --- sandboxd/egress/intercept_test.go | 20 ++++++++++++++++++++ sandboxd/egress/proxy.go | 22 ++++++++++++++++++---- 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/sandboxd/egress/intercept_test.go b/sandboxd/egress/intercept_test.go index fb96256d..8038d446 100644 --- a/sandboxd/egress/intercept_test.go +++ b/sandboxd/egress/intercept_test.go @@ -480,10 +480,30 @@ func TestAValueThatContainsAnotherIsScrubbedWhole(t *testing.T) { } } +func TestAContainedValueEndingAReadWaitsForTheLongerOne(t *testing.T) { + sc := newScrubber([]Credential{ + {Name: "FB_APP_ID", Query: "app_id", Value: "1234567890", Placeholder: "@ID@"}, + {Name: "FB_TOKEN", Query: "access_token", Value: "1234567890|SECRET", Placeholder: "@TOK@"}, + }) + for _, chunks := range [][]string{{"x 1234567890", "|SECRET y"}, {"x 1234567890", " y"}, {"x 1234567890"}} { + readers := make([]io.Reader, len(chunks)) + for i, c := range chunks { + readers[i] = strings.NewReader(c) + } + out, _ := io.ReadAll(&scrubReader{src: io.MultiReader(readers...), sc: sc}) + if strings.Contains(string(out), "SECRET") || strings.Contains(string(out), "1234567890") { + t.Errorf("chunks %q scrubbed to %q, want every value replaced", chunks, out) + } + } +} + func TestAnEncodedParameterNameIsFilled(t *testing.T) { if got := replaceParam("a=1&user%5Btoken%5D=%40K%40", "user[token]", "@K@", "v"); got != "a=1&user%5Btoken%5D=v" { t.Errorf("an encoded name filled as %q, want its value replaced", got) } + if got := replaceMember(`{"access\u005ftoken":"@FB@"}`, "access_token", "@FB@", "v"); got != `{"access\u005ftoken":"v"}` { + t.Errorf("an escaped JSON name filled as %s, want its value replaced", got) + } } func TestARangeCannotSplitAScrubbedQueryValue(t *testing.T) { diff --git a/sandboxd/egress/proxy.go b/sandboxd/egress/proxy.go index d90e6f4f..6139fb7d 100644 --- a/sandboxd/egress/proxy.go +++ b/sandboxd/egress/proxy.go @@ -560,7 +560,7 @@ func (s *scrubReader) Read(b []byte) (int, error) { } func (s *scrubReader) scan() { - buf, i := s.pending, 0 + buf, i, held := s.pending, 0, false for { at, k := -1, 0 for j, e := range s.sc.echoes { @@ -572,17 +572,31 @@ func (s *scrubReader) scan() { break } s.out.Write(buf[i : i+at]) + i += at + if s.err == nil && s.longerMayFollow(buf[i:], len(s.sc.echoes[k].raw)) { + held = true + break + } s.out.WriteString(s.sc.echoes[k].placeholder) - i += at + len(s.sc.echoes[k].raw) + i += len(s.sc.echoes[k].raw) } hold := 0 - if s.err == nil { + switch { + case held: + hold = len(buf) - i + case s.err == nil: hold = s.partial(buf[i:]) } s.out.Write(buf[i : len(buf)-hold]) s.pending = s.pending[:copy(s.pending, buf[len(buf)-hold:])] } +func (s *scrubReader) longerMayFollow(rest []byte, n int) bool { + return slices.ContainsFunc(s.sc.echoes, func(e echo) bool { + return len(e.raw) > n && len(rest) < len(e.raw) && bytes.HasPrefix(e.raw, rest) + }) +} + func (s *scrubReader) partial(tail []byte) int { for k := min(len(tail), s.sc.longest-1); k > 0; k-- { if s.sc.first[tail[len(tail)-k]] && slices.ContainsFunc(s.sc.echoes, func(e echo) bool { @@ -710,7 +724,7 @@ func replaceParam(raw, name, placeholder, value string) string { // replaceMember fills the first top-level string member name holding placeholder, leaving every other byte as sent. func replaceMember(doc, name, placeholder, value string) string { - if !strings.Contains(doc, name) { + if !strings.Contains(doc, `\`) && (!strings.Contains(doc, name) || !strings.Contains(doc, placeholder)) { return doc } dec := jsontext.NewDecoder(strings.NewReader(doc)) From 8b1ad8a885c64b6bb63ad5d29f4f7a7626eda94c Mon Sep 17 00:00:00 2001 From: CMGS Date: Sun, 4 Oct 2026 11:41:37 +0800 Subject: [PATCH 08/11] egress: EvalHost returns on the first always rule, and the scrub drops the JSON-escaped form and the short-body restore The always rule outranks every other, so EvalHost returns on it again instead of scanning on (-3% per CONNECT). A JSON-escaped echo differs from the raw value only for a credential holding a quote, backslash or tab, which no query key does. A body shorter than its Content-Length fails the upstream request with or without the restored prefix. The SOCKS docs say an inject rule splices on 1080. --- docs/egress.md | 7 ++++--- sandboxd/egress/bench_test.go | 1 + sandboxd/egress/policy.go | 6 +++++- sandboxd/egress/proxy.go | 15 ++++----------- 4 files changed, 14 insertions(+), 15 deletions(-) diff --git a/docs/egress.md b/docs/egress.md index cb037656..cbcc472d 100644 --- a/docs/egress.md +++ b/docs/egress.md @@ -53,7 +53,8 @@ through the same code: a rule with a nonempty `methods` list that omits `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 pool policy sets +the tunnel is refused. An `"inject"` rule is a plain one on 1080: the door never +injects, so it splices whatever credentials the claim holds. 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 @@ -367,8 +368,8 @@ same `PATCH`, accepted on a hibernated or archived claim without waking it. (if any) the request carries wins. An empty value injects nothing. - **Echo.** Origins reflect the request URL: a redirect's `Location`, a paging link, an echoed query. On a request that carried a `query` - credential, its value, as sent, query- or path-escaped, or JSON-escaped - (with or without `\/`), reaches the guest as the placeholder in every + credential, its value, as sent, query- or path-escaped, or with `/` written + `\/` as JSON may, reaches the guest as the placeholder in every response header and in the response body. The proxy drops the guest's `Accept-Encoding`, `Range` and `If-Range` on such a request, so it reads the whole body uncompressed, and streams it holding back only a tail that may diff --git a/sandboxd/egress/bench_test.go b/sandboxd/egress/bench_test.go index 93071e1e..79c27170 100644 --- a/sandboxd/egress/bench_test.go +++ b/sandboxd/egress/bench_test.go @@ -113,6 +113,7 @@ func BenchmarkInjectAndScrubHeaders(b *testing.B) { {"query", "a=1&access_token=%40K%40", "", "", Credential{Name: "K", Query: "access_token", Value: key, Placeholder: "@K@"}}, {"form-body", "", "application/x-www-form-urlencoded", "m=hi&access_token=%40K%40", Credential{Name: "K", Query: "access_token", Body: true, Value: key, Placeholder: "@K@"}}, {"json-body", "", "application/json", `{"m":"hi","access_token":"@K@"}`, Credential{Name: "K", Query: "access_token", Body: true, Value: key, Placeholder: "@K@"}}, + {"json-body-escaped-unnamed", "", "application/json", `{"message":"` + strings.Repeat(`hello\nworld `, 150) + `"}`, Credential{Name: "K", Query: "access_token", Body: true, Value: key, Placeholder: "@K@"}}, } { creds := map[string][]Credential{} if arm.cred.Name != "" { diff --git a/sandboxd/egress/policy.go b/sandboxd/egress/policy.go index 48a49f53..31d1aec8 100644 --- a/sandboxd/egress/policy.go +++ b/sandboxd/egress/policy.go @@ -155,7 +155,11 @@ func (p Policy) EvalHost(host string, port uint16) (Rule, Decision) { host = strings.ToLower(host) best := -1 for i, r := range p.Allow { - if r.matchHost(host) && r.matchPort(port) && (best < 0 || r.Intercept > p.Allow[best].Intercept) { + switch { + case !r.matchHost(host) || !r.matchPort(port): + case r.Intercept == InterceptAlways: + return r, DecisionAllow + case best < 0 || r.Intercept > p.Allow[best].Intercept: best = i } } diff --git a/sandboxd/egress/proxy.go b/sandboxd/egress/proxy.go index 6139fb7d..ace0da24 100644 --- a/sandboxd/egress/proxy.go +++ b/sandboxd/egress/proxy.go @@ -772,8 +772,7 @@ func substituteBody(out *http.Request, creds []Credential) []Credential { return nil } b := make([]byte, out.ContentLength) - if n, err := io.ReadFull(out.Body, b); err != nil { - out.Body = io.NopCloser(io.MultiReader(bytes.NewReader(b[:n]), out.Body)) + if _, err := io.ReadFull(out.Body, b); err != nil { return nil } body := string(b) @@ -787,13 +786,7 @@ func substituteBody(out *http.Request, creds []Credential) []Credential { return filled } -// escapings lists s as sent, query- and path-escaped, and JSON-escaped with and without \/; newScrubber pairs a value's list with its placeholder's by index. -func escapings(s string) [5]string { - j := s - if strings.ContainsAny(s, "\"\\\t") { - if q, err := jsontext.AppendQuote(nil, s); err == nil { - j = string(q[1 : len(q)-1]) - } - } - return [5]string{s, url.QueryEscape(s), url.PathEscape(s), j, strings.ReplaceAll(j, "/", `\/`)} +// escapings lists s as sent, query- and path-escaped, and with \/; newScrubber pairs a value's list with its placeholder's by index. +func escapings(s string) [4]string { + return [4]string{s, url.QueryEscape(s), url.PathEscape(s), strings.ReplaceAll(s, "/", `\/`)} } From 6c91afdce6e0dc42f20894eae6d23464a64327bf Mon Sep 17 00:00:00 2001 From: CMGS Date: Sun, 4 Oct 2026 11:58:45 +0800 Subject: [PATCH 09/11] review: producer methods trail the type they produce, echoForm, and shorter comments Proxy.inject returns a scrubber and the test send returns a reply, so each moves below the block of the type it produces (/code layout). echo becomes echoForm, no longer shadowed by test locals named echo. Six comments drop restated facts. --- sandboxd/egress/intercept_test.go | 28 ++++---- sandboxd/egress/policy.go | 4 +- sandboxd/egress/proxy.go | 104 +++++++++++++++--------------- 3 files changed, 68 insertions(+), 68 deletions(-) diff --git a/sandboxd/egress/intercept_test.go b/sandboxd/egress/intercept_test.go index 8038d446..15ed6b8e 100644 --- a/sandboxd/egress/intercept_test.go +++ b/sandboxd/egress/intercept_test.go @@ -737,20 +737,6 @@ func newRequest(t *testing.T, method, target, body string) *http.Request { return req } -func send(t *testing.T, tc *tls.Conn, req *http.Request) reply { - t.Helper() - if err := req.Write(tc); err != nil { - t.Fatalf("write request: %v", err) - } - resp, err := http.ReadResponse(bufio.NewReader(tc), req) - if err != nil { - t.Fatalf("read response: %v", err) - } - body, _ := io.ReadAll(resp.Body) - _ = resp.Body.Close() - return reply{status: resp.StatusCode, header: resp.Header, body: string(body)} -} - type echoed struct { Key, Empty, Query, Body string Length int64 @@ -773,6 +759,20 @@ func (r reply) seen(t *testing.T) (echoed, string) { return e, reflected } +func send(t *testing.T, tc *tls.Conn, req *http.Request) reply { + t.Helper() + if err := req.Write(tc); err != nil { + t.Fatalf("write request: %v", err) + } + resp, err := http.ReadResponse(bufio.NewReader(tc), req) + if err != nil { + t.Fatalf("read response: %v", err) + } + body, _ := io.ReadAll(resp.Body) + _ = resp.Body.Close() + return reply{status: resp.StatusCode, header: resp.Header, body: string(body)} +} + func readPreamble(tb testing.TB, conn net.Conn) string { tb.Helper() var sb strings.Builder diff --git a/sandboxd/egress/policy.go b/sandboxd/egress/policy.go index 31d1aec8..a3bd2427 100644 --- a/sandboxd/egress/policy.go +++ b/sandboxd/egress/policy.go @@ -21,7 +21,7 @@ const ( // Decision is the policy verdict for one request. type Decision int -// InterceptMode is a rule's "intercept": false, true, or "inject" for only the claims that inject on the host; a higher mode outranks a lower one for the same host. +// InterceptMode is a rule's "intercept"; "inject" intercepts only for a claim that injects on the host, and a higher mode outranks a lower one. type InterceptMode uint8 func (m InterceptMode) MarshalJSONTo(enc *jsontext.Encoder) error { @@ -169,7 +169,7 @@ func (p Policy) EvalHost(host string, port uint16) (Rule, Decision) { return p.Allow[best], DecisionAllow } -// EvalInner matches only rules of the mode EvalHost picks for the host, so a plain rule cannot shadow or rescue an intercept rule and an always rule shadows an inject rule. +// EvalInner matches only rules of the mode EvalHost picks for the host, so a lower mode never shadows or rescues it. func (p Policy) EvalInner(host, method string, port uint16) (Rule, Decision) { top, d := p.EvalHost(host, port) if d == DecisionDeny || top.Intercept == InterceptOff { diff --git a/sandboxd/egress/proxy.go b/sandboxd/egress/proxy.go index ace0da24..419b5771 100644 --- a/sandboxd/egress/proxy.go +++ b/sandboxd/egress/proxy.go @@ -279,7 +279,7 @@ func (p *Proxy) serveConnect(w http.ResponseWriter, r *http.Request) { p.reportTransfer(ev, early+sent, received) } -// tunnelDecision treats an inject rule as a plain one when injects is false, on a door that never injects. +// tunnelDecision treats an inject rule as a plain one when injects is false. func (p *Proxy) tunnelDecision(host string, port uint16, injects bool) (decision Decision, intercept bool) { // host-gate interception: the tunnel's CONNECT verb is not the request method. if p.ca != nil { @@ -342,7 +342,7 @@ func (p *Proxy) relay(w http.ResponseWriter, r *http.Request, ev Event, rule Rul var sc *scrubber ev.Injected, sc = p.inject(rule, out, ev.Host, mitm) if sc != nil { - // the transport then decodes compression itself, and no range splits a value, so the scrub sees every echo whole. + // the scrub needs the whole body uncompressed. for _, h := range []string{"Accept-Encoding", "Range", "If-Range"} { out.Header.Del(h) } @@ -426,48 +426,6 @@ func (p *Proxy) reportTransfer(ev Event, sent, received int64) { } } -// inject sets the rule's secret, then on an intercepted request the claim credentials the guest asked for, and returns a scrubber for the query credentials it filled. -func (p *Proxy) inject(rule Rule, out *http.Request, host string, mitm bool) (string, *scrubber) { - if p.secrets == nil { - return "", nil - } - var names, set []string - if rule.Secret != "" { - if header, value, ok := p.secrets.Header(rule.Secret); ok && value != "" { - out.Header.Set(header, value) - names, set = append(names, rule.Secret), append(set, header) - } - } - if !mitm { - return strings.Join(names, ","), nil - } - var filled, body []Credential - for _, c := range p.secrets.Credentials(strings.ToLower(host)) { - if c.Value == "" { - continue - } - if c.Query != "" { - if q := replaceParam(out.URL.RawQuery, c.Query, c.Placeholder, c.Value); q != out.URL.RawQuery { - out.URL.RawQuery, filled = q, append(filled, c) - } else if c.Body { - body = append(body, c) - } - continue - } - if (c.Placeholder != "" && out.Header.Get(c.Header) != c.Placeholder) || - slices.ContainsFunc(set, func(s string) bool { return strings.EqualFold(s, c.Header) }) { - continue - } - out.Header.Set(c.Header, c.Value) - names, set = append(names, "claim:"+c.Name), append(set, c.Header) - } - filled = append(filled, substituteBody(out, body)...) - for _, c := range filled { - names = append(names, "claim:"+c.Name) - } - return strings.Join(names, ","), newScrubber(filled) -} - func (p *Proxy) record(ev Event) { if p.audit != nil { p.audit(ev) @@ -486,7 +444,7 @@ func (c *countingBody) Read(b []byte) (int, error) { return n, err } -type echo struct { +type echoForm struct { old string raw []byte placeholder string @@ -494,7 +452,7 @@ type echo struct { // scrubber turns each filled query credential's value, in the escapings a reflected URL carries, back into its placeholder. type scrubber struct { - echoes []echo + echoes []echoForm first [256]bool longest int } @@ -507,13 +465,13 @@ func newScrubber(filled []Credential) *scrubber { for _, c := range filled { values, placeholders := escapings(c.Value), escapings(c.Placeholder) for i, old := range values { - if !slices.ContainsFunc(sc.echoes, func(e echo) bool { return e.old == old }) { - sc.echoes = append(sc.echoes, echo{old: old, raw: []byte(old), placeholder: placeholders[i]}) + if !slices.ContainsFunc(sc.echoes, func(e echoForm) bool { return e.old == old }) { + sc.echoes = append(sc.echoes, echoForm{old: old, raw: []byte(old), placeholder: placeholders[i]}) sc.first[old[0]], sc.longest = true, max(sc.longest, len(old)) } } } - slices.SortStableFunc(sc.echoes, func(a, b echo) int { return cmp.Compare(len(b.old), len(a.old)) }) + slices.SortStableFunc(sc.echoes, func(a, b echoForm) int { return cmp.Compare(len(b.old), len(a.old)) }) return sc } @@ -530,6 +488,48 @@ func (sc *scrubber) header(h http.Header) { } } +// inject sets the rule's secret, then on an intercepted request the claim credentials the guest asked for, and returns a scrubber for the query credentials it filled. +func (p *Proxy) inject(rule Rule, out *http.Request, host string, mitm bool) (string, *scrubber) { + if p.secrets == nil { + return "", nil + } + var names, set []string + if rule.Secret != "" { + if header, value, ok := p.secrets.Header(rule.Secret); ok && value != "" { + out.Header.Set(header, value) + names, set = append(names, rule.Secret), append(set, header) + } + } + if !mitm { + return strings.Join(names, ","), nil + } + var filled, bodyCreds []Credential + for _, c := range p.secrets.Credentials(strings.ToLower(host)) { + if c.Value == "" { + continue + } + if c.Query != "" { + if q := replaceParam(out.URL.RawQuery, c.Query, c.Placeholder, c.Value); q != out.URL.RawQuery { + out.URL.RawQuery, filled = q, append(filled, c) + } else if c.Body { + bodyCreds = append(bodyCreds, c) + } + continue + } + if (c.Placeholder != "" && out.Header.Get(c.Header) != c.Placeholder) || + slices.ContainsFunc(set, func(s string) bool { return strings.EqualFold(s, c.Header) }) { + continue + } + out.Header.Set(c.Header, c.Value) + names, set = append(names, "claim:"+c.Name), append(set, c.Header) + } + filled = append(filled, substituteBody(out, bodyCreds)...) + for _, c := range filled { + names = append(names, "claim:"+c.Name) + } + return strings.Join(names, ","), newScrubber(filled) +} + // scrubReader streams a response body through a scrubber, holding back only a tail that may begin a match. type scrubReader struct { src io.Reader @@ -592,14 +592,14 @@ func (s *scrubReader) scan() { } func (s *scrubReader) longerMayFollow(rest []byte, n int) bool { - return slices.ContainsFunc(s.sc.echoes, func(e echo) bool { + return slices.ContainsFunc(s.sc.echoes, func(e echoForm) bool { return len(e.raw) > n && len(rest) < len(e.raw) && bytes.HasPrefix(e.raw, rest) }) } func (s *scrubReader) partial(tail []byte) int { for k := min(len(tail), s.sc.longest-1); k > 0; k-- { - if s.sc.first[tail[len(tail)-k]] && slices.ContainsFunc(s.sc.echoes, func(e echo) bool { + if s.sc.first[tail[len(tail)-k]] && slices.ContainsFunc(s.sc.echoes, func(e echoForm) bool { return len(e.raw) > k && bytes.HasPrefix(e.raw, tail[len(tail)-k:]) }) { return k @@ -786,7 +786,7 @@ func substituteBody(out *http.Request, creds []Credential) []Credential { return filled } -// escapings lists s as sent, query- and path-escaped, and with \/; newScrubber pairs a value's list with its placeholder's by index. +// escapings returns the forms an echo of s takes; newScrubber pairs a value's forms with its placeholder's by index. func escapings(s string) [4]string { return [4]string{s, url.QueryEscape(s), url.PathEscape(s), strings.ReplaceAll(s, "/", `\/`)} } From 940889d6abc0f4a8b0a6ffd0180525e7f785cf09 Mon Sep 17 00:00:00 2001 From: CMGS Date: Sun, 4 Oct 2026 12:06:15 +0800 Subject: [PATCH 10/11] egress: plainer tunnelDecision, intercept codec and header scrub; credsmoke shares one context The tunnel gate is one condition instead of a switch with an empty case, the intercept decoder has no fallthrough, and the header scrub calls ReplaceAll alone (it returns the value untouched on no match). The pool buffer size is written as the two scrub chunks newScrubReader splits it into. credsmoke creates one context in main; tests table their named cases with t.Run and share a chunked reader; docs name the CONNECT cost of an inject rule and that only all-true intercept policies refuse SOCKS5. A/B against 8b1ad8a, six interleaved runs: geomean +0.3%, no path moved. --- docs/egress.md | 5 ++- e2e/cmd/credsmoke/main.go | 26 +++++------- sandboxd/egress/bench_test.go | 6 +-- sandboxd/egress/intercept_test.go | 68 +++++++++++++++++-------------- sandboxd/egress/policy.go | 14 +++---- sandboxd/egress/policy_test.go | 2 +- sandboxd/egress/proxy.go | 11 ++--- sandboxd/pool/env_test.go | 28 +++++++------ 8 files changed, 79 insertions(+), 81 deletions(-) diff --git a/docs/egress.md b/docs/egress.md index cbcc472d..c19ef3fc 100644 --- a/docs/egress.md +++ b/docs/egress.md @@ -61,7 +61,7 @@ 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 a nonempty `methods` list without -`CONNECT`, or all `intercept`, is rejected at load. A pool whose policy does +`CONNECT`, or all `intercept: true`, 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 (with `audit_log` on) carry `"method":"SOCKS5"`. @@ -293,7 +293,8 @@ gateway key per user, while the guest holds a placeholder. The proxy reads a snapshot of the claim's env that each env write publishes, without a lock, once per request whose matched rule names a secret or whose -host has a claim credential; other requests pay nothing. +host has a claim credential, and once per `CONNECT` an `"inject"` rule covers; +other requests pay nothing. ### Claim credentials diff --git a/e2e/cmd/credsmoke/main.go b/e2e/cmd/credsmoke/main.go index d13d3ad0..2a8382bc 100644 --- a/e2e/cmd/credsmoke/main.go +++ b/e2e/cmd/credsmoke/main.go @@ -1,6 +1,6 @@ -// credsmoke is the claim-credential acceptance: claims on one intercept pool each -// send their own env-injected header to an HTTPS echo through the proxy, the guest -// never holds the value, and rotation, refusal, clearing and forks behave as the API says. +// credsmoke is the claim-credential acceptance: claims each send their own env-injected header, +// query or body credential through the proxy (-inject-size: on an "inject" pool), the guest never +// holds the value, and rotation, refusal, clearing and forks behave as the API says. package main import ( @@ -39,24 +39,24 @@ func main() { injectSize := flag.String("inject-size", "", "size of the template's pool whose only rule is intercept \"inject\"; empty skips that leg") other := flag.String("other", "example.com", "a second HTTPS host the inject pool must splice") flag.Parse() + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute) var err error switch { case *reattach != "": - err = recheck(*addr, *token, *echo, *reattach) + err = recheck(ctx, *addr, *token, *echo, *reattach) case *injectSize != "": - err = conditional(*addr, *token, *template, sandbox.Size(*injectSize), *echo, *other) + err = conditional(ctx, *addr, *token, *template, sandbox.Size(*injectSize), *echo, *other) default: - err = run(*addr, *token, *template, *echo, *secret, *sse) + err = run(ctx, *addr, *token, *template, *echo, *secret, *sse) } + cancel() if err != nil { fmt.Fprintln(os.Stderr, "credsmoke:", err) os.Exit(1) } } -func run(addr, token, template, echo, secret, sse string) error { - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute) - defer cancel() +func run(ctx context.Context, addr, token, template, echo, secret, sse string) error { stamp := strconv.FormatInt(time.Now().UnixNano(), 10) valueA, valueB := "key-a-"+stamp, "key-b-"+stamp @@ -180,9 +180,7 @@ func run(addr, token, template, echo, secret, sse string) error { return nil } -func recheck(addr, token, echo, reattach string) error { - ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) - defer cancel() +func recheck(ctx context.Context, addr, token, echo, reattach string) error { parts := strings.SplitN(reattach, ":", 3) if len(parts) != 3 { return fmt.Errorf("-reattach %q, want id:token:value", reattach) @@ -204,9 +202,7 @@ func recheck(addr, token, echo, reattach string) error { return nil } -func conditional(addr, token, template string, size sandbox.Size, echo, other string) error { - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute) - defer cancel() +func conditional(ctx context.Context, addr, token, template string, size sandbox.Size, echo, other string) error { claim := func() (*sandbox.Sandbox, error) { _, sb, err := harness.Claim(ctx, addr, token, template, sandbox.WithNetwork(sandbox.NetNone), sandbox.WithSize(size)) return sb, err diff --git a/sandboxd/egress/bench_test.go b/sandboxd/egress/bench_test.go index 79c27170..72879ddb 100644 --- a/sandboxd/egress/bench_test.go +++ b/sandboxd/egress/bench_test.go @@ -149,11 +149,11 @@ func BenchmarkScrubBody(b *testing.B) { }{ {"64KiB-page", func() io.Reader { return bytes.NewReader(page) }}, {"sse-256-events", func() io.Reader { - events := make([]io.Reader, 256) + events := make([]string, 256) for i := range events { - events[i] = bytes.NewReader(event) + events[i] = string(event) } - return io.MultiReader(events...) + return chunked(events...) }}, } { b.Run(arm.name+"/plain", func(b *testing.B) { diff --git a/sandboxd/egress/intercept_test.go b/sandboxd/egress/intercept_test.go index 15ed6b8e..a4417a28 100644 --- a/sandboxd/egress/intercept_test.go +++ b/sandboxd/egress/intercept_test.go @@ -356,10 +356,12 @@ func TestTunnelDecisionOnAnInjectRule(t *testing.T) { {"a credential on the proxy door", creds, "graph.test", true, true}, {"a credential on the SOCKS door", creds, "graph.test", false, false}, } { - p := New(Policy{Allow: []Rule{{Host: "*", Intercept: InterceptInject}}}, tt.secrets, ca, fixedDial(""), nil, nil) - if d, intercept := p.tunnelDecision(tt.host, 443, tt.injects); d != DecisionAllow || intercept != tt.intercept { - t.Errorf("%s: decision %v intercept %t, want an allow with intercept %t", tt.name, d, intercept, tt.intercept) - } + t.Run(tt.name, func(t *testing.T) { + p := New(Policy{Allow: []Rule{{Host: "*", Intercept: InterceptInject}}}, tt.secrets, ca, fixedDial(""), nil, nil) + if d, intercept := p.tunnelDecision(tt.host, 443, tt.injects); d != DecisionAllow || intercept != tt.intercept { + t.Errorf("decision %v intercept %t, want an allow with intercept %t", d, intercept, tt.intercept) + } + }) } } @@ -440,14 +442,16 @@ func TestInterceptSubstitutesABodyCredentialInASmallFormOrJSONBody(t *testing.T) {"other type", "text/plain", "access_token=%40FB%40", "access_token=%40FB%40"}, {"oversized", "application/x-www-form-urlencoded", big, big}, } { - tc := connectTLS(t, front.Listener.Addr().String(), "example.com:443", "example.com", guestRoots) - req := newRequest(t, http.MethodPost, "/echo", tt.body) - req.Header.Set("Content-Type", tt.ctype) - seen, _ := send(t, tc, req).seen(t) - _ = tc.Close() - if seen.Body != tt.want || seen.Length != int64(len(tt.want)) { - t.Errorf("%s: upstream saw body %.80q length %d, want %.80q length %d", tt.name, seen.Body, seen.Length, tt.want, len(tt.want)) - } + t.Run(tt.name, func(t *testing.T) { + tc := connectTLS(t, front.Listener.Addr().String(), "example.com:443", "example.com", guestRoots) + req := newRequest(t, http.MethodPost, "/echo", tt.body) + req.Header.Set("Content-Type", tt.ctype) + seen, _ := send(t, tc, req).seen(t) + _ = tc.Close() + if seen.Body != tt.want || seen.Length != int64(len(tt.want)) { + t.Errorf("upstream saw body %.80q length %d, want %.80q length %d", seen.Body, seen.Length, tt.want, len(tt.want)) + } + }) } } @@ -486,11 +490,7 @@ func TestAContainedValueEndingAReadWaitsForTheLongerOne(t *testing.T) { {Name: "FB_TOKEN", Query: "access_token", Value: "1234567890|SECRET", Placeholder: "@TOK@"}, }) for _, chunks := range [][]string{{"x 1234567890", "|SECRET y"}, {"x 1234567890", " y"}, {"x 1234567890"}} { - readers := make([]io.Reader, len(chunks)) - for i, c := range chunks { - readers[i] = strings.NewReader(c) - } - out, _ := io.ReadAll(&scrubReader{src: io.MultiReader(readers...), sc: sc}) + out, _ := io.ReadAll(&scrubReader{src: chunked(chunks...), sc: sc}) if strings.Contains(string(out), "SECRET") || strings.Contains(string(out), "1234567890") { t.Errorf("chunks %q scrubbed to %q, want every value replaced", chunks, out) } @@ -549,20 +549,18 @@ func TestScrubReaderReplacesAcrossChunksAndHoldsOnlyAPossibleMatch(t *testing.T) {"event tail", []string{"data: x\n\n", "data: SECRET\n\n"}, "data: x\n\n", "data: x\n\ndata: @S@\n\n"}, {"prefix at the end", []string{"x SECR"}, "x ", "x SECR"}, } { - readers := make([]io.Reader, len(tt.chunks)) - for i, c := range tt.chunks { - readers[i] = strings.NewReader(c) - } - s := &scrubReader{src: io.MultiReader(readers...), sc: sc} - buf := make([]byte, 64) - n, _ := s.Read(buf) - if got := string(buf[:n]); got != tt.first { - t.Errorf("%s: first read %q, want %q", tt.name, got, tt.first) - } - rest, _ := io.ReadAll(s) - if got := string(buf[:n]) + string(rest); got != tt.all { - t.Errorf("%s: scrubbed %q, want %q", tt.name, got, tt.all) - } + t.Run(tt.name, func(t *testing.T) { + s := &scrubReader{src: chunked(tt.chunks...), sc: sc} + buf := make([]byte, 64) + n, _ := s.Read(buf) + if got := string(buf[:n]); got != tt.first { + t.Errorf("first read %q, want %q", got, tt.first) + } + rest, _ := io.ReadAll(s) + if got := string(buf[:n]) + string(rest); got != tt.all { + t.Errorf("scrubbed %q, want %q", got, tt.all) + } + }) } } @@ -773,6 +771,14 @@ func send(t *testing.T, tc *tls.Conn, req *http.Request) reply { return reply{status: resp.StatusCode, header: resp.Header, body: string(body)} } +func chunked(chunks ...string) io.Reader { + readers := make([]io.Reader, len(chunks)) + for i, c := range chunks { + readers[i] = strings.NewReader(c) + } + return io.MultiReader(readers...) +} + func readPreamble(tb testing.TB, conn net.Conn) string { tb.Helper() var sb strings.Builder diff --git a/sandboxd/egress/policy.go b/sandboxd/egress/policy.go index a3bd2427..32cc1716 100644 --- a/sandboxd/egress/policy.go +++ b/sandboxd/egress/policy.go @@ -40,17 +40,13 @@ func (m *InterceptMode) UnmarshalJSONFrom(dec *jsontext.Decoder) error { if err != nil { return err } - switch tok.Kind() { - case 'f': + switch k := tok.Kind(); { + case k == 'f': *m = InterceptOff - case 't': + case k == 't': *m = InterceptAlways - case '"': - if tok.String() == "inject" { - *m = InterceptInject - return nil - } - fallthrough + case k == '"' && tok.String() == "inject": + *m = InterceptInject default: return fmt.Errorf("intercept must be true, false or \"inject\", got %s", tok) } diff --git a/sandboxd/egress/policy_test.go b/sandboxd/egress/policy_test.go index 02c55720..7ff18b55 100644 --- a/sandboxd/egress/policy_test.go +++ b/sandboxd/egress/policy_test.go @@ -39,7 +39,7 @@ func TestPolicyEval(t *testing.T) { } } -func TestEvalSkipsInterceptRules(t *testing.T) { +func TestEvalSkipsOnlyAlwaysInterceptRules(t *testing.T) { p := Policy{Allow: []Rule{ {Host: "api.github.com", Secret: "gh", Intercept: InterceptAlways}, {Host: "plain.github.com"}, diff --git a/sandboxd/egress/proxy.go b/sandboxd/egress/proxy.go index 419b5771..df9735cb 100644 --- a/sandboxd/egress/proxy.go +++ b/sandboxd/egress/proxy.go @@ -42,7 +42,7 @@ var ( "Proxy-Connection", "TE", "Trailer", "Transfer-Encoding", "Upgrade", } - copyBufs = sync.Pool{New: func() any { b := make([]byte, 32<<10); return &b }} + copyBufs = sync.Pool{New: func() any { b := make([]byte, 2*scrubChunk); return &b }} ) // TransferFunc receives the payload bytes an allowed tunnel or request moved, once it ends. @@ -283,9 +283,8 @@ func (p *Proxy) serveConnect(w http.ResponseWriter, r *http.Request) { func (p *Proxy) tunnelDecision(host string, port uint16, injects bool) (decision Decision, intercept bool) { // host-gate interception: the tunnel's CONNECT verb is not the request method. if p.ca != nil { - switch rule, d := p.policy.EvalHost(host, port); { - case d != DecisionAllow: - case rule.Intercept == InterceptAlways, rule.Intercept == InterceptInject && injects && p.holdsCredential(host): + if rule, d := p.policy.EvalHost(host, port); d == DecisionAllow && + (rule.Intercept == InterceptAlways || rule.Intercept == InterceptInject && injects && p.holdsCredential(host)) { return DecisionAllow, true } } @@ -479,9 +478,7 @@ func (sc *scrubber) header(h http.Header) { for _, vs := range h { for i, v := range vs { for _, e := range sc.echoes { - if strings.Contains(v, e.old) { - v = strings.ReplaceAll(v, e.old, e.placeholder) - } + v = strings.ReplaceAll(v, e.old, e.placeholder) } vs[i] = v } diff --git a/sandboxd/pool/env_test.go b/sandboxd/pool/env_test.go index 9c9edf6d..9965ffaf 100644 --- a/sandboxd/pool/env_test.go +++ b/sandboxd/pool/env_test.go @@ -585,19 +585,21 @@ func TestAnInjectShapeRidesTheEnvWriteIntoTheCredential(t *testing.T) { {"header with a placeholder", interceptPolicy(), "api.github.com", types.EnvInject{Header: "X-Api-Key", Placeholder: "@KEY@"}}, {"query with body on an inject rule", &egress.Policy{Allow: []egress.Rule{{Host: "*", Intercept: egress.InterceptInject}}}, "graph.example.com", types.EnvInject{Query: "access_token", Body: true, Placeholder: "@FB@"}}, } { - eng := newFakeEngine() - eng.sockRoot = sockRoot(t) - m := egressManager(t, eng, config.PoolSpec{PoolKey: testKey, Warm: 1, Egress: tt.policy}) - sb := mustClaim(t, m, testKey) - inject := tt.inject - inject.Hosts = []string{tt.host} - if err := m.PatchEnv(t.Context(), sb.ID, types.EnvPatch{"KEY": {Value: "k", Guest: hostOnly, Inject: &inject}}, ""); err != nil { - t.Fatalf("%s: %v", tt.name, err) - } - creds := (claimSecrets{m: m, sb: sb}).Credentials(tt.host) - if len(creds) != 1 || creds[0].Header != inject.Header || creds[0].Query != inject.Query || creds[0].Body != inject.Body || creds[0].Placeholder != inject.Placeholder { - t.Errorf("%s: credentials %+v, want one carrying %+v", tt.name, creds, inject) - } + t.Run(tt.name, func(t *testing.T) { + eng := newFakeEngine() + eng.sockRoot = sockRoot(t) + m := egressManager(t, eng, config.PoolSpec{PoolKey: testKey, Warm: 1, Egress: tt.policy}) + sb := mustClaim(t, m, testKey) + inject := tt.inject + inject.Hosts = []string{tt.host} + if err := m.PatchEnv(t.Context(), sb.ID, types.EnvPatch{"KEY": {Value: "k", Guest: hostOnly, Inject: &inject}}, ""); err != nil { + t.Fatalf("patch: %v", err) + } + want := egress.Credential{Name: "KEY", Header: inject.Header, Query: inject.Query, Body: inject.Body, Value: "k", Placeholder: inject.Placeholder} + if creds := (claimSecrets{m: m, sb: sb}).Credentials(tt.host); len(creds) != 1 || creds[0] != want { + t.Errorf("credentials %+v, want [%+v]", creds, want) + } + }) } } From 7faf41da60e04fcffe417462db7a18d99c258dc9 Mon Sep 17 00:00:00 2001 From: CMGS Date: Sun, 4 Oct 2026 12:50:49 +0800 Subject: [PATCH 11/11] egress: a filled value inside another, split by a read, waits for the whole one The hold only covered a longer echo starting where the shorter match did, so with B = X+A+Y split right after A the scan wrote X raw, replaced A and let Y through. A match within the last echo length of a read now waits whenever an unfinished echo starts at or before it, which is what partial already finds; the same rule covers the prefix case and drops the held flag. The egress docs name the JSON escapes of a quote, backslash or tab as outside the scrub. A/B against 940889d, six interleaved runs: -0.1%. --- docs/egress.md | 3 ++- sandboxd/egress/intercept_test.go | 31 +++++++++++++++++++++---------- sandboxd/egress/proxy.go | 22 ++++++---------------- 3 files changed, 29 insertions(+), 27 deletions(-) diff --git a/docs/egress.md b/docs/egress.md index c19ef3fc..da6b1acc 100644 --- a/docs/egress.md +++ b/docs/egress.md @@ -375,7 +375,8 @@ same `PATCH`, accepted on a hibernated or archived claim without waking it. `Accept-Encoding`, `Range` and `If-Range` on such a request, so it reads the whole body uncompressed, and streams it holding back only a tail that may begin a match, so event streams stay live. A value the origin transforms - otherwise is beyond the scrub, and so is a value the origin stored earlier + otherwise is beyond the scrub, such as the JSON escape of a quote, backslash + or tab inside a value, and so is a value the origin stored earlier and returns on a later request that carries no credential: the scrub covers only the response to the request that sent the value. `header` credentials, which origins do not reflect, are not scrubbed and add no work to the response. diff --git a/sandboxd/egress/intercept_test.go b/sandboxd/egress/intercept_test.go index a4417a28..24827d0d 100644 --- a/sandboxd/egress/intercept_test.go +++ b/sandboxd/egress/intercept_test.go @@ -484,16 +484,27 @@ func TestAValueThatContainsAnotherIsScrubbedWhole(t *testing.T) { } } -func TestAContainedValueEndingAReadWaitsForTheLongerOne(t *testing.T) { - sc := newScrubber([]Credential{ - {Name: "FB_APP_ID", Query: "app_id", Value: "1234567890", Placeholder: "@ID@"}, - {Name: "FB_TOKEN", Query: "access_token", Value: "1234567890|SECRET", Placeholder: "@TOK@"}, - }) - for _, chunks := range [][]string{{"x 1234567890", "|SECRET y"}, {"x 1234567890", " y"}, {"x 1234567890"}} { - out, _ := io.ReadAll(&scrubReader{src: chunked(chunks...), sc: sc}) - if strings.Contains(string(out), "SECRET") || strings.Contains(string(out), "1234567890") { - t.Errorf("chunks %q scrubbed to %q, want every value replaced", chunks, out) - } +func TestAContainedValueSplitByAReadIsScrubbedWhole(t *testing.T) { + for _, tt := range []struct { + name, short, long string + }{ + {"prefix", "1234567890", "1234567890|s3cr3tvalue"}, + {"middle", "id", "s3cr3t|id|t41l"}, + {"suffix", "s3cr3tvalue", "1234567890|s3cr3tvalue"}, + } { + t.Run(tt.name, func(t *testing.T) { + sc := newScrubber([]Credential{ + {Name: "SHORT", Query: "a", Value: tt.short, Placeholder: "@A@"}, + {Name: "LONG", Query: "b", Value: tt.long, Placeholder: "@B@"}, + }) + body := "next=" + tt.long + "&x" + for cut := range len(body) + 1 { + out, _ := io.ReadAll(&scrubReader{src: chunked(body[:cut], body[cut:]), sc: sc}) + if string(out) != "next=@B@&x" { + t.Errorf("cut at %d scrubbed to %q, want next=@B@&x", cut, out) + } + } + }) } } diff --git a/sandboxd/egress/proxy.go b/sandboxd/egress/proxy.go index df9735cb..e4813486 100644 --- a/sandboxd/egress/proxy.go +++ b/sandboxd/egress/proxy.go @@ -557,7 +557,7 @@ func (s *scrubReader) Read(b []byte) (int, error) { } func (s *scrubReader) scan() { - buf, i, held := s.pending, 0, false + buf, i := s.pending, 0 for { at, k := -1, 0 for j, e := range s.sc.echoes { @@ -568,32 +568,22 @@ func (s *scrubReader) scan() { if at < 0 { break } - s.out.Write(buf[i : i+at]) - i += at - if s.err == nil && s.longerMayFollow(buf[i:], len(s.sc.echoes[k].raw)) { - held = true + at += i + if s.err == nil && at > len(buf)-s.sc.longest && len(buf)-s.partial(buf[i:]) <= at { break } + s.out.Write(buf[i:at]) s.out.WriteString(s.sc.echoes[k].placeholder) - i += len(s.sc.echoes[k].raw) + i = at + len(s.sc.echoes[k].raw) } hold := 0 - switch { - case held: - hold = len(buf) - i - case s.err == nil: + if s.err == nil { hold = s.partial(buf[i:]) } s.out.Write(buf[i : len(buf)-hold]) s.pending = s.pending[:copy(s.pending, buf[len(buf)-hold:])] } -func (s *scrubReader) longerMayFollow(rest []byte, n int) bool { - return slices.ContainsFunc(s.sc.echoes, func(e echoForm) bool { - return len(e.raw) > n && len(rest) < len(e.raw) && bytes.HasPrefix(e.raw, rest) - }) -} - func (s *scrubReader) partial(tail []byte) int { for k := min(len(tail), s.sc.longest-1); k > 0; k-- { if s.sc.first[tail[len(tail)-k]] && slices.ContainsFunc(s.sc.echoes, func(e echoForm) bool {