http: Split 2-phase auto-HTTPS into 3 phases

This is necessary to avoid a race for sockets. Both the HTTP servers and
CertMagic solvers will try to bind the HTTP/HTTPS ports, but we need to
make sure that our HTTP servers bind first. This is kind of a new thing
now that management is async in Caddy 2.

Also update to CertMagic 0.9.2, which fixes some async use cases at
scale.

4 files changed, 39 insertions, 24 deletions

diff --git a/go.mod b/go.mod
index 57eb414..2f63c28 100644
--- a/go.mod
+++ b/go.mod
@@ -7,7 +7,6 @@ require (
 	github.com/alecthomas/chroma v0.7.0
 	github.com/andybalholm/brotli v0.0.0-20190821151343-b60f0d972eeb
 	github.com/cenkalti/backoff/v3 v3.1.1 // indirect
-	github.com/decker502/dnspod-go v0.2.0 // indirect
 	github.com/dustin/go-humanize v1.0.0
 	github.com/go-acme/lego/v3 v3.3.0
 	github.com/golang/groupcache v0.0.0-20191002201903-404acd9df4cc
@@ -18,7 +17,7 @@ require (
 	github.com/klauspost/cpuid v1.2.2
 	github.com/kylelemons/godebug v1.1.0 // indirect
 	github.com/lucas-clemente/quic-go v0.14.1
-	github.com/mholt/certmagic v0.9.1
+	github.com/mholt/certmagic v0.9.2
 	github.com/miekg/dns v1.1.25 // indirect
 	github.com/muhammadmuzzammil1998/jsonc v0.0.0-20190906142622-1265e9b150c6
 	github.com/naoina/go-stringutil v0.1.0 // indirect
diff --git a/go.sum b/go.sum
index 2a20fa2..7afee00 100644
--- a/go.sum
+++ b/go.sum
@@ -86,7 +86,6 @@ github.com/danwakefield/fnmatch v0.0.0-20160403171240-cbb64ac3d964/go.mod h1:Xd9
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
-github.com/decker502/dnspod-go v0.2.0/go.mod h1:qsurYu1FgxcDwfSwXJdLt4kRsBLZeosEb9uq4Sy+08g=
 github.com/dgrijalva/jwt-go v3.2.0+incompatible/go.mod h1:E3ru+11k8xSBh+hMPgOLZmtrrCbhqsmaPHjLKYnJCaQ=
 github.com/dimchansky/utfbom v1.1.0/go.mod h1:rO41eb7gLfo8SF1jd9F8HplJm1Fewwi4mQvIirEdv+8=
 github.com/dlclark/regexp2 v1.1.6 h1:CqB4MjHw0MFCDj+PHHjiESmHX+N7t0tJzKvC6M97BRg=
@@ -103,9 +102,6 @@ github.com/fatih/structs v1.1.0/go.mod h1:9NiDSp5zOcgEDl+j00MP/WkGVPOlPRLejGD8Ga
 github.com/fsnotify/fsnotify v1.4.7 h1:IXs+QLmnXW2CcXuY+8Mzv/fWEsPGWxqefPtCP5CnV9I=
 github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMoQvtojpjFo=
 github.com/ghodss/yaml v1.0.0/go.mod h1:4dBDuWmgqj2HViK6kFavaiC9ZROes6MMH2rRYeMEF04=
-github.com/go-acme/lego/v3 v3.1.0/go.mod h1:074uqt+JS6plx+c9Xaiz6+L+GBb+7itGtzfcDM2AhEE=
-github.com/go-acme/lego/v3 v3.2.0 h1:z0zvNlL1niv/1qA06V5X1BRC5PeLoGKAlVaWthXQz9c=
-github.com/go-acme/lego/v3 v3.2.0/go.mod h1:074uqt+JS6plx+c9Xaiz6+L+GBb+7itGtzfcDM2AhEE=
 github.com/go-acme/lego/v3 v3.3.0 h1:6BePZsOiYA4/w+M7QDytxQtMfCipMPGnWAHs9pWks98=
 github.com/go-acme/lego/v3 v3.3.0/go.mod h1:iGSY2vQrvQs3WezicSB/oVbO2eCrD88dpWPwb1qLqu0=
 github.com/go-cmd/cmd v1.0.5/go.mod h1:y8q8qlK5wQibcw63djSl/ntiHUHXHGdCkPk0j4QeW4s=
@@ -126,6 +122,7 @@ github.com/golang/groupcache v0.0.0-20191002201903-404acd9df4cc/go.mod h1:cIg4er
 github.com/golang/mock v1.1.1/go.mod h1:oTYuIxOrZwtPieC+H1uAHpcLFnEyAGVDL/k47Jfbm0A=
 github.com/golang/mock v1.2.0 h1:28o5sBqPkBsMGnC6b4MvE2TzSr5/AT4c/1fLqVGIwlk=
 github.com/golang/mock v1.2.0/go.mod h1:oTYuIxOrZwtPieC+H1uAHpcLFnEyAGVDL/k47Jfbm0A=
+github.com/golang/mock v1.3.1 h1:qGJ6qTW+x6xX/my+8YUVl4WNpX9B7+/l2tRsHGZ7f2s=
 github.com/golang/mock v1.3.1/go.mod h1:sBzyDLLjw3U8JLTeZvSv8jJB+tU5PVekmnlKIyFUx0Y=
 github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=
 github.com/golang/protobuf v1.3.0/go.mod h1:Qd/q+1AKNOZr9uGQzbzCmRO6sUih6GTPZv6a1/R87v0=
@@ -228,10 +225,8 @@ github.com/mattn/go-runewidth v0.0.2/go.mod h1:LwmH8dsx7+W8Uxz3IHJYH5QSwggIsqBzp
 github.com/mattn/go-runewidth v0.0.4/go.mod h1:LwmH8dsx7+W8Uxz3IHJYH5QSwggIsqBzpuz5H//U1FU=
 github.com/mattn/go-tty v0.0.0-20180219170247-931426f7535a/go.mod h1:XPvLUNfbS4fJH25nqRHfWLMa1ONC8Amw+mIA639KxkE=
 github.com/matttproud/golang_protobuf_extensions v1.0.1/go.mod h1:D8He9yQNgCq6Z5Ld7szi9bcBfOoFv/3dc6xSMkL2PC0=
-github.com/mholt/certmagic v0.9.0 h1:dYh9sZPDBTcIiPhYM/Qtv3V623/zFH34FmpbrQTpMAc=
-github.com/mholt/certmagic v0.9.0/go.mod h1:91uJzK5K8IWtYQqTi5R2tsxV1pCde+wdGfaRaOZi6aQ=
-github.com/mholt/certmagic v0.9.1 h1:wPzyouOyE+30NIQETJuhTB5ZQWz+0Hy038vaR5WWQDE=
-github.com/mholt/certmagic v0.9.1/go.mod h1:nu8jbsbtwK4205EDH/ZUMTKsfYpJA1Q7MKXHfgTihNw=
+github.com/mholt/certmagic v0.9.2 h1:m+l8jZADOwL2zBVWhaprTnJaRVmRwG4FzR1A8nHwrLw=
+github.com/mholt/certmagic v0.9.2/go.mod h1:nu8jbsbtwK4205EDH/ZUMTKsfYpJA1Q7MKXHfgTihNw=
 github.com/miekg/dns v1.1.15 h1:CSSIDtllwGLMoA6zjdKnaE6Tx6eVUxQ29LUgGetiDCI=
 github.com/miekg/dns v1.1.15/go.mod h1:W1PPwlIAgtquWBMBEV9nkV9Cazfe8ScdGz/Lj7v3Nrg=
 github.com/miekg/dns v1.1.25 h1:dFwPR6SfLtrSwgDcIq2bcU/gVutB4sNApq2HBdqcakg=
@@ -362,7 +357,6 @@ golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACk
 golang.org/x/crypto v0.0.0-20190418165655-df01cb2cc480/go.mod h1:WFFai1msRO1wXaEeE5yQxYXgSfI8pQAWXbQop6sCtWE=
 golang.org/x/crypto v0.0.0-20190510104115-cbcb75029529/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
 golang.org/x/crypto v0.0.0-20190605123033-f99c8df09eb5/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
-golang.org/x/crypto v0.0.0-20190701094942-4def268fd1a4/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
 golang.org/x/crypto v0.0.0-20190829043050-9756ffdc2472/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
 golang.org/x/crypto v0.0.0-20190911031432-227b76d455e7/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
 golang.org/x/crypto v0.0.0-20190923035154-9ee001bba392/go.mod h1:/lpIB1dKB+9EgE3H3cr1v9wB50oz8l4C4h62xy7jSTY=
@@ -420,6 +414,7 @@ golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJ
 golang.org/x/sync v0.0.0-20190227155943-e225da77a7e6/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sync v0.0.0-20190423024810-112230192c58 h1:8gQV6CLnAEikrhgkHFbMAEhagSSnXWGV915qUMm9mrU=
 golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
+golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e h1:vcxGaoTs7kV8m5Np9uUNQin4BrLOthgV7252N8V+FwY=
 golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sys v0.0.0-20180622082034-63fc586f45fe/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
 golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
diff --git a/modules/caddyhttp/autohttps.go b/modules/caddyhttp/autohttps.go
index 69e3318..d60e955 100644
--- a/modules/caddyhttp/autohttps.go
+++ b/modules/caddyhttp/autohttps.go
@@ -62,7 +62,7 @@ func (ahc AutoHTTPSConfig) Skipped(name string, skipSlice []string) bool {
 // HTTPS, and sets up HTTP->HTTPS redirects. This phase must occur
 // at the beginning of provisioning, because it may add routes and
 // even servers to the app, which still need to be set up with the
-// rest of them.
+// rest of them during provisioning.
 func (app *App) automaticHTTPSPhase1(ctx caddy.Context, repl *caddy.Replacer) error {
 	// this map will store associations of HTTP listener
 	// addresses to the routes that do HTTP->HTTPS redirects
@@ -259,10 +259,9 @@ redirRoutesLoop:
 }
 
 // automaticHTTPSPhase2 attaches a TLS app pointer to each
-// server and begins certificate management for all names
-// in the qualifying domain set for each server. This phase
-// must occur after provisioning, and at the beginning of
-// the app start, before starting each of the servers.
+// server. This phase must occur after provisioning, and
+// at the beginning of the app start, before starting each
+// of the servers.
 func (app *App) automaticHTTPSPhase2() error {
 	tlsAppIface, err := app.ctx.App("tls")
 	if err != nil {
@@ -277,6 +276,20 @@ func (app *App) automaticHTTPSPhase2() error {
 		srv.tlsApp = tlsApp
 	}
 
+	return nil
+}
+
+// automaticHTTPSPhase3 begins certificate management for
+// all names in the qualifying domain set for each server.
+// This phase must occur after provisioning and at the end
+// of app start, after all the servers have been started.
+// Doing this last ensures that there won't be any race
+// for listeners on the HTTP or HTTPS ports when management
+// is async (if CertMagic's solvers bind to those ports
+// first, then our servers would fail to bind to them,
+// which would be bad, since CertMagic's bindings are
+// temporary and don't serve the user's sites!).
+func (app *App) automaticHTTPSPhase3() error {
 	// begin managing certificates for enabled servers
 	for srvName, srv := range app.Servers {
 		if srv.AutoHTTPS == nil ||
@@ -294,7 +307,7 @@ func (app *App) automaticHTTPSPhase2() error {
 				// don't obtain another one for it, unless we are
 				// supposed to ignore loaded certificates
 				if !srv.AutoHTTPS.IgnoreLoadedCerts &&
-					len(tlsApp.AllMatchingCertificates(d)) > 0 {
+					len(srv.tlsApp.AllMatchingCertificates(d)) > 0 {
 					app.logger.Info("skipping automatic certificate management because one or more matching certificates are already loaded",
 						zap.String("domain", d),
 						zap.String("server_name", srvName),
@@ -321,10 +334,10 @@ func (app *App) automaticHTTPSPhase2() error {
 				},
 			},
 		}
-		if tlsApp.Automation == nil {
-			tlsApp.Automation = new(caddytls.AutomationConfig)
+		if srv.tlsApp.Automation == nil {
+			srv.tlsApp.Automation = new(caddytls.AutomationConfig)
 		}
-		tlsApp.Automation.Policies = append(tlsApp.Automation.Policies,
+		srv.tlsApp.Automation.Policies = append(srv.tlsApp.Automation.Policies,
 			caddytls.AutomationPolicy{
 				Hosts:      domainsForCerts,
 				Management: acmeManager,
@@ -334,7 +347,7 @@ func (app *App) automaticHTTPSPhase2() error {
 		app.logger.Info("enabling automatic TLS certificate management",
 			zap.Strings("domains", domainsForCerts),
 		)
-		err := tlsApp.Manage(domainsForCerts)
+		err := srv.tlsApp.Manage(domainsForCerts)
 		if err != nil {
 			return fmt.Errorf("%s: managing certificate for %s: %s", srvName, domains, err)
 		}
diff --git a/modules/caddyhttp/caddyhttp.go b/modules/caddyhttp/caddyhttp.go
index fc727d0..576620e 100644
--- a/modules/caddyhttp/caddyhttp.go
+++ b/modules/caddyhttp/caddyhttp.go
@@ -220,11 +220,12 @@ func (app *App) Validate() error {
 // Start runs the app. It finishes automatic HTTPS if enabled,
 // including management of certificates.
 func (app *App) Start() error {
-	// finish setting up automatic HTTPS and manage certs;
-	// this must happen before each server is started
+	// give each server a pointer to the TLS app;
+	// this is required before they are started so
+	// they can solve ACME challenges
 	err := app.automaticHTTPSPhase2()
 	if err != nil {
-		return fmt.Errorf("enabling automatic HTTPS: %v", err)
+		return fmt.Errorf("enabling automatic HTTPS, phase 2: %v", err)
 	}
 
 	for srvName, srv := range app.Servers {
@@ -297,6 +298,13 @@ func (app *App) Start() error {
 		}
 	}
 
+	// finish automatic HTTPS by finally beginning
+	// certificate management
+	err = app.automaticHTTPSPhase3()
+	if err != nil {
+		return fmt.Errorf("finalizing automatic HTTPS: %v", err)
+	}
+
 	return nil
 }