`--http.redirect-to-tls` option always redirects to port 443

[neoaggelos]

Summary

Using stack with --http.redirect-to-tls always redirects traffic to port 443.

Steps to Reproduce

1. ttn-lw-stack start --http.redirect-to-tls
2. curl https://localhost:8885

Several parts come into play here:

1. https://github.com/TheThingsNetwork/lorawan-stack/blob/v3.9/pkg/web/web.go#L198
2. https://github.com/TheThingsNetwork/lorawan-stack/blob/v3.9/pkg/webmiddleware/redirect.go#L65
3. https://github.com/TheThingsNetwork/lorawan-stack/blob/v3.9/pkg/component/web.go#L61

In (1), the value from the map is always used, which is 0 for non-existing entries. In (2), when the port number is 0, then the hostname is returned without any port, which causes the redirect to the default HTTPS port instead (443).

What do you see now?

$ curl https://localhost:8885 -k
<a href="https://localhost/">Found</a>.

$ curl https://localhost:8885 -k -L
curl: (7) Failed to connect to localhost port 443: Connection refused

$ http://localhost:1885 -k
<a href="https://localhost:8885/">Found</a>.

$ curl http://localhost:1885 -k -L
curl: (7) Failed to connect to localhost port 443: Connection refused

What do you want to see instead?

Redirect to the correct HTTPS port.

Environment

3.9.1

How do you propose to implement this?

This needs delicate handling in order to avoid breaking existing installations. We cannot simply redirect to the HTTPS port all the time, since that will break Docker installations (where stack is listening on container port :8885 but traffic is received/sent over the port :443.

Overall, this is a bug, but we cannot simply fix it, because it could end up breaking existing deployments (e.g. by suddenly redirecting traffic to port 8885). I think the cleanest solution to this would be to add a --http.redirect-to-tls-port=<integer>, defaulting to 443. This is backwards compatible with existing configs, and can be used to always redirect to the correct HTTPS port.

How do you propose to test this?

Test locally and in a Docker deployment.

Can you do this yourself and submit a Pull Request?

yes, but let's discuss before diving into this.

[neoaggelos]

cc @htdvisser

[htdvisser]

Sounds good to me, but let's call it http.redirect-to-port (without the tls in the middle)

[neoaggelos]

Sounds good to me, but let's call it http.redirect-to-port (without the tls in the middle)

For the record, my suggestion is to use this flag only when http.redirect-to-tls is enabled (again; to avoid breaking existing configs for no reason), which is why including tls there might be warranted.

[htdvisser]

Why would this break existing configs? The default is to not enable this, and it only needs to be enabled (added) to the example docker-compose.

[htdvisser]

The problem here is not actually The Things Stack doing something wrong, but instead the fact that we map the ports in Docker-Compose.

Perhaps we should instead expose 80/443 in both docker-compose and The Things Stack config, and stop mapping 80-1885 and 443-8885? If you agree, please close this issue and create a new one for doing that.

[neoaggelos]

The problem here is not actually The Things Stack doing something wrong ...

Does it not? The redirect middleware keeps redirecting to port 443, regardless of the configured port (because it always follows the map from https://github.com/TheThingsNetwork/lorawan-stack/blob/v3.9/pkg/web/web.go#L198 (and that value is 0 for the configured HTTPS port). Perhaps a solution would be to add an extra redirect port (e.g. from 8885 to 8885). Also see the curl examples in the description, am I missing something?

I do not really like changing the default ports where the stack binds. Not only to avoid breaking any configs (whether they are covered by compatibility requirements or not), but also because ports < 1024 would require root permissions, and the Docker user (with uid 886) cannot do that.