linkerd2: Host header field in outbound HTTP requests is wrong in some cases

At least in the cases where we use SO_ORIGINAL_DST to determine the destination IP address, the Host header field is set to the SO_ORIGINAL_DST IP address. Instead the Host header field (and :authority) should not change in these cases. In particular, if there wasn’t a Host header field before, none should be added, and if there was a host header field then it should be preserved.

AFAICT, the only time we should change the Host header field is when we manually override the Host in FullyQualifiedAuthority::normalize().

/cc @seanmonstar @hawkw

About this issue

  • Original URL
  • State: closed
  • Created 6 years ago
  • Comments: 19 (19 by maintainers)

Commits related to this issue

Most upvoted comments

We can guess that the request was meant for a proxy if it has an absolute-form target in HTTP/1, but it’s only a guess! It’s not illegal at all to send an absolute-form target directly to the end server. And for HTTP/2, requests always has the :authority, so we can’t really even guess there.

However, the definition of an HTTP proxy is one in which the client specifically CONNECTs to. Since the client never CONNECTs to Conduit, it isn’t an HTTP proxy. therefore, when Conduit receives a GET request like the one you quoted, it shouldn’t apply the rule for HTTP proxies rewriting the Host header.

Reference?

Sorry, I was sloppy. What I meant is better described in the RFC text I quoted above:

An “interception proxy” [RFC3040] (also commonly known as a “transparent proxy” [RFC1919] or “captive portal”) differs from an HTTP proxy because it is not selected by the client. Instead, an interception proxy filters or redirects outgoing TCP port 80 packets (and occasionally other common port traffic).

In our case, the client application is totally unaware of Conduit, so Conduit can’t be acting as a HTTP proxy for it. In particular, as you mentioned in the Gitter discussion, the application that Conduit is proxying may be intending to communicate with an HTTP proxy like this:

Application <-> Conduit <-> Proxy

Therefore, we have to be really careful that we don’t do “proxy stuff” that the application is expecting that proxy to do.

More generally, it goes back to the transparency idea: Unless we’ve explicitly chosen for the proxy to do some non-transparent behavior, all the behavior of the proxy should be transparent by default.

AFAICT, the only time we should change the Host header field is when we manually override the Host in FullyQualifiedAuthority::normalize().

In fact, I think we shouldn’t even change the Host header field in that case, as of now. When we implement TLS then we may need to have a way to do this Host header field override only for the case where we’re using the Conduit TLS infrastructure (i.e. Conduit-to-Conduit transport). Otherwise I think we should not touch it.

Also, I say “Host header field” everywhere here, but the same thinking applies to :authority for HTTP/2.