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.

2 files changed, 34 insertions, 13 deletions

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
 }