Skipper: Allow using 308 (Permanent Redirect) instead of 301 (Moved Permanently) for HTTP to HTTPS redirects

[krzysztofwos]

A redirect using 301 can have undesirable consequences. For example, some user agents change request method from POST to GET. This can lead to rather confusing responses from API endpoints. A redirect using 308 would behave correctly in those cases.

[szuecs]

How would you like to set this?
Options:

• flag to set it global
• annotation to pick it in your ingress definition

[krzysztofwos]

I think a flag to set it global should be sufficient. Based on what I have read about the differences, it seems like 308 is always a better choice unless you really have to support 301 for legacy reasons.

[szuecs]

@aryszka @lmineiro what do you think should we migrate this to plain 308 or should we add an option?

[lmineiro]

I don't think this is a serious problem. I honestly don't understand how allowing user agents to change method can lead to confusing API responses or how Skipper fits in the middle of that.

This being said, And just focusing on the feature itself, I'd say that allowing users to customize which status code to send to clients for the HTTPS redirector give users the flexibility.

On shared environments like Kubernetes Ingress, this may not be that straightforward as the listener that handles this redirect exists outside of the proxy flow.

[lmineiro]

I just went through https://tools.ietf.org/html/rfc7538

I actually don't think HTTP to HTTPS redirector should interfere, at all, with the ability to change the method. This is not its purpose.

If an HTTP request is sent to Skipper when the redirector is enabled no API backend will ever receive that request, regardless of the method of the request. The redirect instructs the user-agent to send the same request using HTTPS. If the user-agent decides to switch from one method to another, this will be the only method the backend API will receive.

@krzysztofwos can you elaborate further on a couple of examples where the HTTP to HTTPS redirector could prevent undesired behavior?

[aryszka]

i would implement this as a global setting.

[mykolasmith]

@lmineiro I can attempt to elaborate on the problem. If I issue a POST request to Skipper using HTTP, it attempts to redirect using a 301 to HTTPS. The 301 status code, however, automatically redirects all requests to GET methods, so your POST body and original method are lost. Here is the curl output which demonstrates this:

```* Connected to (34.236.40.138) port 80 (#0)

POST / HTTP/1.1
Host:
User-Agent: curl/7.54.0
Accept: /
Content-Type: application/json
Content-Length: 132

• upload completely sent off: 132 out of 132 bytes
  < HTTP/1.1 301 Moved Permanently
  < Date: Wed, 20 Jun 2018 14:36:09 GMT
  < Content-Length: 0
  < Connection: keep-alive
  < Location: https://
  < Server: Skipper
  <
• Connection #0 to host left intact
• Issue another request to this URL: 'https:///'
• Switch from POST to GET
• Trying ...
• TCP_NODELAY set
• Connected to () port 443 (#1)
  ```

The HTTP spec defines the following behavior:

| | Permanent | Temporary |
|-|-|-|
|Switch to GET|301|302/303|
|Keep Original Method|308|307|

So, as you can see, any client attempting to POST to an HTTP endpoint at Skipper does not correctly redirect to a POST at the HTTPS endpoint (even if using follow redirects e.g. curl -L)

In fact, I would think that the correct behavior would actually be to default to 308 and give the ability to override to 301. HTTP 308 is a newer standard, but it seems more correct (e.g. 308 Permanent Redirect vs 301 Moved Permanently).

See here: https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/308

The request method and the body will not be altered, whereas 301 may incorrectly sometimes be changed to a GET method.

[lmineiro]

Thank you @mykolasmith. I have a better understanding of the consequences now.

I am still a bit divided on this one.

Thinking beyond the specs and which one would be a better default, I'm left thinking about the efficiency of the clients that would keep using HTTP instead of actually changing the endpoint to HTTPS. This can have the side effect of allowing a more lazy behavior of clients because it will work, although less efficient.

On the other hand, I admit that the redirector was created mainly because the ELBs didn't provide such a feature and we felt that skipper could help users by redirecting them to HTTP. If memory doesn't fail, this was mostly targetted at interactive websites, not programmed clients.

API clients should be configured and use whatever they document is supported. If the endpoint is HTTPS (which I'd expect to be the case), I still think that HTTP should not even work at all.

Skipper doesn't really care or knows if what'll be sitting behind it are interactive websites or APIs. This seems to be the problem here. The expectation is set that it will redirect all clients, programmatic or otherwise, to HTTPS. According to this issue, there is an expectation that it should redirect and allow clients to carry on with the originally intended operation, even if wrongly configured to use HTTP.

With all this being said, It's probably better to change the behavior to use 308 as it will be more useful. The efficiency issue would be the price to pay.

[szuecs]

Then let’s change it to 308 instead of 301. Anyone against this decision?

[MarcusNoble]

I know this is an old / closed issue now but I just wanted to leave a comment for anyone in the future that gets here for the same reason I did (and possibly re-open the discussion about having it configurable).

Internet Explorer 11 on Windows 7 and 8.1 doe not support a 308 redirect. IE11 on Windows 10 does support it but on the older OS it just hangs and shows a white page.

Interestingly, Cloudflare also follows this same behaviour so I'm not sure how much of an issue it actually is in practice. I guess it would depends on your user base.

[szuecs]

@MarcusNoble thanks for the hint, good to know!
Please create a new issue if you think it should be fixed (where should it be configurable cli option or ingress annotation or have both and ingress overrides cli option)?.

[MarcusNoble]

I have no strong opinions about it either way. I can open a new issue to discuss it if you feel it's worth it (others may have more of a need for it than I do)?

[szuecs]

Yes please create the issue, we do not have to fix it now, but we should not loose the need, if someone want to have a simple PR to the project it can be a good one.