Allow forcing X-Forwarded-Proto Header

[timmow]

There are situations where it's not possible to determine the
appropriate value for this header from the incoming connection (for
example when running behind an Amazon ELB in TCP mode where SSL
termination is handled by the ELB). This therefore adds a config option
to allow specifying a value that should be unconditionally added to all
downstream requests.

[cf-gitbot]

We have created an issue in Pivotal Tracker to manage this. You can view the current status of your issue at: https://www.pivotaltracker.com/story/show/117064881.

[shalako]

Hello @timmow

Thank you for this contribution. In the future please consider submitting an issue with a proposal first, as acceptance isn't a guarantee, and it would be a shame to have put in such effort for naught.

Could you please clarify your use case? I'm surprised that an ELB in "TCP Mode" can also terminate SSL. I would expect it to pass through the TCP request to the Gorouter without terminating, and Gorouter to terminate SSL.

If X-Forwarded-Proto is not present in an inbound request, Gorouter will currently append this header in requests to backends, using the scheme of the inbound request. Are you suggesting there are scenarios where Gorouter cannot determine the scheme of the HTTP request?

[Jonty]

@shalako An example setup where gorouter will incorrectly set the X-Forwarded-Proto is as follows, and will be common in any situation where SSL has been offloaded from gorouter due to performance:

1. Use an external TCP SSL terminator such as hitch
2. Communication between the terminator and gorouter is unencrypted as the encryption was already terminated
3. gorouter receives the decrypted communication on port 80
4. gorouter will not see the connection as encrypted, and will set the X-Forwarded-Proto as http, despite the user connection having been https

In this case it is desired that the operator be able to override the X-Forwarded-Proto as deployed applications that expect https will commonly attempt to redirect the user to a secured connection, which results in an infinite redirect as the connection already is secure.

It is also common that people push their SSL certificates to termination appliances in order to limit access to the certificate keys, which leads to the same situation.

[shalako]

Doesn't this solution open up a security vulnerability? If we tell UAA that a connection was secure when it is not you enable man-in-the-middle attacks by passing OAuth tokens over unencrypted connections. I imagine the same is true for applications running on CF.

Infinite loops and sharing unenencrypted sensitive data both sound bad.

Maybe the answer is to use HTTP SSL terminators that can accurately forward the scheme of the client request? Or terminate SSL at gorouter?

We receive a lot of input that operators want to secure every leg of the request to the application. Your feedback that the private network can be trusted in the interest of performance is valuable feedback.

[fhanik]

Use an external TCP SSL terminator such as hitch
Communication between the terminator and gorouter is unencrypted as the encryption was already terminated
gorouter receives the decrypted communication on port 80
gorouter will not see the connection as encrypted, and will set the X-Forwarded-Proto as http, despite the user connection having been https

In this scenario, the hitch should be forwarding the X-Forwarded-Proto, and the IP address of hitch should be configured on the GoRouter as trusted .

All headers from a trusted proxy, will be forwarded downstream

There doesn't seem to be a need to "force the X-Forwarded-Proto" value, but rather the ability to configure trusted proxies.

[shalako]

@fhanik Thank you for jumping in to represent UAA needs.

I think @Jonty may be implying that because their SSL terminator doesn't support HTTP they can't append the proto header. Is that correct?

[Jonty]

@shalako Correct, the SSL terminator (hitch being an example) is not an HTTP proxy and thus cannot inject HTTP headers.

This would only open up a security vulnerability if people allow unencrypted external access to their platform, which is not the situation it is intended to be used in.

As I also pointed out, terminating SSL at gorouter is not always desired both from a key management point of view, and the performance impact.

[keymon]

Hi. I work in the same team than Tim and Jonty.

I wanted to ask if there any update about the decision taken for this PR?

If I have to give my feedback: The option to force the X-Forwarded-Proto Header is quite explicit and optional. An operator who enables this option is obviously aware of what is doing, and we can document it properly in the corresponding bosh release. The use case is clear: allow do SSL termination with a TCP-SSL proxy which cannot inject the HTTPS header.

If there are more concerns or feedback please add more comments. If you prefer to prioritise the approach described in #126, let us know.

[dcarley]

I've rebased this PR against master to resolve merge conflicts. Happily, the Travis build now passes too.

[shalako]

Prioritizing for engineering review.

[dcarley]

Prioritizing for engineering review.

Thanks. I've rebased again, to make the review easier.

[shashwathi]

Hi @keymon @dcarley,

We agree that there is need for more documentation on what condition this property should be used. Does your team feel like adding doc changes to the existing PR or doing a separate PR for README changes?

Regards
Shash && @swetharepakula

[shalako]

@dcarley @keymon @Jonty

Can you imagine a use case for forcing the value of this header to http? I can't. For this reason, we're considering exposing this configuration with the manifest property router.force_forwarded_proto_https. When true, the router would always send X-Forwarded-Proto: https; when false, the router would choose per current behavior.

Does this sound reasonable? It seems like a simpler user experience to choose a boolean than explicitly set the value, and it is consistent with another configuration property, router.route_services_recommend_https

[Jonty]

@shalako The only convoluted scenario I can envision is one where externally a CF app is accessed over HTTP (say, in a corporate network that insists on logging proxies) but the deployment itself uses SSL for all internal links. That is clearly absurd though, so I don't think it's worth considering.

I think your proposal of using router.force_forwarded_proto_https is excellent, especially as this pattern is already used for the route services configuration.

[Jonty]

@shalako I have updated the PR to use the router.force_forwarded_proto_https configuration, rebased it against master, and added documentation as requested by @shashwathi.

[shashwathi]

@Jonty

Thank you for adding the documentation to the PR.

Regards
Shash