flows: Flow.FastHash is commutative by design — any tool keying connections on it merges four 4-tuples into one #5

New issue

Open

opened 2026-08-26 09:53:04 +00:00 by claude · 1 comment

claude commented

2026-08-26 09:53:04 +00:00

Collaborator

Severity: medium (as a gopacket issue) / high for any caller that keys on it · flows.go:161-175

This is daisy-findings.md finding 8. Confirmed, and it collapses more than the two flows that document describes.

Mechanism

// FastHash provides a quick hashing function for a flow, useful if you'd
// like to split up flows by modulos or other load-balancing techniques.
// It uses a variant of Fowler-Noll-Vo hashing, and is guaranteed to collide
// with its reverse flow.  IE: the flow A->B will have the same hash as the flow
// B->A.
func (f Flow) FastHash() (h uint64) {
        // This combination must be commutative.  We don't use ^, since that would
        // give the same hash for all A->A flows.
        h = fnvHash(f.src[:f.slen]) + fnvHash(f.dst[:f.dlen])
        h ^= uint64(f.typ)
        h *= fnvPrime
        return
}

The commutativity is deliberate, documented, and correct for its stated purpose — pinning both directions of a conversation to the same worker. gopacket is not doing anything wrong here in isolation.

The problem is that it is the obvious-looking thing to reach for when building a connection key, and the result is silently wrong. The common idiom is:

func connKey(netFlow, tcpFlow gopacket.Flow) uint64 {
        return netFlow.FastHash() ^ tcpFlow.FastHash()
}

Because both halves are commutative, this collapses not two but four distinct 4-tuples onto one key:

=== FastHash collision (flows.go:167) ===
  49152->8080              FastHash=0xe620d734ebe1daf9
  8080->49152              FastHash=0xe620d734ebe1daf9   equal=true

  netFlow^tcpFlow keys for four distinct 4-tuples:
    A:49152->B:8080  0x269c8016be4ee5cd
    A:8080 ->B:49152 0x269c8016be4ee5cd  same=true
    B:49152->A:8080  0x269c8016be4ee5cd  same=true

A:49152→B:8080 and A:8080→B:49152 are two genuinely different connections. So are A:49152→B:8080 and B:49152→A:8080. All four share a key, found with zero brute force — the attacker just picks their source port.

Impact

An attribution primitive that works in both directions:

Hide. Open your exploit connection with source port P against service port Q, then also open a connection from source port Q to service port P (or bind the mirrored pair). Both fold into one flow record; the exploit's messages are indistinguishable from the decoy's.
Fabricate. Send traffic that merges into a flow you do not own, so the recorded conversation for someone else's connection contains your bytes. Everything downstream inherits it: operator triage, generated block rules, replay snippets, flag-detection attribution.

Costs one extra connection.

What to change

In callers: stop keying on any hash. Key on the ordered tuple itself:

type connKey struct {
        net, transport gopacket.Flow   // gopacket.Flow is comparable
}
var conns map[connKey]*conn

gopacket.Flow is a comparable struct, so a map[connKey] is exact and no slower in practice. If a canonical direction-insensitive key is genuinely wanted, canonicalise explicitly — min(endpoint)/max(endpoint) — rather than relying on a hash's collision behaviour to do it implicitly.

In gopacket: the doc comment already warns the output "is not guaranteed to remain the same through future code revisions, so should not be used to key values in persistent storage." That warning is about stability, not collisions, and it reads as though in-memory keying is fine. Worth adding a sentence that says outright: this is not a connection identifier, and A:p→B:q and A:q→B:p will collide. A Flow.Key() or exported comparable-tuple helper would give callers the obvious right thing to reach for.

Verified against b7d9dbd on Go 1.24.4. PoC: flowsyn.

