diff --git a/caddyhttp/httpserver/https.go b/caddyhttp/httpserver/https.go index 825f028d..1f5465ef 100644 --- a/caddyhttp/httpserver/https.go +++ b/caddyhttp/httpserver/https.go @@ -23,7 +23,7 @@ func activateHTTPS(cctx caddy.Context) error { // place certificates and keys on disk for _, c := range ctx.siteConfigs { - err := c.TLS.ObtainCert(operatorPresent) + err := c.TLS.ObtainCert(c.TLS.Hostname, operatorPresent) if err != nil { return err } diff --git a/caddytls/client.go b/caddytls/client.go index f3cae1be..da510f08 100644 --- a/caddytls/client.go +++ b/caddytls/client.go @@ -18,11 +18,13 @@ import ( // acmeMu ensures that only one ACME challenge occurs at a time. var acmeMu sync.Mutex -// ACMEClient is an acme.Client with custom state attached. +// ACMEClient is a wrapper over acme.Client with +// some custom state attached. It is used to obtain, +// renew, and revoke certificates with ACME. type ACMEClient struct { - *acme.Client AllowPrompts bool config *Config + acmeClient *acme.Client } // newACMEClient creates a new ACMEClient given an email and whether @@ -100,7 +102,11 @@ var newACMEClient = func(config *Config, allowPrompts bool) (*ACMEClient, error) } } - c := &ACMEClient{Client: client, AllowPrompts: allowPrompts, config: config} + c := &ACMEClient{ + AllowPrompts: allowPrompts, + config: config, + acmeClient: client, + } if config.DNSProvider == "" { // Use HTTP and TLS-SNI challenges by default @@ -116,15 +122,15 @@ var newACMEClient = func(config *Config, allowPrompts bool) (*ACMEClient, error) // See if TLS challenge needs to be handled by our own facilities if caddy.HasListenerWithAddress(net.JoinHostPort(config.ListenHost, TLSSNIChallengePort)) { - c.SetChallengeProvider(acme.TLSSNI01, tlsSniSolver{}) + c.acmeClient.SetChallengeProvider(acme.TLSSNI01, tlsSniSolver{}) } // Always respect user's bind preferences by using config.ListenHost - err := c.SetHTTPAddress(net.JoinHostPort(config.ListenHost, useHTTPPort)) + err := c.acmeClient.SetHTTPAddress(net.JoinHostPort(config.ListenHost, useHTTPPort)) if err != nil { return nil, err } - err = c.SetTLSAddress(net.JoinHostPort(config.ListenHost, "")) + err = c.acmeClient.SetTLSAddress(net.JoinHostPort(config.ListenHost, "")) if err != nil { return nil, err } @@ -145,23 +151,50 @@ var newACMEClient = func(config *Config, allowPrompts bool) (*ACMEClient, error) } // Use the DNS challenge exclusively - c.ExcludeChallenges([]acme.Challenge{acme.HTTP01, acme.TLSSNI01}) - c.SetChallengeProvider(acme.DNS01, prov) + c.acmeClient.ExcludeChallenges([]acme.Challenge{acme.HTTP01, acme.TLSSNI01}) + c.acmeClient.SetChallengeProvider(acme.DNS01, prov) } return c, nil } -// Obtain obtains a single certificate for names. It stores the certificate -// on the disk if successful. -func (c *ACMEClient) Obtain(names []string) error { +// Obtain obtains a single certificate for name. It stores the certificate +// on the disk if successful. This function is safe for concurrent use. +// +// Right now our storage mechanism only supports one name per certificate, +// so this function (along with Renew and Revoke) only accepts one domain +// as input. It can be easily modified to support SAN certificates if our +// storage mechanism is upgraded later. +// +// Callers who have access to a Config value should use the ObtainCert +// method on that instead of this lower-level method. +func (c *ACMEClient) Obtain(name string) error { + // Get access to ACME storage + storage, err := c.config.StorageFor(c.config.CAUrl) + if err != nil { + return err + } + + // 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) + } + }() + Attempts: for attempts := 0; attempts < 2; attempts++ { - namesObtaining.Add(names) + namesObtaining.Add([]string{name}) acmeMu.Lock() - certificate, failures := c.ObtainCertificate(names, true, nil) + certificate, failures := c.acmeClient.ObtainCertificate([]string{name}, true, nil) acmeMu.Unlock() - namesObtaining.Remove(names) + namesObtaining.Remove([]string{name}) if len(failures) > 0 { // Error - try to fix it or report it to the user and abort var errMsg string // we'll combine all the failures into a single error message @@ -178,7 +211,7 @@ Attempts: promptedForAgreement = true } if Agreed || !c.AllowPrompts { - err := c.AgreeToTOS() + err := c.acmeClient.AgreeToTOS() if err != nil { return errors.New("error agreeing to updated terms: " + err.Error()) } @@ -193,13 +226,9 @@ Attempts: } // Success - immediately save the certificate resource - storage, err := c.config.StorageFor(c.config.CAUrl) - if err != nil { - return err - } err = saveCertResource(storage, certificate) if err != nil { - return fmt.Errorf("error saving assets for %v: %v", names, err) + return fmt.Errorf("error saving assets for %v: %v", name, err) } break @@ -208,13 +237,11 @@ Attempts: return nil } -// Renew renews the managed certificate for name. Right now our storage -// mechanism only supports one name per certificate, so this function only -// accepts one domain as input. It can be easily modified to support SAN -// certificates if, one day, they become desperately needed enough that our -// storage mechanism is upgraded to be more complex to support SAN certs. +// Renew renews the managed certificate for name. This function is +// safe for concurrent use. // -// Anyway, this function is safe for concurrent use. +// Callers who have access to a Config value should use the RenewCert +// method on that instead of this lower-level method. func (c *ACMEClient) Renew(name string) error { // Get access to ACME storage storage, err := c.config.StorageFor(c.config.CAUrl) @@ -251,7 +278,7 @@ func (c *ACMEClient) Renew(name string) error { for attempts := 0; attempts < 2; attempts++ { namesObtaining.Add([]string{name}) acmeMu.Lock() - newCertMeta, err = c.RenewCertificate(certMeta, true) + newCertMeta, err = c.acmeClient.RenewCertificate(certMeta, true) acmeMu.Unlock() namesObtaining.Remove([]string{name}) if err == nil { @@ -259,10 +286,10 @@ func (c *ACMEClient) Renew(name string) error { break } - // If the legal terms changed and need to be agreed to again, - // we can handle that. + // If the legal terms were updated and need to be + // agreed to again, we can handle that. if _, ok := err.(acme.TOSError); ok { - err := c.AgreeToTOS() + err := c.acmeClient.AgreeToTOS() if err != nil { return err } @@ -304,7 +331,7 @@ func (c *ACMEClient) Revoke(name string) error { return err } - err = c.Client.RevokeCertificate(siteData.Cert) + err = c.acmeClient.RevokeCertificate(siteData.Cert) if err != nil { return err } diff --git a/caddytls/config.go b/caddytls/config.go index ea072aa1..80d30142 100644 --- a/caddytls/config.go +++ b/caddytls/config.go @@ -3,13 +3,9 @@ package caddytls import ( "crypto/tls" "crypto/x509" - "encoding/json" - "errors" "fmt" "io/ioutil" - "time" - "log" "net/url" "strings" @@ -116,24 +112,21 @@ type OnDemandState struct { // If it reaches MaxObtain, on-demand issuances must fail. ObtainedCount int32 - // Based on max_certs in tls config, it specifies the + // Set from max_certs in tls config, it specifies the // maximum number of certificates that can be issued. MaxObtain int32 } -// ObtainCert obtains a certificate for c.Hostname, as long as a certificate -// does not already exist in storage on disk. It only obtains and stores -// certificates (and their keys) to disk, it does not load them into memory. -// If allowPrompts is true, the user may be shown a prompt. If proxyACME is -// true, the relevant ACME challenges will be proxied to the alternate port. -func (c *Config) ObtainCert(allowPrompts bool) error { - return c.obtainCertName(c.Hostname, allowPrompts) -} - -// obtainCertName gets a certificate for name using the ACME config c -// if c and name both qualify. It places the certificate in storage. -// It is a no-op if the storage already has a certificate for name. -func (c *Config) obtainCertName(name string, allowPrompts bool) error { +// ObtainCert obtains a certificate for name using c, as long +// as a certificate does not already exist in storage for that +// name. The name must qualify and c must be flagged as Managed. +// This function is a no-op if storage already has a certificate +// for name. +// +// It only obtains and stores certificates (and their keys), +// it does not load them into memory. If allowPrompts is true, +// the user may be shown a prompt. +func (c *Config) ObtainCert(name string, allowPrompts bool) error { if !c.Managed || !HostQualifies(name) { return nil } @@ -142,29 +135,13 @@ func (c *Config) obtainCertName(name string, allowPrompts bool) error { if err != nil { return err } - siteExists, err := storage.SiteExists(name) if err != nil { return err } - if siteExists { 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) } @@ -173,86 +150,17 @@ func (c *Config) obtainCertName(name string, allowPrompts bool) error { if err != nil { return err } - - return client.Obtain([]string{name}) + return client.Obtain(name) } -// 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 := 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) - siteData, err := storage.LoadSite(name) - if err != nil { - return err - } - var certMeta acme.CertificateResource - err = json.Unmarshal(siteData.Meta, &certMeta) - certMeta.Certificate = siteData.Cert - certMeta.PrivateKey = siteData.Key - +// RenewCert renews the certificate for name using c. It stows the +// renewed certificate and its assets in storage if successful. +func (c *Config) RenewCert(name string, allowPrompts bool) error { client, err := newACMEClient(c, allowPrompts) if err != nil { return err } - - // Perform renewal and retry if necessary, but not too many times. - var newCertMeta acme.CertificateResource - var success bool - for attempts := 0; attempts < 2; attempts++ { - namesObtaining.Add([]string{name}) - acmeMu.Lock() - newCertMeta, err = client.RenewCertificate(certMeta, true) - acmeMu.Unlock() - namesObtaining.Remove([]string{name}) - if err == nil { - success = true - break - } - - // If the legal terms were updated and need to be - // agreed to again, we can handle that. - if _, ok := err.(acme.TOSError); ok { - err := client.AgreeToTOS() - if err != nil { - return err - } - continue - } - - // For any other kind of error, wait 10s and try again. - time.Sleep(10 * time.Second) - } - - if !success { - return errors.New("too many renewal attempts; last error: " + err.Error()) - } - - return saveCertResource(storage, newCertMeta) + return client.Renew(name) } // StorageFor obtains a TLS Storage instance for the given CA URL which should diff --git a/caddytls/filestorage.go b/caddytls/filestorage.go index f5487a05..f0dc03a3 100644 --- a/caddytls/filestorage.go +++ b/caddytls/filestorage.go @@ -123,7 +123,6 @@ func (s FileStorage) readFile(file string) ([]byte, error) { return nil, ErrNotExist(err) } return b, err - } // SiteExists implements Storage.SiteExists by checking for the presence of diff --git a/caddytls/handshake.go b/caddytls/handshake.go index ae9f0cee..102843cd 100644 --- a/caddytls/handshake.go +++ b/caddytls/handshake.go @@ -172,22 +172,25 @@ func (cg configGroup) obtainOnDemandCertificate(name string, cfg *Config) (Certi return cg.getCertDuringHandshake(name, true, false) } - // looks like it's up to us to do all the work and obtain the cert + // looks like it's up to us to do all the work and obtain the cert. + // make a chan others can wait on if needed wait = make(chan struct{}) obtainCertWaitChans[name] = wait obtainCertWaitChansMu.Unlock() - // Unblock waiters and delete waitgroup when we return - defer func() { - obtainCertWaitChansMu.Lock() - close(wait) - delete(obtainCertWaitChans, name) - obtainCertWaitChansMu.Unlock() - }() - + // do the obtain log.Printf("[INFO] Obtaining new certificate for %s", name) + err := cfg.ObtainCert(name, false) - if err := cfg.obtainCertName(name, false); err != nil { + // immediately unblock anyone waiting for it; doing this in + // a defer would risk deadlock because of the recursive call + // to getCertDuringHandshake below when we return! + obtainCertWaitChansMu.Lock() + close(wait) + delete(obtainCertWaitChans, name) + obtainCertWaitChansMu.Unlock() + + if err != nil { // Failed to solve challenge, so don't allow another on-demand // issue for this name to be attempted for a little while. failedIssuanceMu.Lock() @@ -208,7 +211,7 @@ func (cg configGroup) obtainOnDemandCertificate(name string, cfg *Config) (Certi lastIssueTime = time.Now() lastIssueTimeMu.Unlock() - // The certificate is already on disk; now just start over to load it and serve it + // certificate is already on disk; now just start over to load it and serve it return cg.getCertDuringHandshake(name, true, false) } @@ -265,17 +268,18 @@ func (cg configGroup) renewDynamicCertificate(name string, cfg *Config) (Certifi obtainCertWaitChans[name] = wait obtainCertWaitChansMu.Unlock() - // unblock waiters and delete waitgroup when we return - defer func() { - obtainCertWaitChansMu.Lock() - close(wait) - delete(obtainCertWaitChans, name) - obtainCertWaitChansMu.Unlock() - }() - + // do the renew log.Printf("[INFO] Renewing certificate for %s", name) + err := cfg.RenewCert(name, false) + + // immediately unblock anyone waiting for it; doing this in + // a defer would risk deadlock because of the recursive call + // to getCertDuringHandshake below when we return! + obtainCertWaitChansMu.Lock() + close(wait) + delete(obtainCertWaitChans, name) + obtainCertWaitChansMu.Unlock() - err := cfg.renewCertName(name, false) if err != nil { return Certificate{}, err } diff --git a/caddytls/maintain.go b/caddytls/maintain.go index 9ed47ccd..84141504 100644 --- a/caddytls/maintain.go +++ b/caddytls/maintain.go @@ -99,12 +99,20 @@ func RenewManagedCertificates(allowPrompts bool) (err error) { continue } - // 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) + // Get the name which we should use to renew this certificate; + // we only support managing certificates with one name per cert, + // so this should be easy. We can't rely on cert.Config.Hostname + // because it may be a wildcard value from the Caddyfile (e.g. + // *.something.com) which, as of 2016, is not supported by ACME. + var renewName string + for _, name := range cert.Names { + if name != "" { + renewName = name + break + } + } + err := cert.Config.RenewCert(renewName, allowPrompts) if err != nil { if allowPrompts && timeLeft < 0 { // Certificate renewal failed, the operator is present, and the certificate