From 40e05e5a01d50b381fa7a7a472ea38f44518f02c Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Wed, 18 Sep 2019 18:01:32 -0600 Subject: [PATCH] http: Improve auto HTTP->HTTPS redirects, fix edge cases See https://caddy.community/t/v2-issues-with-multiple-server-blocks-in-caddyfile-style-config/6206/13?u=matt Also print pid when using `caddy start` --- cmd/commands.go | 2 +- modules/caddyhttp/caddyhttp.go | 129 +++++++++++++++++---------------- modules/caddyhttp/server.go | 29 +++++--- 3 files changed, 86 insertions(+), 74 deletions(-) diff --git a/cmd/commands.go b/cmd/commands.go index 4eda720e..2526e45d 100644 --- a/cmd/commands.go +++ b/cmd/commands.go @@ -130,7 +130,7 @@ func cmdStart() (int, error) { // when one of the goroutines unblocks, we're done and can exit select { case <-success: - fmt.Println("Successfully started Caddy") + fmt.Printf("Successfully started Caddy (pid=%d)\n", cmd.Process.Pid) case err := <-exit: return caddy.ExitCodeFailedStartup, fmt.Errorf("caddy process exited with error: %v", err) diff --git a/modules/caddyhttp/caddyhttp.go b/modules/caddyhttp/caddyhttp.go index 0a26ad18..c4320ccd 100644 --- a/modules/caddyhttp/caddyhttp.go +++ b/modules/caddyhttp/caddyhttp.go @@ -183,12 +183,8 @@ func (app *App) Start() error { } // enable TLS - httpPort := app.HTTPPort - if httpPort == 0 { - httpPort = DefaultHTTPPort - } _, port, _ := net.SplitHostPort(addr) - if len(srv.TLSConnPolicies) > 0 && port != strconv.Itoa(httpPort) { + if len(srv.TLSConnPolicies) > 0 && port != strconv.Itoa(app.httpPort()) { tlsCfg, err := srv.TLSConnPolicies.TLSConfig(app.ctx) if err != nil { return fmt.Errorf("%s/%s: making TLS configuration: %v", network, addr, err) @@ -273,8 +269,9 @@ func (app *App) automaticHTTPS() error { } tlsApp := tlsAppIface.(*caddytls.TLS) - lnAddrMap := make(map[string]struct{}) - var redirRoutes RouteList + // this map will store associations of HTTP listener + // addresses to the routes that do HTTP->HTTPS redirects + lnAddrRedirRoutes := make(map[string]Route) for srvName, srv := range app.Servers { srv.tlsApp = tlsApp @@ -284,9 +281,9 @@ func (app *App) automaticHTTPS() error { } // skip if all listeners use the HTTP port - if !srv.listenersUseAnyPortOtherThan(app.HTTPPort) { - log.Printf("[INFO] Server %v is only listening on the HTTP port %d, so no automatic HTTPS will be applied to this server", - srv.Listen, app.HTTPPort) + if !srv.listenersUseAnyPortOtherThan(app.httpPort()) { + log.Printf("[INFO] Server %s is only listening on the HTTP port %d, so no automatic HTTPS will be applied to this server", + srvName, app.httpPort()) continue } @@ -334,10 +331,10 @@ func (app *App) automaticHTTPS() error { acmeManager := &caddytls.ACMEManagerMaker{ Challenges: caddytls.ChallengesConfig{ HTTP: caddytls.HTTPChallengeConfig{ - AlternatePort: app.HTTPPort, + AlternatePort: app.HTTPPort, // we specifically want the user-configured port, if any }, TLSALPN: caddytls.TLSALPNChallengeConfig{ - AlternatePort: app.HTTPSPort, + AlternatePort: app.HTTPSPort, // we specifically want the user-configured port, if any }, }, } @@ -367,12 +364,6 @@ func (app *App) automaticHTTPS() error { log.Printf("[INFO] Enabling automatic HTTP->HTTPS redirects for %v", domains) - // notify user if their config might override the HTTP->HTTPS redirects - if srv.listenersIncludePort(app.HTTPPort) { - log.Printf("[WARNING] Server %v is listening on HTTP port %d, so automatic HTTP->HTTPS redirects may be overridden by your own configuration", - srv.Listen, app.HTTPPort) - } - // create HTTP->HTTPS redirects for _, addr := range srv.Listen { netw, host, port, err := caddy.SplitNetworkAddress(addr) @@ -380,28 +371,22 @@ func (app *App) automaticHTTPS() error { return fmt.Errorf("%s: invalid listener address: %v", srvName, addr) } - httpPort := app.HTTPPort - if httpPort == 0 { - httpPort = DefaultHTTPPort - } - httpRedirLnAddr := caddy.JoinNetworkAddress(netw, host, strconv.Itoa(httpPort)) - lnAddrMap[httpRedirLnAddr] = struct{}{} - if parts := strings.SplitN(port, "-", 2); len(parts) == 2 { port = parts[0] } redirTo := "https://{http.request.host}" - httpsPort := app.HTTPSPort - if httpsPort == 0 { - httpsPort = DefaultHTTPSPort - } - if port != strconv.Itoa(httpsPort) { + if port != strconv.Itoa(app.httpsPort()) { redirTo += ":" + port } redirTo += "{http.request.uri}" - redirRoutes = append(redirRoutes, Route{ + // build the plaintext HTTP variant of this address + httpRedirLnAddr := caddy.JoinNetworkAddress(netw, host, strconv.Itoa(app.httpPort())) + + // create the route that does the redirect and associate + // it with the listener address it will be served from + lnAddrRedirRoutes[httpRedirLnAddr] = Route{ MatcherSets: []MatcherSet{ { MatchProtocol("http"), @@ -418,52 +403,70 @@ func (app *App) automaticHTTPS() error { Close: true, }, }, - }) + } + } } } - if len(lnAddrMap) > 0 { - var lnAddrs []string - mapLoop: - for addr := range lnAddrMap { - netw, addrs, err := caddy.ParseNetworkAddress(addr) - if err != nil { - continue - } - for _, a := range addrs { - if app.listenerTaken(netw, a) { - continue mapLoop + // if there are HTTP->HTTPS redirects to add, do so now + if len(lnAddrRedirRoutes) > 0 { + var redirServerAddrs []string + var redirRoutes []Route + + // for each redirect listener, see if there's already a + // server configured to listen on that exact address; if + // so, simply the redirect route to the end of its route + // list; otherwise, we'll create a new server for all the + // listener addresses that are unused and serve the + // remaining redirects from it + redirRoutesLoop: + for addr, redirRoute := range lnAddrRedirRoutes { + for srvName, srv := range app.Servers { + if srv.hasListenerAddress(addr) { + // user has configured a server for the same address + // that the redirect runs from; simply append our + // redirect route to the existing routes, with a + // caveat that their config might override ours + log.Printf("[WARNING] Server %s is listening on %s, so automatic HTTP->HTTPS redirects might be overridden by your own configuration", + srvName, addr) + srv.Routes = append(srv.Routes, redirRoute) + continue redirRoutesLoop } } - lnAddrs = append(lnAddrs, addr) + // no server with this listener address exists; + // save this address and route for custom server + redirServerAddrs = append(redirServerAddrs, addr) + redirRoutes = append(redirRoutes, redirRoute) } - app.Servers["auto_https_redirects"] = &Server{ - Listen: lnAddrs, - Routes: redirRoutes, - AutoHTTPS: &AutoHTTPSConfig{Disabled: true}, - tlsApp: tlsApp, // required to solve HTTP challenge + + // if there are routes remaining which do not belong + // in any existing server, make our own to serve the + // rest of the redirects + if len(redirServerAddrs) > 0 { + app.Servers["remaining_auto_https_redirects"] = &Server{ + Listen: redirServerAddrs, + Routes: redirRoutes, + tlsApp: tlsApp, // required to solve HTTP challenge + } } } return nil } -func (app *App) listenerTaken(network, address string) bool { - for _, srv := range app.Servers { - for _, addr := range srv.Listen { - netw, addrs, err := caddy.ParseNetworkAddress(addr) - if err != nil || netw != network { - continue - } - for _, a := range addrs { - if a == address { - return true - } - } - } +func (app *App) httpPort() int { + if app.HTTPPort == 0 { + return DefaultHTTPPort } - return false + return app.HTTPPort +} + +func (app *App) httpsPort() int { + if app.HTTPSPort == 0 { + return DefaultHTTPSPort + } + return app.HTTPSPort } var defaultALPN = []string{"h2", "http/1.1"} diff --git a/modules/caddyhttp/server.go b/modules/caddyhttp/server.go index 0ccdeea0..c4c306e4 100644 --- a/modules/caddyhttp/server.go +++ b/modules/caddyhttp/server.go @@ -196,17 +196,26 @@ func (s *Server) listenersUseAnyPortOtherThan(otherPort int) bool { return false } -// listenersIncludePort returns true if there are any -// listeners in s that use otherPort. -func (s *Server) listenersIncludePort(otherPort int) bool { +func (s *Server) hasListenerAddress(fullAddr string) bool { + netw, addrs, err := caddy.ParseNetworkAddress(fullAddr) + if err != nil { + return false + } + if len(addrs) != 1 { + return false + } + addr := addrs[0] for _, lnAddr := range s.Listen { - _, addrs, err := caddy.ParseNetworkAddress(lnAddr) - if err == nil { - for _, a := range addrs { - _, port, err := net.SplitHostPort(a) - if err == nil && port == strconv.Itoa(otherPort) { - return true - } + thisNetw, thisAddrs, err := caddy.ParseNetworkAddress(lnAddr) + if err != nil { + continue + } + if thisNetw != netw { + continue + } + for _, a := range thisAddrs { + if a == addr { + return true } } }