http: envoy rejects valid referer

[kyessenov]

Per HTTP semantics (https://httpwg.org/http-core/draft-ietf-httpbis-semantics-latest.html#field.referer), partial URI is a valid value for "referer" header. An example: Referer: /abc/com. Envoy currently always removes this header since it deems it invalid, without any way to disable the sanitization.

Envoy should not be erasing valid header values.

cc @RenjieTang

[kyessenov]

@alyssawilk @yanavlasov @mattklein123

I know this is an old issue, but I think we might be unnecessarily strict here, so I want to revisit this issue. I don't think Envoy needs to enforce RFC unless it's trying to use the header in some ways. Envoy as a proxy never parses the content of referer (it exposes it by-value to authz and CEL, however). So why is it spending cycles (incorrectly) validating it?

Can we disable sanitization for referer and keep it private to envoy-mobile?

[RenjieTang]

@RyanTheOptimist

I think we can probably remove this logic in Envoy-mobile as well?
Looking back at the original Cronet issue, referers were not passed in the URL requests.
The whole sanitization thing came as a byproduct of initializing and setting referer through GURL.

But Envoy-mobile is already propagating referer set by the application. Maybe we should leave the responsibility to the application and avoid the extra sanitization cost in Envoy?

[RyanTheOptimist]

I think that in general we care about being standards compliant so as to avoid issues where non-standards-compliant behavior causes problems. As @kyessenov points out, Envoy is incorrectly strict in this case, and so I think we should fix this (to allow both absolute and partial URIs in referrer). But I don't think we should leave the referrer completely unvalidated, as a matter of general principle. @yanavlasov What's your take on this in the context of UHV?

I think Envoy Mobile, like Envoy, should make sure that it sends standards-compliant traffic and so I'd be supportive of continuing to rely on Envoy's sanitization of this header.

[kyessenov]

@RyanTheOptimist The critical point is non-standards-compliant behavior causes problems. Referer is not used by envoy AFAICT. Changing the header does cause application problems.

[RyanTheOptimist]

Any time we validate any field we run the risk of breaking some application that is sending non-standards compliant traffic. That's true. But that's true for all fields. We regularly get papers where various servers or proxies handle different HTTP edge cases differently. Taking a proxy with non-compliant behavior X and putting it in from of a server with non-compliant behavior Y can led to surprising behavior. Is it possible that there is no server that would do anything wrong if Envoy didn't validate referrer? Maybe. But that doesn't seem like a strong enough argument to justify sending non-compliant traffic. I'm happy to hear what others think.

[kyessenov]

@RyanTheOptimist It depends on the situation. In Envoy as a sidecar, we generally can trust upstreams and downstreams to cooperate and our goal is to minimize the impact of the sidecar without a good reason. For front LBs, it makes a lot of sense to only emit sanitized headers.

It's genuinely surprising that only "referer" is sanitized. If we add a knob to control which RFCs to enforce, where would that knob live and how fine grained do we need it?

[RyanTheOptimist]

I don't think we only sanitize the referrer header. We do lots of different sanitization at different places in the code. Some of it is in quiche, some is in nghttp2, etc. The UHV project aims to unify this validation:

https://docs.google.com/document/d/1iRprAqZt3dek107LZWtBTnb2YRRhUBanBnHvHKMPdkI/edit#heading=h.xgjl2srtytjt
#20261

[kyessenov]

@briansonnenberg is going to look into fixing the validation to the spec I believe