Pluggable TLS Storage (#913)

* Initial concept for pluggable storage (sans tests and docs)

* Add TLS storage docs, test harness, and minor clean up from code review

* Fix issue with caddymain's temporary moveStorage

* Formatting improvement on struct array literal by removing struct name

* Pluggable storage changes:

* Change storage interface to persist all site or user data in one call
* Add lock/unlock calls for renewal and cert obtaining

* Key fields on composite literals

caddy/caddymain/run.go

@@ -173,10 +173,12 @@ func moveStorage() {
 	if os.IsNotExist(err) {
 		return
 	}
-	newPath, err := caddytls.StorageFor(caddytls.DefaultCAUrl)
+	// Just use a default config to get default (file) storage
+	fileStorage, err := new(caddytls.Config).StorageFor(caddytls.DefaultCAUrl)
 	if err != nil {
 		log.Fatalf("[ERROR] Unable to get new path for certificate storage: %v", err)
 	}
+	newPath := string(fileStorage.(caddytls.FileStorage))
 	err = os.MkdirAll(string(newPath), 0700)
 	if err != nil {
 		log.Fatalf("[ERROR] Unable to make new certificate storage path: %v\n\nPlease follow instructions at:\nhttps://github.com/mholt/caddy/issues/902#issuecomment-228876011", err)

caddytls/certificates.go

@@ -92,11 +92,15 @@ func getCertificate(name string) (cert Certificate, matched, defaulted bool) {
 //
 // This function is safe for concurrent use.
 func CacheManagedCertificate(domain string, cfg *Config) (Certificate, error) {
-	storage, err := StorageFor(cfg.CAUrl)
+	storage, err := cfg.StorageFor(cfg.CAUrl)
 	if err != nil {
 		return Certificate{}, err
 	}
-	cert, err := makeCertificateFromDisk(storage.SiteCertFile(domain), storage.SiteKeyFile(domain))
+	siteData, err := storage.LoadSite(domain)
+	if err != nil {
+		return Certificate{}, err
+	}
+	cert, err := makeCertificate(siteData.Cert, siteData.Key)
 	if err != nil {
 		return cert, err
 	}

caddytls/client.go

@@ -4,11 +4,9 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"io/ioutil"
 	"log"
 	"net"
 	"net/url"
-	"os"
 	"strings"
 	"sync"
 	"time"
@@ -30,7 +28,7 @@ type ACMEClient struct {
 // newACMEClient creates a new ACMEClient given an email and whether
 // prompting the user is allowed. It's a variable so we can mock in tests.
 var newACMEClient = func(config *Config, allowPrompts bool) (*ACMEClient, error) {
-	storage, err := StorageFor(config.CAUrl)
+	storage, err := config.StorageFor(config.CAUrl)
 	if err != nil {
 		return nil, err
 	}
@@ -180,7 +178,7 @@ Attempts:
 		}
 
 		// Success - immediately save the certificate resource
-		storage, err := StorageFor(c.config.CAUrl)
+		storage, err := c.config.StorageFor(c.config.CAUrl)
 		if err != nil {
 			return err
 		}
@@ -204,28 +202,33 @@ Attempts:
 // Anyway, this function is safe for concurrent use.
 func (c *ACMEClient) Renew(name string) error {
 	// Get access to ACME storage
-	storage, err := StorageFor(c.config.CAUrl)
+	storage, err := c.config.StorageFor(c.config.CAUrl)
 	if err != nil {
 		return err
 	}
 
+	// We must lock the renewal with the storage engine
+	if lockObtained, err := storage.LockRegister(name); err != nil {
+		return err
+	} else if !lockObtained {
+		log.Printf("[INFO] Certificate for %v is already being renewed elsewhere", name)
+		return nil
+	}
+	defer func() {
+		if err := storage.UnlockRegister(name); err != nil {
+			log.Printf("[ERROR] Unable to unlock renewal lock for %v: %v", name, err)
+		}
+	}()
+
 	// Prepare for renewal (load PEM cert, key, and meta)
-	certBytes, err := ioutil.ReadFile(storage.SiteCertFile(name))
+	siteData, err := storage.LoadSite(name)
-	if err != nil {
-		return err
-	}
-	keyBytes, err := ioutil.ReadFile(storage.SiteKeyFile(name))
-	if err != nil {
-		return err
-	}
-	metaBytes, err := ioutil.ReadFile(storage.SiteMetaFile(name))
 	if err != nil {
 		return err
 	}
 	var certMeta acme.CertificateResource
-	err = json.Unmarshal(metaBytes, &certMeta)
+	err = json.Unmarshal(siteData.Meta, &certMeta)
-	certMeta.Certificate = certBytes
+	certMeta.Certificate = siteData.Cert
-	certMeta.PrivateKey = keyBytes
+	certMeta.PrivateKey = siteData.Key
 
 	// Perform renewal and retry if necessary, but not too many times.
 	var newCertMeta acme.CertificateResource
@@ -265,27 +268,26 @@ func (c *ACMEClient) Renew(name string) error {
 // Revoke revokes the certificate for name and deltes
 // it from storage.
 func (c *ACMEClient) Revoke(name string) error {
-	storage, err := StorageFor(c.config.CAUrl)
+	storage, err := c.config.StorageFor(c.config.CAUrl)
 	if err != nil {
 		return err
 	}
 
-	if !existingCertAndKey(storage, name) {
+	if !storage.SiteExists(name) {
 		return errors.New("no certificate and key for " + name)
 	}
 
-	certFile := storage.SiteCertFile(name)
+	siteData, err := storage.LoadSite(name)
-	certBytes, err := ioutil.ReadFile(certFile)
 	if err != nil {
 		return err
 	}
 
-	err = c.Client.RevokeCertificate(certBytes)
+	err = c.Client.RevokeCertificate(siteData.Cert)
 	if err != nil {
 		return err
 	}
 
-	err = os.Remove(certFile)
+	err = storage.DeleteSite(name)
 	if err != nil {
 		return errors.New("certificate revoked, but unable to delete certificate file: " + err.Error())
 	}

caddytls/config.go

@@ -11,6 +11,9 @@ import (
 
 	"github.com/mholt/caddy"
 	"github.com/xenolf/lego/acme"
+	"log"
+	"net/url"
+	"strings"
 )
 
 // Config describes how TLS should be configured and used.
@@ -94,6 +97,13 @@ type Config struct {
 	// The type of key to use when generating
 	// certificates
 	KeyType acme.KeyType
+
+	// The explicitly set storage creator or nil; use
+	// StorageFor() to get a guaranteed non-nil Storage
+	// instance. Note, Caddy may call this frequently so
+	// implementors are encouraged to cache any heavy
+	// instantiations.
+	StorageCreator StorageCreator
 }
 
 // ObtainCert obtains a certificate for c.Hostname, as long as a certificate
@@ -106,15 +116,28 @@ func (c *Config) ObtainCert(allowPrompts bool) error {
 }
 
 func (c *Config) obtainCertName(name string, allowPrompts bool) error {
-	storage, err := StorageFor(c.CAUrl)
+	storage, err := c.StorageFor(c.CAUrl)
 	if err != nil {
 		return err
 	}
 
-	if !c.Managed || !HostQualifies(name) || existingCertAndKey(storage, name) {
+	if !c.Managed || !HostQualifies(name) || storage.SiteExists(name) {
 		return nil
 	}
 
+	// We must lock the obtain with the storage engine
+	if lockObtained, err := storage.LockRegister(name); err != nil {
+		return err
+	} else if !lockObtained {
+		log.Printf("[INFO] Certificate for %v is already being obtained elsewhere", name)
+		return nil
+	}
+	defer func() {
+		if err := storage.UnlockRegister(name); err != nil {
+			log.Printf("[ERROR] Unable to unlock obtain lock for %v: %v", name, err)
+		}
+	}()
+
 	if c.ACMEEmail == "" {
 		c.ACMEEmail = getEmail(storage, allowPrompts)
 	}
@@ -127,34 +150,43 @@ func (c *Config) obtainCertName(name string, allowPrompts bool) error {
 	return client.Obtain([]string{name})
 }
 
-// RenewCert renews the certificate for c.Hostname.
+// RenewCert renews the certificate for c.Hostname. If there is already a lock
+// on renewal, this will not perform the renewal and no error will occur.
 func (c *Config) RenewCert(allowPrompts bool) error {
 	return c.renewCertName(c.Hostname, allowPrompts)
 }
 
+// renewCertName renews the certificate for the given name. If there is already
+// a lock on renewal, this will not perform the renewal and no error will
+// occur.
 func (c *Config) renewCertName(name string, allowPrompts bool) error {
-	storage, err := StorageFor(c.CAUrl)
+	storage, err := c.StorageFor(c.CAUrl)
 	if err != nil {
 		return err
 	}
 
+	// We must lock the renewal with the storage engine
+	if lockObtained, err := storage.LockRegister(name); err != nil {
+		return err
+	} else if !lockObtained {
+		log.Printf("[INFO] Certificate for %v is already being renewed elsewhere", name)
+		return nil
+	}
+	defer func() {
+		if err := storage.UnlockRegister(name); err != nil {
+			log.Printf("[ERROR] Unable to unlock renewal lock for %v: %v", name, err)
+		}
+	}()
+
 	// Prepare for renewal (load PEM cert, key, and meta)
-	certBytes, err := ioutil.ReadFile(storage.SiteCertFile(c.Hostname))
+	siteData, err := storage.LoadSite(c.Hostname)
-	if err != nil {
-		return err
-	}
-	keyBytes, err := ioutil.ReadFile(storage.SiteKeyFile(c.Hostname))
-	if err != nil {
-		return err
-	}
-	metaBytes, err := ioutil.ReadFile(storage.SiteMetaFile(c.Hostname))
 	if err != nil {
 		return err
 	}
 	var certMeta acme.CertificateResource
-	err = json.Unmarshal(metaBytes, &certMeta)
+	err = json.Unmarshal(siteData.Meta, &certMeta)
-	certMeta.Certificate = certBytes
+	certMeta.Certificate = siteData.Cert
-	certMeta.PrivateKey = keyBytes
+	certMeta.PrivateKey = siteData.Key
 
 	client, err := newACMEClient(c, allowPrompts)
 	if err != nil {
@@ -194,6 +226,53 @@ func (c *Config) renewCertName(name string, allowPrompts bool) error {
 	return saveCertResource(storage, newCertMeta)
 }
 
+// StorageFor obtains a TLS Storage instance for the given CA URL which should
+// be unique for every different ACME CA. If a StorageCreator is set on this
+// Config, it will be used. Otherwise the default file storage implementation
+// is used. When the error is nil, this is guaranteed to return a non-nil
+// Storage instance.
+func (c *Config) StorageFor(caURL string) (Storage, error) {
+	// Validate CA URL
+	if caURL == "" {
+		caURL = DefaultCAUrl
+	}
+	if caURL == "" {
+		return nil, fmt.Errorf("cannot create storage without CA URL")
+	}
+	caURL = strings.ToLower(caURL)
+
+	// scheme required or host will be parsed as path (as of Go 1.6)
+	if !strings.Contains(caURL, "://") {
+		caURL = "https://" + caURL
+	}
+
+	u, err := url.Parse(caURL)
+	if err != nil {
+		return nil, fmt.Errorf("%s: unable to parse CA URL: %v", caURL, err)
+	}
+
+	if u.Host == "" {
+		return nil, fmt.Errorf("%s: no host in CA URL", caURL)
+	}
+
+	// Create the storage based on the URL
+	var s Storage
+	if c.StorageCreator != nil {
+		s, err = c.StorageCreator(u)
+		if err != nil {
+			return nil, fmt.Errorf("%s: unable to create custom storage: %v", caURL, err)
+		}
+	}
+	if s == nil {
+		// We trust that this does not return a nil s when there's a nil err
+		s, err = FileStorageCreator(u)
+		if err != nil {
+			return nil, fmt.Errorf("%s: unable to create file storage: %v", caURL, err)
+		}
+	}
+	return s, nil
+}
+
 // MakeTLSConfig reduces configs into a single tls.Config.
 // If TLS is to be disabled, a nil tls.Config will be returned.
 func MakeTLSConfig(configs []*Config) (*tls.Config, error) {

caddytls/config_test.go

@@ -0,0 +1,130 @@
+package caddytls
+
+import (
+	"errors"
+	"net/url"
+	"reflect"
+	"testing"
+)
+
+func TestStorageForNoURL(t *testing.T) {
+	c := &Config{}
+	if _, err := c.StorageFor(""); err == nil {
+		t.Fatal("Expected error on empty URL")
+	}
+}
+
+func TestStorageForLowercasesAndPrefixesScheme(t *testing.T) {
+	resultStr := ""
+	c := &Config{
+		StorageCreator: func(caURL *url.URL) (Storage, error) {
+			resultStr = caURL.String()
+			return nil, nil
+		},
+	}
+	if _, err := c.StorageFor("EXAMPLE.COM/BLAH"); err != nil {
+		t.Fatal(err)
+	}
+	if resultStr != "https://example.com/blah" {
+		t.Fatalf("Unexpected CA URL string: %v", resultStr)
+	}
+}
+
+func TestStorageForBadURL(t *testing.T) {
+	c := &Config{}
+	if _, err := c.StorageFor("http://192.168.0.%31/"); err == nil {
+		t.Fatal("Expected error for bad URL")
+	}
+}
+
+func TestStorageForDefault(t *testing.T) {
+	c := &Config{}
+	s, err := c.StorageFor("example.com")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if reflect.TypeOf(s).Name() != "FileStorage" {
+		t.Fatalf("Unexpected storage type: %v", reflect.TypeOf(s).Name())
+	}
+}
+
+func TestStorageForCustom(t *testing.T) {
+	storage := fakeStorage("fake")
+	c := &Config{
+		StorageCreator: func(caURL *url.URL) (Storage, error) {
+			return storage, nil
+		},
+	}
+	s, err := c.StorageFor("example.com")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if s != storage {
+		t.Fatal("Unexpected storage")
+	}
+}
+
+func TestStorageForCustomError(t *testing.T) {
+	c := &Config{
+		StorageCreator: func(caURL *url.URL) (Storage, error) {
+			return nil, errors.New("some error")
+		},
+	}
+	if _, err := c.StorageFor("example.com"); err == nil {
+		t.Fatal("Expecting error")
+	}
+}
+
+func TestStorageForCustomNil(t *testing.T) {
+	// Should fall through to the default
+	c := &Config{
+		StorageCreator: func(caURL *url.URL) (Storage, error) {
+			return nil, nil
+		},
+	}
+	s, err := c.StorageFor("example.com")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if reflect.TypeOf(s).Name() != "FileStorage" {
+		t.Fatalf("Unexpected storage type: %v", reflect.TypeOf(s).Name())
+	}
+}
+
+type fakeStorage string
+
+func (s fakeStorage) SiteExists(domain string) bool {
+	panic("no impl")
+}
+
+func (s fakeStorage) LoadSite(domain string) (*SiteData, error) {
+	panic("no impl")
+}
+
+func (s fakeStorage) StoreSite(domain string, data *SiteData) error {
+	panic("no impl")
+}
+
+func (s fakeStorage) DeleteSite(domain string) error {
+	panic("no impl")
+}
+
+func (s fakeStorage) LockRegister(domain string) (bool, error) {
+	panic("no impl")
+}
+
+func (s fakeStorage) UnlockRegister(domain string) error {
+	panic("no impl")
+}
+
+func (s fakeStorage) LoadUser(email string) (*UserData, error) {
+	panic("no impl")
+}
+
+func (s fakeStorage) StoreUser(email string, data *UserData) error {
+	panic("no impl")
+}
+
+func (s fakeStorage) MostRecentUserEmail() string {
+	panic("no impl")
+}

caddytls/crypto.go

@@ -14,21 +14,15 @@ import (
 	"errors"
 	"fmt"
 	"io"
-	"io/ioutil"
 	"math/big"
 	"net"
-	"os"
 	"time"
 
 	"github.com/xenolf/lego/acme"
 )
 
-// loadPrivateKey loads a PEM-encoded ECC/RSA private key from file.
+// loadPrivateKey loads a PEM-encoded ECC/RSA private key from an array of bytes.
-func loadPrivateKey(file string) (crypto.PrivateKey, error) {
+func loadPrivateKey(keyBytes []byte) (crypto.PrivateKey, error) {
-	keyBytes, err := ioutil.ReadFile(file)
-	if err != nil {
-		return nil, err
-	}
 	keyBlock, _ := pem.Decode(keyBytes)
 
 	switch keyBlock.Type {
@@ -41,8 +35,8 @@ func loadPrivateKey(file string) (crypto.PrivateKey, error) {
 	return nil, errors.New("unknown private key type")
 }
 
-// savePrivateKey saves a PEM-encoded ECC/RSA private key to file.
+// savePrivateKey saves a PEM-encoded ECC/RSA private key to an array of bytes.
-func savePrivateKey(key crypto.PrivateKey, file string) error {
+func savePrivateKey(key crypto.PrivateKey) ([]byte, error) {
 	var pemType string
 	var keyBytes []byte
 	switch key := key.(type) {
@@ -51,7 +45,7 @@ func savePrivateKey(key crypto.PrivateKey, file string) error {
 		pemType = "EC"
 		keyBytes, err = x509.MarshalECPrivateKey(key)
 		if err != nil {
-			return err
+			return nil, err
 		}
 	case *rsa.PrivateKey:
 		pemType = "RSA"
@@ -59,13 +53,7 @@ func savePrivateKey(key crypto.PrivateKey, file string) error {
 	}
 
 	pemKey := pem.Block{Type: pemType + " PRIVATE KEY", Bytes: keyBytes}
-	keyOut, err := os.Create(file)
+	return pem.EncodeToMemory(&pemKey), nil
-	if err != nil {
-		return err
-	}
-	keyOut.Chmod(0600)
-	defer keyOut.Close()
-	return pem.Encode(keyOut, &pemKey)
 }
 
 // stapleOCSP staples OCSP information to cert for hostname name.

caddytls/crypto_test.go

@@ -9,42 +9,24 @@ import (
 	"crypto/rsa"
 	"crypto/tls"
 	"crypto/x509"
-	"os"
-	"runtime"
 	"testing"
 	"time"
 )
 
 func TestSaveAndLoadRSAPrivateKey(t *testing.T) {
-	keyFile := "test.key"
-	defer os.Remove(keyFile)
-
 	privateKey, err := rsa.GenerateKey(rand.Reader, 128) // make tests faster; small key size OK for testing
 	if err != nil {
 		t.Fatal(err)
 	}
 
 	// test save
-	err = savePrivateKey(privateKey, keyFile)
+	savedBytes, err := savePrivateKey(privateKey)
 	if err != nil {
 		t.Fatal("error saving private key:", err)
 	}
 
-	// it doesn't make sense to test file permission on windows
-	if runtime.GOOS != "windows" {
-		// get info of the key file
-		info, err := os.Stat(keyFile)
-		if err != nil {
-			t.Fatal("error stating private key:", err)
-		}
-		// verify permission of key file is correct
-		if info.Mode().Perm() != 0600 {
-			t.Error("Expected key file to have permission 0600, but it wasn't")
-		}
-	}
-
 	// test load
-	loadedKey, err := loadPrivateKey(keyFile)
+	loadedKey, err := loadPrivateKey(savedBytes)
 	if err != nil {
 		t.Error("error loading private key:", err)
 	}
@@ -56,35 +38,19 @@ func TestSaveAndLoadRSAPrivateKey(t *testing.T) {
 }
 
 func TestSaveAndLoadECCPrivateKey(t *testing.T) {
-	keyFile := "test.key"
-	defer os.Remove(keyFile)
-
 	privateKey, err := ecdsa.GenerateKey(elliptic.P384(), rand.Reader)
 	if err != nil {
 		t.Fatal(err)
 	}
 
 	// test save
-	err = savePrivateKey(privateKey, keyFile)
+	savedBytes, err := savePrivateKey(privateKey)
 	if err != nil {
 		t.Fatal("error saving private key:", err)
 	}
 
-	// it doesn't make sense to test file permission on windows
-	if runtime.GOOS != "windows" {
-		// get info of the key file
-		info, err := os.Stat(keyFile)
-		if err != nil {
-			t.Fatal("error stating private key:", err)
-		}
-		// verify permission of key file is correct
-		if info.Mode().Perm() != 0600 {
-			t.Error("Expected key file to have permission 0600, but it wasn't")
-		}
-	}
-
 	// test load
-	loadedKey, err := loadPrivateKey(keyFile)
+	loadedKey, err := loadPrivateKey(savedBytes)
 	if err != nil {
 		t.Error("error loading private key:", err)
 	}

caddytls/filestorage.go

@@ -0,0 +1,239 @@
+package caddytls
+
+import (
+	"github.com/mholt/caddy"
+	"io/ioutil"
+	"net/url"
+	"os"
+	"path/filepath"
+	"strings"
+)
+
+// storageBasePath is the root path in which all TLS/ACME assets are
+// stored. Do not change this value during the lifetime of the program.
+var storageBasePath = filepath.Join(caddy.AssetsPath(), "acme")
+
+// FileStorageCreator creates a new Storage instance backed by the local
+// disk. The resulting Storage instance is guaranteed to be non-nil if
+// there is no error. This can be used by "middleware" implementations that
+// may want to proxy the disk storage.
+func FileStorageCreator(caURL *url.URL) (Storage, error) {
+	return FileStorage(filepath.Join(storageBasePath, caURL.Host)), nil
+}
+
+// FileStorage is a root directory and facilitates forming file paths derived
+// from it. It is used to get file paths in a consistent, cross- platform way
+// for persisting ACME assets on the file system.
+type FileStorage string
+
+// sites gets the directory that stores site certificate and keys.
+func (s FileStorage) sites() string {
+	return filepath.Join(string(s), "sites")
+}
+
+// site returns the path to the folder containing assets for domain.
+func (s FileStorage) site(domain string) string {
+	domain = strings.ToLower(domain)
+	return filepath.Join(s.sites(), domain)
+}
+
+// siteCertFile returns the path to the certificate file for domain.
+func (s FileStorage) siteCertFile(domain string) string {
+	domain = strings.ToLower(domain)
+	return filepath.Join(s.site(domain), domain+".crt")
+}
+
+// siteKeyFile returns the path to domain's private key file.
+func (s FileStorage) siteKeyFile(domain string) string {
+	domain = strings.ToLower(domain)
+	return filepath.Join(s.site(domain), domain+".key")
+}
+
+// siteMetaFile returns the path to the domain's asset metadata file.
+func (s FileStorage) siteMetaFile(domain string) string {
+	domain = strings.ToLower(domain)
+	return filepath.Join(s.site(domain), domain+".json")
+}
+
+// users gets the directory that stores account folders.
+func (s FileStorage) users() string {
+	return filepath.Join(string(s), "users")
+}
+
+// user gets the account folder for the user with email
+func (s FileStorage) user(email string) string {
+	if email == "" {
+		email = emptyEmail
+	}
+	email = strings.ToLower(email)
+	return filepath.Join(s.users(), email)
+}
+
+// emailUsername returns the username portion of an email address (part before
+// '@') or the original input if it can't find the "@" symbol.
+func emailUsername(email string) string {
+	at := strings.Index(email, "@")
+	if at == -1 {
+		return email
+	} else if at == 0 {
+		return email[1:]
+	}
+	return email[:at]
+}
+
+// userRegFile gets the path to the registration file for the user with the
+// given email address.
+func (s FileStorage) userRegFile(email string) string {
+	if email == "" {
+		email = emptyEmail
+	}
+	email = strings.ToLower(email)
+	fileName := emailUsername(email)
+	if fileName == "" {
+		fileName = "registration"
+	}
+	return filepath.Join(s.user(email), fileName+".json")
+}
+
+// userKeyFile gets the path to the private key file for the user with the
+// given email address.
+func (s FileStorage) userKeyFile(email string) string {
+	if email == "" {
+		email = emptyEmail
+	}
+	email = strings.ToLower(email)
+	fileName := emailUsername(email)
+	if fileName == "" {
+		fileName = "private"
+	}
+	return filepath.Join(s.user(email), fileName+".key")
+}
+
+// readFile abstracts a simple ioutil.ReadFile, making sure to return an
+// ErrStorageNotFound instance when the file is not found.
+func (s FileStorage) readFile(file string) ([]byte, error) {
+	byts, err := ioutil.ReadFile(file)
+	if os.IsNotExist(err) {
+		return nil, ErrStorageNotFound
+	}
+	return byts, err
+}
+
+// SiteExists implements Storage.SiteExists by checking for the presence of
+// cert and key files.
+func (s FileStorage) SiteExists(domain string) bool {
+	_, err := os.Stat(s.siteCertFile(domain))
+	if err != nil {
+		return false
+	}
+	_, err = os.Stat(s.siteKeyFile(domain))
+	if err != nil {
+		return false
+	}
+	return true
+}
+
+// LoadSite implements Storage.LoadSite by loading it from disk. If it is not
+// present, the ErrStorageNotFound error instance is returned.
+func (s FileStorage) LoadSite(domain string) (*SiteData, error) {
+	var err error
+	siteData := new(SiteData)
+	siteData.Cert, err = s.readFile(s.siteCertFile(domain))
+	if err == nil {
+		siteData.Key, err = s.readFile(s.siteKeyFile(domain))
+	}
+	if err == nil {
+		siteData.Meta, err = s.readFile(s.siteMetaFile(domain))
+	}
+	return siteData, err
+}
+
+// StoreSite implements Storage.StoreSite by writing it to disk. The base
+// directories needed for the file are automatically created as needed.
+func (s FileStorage) StoreSite(domain string, data *SiteData) error {
+	err := os.MkdirAll(s.site(domain), 0700)
+	if err != nil {
+		return err
+	}
+	err = ioutil.WriteFile(s.siteCertFile(domain), data.Cert, 0600)
+	if err == nil {
+		err = ioutil.WriteFile(s.siteKeyFile(domain), data.Key, 0600)
+	}
+	if err == nil {
+		err = ioutil.WriteFile(s.siteMetaFile(domain), data.Meta, 0600)
+	}
+	return err
+}
+
+// DeleteSite implements Storage.DeleteSite by deleting just the cert from
+// disk. If it is not present, the ErrStorageNotFound error instance is
+// returned.
+func (s FileStorage) DeleteSite(domain string) error {
+	err := os.Remove(s.siteCertFile(domain))
+	if os.IsNotExist(err) {
+		return ErrStorageNotFound
+	}
+	return err
+}
+
+// LockRegister implements Storage.LockRegister by just returning true because
+// it is not a multi-server storage implementation.
+func (s FileStorage) LockRegister(domain string) (bool, error) {
+	return true, nil
+}
+
+// UnlockRegister implements Storage.UnlockRegister as a no-op because it is
+// not a multi-server storage implementation.
+func (s FileStorage) UnlockRegister(domain string) error {
+	return nil
+}
+
+// LoadUser implements Storage.LoadUser by loading it from disk. If it is not
+// present, the ErrStorageNotFound error instance is returned.
+func (s FileStorage) LoadUser(email string) (*UserData, error) {
+	var err error
+	userData := new(UserData)
+	userData.Reg, err = s.readFile(s.userRegFile(email))
+	if err == nil {
+		userData.Key, err = s.readFile(s.userKeyFile(email))
+	}
+	return userData, err
+}
+
+// StoreUser implements Storage.StoreUser by writing it to disk. The base
+// directories needed for the file are automatically created as needed.
+func (s FileStorage) StoreUser(email string, data *UserData) error {
+	err := os.MkdirAll(s.user(email), 0700)
+	if err != nil {
+		return err
+	}
+	err = ioutil.WriteFile(s.userRegFile(email), data.Reg, 0600)
+	if err == nil {
+		err = ioutil.WriteFile(s.userKeyFile(email), data.Key, 0600)
+	}
+	return err
+}
+
+// MostRecentUserEmail implements Storage.MostRecentUserEmail by finding the
+// most recently written sub directory in the users' directory. It is named
+// after the email address. This corresponds to the most recent call to
+// StoreUser.
+func (s FileStorage) MostRecentUserEmail() string {
+	userDirs, err := ioutil.ReadDir(s.users())
+	if err != nil {
+		return ""
+	}
+	var mostRecent os.FileInfo
+	for _, dir := range userDirs {
+		if !dir.IsDir() {
+			continue
+		}
+		if mostRecent == nil || dir.ModTime().After(mostRecent.ModTime()) {
+			mostRecent = dir
+		}
+	}
+	if mostRecent != nil {
+		return mostRecent.Name()
+	}
+	return ""
+}

caddytls/filestorage_test.go

@@ -0,0 +1,6 @@
+package caddytls
+
+// *********************************** NOTE ********************************
+// Due to circular package dependencies with the storagetest sub package and
+// the fact that we want to use that harness to test file storage, the tests
+// for file storage are done in the storagetest package.

caddytls/maintain.go

@@ -93,7 +93,10 @@ func RenewManagedCertificates(allowPrompts bool) (err error) {
 				continue
 			}
 
-			// this works well because managed certs are only associated with one name per config
+			// This works well because managed certs are only associated with one name per config.
+			// Note, the renewal inside here may not actually occur and no error will be returned
+			// due to renewal lock (i.e. because a renewal is already happening). This lack of
+			// error is by intention to force cache invalidation as though it has renewed.
 			err := cert.Config.RenewCert(allowPrompts)
 
 			if err != nil {

caddytls/storage.go

@@ -1,134 +1,105 @@
 package caddytls
 
 import (
-	"fmt"
+	"errors"
 	"net/url"
-	"path/filepath"
-	"strings"
-
-	"github.com/mholt/caddy"
 )
 
-// StorageFor gets the storage value associated with the
+// ErrStorageNotFound is returned by Storage implementations when data is
-// caURL, which should be unique for every different
+// expected to be present but is not.
-// ACME CA.
+var ErrStorageNotFound = errors.New("data not found")
-func StorageFor(caURL string) (Storage, error) {
-	if caURL == "" {
-		caURL = DefaultCAUrl
-	}
-	if caURL == "" {
-		return "", fmt.Errorf("cannot create storage without CA URL")
-	}
-	caURL = strings.ToLower(caURL)
 
-	// scheme required or host will be parsed as path (as of Go 1.6)
+// StorageCreator is a function type that is used in the Config to instantiate
-	if !strings.Contains(caURL, "://") {
+// a new Storage instance. This function can return a nil Storage even without
-		caURL = "https://" + caURL
+// an error.
-	}
+type StorageCreator func(caURL *url.URL) (Storage, error)
 
-	u, err := url.Parse(caURL)
+// SiteData contains persisted items pertaining to an individual site.
-	if err != nil {
+type SiteData struct {
-		return "", fmt.Errorf("%s: unable to parse CA URL: %v", caURL, err)
+	// Cert is the public cert byte array.
-	}
+	Cert []byte
-
+	// Key is the private key byte array.
-	if u.Host == "" {
+	Key []byte
-		return "", fmt.Errorf("%s: no host in CA URL", caURL)
+	// Meta is metadata about the site used by Caddy.
-	}
+	Meta []byte
-
-	return Storage(filepath.Join(storageBasePath, u.Host)), nil
 }
 
-// Storage is a root directory and facilitates
+// UserData contains persisted items pertaining to a user.
-// forming file paths derived from it. It is used
+type UserData struct {
-// to get file paths in a consistent, cross-
+	// Reg is the user registration byte array.
-// platform way for persisting ACME assets.
+	Reg []byte
-// on the file system.
+	// Key is the user key byte array.
-type Storage string
+	Key []byte
-
-// Sites gets the directory that stores site certificate and keys.
-func (s Storage) Sites() string {
-	return filepath.Join(string(s), "sites")
 }
 
-// Site returns the path to the folder containing assets for domain.
+// Storage is an interface abstracting all storage used by the Caddy's TLS
-func (s Storage) Site(domain string) string {
+// subsystem. Implementations of this interface store site data along with
-	domain = strings.ToLower(domain)
+// user data.
-	return filepath.Join(s.Sites(), domain)
+type Storage interface {
-}
 
-// SiteCertFile returns the path to the certificate file for domain.
+	// SiteDataExists returns true if this site info exists in storage.
-func (s Storage) SiteCertFile(domain string) string {
+	// Site data is considered present when StoreSite has been called
-	domain = strings.ToLower(domain)
+	// successfully (without DeleteSite having been called of course).
-	return filepath.Join(s.Site(domain), domain+".crt")
+	SiteExists(domain string) bool
-}
 
-// SiteKeyFile returns the path to domain's private key file.
+	// LoadSite obtains the site data from storage for the given domain and
-func (s Storage) SiteKeyFile(domain string) string {
+	// returns. If data for the domain does not exist, the
-	domain = strings.ToLower(domain)
+	// ErrStorageNotFound error instance is returned. For multi-server
-	return filepath.Join(s.Site(domain), domain+".key")
+	// storage, care should be taken to make this load atomic to prevent
-}
+	// race conditions that happen with multiple data loads.
+	LoadSite(domain string) (*SiteData, error)
 
-// SiteMetaFile returns the path to the domain's asset metadata file.
+	// StoreSite persists the given site data for the given domain in
-func (s Storage) SiteMetaFile(domain string) string {
+	// storage. For multi-server storage, care should be taken to make this
-	domain = strings.ToLower(domain)
+	// call atomic to prevent half-written data on failure of an internal
-	return filepath.Join(s.Site(domain), domain+".json")
+	// intermediate storage step. Implementers can trust that at runtime
-}
+	// this function will only be invoked after LockRegister and before
+	// UnlockRegister of the same domain.
+	StoreSite(domain string, data *SiteData) error
 
-// Users gets the directory that stores account folders.
+	// DeleteSite deletes the site for the given domain from storage.
-func (s Storage) Users() string {
+	// Multi-server implementations should attempt to make this atomic. If
-	return filepath.Join(string(s), "users")
+	// the site does not exist, the ErrStorageNotFound error instance is
-}
+	// returned.
+	DeleteSite(domain string) error
 
-// User gets the account folder for the user with email.
+	// LockRegister is called before Caddy attempts to obtain or renew a
-func (s Storage) User(email string) string {
+	// certificate. This function is used as a mutex/semaphore for making
-	if email == "" {
+	// sure something else isn't already attempting obtain/renew. It should
-		email = emptyEmail
+	// return true (without error) if the lock is successfully obtained
-	}
+	// meaning nothing else is attempting renewal. It should return false
-	email = strings.ToLower(email)
+	// (without error) if this domain is already locked by something else
-	return filepath.Join(s.Users(), email)
+	// attempting renewal. As a general rule, if this isn't multi-server
-}
+	// shared storage, this should always return true. To prevent deadlocks
+	// for multi-server storage, all internal implementations should put a
+	// reasonable expiration on this lock in case UnlockRegister is unable to
+	// be called due to system crash. Errors should only be returned in
+	// exceptional cases. Any error will prevent renewal.
+	LockRegister(domain string) (bool, error)
 
-// UserRegFile gets the path to the registration file for
+	// UnlockRegister is called after Caddy has attempted to obtain or renew
-// the user with the given email address.
+	// a certificate, regardless of whether it was successful. If
-func (s Storage) UserRegFile(email string) string {
+	// LockRegister essentially just returns true because this is not
-	if email == "" {
+	// multi-server storage, this can be a no-op. Otherwise this should
-		email = emptyEmail
+	// attempt to unlock the lock obtained in this process by LockRegister.
-	}
+	// If no lock exists, the implementation should not return an error. An
-	email = strings.ToLower(email)
+	// error is only for exceptional cases.
-	fileName := emailUsername(email)
+	UnlockRegister(domain string) error
-	if fileName == "" {
-		fileName = "registration"
-	}
-	return filepath.Join(s.User(email), fileName+".json")
-}
 
-// UserKeyFile gets the path to the private key file for
+	// LoadUser obtains user data from storage for the given email and
-// the user with the given email address.
+	// returns it. If data for the email does not exist, the
-func (s Storage) UserKeyFile(email string) string {
+	// ErrStorageNotFound error instance is returned. Multi-server
-	if email == "" {
+	// implementations should take care to make this operation atomic for
-		email = emptyEmail
+	// all loaded data items.
-	}
+	LoadUser(email string) (*UserData, error)
-	email = strings.ToLower(email)
-	fileName := emailUsername(email)
-	if fileName == "" {
-		fileName = "private"
-	}
-	return filepath.Join(s.User(email), fileName+".key")
-}
 
-// emailUsername returns the username portion of an
+	// StoreUser persists the given user data for the given email in
-// email address (part before '@') or the original
+	// storage. Multi-server implementations should take care to make this
-// input if it can't find the "@" symbol.
+	// operation atomic for all stored data items.
-func emailUsername(email string) string {
+	StoreUser(email string, data *UserData) error
-	at := strings.Index(email, "@")
-	if at == -1 {
-		return email
-	} else if at == 0 {
-		return email[1:]
-	}
-	return email[:at]
-}
 
-// storageBasePath is the root path in which all TLS/ACME assets are
+	// MostRecentUserEmail provides the most recently used email parameter
-//  stored. Do not change this value during the lifetime of the program.
+	// in StoreUser. The result is an empty string if there are no
-var storageBasePath = filepath.Join(caddy.AssetsPath(), "acme")
+	// persisted users in storage.
+	MostRecentUserEmail() string
+}

caddytls/storage_test.go

@@ -1,135 +0,0 @@
-package caddytls
-
-import (
-	"path/filepath"
-	"testing"
-)
-
-func TestStorageFor(t *testing.T) {
-	// first try without DefaultCAUrl set
-	DefaultCAUrl = ""
-	_, err := StorageFor("")
-	if err == nil {
-		t.Errorf("Without a default CA, expected error, but didn't get one")
-	}
-	st, err := StorageFor("https://example.com/foo")
-	if err != nil {
-		t.Errorf("Without a default CA but given input, expected no error, but got: %v", err)
-	}
-	if string(st) != filepath.Join(storageBasePath, "example.com") {
-		t.Errorf("Without a default CA but given input, expected '%s' not '%s'", "example.com", st)
-	}
-
-	// try with the DefaultCAUrl set
-	DefaultCAUrl = "https://defaultCA/directory"
-	for i, test := range []struct {
-		input, expect string
-		shouldErr     bool
-	}{
-		{"https://acme-staging.api.letsencrypt.org/directory", "acme-staging.api.letsencrypt.org", false},
-		{"https://foo/boo?bar=q", "foo", false},
-		{"http://foo", "foo", false},
-		{"", "defaultca", false},
-		{"https://FooBar/asdf", "foobar", false},
-		{"noscheme/path", "noscheme", false},
-		{"/nohost", "", true},
-		{"https:///nohost", "", true},
-		{"FooBar", "foobar", false},
-	} {
-		st, err := StorageFor(test.input)
-		if err == nil && test.shouldErr {
-			t.Errorf("Test %d: Expected an error, but didn't get one", i)
-		} else if err != nil && !test.shouldErr {
-			t.Errorf("Test %d: Expected no errors, but got: %v", i, err)
-		}
-		want := filepath.Join(storageBasePath, test.expect)
-		if test.shouldErr {
-			want = ""
-		}
-		if string(st) != want {
-			t.Errorf("Test %d: Expected '%s' but got '%s'", i, want, string(st))
-		}
-	}
-}
-
-func TestStorage(t *testing.T) {
-	storage := Storage("./le_test")
-
-	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("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("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("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("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("le_test", "users"), storage.Users(); actual != expected {
-		t.Errorf("Expected Users() to return '%s' but got '%s'", expected, actual)
-	}
-	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("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("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("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("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("le_test", "users", emptyEmail, emptyEmail+".key"), storage.UserKeyFile(""); actual != expected {
-		t.Errorf("Expected UserKeyFile(\"\") to return '%s' but got '%s'", expected, actual)
-	}
-}
-
-func TestEmailUsername(t *testing.T) {
-	for i, test := range []struct {
-		input, expect string
-	}{
-		{
-			input:  "username@example.com",
-			expect: "username",
-		},
-		{
-			input:  "plus+addressing@example.com",
-			expect: "plus+addressing",
-		},
-		{
-			input:  "me+plus-addressing@example.com",
-			expect: "me+plus-addressing",
-		},
-		{
-			input:  "not-an-email",
-			expect: "not-an-email",
-		},
-		{
-			input:  "@foobar.com",
-			expect: "foobar.com",
-		},
-		{
-			input:  emptyEmail,
-			expect: emptyEmail,
-		},
-		{
-			input:  "",
-			expect: "",
-		},
-	} {
-		if actual := emailUsername(test.input); actual != test.expect {
-			t.Errorf("Test %d: Expected username to be '%s' but was '%s'", i, test.expect, actual)
-		}
-	}
-}

caddytls/storagetest/memorystorage.go

@@ -0,0 +1,132 @@
+package storagetest
+
+import (
+	"github.com/mholt/caddy/caddytls"
+	"net/url"
+	"sync"
+)
+
+// memoryMutex is a mutex used to control access to memoryStoragesByCAURL.
+var memoryMutex sync.Mutex
+
+// memoryStoragesByCAURL is a map keyed by a CA URL string with values of
+// instantiated memory stores. Do not access this directly, it is used by
+// InMemoryStorageCreator.
+var memoryStoragesByCAURL = make(map[string]*InMemoryStorage)
+
+// InMemoryStorageCreator is a caddytls.Storage.StorageCreator to create
+// InMemoryStorage instances for testing.
+func InMemoryStorageCreator(caURL *url.URL) (caddytls.Storage, error) {
+	urlStr := caURL.String()
+	memoryMutex.Lock()
+	defer memoryMutex.Unlock()
+	storage := memoryStoragesByCAURL[urlStr]
+	if storage == nil {
+		storage = NewInMemoryStorage()
+		memoryStoragesByCAURL[urlStr] = storage
+	}
+	return storage, nil
+}
+
+// InMemoryStorage is a caddytls.Storage implementation for use in testing.
+// It simply stores information in runtime memory.
+type InMemoryStorage struct {
+	// Sites are exposed for testing purposes.
+	Sites map[string]*caddytls.SiteData
+	// Users are exposed for testing purposes.
+	Users map[string]*caddytls.UserData
+	// LastUserEmail is exposed for testing purposes.
+	LastUserEmail string
+}
+
+// NewInMemoryStorage constructs an InMemoryStorage instance. For use with
+// caddytls, the InMemoryStorageCreator should be used instead.
+func NewInMemoryStorage() *InMemoryStorage {
+	return &InMemoryStorage{
+		Sites: make(map[string]*caddytls.SiteData),
+		Users: make(map[string]*caddytls.UserData),
+	}
+}
+
+// SiteExists implements caddytls.Storage.SiteExists in memory.
+func (s *InMemoryStorage) SiteExists(domain string) bool {
+	_, siteExists := s.Sites[domain]
+	return siteExists
+}
+
+// Clear completely clears all values associated with this storage.
+func (s *InMemoryStorage) Clear() {
+	s.Sites = make(map[string]*caddytls.SiteData)
+	s.Users = make(map[string]*caddytls.UserData)
+	s.LastUserEmail = ""
+}
+
+// LoadSite implements caddytls.Storage.LoadSite in memory.
+func (s *InMemoryStorage) LoadSite(domain string) (*caddytls.SiteData, error) {
+	siteData, ok := s.Sites[domain]
+	if !ok {
+		return nil, caddytls.ErrStorageNotFound
+	}
+	return siteData, nil
+}
+
+func copyBytes(from []byte) []byte {
+	copiedBytes := make([]byte, len(from))
+	copy(copiedBytes, from)
+	return copiedBytes
+}
+
+// StoreSite implements caddytls.Storage.StoreSite in memory.
+func (s *InMemoryStorage) StoreSite(domain string, data *caddytls.SiteData) error {
+	copiedData := new(caddytls.SiteData)
+	copiedData.Cert = copyBytes(data.Cert)
+	copiedData.Key = copyBytes(data.Key)
+	copiedData.Meta = copyBytes(data.Meta)
+	s.Sites[domain] = copiedData
+	return nil
+}
+
+// DeleteSite implements caddytls.Storage.DeleteSite in memory.
+func (s *InMemoryStorage) DeleteSite(domain string) error {
+	if _, ok := s.Sites[domain]; !ok {
+		return caddytls.ErrStorageNotFound
+	}
+	delete(s.Sites, domain)
+	return nil
+}
+
+// LockRegister implements Storage.LockRegister by just returning true because
+// it is not a multi-server storage implementation.
+func (s *InMemoryStorage) LockRegister(domain string) (bool, error) {
+	return true, nil
+}
+
+// UnlockRegister implements Storage.UnlockRegister as a no-op because it is
+// not a multi-server storage implementation.
+func (s *InMemoryStorage) UnlockRegister(domain string) error {
+	return nil
+}
+
+// LoadUser implements caddytls.Storage.LoadUser in memory.
+func (s *InMemoryStorage) LoadUser(email string) (*caddytls.UserData, error) {
+	userData, ok := s.Users[email]
+	if !ok {
+		return nil, caddytls.ErrStorageNotFound
+	}
+	return userData, nil
+}
+
+// StoreUser implements caddytls.Storage.StoreUser in memory.
+func (s *InMemoryStorage) StoreUser(email string, data *caddytls.UserData) error {
+	copiedData := new(caddytls.UserData)
+	copiedData.Reg = copyBytes(data.Reg)
+	copiedData.Key = copyBytes(data.Key)
+	s.Users[email] = copiedData
+	s.LastUserEmail = email
+	return nil
+}
+
+// MostRecentUserEmail implements caddytls.Storage.MostRecentUserEmail in memory.
+func (s *InMemoryStorage) MostRecentUserEmail() string {
+	return s.LastUserEmail
+}

caddytls/storagetest/memorystorage_test.go

@@ -0,0 +1,12 @@
+package storagetest
+
+import "testing"
+
+func TestMemoryStorage(t *testing.T) {
+	storage := NewInMemoryStorage()
+	storageTest := &StorageTest{
+		Storage:  storage,
+		PostTest: storage.Clear,
+	}
+	storageTest.Test(t, false)
+}

caddytls/storagetest/storagetest.go

@@ -0,0 +1,270 @@
+// Package storagetest provides utilities to assist in testing caddytls.Storage
+// implementations.
+package storagetest
+
+import (
+	"bytes"
+	"errors"
+	"fmt"
+	"github.com/mholt/caddy/caddytls"
+	"testing"
+)
+
+// StorageTest is a test harness that contains tests to execute all exposed
+// parts of a Storage implementation.
+type StorageTest struct {
+	// Storage is the implementation to use during tests. This must be
+	// present.
+	caddytls.Storage
+
+	// PreTest, if present, is called before every test. Any error returned
+	// is returned from the test and the test does not continue.
+	PreTest func() error
+
+	// PostTest, if present, is executed after every test via defer which
+	// means it executes even on failure of the test (but not on failure of
+	// PreTest).
+	PostTest func()
+
+	// AfterUserEmailStore, if present, is invoked during
+	// TestMostRecentUserEmail after each storage just in case anything
+	// needs to be mocked.
+	AfterUserEmailStore func(email string) error
+}
+
+// TestFunc holds information about a test.
+type TestFunc struct {
+	// Name is the friendly name of the test.
+	Name string
+
+	// Fn is the function that is invoked for the test.
+	Fn func() error
+}
+
+// runPreTest runs the PreTest function if present.
+func (s *StorageTest) runPreTest() error {
+	if s.PreTest != nil {
+		return s.PreTest()
+	}
+	return nil
+}
+
+// runPostTest runs the PostTest function if present.
+func (s *StorageTest) runPostTest() {
+	if s.PostTest != nil {
+		s.PostTest()
+	}
+}
+
+// AllFuncs returns all test functions that are part of this harness.
+func (s *StorageTest) AllFuncs() []TestFunc {
+	return []TestFunc{
+		{"TestSiteInfoExists", s.TestSiteExists},
+		{"TestSite", s.TestSite},
+		{"TestUser", s.TestUser},
+		{"TestMostRecentUserEmail", s.TestMostRecentUserEmail},
+	}
+}
+
+// Test executes the entire harness using the testing package. Failures are
+// reported via T.Fatal. If eagerFail is true, the first failure causes all
+// testing to stop immediately.
+func (s *StorageTest) Test(t *testing.T, eagerFail bool) {
+	if errs := s.TestAll(eagerFail); len(errs) > 0 {
+		ifaces := make([]interface{}, len(errs))
+		for i, err := range errs {
+			ifaces[i] = err
+		}
+		t.Fatal(ifaces...)
+	}
+}
+
+// TestAll executes the entire harness and returns the results as an array of
+// errors. If eagerFail is true, the first failure causes all testing to stop
+// immediately.
+func (s *StorageTest) TestAll(eagerFail bool) (errs []error) {
+	for _, fn := range s.AllFuncs() {
+		if err := fn.Fn(); err != nil {
+			errs = append(errs, fmt.Errorf("%v failed: %v", fn.Name, err))
+			if eagerFail {
+				return
+			}
+		}
+	}
+	return
+}
+
+var simpleSiteData = &caddytls.SiteData{
+	Cert: []byte("foo"),
+	Key:  []byte("bar"),
+	Meta: []byte("baz"),
+}
+var simpleSiteDataAlt = &caddytls.SiteData{
+	Cert: []byte("qux"),
+	Key:  []byte("quux"),
+	Meta: []byte("corge"),
+}
+
+// TestSiteExists tests Storage.SiteExists.
+func (s *StorageTest) TestSiteExists() error {
+	if err := s.runPreTest(); err != nil {
+		return err
+	}
+	defer s.runPostTest()
+
+	// Should not exist at first
+	if s.SiteExists("example.com") {
+		return errors.New("Site should not exist")
+	}
+
+	// Should exist after we store it
+	if err := s.StoreSite("example.com", simpleSiteData); err != nil {
+		return err
+	}
+	if !s.SiteExists("example.com") {
+		return errors.New("Expected site to exist")
+	}
+
+	// Site should no longer exist after we delete it
+	if err := s.DeleteSite("example.com"); err != nil {
+		return err
+	}
+	if s.SiteExists("example.com") {
+		return errors.New("Site should not exist after delete")
+	}
+	return nil
+}
+
+// TestSite tests Storage.LoadSite, Storage.StoreSite, and Storage.DeleteSite.
+func (s *StorageTest) TestSite() error {
+	if err := s.runPreTest(); err != nil {
+		return err
+	}
+	defer s.runPostTest()
+
+	// Should be a not-found error at first
+	if _, err := s.LoadSite("example.com"); err != caddytls.ErrStorageNotFound {
+		return fmt.Errorf("Expected ErrStorageNotFound from load, got: %v", err)
+	}
+
+	// Delete should also be a not-found error at first
+	if err := s.DeleteSite("example.com"); err != caddytls.ErrStorageNotFound {
+		return fmt.Errorf("Expected ErrStorageNotFound from delete, got: %v", err)
+	}
+
+	// Should store successfully and then load just fine
+	if err := s.StoreSite("example.com", simpleSiteData); err != nil {
+		return err
+	}
+	if siteData, err := s.LoadSite("example.com"); err != nil {
+		return err
+	} else if !bytes.Equal(siteData.Cert, simpleSiteData.Cert) {
+		return errors.New("Unexpected cert returned after store")
+	} else if !bytes.Equal(siteData.Key, simpleSiteData.Key) {
+		return errors.New("Unexpected key returned after store")
+	} else if !bytes.Equal(siteData.Meta, simpleSiteData.Meta) {
+		return errors.New("Unexpected meta returned after store")
+	}
+
+	// Overwrite should work just fine
+	if err := s.StoreSite("example.com", simpleSiteDataAlt); err != nil {
+		return err
+	}
+	if siteData, err := s.LoadSite("example.com"); err != nil {
+		return err
+	} else if !bytes.Equal(siteData.Cert, simpleSiteDataAlt.Cert) {
+		return errors.New("Unexpected cert returned after overwrite")
+	}
+
+	// It should delete fine and then not be there
+	if err := s.DeleteSite("example.com"); err != nil {
+		return err
+	}
+	if _, err := s.LoadSite("example.com"); err != caddytls.ErrStorageNotFound {
+		return fmt.Errorf("Expected ErrStorageNotFound after delete, got: %v", err)
+	}
+
+	return nil
+}
+
+var simpleUserData = &caddytls.UserData{
+	Reg: []byte("foo"),
+	Key: []byte("bar"),
+}
+var simpleUserDataAlt = &caddytls.UserData{
+	Reg: []byte("baz"),
+	Key: []byte("qux"),
+}
+
+// TestUser tests Storage.LoadUser and Storage.StoreUser.
+func (s *StorageTest) TestUser() error {
+	if err := s.runPreTest(); err != nil {
+		return err
+	}
+	defer s.runPostTest()
+
+	// Should be a not-found error at first
+	if _, err := s.LoadUser("foo@example.com"); err != caddytls.ErrStorageNotFound {
+		return fmt.Errorf("Expected ErrStorageNotFound from load, got: %v", err)
+	}
+
+	// Should store successfully and then load just fine
+	if err := s.StoreUser("foo@example.com", simpleUserData); err != nil {
+		return err
+	}
+	if userData, err := s.LoadUser("foo@example.com"); err != nil {
+		return err
+	} else if !bytes.Equal(userData.Reg, simpleUserData.Reg) {
+		return errors.New("Unexpected reg returned after store")
+	} else if !bytes.Equal(userData.Key, simpleUserData.Key) {
+		return errors.New("Unexpected key returned after store")
+	}
+
+	// Overwrite should work just fine
+	if err := s.StoreUser("foo@example.com", simpleUserDataAlt); err != nil {
+		return err
+	}
+	if userData, err := s.LoadUser("foo@example.com"); err != nil {
+		return err
+	} else if !bytes.Equal(userData.Reg, simpleUserDataAlt.Reg) {
+		return errors.New("Unexpected reg returned after overwrite")
+	}
+
+	return nil
+}
+
+// TestMostRecentUserEmail tests Storage.MostRecentUserEmail.
+func (s *StorageTest) TestMostRecentUserEmail() error {
+	if err := s.runPreTest(); err != nil {
+		return err
+	}
+	defer s.runPostTest()
+
+	// Should be empty on first run
+	if e := s.MostRecentUserEmail(); e != "" {
+		return fmt.Errorf("Expected empty most recent user on first run, got: %v", e)
+	}
+
+	// If we store user, then that one should be returned
+	if err := s.StoreUser("foo1@example.com", simpleUserData); err != nil {
+		return err
+	}
+	if s.AfterUserEmailStore != nil {
+		s.AfterUserEmailStore("foo1@example.com")
+	}
+	if e := s.MostRecentUserEmail(); e != "foo1@example.com" {
+		return fmt.Errorf("Unexpected most recent email after first store: %v", e)
+	}
+
+	// If we store another user, then that one should be returned
+	if err := s.StoreUser("foo2@example.com", simpleUserDataAlt); err != nil {
+		return err
+	}
+	if s.AfterUserEmailStore != nil {
+		s.AfterUserEmailStore("foo2@example.com")
+	}
+	if e := s.MostRecentUserEmail(); e != "foo2@example.com" {
+		return fmt.Errorf("Unexpected most recent email after user key: %v", e)
+	}
+	return nil
+}

caddytls/storagetest/storagetest_test.go

@@ -0,0 +1,39 @@
+package storagetest
+
+import (
+	"fmt"
+	"github.com/mholt/caddy/caddytls"
+	"os"
+	"path/filepath"
+	"testing"
+	"time"
+)
+
+// TestFileStorage tests the file storage set with the test harness in this
+// package.
+func TestFileStorage(t *testing.T) {
+	emailCounter := 0
+	storageTest := &StorageTest{
+		Storage:  caddytls.FileStorage("./testdata"),
+		PostTest: func() { os.RemoveAll("./testdata") },
+		AfterUserEmailStore: func(email string) error {
+			// We need to change the dir mod time to show a
+			// that certain dirs are newer.
+			emailCounter++
+			fp := filepath.Join("./testdata", "users", email)
+
+			// What we will do is subtract 10 days from today and
+			// then add counter * seconds to make the later
+			// counters newer. We accept that this isn't exactly
+			// how the file storage works because it only changes
+			// timestamps on *newly seen* users, but it achieves
+			// the result that the harness expects.
+			chTime := time.Now().AddDate(0, 0, -10).Add(time.Duration(emailCounter) * time.Second)
+			if err := os.Chtimes(fp, chTime, chTime); err != nil {
+				return fmt.Errorf("Unable to change file time for %v: %v", fp, err)
+			}
+			return nil
+		},
+	}
+	storageTest.Test(t, false)
+}

caddytls/tls.go

@@ -16,9 +16,7 @@ package caddytls
 
 import (
 	"encoding/json"
-	"io/ioutil"
 	"net"
-	"os"
 	"strings"
 
 	"github.com/xenolf/lego/acme"
@@ -47,53 +45,21 @@ func HostQualifies(hostname string) bool {
 		net.ParseIP(hostname) == nil
 }
 
-// existingCertAndKey returns true if the hostname has
-// a certificate and private key in storage already under
-// the storage provided, otherwise it returns false.
-func existingCertAndKey(storage Storage, hostname string) bool {
-	_, err := os.Stat(storage.SiteCertFile(hostname))
-	if err != nil {
-		return false
-	}
-	_, err = os.Stat(storage.SiteKeyFile(hostname))
-	if err != nil {
-		return false
-	}
-	return true
-}
-
 // saveCertResource saves the certificate resource to disk. This
 // includes the certificate file itself, the private key, and the
 // metadata file.
 func saveCertResource(storage Storage, cert acme.CertificateResource) error {
-	err := os.MkdirAll(storage.Site(cert.Domain), 0700)
+	// Save cert, private key, and metadata
-	if err != nil {
+	siteData := &SiteData{
-		return err
+		Cert: cert.Certificate,
+		Key:  cert.PrivateKey,
 	}
-
+	var err error
-	// Save cert
+	siteData.Meta, err = json.MarshalIndent(&cert, "", "\t")
-	err = ioutil.WriteFile(storage.SiteCertFile(cert.Domain), cert.Certificate, 0600)
+	if err == nil {
-	if err != nil {
+		err = storage.StoreSite(cert.Domain, siteData)
-		return err
 	}
-
+	return err
-	// Save private key
-	err = ioutil.WriteFile(storage.SiteKeyFile(cert.Domain), cert.PrivateKey, 0600)
-	if err != nil {
-		return err
-	}
-
-	// Save cert metadata
-	jsonBytes, err := json.MarshalIndent(&cert, "", "\t")
-	if err != nil {
-		return err
-	}
-	err = ioutil.WriteFile(storage.SiteMetaFile(cert.Domain), jsonBytes, 0600)
-	if err != nil {
-		return err
-	}
-
-	return nil
 }
 
 // Revoke revokes the certificate for host via ACME protocol.

caddytls/tls_test.go

@@ -1,7 +1,6 @@
 package caddytls
 
 import (
-	"io/ioutil"
 	"os"
 	"testing"
 
@@ -80,7 +79,7 @@ func TestQualifiesForManagedTLS(t *testing.T) {
 }
 
 func TestSaveCertResource(t *testing.T) {
-	storage := Storage("./le_test_save")
+	storage := FileStorage("./le_test_save")
 	defer func() {
 		err := os.RemoveAll(string(storage))
 		if err != nil {
@@ -110,33 +109,23 @@ func TestSaveCertResource(t *testing.T) {
 		t.Fatalf("Expected no error, got: %v", err)
 	}
 
-	certFile, err := ioutil.ReadFile(storage.SiteCertFile(domain))
+	siteData, err := storage.LoadSite(domain)
 	if err != nil {
-		t.Errorf("Expected no error reading certificate file, got: %v", err)
+		t.Errorf("Expected no error reading site, got: %v", err)
 	}
-	if string(certFile) != certContents {
+	if string(siteData.Cert) != certContents {
-		t.Errorf("Expected certificate file to contain '%s', got '%s'", certContents, string(certFile))
+		t.Errorf("Expected certificate file to contain '%s', got '%s'", certContents, string(siteData.Cert))
 	}
-
+	if string(siteData.Key) != keyContents {
-	keyFile, err := ioutil.ReadFile(storage.SiteKeyFile(domain))
+		t.Errorf("Expected private key file to contain '%s', got '%s'", keyContents, string(siteData.Key))
-	if err != nil {
-		t.Errorf("Expected no error reading private key file, got: %v", err)
 	}
-	if string(keyFile) != keyContents {
+	if string(siteData.Meta) != metaContents {
-		t.Errorf("Expected private key file to contain '%s', got '%s'", keyContents, string(keyFile))
+		t.Errorf("Expected meta file to contain '%s', got '%s'", metaContents, string(siteData.Meta))
-	}
-
-	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))
 	}
 }
 
 func TestExistingCertAndKey(t *testing.T) {
-	storage := Storage("./le_test_existing")
+	storage := FileStorage("./le_test_existing")
 	defer func() {
 		err := os.RemoveAll(string(storage))
 		if err != nil {
@@ -146,7 +135,7 @@ func TestExistingCertAndKey(t *testing.T) {
 
 	domain := "example.com"
 
-	if existingCertAndKey(storage, domain) {
+	if storage.SiteExists(domain) {
 		t.Errorf("Did NOT expect %v to have existing cert or key, but it did", domain)
 	}
 
@@ -159,7 +148,7 @@ func TestExistingCertAndKey(t *testing.T) {
 		t.Fatalf("Expected no error, got: %v", err)
 	}
 
-	if !existingCertAndKey(storage, domain) {
+	if !storage.SiteExists(domain) {
 		t.Errorf("Expected %v to have existing cert and key, but it did NOT", domain)
 	}
 }

caddytls/user.go

@@ -10,7 +10,6 @@ import (
 	"errors"
 	"fmt"
 	"io"
-	"io/ioutil"
 	"os"
 	"strings"
 
@@ -67,20 +66,9 @@ func getEmail(storage Storage, userPresent bool) string {
 	leEmail := DefaultEmail
 	if leEmail == "" {
 		// Then try to get most recent user email
-		userDirs, err := ioutil.ReadDir(storage.Users())
+		leEmail = storage.MostRecentUserEmail()
-		if err == nil {
+		// Save for next time
-			var mostRecent os.FileInfo
+		DefaultEmail = leEmail
-			for _, dir := range userDirs {
-				if !dir.IsDir() {
-					continue
-				}
-				if mostRecent == nil || dir.ModTime().After(mostRecent.ModTime()) {
-					leEmail = dir.Name()
-					DefaultEmail = leEmail // save for next time
-					mostRecent = dir
-				}
-			}
-		}
 	}
 	if leEmail == "" && userPresent {
 		// Alas, we must bother the user and ask for an email address;
@@ -112,25 +100,24 @@ func getEmail(storage Storage, userPresent bool) string {
 func getUser(storage Storage, email string) (User, error) {
 	var user User
 
-	// open user file
+	// open user reg
-	regFile, err := os.Open(storage.UserRegFile(email))
+	userData, err := storage.LoadUser(email)
 	if err != nil {
-		if os.IsNotExist(err) {
+		if err == ErrStorageNotFound {
 			// create a new user
 			return newUser(email)
 		}
 		return user, err
 	}
-	defer regFile.Close()
 
 	// load user information
-	err = json.NewDecoder(regFile).Decode(&user)
+	err = json.Unmarshal(userData.Reg, &user)
 	if err != nil {
 		return user, err
 	}
 
 	// load their private key
-	user.key, err = loadPrivateKey(storage.UserKeyFile(email))
+	user.key, err = loadPrivateKey(userData.Key)
 	if err != nil {
 		return user, err
 	}
@@ -144,25 +131,17 @@ func getUser(storage Storage, email string) (User, error) {
 // wherein the user should be saved. It should be the storage
 // for the CA with which user has an account.
 func saveUser(storage Storage, user User) error {
-	// make user account folder
+	// Save the private key and registration
-	err := os.MkdirAll(storage.User(user.Email), 0700)
+	userData := new(UserData)
-	if err != nil {
+	var err error
-		return err
+	userData.Key, err = savePrivateKey(user.key)
+	if err == nil {
+		userData.Reg, err = json.MarshalIndent(&user, "", "\t")
 	}
-
+	if err == nil {
-	// save private key file
+		err = storage.StoreUser(user.Email, userData)
-	err = savePrivateKey(user.key, storage.UserKeyFile(user.Email))
-	if err != nil {
-		return err
 	}
-
+	return err
-	// save registration file
-	jsonBytes, err := json.MarshalIndent(&user, "", "\t")
-	if err != nil {
-		return err
-	}
-
-	return ioutil.WriteFile(storage.UserRegFile(user.Email), jsonBytes, 0600)
 }
 
 // promptUserAgreement prompts the user to agree to the agreement

caddytls/user_test.go

@@ -5,15 +5,17 @@ import (
 	"crypto/rand"
 	"crypto/rsa"
 	"io"
-	"os"
 	"strings"
 	"testing"
 	"time"
 
 	"github.com/xenolf/lego/acme"
+	"os"
 )
 
 func TestUser(t *testing.T) {
+	defer testStorage.clean()
+
 	privateKey, err := rsa.GenerateKey(rand.Reader, 128)
 	if err != nil {
 		t.Fatalf("Could not generate test private key: %v", err)
@@ -53,7 +55,7 @@ func TestNewUser(t *testing.T) {
 }
 
 func TestSaveUser(t *testing.T) {
-	defer os.RemoveAll(string(testStorage))
+	defer testStorage.clean()
 
 	email := "me@foobar.com"
 	user, err := newUser(email)
@@ -65,18 +67,14 @@ func TestSaveUser(t *testing.T) {
 	if err != nil {
 		t.Fatalf("Error saving user: %v", err)
 	}
-	_, err = os.Stat(testStorage.UserRegFile(email))
+	_, err = testStorage.LoadUser(email)
 	if err != nil {
-		t.Errorf("Cannot access user registration file, error: %v", err)
+		t.Errorf("Cannot access user data, error: %v", err)
-	}
-	_, err = os.Stat(testStorage.UserKeyFile(email))
-	if err != nil {
-		t.Errorf("Cannot access user private key file, error: %v", err)
 	}
 }
 
 func TestGetUserDoesNotAlreadyExist(t *testing.T) {
-	defer os.RemoveAll(string(testStorage))
+	defer testStorage.clean()
 
 	user, err := getUser(testStorage, "user_does_not_exist@foobar.com")
 	if err != nil {
@@ -89,7 +87,7 @@ func TestGetUserDoesNotAlreadyExist(t *testing.T) {
 }
 
 func TestGetUserAlreadyExists(t *testing.T) {
-	defer os.RemoveAll(string(testStorage))
+	defer testStorage.clean()
 
 	email := "me@foobar.com"
 
@@ -128,7 +126,7 @@ func TestGetEmail(t *testing.T) {
 	os.Stdout = nil
 	defer func() { os.Stdout = origStdout }()
 
-	defer os.RemoveAll(string(testStorage))
+	defer testStorage.clean()
 	DefaultEmail = "test2@foo.com"
 
 	// Test1: Use default email from flag (or user previously typing it)
@@ -166,12 +164,12 @@ func TestGetEmail(t *testing.T) {
 		}
 
 		// Change modified time so they're all different, so the test becomes deterministic
-		f, err := os.Stat(testStorage.User(eml))
+		f, err := os.Stat(testStorage.user(eml))
 		if err != nil {
 			t.Fatalf("Could not access user folder for '%s': %v", eml, err)
 		}
 		chTime := f.ModTime().Add(-(time.Duration(i) * time.Second))
-		if err := os.Chtimes(testStorage.User(eml), chTime, chTime); err != nil {
+		if err := os.Chtimes(testStorage.user(eml), chTime, chTime); err != nil {
 			t.Fatalf("Could not change user folder mod time for '%s': %v", eml, err)
 		}
 	}
@@ -181,4 +179,8 @@ func TestGetEmail(t *testing.T) {
 	}
 }
 
-var testStorage = Storage("./testdata")
+var testStorage = FileStorage("./testdata")
+
+func (s FileStorage) clean() error {
+	return os.RemoveAll(string(s))
+}