amadan
edge: VerifyDynamic — the dynamic-auth check as a reusable gate
Extract ServeHTTP's dynamic-auth branch into a named method. Fleet-API surfaces need the same answer — "does (name, token) authenticate as a fleet channel, and whose?" — with no ResponseWriter to 401 with. The "/" requirement is now explicit rather than an implicit reliance on ValidTunnelName never admitting a slash: a static client's plaintext token can no longer be admitted by this path whatever h.clients holds. Constant-time compare, hash-never-plaintext, and the single-401 ordering in ServeHTTP are all preserved exactly.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2 files changed,
+70
−9
internal/edge/tunnel.go+34 −9internal/edge/tunnel_test.go+36 −0
diff --git a/internal/edge/tunnel.go b/internal/edge/tunnel.go| index d40999a60..1a1c4ecec 100644 |
| --- a/internal/edge/tunnel.go |
| +++ b/internal/edge/tunnel.go |
| @@ -154,15 +154,8 @@ func (h *TunnelHub) ServeHTTP(w http.ResponseWriter, r *http.Request) { |
| // that fact is known. |
| viaDynamic := false |
| if !authOK && name != "" && hasBearer { |
| - h.mu.Lock() |
| - wantHash, dynOK := h.dynamicAuth[name] |
| - h.mu.Unlock() |
| - if dynOK && wantHash != "" { |
| - sum := sha256.Sum256([]byte(token)) |
| - gotHash := hex.EncodeToString(sum[:]) |
| - authOK = subtle.ConstantTimeCompare([]byte(gotHash), []byte(wantHash)) == 1 |
| - viaDynamic = authOK |
| - } |
| + _, viaDynamic = h.VerifyDynamic(name, token) |
| + authOK = viaDynamic |
| } |
| if !authOK { |
| http.Error(w, "unauthorized", http.StatusUnauthorized) |
| @@ -253,6 +246,38 @@ func (h *TunnelHub) pingLoop(name string, cc *http2.ClientConn, raw net.Conn, dy |
| } |
| } |
| +// VerifyDynamic reports whether (name, token) authenticates as a dynamic |
| +// (registry-backed fleet) channel, and which fleet owns it. It is the single |
| +// answer to that question: ServeHTTP's dynamic branch is one caller, and |
| +// surfaces with no ResponseWriter to 401 with are the others. |
| +// |
| +// 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 |
| +// 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 |
| +// prefix information by timing. |
| +func (h *TunnelHub) VerifyDynamic(name, token string) (fleet string, ok bool) { |
| + fleet, _, hasSlash := strings.Cut(name, "/") |
| + if !hasSlash || fleet == "" { |
| + return "", false |
| + } |
| + h.mu.Lock() |
| + wantHash, known := h.dynamicAuth[name] |
| + h.mu.Unlock() |
| + if !known || wantHash == "" { |
| + return "", false |
| + } |
| + sum := sha256.Sum256([]byte(token)) |
| + gotHash := hex.EncodeToString(sum[:]) |
| + if subtle.ConstantTimeCompare([]byte(gotHash), []byte(wantHash)) != 1 { |
| + return "", false |
| + } |
| + return fleet, true |
| +} |
| + |
| // SetDynamicAuth replaces the entire registry-backed credential set — |
| // channel "<fleet>/<label>" -> sha256 hex of its bearer token — with auth. |
| // The whole-map REPLACE (not an incremental merge) is what makes a NEW dial |
diff --git a/internal/edge/tunnel_test.go b/internal/edge/tunnel_test.go| index 3f7b278c7..200a68b5a 100644 |
| --- a/internal/edge/tunnel_test.go |
| +++ b/internal/edge/tunnel_test.go |
| @@ -197,6 +197,42 @@ func TestStaticClientStillAuthenticatesAlongsideDynamicSet(t *testing.T) { |
| } |
| } |
| +// TestVerifyDynamic covers the extracted gate directly, without a handshake: |
| +// VerifyDynamic is the one place that answers "does (name, token) authenticate |
| +// as a fleet channel, and whose?" — ServeHTTP's dynamic branch is now just a |
| +// caller, and later fleet-API surfaces (which have no ResponseWriter to 401 |
| +// with) need the same answer. |
| +func TestVerifyDynamic(t *testing.T) { |
| + // The static client is configured on the SAME hub on purpose: a static |
| + // name must never authenticate through this gate, even presenting the |
| + // correct plaintext token h.clients holds for it. ServeHTTP's static |
| + // compare is the only path that may admit it. |
| + hub := NewTunnelHub("tunnel.test", map[string]string{"hq": "sekrit"}) |
| + hub.SetDynamicAuth(map[string]string{"pi/box-a": sha256Hex("tok")}) |
| + for _, tc := range []struct { |
| + desc, name, token string |
| + wantFleet string |
| + wantOK bool |
| + }{ |
| + {desc: "static name with its own correct plaintext token", name: "hq", token: "sekrit"}, |
| + {desc: "unknown dynamic name", name: "pi/box-z", token: "tok"}, |
| + {desc: "right name, wrong token", name: "pi/box-a", token: "wrong"}, |
| + {desc: "right name and token", name: "pi/box-a", token: "tok", wantFleet: "pi", wantOK: true}, |
| + } { |
| + fleet, ok := hub.VerifyDynamic(tc.name, tc.token) |
| + if ok != tc.wantOK || fleet != tc.wantFleet { |
| + t.Errorf("%s: VerifyDynamic(%q, %q) = (%q, %v), want (%q, %v)", |
| + tc.desc, tc.name, tc.token, fleet, ok, tc.wantFleet, tc.wantOK) |
| + } |
| + } |
| + // A SetDynamicAuth swap that drops the name must refuse it the instant it |
| + // returns — the same whole-map-replace property detach relies on. |
| + hub.SetDynamicAuth(map[string]string{"pi/box-b": sha256Hex("tok")}) |
| + if fleet, ok := hub.VerifyDynamic("pi/box-a", "tok"); ok || fleet != "" { |
| + t.Errorf("after a SetDynamicAuth swap dropping the name: got (%q, %v), want (\"\", false)", 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. |