Skip to content

Commit

Permalink
[internal-branch.go1.18-vendor] http2: limit canonical header cache b…
Browse files Browse the repository at this point in the history
…y bytes, not entries

The canonical header cache is a per-connection cache mapping header
keys to their canonicalized form. (For example, "foo-bar" => "Foo-Bar").
We limit the number of entries in the cache to prevent an attacker
from consuming unbounded amounts of memory by sending many unique
keys, but a small number of very large keys can still consume an
unreasonable amount of memory.

Track the amount of memory consumed by the cache and limit it based
on memory rather than number of entries.

Thanks to Josselin Costanzi for reporting this issue.

For golang/go#56350
For golang/go#57008
Fixes CVE-2022-41717

Change-Id: Ief3c141001524fd3776958ecc8556c724427f063
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1619953
Reviewed-by: Roland Shoemaker <[email protected]>
Reviewed-by: Julie Qiu <[email protected]>
Run-TryBot: Damien Neil <[email protected]>
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1662692
Reviewed-by: Tatiana Bradley <[email protected]>
Reviewed-on: https://go-review.googlesource.com/c/net/+/455735
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Damien Neil <[email protected]>
Reviewed-by: Jenny Rakoczy <[email protected]>
  • Loading branch information
neild authored and heschi committed Dec 9, 2022
1 parent 0a43f88 commit 4205dd4
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 7 deletions.
18 changes: 11 additions & 7 deletions http2/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -528,6 +528,7 @@ type serverConn struct {
headerTableSize uint32
peerMaxHeaderListSize uint32 // zero means unknown (default)
canonHeader map[string]string // http2-lower-case -> Go-Canonical-Case
canonHeaderKeysSize int // canonHeader keys size in bytes
writingFrame bool // started writing a frame (on serve goroutine or separate)
writingFrameAsync bool // started a frame on its own goroutine but haven't heard back on wroteFrameCh
needsFrameFlush bool // last frame write wasn't a flush
Expand Down Expand Up @@ -704,6 +705,13 @@ func (sc *serverConn) condlogf(err error, format string, args ...interface{}) {
}
}

// maxCachedCanonicalHeadersKeysSize is an arbitrarily-chosen limit on the size
// of the entries in the canonHeader cache.
// This should be larger than the size of unique, uncommon header keys likely to
// be sent by the peer, while not so high as to permit unreasonable memory usage
// if the peer sends an unbounded number of unique header keys.
const maxCachedCanonicalHeadersKeysSize = 2048

func (sc *serverConn) canonicalHeader(v string) string {
sc.serveG.check()
buildCommonHeaderMapsOnce()
Expand All @@ -719,14 +727,10 @@ func (sc *serverConn) canonicalHeader(v string) string {
sc.canonHeader = make(map[string]string)
}
cv = http.CanonicalHeaderKey(v)
// maxCachedCanonicalHeaders is an arbitrarily-chosen limit on the number of
// entries in the canonHeader cache. This should be larger than the number
// of unique, uncommon header keys likely to be sent by the peer, while not
// so high as to permit unreaasonable memory usage if the peer sends an unbounded
// number of unique header keys.
const maxCachedCanonicalHeaders = 32
if len(sc.canonHeader) < maxCachedCanonicalHeaders {
size := 100 + len(v)*2 // 100 bytes of map overhead + key + value
if sc.canonHeaderKeysSize+size <= maxCachedCanonicalHeadersKeysSize {
sc.canonHeader[v] = cv
sc.canonHeaderKeysSize += size
}
return cv
}
Expand Down
26 changes: 26 additions & 0 deletions http2/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4411,3 +4411,29 @@ func TestProtocolErrorAfterGoAway(t *testing.T) {
}
}
}

// TestCanonicalHeaderCacheGrowth verifies that the canonical header cache
// size is capped to a reasonable level.
func TestCanonicalHeaderCacheGrowth(t *testing.T) {
for _, size := range []int{1, (1 << 20) - 10} {
base := strings.Repeat("X", size)
sc := &serverConn{
serveG: newGoroutineLock(),
}
const count = 1000
for i := 0; i < count; i++ {
h := fmt.Sprintf("%v-%v", base, i)
c := sc.canonicalHeader(h)
if len(h) != len(c) {
t.Errorf("sc.canonicalHeader(%q) = %q, want same length", h, c)
}
}
total := 0
for k, v := range sc.canonHeader {
total += len(k) + len(v) + 100
}
if total > maxCachedCanonicalHeadersKeysSize {
t.Errorf("after adding %v ~%v-byte headers, canonHeader cache is ~%v bytes, want <%v", count, size, total, maxCachedCanonicalHeadersKeysSize)
}
}
}

0 comments on commit 4205dd4

Please sign in to comment.