letsencrypt: Fix OCSP stapling and restarts with new LE-capable hosts

Before, Caddy couldn't support graceful (zero-downtime) restarts when the reloaded Caddyfile had a host in it that was elligible for a LE certificate because the port was already in use. This commit makes it possible to do zero-downtime reloads and issue certificates for new hosts that need it. Supports only http-01 challenge at this time.

OCSP stapling is improved in that it updates before the expiration time when the validity window has shifted forward. See 30c94908. Before it only used to update when the status changed.

This commit also sets the user agent for Let's Encrypt requests with a string containing "Caddy".

caddy/caddy.go

@@ -191,6 +191,7 @@ func startServers(groupings bindingGroup) error {
 			return err
 		}
 		s.HTTP2 = HTTP2                             // TODO: This setting is temporary
+		s.ReqCallback = letsencrypt.RequestCallback // ensures we can solve ACME challenges while running
 
 		var ln server.ListenerFile
 		if IsRestart() {

caddy/letsencrypt/crypto_test.go

@@ -40,12 +40,12 @@ func TestSaveAndLoadRSAPrivateKey(t *testing.T) {
 	}
 }
 
-// rsaPrivateKeyBytes returns the bytes of DER-encoded key.
-func rsaPrivateKeyBytes(key *rsa.PrivateKey) []byte {
-	return x509.MarshalPKCS1PrivateKey(key)
-}
-
 // rsaPrivateKeysSame compares the bytes of a and b and returns true if they are the same.
 func rsaPrivateKeysSame(a, b *rsa.PrivateKey) bool {
 	return bytes.Equal(rsaPrivateKeyBytes(a), rsaPrivateKeyBytes(b))
 }
+
+// rsaPrivateKeyBytes returns the bytes of DER-encoded key.
+func rsaPrivateKeyBytes(key *rsa.PrivateKey) []byte {
+	return x509.MarshalPKCS1PrivateKey(key)
+}

caddy/letsencrypt/handler.go

@@ -2,30 +2,21 @@ package letsencrypt
 
 import (
 	"crypto/tls"
+	"log"
 	"net"
 	"net/http"
 	"net/http/httputil"
 	"net/url"
 	"strings"
-
-	"github.com/mholt/caddy/middleware"
 )
 
 const challengeBasePath = "/.well-known/acme-challenge"
 
-// Handler is a Caddy middleware that can proxy ACME challenge
-// requests to the real ACME client endpoint. This is necessary
-// to renew certificates while the server is running.
-type Handler struct {
-	Next middleware.Handler
-	//ChallengeActive int32 // (TODO) use sync/atomic to set/get this flag safely and efficiently
-}
-
-// ServeHTTP is basically a no-op unless an ACME challenge is active on this host
-// and the request path matches the expected path exactly.
-func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error) {
-	// Proxy challenge requests to ACME client
-	// TODO: Only do this if a challenge is active?
+// RequestCallback proxies challenge requests to ACME client if the
+// request path starts with challengeBasePath. It returns true if it
+// handled the request and no more needs to be done; it returns false
+// if this call was a no-op and the request still needs handling.
+func RequestCallback(w http.ResponseWriter, r *http.Request) bool {
 	if strings.HasPrefix(r.URL.Path, challengeBasePath) {
 		scheme := "http"
 		if r.TLS != nil {
@@ -37,9 +28,11 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error)
 			hostname = r.URL.Host
 		}
 
-		upstream, err := url.Parse(scheme + "://" + hostname + ":" + alternatePort)
+		upstream, err := url.Parse(scheme + "://" + hostname + ":" + AlternatePort)
 		if err != nil {
-			return http.StatusInternalServerError, err
+			w.WriteHeader(http.StatusInternalServerError)
+			log.Printf("[ERROR] letsencrypt handler: %v", err)
+			return true
 		}
 
 		proxy := httputil.NewSingleHostReverseProxy(upstream)
@@ -48,8 +41,8 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, error)
 		}
 		proxy.ServeHTTP(w, r)
 
-		return 0, nil
+		return true
 	}
 
-	return h.Next.ServeHTTP(w, r)
+	return false
 }

caddy/letsencrypt/letsencrypt_test.go

