Clean up berrors (letsencrypt#2724)

This PR removes two berrors that aren't used anywhere in the codebase:

TooManyRequests , a holdover from AMQP, and is no longer used.
UnsupportedIdentifier, used just for rejecting IDNs, which we no longer do.
In addition, the SignatureValidation error was only used by the WFE so it is moved there and unexported.

Note for reviewers: To remove berrors.UnsupportedIdentifierError I replaced the errIDNNotSupported error in policy/pa.go with a berrors.MalformedError with the same name. This allows removing UnsupportedIdentifierError ahead of letsencrypt#2712 which removes the IDNASupport feature flag. This seemed OK to me, but I can restore UnsupportedIdentifierError and clean it up after 2712 if that's preferred.

Resolves letsencrypt#2709

core/core_test.go

@@ -96,7 +96,6 @@ func TestErrors(t *testing.T) {
 		MalformedRequestError(testMessage),
 		UnauthorizedError(testMessage),
 		NotFoundError(testMessage),
-		SignatureValidationError(testMessage),
 	}
 
 	for i, err := range errors {

core/util.go

@@ -69,11 +69,6 @@ type NotFoundError string
 // LengthRequiredError indicates a POST was sent with no Content-Length.
 type LengthRequiredError string
 
-// SignatureValidationError indicates that the user's signature could not
-// be verified, either through adversarial activity, or misconfiguration of
-// the user client.
-type SignatureValidationError string
-
 // NoSuchRegistrationError indicates that a registration could not be found.
 type NoSuchRegistrationError string
 
@@ -87,17 +82,16 @@ type TooManyRPCRequestsError string
 // BadNonceError indicates an empty of invalid nonce was provided
 type BadNonceError string
 
-func (e InternalServerError) Error() string      { return string(e) }
-func (e NotSupportedError) Error() string        { return string(e) }
-func (e MalformedRequestError) Error() string    { return string(e) }
-func (e UnauthorizedError) Error() string        { return string(e) }
-func (e NotFoundError) Error() string            { return string(e) }
-func (e LengthRequiredError) Error() string      { return string(e) }
-func (e SignatureValidationError) Error() string { return string(e) }
-func (e NoSuchRegistrationError) Error() string  { return string(e) }
-func (e RateLimitedError) Error() string         { return string(e) }
-func (e TooManyRPCRequestsError) Error() string  { return string(e) }
-func (e BadNonceError) Error() string            { return string(e) }
+func (e InternalServerError) Error() string     { return string(e) }
+func (e NotSupportedError) Error() string       { return string(e) }
+func (e MalformedRequestError) Error() string   { return string(e) }
+func (e UnauthorizedError) Error() string       { return string(e) }
+func (e NotFoundError) Error() string           { return string(e) }
+func (e LengthRequiredError) Error() string     { return string(e) }
+func (e NoSuchRegistrationError) Error() string { return string(e) }
+func (e RateLimitedError) Error() string        { return string(e) }
+func (e TooManyRPCRequestsError) Error() string { return string(e) }
+func (e BadNonceError) Error() string           { return string(e) }
 
 // Random stuff
 

errors/errors.go

@@ -11,11 +11,8 @@ const (
 	Malformed
 	Unauthorized
 	NotFound
-	SignatureValidation
 	RateLimit
-	TooManyRequests
 	RejectedIdentifier
-	UnsupportedIdentifier
 	InvalidEmail
 	ConnectionFailure
 )
@@ -67,26 +64,14 @@ func NotFoundError(msg string, args ...interface{}) error {
 	return New(NotFound, msg, args...)
 }
 
-func SignatureValidationError(msg string, args ...interface{}) error {
-	return New(SignatureValidation, msg, args...)
-}
-
 func RateLimitError(msg string, args ...interface{}) error {
 	return New(RateLimit, msg, args...)
 }
 
-func TooManyRequestsError(msg string, args ...interface{}) error {
-	return New(TooManyRequests, msg, args...)
-}
-
 func RejectedIdentifierError(msg string, args ...interface{}) error {
 	return New(RejectedIdentifier, msg, args...)
 }
 
-func UnsupportedIdentifierError(msg string, args ...interface{}) error {
-	return New(UnsupportedIdentifier, msg, args...)
-}
-
 func InvalidEmailError(msg string, args ...interface{}) error {
 	return New(InvalidEmail, msg, args...)
 }

grpc/bcodes.go

@@ -25,7 +25,6 @@ const (
 	UnauthorizedError
 	NotFoundError
 	LengthRequiredError
-	SignatureValidationError
 	RateLimitedError
 	BadNonceError
 	NoSuchRegistrationError
@@ -50,8 +49,6 @@ func errorToCode(err error) codes.Code {
 		return NotFoundError
 	case core.LengthRequiredError:
 		return LengthRequiredError
-	case core.SignatureValidationError:
-		return SignatureValidationError
 	case core.RateLimitedError:
 		return RateLimitedError
 	case core.BadNonceError:
@@ -144,8 +141,6 @@ func unwrapError(err error, md metadata.MD) error {
 		return core.UnauthorizedError(errBody)
 	case NotFoundError:
 		return core.NotFoundError(errBody)
-	case SignatureValidationError:
-		return core.SignatureValidationError(errBody)
 	case NoSuchRegistrationError:
 		return core.NoSuchRegistrationError(errBody)
 	case RateLimitedError:

grpc/bcodes_test.go

@@ -21,7 +21,6 @@ func TestErrors(t *testing.T) {
 		{core.UnauthorizedError("test 3"), UnauthorizedError},
 		{core.NotFoundError("test 4"), NotFoundError},
 		{core.LengthRequiredError("test 5"), LengthRequiredError},
-		{core.SignatureValidationError("test 6"), SignatureValidationError},
 		{core.RateLimitedError("test 7"), RateLimitedError},
 		{core.BadNonceError("test 8"), BadNonceError},
 		{core.NoSuchRegistrationError("test 9"), NoSuchRegistrationError},

policy/pa.go

@@ -141,8 +141,9 @@ var (
 	errTooFewLabels        = berrors.MalformedError("DNS name does not have enough labels")
 	errLabelTooShort       = berrors.MalformedError("DNS label is too short")
 	errLabelTooLong        = berrors.MalformedError("DNS label is too long")
-	errIDNNotSupported     = berrors.UnsupportedIdentifierError("Internationalized domain names (starting with xn--) not yet supported")
-	errMalformedIDN        = berrors.MalformedError("DNS label contains malformed punycode")
+	// TODO(@cpu): Delete `errIDNNotSupported` when IDNASupport feature flag is removed.
+	errIDNNotSupported = berrors.MalformedError("Internationalized domain names (starting with xn--) not yet supported")
+	errMalformedIDN    = berrors.MalformedError("DNS label contains malformed punycode")
 )
 
 // WillingToIssue determines whether the CA is willing to issue for the provided

probs/probs.go

@@ -7,17 +7,16 @@ import (
 
 // Error types that can be used in ACME payloads
 const (
-	ConnectionProblem            = ProblemType("urn:acme:error:connection")
-	MalformedProblem             = ProblemType("urn:acme:error:malformed")
-	ServerInternalProblem        = ProblemType("urn:acme:error:serverInternal")
-	TLSProblem                   = ProblemType("urn:acme:error:tls")
-	UnauthorizedProblem          = ProblemType("urn:acme:error:unauthorized")
-	UnknownHostProblem           = ProblemType("urn:acme:error:unknownHost")
-	RateLimitedProblem           = ProblemType("urn:acme:error:rateLimited")
-	BadNonceProblem              = ProblemType("urn:acme:error:badNonce")
-	InvalidEmailProblem          = ProblemType("urn:acme:error:invalidEmail")
-	RejectedIdentifierProblem    = ProblemType("urn:acme:error:rejectedIdentifier")
-	UnsupportedIdentifierProblem = ProblemType("urn:acme:error:unsupportedIdentifier")
+	ConnectionProblem         = ProblemType("urn:acme:error:connection")
+	MalformedProblem          = ProblemType("urn:acme:error:malformed")
+	ServerInternalProblem     = ProblemType("urn:acme:error:serverInternal")
+	TLSProblem                = ProblemType("urn:acme:error:tls")
+	UnauthorizedProblem       = ProblemType("urn:acme:error:unauthorized")
+	UnknownHostProblem        = ProblemType("urn:acme:error:unknownHost")
+	RateLimitedProblem        = ProblemType("urn:acme:error:rateLimited")
+	BadNonceProblem           = ProblemType("urn:acme:error:badNonce")
+	InvalidEmailProblem       = ProblemType("urn:acme:error:invalidEmail")
+	RejectedIdentifierProblem = ProblemType("urn:acme:error:rejectedIdentifier")
 )
 
 // ProblemType defines the error types in the ACME protocol
@@ -49,7 +48,7 @@ func ProblemDetailsToStatusCode(prob *ProblemDetails) int {
 		return prob.HTTPStatus
 	}
 	switch prob.Type {
-	case ConnectionProblem, MalformedProblem, TLSProblem, UnknownHostProblem, BadNonceProblem, InvalidEmailProblem, RejectedIdentifierProblem, UnsupportedIdentifierProblem:
+	case ConnectionProblem, MalformedProblem, TLSProblem, UnknownHostProblem, BadNonceProblem, InvalidEmailProblem, RejectedIdentifierProblem:
 		return http.StatusBadRequest
 	case ServerInternalProblem:
 		return http.StatusInternalServerError
@@ -82,16 +81,6 @@ func RejectedIdentifier(detail string) *ProblemDetails {
 	}
 }
 
-// UnsupportedIdentifier returns a ProblemDetails with a UnsupportedIdentifierProblem and a 400 Bad
-// Request status code.
-func UnsupportedIdentifier(detail string) *ProblemDetails {
-	return &ProblemDetails{
-		Type:       UnsupportedIdentifierProblem,
-		Detail:     detail,
-		HTTPStatus: http.StatusBadRequest,
-	}
-}
-
 // Conflict returns a ProblemDetails with a MalformedProblem and a 409 Conflict
 // status code.
 func Conflict(detail string) *ProblemDetails {

probs/probs_test.go

@@ -61,7 +61,6 @@ func TestProblemDetailsConvenience(t *testing.T) {
 		{BadNonce("bad nonce detail"), BadNonceProblem, http.StatusBadRequest, "bad nonce detail"},
 		{TLSError("TLS error detail"), TLSProblem, http.StatusBadRequest, "TLS error detail"},
 		{RejectedIdentifier("rejected identifier detail"), RejectedIdentifierProblem, http.StatusBadRequest, "rejected identifier detail"},
-		{UnsupportedIdentifier("unsupported identifier detail"), UnsupportedIdentifierProblem, http.StatusBadRequest, "unsupported identifier detail"},
 	}
 
 	for _, c := range testCases {

wfe/jose.go

@@ -3,10 +3,9 @@ package wfe
 import (
 	"crypto/ecdsa"
 	"crypto/rsa"
+	"fmt"
 
 	"gopkg.in/square/go-jose.v1"
-
-	berrors "github.com/letsencrypt/boulder/errors"
 )
 
 func algorithmForKey(key *jose.JsonWebKey) (string, error) {
@@ -23,7 +22,7 @@ func algorithmForKey(key *jose.JsonWebKey) (string, error) {
 			return string(jose.ES512), nil
 		}
 	}
-	return "", berrors.SignatureValidationError("no signature algorithms suitable for given key type")
+	return "", signatureValidationError("no signature algorithms suitable for given key type")
 }
 
 const (
@@ -44,16 +43,16 @@ func checkAlgorithm(key *jose.JsonWebKey, parsedJws *jose.JsonWebSignature) (str
 	}
 	jwsAlgorithm := parsedJws.Signatures[0].Header.Algorithm
 	if jwsAlgorithm != algorithm {
-		return invalidJWSAlgorithm, berrors.SignatureValidationError(
+		return invalidJWSAlgorithm, signatureValidationError(fmt.Sprintf(
 			"signature type '%s' in JWS header is not supported, expected one of RS256, ES256, ES384 or ES512",
 			jwsAlgorithm,
-		)
+		))
 	}
 	if key.Algorithm != "" && key.Algorithm != algorithm {
-		return invalidAlgorithmOnKey, berrors.SignatureValidationError(
+		return invalidAlgorithmOnKey, signatureValidationError(fmt.Sprintf(
 			"algorithm '%s' on JWK is unacceptable",
 			key.Algorithm,
-		)
+		))
 	}
 	return "", nil
 }

wfe/probs.go

@@ -17,22 +17,20 @@ func problemDetailsForBoulderError(err *berrors.BoulderError, msg string) *probs
 			Detail:     fmt.Sprintf("%s :: %s", msg, err),
 			HTTPStatus: http.StatusNotImplemented,
 		}
-	case berrors.Malformed, berrors.SignatureValidation:
+	case berrors.Malformed:
 		return probs.Malformed(fmt.Sprintf("%s :: %s", msg, err))
 	case berrors.Unauthorized:
 		return probs.Unauthorized(fmt.Sprintf("%s :: %s", msg, err))
 	case berrors.NotFound:
 		return probs.NotFound(fmt.Sprintf("%s :: %s", msg, err))
 	case berrors.RateLimit:
 		return probs.RateLimited(fmt.Sprintf("%s :: %s", msg, err))
-	case berrors.InternalServer, berrors.TooManyRequests:
+	case berrors.InternalServer:
 		// Internal server error messages may include sensitive data, so we do
 		// not include it.
 		return probs.ServerInternal(msg)
 	case berrors.RejectedIdentifier:
 		return probs.RejectedIdentifier(fmt.Sprintf("%s :: %s", msg, err))
-	case berrors.UnsupportedIdentifier:
-		return probs.UnsupportedIdentifier(msg)
 	case berrors.InvalidEmail:
 		return probs.InvalidEmail(fmt.Sprintf("%s :: %s", msg, err))
 	default:
@@ -52,6 +50,8 @@ func problemDetailsForError(err error, msg string) *probs.ProblemDetails {
 		return e
 	case *berrors.BoulderError:
 		return problemDetailsForBoulderError(e, msg)
+	case signatureValidationError:
+		return probs.Malformed(fmt.Sprintf("%s :: %s", msg, err))
 	case core.MalformedRequestError:
 		return probs.Malformed(fmt.Sprintf("%s :: %s", msg, err))
 	case core.NotSupportedError:
@@ -68,8 +68,6 @@ func problemDetailsForError(err error, msg string) *probs.ProblemDetails {
 		prob := probs.Malformed("missing Content-Length header")
 		prob.HTTPStatus = http.StatusLengthRequired
 		return prob
-	case core.SignatureValidationError:
-		return probs.Malformed(fmt.Sprintf("%s :: %s", msg, err))
 	case core.RateLimitedError:
 		return probs.RateLimited(fmt.Sprintf("%s :: %s", msg, err))
 	case core.BadNonceError:

wfe/probs_test.go

@@ -35,7 +35,7 @@ func TestProblemDetailsFromError(t *testing.T) {
 		{core.MalformedRequestError(detailMsg), 400, probs.MalformedProblem, fullDetail},
 		{core.UnauthorizedError(detailMsg), 403, probs.UnauthorizedProblem, fullDetail},
 		{core.NotFoundError(detailMsg), 404, probs.MalformedProblem, fullDetail},
-		{core.SignatureValidationError(detailMsg), 400, probs.MalformedProblem, fullDetail},
+		{signatureValidationError(detailMsg), 400, probs.MalformedProblem, fullDetail},
 		{core.RateLimitedError(detailMsg), 429, probs.RateLimitedProblem, fullDetail},
 		{core.BadNonceError(detailMsg), 400, probs.BadNonceProblem, fullDetail},
 		//    The content length error has its own specific detail message
@@ -48,7 +48,6 @@ func TestProblemDetailsFromError(t *testing.T) {
 		{berrors.MalformedError(detailMsg), 400, probs.MalformedProblem, fullDetail},
 		{berrors.UnauthorizedError(detailMsg), 403, probs.UnauthorizedProblem, fullDetail},
 		{berrors.NotFoundError(detailMsg), 404, probs.MalformedProblem, fullDetail},
-		{berrors.SignatureValidationError(detailMsg), 400, probs.MalformedProblem, fullDetail},
 		{berrors.RateLimitError(detailMsg), 429, probs.RateLimitedProblem, fullDetail},
 		{berrors.InvalidEmailError(detailMsg), 400, probs.InvalidEmailProblem, fullDetail},
 		{berrors.RejectedIdentifierError(detailMsg), 400, probs.RejectedIdentifierProblem, fullDetail},

wfe/wfe.go

@@ -96,6 +96,13 @@ type WebFrontEndImpl struct {
 	AllowAuthzDeactivation bool
 }
 
+// signatureValidationError indicates that the user's signature could not
+// be verified, either through adversarial activity, or misconfiguration of
+// the user client.
+type signatureValidationError string
+
+func (e signatureValidationError) Error() string { return string(e) }
+
 // NewWebFrontEndImpl constructs a web service for Boulder
 func NewWebFrontEndImpl(
 	stats metrics.Scope,