amadan

carlos / platform Public

edge: backhaul review fixes — per-host idle pool, forged-hop pin

Review round 1 on Task 4.

MaxIdleConnsPerHost was left at the stdlib default of 2. The pool key here is
the HOSTNAME, not one constant localhost authority the way unixProxy/tcpProxy's
is, so MaxIdleConns:32 capped the box while each busy backhauled host kept two
idle connections — and the third concurrent request to a hot host paid the
cross-region TCP+TLS handshake the transport cache exists to avoid. Raised to
32, equal to the total, with the divergence from the house style spelled out.

Pin the forged hop for a REMOTE-homed host: a stranger sending X-Carlos-Hop
with a garbage signature and a chosen X-Forwarded-For is backhauled as the
ordinary client they are — 200 through the hop with no Retry-After (they learn
nothing about where the host lives), a FRESH signature this edge minted rather
than their garbage carried onward under its name, and a single-element chain
naming their own address instead of the one they claimed.
Paul Campbell pushed by paul@keymail.dev 01939120a91de1da6d9b69b22deb98d032ba8d04 parent bdcf6f6
2 files changed, +65 −2
  • internal/edge/edge.go +10 −2
  • internal/edge/homemap_test.go +55 −0
diff --git a/internal/edge/edge.go b/internal/edge/edge.go
index 9879ded98..96ea213d1 100644
--- a/internal/edge/edge.go
+++ b/internal/edge/edge.go
@@ -1875,8 +1875,16 @@ func (e *Edge) backhaulTransport(eip string) *http.Transport {
},
TLSHandshakeTimeout: backhaulDialTimeout,
ResponseHeaderTimeout: backhaulHeaderTimeout,
- MaxIdleConns: 32,
- IdleConnTimeout: 90 * time.Second,
+ // PER-HOST as well as in total, which is where this diverges from
+ // unixProxy/tcpProxy's bare MaxIdleConns: their pool key is one constant
+ // localhost authority, so the stdlib default of 2 idle conns per host is
+ // really 2 per BACKEND. Here the key is the hostname, so the default
+ // would cap each backhauled host at 2 — and the third concurrent request
+ // to a busy host would pay the cross-region TCP+TLS handshake this cache
+ // exists to avoid. Equal to MaxIdleConns so the total is the only bound.
+ MaxIdleConns: 32,
+ MaxIdleConnsPerHost: 32,
+ IdleConnTimeout: 90 * time.Second,
}
// Two requests for a box nobody has dialled yet can both build one; the
// loser's transport is simply dropped, having opened nothing and started no
diff --git a/internal/edge/homemap_test.go b/internal/edge/homemap_test.go
index 2bfd2edb1..dcbf19128 100644
--- a/internal/edge/homemap_test.go
+++ b/internal/edge/homemap_test.go
@@ -749,6 +749,61 @@ func TestBackhaulRefusesToDialWithoutAMeshKey(t *testing.T) {
}
}
+// A stranger forging the hop vocabulary for a remote-homed host is backhauled
+// like any other client, and gains exactly nothing by having tried.
+//
+// Two halves have to hold at once and they meet only here. acceptHop deletes
+// both hop headers and — verification having failed — strips every forwarding
+// claim the forger made, so the request reaching this point is an ordinary
+// one; backhaul then mints a FRESH signature under the deployment's own key,
+// so the home edge sees a hop it can verify and an X-Forwarded-For naming the
+// forger. What the forger must NOT get is the stale-map 503 (which would tell
+// them the host lives elsewhere), a chain of their own choosing, or their
+// garbage signature carried onward under this edge's name.
+func TestForgedHopForARemoteHostIsBackhauledAsAnOrdinaryClient(t *testing.T) {
+ e, m, key := hopEdge(t, "annie", "local.test")
+ rec, eip := fakeHomeEdge(t, nil)
+ homedOn(t, m, "betty", eip, "example.com")
+
+ const garbage = "0:deadbeef"
+ resp := hopRequest(t, e, "example.com", map[string]string{
+ hopHeader: "1",
+ hopAuthHeader: garbage,
+ "X-Forwarded-For": "203.0.113.9",
+ })
+ if resp.StatusCode != http.StatusOK {
+ t.Fatalf("status = %d, want 200 — a forged hop is an ordinary client, not a refusal", resp.StatusCode)
+ }
+ if got := resp.Header.Get("Retry-After"); got != "" {
+ t.Fatalf("Retry-After = %q — the stale-map answer must stay unreachable without the mesh key", got)
+ }
+ if body := readBody(t, resp); body != homeEdgeBody {
+ t.Fatalf("body = %q, want the home edge's answer %q", body, homeEdgeBody)
+ }
+ seen := rec.get()
+ if seen.n != 1 {
+ t.Fatalf("home edge saw %d request(s), want exactly one", seen.n)
+ }
+ if seen.auth == garbage {
+ t.Fatalf("%s = %q — the forger's signature must never be forwarded under this edge's name", hopAuthHeader, seen.auth)
+ }
+ if seen.hop != "1" || !verifyHopEitherKey(key, nil, "example.com", seen.auth, time.Now()) {
+ t.Fatalf("hop=%q auth=%q — the hop must carry a FRESH signature this edge minted", seen.hop, seen.auth)
+ }
+ // The forger's claimed chain is gone and the one that arrives names the
+ // forger's own address, once. Anything else would let a stranger choose
+ // what the app (and the console's rate limiter) believes about who called.
+ if len(seen.xff) != 1 || strings.Contains(seen.xff[0], ",") {
+ t.Fatalf("X-Forwarded-For = %q, want a single-element chain", seen.xff)
+ }
+ if strings.Contains(seen.xff[0], "203.0.113.9") {
+ t.Fatalf("X-Forwarded-For = %q — a forged chain must be discarded, not carried", seen.xff)
+ }
+ if ip := net.ParseIP(seen.xff[0]); ip == nil || !ip.IsLoopback() {
+ t.Fatalf("X-Forwarded-For = %q, want the forger's own (loopback) address", seen.xff)
+ }
+}
+
// A catch-all row in another box's edges file resolves through the same
// one-label wildcard rule lookupUpstream uses — but the signature must be over
// the CONCRETE request host, because that is what the home edge verifies