From 7bd2adf0dc3a6f4c0eefe3072043de42d1cd8273 Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Thu, 11 Feb 2016 15:37:51 -0700 Subject: [PATCH] Fix edge case related to reloaded configs and ACME challenge If Caddy is running but not listening on port 80, reloading Caddy with a new Caddyfile that needs to obtain a TLS cert from the CA would fail, because it was just assumed that, if reloading, port 80 as already in use. That is not always the case, so we scan the servers to see if one of them is listening on port 80, and we configure the ACME client accordingly. Kind of a hack... but it works. --- caddy/https/https.go | 26 ++++++++++++++++++++------ caddy/restart.go | 16 +++++++++++++++- 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/caddy/https/https.go b/caddy/https/https.go index 2dd1bea39..4f8e989c3 100644 --- a/caddy/https/https.go +++ b/caddy/https/https.go @@ -49,7 +49,7 @@ func Activate(configs []server.Config) ([]server.Config, error) { MarkQualified(configs) // place certificates and keys on disk - err := ObtainCerts(configs, true) + err := ObtainCerts(configs, true, false) if err != nil { return configs, err } @@ -109,10 +109,12 @@ func MarkQualified(configs []server.Config) { } } -// ObtainCerts obtains certificates for all these configs as long as a certificate does not -// already exist on disk. It does not modify the configs at all; it only obtains and stores -// certificates and keys to the disk. -func ObtainCerts(configs []server.Config, allowPrompts bool) error { +// ObtainCerts obtains certificates for all these configs as long as a +// certificate does not already exist on disk. It does not modify the +// configs at all; it only obtains and stores certificates and keys to +// the disk. If allowPrompts is true, the user may be shown a prompt. +// If proxyACME is true, the ACME challenges will be proxied to our alt port. +func ObtainCerts(configs []server.Config, allowPrompts, proxyACME bool) error { // We group configs by email so we don't make the same clients over and // over. This has the potential to prompt the user for an email, but we // prevent that by assuming that if we already have a listener that can @@ -131,7 +133,19 @@ func ObtainCerts(configs []server.Config, allowPrompts bool) error { continue } - client.Configure(cfg.BindHost) + // c.Configure assumes that allowPrompts == !proxyACME, + // but that's not always true. For example, a restart where + // the user isn't present and we're not listening on port 80. + // TODO: This could probably be refactored better. + if proxyACME { + client.SetHTTPAddress(net.JoinHostPort(cfg.BindHost, AlternatePort)) + client.SetTLSAddress(net.JoinHostPort(cfg.BindHost, AlternatePort)) + client.ExcludeChallenges([]acme.Challenge{acme.TLSSNI01, acme.DNS01}) + } else { + client.SetHTTPAddress(net.JoinHostPort(cfg.BindHost, "")) + client.SetTLSAddress(net.JoinHostPort(cfg.BindHost, "")) + client.ExcludeChallenges([]acme.Challenge{acme.DNS01}) + } err := client.Obtain([]string{cfg.Host}) if err != nil { diff --git a/caddy/restart.go b/caddy/restart.go index 255f9cd7c..cc1ac516f 100644 --- a/caddy/restart.go +++ b/caddy/restart.go @@ -8,6 +8,7 @@ import ( "errors" "io/ioutil" "log" + "net" "os" "os/exec" "path" @@ -142,8 +143,21 @@ func getCertsForNewCaddyfile(newCaddyfile Input) error { // (can ignore error since we aren't actually using the certs) https.EnableTLS(configs, false) + // find out if we can let the acme package start its own challenge listener + // on port 80 + var proxyACME bool + serversMu.Lock() + for _, s := range servers { + _, port, _ := net.SplitHostPort(s.Addr) + if port == "80" { + proxyACME = true + break + } + } + serversMu.Unlock() + // place certs on the disk - err = https.ObtainCerts(configs, false) + err = https.ObtainCerts(configs, false, proxyACME) if err != nil { return errors.New("obtaining certs: " + err.Error()) }