letsencrypt: More tests, other minor improvements

caddy/letsencrypt/letsencrypt.go

@@ -267,9 +267,13 @@ func MakePlaintextRedirects(allConfigs []server.Config) []server.Config {
 	return allConfigs
 }
 
-// ConfigQualifies returns true if the config at cfgIndex (within allConfigs)
-// qualifes for automatic LE activation. It does NOT check to see if a cert
-// and key already exist for the config.
+// ConfigQualifies returns true if cfg qualifies for
+// fully managed TLS. It does NOT check to see if a
+// cert and key already exist for the config. If the
+// config does qualify, you should set cfg.TLS.Managed
+// to true and use that instead, because the process of
+// setting up the config may make it look like it
+// doesn't qualify even though it originally did.
 func ConfigQualifies(cfg server.Config) bool {
 	return cfg.TLS.Certificate == "" && // user could provide their own cert and key
 		cfg.TLS.Key == "" &&
@@ -289,13 +293,16 @@ func ConfigQualifies(cfg server.Config) bool {
 // not eligible because we cannot obtain certificates
 // for those names.
 func HostQualifies(hostname string) bool {
-	return hostname != "localhost" &&
+	return hostname != "localhost" && // localhost is ineligible
+
+		// hostname must not be empty
 		strings.TrimSpace(hostname) != "" &&
-		net.ParseIP(hostname) == nil && // cannot be an IP address, see: https://community.letsencrypt.org/t/certificate-for-static-ip/84/2?u=mholt
 
-		// These special cases can sneak through if specified with -host and with empty/no Caddyfile
-		hostname != "[::]" &&
-		hostname != "[::1]"
+		// cannot be an IP address, see
+		// https://community.letsencrypt.org/t/certificate-for-static-ip/84/2?u=mholt
+		// (also trim [] from either end, since that special case can sneak through
+		// for IPv6 addresses using the -host flag and with empty/no Caddyfile)
+		net.ParseIP(strings.Trim(hostname, "[]")) == nil
 }
 
 // existingCertAndKey returns true if the host has a certificate

caddy/letsencrypt/letsencrypt_test.go

 package letsencrypt
 
 import (
+	"io/ioutil"
 	"net/http"
+	"os"
 	"testing"
 
 	"github.com/mholt/caddy/middleware/redirect"
 	"github.com/mholt/caddy/server"
+	"github.com/xenolf/lego/acme"
 )
 
 func TestHostQualifies(t *testing.T) {
@@ -38,11 +41,37 @@ func TestHostQualifies(t *testing.T) {
 	}
 }
 
+func TestConfigQualifies(t *testing.T) {
+	for i, test := range []struct {
+		cfg    server.Config
+		expect bool
+	}{
+		{server.Config{Host: "localhost"}, false},
+		{server.Config{Host: "example.com"}, true},
+		{server.Config{Host: "example.com", TLS: server.TLSConfig{Certificate: "cert.pem"}}, false},
+		{server.Config{Host: "example.com", TLS: server.TLSConfig{Key: "key.pem"}}, false},
+		{server.Config{Host: "example.com", TLS: server.TLSConfig{LetsEncryptEmail: "off"}}, false},
+		{server.Config{Host: "example.com", TLS: server.TLSConfig{LetsEncryptEmail: "foo@bar.com"}}, true},
+		{server.Config{Host: "example.com", Scheme: "http"}, false},
+		{server.Config{Host: "example.com", Port: "80"}, false},
+		{server.Config{Host: "example.com", Port: "1234"}, true},
+		{server.Config{Host: "example.com", Scheme: "https"}, true},
+		{server.Config{Host: "example.com", Port: "80", Scheme: "https"}, false},
+	} {
+		if test.expect && !ConfigQualifies(test.cfg) {
+			t.Errorf("Test %d: Expected config to qualify, but it did NOT: %#v", i, test.cfg)
+		}
+		if !test.expect && ConfigQualifies(test.cfg) {
+			t.Errorf("Test %d: Expected config to NOT qualify, but it did: %#v", i, test.cfg)
+		}
+	}
+}
+
 func TestRedirPlaintextHost(t *testing.T) {
 	cfg := redirPlaintextHost(server.Config{
 		Host:     "example.com",
 		BindHost: "93.184.216.34",
-		Port:     "80",
+		Port:     "1234",
 	})
 
 	// Check host and port
@@ -76,10 +105,74 @@ func TestRedirPlaintextHost(t *testing.T) {
 	if actual, expected := handler.Rules[0].FromPath, "/"; actual != expected {
 		t.Errorf("Expected redirect rule to be for path '%s' but is actually for '%s'", expected, actual)
 	}
-	if actual, expected := handler.Rules[0].To, "https://example.com{uri}"; actual != expected {
+	if actual, expected := handler.Rules[0].To, "https://example.com:1234{uri}"; actual != expected {
 		t.Errorf("Expected redirect rule to be to URL '%s' but is actually to '%s'", expected, actual)
 	}
 	if actual, expected := handler.Rules[0].Code, http.StatusMovedPermanently; actual != expected {
 		t.Errorf("Expected redirect rule to have code %d but was %d", expected, actual)
 	}
+
+	// browsers can interpret default ports with scheme, so make sure the port
+	// doesn't get added in explicitly for default ports.
+	cfg = redirPlaintextHost(server.Config{Host: "example.com", Port: "443"})
+	handler, ok = cfg.Middleware["/"][0](nil).(redirect.Redirect)
+	if actual, expected := handler.Rules[0].To, "https://example.com{uri}"; actual != expected {
+		t.Errorf("(Default Port) Expected redirect rule to be to URL '%s' but is actually to '%s'", expected, actual)
+	}
+}
+
+func TestSaveCertResource(t *testing.T) {
+	storage = Storage("./le_test")
+	defer func() {
+		err := os.RemoveAll(string(storage))
+		if err != nil {
+			t.Fatalf("Could not remove temporary storage directory (%s): %v", storage, err)
+		}
+	}()
+
+	domain := "example.com"
+	certContents := "certificate"
+	keyContents := "private key"
+	metaContents := `{
+	"domain": "example.com",
+	"certUrl": "https://example.com/cert",
+	"certStableUrl": "https://example.com/cert/stable"
+}`
+
+	cert := acme.CertificateResource{
+		Domain:        domain,
+		CertURL:       "https://example.com/cert",
+		CertStableURL: "https://example.com/cert/stable",
+		PrivateKey:    []byte(keyContents),
+		Certificate:   []byte(certContents),
+	}
+
+	err := saveCertResource(cert)
+	if err != nil {
+		t.Fatalf("Expected no error, got: %v", err)
+	}
+
+	certFile, err := ioutil.ReadFile(storage.SiteCertFile(domain))
+	if err != nil {
+		t.Errorf("Expected no error reading certificate file, got: %v", err)
+	}
+	if string(certFile) != certContents {
+		t.Errorf("Expected certificate file to contain '%s', got '%s'", certContents, string(certFile))
+	}
+
+	keyFile, err := ioutil.ReadFile(storage.SiteKeyFile(domain))
+	if err != nil {
+		t.Errorf("Expected no error reading private key file, got: %v", err)
+	}
+	if string(keyFile) != keyContents {
+		t.Errorf("Expected private key file to contain '%s', got '%s'", keyContents, string(keyFile))
+	}
+
+	metaFile, err := ioutil.ReadFile(storage.SiteMetaFile(domain))
+	if err != nil {
+		t.Errorf("Expected no error reading meta file, got: %v", err)
+	}
+	if string(metaFile) != metaContents {
+		t.Errorf("Expected meta file to contain '%s', got '%s'", metaContents, string(metaFile))
+	}
 }

caddy/letsencrypt/maintain.go

@@ -50,12 +50,16 @@ func maintainAssets(configs []server.Config, stopChan chan struct{}) {
 			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) / 2)
+
+				// only check for updated OCSP validity window if refreshTime is in the past
 				if time.Now().After(refreshTime) {
 					_, newResp, err := acme.GetOCSPForCert(*bundle)
 					if err != nil {
 						log.Printf("[ERROR] Checking OCSP for bundle: %v", err)
 						continue
 					}
+
+					// we're not looking for different status, just a more future expiration
 					if newResp.NextUpdate != oldResp.NextUpdate {
 						if OnChange != nil {
 							log.Printf("[INFO] Updating OCSP stapling to extend validity period to %v", newResp.NextUpdate)

caddy/letsencrypt/storage_test.go

@@ -6,44 +6,44 @@ import (
 )
 
 func TestStorage(t *testing.T) {
-	storage = Storage("./letsencrypt")
+	storage = Storage("./le_test")
 
-	if expected, actual := filepath.Join("letsencrypt", "sites"), storage.Sites(); actual != expected {
+	if expected, actual := filepath.Join("le_test", "sites"), storage.Sites(); actual != expected {
 		t.Errorf("Expected Sites() to return '%s' but got '%s'", expected, actual)
 	}
-	if expected, actual := filepath.Join("letsencrypt", "sites", "test.com"), storage.Site("test.com"); actual != expected {
+	if expected, actual := filepath.Join("le_test", "sites", "test.com"), storage.Site("test.com"); actual != expected {
 		t.Errorf("Expected Site() to return '%s' but got '%s'", expected, actual)
 	}
-	if expected, actual := filepath.Join("letsencrypt", "sites", "test.com", "test.com.crt"), storage.SiteCertFile("test.com"); actual != expected {
+	if expected, actual := filepath.Join("le_test", "sites", "test.com", "test.com.crt"), storage.SiteCertFile("test.com"); actual != expected {
 		t.Errorf("Expected SiteCertFile() to return '%s' but got '%s'", expected, actual)
 	}
-	if expected, actual := filepath.Join("letsencrypt", "sites", "test.com", "test.com.key"), storage.SiteKeyFile("test.com"); actual != expected {
+	if expected, actual := filepath.Join("le_test", "sites", "test.com", "test.com.key"), storage.SiteKeyFile("test.com"); actual != expected {
 		t.Errorf("Expected SiteKeyFile() to return '%s' but got '%s'", expected, actual)
 	}
-	if expected, actual := filepath.Join("letsencrypt", "sites", "test.com", "test.com.json"), storage.SiteMetaFile("test.com"); actual != expected {
+	if expected, actual := filepath.Join("le_test", "sites", "test.com", "test.com.json"), storage.SiteMetaFile("test.com"); actual != expected {
 		t.Errorf("Expected SiteMetaFile() to return '%s' but got '%s'", expected, actual)
 	}
-	if expected, actual := filepath.Join("letsencrypt", "users"), storage.Users(); actual != expected {
+	if expected, actual := filepath.Join("le_test", "users"), storage.Users(); actual != expected {
 		t.Errorf("Expected Users() to return '%s' but got '%s'", expected, actual)
 	}
-	if expected, actual := filepath.Join("letsencrypt", "users", "me@example.com"), storage.User("me@example.com"); actual != expected {
+	if expected, actual := filepath.Join("le_test", "users", "me@example.com"), storage.User("me@example.com"); actual != expected {
 		t.Errorf("Expected User() to return '%s' but got '%s'", expected, actual)
 	}
-	if expected, actual := filepath.Join("letsencrypt", "users", "me@example.com", "me.json"), storage.UserRegFile("me@example.com"); actual != expected {
+	if expected, actual := filepath.Join("le_test", "users", "me@example.com", "me.json"), storage.UserRegFile("me@example.com"); actual != expected {
 		t.Errorf("Expected UserRegFile() to return '%s' but got '%s'", expected, actual)
 	}
-	if expected, actual := filepath.Join("letsencrypt", "users", "me@example.com", "me.key"), storage.UserKeyFile("me@example.com"); actual != expected {
+	if expected, actual := filepath.Join("le_test", "users", "me@example.com", "me.key"), storage.UserKeyFile("me@example.com"); actual != expected {
 		t.Errorf("Expected UserKeyFile() to return '%s' but got '%s'", expected, actual)
 	}
 
 	// Test with empty emails
-	if expected, actual := filepath.Join("letsencrypt", "users", emptyEmail), storage.User(emptyEmail); actual != expected {
+	if expected, actual := filepath.Join("le_test", "users", emptyEmail), storage.User(emptyEmail); actual != expected {
 		t.Errorf("Expected User(\"\") to return '%s' but got '%s'", expected, actual)
 	}
-	if expected, actual := filepath.Join("letsencrypt", "users", emptyEmail, emptyEmail+".json"), storage.UserRegFile(""); actual != expected {
+	if expected, actual := filepath.Join("le_test", "users", emptyEmail, emptyEmail+".json"), storage.UserRegFile(""); actual != expected {
 		t.Errorf("Expected UserRegFile(\"\") to return '%s' but got '%s'", expected, actual)
 	}
-	if expected, actual := filepath.Join("letsencrypt", "users", emptyEmail, emptyEmail+".key"), storage.UserKeyFile(""); actual != expected {
+	if expected, actual := filepath.Join("le_test", "users", emptyEmail, emptyEmail+".key"), storage.UserKeyFile(""); actual != expected {
 		t.Errorf("Expected UserKeyFile(\"\") to return '%s' but got '%s'", expected, actual)
 	}
 }