From 77eae62d9f71f50e0a56af15fc15686e3df22322 Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Sun, 10 Jan 2016 23:40:55 -0700 Subject: [PATCH] letsencrypt: Don't prompt if user is not there This change fixes the scenario where you reload the config and it tries to obtain a cert from the ACME server, but no email address is found or terms have not been agreed to in-process. This is unfortunate but it should not stop the server from reloading, so we assume empty email address in this case. --- caddy/letsencrypt/letsencrypt.go | 19 ++++++++++--------- caddy/letsencrypt/letsencrypt_test.go | 6 +++--- caddy/letsencrypt/user.go | 13 ++++++------- caddy/letsencrypt/user_test.go | 9 ++++----- caddy/restart.go | 21 +-------------------- 5 files changed, 24 insertions(+), 44 deletions(-) diff --git a/caddy/letsencrypt/letsencrypt.go b/caddy/letsencrypt/letsencrypt.go index cc7aa9d8..1481e112 100644 --- a/caddy/letsencrypt/letsencrypt.go +++ b/caddy/letsencrypt/letsencrypt.go @@ -116,11 +116,11 @@ func MarkQualified(configs []server.Config) { // // TODO: Right now by potentially prompting about ToS error, we assume this function is only // called at startup, but that is not always the case because it could be during a restart. -func ObtainCerts(configs []server.Config, optPort string) error { - groupedConfigs := groupConfigsByEmail(configs) +func ObtainCerts(configs []server.Config, altPort string) error { + groupedConfigs := groupConfigsByEmail(configs, altPort != "") // don't prompt user if server already running for email, group := range groupedConfigs { - client, err := newClientPort(email, optPort) + client, err := newClientPort(email, altPort) if err != nil { return errors.New("error creating client: " + err.Error()) } @@ -147,11 +147,11 @@ func ObtainCerts(configs []server.Config, optPort string) error { // TODO: Double-check, will obtainErr ever be nil? if tosErr, ok := obtainErr.(acme.TOSError); ok { // Terms of Service agreement error; we can probably deal with this - if !Agreed && !promptedForAgreement { + if !Agreed && !promptedForAgreement && altPort == "" { // don't prompt if server is already running Agreed = promptUserAgreement(tosErr.Detail, true) // TODO: Use latest URL promptedForAgreement = true } - if Agreed { + if Agreed || altPort != "" { err := client.AgreeToTOS() if err != nil { return errors.New("error agreeing to updated terms: " + err.Error()) @@ -174,14 +174,15 @@ func ObtainCerts(configs []server.Config, optPort string) error { // groupConfigsByEmail groups configs by the email address to be used by its // ACME client. It only includes configs that are marked as fully managed. -// This is the function that may prompt for an email address. -func groupConfigsByEmail(configs []server.Config) map[string][]server.Config { +// This is the function that may prompt for an email address, unless skipPrompt +// is true, in which case it will assume an empty email address. +func groupConfigsByEmail(configs []server.Config, skipPrompt bool) map[string][]server.Config { initMap := make(map[string][]server.Config) for _, cfg := range configs { if !cfg.TLS.Managed { continue } - leEmail := getEmail(cfg) + leEmail := getEmail(cfg, skipPrompt) initMap[leEmail] = append(initMap[leEmail], cfg) } return initMap @@ -451,7 +452,7 @@ func Revoke(host string) error { return errors.New("no certificate and key for " + host) } - email := getEmail(server.Config{Host: host}) + email := getEmail(server.Config{Host: host}, false) if email == "" { return errors.New("email is required to revoke") } diff --git a/caddy/letsencrypt/letsencrypt_test.go b/caddy/letsencrypt/letsencrypt_test.go index 606e08a9..2cce9405 100644 --- a/caddy/letsencrypt/letsencrypt_test.go +++ b/caddy/letsencrypt/letsencrypt_test.go @@ -280,7 +280,7 @@ func TestEnableTLS(t *testing.T) { } func TestGroupConfigsByEmail(t *testing.T) { - if groupConfigsByEmail([]server.Config{}) == nil { + if groupConfigsByEmail([]server.Config{}, false) == nil { t.Errorf("With empty input, returned map was nil, but expected non-nil map") } @@ -292,9 +292,9 @@ func TestGroupConfigsByEmail(t *testing.T) { server.Config{Host: "sub4.example.com", TLS: server.TLSConfig{LetsEncryptEmail: "", Managed: true}}, server.Config{Host: "sub5.example.com", TLS: server.TLSConfig{LetsEncryptEmail: ""}}, // not managed } - DefaultEmail = "test@example.com" // bypass prompt during tests... + DefaultEmail = "test@example.com" - groups := groupConfigsByEmail(configs) + groups := groupConfigsByEmail(configs, true) if groups == nil { t.Fatalf("Returned map was nil, but expected values") diff --git a/caddy/letsencrypt/user.go b/caddy/letsencrypt/user.go index dbcc0f49..fca50ec9 100644 --- a/caddy/letsencrypt/user.go +++ b/caddy/letsencrypt/user.go @@ -114,8 +114,10 @@ func newUser(email string) (User, error) { // address from the user to use for TLS for cfg. If it // cannot get an email address, it returns empty string. // (It will warn the user of the consequences of an -// empty email.) -func getEmail(cfg server.Config) string { +// empty email.) If skipPrompt is true, the user will +// NOT be prompted and an empty email will be returned +// instead. +func getEmail(cfg server.Config, skipPrompt bool) string { // First try the tls directive from the Caddyfile leEmail := cfg.TLS.LetsEncryptEmail if leEmail == "" { @@ -132,15 +134,12 @@ func getEmail(cfg server.Config) string { continue } if mostRecent == nil || dir.ModTime().After(mostRecent.ModTime()) { - mostRecent = dir + leEmail = dir.Name() } } - if mostRecent != nil { - leEmail = mostRecent.Name() - } } } - if leEmail == "" { + if leEmail == "" && !skipPrompt { // Alas, we must bother the user and ask for an email address; // if they proceed they also agree to the SA. reader := bufio.NewReader(stdin) diff --git a/caddy/letsencrypt/user_test.go b/caddy/letsencrypt/user_test.go index 1f9c9e4f..765bd3d4 100644 --- a/caddy/letsencrypt/user_test.go +++ b/caddy/letsencrypt/user_test.go @@ -140,13 +140,13 @@ func TestGetEmail(t *testing.T) { LetsEncryptEmail: "test1@foo.com", }, } - actual := getEmail(config) + actual := getEmail(config, false) if actual != "test1@foo.com" { t.Errorf("Did not get correct email from config; expected '%s' but got '%s'", "test1@foo.com", actual) } // Test2: Use default email from flag (or user previously typing it) - actual = getEmail(server.Config{}) + actual = getEmail(server.Config{}, false) if actual != DefaultEmail { t.Errorf("Did not get correct email from config; expected '%s' but got '%s'", DefaultEmail, actual) } @@ -158,7 +158,7 @@ func TestGetEmail(t *testing.T) { if err != nil { t.Fatalf("Could not simulate user input, error: %v", err) } - actual = getEmail(server.Config{}) + actual = getEmail(server.Config{}, false) if actual != "test3@foo.com" { t.Errorf("Did not get correct email from user input prompt; expected '%s' but got '%s'", "test3@foo.com", actual) } @@ -189,8 +189,7 @@ func TestGetEmail(t *testing.T) { t.Fatalf("Could not change user folder mod time for '%s': %v", eml, err) } } - - actual = getEmail(server.Config{}) + actual = getEmail(server.Config{}, false) if actual != "test4-3@foo.com" { t.Errorf("Did not get correct email from storage; expected '%s' but got '%s'", "test4-3@foo.com", actual) } diff --git a/caddy/restart.go b/caddy/restart.go index 0c3cc218..56c85616 100644 --- a/caddy/restart.go +++ b/caddy/restart.go @@ -13,7 +13,6 @@ import ( "path" "github.com/mholt/caddy/caddy/letsencrypt" - "github.com/mholt/caddy/server" ) func init() { @@ -137,26 +136,8 @@ func getCertsForNewCaddyfile(newCaddyfile Input) error { // we must make sure port is set before we group by bind address letsencrypt.EnableTLS(configs) - // we only need to issue certs for hosts where we already have an active listener - groupings, err := arrangeBindings(configs) - if err != nil { - return errors.New("arranging bindings: " + err.Error()) - } - var configsToSetup []server.Config - serversMu.Lock() -GroupLoop: - for _, group := range groupings { - for _, server := range servers { - if server.Addr == group.BindAddr.String() { - configsToSetup = append(configsToSetup, group.Configs...) - continue GroupLoop - } - } - } - serversMu.Unlock() - // place certs on the disk - err = letsencrypt.ObtainCerts(configsToSetup, letsencrypt.AlternatePort) + err = letsencrypt.ObtainCerts(configs, letsencrypt.AlternatePort) if err != nil { return errors.New("obtaining certs: " + err.Error()) }