@@ -23,9 +23,11 @@ func TestHostQualifies(t *testing.T) {
 		{"", false},
 		{" ", false},
 		{"0.0.0.0", false},
-		{"192.168.1.3", true},
-		{"10.0.2.1", true},
+		{"192.168.1.3", false},
+		{"10.0.2.1", false},
+		{"169.112.53.4", false},
 		{"foobar.com", true},
+		{"sub.foobar.com", true},
 	} {
 		if HostQualifies(test.host) && !test.expect {
 			t.Errorf("Test %d: Expected '%s' to NOT qualify, but it did", i, test.host)
@@ -39,14 +41,14 @@ func TestHostQualifies(t *testing.T) {
 func TestRedirPlaintextHost(t *testing.T) {
 	cfg := redirPlaintextHost(server.Config{
 		Host: "example.com",
-		Port: "http",
+		Port: "80",
 	})
 
 	// Check host and port
 	if actual, expected := cfg.Host, "example.com"; actual != expected {
 		t.Errorf("Expected redir config to have host %s but got %s", expected, actual)
 	}
-	if actual, expected := cfg.Port, "http"; actual != expected {
+	if actual, expected := cfg.Port, "80"; actual != expected {
 		t.Errorf("Expected redir config to have port '%s' but got '%s'", expected, actual)
 	}
 

caddy/letsencrypt/maintain.go

@@ -27,8 +27,8 @@ var OnChange func() error
 // which you'll close when maintenance should stop, to allow this
 // goroutine to clean up after itself and unblock.
 func maintainAssets(configs []server.Config, stopChan chan struct{}) {
-	renewalTicker := time.NewTicker(renewInterval)
-	ocspTicker := time.NewTicker(ocspInterval)
+	renewalTicker := time.NewTicker(RenewInterval)
+	ocspTicker := time.NewTicker(OCSPInterval)
 
 	for {
 		select {
@@ -47,17 +47,27 @@ func maintainAssets(configs []server.Config, stopChan chan struct{}) {
 				}
 			}
 		case <-ocspTicker.C:
-			for bundle, oldStatus := range ocspStatus {
-				_, newStatus, err := acme.GetOCSPForCert(*bundle)
-				if err == nil && newStatus != oldStatus && OnChange != nil {
-					log.Printf("[INFO] OCSP status changed from %v to %v", oldStatus, newStatus)
+			for bundle, oldResp := range ocspCache {
+				// start checking OCSP staple about halfway through validity period for good measure
+				refreshTime := oldResp.ThisUpdate.Add(oldResp.NextUpdate.Sub(oldResp.ThisUpdate) / 10)
+				if time.Now().After(refreshTime) {
+					_, newResp, err := acme.GetOCSPForCert(*bundle)
+					if err != nil {
+						log.Printf("[ERROR] Checking OCSP for bundle: %v", err)
+						continue
+					}
+					if newResp.NextUpdate != oldResp.NextUpdate {
+						if OnChange != nil {
+							log.Printf("[INFO] Updating OCSP stapling to extend validity period to %v", newResp.NextUpdate)
 							err := OnChange()
 							if err != nil {
-						log.Printf("[ERROR] OnChange after OCSP update: %v", err)
+								log.Printf("[ERROR] OnChange after OCSP trigger: %v", err)
 							}
 							break
 						}
 					}
+				}
+			}
 		case <-stopChan:
 			renewalTicker.Stop()
 			ocspTicker.Stop()
@@ -107,7 +117,7 @@ func renewCertificates(configs []server.Config, useCustomPort bool) (int, []erro
 			log.Printf("[INFO] Certificate for %s has %d days remaining; attempting renewal", cfg.Host, daysLeft)
 			var client *acme.Client
 			if useCustomPort {
-				client, err = newClientPort("", alternatePort) // email not used for renewal
+				client, err = newClientPort("", AlternatePort) // email not used for renewal
 			} else {
 				client, err = newClient("")
 			}
@@ -134,7 +144,7 @@ func renewCertificates(configs []server.Config, useCustomPort bool) (int, []erro
 
 			// Renew certificate
 		Renew:
-			newCertMeta, err := client.RenewCertificate(certMeta, true, true)
+			newCertMeta, err := client.RenewCertificate(certMeta, true)
 			if err != nil {
 				if _, ok := err.(acme.TOSError); ok {
 					err := client.AgreeToTOS()
@@ -145,24 +155,20 @@ func renewCertificates(configs []server.Config, useCustomPort bool) (int, []erro
 				}
 
 				time.Sleep(10 * time.Second)
-				newCertMeta, err = client.RenewCertificate(certMeta, true, true)
+				newCertMeta, err = client.RenewCertificate(certMeta, true)
 				if err != nil {
 					errs = append(errs, err)
 					continue
 				}
 			}
 
-			saveCertsAndKeys([]acme.CertificateResource{newCertMeta})
+			saveCertResource(newCertMeta)
 			n++
-		} else if daysLeft <= 30 {
-			// Warn on 30 days remaining. TODO: Just do this once...
+		} else if daysLeft <= 21 {
+			// Warn on 21 days remaining. TODO: Just do this once...
 			log.Printf("[WARNING] Certificate for %s has %d days remaining; will automatically renew when 14 days remain\n", cfg.Host, daysLeft)
 		}
 	}
 
 	return n, errs
 }
-
-// acmeHandlers is a map of host to ACME handler. These
-// are used to proxy ACME requests to the ACME client.
-var acmeHandlers = make(map[string]*Handler)

caddy/restart.go

@@ -3,11 +3,17 @@
 package caddy
 
 import (
+	"bytes"
 	"encoding/gob"
+	"errors"
 	"io/ioutil"
 	"log"
 	"os"
 	"os/exec"
+	"path"
+
+	"github.com/mholt/caddy/caddy/letsencrypt"
+	"github.com/mholt/caddy/server"
 )
 
 func init() {
@@ -33,6 +39,12 @@ func Restart(newCaddyfile Input) error {
 		caddyfileMu.Unlock()
 	}
 
+	// Get certificates for any new hosts in the new Caddyfile without causing downtime
+	err := getCertsForNewCaddyfile(newCaddyfile)
+	if err != nil {
+		return errors.New("TLS preload: " + err.Error())
+	}
+
 	if len(os.Args) == 0 { // this should never happen, but...
 		os.Args = []string{""}
 	}
@@ -61,7 +73,7 @@ func Restart(newCaddyfile Input) error {
 
 	// Pass along relevant file descriptors to child process; ordering
 	// is very important since we rely on these being in certain positions.
-	extraFiles := []*os.File{sigwpipe}
+	extraFiles := []*os.File{sigwpipe} // fd 3
 
 	// Add file descriptors of all the sockets
 	serversMu.Lock()
@@ -110,3 +122,45 @@ func Restart(newCaddyfile Input) error {
 	// Looks like child is successful; we can exit gracefully.
 	return Stop()
 }
+
+func getCertsForNewCaddyfile(newCaddyfile Input) error {
+	// parse the new caddyfile only up to (and including) TLS
+	// so we can know what we need to get certs for.
+	configs, _, _, err := loadConfigsUpToIncludingTLS(path.Base(newCaddyfile.Path()), bytes.NewReader(newCaddyfile.Body()))
+	if err != nil {
+		return errors.New("loading Caddyfile: " + err.Error())
+	}
+
+	// TODO: Yuck, this is hacky. port 443 not set until letsencrypt is activated, so we change it here.
+	for i := range configs {
+		if configs[i].Port == "" && letsencrypt.ConfigQualifies(configs, i) {
+			configs[i].Port = "443"
+		}
+	}
+
+	// only get certs for configs that bind to an address we're already listening on
+	groupings, err := arrangeBindings(configs)
+	if err != nil {
+		return errors.New("arranging bindings: " + err.Error())
+	}
+	var configsToSetup []server.Config
+	serversMu.Lock()
+GroupLoop:
+	for _, group := range groupings {
+		for _, server := range servers {
+			if server.Addr == group.BindAddr.String() {
+				configsToSetup = append(configsToSetup, group.Configs...)
+				continue GroupLoop
+			}
+		}
+	}
+	serversMu.Unlock()
+
+	// obtain certs for eligible configs; letsencrypt pkg will filter out the rest.
+	configs, err = letsencrypt.ObtainCertsAndConfigure(configsToSetup, letsencrypt.AlternatePort)
+	if err != nil {
+		return errors.New("obtaining certs: " + err.Error())
+	}
+
+	return nil
+}

main.go

@@ -14,6 +14,7 @@ import (
 
 	"github.com/mholt/caddy/caddy"
 	"github.com/mholt/caddy/caddy/letsencrypt"
+	"github.com/xenolf/lego/acme"
 )
 
 var (
@@ -53,6 +54,7 @@ func main() {
 
 	caddy.AppName = appName
 	caddy.AppVersion = appVersion
+	acme.UserAgent = appName + "/" + appVersion
 
 	// set up process log before anything bad happens
 	switch logfile {

server/server.go

@@ -33,6 +33,7 @@ type Server struct {
 	httpWg      sync.WaitGroup         // used to wait on outstanding connections
 	startChan   chan struct{}          // used to block until server is finished starting
 	connTimeout time.Duration          // the maximum duration of a graceful shutdown
+	ReqCallback OptionalCallback       // if non-nil, is executed at the beginning of every request
 }
 
 // ListenerFile represents a listener.
@@ -41,6 +42,11 @@ type ListenerFile interface {
 	File() (*os.File, error)
 }
 
+// OptionalCallback is a function that may or may not handle a request.
+// It returns whether or not it handled the request. If it handled the
+// request, it is presumed that no further request handling should occur.
+type OptionalCallback func(http.ResponseWriter, *http.Request) bool
+
 // New creates a new Server which will bind to addr and serve
 // the sites/hosts configured in configs. Its listener will
 // gracefully close when the server is stopped which will take
@@ -309,6 +315,13 @@ func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		}
 	}()
 
+	w.Header().Set("Server", "Caddy")
+
+	// Execute the optional request callback if it exists
+	if s.ReqCallback != nil && s.ReqCallback(w, r) {
+		return
+	}
+
 	host, _, err := net.SplitHostPort(r.Host)
 	if err != nil {
 		host = r.Host // oh well
@@ -324,8 +337,6 @@ func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	}
 
 	if vh, ok := s.vhosts[host]; ok {
-		w.Header().Set("Server", "Caddy")
-
 		status, _ := vh.stack.ServeHTTP(w, r)
 
 		// Fallback error response in case error handling wasn't chained in