From 6f9e487d7dcaf9592d19306c2d66cfa05e82a32d Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Thu, 5 Nov 2015 23:43:42 -0700 Subject: [PATCH 1/2] Make acme.Logger optional; otherwise use standard log.Logger Also fixed lil' vet warning --- acme/client.go | 45 ++++++++++++++++++----------------- acme/client_test.go | 2 +- acme/simple_http_challenge.go | 8 +++---- 3 files changed, 28 insertions(+), 27 deletions(-) diff --git a/acme/client.go b/acme/client.go index 3acb1154..cfaf0533 100644 --- a/acme/client.go +++ b/acme/client.go @@ -10,22 +10,23 @@ import ( "io/ioutil" "log" "net/http" - "os" "regexp" "strconv" "strings" "time" ) -// Logger is used to log errors; if nil, the default log.Logger is used. +// Logger is an optional custom logger. var Logger *log.Logger -// logger is an helper function to retrieve the available logger -func logger() *log.Logger { - if Logger == nil { - Logger = log.New(os.Stderr, "", log.LstdFlags) +// logf writes a log entry. It uses Logger if not +// nil, otherwise it uses the default log.Logger. +func logf(format string, args ...interface{}) { + if Logger != nil { + Logger.Printf(format, args...) + } else { + log.Printf(format, args...) } - return Logger } // User interface is to be implemented by users of this library. @@ -105,7 +106,7 @@ func NewClient(caURL string, usr User, keyBits int, optPort string) (*Client, er // Register the current account to the ACME server. func (c *Client) Register() (*RegistrationResource, error) { - logger().Print("Registering account ... ") + logf("Registering account ... ") regMsg := registrationMessage{ Resource: "new-reg", @@ -184,7 +185,7 @@ func (c *Client) AgreeToTOS() error { // If bundle is true, the []byte contains both the issuer certificate and // your issued certificate as a bundle. func (c *Client) ObtainCertificates(domains []string, bundle bool) ([]CertificateResource, map[string]error) { - logger().Print("Obtaining certificates...") + logf("Obtaining certificates...") challenges, failures := c.getChallenges(domains) if len(challenges) == 0 { return nil, failures @@ -199,7 +200,7 @@ func (c *Client) ObtainCertificates(domains []string, bundle bool) ([]Certificat return nil, failures } - logger().Print("Validations succeeded. Getting certificates") + logf("Validations succeeded. Getting certificates") certs, err := c.requestCertificates(challenges, bundle) for k, v := range err { @@ -263,7 +264,7 @@ func (c *Client) RenewCertificate(cert CertificateResource, revokeOld bool, bund // This is just meant to be informal for the user. timeLeft := x509Cert.NotAfter.Sub(time.Now().UTC()) - logger().Printf("[%s] Trying to renew certificate with %d hours remaining.", cert.Domain, int(timeLeft.Hours())) + logf("[%s] Trying to renew certificate with %d hours remaining.", cert.Domain, int(timeLeft.Hours())) // The first step of renewal is to check if we get a renewed cert // directly from the cert URL. @@ -285,7 +286,7 @@ func (c *Client) RenewCertificate(cert CertificateResource, revokeOld bool, bund // If the server responds with a different certificate we are effectively renewed. // TODO: Further test if we can actually use the new certificate (Our private key works) if !x509Cert.Equal(serverCert) { - logger().Printf("[%s] The server responded with a renewed certificate.", cert.Domain) + logf("[%s] The server responded with a renewed certificate.", cert.Domain) if revokeOld { c.RevokeCertificate(cert.Certificate) } @@ -299,7 +300,7 @@ func (c *Client) RenewCertificate(cert CertificateResource, revokeOld bool, bund issuerCert, err := c.getIssuerCertificate(links["up"]) if err != nil { // If we fail to aquire the issuer cert, return the issued certificate - do not fail. - logger().Printf("[%s] Could not bundle issuer certificate.\n%v", cert.Domain, err) + logf("[%s] Could not bundle issuer certificate.\n%v", cert.Domain, err) } else { // Success - append the issuer cert to the issued cert. issuerCert = pemEncode(derCertificateBytes(issuerCert)) @@ -356,7 +357,7 @@ func (c *Client) chooseSolvers(auth authorization, domain string) map[int]solver if solver, ok := c.solvers[auth.Challenges[idx].Type]; ok { solvers[idx] = solver } else { - logger().Printf("Could not find solver for: %s", auth.Challenges[idx].Type) + logf("Could not find solver for: %s", auth.Challenges[idx].Type) } } @@ -392,7 +393,7 @@ func (c *Client) getChallenges(domains []string) ([]*authorizationResource, map[ links := parseLinks(resp.Header["Link"]) if links["next"] == "" { - logger().Println("The server did not provide enough information to proceed.") + logf("The server did not provide enough information to proceed.") return } @@ -514,7 +515,7 @@ func (c *Client) requestCertificate(authz *authorizationResource, result chan Ce issuerCert, err := c.getIssuerCertificate(links["up"]) if err != nil { // If we fail to aquire the issuer cert, return the issued certificate - do not fail. - logger().Printf("[%s] Could not bundle issuer certificate.\n%v", authz.Domain, err) + logf("[%s] Could not bundle issuer certificate.\n%v", authz.Domain, err) } else { // Success - append the issuer cert to the issued cert. issuerCert = pemEncode(derCertificateBytes(issuerCert)) @@ -523,7 +524,7 @@ func (c *Client) requestCertificate(authz *authorizationResource, result chan Ce } cerRes.Certificate = issuedCert - logger().Printf("[%s] Server responded with a certificate.", authz.Domain) + logf("[%s] Server responded with a certificate.", authz.Domain) result <- cerRes return } @@ -537,7 +538,7 @@ func (c *Client) requestCertificate(authz *authorizationResource, result chan Ce return } - logger().Printf("[%s] Server responded with status 202. Respecting retry-after of: %d", authz.Domain, retryAfter) + logf("[%s] Server responded with status 202. Respecting retry-after of: %d", authz.Domain, retryAfter) time.Sleep(time.Duration(retryAfter) * time.Second) break @@ -557,7 +558,7 @@ func (c *Client) requestCertificate(authz *authorizationResource, result chan Ce // getIssuerCertificate requests the issuer certificate and caches it for // subsequent requests. func (c *Client) getIssuerCertificate(url string) ([]byte, error) { - logger().Printf("Requesting issuer cert from: %s", url) + logf("Requesting issuer cert from: %s", url) if c.issuerCert != nil { return c.issuerCert, nil } @@ -582,15 +583,15 @@ func (c *Client) getIssuerCertificate(url string) ([]byte, error) { } func logResponseHeaders(resp *http.Response) { - logger().Println(resp.Status) + logf(resp.Status) for k, v := range resp.Header { - logger().Printf("-- %s: %s", k, v) + logf("-- %s: %s", k, v) } } func logResponseBody(resp *http.Response) { body, _ := ioutil.ReadAll(resp.Body) - logger().Printf("Returned json data: \n%s", body) + logf("Returned json data: \n%s", body) } func parseLinks(links []string) map[string]string { diff --git a/acme/client_test.go b/acme/client_test.go index 228ab977..1a29bf45 100644 --- a/acme/client_test.go +++ b/acme/client_test.go @@ -44,7 +44,7 @@ func TestNewClient(t *testing.T) { } if expected, actual := 1, len(client.solvers); actual != expected { - t.Fatal("Expected %d solver(s), got %d", expected, actual) + t.Fatalf("Expected %d solver(s), got %d", expected, actual) } simphttp, ok := client.solvers["simpleHttp"].(*simpleHTTPChallenge) diff --git a/acme/simple_http_challenge.go b/acme/simple_http_challenge.go index 4d5120d4..cab45f11 100644 --- a/acme/simple_http_challenge.go +++ b/acme/simple_http_challenge.go @@ -27,7 +27,7 @@ type simpleHTTPChallenge struct { func (s *simpleHTTPChallenge) Solve(chlng challenge, domain string) error { - logger().Print("Trying to solve SimpleHTTP") + logf("Trying to solve SimpleHTTP") // Generate random string for the path. The acme server will // access this path on the server in order to validate the request @@ -68,7 +68,7 @@ Loop: if OnSimpleHTTPEnd != nil { OnSimpleHTTPEnd(true) } - logger().Print("The server validated our request") + logf("The server validated our request") break Loop case "pending": break @@ -148,9 +148,9 @@ func (s *simpleHTTPChallenge) startHTTPSServer(domain string, token string) (net if strings.HasPrefix(r.Host, domain) && r.Method == "GET" { w.Header().Add("Content-Type", "application/jose+json") w.Write([]byte(signedCompact)) - logger().Print("Served JWS payload...") + logf("Served JWS payload...") } else { - logger().Printf("Received request for domain %s with method %s", r.Host, r.Method) + logf("Received request for domain %s with method %s", r.Host, r.Method) w.Write([]byte("TEST")) } }) From 10f2b59add4f551e3b077eb533ac98589853be2f Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Fri, 6 Nov 2015 23:22:32 -0700 Subject: [PATCH 2/2] Removed unused functions, more consistent/readable debugging --- acme/client.go | 46 ++++++++++++++---------------- acme/error.go | 2 +- acme/simple_http_challenge_test.go | 2 +- 3 files changed, 23 insertions(+), 27 deletions(-) diff --git a/acme/client.go b/acme/client.go index cfaf0533..fd1c9b92 100644 --- a/acme/client.go +++ b/acme/client.go @@ -106,7 +106,10 @@ func NewClient(caURL string, usr User, keyBits int, optPort string) (*Client, er // Register the current account to the ACME server. func (c *Client) Register() (*RegistrationResource, error) { - logf("Registering account ... ") + if c == nil || c.user == nil { + return nil, errors.New("acme: cannot register a nil client or user") + } + logf("[INFO] acme: Registering account for %s", c.user.GetEmail()) regMsg := registrationMessage{ Resource: "new-reg", @@ -150,7 +153,7 @@ func (c *Client) Register() (*RegistrationResource, error) { if links["next"] != "" { reg.NewAuthzURL = links["next"] } else { - return nil, errors.New("The server did not return enough information to proceed...") + return nil, errors.New("acme: The server did not return 'next' link to proceed") } return reg, nil @@ -185,7 +188,12 @@ func (c *Client) AgreeToTOS() error { // If bundle is true, the []byte contains both the issuer certificate and // your issued certificate as a bundle. func (c *Client) ObtainCertificates(domains []string, bundle bool) ([]CertificateResource, map[string]error) { - logf("Obtaining certificates...") + if bundle { + logf("[INFO] acme: Obtaining bundled certificates for %v", strings.Join(domains, ", ")) + } else { + logf("[INFO] acme: Obtaining certificates for %v", strings.Join(domains, ", ")) + } + challenges, failures := c.getChallenges(domains) if len(challenges) == 0 { return nil, failures @@ -200,7 +208,7 @@ func (c *Client) ObtainCertificates(domains []string, bundle bool) ([]Certificat return nil, failures } - logf("Validations succeeded. Getting certificates") + logf("[INFO] acme: Validations succeeded; requesting certificates") certs, err := c.requestCertificates(challenges, bundle) for k, v := range err { @@ -264,7 +272,7 @@ func (c *Client) RenewCertificate(cert CertificateResource, revokeOld bool, bund // This is just meant to be informal for the user. timeLeft := x509Cert.NotAfter.Sub(time.Now().UTC()) - logf("[%s] Trying to renew certificate with %d hours remaining.", cert.Domain, int(timeLeft.Hours())) + logf("[INFO] acme: [%s] Trying renewal with %d hours remaining", cert.Domain, int(timeLeft.Hours())) // The first step of renewal is to check if we get a renewed cert // directly from the cert URL. @@ -286,7 +294,7 @@ func (c *Client) RenewCertificate(cert CertificateResource, revokeOld bool, bund // If the server responds with a different certificate we are effectively renewed. // TODO: Further test if we can actually use the new certificate (Our private key works) if !x509Cert.Equal(serverCert) { - logf("[%s] The server responded with a renewed certificate.", cert.Domain) + logf("[INFO] acme: [%s] Server responded with renewed certificate", cert.Domain) if revokeOld { c.RevokeCertificate(cert.Certificate) } @@ -300,7 +308,7 @@ func (c *Client) RenewCertificate(cert CertificateResource, revokeOld bool, bund issuerCert, err := c.getIssuerCertificate(links["up"]) if err != nil { // If we fail to aquire the issuer cert, return the issued certificate - do not fail. - logf("[%s] Could not bundle issuer certificate.\n%v", cert.Domain, err) + logf("[ERROR] acme: [%s] Could not bundle issuer certificate: %v", cert.Domain, err) } else { // Success - append the issuer cert to the issued cert. issuerCert = pemEncode(derCertificateBytes(issuerCert)) @@ -341,7 +349,7 @@ func (c *Client) solveChallenges(challenges []*authorizationResource) map[string } } } else { - failures[authz.Domain] = fmt.Errorf("Could not determine solvers for %s", authz.Domain) + failures[authz.Domain] = fmt.Errorf("acme: Could not determine solvers for %s", authz.Domain) } } @@ -357,7 +365,7 @@ func (c *Client) chooseSolvers(auth authorization, domain string) map[int]solver if solver, ok := c.solvers[auth.Challenges[idx].Type]; ok { solvers[idx] = solver } else { - logf("Could not find solver for: %s", auth.Challenges[idx].Type) + logf("[ERROR] acme: Could not find solver for: %s", auth.Challenges[idx].Type) } } @@ -393,7 +401,7 @@ func (c *Client) getChallenges(domains []string) ([]*authorizationResource, map[ links := parseLinks(resp.Header["Link"]) if links["next"] == "" { - logf("The server did not provide enough information to proceed.") + logf("[ERROR] acme: Server did not provide next link to proceed") return } @@ -515,7 +523,7 @@ func (c *Client) requestCertificate(authz *authorizationResource, result chan Ce issuerCert, err := c.getIssuerCertificate(links["up"]) if err != nil { // If we fail to aquire the issuer cert, return the issued certificate - do not fail. - logf("[%s] Could not bundle issuer certificate.\n%v", authz.Domain, err) + logf("[WARNING] acme: [%s] Could not bundle issuer certificate: %v", authz.Domain, err) } else { // Success - append the issuer cert to the issued cert. issuerCert = pemEncode(derCertificateBytes(issuerCert)) @@ -538,7 +546,7 @@ func (c *Client) requestCertificate(authz *authorizationResource, result chan Ce return } - logf("[%s] Server responded with status 202. Respecting retry-after of: %d", authz.Domain, retryAfter) + logf("[INFO] acme: [%s] Server responded with status 202; retrying after %ds", authz.Domain, retryAfter) time.Sleep(time.Duration(retryAfter) * time.Second) break @@ -558,7 +566,7 @@ func (c *Client) requestCertificate(authz *authorizationResource, result chan Ce // getIssuerCertificate requests the issuer certificate and caches it for // subsequent requests. func (c *Client) getIssuerCertificate(url string) ([]byte, error) { - logf("Requesting issuer cert from: %s", url) + logf("[INFO] acme: Requesting issuer cert from %s", url) if c.issuerCert != nil { return c.issuerCert, nil } @@ -582,18 +590,6 @@ func (c *Client) getIssuerCertificate(url string) ([]byte, error) { return issuerBytes, err } -func logResponseHeaders(resp *http.Response) { - logf(resp.Status) - for k, v := range resp.Header { - logf("-- %s: %s", k, v) - } -} - -func logResponseBody(resp *http.Response) { - body, _ := ioutil.ReadAll(resp.Body) - logf("Returned json data: \n%s", body) -} - func parseLinks(links []string) map[string]string { aBrkt := regexp.MustCompile("[<>]") slver := regexp.MustCompile("(.+) *= *\"(.+)\"") diff --git a/acme/error.go b/acme/error.go index ec37c87a..d44d1581 100644 --- a/acme/error.go +++ b/acme/error.go @@ -18,7 +18,7 @@ type RemoteError struct { } func (e RemoteError) Error() string { - return fmt.Sprintf("[%d] Type: %s Detail: %s", e.StatusCode, e.Type, e.Detail) + return fmt.Sprintf("acme: Error %d - %s - %s", e.StatusCode, e.Type, e.Detail) } // TOSError represents the error which is returned if the user needs to diff --git a/acme/simple_http_challenge_test.go b/acme/simple_http_challenge_test.go index 3e54277a..5c0d460e 100644 --- a/acme/simple_http_challenge_test.go +++ b/acme/simple_http_challenge_test.go @@ -138,7 +138,7 @@ func TestSimpleHTTPServerError(t *testing.T) { if err := solver.Solve(clientChallenge, "127.0.0.1"); err == nil { t.Error("UNEXPECTED: Expected Solve to return an error but the error was nil.") } else { - expectedError := "[500] Type: urn:acme:error:unauthorized Detail: Error creating new authz :: Syntax error" + expectedError := "acme: Error 500 - urn:acme:error:unauthorized - Error creating new authz :: Syntax error" if err.Error() != expectedError { t.Errorf("Expected error |%s| but instead got |%s|", expectedError, err.Error()) }