Use explicit fmt.Sprintf for ProblemDetails (#4787)

In #3708, we added formatters for the the convenience methods in the `probs` package. However, in #4783, @alexzorin pointed out that we were incorrectly passing an error message through fmt.Sprintf as the format parameter rather than as a value parameter. I proposed a fix in #4784, but during code review we concluded that the underlying problem was the pattern of using format-style functions that don't have some variant of printf in the name. That makes this wrong: `probs.DNS(err.Error())`, and this right: `probs.DNS("%s", err)`. Since that's an easy mistake to make and a hard one to spot during code review, we're going to stop using this particular pattern and call `fmt.Sprintf` directly. This PR reverts #3708 and adds some `fmt.Sprintf` where needed.
2020-04-21 14:36:11 -07:00 · 2020-04-21 14:36:11 -07:00 · 4a2029b293
parent 87fb6028c1
commit 4a2029b293
13 changed files with 135 additions and 96 deletions
--- a/probs/probs.go
+++ b/probs/probs.go
@ -110,30 +110,30 @@ func ProblemDetailsToStatusCode(prob *ProblemDetails) int {

 // BadNonce returns a ProblemDetails with a BadNonceProblem and a 400 Bad
 // Request status code.
-func BadNonce(detail string, a ...interface{}) *ProblemDetails {
+func BadNonce(detail string) *ProblemDetails {
 	return &ProblemDetails{
 		Type:       BadNonceProblem,
-		Detail:     fmt.Sprintf(detail, a...),
+		Detail:     detail,
 		HTTPStatus: http.StatusBadRequest,
 	}
 }

 // RejectedIdentifier returns a ProblemDetails with a RejectedIdentifierProblem and a 400 Bad
 // Request status code.
-func RejectedIdentifier(detail string, a ...interface{}) *ProblemDetails {
+func RejectedIdentifier(detail string) *ProblemDetails {
 	return &ProblemDetails{
 		Type:       RejectedIdentifierProblem,
-		Detail:     fmt.Sprintf(detail, a...),
+		Detail:     detail,
 		HTTPStatus: http.StatusBadRequest,
 	}
 }

 // Conflict returns a ProblemDetails with a MalformedProblem and a 409 Conflict
 // status code.
-func Conflict(detail string, a ...interface{}) *ProblemDetails {
+func Conflict(detail string) *ProblemDetails {
 	return &ProblemDetails{
 		Type:       MalformedProblem,
-		Detail:     fmt.Sprintf(detail, a...),
+		Detail:     detail,
 		HTTPStatus: http.StatusConflict,
 	}
 }
@ -150,10 +150,13 @@ func AlreadyRevoked(detail string, a ...interface{}) *ProblemDetails {

 // Malformed returns a ProblemDetails with a MalformedProblem and a 400 Bad
 // Request status code.
-func Malformed(detail string, a ...interface{}) *ProblemDetails {
+func Malformed(detail string, args ...interface{}) *ProblemDetails {
+	if len(args) > 0 {
+		detail = fmt.Sprintf(detail, args...)
+	}
 	return &ProblemDetails{
 		Type:       MalformedProblem,
-		Detail:     fmt.Sprintf(detail, a...),
+		Detail:     detail,
 		HTTPStatus: http.StatusBadRequest,
 	}
 }
@ -180,30 +183,30 @@ func BadPublicKey(detail string, a ...interface{}) *ProblemDetails {

 // NotFound returns a ProblemDetails with a MalformedProblem and a 404 Not Found
 // status code.
-func NotFound(detail string, a ...interface{}) *ProblemDetails {
+func NotFound(detail string) *ProblemDetails {
 	return &ProblemDetails{
 		Type:       MalformedProblem,
-		Detail:     fmt.Sprintf(detail, a...),
+		Detail:     detail,
 		HTTPStatus: http.StatusNotFound,
 	}
 }

 // ServerInternal returns a ProblemDetails with a ServerInternalProblem and a
 // 500 Internal Server Failure status code.
-func ServerInternal(detail string, a ...interface{}) *ProblemDetails {
+func ServerInternal(detail string) *ProblemDetails {
 	return &ProblemDetails{
 		Type:       ServerInternalProblem,
-		Detail:     fmt.Sprintf(detail, a...),
+		Detail:     detail,
 		HTTPStatus: http.StatusInternalServerError,
 	}
 }

 // Unauthorized returns a ProblemDetails with an UnauthorizedProblem and a 403
 // Forbidden status code.
-func Unauthorized(detail string, a ...interface{}) *ProblemDetails {
+func Unauthorized(detail string) *ProblemDetails {
 	return &ProblemDetails{
 		Type:       UnauthorizedProblem,
-		Detail:     fmt.Sprintf(detail, a...),
+		Detail:     detail,
 		HTTPStatus: http.StatusForbidden,
 	}
 }
@ -230,76 +233,76 @@ func ContentLengthRequired() *ProblemDetails {

 // InvalidContentType returns a ProblemDetails suitable for a missing
 // ContentType header, or an incorrect ContentType header
-func InvalidContentType(detail string, a ...interface{}) *ProblemDetails {
+func InvalidContentType(detail string) *ProblemDetails {
 	return &ProblemDetails{
 		Type:       MalformedProblem,
-		Detail:     fmt.Sprintf(detail, a...),
+		Detail:     detail,
 		HTTPStatus: http.StatusUnsupportedMediaType,
 	}
 }

 // InvalidEmail returns a ProblemDetails representing an invalid email address
 // error
-func InvalidEmail(detail string, a ...interface{}) *ProblemDetails {
+func InvalidEmail(detail string) *ProblemDetails {
 	return &ProblemDetails{
 		Type:       InvalidEmailProblem,
-		Detail:     fmt.Sprintf(detail, a...),
+		Detail:     detail,
 		HTTPStatus: http.StatusBadRequest,
 	}
 }

 // ConnectionFailure returns a ProblemDetails representing a ConnectionProblem
 // error
-func ConnectionFailure(detail string, a ...interface{}) *ProblemDetails {
+func ConnectionFailure(detail string) *ProblemDetails {
 	return &ProblemDetails{
 		Type:       ConnectionProblem,
-		Detail:     fmt.Sprintf(detail, a...),
+		Detail:     detail,
 		HTTPStatus: http.StatusBadRequest,
 	}
 }

 // RateLimited returns a ProblemDetails representing a RateLimitedProblem error
-func RateLimited(detail string, a ...interface{}) *ProblemDetails {
+func RateLimited(detail string) *ProblemDetails {
 	return &ProblemDetails{
 		Type:       RateLimitedProblem,
-		Detail:     fmt.Sprintf(detail, a...),
+		Detail:     detail,
 		HTTPStatus: statusTooManyRequests,
 	}
 }

 // TLSError returns a ProblemDetails representing a TLSProblem error
-func TLSError(detail string, a ...interface{}) *ProblemDetails {
+func TLSError(detail string) *ProblemDetails {
 	return &ProblemDetails{
 		Type:       TLSProblem,
-		Detail:     fmt.Sprintf(detail, a...),
+		Detail:     detail,
 		HTTPStatus: http.StatusBadRequest,
 	}
 }

 // AccountDoesNotExist returns a ProblemDetails representing an
 // AccountDoesNotExistProblem error
-func AccountDoesNotExist(detail string, a ...interface{}) *ProblemDetails {
+func AccountDoesNotExist(detail string) *ProblemDetails {
 	return &ProblemDetails{
 		Type:       AccountDoesNotExistProblem,
-		Detail:     fmt.Sprintf(detail, a...),
+		Detail:     detail,
 		HTTPStatus: http.StatusBadRequest,
 	}
 }

 // CAA returns a ProblemDetails representing a CAAProblem
-func CAA(detail string, a ...interface{}) *ProblemDetails {
+func CAA(detail string) *ProblemDetails {
 	return &ProblemDetails{
 		Type:       CAAProblem,
-		Detail:     fmt.Sprintf(detail, a...),
+		Detail:     detail,
 		HTTPStatus: http.StatusForbidden,
 	}
 }

 // DNS returns a ProblemDetails representing a DNSProblem
-func DNS(detail string, a ...interface{}) *ProblemDetails {
+func DNS(detail string) *ProblemDetails {
 	return &ProblemDetails{
 		Type:       DNSProblem,
-		Detail:     fmt.Sprintf(detail, a...),
+		Detail:     detail,
 		HTTPStatus: http.StatusBadRequest,
 	}
 }
--- a/va/caa.go
+++ b/va/caa.go
@ -50,12 +50,12 @@ func (va *ValidationAuthorityImpl) checkCAA(
 	params *caaParams) *probs.ProblemDetails {
 	present, valid, records, err := va.checkCAARecords(ctx, identifier, params)
 	if err != nil {
-		return probs.DNS("%v", err)
+		return probs.DNS(err.Error())
 	}

 	recordsStr, err := json.Marshal(&records)
 	if err != nil {
-		return probs.CAA("CAA records for %s were malformed", identifier.Value)
+		return probs.CAA(fmt.Sprintf("CAA records for %s were malformed", identifier.Value))
 	}

 	accountID, challengeType := "unknown", "unknown"
@ -69,7 +69,7 @@ func (va *ValidationAuthorityImpl) checkCAA(
 	va.log.AuditInfof("Checked CAA records for %s, [Present: %t, Account ID: %s, Challenge: %s, Valid for issuance: %t] Records=%s",
 		identifier.Value, present, accountID, challengeType, valid, recordsStr)
 	if !valid {
-		return probs.CAA("CAA record for %s prevents issuance", identifier.Value)
+		return probs.CAA(fmt.Sprintf("CAA record for %s prevents issuance", identifier.Value))
 	}
 	return nil
 }
--- a/va/dns.go
+++ b/va/dns.go
@ -68,7 +68,7 @@ func (va *ValidationAuthorityImpl) validateDNS01(ctx context.Context, ident iden
 	// troubleshooters to differentiate between no TXT records and
 	// invalid/incorrect TXT records.
 	if len(txts) == 0 {
-		return nil, probs.Unauthorized("No TXT record found at %s", challengeSubdomain)
+		return nil, probs.Unauthorized(fmt.Sprintf("No TXT record found at %s", challengeSubdomain))
 	}

 	for _, element := range txts {
@ -86,6 +86,6 @@ func (va *ValidationAuthorityImpl) validateDNS01(ctx context.Context, ident iden
 	if len(txts) > 1 {
 		andMore = fmt.Sprintf(" (and %d more)", len(txts)-1)
 	}
-	return nil, probs.Unauthorized("Incorrect TXT record %q%s found at %s",
-		replaceInvalidUTF8([]byte(invalidRecord)), andMore, challengeSubdomain)
+	return nil, probs.Unauthorized(fmt.Sprintf("Incorrect TXT record %q%s found at %s",
+		replaceInvalidUTF8([]byte(invalidRecord)), andMore, challengeSubdomain))
 }
--- a/va/http.go
+++ b/va/http.go
@ -639,8 +639,8 @@ func (va *ValidationAuthorityImpl) validateHTTP01(ctx context.Context, ident ide
 	payload := strings.TrimRight(string(body), whitespaceCutset)

 	if payload != challenge.ProvidedKeyAuthorization {
-		problem := probs.Unauthorized("The key authorization file from the server did not match this challenge %q != %q",
-			challenge.ProvidedKeyAuthorization, payload)
+		problem := probs.Unauthorized(fmt.Sprintf("The key authorization file from the server did not match this challenge %q != %q",
+			challenge.ProvidedKeyAuthorization, payload))
 		va.log.Infof("%s for %s", problem.Detail, ident)
 		return validationRecords, problem
 	}
--- a/va/http_test.go
+++ b/va/http_test.go
@ -575,6 +575,12 @@ func httpTestSrv(t *testing.T) *httptest.Server {
 			http.StatusMovedPermanently)
 	})

+	// A path that returns a body containing printf formatting verbs
+	mux.HandleFunc("/printf-verbs", func(resp http.ResponseWriter, req *http.Request) {
+		resp.WriteHeader(http.StatusOK)
+		fmt.Fprint(resp, "%"+"2F.well-known%"+"2F"+tooLargeBuf.String())
+	})
+
 	return server
 }

@ -763,8 +769,8 @@ func TestFetchHTTP(t *testing.T) {
 			Name: "Redirect loop",
 			Host: "example.com",
 			Path: "/loop",
-			ExpectedProblem: probs.ConnectionFailure(
-				"Fetching http://example.com:%d/loop: Too many redirects", httpPort),
+			ExpectedProblem: probs.ConnectionFailure(fmt.Sprintf(
+				"Fetching http://example.com:%d/loop: Too many redirects", httpPort)),
 			ExpectedRecords: expectedLoopRecords,
 		},
 		{
@ -789,9 +795,9 @@ func TestFetchHTTP(t *testing.T) {
 			Name: "Redirect to bad port",
 			Host: "example.com",
 			Path: "/redir-bad-port",
-			ExpectedProblem: probs.ConnectionFailure(
+			ExpectedProblem: probs.ConnectionFailure(fmt.Sprintf(
 				"Fetching https://example.com:1987: Invalid port in redirect target. "+
-					"Only ports %d and 443 are supported, not 1987", httpPort),
+					"Only ports %d and 443 are supported, not 1987", httpPort)),
 			ExpectedRecords: []core.ValidationRecord{
 				{
 					Hostname:          "example.com",
@ -856,10 +862,10 @@ func TestFetchHTTP(t *testing.T) {
 			Name: "Response too large",
 			Host: "example.com",
 			Path: "/resp-too-big",
-			ExpectedProblem: probs.Unauthorized(
+			ExpectedProblem: probs.Unauthorized(fmt.Sprintf(
 				"Invalid response from http://example.com/resp-too-big "+
 					"[127.0.0.1]: %q", expectedTruncatedResp.String(),
-			),
+			)),
 			ExpectedRecords: []core.ValidationRecord{
 				{
 					Hostname:          "example.com",
@ -947,6 +953,27 @@ func TestFetchHTTP(t *testing.T) {
 				},
 			},
 		},
+		{
+			Name: "Reflected response body containing printf verbs",
+			Host: "example.com",
+			Path: "/printf-verbs",
+			ExpectedProblem: &probs.ProblemDetails{
+				Type: probs.UnauthorizedProblem,
+				Detail: fmt.Sprintf("Invalid response from "+
+					"http://example.com/printf-verbs [127.0.0.1]: %q",
+					("%2F.well-known%2F" + expectedTruncatedResp.String())[:maxResponseSize]),
+				HTTPStatus: http.StatusForbidden,
+			},
+			ExpectedRecords: []core.ValidationRecord{
+				{
+					Hostname:          "example.com",
+					Port:              strconv.Itoa(httpPort),
+					URL:               "http://example.com/printf-verbs",
+					AddressesResolved: []net.IP{net.ParseIP("127.0.0.1")},
+					AddressUsed:       net.ParseIP("127.0.0.1"),
+				},
+			},
+		},
 	}

 	for _, tc := range testCases {
--- a/va/tlsalpn.go
+++ b/va/tlsalpn.go
@ -141,7 +141,7 @@ func (va *ValidationAuthorityImpl) getTLSCerts(
 	certs := cs.PeerCertificates
 	if len(certs) == 0 {
 		va.log.Infof("%s challenge for %s resulted in no certificates", challenge.Type, identifier.Value)
-		return nil, nil, probs.Unauthorized("No certs presented for %s challenge", challenge.Type)
+		return nil, nil, probs.Unauthorized(fmt.Sprintf("No certs presented for %s challenge", challenge.Type))
 	}
 	for i, cert := range certs {
 		va.log.AuditInfof("%s challenge for %s received certificate (%d of %d): cert=[%s]",
--- a/va/va.go
+++ b/va/va.go
@ -290,7 +290,7 @@ func detailedError(err error) *probs.ProblemDetails {
 		} else if netErr.Timeout() && netErr.Op == "dial" {
 			return probs.ConnectionFailure("Timeout during connect (likely firewall problem)")
 		} else if netErr.Timeout() {
-			return probs.ConnectionFailure("Timeout during %s (your server may be slow or overloaded)", netErr.Op)
+			return probs.ConnectionFailure(fmt.Sprintf("Timeout during %s (your server may be slow or overloaded)", netErr.Op))
 		}
 	}
 	if err, ok := err.(net.Error); ok && err.Timeout() {
--- a/va/va_test.go
+++ b/va/va_test.go
@ -325,9 +325,9 @@ func TestMultiVA(t *testing.T) {
 		"MultiVAFullResults": true,
 	}

-	unauthorized := probs.Unauthorized(
+	unauthorized := probs.Unauthorized(fmt.Sprintf(
 		`The key authorization file from the server did not match this challenge %q != "???"`,
-		expectedKeyAuthorization)
+		expectedKeyAuthorization))

 	expectedInternalErrLine := fmt.Sprintf(
 		`ERR: \[AUDIT\] Remote VA "broken".PerformValidation failed: %s`,
@ -412,9 +412,9 @@ func TestMultiVA(t *testing.T) {
 			RemoteVAs:  remoteVAs,
 			AllowedUAs: map[string]bool{localUA: true, remoteUA2: true},
 			Features:   enforceMultiVA,
-			ExpectedProb: probs.Unauthorized(
+			ExpectedProb: probs.Unauthorized(fmt.Sprintf(
 				`During secondary validation: The key authorization file from the server did not match this challenge %q != "???"`,
-				expectedKeyAuthorization),
+				expectedKeyAuthorization)),
 		},
 		{
 			// With one remote VA cancelled there should not be a validation failure
@ -454,9 +454,9 @@ func TestMultiVA(t *testing.T) {
 			RemoteVAs:  remoteVAs,
 			AllowedUAs: map[string]bool{localUA: true},
 			Features:   enforceMultiVAFullResults,
-			ExpectedProb: probs.Unauthorized(
+			ExpectedProb: probs.Unauthorized(fmt.Sprintf(
 				`During secondary validation: The key authorization file from the server did not match this challenge %q != "???"`,
-				expectedKeyAuthorization),
+				expectedKeyAuthorization)),
 		},
 	}

--- a/web/probs.go
+++ b/web/probs.go
@ -1,6 +1,8 @@
 package web

 import (
+	"fmt"
+
 	berrors "github.com/letsencrypt/boulder/errors"
 	"github.com/letsencrypt/boulder/probs"
 )
@ -10,35 +12,35 @@ func problemDetailsForBoulderError(err *berrors.BoulderError, msg string) *probs

 	switch err.Type {
 	case berrors.Malformed:
-		outProb = probs.Malformed("%s :: %s", msg, err)
+		outProb = probs.Malformed(fmt.Sprintf("%s :: %s", msg, err))
 	case berrors.Unauthorized:
-		outProb = probs.Unauthorized("%s :: %s", msg, err)
+		outProb = probs.Unauthorized(fmt.Sprintf("%s :: %s", msg, err))
 	case berrors.NotFound:
-		outProb = probs.NotFound("%s :: %s", msg, err)
+		outProb = probs.NotFound(fmt.Sprintf("%s :: %s", msg, err))
 	case berrors.RateLimit:
-		outProb = probs.RateLimited("%s :: %s", msg, err)
+		outProb = probs.RateLimited(fmt.Sprintf("%s :: %s", msg, err))
 	case berrors.InternalServer:
 		// Internal server error messages may include sensitive data, so we do
 		// not include it.
 		outProb = probs.ServerInternal(msg)
 	case berrors.RejectedIdentifier:
-		outProb = probs.RejectedIdentifier("%s :: %s", msg, err)
+		outProb = probs.RejectedIdentifier(fmt.Sprintf("%s :: %s", msg, err))
 	case berrors.InvalidEmail:
-		outProb = probs.InvalidEmail("%s :: %s", msg, err)
+		outProb = probs.InvalidEmail(fmt.Sprintf("%s :: %s", msg, err))
 	case berrors.WrongAuthorizationState:
-		outProb = probs.Malformed("%s :: %s", msg, err)
+		outProb = probs.Malformed(fmt.Sprintf("%s :: %s", msg, err))
 	case berrors.CAA:
-		outProb = probs.CAA("%s :: %s", msg, err)
+		outProb = probs.CAA(fmt.Sprintf("%s :: %s", msg, err))
 	case berrors.MissingSCTs:
 		// MissingSCTs are an internal server error, but with a specific error
 		// message related to the SCT problem
-		outProb = probs.ServerInternal("%s :: %s", msg, "Unable to meet CA SCT embedding requirements")
+		outProb = probs.ServerInternal(fmt.Sprintf("%s :: %s", msg, "Unable to meet CA SCT embedding requirements"))
 	case berrors.OrderNotReady:
-		outProb = probs.OrderNotReady("%s :: %s", msg, err)
+		outProb = probs.OrderNotReady(fmt.Sprintf("%s :: %s", msg, err))
 	case berrors.BadPublicKey:
-		outProb = probs.BadPublicKey("%s :: %s", msg, err)
+		outProb = probs.BadPublicKey(fmt.Sprintf("%s :: %s", msg, err))
 	case berrors.BadCSR:
-		outProb = probs.BadCSR("%s :: %s", msg, err)
+		outProb = probs.BadCSR(fmt.Sprintf("%s :: %s", msg, err))
 	default:
 		// Internal server error messages may include sensitive data, so we do
 		// not include it.
--- a/wfe/wfe.go
+++ b/wfe/wfe.go
@ -552,7 +552,7 @@ func (wfe *WebFrontEndImpl) verifyPOST(ctx context.Context, logEvent *web.Reques

 	// Only check for validity if we are actually checking the registration
 	if regCheck && reg.Status != core.StatusValid {
-		return nil, nil, reg, probs.Unauthorized("Registration is not valid, has status '%s'", reg.Status)
+		return nil, nil, reg, probs.Unauthorized(fmt.Sprintf("Registration is not valid, has status '%s'", reg.Status))
 	}

 	if statName, err := checkAlgorithm(key, parsedJws); err != nil {
@ -582,7 +582,7 @@ func (wfe *WebFrontEndImpl) verifyPOST(ctx context.Context, logEvent *web.Reques
 	if wfe.remoteNonceService != nil {
 		valid, err := nonce.RemoteRedeem(ctx, wfe.noncePrefixMap, nonceStr)
 		if err != nil {
-			return nil, nil, reg, probs.ServerInternal("failed to verify nonce validity: %s", err)
+			return nil, nil, reg, probs.ServerInternal(fmt.Sprintf("failed to verify nonce validity: %s", err))
 		}
 		nonceValid = valid
 	} else {
@ -590,7 +590,7 @@ func (wfe *WebFrontEndImpl) verifyPOST(ctx context.Context, logEvent *web.Reques
 	}
 	if !nonceValid {
 		wfe.joseErrorCounter.WithLabelValues("JWSInvalidNonce").Inc()
-		return nil, nil, reg, probs.BadNonce("JWS has invalid anti-replay nonce %s", nonceStr)
+		return nil, nil, reg, probs.BadNonce(fmt.Sprintf("JWS has invalid anti-replay nonce %s", nonceStr))
 	}

 	// Check that the "resource" field is present and has the correct value
@ -1275,8 +1275,8 @@ func (wfe *WebFrontEndImpl) Registration(ctx context.Context, logEvent *web.Requ
 	// extraneous requests to the RA we have to add this bypass.
 	if len(update.Agreement) > 0 && update.Agreement != currReg.Agreement &&
 		update.Agreement != wfe.SubscriberAgreementURL {
-		problem := probs.Malformed("Provided agreement URL [%s] does not match current agreement URL [%s]", update.Agreement, wfe.SubscriberAgreementURL)
-		wfe.sendError(response, logEvent, problem, nil)
+		msg := fmt.Sprintf("Provided agreement URL [%s] does not match current agreement URL [%s]", update.Agreement, wfe.SubscriberAgreementURL)
+		wfe.sendError(response, logEvent, probs.Malformed(msg), nil)
 		return
 	}

--- a/wfe2/stale.go
+++ b/wfe2/stale.go
@ -1,6 +1,7 @@
 package wfe2

 import (
+	"fmt"
 	"net/http"
 	"strings"
 	"time"
@ -63,11 +64,11 @@ func (wfe *WebFrontEndImpl) staleEnoughToGETAuthz(authz core.Authorization) *pro
 // wfe.staleTimeout then an unauthorized problem is returned.
 func (wfe *WebFrontEndImpl) staleEnoughToGET(resourceType string, createDate time.Time) *probs.ProblemDetails {
 	if wfe.clk.Since(createDate) < wfe.staleTimeout {
-		return probs.Unauthorized(
+		return probs.Unauthorized(fmt.Sprintf(
 			"%s is too new for GET API. "+
 				"You should only use this non-standard API to access resources created more than %s ago",
 			resourceType,
-			wfe.staleTimeout)
+			wfe.staleTimeout))
 	}
 	return nil
 }
--- a/wfe2/verify.go
+++ b/wfe2/verify.go
@ -101,7 +101,8 @@ func checkJWSAuthType(jws *jose.JSONWebSignature) (jwsAuthType, *probs.ProblemDe
 	header := jws.Signatures[0].Header
 	// There must not be a Key ID *and* an embedded JWK
 	if header.KeyID != "" && header.JSONWebKey != nil {
-		return invalidAuthType, probs.Malformed("jwk and kid header fields are mutually exclusive")
+		return invalidAuthType, probs.Malformed(
+			"jwk and kid header fields are mutually exclusive")
 	} else if header.KeyID != "" {
 		return embeddedKeyID, nil
 	} else if header.JSONWebKey != nil {
@ -150,13 +151,13 @@ func (wfe *WebFrontEndImpl) validPOSTRequest(request *http.Request) *probs.Probl
 	// JSON serialization.
 	if _, present := request.Header["Content-Type"]; !present {
 		wfe.stats.httpErrorCount.With(prometheus.Labels{"type": "NoContentType"}).Inc()
-		return probs.InvalidContentType("No Content-Type header on POST. Content-Type must be %q",
-			expectedJWSContentType)
+		return probs.InvalidContentType(fmt.Sprintf("No Content-Type header on POST. Content-Type must be %q",
+			expectedJWSContentType))
 	}
 	if contentType := request.Header.Get("Content-Type"); contentType != expectedJWSContentType {
 		wfe.stats.httpErrorCount.With(prometheus.Labels{"type": "WrongContentType"}).Inc()
-		return probs.InvalidContentType("Invalid Content-Type header on POST. Content-Type must be %q",
-			expectedJWSContentType)
+		return probs.InvalidContentType(fmt.Sprintf("Invalid Content-Type header on POST. Content-Type must be %q",
+			expectedJWSContentType))
 	}

 	// Per 6.4.1 "Replay-Nonce" clients should not send a Replay-Nonce header in
@ -191,7 +192,7 @@ func (wfe *WebFrontEndImpl) validNonce(ctx context.Context, jws *jose.JSONWebSig
 	if wfe.remoteNonceService != nil {
 		valid, err := nonce.RemoteRedeem(ctx, wfe.noncePrefixMap, header.Nonce)
 		if err != nil {
-			return probs.ServerInternal("failed to verify nonce validity: %s", err)
+			return probs.ServerInternal(fmt.Sprintf("failed to verify nonce validity: %s", err))
 		}
 		nonceValid = valid
 	} else {
@ -199,7 +200,7 @@ func (wfe *WebFrontEndImpl) validNonce(ctx context.Context, jws *jose.JSONWebSig
 	}
 	if !nonceValid {
 		wfe.stats.joseErrorCount.With(prometheus.Labels{"type": "JWSInvalidNonce"}).Inc()
-		return probs.BadNonce("JWS has an invalid anti-replay nonce: %q", header.Nonce)
+		return probs.BadNonce(fmt.Sprintf("JWS has an invalid anti-replay nonce: %q", header.Nonce))
 	}
 	return nil
 }
@ -236,8 +237,9 @@ func (wfe *WebFrontEndImpl) validPOSTURL(
 	// header
 	if expectedURL.String() != headerURL {
 		wfe.stats.joseErrorCount.With(prometheus.Labels{"type": "JWSMismatchedURL"}).Inc()
-		return probs.Malformed("JWS header parameter 'url' incorrect. Expected %q got %q",
-			expectedURL.String(), headerURL)
+		return probs.Malformed(fmt.Sprintf(
+			"JWS header parameter 'url' incorrect. Expected %q got %q",
+			expectedURL.String(), headerURL))
 	}
 	return nil
 }
@ -267,8 +269,9 @@ func (wfe *WebFrontEndImpl) matchJWSURLs(outer, inner *jose.JSONWebSignature) *p
 	// Verify that the outer URL matches the inner URL
 	if outerURL != innerURL {
 		wfe.stats.joseErrorCount.With(prometheus.Labels{"type": "KeyRolloverMismatchedURLs"}).Inc()
-		return probs.Malformed("Outer JWS 'url' value %q does not match inner JWS 'url' value %q",
-			outerURL, innerURL)
+		return probs.Malformed(fmt.Sprintf(
+			"Outer JWS 'url' value %q does not match inner JWS 'url' value %q",
+			outerURL, innerURL))
 	}

 	return nil
@ -299,7 +302,7 @@ func (wfe *WebFrontEndImpl) parseJWS(body []byte) (*jose.JSONWebSignature, *prob
 	if unprotected.Header != nil {
 		wfe.stats.joseErrorCount.With(prometheus.Labels{"type": "JWSUnprotectedHeaders"}).Inc()
 		return nil, probs.Malformed(
-			`JWS "header" field not allowed. All headers must be in "protected" field`)
+			"JWS \"header\" field not allowed. All headers must be in \"protected\" field")
 	}

 	// ACME v2 never uses the "signatures" array of JSON serialized JWS, just the
@ -307,7 +310,7 @@ func (wfe *WebFrontEndImpl) parseJWS(body []byte) (*jose.JSONWebSignature, *prob
 	if len(unprotected.Signatures) > 0 {
 		wfe.stats.joseErrorCount.With(prometheus.Labels{"type": "JWSMultiSig"}).Inc()
 		return nil, probs.Malformed(
-			`JWS "signatures" field not allowed. Only the "signature" field should contain a signature`)
+			"JWS \"signatures\" field not allowed. Only the \"signature\" field should contain a signature")
 	}

 	// Parse the JWS using go-jose and enforce that the expected one non-empty
@ -450,7 +453,8 @@ func (wfe *WebFrontEndImpl) lookupJWK(
 		// If the account isn't found, return a suitable problem
 		if berrors.Is(err, berrors.NotFound) {
 			wfe.stats.joseErrorCount.With(prometheus.Labels{"type": "JWSKeyIDNotFound"}).Inc()
-			return nil, nil, probs.AccountDoesNotExist("Account %q not found", accountURL)
+			return nil, nil, probs.AccountDoesNotExist(fmt.Sprintf(
+				"Account %q not found", accountURL))
 		}

 		// If there was an error and it isn't a "Not Found" error, return
@ -458,13 +462,15 @@ func (wfe *WebFrontEndImpl) lookupJWK(
 		wfe.stats.joseErrorCount.With(prometheus.Labels{"type": "JWSKeyIDLookupFailed"}).Inc()
 		// Add an error to the log event with the internal error message
 		logEvent.AddError(fmt.Sprintf("Error calling SA.GetRegistration: %s", err.Error()))
-		return nil, nil, probs.ServerInternal("Error retrieving account %q", accountURL)
+		return nil, nil, probs.ServerInternal(fmt.Sprintf(
+			"Error retrieving account %q", accountURL))
 	}

 	// Verify the account is not deactivated
 	if account.Status != core.StatusValid {
 		wfe.stats.joseErrorCount.With(prometheus.Labels{"type": "JWSKeyIDAccountInvalid"}).Inc()
-		return nil, nil, probs.Unauthorized("Account is not valid, has status %q", account.Status)
+		return nil, nil, probs.Unauthorized(
+			fmt.Sprintf("Account is not valid, has status %q", account.Status))
 	}

 	// Update the logEvent with the account information and return the JWK
--- a/wfe2/wfe.go
+++ b/wfe2/wfe.go
@ -2031,10 +2031,10 @@ func (wfe *WebFrontEndImpl) GetOrder(ctx context.Context, logEvent *web.RequestE
 	order, err := wfe.SA.GetOrder(ctx, &sapb.OrderRequest{Id: &orderID, UseV2Authorizations: &useV2Authzs})
 	if err != nil {
 		if berrors.Is(err, berrors.NotFound) {
-			wfe.sendError(response, logEvent, probs.NotFound("No order for ID %d", orderID), err)
+			wfe.sendError(response, logEvent, probs.NotFound(fmt.Sprintf("No order for ID %d", orderID)), err)
 			return
 		}
-		wfe.sendError(response, logEvent, probs.ServerInternal("Failed to retrieve order for ID %d", orderID), err)
+		wfe.sendError(response, logEvent, probs.ServerInternal(fmt.Sprintf("Failed to retrieve order for ID %d", orderID)), err)
 		return
 	}

@ -2046,7 +2046,7 @@ func (wfe *WebFrontEndImpl) GetOrder(ctx context.Context, logEvent *web.RequestE
 	}

 	if *order.RegistrationID != acctID {
-		wfe.sendError(response, logEvent, probs.NotFound("No order found for account ID %d", acctID), nil)
+		wfe.sendError(response, logEvent, probs.NotFound(fmt.Sprintf("No order found for account ID %d", acctID)), nil)
 		return
 	}

@ -2054,7 +2054,7 @@ func (wfe *WebFrontEndImpl) GetOrder(ctx context.Context, logEvent *web.RequestE
 	// POST-as-GET request and we need to verify the requesterAccount is the
 	// order's owner.
 	if requesterAccount != nil && *order.RegistrationID != requesterAccount.ID {
-		wfe.sendError(response, logEvent, probs.NotFound("No order found for account ID %d", acctID), nil)
+		wfe.sendError(response, logEvent, probs.NotFound(fmt.Sprintf("No order found for account ID %d", acctID)), nil)
 		return
 	}

@ -2101,22 +2101,22 @@ func (wfe *WebFrontEndImpl) FinalizeOrder(ctx context.Context, logEvent *web.Req
 	order, err := wfe.SA.GetOrder(ctx, &sapb.OrderRequest{Id: &orderID, UseV2Authorizations: &useV2Authzs})
 	if err != nil {
 		if berrors.Is(err, berrors.NotFound) {
-			wfe.sendError(response, logEvent, probs.NotFound("No order for ID %d", orderID), err)
+			wfe.sendError(response, logEvent, probs.NotFound(fmt.Sprintf("No order for ID %d", orderID)), err)
 			return
 		}
-		wfe.sendError(response, logEvent, probs.ServerInternal("Failed to retrieve order for ID %d", orderID), err)
+		wfe.sendError(response, logEvent, probs.ServerInternal(fmt.Sprintf("Failed to retrieve order for ID %d", orderID)), err)
 		return
 	}

 	if *order.RegistrationID != acctID {
-		wfe.sendError(response, logEvent, probs.NotFound("No order found for account ID %d", acctID), nil)
+		wfe.sendError(response, logEvent, probs.NotFound(fmt.Sprintf("No order found for account ID %d", acctID)), nil)
 		return
 	}

 	// If the authenticated account ID doesn't match the order's registration ID
 	// pretend it doesn't exist and abort.
 	if acct.ID != *order.RegistrationID {
-		wfe.sendError(response, logEvent, probs.NotFound("No order found for account ID %d", acct.ID), nil)
+		wfe.sendError(response, logEvent, probs.NotFound(fmt.Sprintf("No order found for account ID %d", acct.ID)), nil)
 		return
 	}

@ -2133,7 +2133,7 @@ func (wfe *WebFrontEndImpl) FinalizeOrder(ctx context.Context, logEvent *web.Req
 	// If the order is expired we can not finalize it and must return an error
 	orderExpiry := time.Unix(*order.Expires, 0)
 	if orderExpiry.Before(wfe.clk.Now()) {
-		wfe.sendError(response, logEvent, probs.NotFound("Order %d is expired", *order.Id), nil)
+		wfe.sendError(response, logEvent, probs.NotFound(fmt.Sprintf("Order %d is expired", *order.Id)), nil)
 		return
 	}