Harden DistributedCacheTagHelper key serialization to be injective (CacheTagKey)

[rokonec]

Summary

CacheTagKey.GenerateKey() serializes the cache-key state by concatenating components with an unescaped || token separator. The serialization is not injective: two different structured key states (different sets of vary-by name/value pairs) can produce the same serialized string. For the DistributedCacheTagHelper path this serialized string is hashed and used as the distributed storage key, so distinct logical key states can collapse onto a single distributed cache entry.

This is filed as a defense-in-depth hardening item. The request is to make the distributed key serialization injective so distinct key states always map to distinct storage keys.

What is wrong

• CacheTagKey.GenerateKey() builds the key as text using the constant CacheKeyTokenSeparator = "||" for both intra-pair and inter-pair boundaries, with no escaping or length-prefixing of component values. Because the same token separates a name from its value and one pair from the next, the mapping from structured state to string is ambiguous (not injective).
• DistributedCacheTagHelperService derives the distributed storage key from GenerateHashedKey() (SHA-256 over GenerateKey()), and also embeds the serialized key alongside the cached payload as a validation check. Both the hash input and the embedded validation bytes are the ambiguous serialization, so neither detects two distinct states that serialize identically.
• The in-memory CacheTagHelper path is not affected: it keys IMemoryCache by the structured CacheTagKey object, whose Equals/GetHashCode compare the individual components. Only the distributed path relies on the flattened string.

Why it matters (defense in depth)

• Correctness: independent of any adversary, an application that varies a distributed-cached fragment by values that can contain the separator sequence can get incorrect cache reuse (one logical variant served for another). This is a latent caching-correctness bug.
• Hardening: the distributed cache key is an isolation boundary between logical fragments. An injective key generator guarantees that distinct vary states cannot share a storage entry, strengthening fragment isolation as a matter of robustness rather than relying on applications to avoid particular characters in vary values.
• The existing embedded-serialized-key validation was designed to catch different strings that share a hash digest; it cannot catch two states that share the same pre-hash serialization. Making the serialization injective closes that gap at the source.

Affected code

• src/Mvc/Mvc.TagHelpers/src/Cache/CacheTagKey.cs:26-33 - CacheKeyTokenSeparator and vary-section name constants.
• src/Mvc/Mvc.TagHelpers/src/Cache/CacheTagKey.cs:113-167 - GenerateKey() string assembly (scalars, collections, user, culture).
• src/Mvc/Mvc.TagHelpers/src/Cache/CacheTagKey.cs:169-183 - GenerateHashedKey() (SHA-256 over the generated string).
• src/Mvc/Mvc.TagHelpers/src/Cache/CacheTagKey.cs:292-320 - AddStringCollection() collection serialization (the ambiguous name/value framing).
• src/Mvc/Mvc.TagHelpers/src/Cache/DistributedCacheTagHelperService.cs:65-117 - lookup, render-on-miss, and value encoding using the serialized/hashed key.
• src/Mvc/Mvc.TagHelpers/src/Cache/DistributedCacheTagHelperService.cs:164-202 - embedded serialized-key encode/validate on the cached value.

Parity note: the relevant source is byte-identical across origin/main and origin/release/10.0, release/9.0, release/8.0, so the same fix applies to servicing branches. Introduced by commit 341430eae508912d3a868bf636b86ec04155378e ("Introducing CacheTagKey").

Recommended fix

Selected approach: replace the delimiter-based text serialization with a single versioned, injective canonical encoding, and use the exact same canonical bytes for (a) the SHA-256 storage key and (b) the embedded validation bytes.

Design points:

• Frame every component with an explicit length (or an unambiguous binary layout) so no value can be confused with a separator or with an adjacent component. Encode collections with an explicit element count, and frame each name and each value independently.
• Preserve the current structured identity exactly: base key/prefix, vary-by, the four ordered collections (cookie, header, query, route), vary-by-user (username) and vary-by-culture (culture + UI culture). Keep expiration fields out of the content key, matching current behavior.
• Preserve existing normalization: a missing selected value maps to empty string today (ExtractCollection), and header/query StringValues are already collapsed to a single string before the pair is built. Keep both behaviors; only the framing changes.
• Distinguish null vs empty consistently (for example a null collection vs a present-but-empty collection), so the encoding matches the structured object identity rather than reintroducing hidden aliases.
• Version and namespace the output: prefix the distributed storage key with an explicit format version. Increment the version so pre-fix (v1) entries are not read by post-fix code.

Public surface / compatibility:

• GenerateKey() and GenerateHashedKey() are public. Prefer keeping their signatures; GenerateKey() may return a versioned printable representation of the canonical bytes rather than the legacy grammar. No PublicAPI.Shipped.txt change is expected if signatures are unchanged.
• Cache invalidation: changing the key format makes existing v1 distributed entries unreachable. They should expire under their existing policy. Do not implement a v1 read-fallback or dual-write, since either would reinstate the ambiguous mapping on a v2 miss. During a rolling deployment, patched and unpatched instances will use separate namespaces (temporary duplicate entries and cold misses); call this out in release notes.

Alternatives considered:

• Escaping the separator: requires a complete, permanently stable escape grammar for the escape char, ||, the collection parentheses, and any future component; a single missed context reintroduces the ambiguity. Rejected.
• Rejecting values that contain the separator: breaks legitimate vary values and turns valid requests into failures or cache bypasses; it filters symptoms rather than fixing the serialization. Rejected.
• Textual length-prefixing: viable and injective, but adds decimal-parsing and character-vs-byte-length edge cases with no operational benefit over a binary canonical form (the final storage key is already a digest). Not selected.

Acceptance criteria

• For any two CacheTagKey states that are unequal by Equals, GenerateKey() and GenerateHashedKey() produce different results (injectivity), including when values contain the separator or collection punctuation.
• The distributed storage key, the hash input, and the embedded validation bytes all derive from one canonical encoding.
• The in-memory CacheTagHelper behavior is unchanged.
• Deterministic format vectors lock the encoding: ASCII, non-ASCII, empty, null-vs-empty, each collection boundary, each pair boundary, user, and culture components.
• A regression test proves two previously colliding vary states resolve to distinct distributed entries and render independently.
• A test confirms v2 code does not read a pre-populated v1 (legacy) storage entry.
• The same encoding is applied on main and backported unchanged to release/8.0, 9.0, 10.0; release-note text covers one-time cache misses and expiration of unreachable legacy entries.

References

• Existing tests to extend: src/Mvc/Mvc.TagHelpers/test/CacheTagKeyTest.cs and src/Mvc/Mvc.TagHelpers/test/DistributedCacheTagHelperTest.cs.
• Detailed design and remediation notes were produced during investigation and can be shared with the implementer on request.