From 97487e6f0d35c06b4e567d70c5c9c4fc95735d5a Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Thu, 19 Apr 2018 18:07:12 -0600 Subject: [PATCH] vendor: Update lego to fix error handling bug (closes #2124) --- caddytls/client.go | 8 +- .../github.com/xenolf/lego/acmev2/client.go | 77 ++++++++++++------- vendor/manifest | 2 +- 3 files changed, 52 insertions(+), 35 deletions(-) diff --git a/caddytls/client.go b/caddytls/client.go index 7c747194e..7197d917c 100644 --- a/caddytls/client.go +++ b/caddytls/client.go @@ -244,10 +244,8 @@ func (c *ACMEClient) Obtain(name string) error { acmeMu.Unlock() namesObtaining.Remove([]string{name}) if err != nil { - // Error - try to fix it or report it to the user and abort - + // for a certain kind of error, we can enumerate the error per-domain if failures, ok := err.(acme.ObtainError); ok && len(failures) > 0 { - // in this case, we can enumerate the error per-domain var errMsg string // combine all the failures into a single error message for errDomain, obtainErr := range failures { if obtainErr == nil { @@ -261,9 +259,7 @@ func (c *ACMEClient) Obtain(name string) error { return fmt.Errorf("[%s] failed to obtain certificate: %v", name, err) } - // double-check that we actually got a certificate; check a couple fields - // TODO: This is a temporary workaround for what I think is a bug in the acmev2 package (March 2018) - // but it might not hurt to keep this extra check in place (April 18, 2018: might be fixed now.) + // double-check that we actually got a certificate, in case there's a bug upstream (see issue #2121) if certificate.Domain == "" || certificate.Certificate == nil { return errors.New("returned certificate was empty; probably an unchecked error obtaining it") } diff --git a/vendor/github.com/xenolf/lego/acmev2/client.go b/vendor/github.com/xenolf/lego/acmev2/client.go index 3698f0eca..904f07fbc 100644 --- a/vendor/github.com/xenolf/lego/acmev2/client.go +++ b/vendor/github.com/xenolf/lego/acmev2/client.go @@ -189,7 +189,7 @@ func (c *Client) ResolveAccountByKey() (*RegistrationResource, error) { logf("[INFO] acme: Trying to resolve account by key") acc := accountMessage{OnlyReturnExisting: true} - hdr, err := postJSON(c.jws, c.directory.NewAccountURL, acc, &acc) + hdr, err := postJSON(c.jws, c.directory.NewAccountURL, acc, nil) if err != nil { return nil, err } @@ -294,24 +294,24 @@ DNSNames: if err != nil { return CertificateResource{}, err } - authz, failures := c.getAuthzForOrder(order) - // If any challenge fails - return. Do not generate partial SAN certificates. - if len(failures) > 0 { + authz, err := c.getAuthzForOrder(order) + if err != nil { + // If any challenge fails, return. Do not generate partial SAN certificates. /*for _, auth := range authz { c.disableAuthz(auth) }*/ - - return CertificateResource{}, failures + return CertificateResource{}, err } - errs := c.solveChallengeForAuthz(authz) - // If any challenge fails - return. Do not generate partial SAN certificates. - if len(errs) > 0 { - return CertificateResource{}, errs + err = c.solveChallengeForAuthz(authz) + if err != nil { + // If any challenge fails, return. Do not generate partial SAN certificates. + return CertificateResource{}, err } logf("[INFO][%s] acme: Validations succeeded; requesting certificates", strings.Join(domains, ", ")) + failures := make(ObtainError) cert, err := c.requestCertificateForCsr(order, bundle, csr.Raw, nil) if err != nil { for _, chln := range authz { @@ -322,7 +322,12 @@ DNSNames: // Add the CSR to the certificate so that it can be used for renewals. cert.CSR = pemEncode(&csr) - return cert, failures + // do not return an empty failures map, because + // it would still be a non-nil error value + if len(failures) > 0 { + return cert, failures + } + return cert, nil } // ObtainCertificate tries to obtain a single certificate using all domains passed into it. @@ -336,7 +341,7 @@ DNSNames: // the whole certificate will fail. func (c *Client) ObtainCertificate(domains []string, bundle bool, privKey crypto.PrivateKey, mustStaple bool) (CertificateResource, error) { if len(domains) == 0 { - return CertificateResource{}, errors.New("Passed no domains into ObtainCertificate") + return CertificateResource{}, errors.New("No domains to obtain a certificate for") } if bundle { @@ -349,24 +354,24 @@ func (c *Client) ObtainCertificate(domains []string, bundle bool, privKey crypto if err != nil { return CertificateResource{}, err } - authz, failures := c.getAuthzForOrder(order) - // If any challenge fails - return. Do not generate partial SAN certificates. - if len(failures) > 0 { + authz, err := c.getAuthzForOrder(order) + if err != nil { + // If any challenge fails, return. Do not generate partial SAN certificates. /*for _, auth := range authz { c.disableAuthz(auth) }*/ - - return CertificateResource{}, failures + return CertificateResource{}, err } - errs := c.solveChallengeForAuthz(authz) - // If any challenge fails - return. Do not generate partial SAN certificates. - if len(errs) > 0 { - return CertificateResource{}, errs + err = c.solveChallengeForAuthz(authz) + if err != nil { + // If any challenge fails, return. Do not generate partial SAN certificates. + return CertificateResource{}, err } logf("[INFO][%s] acme: Validations succeeded; requesting certificates", strings.Join(domains, ", ")) + failures := make(ObtainError) cert, err := c.requestCertificateForOrder(order, bundle, privKey, mustStaple) if err != nil { for _, auth := range authz { @@ -374,7 +379,12 @@ func (c *Client) ObtainCertificate(domains []string, bundle bool, privKey crypto } } - return cert, failures + // do not return an empty failures map, because + // it would still be a non-nil error value + if len(failures) > 0 { + return cert, failures + } + return cert, nil } // RevokeCertificate takes a PEM encoded certificate or bundle and tries to revoke it at the CA. @@ -485,9 +495,10 @@ func (c *Client) createOrderForIdentifiers(domains []string) (orderResource, err // Looks through the challenge combinations to find a solvable match. // Then solves the challenges in series and returns. -func (c *Client) solveChallengeForAuthz(authorizations []authorization) ObtainError { - // loop through the resources, basically through the domains. +func (c *Client) solveChallengeForAuthz(authorizations []authorization) error { failures := make(ObtainError) + + // loop through the resources, basically through the domains. for _, authz := range authorizations { if authz.Status == "valid" { // Boulder might recycle recent validated authz (see issue #267) @@ -508,7 +519,12 @@ func (c *Client) solveChallengeForAuthz(authorizations []authorization) ObtainEr } } - return failures + // be careful not to return an empty failures map, for + // even an empty ObtainError is a non-nil error value + if len(failures) > 0 { + return failures + } + return nil } // Checks all challenges from the server in order and returns the first matching solver. @@ -523,7 +539,7 @@ func (c *Client) chooseSolver(auth authorization, domain string) (int, solver) { } // Get the challenges needed to proof our identifier to the ACME server. -func (c *Client) getAuthzForOrder(order orderResource) ([]authorization, ObtainError) { +func (c *Client) getAuthzForOrder(order orderResource) ([]authorization, error) { resc, errc := make(chan authorization), make(chan domainError) delay := time.Second / overallRequestLimit @@ -544,7 +560,7 @@ func (c *Client) getAuthzForOrder(order orderResource) ([]authorization, ObtainE } var responses []authorization - failures := make(map[string]error) + failures := make(ObtainError) for i := 0; i < len(order.Authorizations); i++ { select { case res := <-resc: @@ -559,7 +575,12 @@ func (c *Client) getAuthzForOrder(order orderResource) ([]authorization, ObtainE close(resc) close(errc) - return responses, failures + // be careful to not return an empty failures map; + // even if empty, they become non-nil error values + if len(failures) > 0 { + return responses, failures + } + return responses, nil } func logAuthz(order orderResource) { diff --git a/vendor/manifest b/vendor/manifest index b106ecf28..ca99f1172 100644 --- a/vendor/manifest +++ b/vendor/manifest @@ -193,7 +193,7 @@ "importpath": "github.com/xenolf/lego/acmev2", "repository": "https://github.com/xenolf/lego", "vcs": "git", - "revision": "6e962fbfb37f9ea4a8201e32acb1b94ffb3b8398", + "revision": "fad2257e11ae4ff31ed03739386873aa405dec2d", "branch": "acmev2", "path": "/acmev2", "notests": true