caddyhttp: Redirect HTTP requests on the HTTPS port to https://

[francislavoie]

This is very much not done yet, but I just wanted to show the proof of concept.

---

Background: in certain cases, users would like HTTP->HTTPS redirects in situations where they only actually serve HTTPS on one port; if an HTTP request is made to that port, Caddy currently responds with Client sent an HTTP request to an HTTPS server. This response comes directly from Go's stdlib (added in https://go-review.googlesource.com/c/go/+/143177/, see golang/go#23689 (comment) for context):

https://github.com/golang/go/blob/56c3856d529d72e280ad6b185f7927657de86c37/src/net/http/server.go#L1817-1825

While it could be argued that HTTP responses should never be sent on the HTTPS port... Go already does it.

This is alright, but Caddy could go a step further and do an HTTP->HTTPS redirect, which would make HTTP clients happier, skipping the error and instead going straight to HTTPS. This can help avoid confusion for users making mistakes, etc.

---

So after hacking on this a bit, the idea I came up with is to make tlsPlaceholderWrapper a bit more useful by peeking the first bytes of the connection. Turns out that if we just call Peek() on the wrapped request, then the stdlib TLS listener will return an error tls.RecordHeaderError that we can watch for, which is the same one that gets handled for writing the Client sent an HTTP request to an HTTPS server. message. We can do the same check on the first 5 bytes to see if it looks like HTTP, and write our own response instead.

Note that tlsPlaceholderWrapper is now always registered, and is no longer skipped when performing the wrapping. If I didn't make this change, then the logic in the wrapper would never get called.

We'll probably want to rename tlsPlaceholderWrapper to something more meaningful now that it has a secondary purpose 🤷‍♂️

Still TODO, I need to figure out how to get the wrapper to read a bit more of the request in the error case to grab the Host header to help us perform the redirect. I have a hunch that might not be possible with this current approach, because any attempts to read would trigger the same error since the stdlib TLS listener is still under ours. I'll have to dig into this more another day.

[francislavoie]

Caddyfile I'm using to test this:

{
        http_port 8880
        https_port 8881
        admin off
        debug
}

localhost {
        respond "HTTPS"
}

Then making these requests:

$ curl -vk https://localhost:8881
$ curl -vkL http://localhost:8881

First should just show HTTPS with no redirect, second should first hit the error case then get served a redirect, then see HTTPS.

[yroc92]

For what its worth, I'm running into this issue that you're solving and look forward to your fix! I have just one port (8844) for a domain being served with TLS in my Caddyfile. When accessing it with http://example.com:8844 I get back Client sent an HTTP request to an HTTPS server..

I attempted using redir when the protocol is http but the error is returned before it reaches the directive as you've described.

[francislavoie]

One of the open questions I had was how to parse the request headers, and I have my answer by having seen the code in

caddy/modules/caddyhttp/reverseproxy/fastcgi/client.go

Line 420 in 24fda75

mimeHeader, err := tp.ReadMIMEHeader()

, i.e. textproto.NewReader(rb).ReadMIMEHeader(). That'll let us grab the Host header from the request.

Still need to figure out how to properly wrap the listener so we can actually grab the bytes when we want to without getting an error from the underlying TLS listener.

Edit: The above is dumb lol, I can just use http.ReadRequest 🤣 silly me.

[francislavoie]

I figured out how to do this properly!!

I scrapped the idea of overloading the tlsPlaceholderWrapper. Idk what I was thinking, I just didn't understand the pipeline, really.

Instead, I created a new listener httpRedirectListener which checks the first 5 bytes of the request (which is fast) and returns early if it doesn't look like HTTP. If it does look like HTTP though, it will parse the request manually from the bytes, grab the Host and URL, then assemble a HTTP->HTTPS redirect response, write it out, then close the connection.

I always wrap the listener with httpRedirectListener just before creating the TLS listener, so that we always have that check in place.

Steps I took to test this are essentially the same. Caddyfile:

{
    http_port 8880
    https_port 8881
    admin off
    debug
}

localhost {
    respond "HTTPS"
}

Curl request which should show that it redirected to HTTPS, with the header Location: https://localhost:8881/

$ curl -vkL http://localhost:8881

Notice that the debug logs will contain the following on any of these kinds of requests:

2021/11/06 04:06:35.627 DEBUG   http.stdlib     http: TLS handshake error from [::1]:49510: redirected HTTP request on HTTPS port

This message can probably be adjusted, but I return this error message because otherwise the TLS listener will try to read from the connection which is already closed. I don't think it's possible to quiet it, but either way, no biggie.

[mholt]

Thanks for this PR, looks interesting. I'm hoping to dive into this in more detail soon when I can give it some thought. My gut tells me this is something we have to be careful with, or that there will be some unintended side effects or something.

[mholt]

Very nice overall. However, does this actually work? Like, why would a client make an HTTPS request to the same port it's already connecting to if it tried HTTP to it in the first place?

[francislavoie]

This is typically helpful for situations where a different port than 443 is being used for HTTPS. Such as 8443, maybe. So if a link such as example.com:8443 is given and clicked, it would typically default to http:// by the browser. Then the user would see an error because they made an HTTP request to an HTTPS server. Instead, we can redirect to https:// to make sure the browser actually makes a valid request.

And yes, this definitely does work, try it out as described above, both with curl and a browser (before and after the patch)

[mholt]

Ok ok, last pass I think :)

[mholt]

LGTM now. Thanks!