linkerd / linkerd2

Ultralight, security-first service mesh for Kubernetes. Main repo for Linkerd 2.x.
https://linkerd.io
Apache License 2.0
10.69k stars 1.28k forks source link

HTTP Header casing is modified #3964

Open javaes opened 4 years ago

javaes commented 4 years ago

Bug Report

What is the issue?

When doing an HTTP call to my service, the headers of my HTTP requests are modified. All header keys are changed to lower case.

Even though the HTTP standard says that headers are not case sensitive, this can be implemented differently in some servers. Using linkerd makes it impossible to integrate with these services which depend on proper header key casing.

How can it be reproduced?

Deploy two applications with linkerd proxies enabled. Application A is calling application B with an HTTP request which contains headers which have upper case fields. When inspecting the incoming request in application B, you will see that all header keys are lower cased now.

When calling Application B directly without passing through the linkerd proxy the logs still show the original headers.

Environment

Possible solution

ihcsim commented 4 years ago

Relevant https://github.com/linkerd/linkerd2/issues/2878.

javaes commented 4 years ago

Sorry for duplicating the linked issue. Even though it's closed because of being RFC compliant, I think this is still a relevant issue because this behavior is not expected by proxy users and it basically breaks our integration with 3rd party APIs which we can't change. Please consider re-opening the original issue.

christianhuening commented 4 years ago

@javaes Can you say how the expected behaviour on the 3rd party side is? Do they expect Title-Case or any other format which can be baked into a transformational rule?

javaes commented 4 years ago

@christianhuening the 3rd party where I've encountered this issue is expecting title cased header keys, but I think this would still be a bold assumption to make for more 3rd party APIs since the project is about integrating a lot of APIs.

JacobHenner commented 2 years ago

I'd be willing to investigate what would be required to address this, but I want to first confirm that the proposal(s) would not be dismissed for the reason mentioned in https://github.com/linkerd/linkerd2/issues/2878#issuecomment-515135376:

in this case we are RFC compliant and changing this behavior would impact proxy performance

olix0r commented 2 years ago

@JacobHenner it won't be rejected out of hand, but I also view it as fairly low priority and won't be able to devote much time (in the short-term, at least) to getting these changes vetted: this change only would only exist to satisfy HTTP endpoints that don't actually honor the HTTP RFCs. That's a losing battle, in my opinion.

Furthermore, because Linkerd transports in-mesh HTTP traffic over HTTP/2, it's going to be pretty complex to preserve header casing (since HTTP/2 is explicitly case-agnostic) for in-mesh traffic.

I know there's been some discussion in the hyper & http crates, so it may be more feasible than the last time we looked, but I worry about the complexity of this change.

If you're willing to do the work to come with a proposal of what this change would actually entail (from a code point of view), the benchmarking to compare behavior, etc, we can probably help/guide. But I can't promise that we'll want to take the change if we feel the complexity/maintenance burden outweighs the benefits.

olix0r commented 2 years ago

Maybe a better question to come back to: what do you actually want from Linkerd in these cases--mTLS? load balancing? metrics? Would it be suitable to have Linkerd handle this communication opaquely (as TCP) so it doesn't even parse the HTTP headers?

JacobHenner commented 2 years ago

@JacobHenner it won't be rejected out of hand, but I also view it as fairly low priority and won't be able to devote much time (in the short-term, at least) to getting these changes vetted: this change only would only exist to satisfy HTTP endpoints that don't actually honor the HTTP RFCs. That's a losing battle, in my opinion.

Understandable, thanks for clarifying. In that case I'll first focus on the other options we have available, and revisit this if it becomes strictly necessary.

Maybe a better question to come back to: what do you actually want from Linkerd in these cases--mTLS? load balancing? metrics?

mTLS at minimum, but preferably the maximum feature set possible for each type of destination.

Would it be suitable to have Linkerd handle this communication opaquely (as TCP) so it doesn't even parse the HTTP headers?

In some cases this would suffice (e.g. mTLS is the only required feature), and in others this would be less-than-desirable (e.g. when devs expect the more advanced features to function).

That said, my biggest concern is that devs might not realize that their application(s) depend on non-RFC-conforming case-sensitive HTTP headers, so they likely won't know to mark ports opaque until the unexpected header casing change causes a service disruption that's investigated and debugged. This complicates rolling out linkerd, as enabling proxy injection at the namespace level (without affirmative consent from each application owner) is potentially a more dangerous operation.

olix0r commented 2 years ago

@JacobHenner That's helpful context. It's unfortunate that this feature is basically at odds with request multiplexing over HTTP/2. We could potentially support a mode where headers are forcibly transformed to Title-Case instead of lower-case.

Otherwise, I think you'd have to disable request multiplexing for HTTP/1. If someone has time to see what it takes to have the proxy (configure Hyper to) preserve the original case, we could entertain that change.