From 7a1feb3c515bed441c94923d2dc0e92602d3e7ca Mon Sep 17 00:00:00 2001 From: Ludovic Fernandez Date: Tue, 15 May 2018 17:28:02 +0200 Subject: [PATCH] fix: acme errors management. --- Gopkg.lock | 2 +- acme/acme.go | 10 +- provider/acme/provider.go | 6 +- .../github.com/xenolf/lego/acmev2/client.go | 102 ++++++++++-------- vendor/github.com/xenolf/lego/acmev2/error.go | 13 +++ 5 files changed, 82 insertions(+), 51 deletions(-) diff --git a/Gopkg.lock b/Gopkg.lock index 7c251c154..0f12d06d0 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -1278,7 +1278,7 @@ "providers/dns/route53", "providers/dns/vultr" ] - revision = "2817d2131186742bc98830c73a5d9c255b3f4537" + revision = "3d653ee2ee38f1d71beb5f09b37b23344eff0ab3" source = "github.com/containous/lego" [[projects]] diff --git a/acme/acme.go b/acme/acme.go index f32445aeb..3e9a17549 100644 --- a/acme/acme.go +++ b/acme/acme.go @@ -611,11 +611,13 @@ func (a *ACME) getDomainsCertificates(domains []string) (*Certificate, error) { domains = fun.Map(types.CanonicalDomain, domains).([]string) log.Debugf("Loading ACME certificates %s...", domains) bundle := true - certificate, failures := a.client.ObtainCertificate(domains, bundle, nil, OSCPMustStaple) - if len(failures) > 0 { - log.Error(failures) - return nil, fmt.Errorf("cannot obtain certificates %+v", failures) + + certificate, err := a.client.ObtainCertificate(domains, bundle, nil, OSCPMustStaple) + if err != nil { + log.Error(err) + return nil, fmt.Errorf("cannot obtain certificates: %+v", err) } + log.Debugf("Loaded ACME certificates %s", domains) return &Certificate{ Domain: certificate.Domain, diff --git a/provider/acme/provider.go b/provider/acme/provider.go index 26d8faedf..ccc3934bd 100644 --- a/provider/acme/provider.go +++ b/provider/acme/provider.go @@ -226,9 +226,9 @@ func (p *Provider) resolveCertificate(domain types.Domain, domainFromConfigurati bundle := true - certificate, failures := client.ObtainCertificate(uncheckedDomains, bundle, nil, OSCPMustStaple) - if len(failures) > 0 { - return nil, fmt.Errorf("cannot obtain certificates %+v", failures) + certificate, err := client.ObtainCertificate(uncheckedDomains, bundle, nil, OSCPMustStaple) + if err != nil { + return nil, fmt.Errorf("cannot obtain certificates: %+v", err) } if len(certificate.Certificate) == 0 || len(certificate.PrivateKey) == 0 { diff --git a/vendor/github.com/xenolf/lego/acmev2/client.go b/vendor/github.com/xenolf/lego/acmev2/client.go index 649c8d9cf..ff3dc866b 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 } @@ -265,7 +265,7 @@ func (c *Client) QueryRegistration() (*RegistrationResource, error) { // your issued certificate as a bundle. // This function will never return a partial certificate. If one domain in the list fails, // the whole certificate will fail. -func (c *Client) ObtainCertificateForCSR(csr x509.CertificateRequest, bundle bool) (CertificateResource, map[string]error) { +func (c *Client) ObtainCertificateForCSR(csr x509.CertificateRequest, bundle bool) (CertificateResource, error) { // figure out what domains it concerns // start with the common name domains := []string{csr.Subject.CommonName} @@ -292,30 +292,26 @@ DNSNames: order, err := c.createOrderForIdentifiers(domains) if err != nil { - identErrors := make(map[string]error) - for _, auth := range order.Identifiers { - identErrors[auth.Value] = err - } - return CertificateResource{}, identErrors + 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 { @@ -326,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. @@ -338,7 +339,11 @@ DNSNames: // your issued certificate as a bundle. // This function will never return a partial certificate. If one domain in the list fails, // the whole certificate will fail. -func (c *Client) ObtainCertificate(domains []string, bundle bool, privKey crypto.PrivateKey, mustStaple bool) (CertificateResource, map[string]error) { +func (c *Client) ObtainCertificate(domains []string, bundle bool, privKey crypto.PrivateKey, mustStaple bool) (CertificateResource, error) { + if len(domains) == 0 { + return CertificateResource{}, errors.New("No domains to obtain a certificate for") + } + if bundle { logf("[INFO][%s] acme: Obtaining bundled SAN certificate", strings.Join(domains, ", ")) } else { @@ -347,30 +352,26 @@ func (c *Client) ObtainCertificate(domains []string, bundle bool, privKey crypto order, err := c.createOrderForIdentifiers(domains) if err != nil { - identErrors := make(map[string]error) - for _, auth := range order.Identifiers { - identErrors[auth.Value] = err - } - return CertificateResource{}, identErrors + 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 { @@ -378,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. @@ -433,7 +439,7 @@ func (c *Client) RenewCertificate(cert CertificateResource, bundle, mustStaple b return CertificateResource{}, err } newCert, failures := c.ObtainCertificateForCSR(*csr, bundle) - return newCert, failures[cert.Domain] + return newCert, failures } var privKey crypto.PrivateKey @@ -445,7 +451,6 @@ func (c *Client) RenewCertificate(cert CertificateResource, bundle, mustStaple b } var domains []string - var failures map[string]error // check for SAN certificate if len(x509Cert.DNSNames) > 1 { domains = append(domains, x509Cert.Subject.CommonName) @@ -459,8 +464,8 @@ func (c *Client) RenewCertificate(cert CertificateResource, bundle, mustStaple b domains = append(domains, x509Cert.Subject.CommonName) } - newCert, failures := c.ObtainCertificate(domains, bundle, privKey, mustStaple) - return newCert, failures[cert.Domain] + newCert, err := c.ObtainCertificate(domains, bundle, privKey, mustStaple) + return newCert, err } func (c *Client) createOrderForIdentifiers(domains []string) (orderResource, error) { @@ -490,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) map[string]error { +func (c *Client) solveChallengeForAuthz(authorizations []authorization) error { + failures := make(ObtainError) + // loop through the resources, basically through the domains. - failures := make(map[string]error) for _, authz := range authorizations { if authz.Status == "valid" { // Boulder might recycle recent validated authz (see issue #267) @@ -513,7 +519,12 @@ func (c *Client) solveChallengeForAuthz(authorizations []authorization) map[stri } } - 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. @@ -528,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, map[string]error) { +func (c *Client) getAuthzForOrder(order orderResource) ([]authorization, error) { resc, errc := make(chan authorization), make(chan domainError) delay := time.Second / overallRequestLimit @@ -549,7 +560,7 @@ func (c *Client) getAuthzForOrder(order orderResource) ([]authorization, map[str } var responses []authorization - failures := make(map[string]error) + failures := make(ObtainError) for i := 0; i < len(order.Authorizations); i++ { select { case res := <-resc: @@ -564,7 +575,12 @@ func (c *Client) getAuthzForOrder(order orderResource) ([]authorization, map[str 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/github.com/xenolf/lego/acmev2/error.go b/vendor/github.com/xenolf/lego/acmev2/error.go index 15cbc02bb..b92ce734c 100644 --- a/vendor/github.com/xenolf/lego/acmev2/error.go +++ b/vendor/github.com/xenolf/lego/acmev2/error.go @@ -1,6 +1,7 @@ package acmev2 import ( + "bytes" "encoding/json" "fmt" "io/ioutil" @@ -13,6 +14,18 @@ const ( invalidNonceError = "urn:ietf:params:acme:error:badNonce" ) +// ObtainError is returned when there are specific errors available +// per domain. For example in ObtainCertificate +type ObtainError map[string]error + +func (e ObtainError) Error() string { + buffer := bytes.NewBufferString("acme: Error -> One or more domains had a problem:\n") + for dom, err := range e { + buffer.WriteString(fmt.Sprintf("[%s] %s\n", dom, err)) + } + return buffer.String() +} + // RemoteError is the base type for all errors specific to the ACME protocol. type RemoteError struct { StatusCode int `json:"status,omitempty"`