From b766dab9faa617efc371e704e6171e98f57aae8d Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Wed, 28 Sep 2016 18:29:46 -0600 Subject: [PATCH] tls: Reorder some logic to avoid subtle, undocumented behavior By calling SetTLSAddress, the acme package reset the challenge provider to the default one instead of keeping the custom one we specified before with SetChallengeProvider. Yikes. This means that Caddy would try to open a listener on port 443 even though we should have been handling it with our provider, causing the challenge to fail, since usually port 443 is in use. So this change just reorders the calls so that our provider takes precedence. cf. https://github.com/xenolf/lego/pull/292 --- caddytls/client.go | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/caddytls/client.go b/caddytls/client.go index 8345c431..e173b8e7 100644 --- a/caddytls/client.go +++ b/caddytls/client.go @@ -120,12 +120,10 @@ 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.acmeClient.SetChallengeProvider(acme.TLSSNI01, tlsSniSolver{}) - } - - // Always respect user's bind preferences by using config.ListenHost + // Always respect user's bind preferences by using config.ListenHost. + // NOTE(Sep'16): At time of writing, SetHTTPAddress() and SetTLSaddress() + // must be called before SetChallengeProvider(), since they reset the + // challenge provider back to the default one! err := c.acmeClient.SetHTTPAddress(net.JoinHostPort(config.ListenHost, useHTTPPort)) if err != nil { return nil, err @@ -134,6 +132,11 @@ var newACMEClient = func(config *Config, allowPrompts bool) (*ACMEClient, error) if err != nil { return nil, err } + + // See if TLS challenge needs to be handled by our own facilities + if caddy.HasListenerWithAddress(net.JoinHostPort(config.ListenHost, TLSSNIChallengePort)) { + c.acmeClient.SetChallengeProvider(acme.TLSSNI01, tlsSniSolver{}) + } } else { // Otherwise, DNS challenge it is