**Severity: medium** (as a gopacket issue) / **high** for any caller that keys on it · `flows.go:161-175` This is `daisy-findings.md` finding 8. Confirmed, and it collapses more than the two flows that document describes. ## Mechanism ```go // FastHash provides a quick hashing function for a flow, useful if you'd // like to split up flows by modulos or other load-balancing techniques. // It uses a variant of Fowler-Noll-Vo hashing, and is guaranteed to collide // with its reverse flow. IE: the flow A->B will have the same hash as the flow // B->A. func (f Flow) FastHash() (h uint64) { // This combination must be commutative. We don't use ^, since that would // give the same hash for all A->A flows. h = fnvHash(f.src[:f.slen]) + fnvHash(f.dst[:f.dlen]) h ^= uint64(f.typ) h *= fnvPrime return } ``` The commutativity is deliberate, documented, and correct for its stated purpose — pinning both directions of a conversation to the same worker. gopacket is not doing anything wrong here in isolation. The problem is that it is the obvious-looking thing to reach for when building a connection key, and the result is silently wrong. The common idiom is: ```go func connKey(netFlow, tcpFlow gopacket.Flow) uint64 { return netFlow.FastHash() ^ tcpFlow.FastHash() } ``` Because **both** halves are commutative, this collapses not two but **four** distinct 4-tuples onto one key: ``` === FastHash collision (flows.go:167) === 49152->8080 FastHash=0xe620d734ebe1daf9 8080->49152 FastHash=0xe620d734ebe1daf9 equal=true netFlow^tcpFlow keys for four distinct 4-tuples: A:49152->B:8080 0x269c8016be4ee5cd A:8080 ->B:49152 0x269c8016be4ee5cd same=true B:49152->A:8080 0x269c8016be4ee5cd same=true ``` `A:49152→B:8080` and `A:8080→B:49152` are two genuinely different connections. So are `A:49152→B:8080` and `B:49152→A:8080`. All four share a key, found with **zero brute force** — the attacker just picks their source port. ## Impact An attribution primitive that works in both directions: - **Hide.** Open your exploit connection with source port `P` against service port `Q`, then also open a connection from source port `Q` to service port `P` (or bind the mirrored pair). Both fold into one flow record; the exploit's messages are indistinguishable from the decoy's. - **Fabricate.** Send traffic that merges into a flow you do not own, so the recorded conversation for someone else's connection contains your bytes. Everything downstream inherits it: operator triage, generated block rules, replay snippets, flag-detection attribution. Costs one extra connection. ## What to change **In callers:** stop keying on any hash. Key on the ordered tuple itself: ```go type connKey struct { net, transport gopacket.Flow // gopacket.Flow is comparable } var conns map[connKey]*conn ``` `gopacket.Flow` is a comparable struct, so a `map[connKey]` is exact and no slower in practice. If a canonical direction-insensitive key is genuinely wanted, canonicalise explicitly — `min(endpoint)/max(endpoint)` — rather than relying on a hash's collision behaviour to do it implicitly. **In gopacket:** the doc comment already warns the output "is not guaranteed to remain the same through future code revisions, so should not be used to key values in persistent storage." That warning is about *stability*, not *collisions*, and it reads as though in-memory keying is fine. Worth adding a sentence that says outright: this is not a connection identifier, and `A:p→B:q` and `A:q→B:p` will collide. A `Flow.Key()` or exported comparable-tuple helper would give callers the obvious right thing to reach for. --- *Verified against `b7d9dbd` on Go 1.24.4. PoC: `flowsyn`.*

claude added the

labels

2026-08-26 09:53:06 +00:00

claude commented

2026-08-26 12:29:12 +00:00

Author

Collaborator

Independent verification — reproduces in `gopacket/gopacket v1.7.0`, including the exact reported hash

Confirmed against the maintained fork. The FastHash value matches the one in this issue byte for byte:

=== #5 Flow.FastHash commutativity (gopacket/gopacket v1.7.0) ===
  tcp 49152->8080  FastHash=0xe620d734ebe1daf9
  tcp 8080->49152  FastHash=0xe620d734ebe1daf9  equal=true

  connKey(netFlow, tcpFlow) = netFlow.FastHash() ^ tcpFlow.FastHash():
    A:49152 -> B:8080   0x21051330ff6ed929  same_as_first=true
    A:8080  -> B:49152  0x21051330ff6ed929  same_as_first=true
    B:49152 -> A:8080   0x21051330ff6ed929  same_as_first=true
    B:8080  -> A:49152  0x21051330ff6ed929  same_as_first=true
    A:49153 -> B:8080   0x2138b530ff51f400  same_as_first=false

(The key differs from the issue's 0x269c… only because the IP endpoints differ — the issue doesn't state which addresses it used. The collapse behaviour is identical, and the control with a single incremented source port separates correctly.)

The idiom is not hypothetical

This issue frames the risk as "the obvious-looking thing to reach for". Worth recording that the consumer audited alongside it does exactly that, verbatim, as its connection key:

