Gorouter panics on certain B3 headers #346
Description
Is this a security vulnerability?
No. It would be a security vulnerability because the error causes gorouter to panic. However, the panic is recovered by the panic-check handler, so gorouter does not crash.
Issue
In production systems, we have observed various calls to the "panic-check" handler. They all look like this one:
Panic Check Log
``` { "log_level": 3, "timestamp": "2023-09-15T10:02:27.639222020Z", "message": "panic-check", "source": "vcap.gorouter", "data": { "host": "one-mds-log-api.cfapps.sap.hana.ondemand.com", "error": { "error": "runtime error: slice bounds out of range [:20] with length 16", "stacktrace": "goroutine 4020 [running]: github.com/uber-go/zap.takeStacktrace({0xc011761800?, 0xd52ac0?, 0xc01a534b80?}, 0xc5?) /Users/d068861/routing-lab/routing-release/src/code.cloudfoundry.org/vendor/github.com/uber-go/zap/stacktrace.go:39 +0xa5 github.com/uber-go/zap.Stack() /Users/d068861/routing-lab/routing-release/src/code.cloudfoundry.org/vendor/github.com/uber-go/zap/field.go:155 +0x79 code.cloudfoundry.org/gorouter/handlers.(*panicCheck).ServeHTTP.func1() /Users/d068861/routing-lab/routing-release/src/code.cloudfoundry.org/gorouter/handlers/paniccheck.go:44 +0x251 panic({0xbf6900, 0xc01980be90}) /usr/local/go/src/runtime/panic.go:884 +0x213 code.cloudfoundry.org/gorouter/handlers.(*RequestInfo).SetTraceInfo(0xc484a3?, {0xc019fd6620?, 0xc00dbc5298?}, {0xc019fd6640?, 0x1?}) /Users/d068861/routing-lab/routing-release/src/code.cloudfoundry.org/gorouter/handlers/requestinfo.go:107 +0x387 code.cloudfoundry.org/gorouter/handlers.(*Zipkin).ServeHTTP(0xc00017e318, {0xd5a8a0?, 0xc018e73b30?}, 0xc018e7dc00, 0x40c4ad?) /Users/d068861/routing-lab/routing-release/src/code.cloudfoundry.org/gorouter/handlers/zipkin.go:60 +0x958 github.com/urfave/negroni.middleware.ServeHTTP({{0xd55220?, 0xc00017e318?}, 0xc00017f5f0?}, {0xd5a8a0, 0xc018e73b30}, 0xc0198ad500?) /Users/d068861/routing-lab/routing-release/src/code.cloudfoundry.org/vendor/github.com/urfave/negroni/negroni.go:38 +0xb6 code.cloudfoundry.org/gorouter/handlers.(*proxyWriterHandler).ServeHTTP(0xc000193820, {0x7f3b5512f120?, 0xc013642ea8}, 0xf8?, 0xc010e98180) /Users/d068861/routing-lab/routing-release/src/code.cloudfoundry.org/gorouter/handlers/proxywriter.go:34 +0x222 github.com/urfave/negroni.middleware.ServeHTTP({{0xd55380?, 0xc000193820?}, 0xc00017f5d8?}, {0x7f3b5512f120, 0xc013642ea8}, 0x4902d7?) /Users/d068861/routing-lab/routing-release/src/code.cloudfoundry.org/vendor/github.com/urfave/negroni/negroni.go:38 +0xb6 code.cloudfoundry.org/gorouter/handlers.(*RequestInfoHandler).ServeHTTP(0x417a50?, {0x7f3b5512f120, 0xc013642ea8}, 0xc018e7db00, 0xc010e98160) /Users/d068861/routing-lab/routing-release/src/code.cloudfoundry.org/gorouter/handlers/requestinfo.go:175 +0x16e github.com/urfave/negroni.middleware.ServeHTTP({{0xd551a0?, 0x12e36b0?}, 0xc00017f5c0?}, {0x7f3b5512f120, 0xc013642ea8}, 0xb94320?) /Users/d068861/routing-lab/routing-release/src/code.cloudfoundry.org/vendor/github.com/urfave/negroni/negroni.go:38 +0xb6 code.cloudfoundry.org/gorouter/handlers.(*panicCheck).ServeHTTP(0x8?, {0x7f3b5512f120?, 0xc013642ea8?}, 0x0?, 0xc00dbc5a50?) /Users/d068861/routing-lab/routing-release/src/code.cloudfoundry.org/gorouter/handlers/paniccheck.go:53 +0x86 github.com/urfave/negroni.middleware.ServeHTTP({{0xd55320?, 0xc00017e360?}, 0xc00017f5a8?}, {0x7f3b5512f120, 0xc013642ea8}, 0xc019c56e00?) /Users/d068861/routing-lab/routing-release/src/code.cloudfoundry.org/vendor/github.com/urfave/negroni/negroni.go:38 +0xb6 github.com/urfave/negroni.(*Negroni).ServeHTTP(0xc000038e40, {0xd5b0e0?, 0xc0198dc1c0}, 0x46f42e?) /Users/d068861/routing-lab/routing-release/src/code.cloudfoundry.org/vendor/github.com/urfave/negroni/negroni.go:96 +0x125 net/http.serverHandler.ServeHTTP({0xc0198ad500?}, {0xd5b0e0, 0xc0198dc1c0}, 0xc018e7db00) /usr/local/go/src/net/http/server.go:2936 +0x316 net/http.(*conn).serve(0xc019908bd0, {0xd5ba50, 0xc01181dbc0}) /usr/local/go/src/net/http/server.go:1995 +0x612 created by net/http.(*Server).Serve /usr/local/go/src/net/http/server.go:3089 +0x5ed " } } } ```For debugging, I've added more log statements to get the actual values of the b3 headers that caused this.
"b3-header":"8eca11bb567a36a9-ae7070a7c9a32917-0-49bde5c0a8036d7d"
gets parsed to"traceid":"8eca11bb567a36a9","id":"ae7070a7c9a32917""b3-header":"239bd225429ffcca-239bd225429ffcca-0"gets parsed to"traceid":"239bd225429ffcca","id":"239bd225429ffcca"
Both variants are parsed by b3.ParseSingleHeader(existingContext) without error. Subsequently, they get handed over to requestInfo.SetTraceInfo where hard assumptions are made on the length of traceID. If the string is shorter than 20 characters we get the slice bounds out of range error. In both cases, the traceIDis only 16 chars long which causes the panic.
Affected Versions
Starting from routing-release 269
Steps to Reproduce
Send a HTTP request with either of the two mentioned b3-headers to gorouter.
Expected result
Gorouter should not panic. It should either accept or reject the b3 headers as trace context.
Current result
Gorouter panics. The request fails.
Possible Fix
- Gorouter should never access strings hard-coded without checking their length first.
- Gorouter should either accept or reject b3 headers gracefully, so I assume we need additional handling for the case where zipkin does parse successfully but Gorouter expects a different format for the trace id.
- From what I've read, the traceid has a minimum length of 16 and maximum length of 32 chars. So Gorouter can't make the assumption that it is always at least 20 chars long.