Feature request for modifying how zipkin tracer acts upon receiving bad headers

[danielkwinsor]

Description:

Send non hex characters as request header x-b3-traceid or x-b3-spanid, or send headers that are not of 64/128bit hex length. The tracer operates correctly and securely in the sense that it was not a sampled trace and was not submitted to zipkin collector.

However, this unverified data can then be sent to upstream, to access logs, added in headers_to_add, etc, with a small possible risk of introducing malicious input handled somewhere that was expecting a mandated format. Request to validate the traceid and span ids are of appropriate format, otherwise reject the supplied trace and start a new trace (just like what happens when trace is missing entirely) so that systems expect to always have valid headers with this config. An alternative is to strip all the headers. I believe that starting a new trace is the behavior of spring-cloud-sleuth.

Config:

+                  tracing:
+                    provider:
+                      name: envoy.tracers.zipkin
+                      typed_config:
+                        "@type": type.googleapis.com/envoy.config.trace.v3.ZipkinConfig
+                        trace_id_128bit: true
+                        collector_cluster: remote_zipkin
+                        collector_endpoint: "/api/v2/spans"
+                        collector_endpoint_version: HTTP_JSON

Logs: There are no logs at debug level when this rejects the trace. Possibly could be logged or gauged.

Relevant: The below curl output demonstrates echoing back the input – notice it is not sampled with x-b3-sampled: 1

curl -k -v https://localhost/echo -H "x-b3-traceid: 273d1yyyyyyyyyyyyyyy <>{} @#$%^&*()" -H "x-b3-spanid: c115c94374c93923"
> GET /echo HTTP/1.1
> Host: localhost
> User-Agent: curl/7.54.0
> Accept: */*
> x-b3-traceid: 273d1yyyyyyyyyyyyyyy <>{} @#$%^&*()
> x-b3-spanid: c115c94374c93923
>
< HTTP/1.1 200 OK
< date: Mon Aug 17 2020 13:23:46 GMT-0700 (Pacific Daylight Time)
< content-type: text/plain
< access-control-allow-origin: *
< x-envoy-upstream-service-time: 1
< x-response-traceid: 273d1yyyyyyyyyyyyyyy <>{} @#$%^&*() ****<- added via response_headers_to_add
< server: envoy
< transfer-encoding: chunked
< 
GET /echo HTTP/1.1
host: localhost
user-agent: curl/7.54.0
accept: */*
x-b3-traceid: 273d1yyyyyyyyyyyyyyy <>{} @#$%^&*()
x-b3-spanid: c115c94374c93923
x-forwarded-proto: https
x-envoy-expected-rq-timeout-ms: 15000
content-length: 0

[mattklein123]

cc @objectiser

[objectiser]

@danielkwinsor Agree, should be treated as if no trace context has been supplied, and start a new trace (with new sampling decision).

Would you be interested in contributing a PR?

[danielkwinsor]

Revisiting this topic in light of log4j2 exploit in the wild -- yes, true, envoy is not vuln to that, but envoy may propagate an invalid x-b3-traceid header that could later be logged by a system that is vuln.