From e6184353dbc6ef65f7b20255e3965ed85857257f Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Thu, 8 Oct 2026 14:35:50 +0200 Subject: [PATCH] hub R-138: the token's one zone must BE the customer's domain (order-independent; security review) Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_0159rPz1ZhFKsS53msqPYxtS --- hub/internal/cloudflare/reach.go | 11 ++--- hub/internal/cloudflare/reach_test.go | 11 +++-- hub/internal/web/configs.go | 25 +++------- hub/internal/web/r138_cf_token_reach_test.go | 49 ++++++++++---------- 4 files changed, 42 insertions(+), 54 deletions(-) diff --git a/hub/internal/cloudflare/reach.go b/hub/internal/cloudflare/reach.go index 45865f13..ecf7d964 100644 --- a/hub/internal/cloudflare/reach.go +++ b/hub/internal/cloudflare/reach.go @@ -67,15 +67,12 @@ func TokenZones(ctx context.Context, base, token string) (names []string, total return names, total, nil } -// ZoneCovers reports whether a customer domain is the zone itself or a name under it, on a label boundary -// („notexample.hu" is not under „example.hu"). Case and a trailing dot are ignored. -func ZoneCovers(zoneName, domain string) bool { +// ZoneEquals reports whether a zone name IS the customer's domain. Case and a trailing dot are ignored. A zone above +// the domain does not count: it may hold another customer's sibling domain, now or later (R-138, decision 190). +func ZoneEquals(zoneName, domain string) bool { z := strings.TrimSuffix(strings.ToLower(strings.TrimSpace(zoneName)), ".") d := strings.TrimSuffix(strings.ToLower(strings.TrimSpace(domain)), ".") - if z == "" || d == "" { - return false - } - return d == z || strings.HasSuffix(d, "."+z) + return z != "" && z == d } func redact(s, token string) string { diff --git a/hub/internal/cloudflare/reach_test.go b/hub/internal/cloudflare/reach_test.go index 821b1fba..fe982030 100644 --- a/hub/internal/cloudflare/reach_test.go +++ b/hub/internal/cloudflare/reach_test.go @@ -9,22 +9,23 @@ import ( "testing" ) -func TestZoneCovers(t *testing.T) { +func TestZoneEquals(t *testing.T) { for _, c := range []struct { zone, domain string want bool }{ {"example.hu", "example.hu", true}, - {"example.hu", "felhom.example.hu", true}, - {"Example.HU.", "home.example.hu", true}, + {"example.hu", "felhom.example.hu", false}, + {"Example.HU.", "example.hu", true}, + {"example.hu", "home.example.hu", false}, {"example.hu", "notexample.hu", false}, {"example.hu", "example.hu.evil.hu", false}, {"home.example.hu", "example.hu", false}, {"", "example.hu", false}, {"example.hu", "", false}, } { - if got := ZoneCovers(c.zone, c.domain); got != c.want { - t.Errorf("ZoneCovers(%q,%q)=%v want %v", c.zone, c.domain, got, c.want) + if got := ZoneEquals(c.zone, c.domain); got != c.want { + t.Errorf("ZoneEquals(%q,%q)=%v want %v", c.zone, c.domain, got, c.want) } } } diff --git a/hub/internal/web/configs.go b/hub/internal/web/configs.go index f3b696b0..c86d7b54 100644 --- a/hub/internal/web/configs.go +++ b/hub/internal/web/configs.go @@ -1850,9 +1850,10 @@ func normDomainForm(d string) string { return strings.TrimSuffix(strings.TrimSpa // cfTokenReachMessage is the operator's sentence for a refused Cloudflare API token ("" = allowed). R-138 option C // (operator ruling 2026-10-08, `09` §3 decision 190): the hub asks Cloudflare which zones the token can see and allows -// it only when it sees EXACTLY ONE zone and the customer's domain is that zone or a name under it (`01` §7: every -// customer has their own domain — the zone is the customer's, so a dashboard name like felhom. under it is -// fine; a second zone is someone else's). An empty token (HTTP-01) is never checked. Fail closed: when Cloudflare +// it only when it sees EXACTLY ONE zone and that zone IS the customer's domain (`01` §7: every customer has their own +// domain). A zone ABOVE the domain is refused even when no other customer sits under it today: a sibling customer +// added later (a.parent.hu, then b.parent.hu — R-415 compares the two domains only) would be reachable by the key +// already stored, so the check could not hold over time (security review 2026-10-08). An empty token (HTTP-01) is never checked. Fail closed: when Cloudflare // cannot be asked, the save is refused and nothing changes. The token is never logged and never in a sentence. // Pinned by TestR138_* (r138_cf_token_reach_test.go). func (s *Server) cfTokenReachMessage(ctx context.Context, customerID, domain, token string) string { @@ -1876,22 +1877,10 @@ func (s *Server) cfTokenReachMessage(ctx context.Context, customerID, domain, to case len(names) != 1: s.logger.Printf("[WARN] cloudflare token check for %s: Cloudflare counted 1 zone but listed %d — save refused", customerID, len(names)) return "Cloudflare's answer about this API token was incomplete, so it was not checked — nothing was saved and the previous token stays. Try again in a few minutes." - case !cfClient.ZoneCovers(names[0], domain): - s.logger.Printf("[WARN] cloudflare token check for %s: the token's one zone %q does not cover domain %q — save refused", customerID, names[0], domain) + case !cfClient.ZoneEquals(names[0], domain): + s.logger.Printf("[WARN] cloudflare token check for %s: the token's one zone %q is not domain %q — save refused", customerID, names[0], domain) return fmt.Sprintf("The Cloudflare API token reaches zone %q, which is not this customer's domain %q. Nothing was saved.", names[0], domain) } - // A zone ABOVE the customer's domain may also hold another customer's sibling domain (a.parent.hu and - // b.parent.hu under zone parent.hu pass the R-415 guard, which compares the two domains only). So the zone - // itself must not equal, contain or lie under any other customer's domain, nor under felhom.eu. Security - // review 2026-10-08; pinned by TestR138_ParentZoneHoldingAnotherCustomerRefused. - if other, err := s.store.DomainConflict(customerID, names[0]); err != nil || other != "" { - if err != nil { - s.logger.Printf("[ERROR] cloudflare token check for %s: the zone could not be checked against the other customers: %v — save refused", customerID, err) - return "The Cloudflare API token's zone could not be checked against the other customers — nothing was saved. Try again." - } - s.logger.Printf("[WARN] cloudflare token check for %s: the token's zone %q also covers customer %s's domain — save refused (R-138)", customerID, names[0], other) - return fmt.Sprintf("The Cloudflare API token reaches zone %q, which also holds the domain of customer %q — make the customer's own domain a zone of its own. Nothing was saved.", names[0], other) - } - s.logger.Printf("[INFO] cloudflare token check for %s: the token sees 1 zone (%s), which covers the customer's domain — allowed", customerID, names[0]) + s.logger.Printf("[INFO] cloudflare token check for %s: the token sees 1 zone (%s), which is the customer's domain — allowed", customerID, names[0]) return "" } diff --git a/hub/internal/web/r138_cf_token_reach_test.go b/hub/internal/web/r138_cf_token_reach_test.go index 1ccc47e4..16e5bd5b 100644 --- a/hub/internal/web/r138_cf_token_reach_test.go +++ b/hub/internal/web/r138_cf_token_reach_test.go @@ -121,10 +121,11 @@ func TestR138_CreateAcceptsOwnZoneToken(t *testing.T) { if f.calls.Load() != 1 { t.Errorf("expected exactly 1 Cloudflare call, got %d", f.calls.Load()) } - // A domain UNDER the token's one zone is the customer's own too (the zone is the customer's, `01` §7). + // A domain UNDER the token's one zone is refused: the zone must BE the customer's domain (security review + // 2026-10-08 — a zone above the domain could hold a sibling customer added later). s2, _, _ := r138Server(t) - if rr := r138Create(s2, "sub", "home.example.hu", tokOwn); rr.Code != http.StatusSeeOther { - t.Errorf("a domain under the token's one zone must be accepted; got %d %s", rr.Code, rr.Body.String()) + if rr := r138Create(s2, "sub", "home.example.hu", tokOwn); rr.Code == http.StatusSeeOther { + t.Errorf("a domain under the token's one zone must be refused") } } @@ -237,27 +238,27 @@ func TestR138_TokenNeverInLogsOrPage(t *testing.T) { } } -// A token whose one zone sits ABOVE the customer's domain and also holds another customer's sibling domain is -// refused (security review 2026-10-08: the R-415 guard compares the two domains only, and a.parent.hu and -// b.parent.hu do not overlap). Control: the same parent-zone token is accepted when no other customer lives under it. -// RED-PROOF: remove the DomainConflict(zone) block in cfTokenReachMessage → customer „a" is stored → FAILS. +// A token whose one zone sits ABOVE the customer's domain is refused — in EITHER order of creation. Security review +// 2026-10-08: checking the zone against today's customers only was order-dependent (a sibling b.parent.hu created +// AFTER a.parent.hu's parent-zone key was stored passes R-415, which compares the two domains only). The zone must +// equal the domain. RED-PROOF: ZoneEquals → suffix match (the old ZoneCovers) → „a" is stored → FAILS. func TestR138_ParentZoneHoldingAnotherCustomerRefused(t *testing.T) { - s, _, _ := r138Server(t) - if rr := r138Create(s, "b", "b.parent.hu", ""); rr.Code != http.StatusSeeOther { - t.Fatalf("setup: customer b without a token must be created; got %d %s", rr.Code, rr.Body.String()) - } - rr := r138Create(s, "a", "a.parent.hu", tokParent) - if rr.Code == http.StatusSeeOther { - t.Fatalf("a token for zone parent.hu, which also holds customer b, was accepted") - } - if !strings.Contains(rr.Body.String(), "also holds the domain of customer") { - t.Errorf("the refusal must name the shared zone; got %d", rr.Code) - } - if _, ok := r138StoredToken(t, s, "a"); ok { - t.Errorf("a config was stored for a refused parent-zone token") - } - s2, _, _ := r138Server(t) - if rr := r138Create(s2, "a", "a.parent.hu", tokParent); rr.Code != http.StatusSeeOther { - t.Errorf("control: the parent-zone token with no other customer under it must be accepted; got %d %s", rr.Code, rr.Body.String()) + for _, order := range []string{"sibling first", "sibling after"} { + s, _, _ := r138Server(t) + if order == "sibling first" { + if rr := r138Create(s, "b", "b.parent.hu", ""); rr.Code != http.StatusSeeOther { + t.Fatalf("setup: customer b without a token must be created; got %d %s", rr.Code, rr.Body.String()) + } + } + rr := r138Create(s, "a", "a.parent.hu", tokParent) + if rr.Code == http.StatusSeeOther { + t.Errorf("%s: a token for zone parent.hu was accepted for a.parent.hu", order) + } + if !strings.Contains(rr.Body.String(), "which is not this customer") { + t.Errorf("%s: the refusal must say the zone is not the domain; got %d", order, rr.Code) + } + if _, ok := r138StoredToken(t, s, "a"); ok { + t.Errorf("%s: a config was stored for a refused parent-zone token", order) + } } }