No Longer Able To Duplicate The Value of the X-B3-Traceid Header using request_headers_to_add

[jacobneiltaylor]

If you are reporting any crash or any potential security issue, do not
open an issue in this repo. Please report the issue via emailing
envoy-security@googlegroups.com where the issue will be triaged appropriately.

Title: No Longer Able To Duplicate The Value of the X-B3-Traceid Header using request_headers_to_add

Description:
Under previously released versions of Envoy (<1.22) we were able to duplicate the value of the X-B3-Traceid header to another upstream header that we use internally. This is to support customers who still expect a custom header.

We achieved this using header formatting in the request_headers_to_add configuration block, like so:

  routes:
    - match: 
        prefix: "/"
      route: { cluster: app }
      request_headers_to_add:
        - header:
            key: Legacy-Trace-Header
            value: "%REQ(x-b3-traceid)%"
          append: false

However, since 1.22, this behaviour has changed.

Now, there are two separate outcomes depending on the presence of the x-b3-traceid header in the downstream request:

• If the header is not sent, the custom header is missing entirely
• If the header is sent, the header is passed to the cluster with no modification.

Repro steps:

1. Clone my demonstration repo.
2. Run docker compose up
3. Load the following links up in your browser, corresponding to different Envoy versions. Notice how, on 1.22 and up, the Example-Traceid header is absent in the echoed response for all versions > 1.21.
   • v1.19.0
   • v1.20.0
   • v1.21.0
   • v1.22.0
   • v1.23.0
   • v1.24.0
4. Send a crafted response to http://localhost:1210 with the header x-b3-traceid set to foobar. Notice that the Example-Traceid header value seen in the JSON response is overwritten by the trace ID generated by Envoy.
5. Send a crafted response to http://localhost:1220 with the header x-b3-traceid set to foobar. Notice that the Example-Traceid header value seen in the JSON response is not overwritten by the trace ID generated by Envoy.

If this is not the expected way to achieve this, then we'll happily use an alternative configuration pattern to achieve the same result elsewhere.

Note: The Envoy_collect tool
gathers a tarball with debug logs, config and the following admin
endpoints: /stats, /clusters and /server_info. Please note if there are
privacy concerns, sanitize the data prior to sharing the tarball/pasting.

Admin and Stats Output:

Include the admin output for the following endpoints: /stats,
/clusters, /routes, /server_info. For more information, refer to the
admin endpoint documentation.

Note: If there are privacy concerns, sanitize the data prior to
sharing.

Config: Available here

Logs:

Include the access logs and the Envoy logs.

Note: If there are privacy concerns, sanitize the data prior to
sharing.

Call Stack:

If the Envoy binary is crashing, a call stack is required.
Please refer to the Bazel Stack trace documentation.

[zuercher]

Hm. I think this is probably the likely culprit: #20503. The request_headers_to_add operations are performed in that call to finalizeRequestHeaders and the injection of the tracing span's context into the request headers no longer happens before that point. That explains why the tracing headers make it upstream but your copy to an alternate name fails. cc @wbpcode

[wbpcode]

Yeah, it's my mistake. It' an unexpected behavior change. It's easy to revert the behavior but there may be repeated injectings.

[wbpcode]

/assign I will try to make a fix.

[repokitteh-read-only]

🙀 Error while processing event:

evaluation error
error: function _rk_error error: path contains forbidden characters:
 Traceback (most recent call last):
  bootstrap:143: in <toplevel>
  bootstrap:135: in _main
  bootstrap:62: in _call
  bootstrap:15: in _call1
  github.com/repokitteh/modules/assign.star:18: in _assign
  github:395: in issue_check_assignee
  github:131: in call
Error: function _rk_error error: path contains forbidden characters

🐱

Caused by: a #23972 (comment) was created by @wbpcode.

see: more, trace.

[github-actions]

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

[kyessenov]

Our users hit this issue as well. I'm thinking that (1) it's not safe to assume the order of header insertion in HCM, (2) trace ID is actually available on downstream but it's not exposed to the formatters.

[wbpcode]

Our users hit this issue as well. I'm thinking that (1) it's not safe to assume the order of header insertion in HCM, (2) trace ID is actually available on downstream but it's not exposed to the formatters.

I am thinking should we create a header mutation filter that could be configured as downstream http filter or upstream http filter. Then we can control the order of header insertion/mutation completely? WDYT cc @kyessenov

[kyessenov]

I like the idea of breaking apart the HCM features with dedicated extensions. Might be complicated because these header operations nest and inherit between routes, hosts, and actions.

[wbpcode]

I want to give it a try. May be we can make it a alternative of current headers_to_add. Anyway, a http filter will not make thing worse.

[kyessenov]

There is a valid point about moving some of the HCM features to upstream HTTP filters because then they could be "phased" after the downstream processing, and apply to non HTTP downstream (e.g. TCP). Before you make a PR, I think this might be worth discussing with @alyssawilk and others.

[github-actions]

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.