diff --git a/probs/probs.go b/probs/probs.go index f9307aec5..aa436228f 100644 --- 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, } } diff --git a/va/caa.go b/va/caa.go index 62f37b6a7..218bf08ec 100644 --- 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 } diff --git a/va/dns.go b/va/dns.go index ef5ca9c10..39d7c6952 100644 --- 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)) } diff --git a/va/http.go b/va/http.go index f24946836..0727668cc 100644 --- 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 } diff --git a/va/http_test.go b/va/http_test.go index 027fca89f..457f59f3c 100644 --- 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 { diff --git a/va/tlsalpn.go b/va/tlsalpn.go index e0741cc39..481ade752 100644 --- 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]", diff --git a/va/va.go b/va/va.go index 9371ad41f..d3a693935 100644 --- 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() { diff --git a/va/va_test.go b/va/va_test.go index 6a037d7e8..2b4b1ed64 100644 --- 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)), }, } diff --git a/web/probs.go b/web/probs.go index d3374b5a8..2e4576209 100644 --- 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. diff --git a/wfe/wfe.go b/wfe/wfe.go index 8f702e79b..f2f2f61a8 100644 --- 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 } diff --git a/wfe2/stale.go b/wfe2/stale.go index 5121e63fc..28e24c824 100644 --- 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 } diff --git a/wfe2/verify.go b/wfe2/verify.go index b823b5d3f..5f5fa97ff 100644 --- 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 diff --git a/wfe2/wfe.go b/wfe2/wfe.go index 59f01539f..164f77771 100644 --- 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 }