From e99b3af0a5c213ac666cb842e8286ea49db17eb5 Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Fri, 30 Oct 2015 15:55:59 -0600 Subject: [PATCH] letsencrypt: Numerous bug fixes --- caddy/caddy.go | 2 +- caddy/caddyfile/json_test.go | 2 +- caddy/config.go | 9 +- caddy/letsencrypt/letsencrypt.go | 147 ++++++++++++++++++------------- caddy/letsencrypt/maintain.go | 8 +- main.go | 2 +- 6 files changed, 96 insertions(+), 74 deletions(-) diff --git a/caddy/caddy.go b/caddy/caddy.go index aa492b06b..830f046d5 100644 --- a/caddy/caddy.go +++ b/caddy/caddy.go @@ -276,7 +276,7 @@ func Wait() { // the Caddyfile. If loader does not return a Caddyfile, the // default one will be returned. Thus, if there are no other // errors, this function always returns at least the default -// Caddyfile. +// Caddyfile (not the previously-used Caddyfile). func LoadCaddyfile(loader func() (Input, error)) (cdyfile Input, err error) { // If we are a fork, finishing the restart is highest priority; // piped input is required in this case. diff --git a/caddy/caddyfile/json_test.go b/caddy/caddyfile/json_test.go index 44abf982f..f0848b1bd 100644 --- a/caddy/caddyfile/json_test.go +++ b/caddy/caddyfile/json_test.go @@ -62,7 +62,7 @@ baz" { // 8 caddyfile: `http://host, https://host { }`, - json: `[{"hosts":["host:http","host:https"],"body":{}}]`, // hosts in JSON are always host:port format (if port is specified) + json: `[{"hosts":["host:http","host:https"],"body":{}}]`, // hosts in JSON are always host:port format (if port is specified), for consistency }, } diff --git a/caddy/config.go b/caddy/config.go index 53432b4e9..503faecc3 100644 --- a/caddy/config.go +++ b/caddy/config.go @@ -89,10 +89,6 @@ func load(filename string, input io.Reader) ([]server.Config, error) { } } - if config.Port == "" { - config.Port = Port - } - configs = append(configs, config) } } @@ -145,6 +141,11 @@ func arrangeBindings(allConfigs []server.Config) (Group, error) { // Group configs by bind address for _, conf := range allConfigs { + // use default port if none is specified + if conf.Port == "" { + conf.Port = Port + } + bindAddr, warnErr, fatalErr := resolveAddr(conf) if fatalErr != nil { return groupings, fatalErr diff --git a/caddy/letsencrypt/letsencrypt.go b/caddy/letsencrypt/letsencrypt.go index df8225843..d131fbb80 100644 --- a/caddy/letsencrypt/letsencrypt.go +++ b/caddy/letsencrypt/letsencrypt.go @@ -39,32 +39,35 @@ import ( // some may have been appended, for example, to redirect // plaintext HTTP requests to their HTTPS counterpart. func Activate(configs []server.Config) ([]server.Config, error) { + // just in case previous caller forgot... + Deactivate() // TODO: Is multiple activation (before a deactivation) an error? + // reset cached ocsp statuses from any previous activations + ocspStatus = make(map[*[]byte]int) + // Identify and configure any eligible hosts for which // we already have certs and keys in storage from last time. configLen := len(configs) // avoid infinite loop since this loop appends plaintext to the slice for i := 0; i < configLen; i++ { - if existingCertAndKey(configs[i].Host) && configs[i].TLS.LetsEncryptEmail != "off" { + if existingCertAndKey(configs[i].Host) && configQualifies(configs[i], configs) { configs = autoConfigure(&configs[i], configs) } } - // Filter the configs by what we can maintain automatically - filteredConfigs := filterConfigs(configs) - - // Renew any existing certificates that need renewal - renewCertificates(filteredConfigs) - - // Group configs by LE email address; this will help us - // reduce round-trips when getting the certs. - groupedConfigs, err := groupConfigsByEmail(filteredConfigs) + // Group configs by email address; only configs that are eligible + // for TLS management are included. We group by email so that we + // can request certificates in batches with the same client. + // Note: The return value is a map, and iteration over a map is + // not ordered. I don't think it will be a problem, but if an + // ordering problem arises, look at this carefully. + groupedConfigs, err := groupConfigsByEmail(configs) if err != nil { return configs, err } - // Loop through each email address and obtain certs; this way, we can obtain more - // than one certificate per email address, and still save them individually. + // obtain certificates for configs that need one, and reconfigure each + // config to use the certificates for leEmail, serverConfigs := range groupedConfigs { // make client to service this email address with CA server client, err := newClient(leEmail) @@ -75,7 +78,7 @@ func Activate(configs []server.Config) ([]server.Config, error) { // client is ready, so let's get free, trusted SSL certificates! yeah! certificates, err := obtainCertificates(client, serverConfigs) if err != nil { - return configs, errors.New("error obtaining cert: " + err.Error()) + return configs, errors.New("error getting certs: " + err.Error()) } // ... that's it. save the certs, keys, and metadata files to disk @@ -84,15 +87,17 @@ func Activate(configs []server.Config) ([]server.Config, error) { return configs, errors.New("error saving assets: " + err.Error()) } - // it all comes down to this: turning TLS on for all the configs - for _, cfg := range serverConfigs { - configs = autoConfigure(cfg, configs) + // it all comes down to this: turning on TLS with all the new certs + for i := 0; i < len(serverConfigs); i++ { + configs = autoConfigure(serverConfigs[i], configs) } } - Deactivate() // in case previous caller wasn't clean about it - stopChan = make(chan struct{}) - go maintainAssets(filteredConfigs, stopChan) + // renew all certificates that need renewal + renewCertificates(configs) + + // keep certificates renewed and OCSP stapling updated + go maintainAssets(configs, stopChan) return configs, nil } @@ -108,55 +113,51 @@ func Deactivate() (err error) { } }() close(stopChan) + stopChan = make(chan struct{}) return } -// filterConfigs filters and returns configs that are eligible for automatic -// TLS by skipping configs that do not qualify for automatic maintenance -// of assets. Configurations with a manual TLS configuration or that already -// have an HTTPS counterpart host defined will be skipped. -func filterConfigs(configs []server.Config) []server.Config { - var filtered []server.Config +// configQualifies returns true if cfg qualifes for automatic LE activation, +// but it does require the list of all configs to be passed in as well. +// It does NOT check to see if a cert and key already exist for cfg. +func configQualifies(cfg server.Config, allConfigs []server.Config) bool { + return cfg.TLS.Certificate == "" && // user could provide their own cert and key + cfg.TLS.Key == "" && - // configQualifies returns true if cfg qualifes for automatic LE activation - configQualifies := func(cfg server.Config) bool { - return cfg.TLS.Certificate == "" && // user could provide their own cert and key - cfg.TLS.Key == "" && + // user can force-disable automatic HTTPS for this host + cfg.Port != "http" && + cfg.TLS.LetsEncryptEmail != "off" && - // user can force-disable automatic HTTPS for this host - cfg.Port != "http" && - cfg.TLS.LetsEncryptEmail != "off" && + // obviously we get can't certs for loopback or internal hosts + cfg.Host != "localhost" && + cfg.Host != "" && + cfg.Host != "0.0.0.0" && + cfg.Host != "::1" && + !strings.HasPrefix(cfg.Host, "127.") && + // TODO: Also exclude 10.* and 192.168.* addresses? - // obviously we get can't certs for loopback or internal hosts - cfg.Host != "localhost" && - cfg.Host != "" && - cfg.Host != "0.0.0.0" && - cfg.Host != "::1" && - !strings.HasPrefix(cfg.Host, "127.") && - !strings.HasPrefix(cfg.Host, "10.") && - - // make sure an HTTPS version of this config doesn't exist in the list already - !hostHasOtherScheme(cfg.Host, "https", configs) - } - - for _, cfg := range configs { - if configQualifies(cfg) { - filtered = append(filtered, cfg) - } - } - - return filtered + // make sure an HTTPS version of this config doesn't exist in the list already + !hostHasOtherScheme(cfg.Host, "https", allConfigs) } // groupConfigsByEmail groups configs by user email address. The returned map is // a map of email address to the configs that are serviced under that account. -// If an email address is not available, the user will be prompted to provide one. -// This function assumes that all configs passed in qualify for automatic management. +// If an email address is not available for an eligible config, the user will be +// prompted to provide one. The returned map contains pointers to the original +// server config values. func groupConfigsByEmail(configs []server.Config) (map[string][]*server.Config, error) { initMap := make(map[string][]*server.Config) for i := 0; i < len(configs); i++ { + // filter out configs that we already have certs for and + // that we won't be obtaining certs for - this way we won't + // bother the user for an email address unnecessarily and + // we don't obtain new certs for a host we already have certs for. + if existingCertAndKey(configs[i].Host) || !configQualifies(configs[i], configs) { + continue + } leEmail := getEmail(configs[i]) if leEmail == "" { + // TODO: This may not be an error; just a poor choice by the user return nil, errors.New("must have email address to serve HTTPS without existing certificate and key") } initMap[leEmail] = append(initMap[leEmail], &configs[i]) @@ -280,7 +281,8 @@ func saveCertsAndKeys(certificates []acme.CertificateResource) error { // autoConfigure enables TLS on cfg and appends, if necessary, a new config // to allConfigs that redirects plaintext HTTP to its new HTTPS counterpart. // It expects the certificate and key to already be in storage. It returns -// the new list of allConfigs. +// the new list of allConfigs, since it may append a new config. This function +// assumes that cfg was already set up for HTTPS. func autoConfigure(cfg *server.Config, allConfigs []server.Config) []server.Config { bundleBytes, err := ioutil.ReadFile(storage.SiteCertFile(cfg.Host)) // TODO: Handle these errors better @@ -294,7 +296,9 @@ func autoConfigure(cfg *server.Config, allConfigs []server.Config) []server.Conf cfg.TLS.Certificate = storage.SiteCertFile(cfg.Host) cfg.TLS.Key = storage.SiteKeyFile(cfg.Host) cfg.TLS.Enabled = true - cfg.Port = "https" + if cfg.Port == "" { + cfg.Port = "https" + } // Set up http->https redirect as long as there isn't already // a http counterpart in the configs @@ -308,11 +312,21 @@ func autoConfigure(cfg *server.Config, allConfigs []server.Config) []server.Conf // hostHasOtherScheme tells you whether there is another config in the list // for the same host but with the port equal to scheme. For example, to see // if example.com has a https variant already, pass in example.com and -// "https" along with the list of configs. +// "https" along with the list of configs. This function considers "443" +// and "https" to be the same scheme, as well as "http" and "80". func hostHasOtherScheme(host, scheme string, allConfigs []server.Config) bool { + if scheme == "80" { + scheme = "http" + } else if scheme == "443" { + scheme = "https" + } for _, otherCfg := range allConfigs { - if otherCfg.Host == host && otherCfg.Port == scheme { - return true + if otherCfg.Host == host { + if (otherCfg.Port == scheme) || + (scheme == "https" && otherCfg.Port == "443") || + (scheme == "http" && otherCfg.Port == "80") { + return true + } } } return false @@ -323,12 +337,17 @@ func hostHasOtherScheme(host, scheme string, allConfigs []server.Config) bool { // be the HTTPS configuration. The returned configuration is set // to listen on the "http" port (port 80). func redirPlaintextHost(cfg server.Config) server.Config { + toUrl := "https://" + cfg.Host + if cfg.Port != "https" && cfg.Port != "http" { + toUrl += ":" + cfg.Port + } + redirMidware := func(next middleware.Handler) middleware.Handler { return redirect.Redirect{Next: next, Rules: []redirect.Rule{ { FromScheme: "http", FromPath: "/", - To: "https://" + cfg.Host + "{uri}", + To: toUrl + "{uri}", Code: http.StatusMovedPermanently, }, }} @@ -391,13 +410,15 @@ var ( // Some essential values related to the Let's Encrypt process const ( - // The port to expose to the CA server for Simple HTTP Challenge - exposePort = "5001" + // The port to expose to the CA server for Simple HTTP Challenge. + // NOTE: Let's Encrypt requires port 443. If exposePort is not 443, + // then port 443 must be forwarded to exposePort. + exposePort = "443" - // How often to check certificates for renewal + // How often to check certificates for renewal. renewInterval = 24 * time.Hour - // How often to update OCSP stapling + // How often to update OCSP stapling. ocspInterval = 1 * time.Hour ) diff --git a/caddy/letsencrypt/maintain.go b/caddy/letsencrypt/maintain.go index 62af5e1ca..ae3647493 100644 --- a/caddy/letsencrypt/maintain.go +++ b/caddy/letsencrypt/maintain.go @@ -25,7 +25,7 @@ var OnChange func() error // // You must pass in the server configs to maintain and the channel // which you'll close when maintenance should stop, to allow this -// goroutine to clean up after itself. +// goroutine to clean up after itself and unblock. func maintainAssets(configs []server.Config, stopChan chan struct{}) { renewalTicker := time.NewTicker(renewInterval) ocspTicker := time.NewTicker(ocspInterval) @@ -66,7 +66,7 @@ func maintainAssets(configs []server.Config, stopChan chan struct{}) { // renewCertificates loops through all configured site and // looks for certificates to renew. Nothing is mutated -// through this function. The changes happen directly on disk. +// through this function; all changes happen directly on disk. // It returns the number of certificates renewed and any errors // that occurred. It only performs a renewal if necessary. func renewCertificates(configs []server.Config) (int, []error) { @@ -75,7 +75,7 @@ func renewCertificates(configs []server.Config) (int, []error) { var n int for _, cfg := range configs { - // Host must be TLS-enabled and have assets managed by LE + // Host must be TLS-enabled and have existing assets managed by LE if !cfg.TLS.Enabled || !existingCertAndKey(cfg.Host) { continue } @@ -100,7 +100,7 @@ func renewCertificates(configs []server.Config) (int, []error) { // Renew with a week or less remaining. if daysLeft <= 7 { log.Printf("[INFO] There are %d days left on the certificate of %s. Trying to renew now.", daysLeft, cfg.Host) - client, err := newClient(getEmail(cfg)) + client, err := newClient("") // email not used for renewal if err != nil { errs = append(errs, err) continue diff --git a/main.go b/main.go index aa3ed0d83..3c4930035 100644 --- a/main.go +++ b/main.go @@ -41,7 +41,7 @@ func init() { // TODO: Production endpoint is: https://acme-v01.api.letsencrypt.org flag.StringVar(&letsencrypt.CAUrl, "ca", "https://acme-staging.api.letsencrypt.org", "Certificate authority ACME server") flag.BoolVar(&letsencrypt.Agreed, "agree", false, "Agree to Let's Encrypt Subscriber Agreement") - flag.StringVar(&letsencrypt.DefaultEmail, "email", "", "Default email address to use for Let's Encrypt transactions") + flag.StringVar(&letsencrypt.DefaultEmail, "email", "", "Default Let's Encrypt account email address") flag.StringVar(&revoke, "revoke", "", "Hostname for which to revoke the certificate") }