func connKey(netFlow, tcpFlow gopacket.Flow) uint64 {
	return netFlow.FastHash() ^ tcpFlow.FastHash()
}

Same two-commutative-halves construction, same four-way collapse. It reached production without anyone noticing, which is the strongest argument for the severity split this issue proposes (medium for gopacket, high for the caller).

Suggested doc change

The doc comment is accurate about what FastHash guarantees, but the guarantee is stated as a feature ("guaranteed to collide with its reverse flow") without a corresponding warning about what that rules out. A single added sentence would likely have prevented this:

Because this is commutative, it MUST NOT be used as a connection identity key — A:x→B:y, A:y→B:x, B:x→A:y and B:y→A:x all hash identically, and combining a net-flow and a transport-flow hash with XOR does not fix that. Use Flow.Endpoints() with a canonical ordering if you need a key that distinguishes directions.

Also worth considering a Flow.OrderedHash() (or exporting the endpoint bytes in a canonical order) so the correct thing is as easy to reach for as the wrong one. Right now every caller has to invent it.

PoC

func pflow(a, b uint16) gopacket.Flow {
	f, _ := gopacket.FlowFromEndpoints(
		layers.NewTCPPortEndpoint(layers.TCPPort(a)),
		layers.NewTCPPortEndpoint(layers.TCPPort(b)))
	return f
}
fmt.Println(pflow(49152, 8080).FastHash() == pflow(8080, 49152).FastHash()) // true

Verified on Go 1.24.4 against gopacket/gopacket v1.7.0.

## Independent verification — reproduces in `gopacket/gopacket v1.7.0`, including the exact reported hash Confirmed against the maintained fork. The `FastHash` value matches the one in this issue byte for byte: ``` === #5 Flow.FastHash commutativity (gopacket/gopacket v1.7.0) === tcp 49152->8080 FastHash=0xe620d734ebe1daf9 tcp 8080->49152 FastHash=0xe620d734ebe1daf9 equal=true connKey(netFlow, tcpFlow) = netFlow.FastHash() ^ tcpFlow.FastHash(): A:49152 -> B:8080 0x21051330ff6ed929 same_as_first=true A:8080 -> B:49152 0x21051330ff6ed929 same_as_first=true B:49152 -> A:8080 0x21051330ff6ed929 same_as_first=true B:8080 -> A:49152 0x21051330ff6ed929 same_as_first=true A:49153 -> B:8080 0x2138b530ff51f400 same_as_first=false ``` (The key differs from the issue's `0x269c…` only because the IP endpoints differ — the issue doesn't state which addresses it used. The collapse behaviour is identical, and the control with a single incremented source port separates correctly.) ### The idiom is not hypothetical This issue frames the risk as *"the obvious-looking thing to reach for"*. Worth recording that the consumer audited alongside it does exactly that, verbatim, as its connection key: ```go func connKey(netFlow, tcpFlow gopacket.Flow) uint64 { return netFlow.FastHash() ^ tcpFlow.FastHash() } ``` Same two-commutative-halves construction, same four-way collapse. It reached production without anyone noticing, which is the strongest argument for the severity split this issue proposes (medium for gopacket, high for the caller). ### Suggested doc change The doc comment is accurate about what `FastHash` guarantees, but the guarantee is stated as a *feature* ("guaranteed to collide with its reverse flow") without a corresponding warning about what that rules out. A single added sentence would likely have prevented this: > Because this is commutative, it MUST NOT be used as a connection identity key — `A:x→B:y`, `A:y→B:x`, `B:x→A:y` and `B:y→A:x` all hash identically, and combining a net-flow and a transport-flow hash with XOR does not fix that. Use `Flow.Endpoints()` with a canonical ordering if you need a key that distinguishes directions. Also worth considering a `Flow.OrderedHash()` (or exporting the endpoint bytes in a canonical order) so the correct thing is as easy to reach for as the wrong one. Right now every caller has to invent it. ### PoC ```go func pflow(a, b uint16) gopacket.Flow { f, _ := gopacket.FlowFromEndpoints( layers.NewTCPPortEndpoint(layers.TCPPort(a)), layers.NewTCPPortEndpoint(layers.TCPPort(b))) return f } fmt.Println(pflow(49152, 8080).FastHash() == pflow(8080, 49152).FastHash()) // true ``` --- *Verified on Go 1.24.4 against `gopacket/gopacket v1.7.0`.*