diff --git a/cmd/ingestor/main.go b/cmd/ingestor/main.go index ec4ef406..d3ed0dda 100644 --- a/cmd/ingestor/main.go +++ b/cmd/ingestor/main.go @@ -1635,12 +1635,25 @@ func loadRegionKeys(cfg *Config) map[string][]byte { return keys } -// matchScope performs one HMAC-SHA256 per configured region. Expected -// len(regionKeys) ≤ 50; beyond that, consider a pre-indexed lookup table. -func matchScope(regionKeys map[string][]byte, payloadType byte, payloadRaw []byte, code1 string) string { +// matchingRegions returns every configured region whose derived code equals +// the packet's code1, rather than the first one found. +// +// The distinction matters because code1 is two bytes: two configured regions +// collide on a given payload with probability 1/65536, and at 159 keys on a +// live instance that is roughly 0.25% of transport-scoped packets, hundreds a +// week rather than a curiosity. Returning the first match made the stored +// region name depend on Go's randomised map iteration order, so the same +// packet could be named differently on two runs and neither answer was +// evidence of anything. +// +// The cost is unchanged: this is the same single pass over the same keys, it +// just does not stop early. There is no indexable shortcut, because code1 is +// an HMAC over the payload and nothing here is payload-independent. +func matchingRegions(regionKeys map[string][]byte, payloadType byte, payloadRaw []byte, code1 string) []string { if code1 == "0000" || len(regionKeys) == 0 || len(payloadRaw) == 0 { - return "" + return nil } + var matched []string for name, key := range regionKeys { mac := hmac.New(sha256.New, key) mac.Write([]byte{payloadType}) @@ -1654,9 +1667,35 @@ func matchScope(regionKeys map[string][]byte, payloadType byte, payloadRaw []byt } codeBytes := [2]byte{byte(code & 0xFF), byte(code >> 8)} if strings.ToUpper(hex.EncodeToString(codeBytes[:])) == code1 { - return name + matched = append(matched, name) } } + return matched +} + +// matchScope names one packet's region scope, or returns "" when it cannot be +// named with confidence. +// +// Exactly one match names the packet. Several matches name nothing: the two +// candidates are equally sourced, there is no principled winner, and storing +// a wrong region name is worse than storing none. "" is already the ingestor's +// "transport-scoped but unnameable" state (scopeNameForDB), so an ambiguous +// packet lands in a state the rest of the system already understands rather +// than in a new one. +// +// The collision is logged because it is otherwise invisible: the packet is +// stored exactly like one whose region this instance holds no key for, and an +// operator looking at a growing unnameable count deserves to see which of +// their configured regions are colliding. +func matchScope(regionKeys map[string][]byte, payloadType byte, payloadRaw []byte, code1 string) string { + matched := matchingRegions(regionKeys, payloadType, payloadRaw, code1) + switch len(matched) { + case 0: + return "" + case 1: + return matched[0] + } + log.Printf("[regions] ambiguous collision between %v; storing unmatched", matched) return "" } diff --git a/cmd/ingestor/main_test.go b/cmd/ingestor/main_test.go index 9d241f46..cbc6293c 100644 --- a/cmd/ingestor/main_test.go +++ b/cmd/ingestor/main_test.go @@ -2,6 +2,8 @@ package main import ( "bytes" + "crypto/hmac" + "crypto/sha256" "database/sql" "encoding/hex" "encoding/json" @@ -10,6 +12,7 @@ import ( "os" "path/filepath" "runtime" + "strings" "testing" "time" @@ -1186,3 +1189,88 @@ func TestHandleMessageAdvert_MatchedScopeUpdatesDefaultScope(t *testing.T) { t.Errorf("default_scope after matched-scope advert = %q (valid=%v), want #de", got.String, got.Valid) } } + +// codeForRegion derives the on-wire code1 a sender in this region would emit +// for this payload: the forward direction of what matchScope inverts. +func codeForRegion(name string, payloadType byte, payload []byte) string { + if !strings.HasPrefix(name, "#") { + name = "#" + name + } + sum := sha256.Sum256([]byte(name)) + mac := hmac.New(sha256.New, sum[:16]) + mac.Write([]byte{payloadType}) + mac.Write(payload) + h := mac.Sum(nil) + code := uint16(h[0]) | uint16(h[1])<<8 + if code == 0 { + code = 1 + } else if code == 0xFFFF { + code = 0xFFFE + } + return strings.ToUpper(hex.EncodeToString([]byte{byte(code & 0xFF), byte(code >> 8)})) +} + +// findRegionCollision searches for a payload whose code1 is identical under two +// region names. code1 is two bytes, so one turns up after ~65k tries and the +// search costs a fraction of a second. +// +// A hand-built fixture cannot stand in here: the whole point of the case below +// is what happens when the matcher genuinely finds two names for one packet, +// and a fabricated code1 would only prove the test agrees with itself. +func findRegionCollision(t *testing.T, nameA, nameB string, payloadType byte) ([]byte, string) { + t.Helper() + payload := make([]byte, 4) + for i := 0; i < 1<<22; i++ { + payload[0], payload[1] = byte(i), byte(i>>8) + payload[2], payload[3] = byte(i>>16), byte(i>>24) + if a, b := codeForRegion(nameA, payloadType, payload), codeForRegion(nameB, payloadType, payload); a == b { + return append([]byte(nil), payload...), a + } + } + t.Fatalf("no code1 collision between %s and %s in 2^22 payloads", nameA, nameB) + return nil, "" +} + +// TestMatchScopeNamesAnUnambiguousPacket is the ordinary case: one configured +// region derives the packet's code1, so the packet carries that region's name. +func TestMatchScopeNamesAnUnambiguousPacket(t *testing.T) { + keys := loadRegionKeys(&Config{HashRegions: []string{"#be", "#nl"}}) + payload := []byte{0x01, 0x02, 0x03, 0x04} + code1 := codeForRegion("#be", 5, payload) + + if got := matchScope(keys, 5, payload, code1); got != "#be" { + t.Errorf("matchScope = %q, want %q", got, "#be") + } +} + +// TestMatchScopeStoresAmbiguousAsUnmatched pins the reason this changed. Two +// configured regions derive the same code1 for one payload; naming the packet +// after either is a coin flip, and before this the flip was Go's map iteration +// order, so the same packet could be stored under different regions on two +// runs of the same binary. +func TestMatchScopeStoresAmbiguousAsUnmatched(t *testing.T) { + payload, code1 := findRegionCollision(t, "#be", "#zz", 5) + keys := loadRegionKeys(&Config{HashRegions: []string{"#be", "#zz"}}) + + if n := len(matchingRegions(keys, 5, payload, code1)); n != 2 { + t.Fatalf("matchingRegions returned %d names, want 2 — the collision fixture is wrong", n) + } + if got := matchScope(keys, 5, payload, code1); got != "" { + t.Errorf("matchScope = %q, want the unmatched state: two equally-sourced candidates have no principled winner", got) + } +} + +// TestMatchScopeIsOrderIndependent runs the ambiguous case repeatedly. Map +// iteration order is randomised per range in Go, so a first-match matcher +// returns different names across iterations of this loop; the answer must not +// move. +func TestMatchScopeIsOrderIndependent(t *testing.T) { + payload, code1 := findRegionCollision(t, "#be", "#zz", 5) + keys := loadRegionKeys(&Config{HashRegions: []string{"#be", "#zz"}}) + + for i := 0; i < 50; i++ { + if got := matchScope(keys, 5, payload, code1); got != "" { + t.Fatalf("iteration %d: matchScope = %q, want a stable answer across map iteration orders", i, got) + } + } +}