amadan
edge: pin VerifyDynamic's slash gate with a test only it can pass
Review catch: the static case used "hq", absent from dynamicAuth, so it
was refused by the map miss whether or not the "/" gate existed — the
deliberate tightening had no test holding it.
Add a hub whose dynamicAuth deliberately holds slash-less keys under
hashes that MATCH the presented tokens, so every other check passes and
only the gate can refuse: "hq" (a static channel name), "/box-a" (empty
fleet segment), "noslash". Verified by mutation — with the gate removed
all three return ok=true ("/box-a" as ok with an EMPTY fleet, the exact
failure the second half prevents) while the original table still passes.
Also document the fleet == "" half, which the comment left unexplained.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2 files changed,
+36
−1
internal/edge/tunnel.go+6 −1internal/edge/tunnel_test.go+30 −0
diff --git a/internal/edge/tunnel.go b/internal/edge/tunnel.go| index 1a1c4ecec..9cf631a03 100644 |
| --- a/internal/edge/tunnel.go |
| +++ b/internal/edge/tunnel.go |
| @@ -254,7 +254,12 @@ func (h *TunnelHub) pingLoop(name string, cc *http2.ClientConn, raw net.Conn, dy |
| // The "/" requirement is a gate, not a parse convenience: static channel names |
| // are ValidTunnelName-checked and never contain "/", so refusing slash-less |
| // names here means a static client's plaintext token can never be admitted by |
| -// this path, whatever h.clients (or a hand-built dynamicAuth) holds. The |
| +// this path, whatever h.clients (or a hand-built dynamicAuth) holds. An empty |
| +// fleet segment ("/box-a") is refused for the same reason: it would otherwise |
| +// authenticate into fleet "", a name no real fleet has, and every ownership |
| +// check downstream compares against the fleet this returns. |
| +// |
| +// The |
| // presented token is never compared in the clear — only its sha256 hex is, |
| // against the stored hash — so a leaked dump of this table yields no usable |
| // bearer token, and the compare is constant-time so a wrong token leaks no |
diff --git a/internal/edge/tunnel_test.go b/internal/edge/tunnel_test.go| index 200a68b5a..4ba3c095a 100644 |
| --- a/internal/edge/tunnel_test.go |
| +++ b/internal/edge/tunnel_test.go |
| @@ -233,6 +233,36 @@ func TestVerifyDynamic(t *testing.T) { |
| } |
| } |
| +// TestVerifyDynamicRefusesSlashlessNameInDynamicAuth is the one case only the |
| +// "/" gate can refuse, and so the only test that actually pins it: the name is |
| +// PRESENT in dynamicAuth under a hash that matches the presented token, so |
| +// every other check in VerifyDynamic passes. Delete the gate and this test |
| +// fails; the table above would not, because "hq" simply misses that map. |
| +// |
| +// It makes the doc comment's claim real: no slash-less name authenticates |
| +// here, whatever dynamicAuth holds. Production never writes such a key |
| +// (FleetChannelAuth emits "<fleet>/<label>" only) — the point is that a future |
| +// writer that did could not silently turn a static channel name, or a name |
| +// with no fleet at all, into a fleet-authenticated one. |
| +func TestVerifyDynamicRefusesSlashlessNameInDynamicAuth(t *testing.T) { |
| + hub := NewTunnelHub("tunnel.test", map[string]string{"hq": "sekrit"}) |
| + hub.SetDynamicAuth(map[string]string{ |
| + "hq": sha256Hex("sekrit"), // a static channel name, hash-correct |
| + "/box-a": sha256Hex("tok"), // empty fleet segment, hash-correct |
| + "noslash": sha256Hex("tok"), |
| + }) |
| + for _, tc := range []struct{ desc, name, token string }{ |
| + {"static name, hash-correct, present in dynamicAuth", "hq", "sekrit"}, |
| + {"empty fleet segment", "/box-a", "tok"}, |
| + {"bare name with no separator", "noslash", "tok"}, |
| + } { |
| + if fleet, ok := hub.VerifyDynamic(tc.name, tc.token); ok || fleet != "" { |
| + t.Errorf("%s: VerifyDynamic(%q, %q) = (%q, %v), want (\"\", false)", |
| + tc.desc, tc.name, tc.token, fleet, ok) |
| + } |
| + } |
| +} |
| + |
| // TestCloseChannelClosesAndRemovesLiveConn: CloseChannel is the detach/rotate |
| // primitive — force-close a live conn and drop it from the map. No real h2 |
| // handshake needed: CloseChannel only ever touches .raw. |