From de3accf531a7dd4eff65ae20e9787fc62045b674 Mon Sep 17 00:00:00 2001 From: Craig Peterson <192540+captncraig@users.noreply.github.com> Date: Sat, 8 Sep 2018 05:56:51 -0400 Subject: [PATCH] Submit all dns records up front, then validate serially (#607) --- acme/client.go | 75 +++++++++++++++++++++++++++++++++++++------ acme/dns_challenge.go | 33 ++++++++++++++----- 2 files changed, 91 insertions(+), 17 deletions(-) diff --git a/acme/client.go b/acme/client.go index b8daa751..d8439448 100644 --- a/acme/client.go +++ b/acme/client.go @@ -41,6 +41,17 @@ type solver interface { Solve(challenge challenge, domain string) error } +// Interface for challenges like dns, where we can set a record in advance for ALL challenges. +// This saves quite a bit of time vs creating the records and solving them serially. +type presolver interface { + PreSolve(challenge challenge, domain string) error +} + +// Interface for challenges like dns, where we can solve all the challenges before to delete them. +type cleanup interface { + CleanUp(challenge challenge, domain string) error +} + type validateFunc func(j *jws, domain, uri string, chlng challenge) error // Client is the user-friendy way to ACME @@ -548,29 +559,75 @@ func (c *Client) createOrderForIdentifiers(domains []string) (orderResource, err return orderRes, nil } +// an authz with the solver we have chosen and the index of the challenge associated with it +type selectedAuthSolver struct { + authz authorization + challengeIndex int + solver solver +} + // Looks through the challenge combinations to find a solvable match. // Then solves the challenges in series and returns. func (c *Client) solveChallengeForAuthz(authorizations []authorization) error { failures := make(ObtainError) - // loop through the resources, basically through the domains. + authSolvers := []*selectedAuthSolver{} + + // loop through the resources, basically through the domains. First pass just selects a solver for each authz. for _, authz := range authorizations { if authz.Status == "valid" { // Boulder might recycle recent validated authz (see issue #267) log.Infof("[%s] acme: Authorization already valid; skipping challenge", authz.Identifier.Value) continue } - - // no solvers - no solving if i, solver := c.chooseSolver(authz, authz.Identifier.Value); solver != nil { - err := solver.Solve(authz.Challenges[i], authz.Identifier.Value) - if err != nil { - //c.disableAuthz(authz.Identifier) + authSolvers = append(authSolvers, &selectedAuthSolver{ + authz: authz, + challengeIndex: i, + solver: solver, + }) + } else { + failures[authz.Identifier.Value] = fmt.Errorf("[%s] acme: Could not determine solvers", authz.Identifier.Value) + } + } + + // for all valid presolvers, first submit the challenges so they have max time to propigate + for _, item := range authSolvers { + authz := item.authz + i := item.challengeIndex + if presolver, ok := item.solver.(presolver); ok { + if err := presolver.PreSolve(authz.Challenges[i], authz.Identifier.Value); err != nil { failures[authz.Identifier.Value] = err } - } else { - //c.disableAuthz(authz) - failures[authz.Identifier.Value] = fmt.Errorf("[%s] acme: Could not determine solvers", authz.Identifier.Value) + } + } + + defer func() { + // clean all created TXT records + for _, item := range authSolvers { + if cleanup, ok := item.solver.(cleanup); ok { + if failures[item.authz.Identifier.Value] != nil { + // already failed in previous loop + continue + } + err := cleanup.CleanUp(item.authz.Challenges[item.challengeIndex], item.authz.Identifier.Value) + if err != nil { + log.Warnf("Error cleaning up %s: %v ", item.authz.Identifier.Value, err) + } + } + } + }() + + // finally solve all challenges for real + for _, item := range authSolvers { + authz := item.authz + i := item.challengeIndex + if failures[authz.Identifier.Value] != nil { + // already failed in previous loop + continue + } + if err := item.solver.Solve(authz.Challenges[i], authz.Identifier.Value); err != nil { + failures[authz.Identifier.Value] = err } } diff --git a/acme/dns_challenge.go b/acme/dns_challenge.go index c8a35eb8..2a43ce37 100644 --- a/acme/dns_challenge.go +++ b/acme/dns_challenge.go @@ -71,8 +71,10 @@ type dnsChallenge struct { provider ChallengeProvider } -func (s *dnsChallenge) Solve(chlng challenge, domain string) error { - log.Infof("[%s] acme: Trying to solve DNS-01", domain) +// PreSolve just submits the txt record to the dns provider. It does not validate record propagation, or +// do anything at all with the acme server. +func (s *dnsChallenge) PreSolve(chlng challenge, domain string) error { + log.Infof("[%s] acme: Preparing to solve DNS-01", domain) if s.provider == nil { return errors.New("no DNS Provider configured") @@ -88,12 +90,18 @@ func (s *dnsChallenge) Solve(chlng challenge, domain string) error { if err != nil { return fmt.Errorf("error presenting token: %s", err) } - defer func() { - err := s.provider.CleanUp(domain, chlng.Token, keyAuth) - if err != nil { - log.Warnf("Error cleaning up %s: %v ", domain, err) - } - }() + + return nil +} + +func (s *dnsChallenge) Solve(chlng challenge, domain string) error { + log.Infof("[%s] acme: Trying to solve DNS-01", domain) + + // Generate the Key Authorization for the challenge + keyAuth, err := getKeyAuthorization(chlng.Token, s.jws.privKey) + if err != nil { + return err + } fqdn, value, _ := DNS01Record(domain, keyAuth) @@ -117,6 +125,15 @@ func (s *dnsChallenge) Solve(chlng challenge, domain string) error { return s.validate(s.jws, domain, chlng.URL, challenge{Type: chlng.Type, Token: chlng.Token, KeyAuthorization: keyAuth}) } +// CleanUp cleans the challenge +func (s *dnsChallenge) CleanUp(chlng challenge, domain string) error { + keyAuth, err := getKeyAuthorization(chlng.Token, s.jws.privKey) + if err != nil { + return err + } + return s.provider.CleanUp(domain, chlng.Token, keyAuth) +} + // checkDNSPropagation checks if the expected TXT record has been propagated to all authoritative nameservers. func checkDNSPropagation(fqdn, value string) (bool, error) { // Initial attempt to resolve at the recursive NS