whatwg / fetch

Fetch Standard
https://fetch.spec.whatwg.org/
Other
2.11k stars 328 forks source link

CORS safelisting trace context header #911

Open hmdhk opened 5 years ago

hmdhk commented 5 years ago

Problem statement

W3C trace context defines a format to exchange distributed trace context between different software components. Specifically for HTTP protocol, the specification defines the format for two headers, namely `traceparent` and `tracestate`.

From the trace context specification:

Trace context is split into two individual propagation fields supporting interoperability and vendor-specific extensibility:

For example, a client traced in the congo system adds the following headers to an outbound http request:

traceparent: 00-0af7651916cd43dd8448eb211c80319c-b7ad6b7169203331-01

tracestate: congo=t61rcWkgMzE

Web applications use this context propagation mechanism by adding the traceparent and tracestate headers to outbound HTTP requests. There are already implementations from observability providers, including the Elastic APM Real User Monitoring agent, OpenTelemetry, and Dynatrace’s agents. However, web applications commonly make use of services hosted on different origins than the application itself. It is desirable to include cross-origin HTTP requests in captured traces, meaning that we need to propagate trace context with these requests.

Scenarios where this is necessary:

  1. A developer builds a web application that calls a third party API, and the API is not behaving as expected. They contact the API’s provider and pass along the trace IDs of problematic requests. The third party vendor is able to match these IDs to traces and other telemetry in their systems so that they can more quickly debug the issue.

In order to comply with the Cross Origin Resource Sharing (CORS) protocol browsers need to verify that the server would allow access to the HTTP resource. A concise algorithm, defined by the W3C, is used to determine if the HTTP request requires a preflight (OPTIONS) request which is used to verify the server’s consent for receiving the following HTTP request. One of the criterias for this algorithm is the HTTP headers included in the request. Essentially if one of the request headers is not part of the CORS-safelisted request headers, the preflight request is required. In the case of trace context both `traceparent` and `tracestate` are not part of the CORS-safelist therefore adding these headers to HTTP requests would necessitate a preflight request, this poses two challenges for observability providers:

  1. The preflight request increases the latency for the original HTTP request since there is an extra roundtrip to verify that the application can access the HTTP resource.
  2. Unless the server (managed by observability user) is modified to respond appropriately to the preflight request, the request would fail. This prevents observability providers from propagating the trace context across cross-origin requests by default, therefore relying on the user to modify each of their services.

Solution

I propose to add both `traceparent` and `tracestate` to the CORS-safelisted request headers. By doing so browsers can avoid the preflight request for these headers and therefore allowing observability providers to add these headers by default to all HTTP requests.

The proposed solution is aligned with the CORS protocol since these headers do not require any state change on the server side and are only meant to exchange the trace information.

annevk commented 5 years ago

If I understand it correctly this is a web application level protocol not intended to be implemented by browsers? (Whether it's implemented by browsers does not matter so much for this issue, but in that case I might have some other feedback.)

We don't extend that algorithm generally as it gives attackers more opportunity to attack unsuspecting servers. The solution to the additional roundtrips might be some variant of https://github.com/WICG/origin-policy/, assuming you hit a single origin multiple times.

sideshowbarker commented 5 years ago

I can’t find anywhere in the issue description that provides an explanation of any problem specific/unique to exchanging distributed trace contexts that’s caused by the CORS preflight.

As far as the problems that are cited (in “this poses two challenges for observability providers”):

The preflight request increases the latency for the original HTTP request since there is an extra roundtrip to verify that the application can access the HTTP resource

That’s a general problem/cost with any situation at all where a preflight happens. It’s not on its own sufficient justification at all for special-casing the traceparent and tracestate request headers.

Unless the server (managed by observability user) is modified to respond appropriately to the preflight request, the request would fail. This prevents observability providers from propagating the trace context across cross-origin requests by default, therefore relying on the user to modify each of their services.

That’s also a general requirement with any situation at all where a preflight can happen. It’s also not on its own sufficient justification at all for special-casing traceparent and tracestate.

A server managed by an observability user must anyway at least be modified to send the Access-Control-Allow-Origin response header back, right? Otherwise, no frontend JavaScript code will be able to access responses from that server anyway — even for simple requests that require no preflight.

Given that, in what way is it significantly more difficult for an observability user to modify their server-side system with complete CORS support, including handling for the preflight OPTIONS request?

And doesn’t an observability user already need to modify their server-side system in some way to participate in exchanging (consuming?) of distributed trace contexts to begin with?

If so, why wouldn’t it be expected that when making the necessary modifications to participate in exchanging (consuming?) of distributed trace contexts to begin with, the observability user would at that time also fully CORS-enable their server-side system — including handling for preflights?

What is special/unique about the observability-user case that makes it specifically more challenging for observability users to add CORS-preflight support to their systems than it is for anyone else managing a server-side system intended to receive cross-origin requests from frontend code?

hmdhk commented 5 years ago

Thanks @annevk and @sideshowbarker for the feedback.

If I understand it correctly this is a web application level protocol not intended to be implemented by browsers? (Whether it's implemented by browsers does not matter so much for this issue, but in that case I might have some other feedback.)

The trace context proposal is not intended to be implemented by browsers.

That’s a general problem/cost with any situation at all where a preflight happens. It’s not on its own sufficient justification at all for special-casing the traceparent and tracestate request headers

This is of course the cost of any cross-origin request, but in the case of observability the traceparent header would be added to most (if not all) requests which makes the cost to be amplified specially since the header would be added to requests that otherwise would not need a preflight request. This is specially an issue for an observability tool that is expected not to interfere with the application's performance. Secondly, this cost can be avoid considering that trace context proposal a W3C candidate recommendation proposal.

What is special/unique about the observability-user case that makes it specifically more challenging for observability users to add CORS-preflight support to their systems than it is for anyone else managing a server-side system intended to receive cross-origin requests from frontend code?

The challenge with the observability use-case is that it potentially touches all of the components of an application, some of which might not consume the trace-context header but might include it in their logs or might only pass it along.

I would also like to ask, what are criteria that a header need to meet in order to be added to the CORS safelist? In other words, what are the criteria that the current members of CORS safelist meet that justify their place on the list (e.g Content-Type)?

annevk commented 5 years ago

Exceptions are based on capabilities of existing features, primarily <form>. A couple headers were added out of naivety. Basically extensions of this list should not happen as they put end user security at risk.

dyladan commented 3 years ago

Hello, member of the W3C working group which developed the Trace Context spec here. Maybe I can provide some context.

  1. The spec may indeed be eventually implemented by browsers, but I think it is unlikely in its current form.
  2. The safelist need not include both headers. While we believe tracestate and traceparent are both valuable, it is possible to use traceparent without tracestate header. The traceparent header is a very restrictive format which would be significantly more difficult to abuse and, in my opinion, a safer inclusion into a safelist.
  3. The primary concern is that the Trace Context specification is meant to be included on all outgoing requests, whether the client controls the server or not. This means that requests that would ordinarily not have triggered a preflight request now will. This can cause requests to fail which would not have otherwise failed. There is no way to know in advance which requests these are. If the traceparent is not included on all outgoing requests the primary benefit of the header is lost.
dyladan commented 3 years ago

@annevk I saw that you flagged this as needing implementer interest. In this instance are you asking for browser vendor implementors or server-side or application level implementation? There have been quite a few implementations at the application level on both client and server side, but thus far we haven't engaged with browsers to receive built-in support for the trace context spec.

annevk commented 3 years ago

Implementers of Fetch, with a focus on browsers.