New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

httpserver/https.go: redirPlaintextHost does not properly handle -https-port flag #2422

Closed
whitestrake opened this Issue Jan 11, 2019 · 2 comments

Comments

2 participants
@whitestrake
Copy link
Contributor

whitestrake commented Jan 11, 2019

1. What version of Caddy are you using (caddy -version)?

Caddy 0.11.1 (non-commercial use only)

2. What are you trying to do?

  1. Enable Automatic HTTPS with DNS validation
  2. Set a non-standard default HTTPS port via -https-port flag
  3. Have the HTTP listener issue a Location header with the correct HTTPS port

3. What is your entire Caddyfile?

foo.whitestrake.net {
  tls {
    dns cloudflare
  }
  status 200 /
}

4. How did you run Caddy (give the full command and describe the execution environment)?

whitestrake at apollo in ~/Projects/test
❯ env CLOUDFLARE_EMAIL=[snip] CLOUDFLARE_API_KEY=[snip] caddy -http-port 8080 --https-port 8443
Activating privacy features... done.
https://foo.whitestrake.net:8443
http://foo.whitestrake.net:8080

5. Please paste any relevant HTTP request(s) here.

whitestrake at apollo in ~
❯ curl -I http://foo.whitestrake.net:8080/ --resolve foo.whitestrake.net:8080:127.0.0.1
HTTP/1.1 301 Moved Permanently
Connection: close
Content-Type: text/html; charset=utf-8
Location: https://foo.whitestrake.net/
Server: Caddy
Date: Fri, 11 Jan 2019 03:27:52 GMT

6. What did you expect to see?

Location: https://foo.whitestrake.net:8443/

7. What did you see instead (give full error messages and/or log)?

Location: https://foo.whitestrake.net/

8. How can someone who is starting from scratch reproduce the bug as minimally as possible?

Minimal configuration as above.


The error seems to be the method redirPlaintextHost uses to check whether it should exclude an explicit port from the Location value.

It currently checks whether the site's configured port matches the HTTPSPort; if they're the same, it does not include port information in the Location:

if redirPort == HTTPSPort {
// By default, HTTPSPort should be DefaultHTTPSPort,
// which of course doesn't need to be explicitly stated
// in the Location header. Even if HTTPSPort is changed
// so that it is no longer DefaultHTTPSPort, we shouldn't
// append it to the URL in the Location because changing
// the HTTPS port is assumed to be an internal-only change
// (in other words, we assume port forwarding is going on);
// but redirects go back to a presumably-external client.
// (If redirect clients are also internal, that is more
// advanced, and the user should configure HTTP->HTTPS
// redirects themselves.)
redirPort = ""
}

It seems like this check makes the assumption that HTTPSPort is the default HTTPS port, 443. This is not the case when the user launches Caddy with the -https-port flag, which we can see sets HTTPSPort directly:

flag.StringVar(&HTTPSPort, "https-port", HTTPSPort, "Default port to use for HTTPS")

A simple fix would be to modify redirPlaintextHost so that it checks against the value of DefaultHTTPSPort instead of HTTPSPort, so that when the latter is modified, it will actually include this non-standard port information in the Location header.

DefaultHTTPSPort = "443"


This problem was encountered by Cameron (user 10days) on the Community forums:

https://caddy.community/t/caddy-on-seedbox-slot-as-reverse-proxy-using-cloudflare-dns/5064/3?u=whitestrake

@whitestrake

This comment has been minimized.

Copy link
Contributor

whitestrake commented Jan 11, 2019

So I guess the core of this issue is really a question of whether we want to tell people, "only use -https-port for internal stuff and set your ports explicitly for external stuff".

For people like Cameron, who can't bind 80 and 443 on their hosts but want to use Caddy to serve their websites externally, the -http-port and -https-port flags are very convenient.

@mholt

This comment has been minimized.

Copy link
Owner

mholt commented Jan 11, 2019

I think the comment:

// (If redirect clients are also internal, that is more 
// advanced, and the user should configure HTTP->HTTPS 
// redirects themselves.) 

Applies equally to the inverse case (his), where the HTTPS port being changed externally is also an advanced use case, where the redirect should simply be configured by the user.

Caddy makes the (more common) case of needing to serve HTTPS internally on a different port very easy, while preserving external HTTP/S semantics and conventions.

So, I would just configure it manually as you've suggested, in his case.

Thanks for the great issue report!

@mholt mholt closed this Jan 11, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment