grpc: Pass data necessary for Retry-After headers in BoulderErrors (#6415)
- Add a new field, `RetryAfter` to `BoulderError`s - Add logic to wrap/unwrap the value of the `RetryAfter` field to our gRPC error interceptor - Plumb `RetryAfter` for `DuplicateCertificateError` emitted by RA to the WFE client response header Part of #6256
This commit is contained in:
Samantha
GitHub
parent
e7919df533
commit
bdd9ad9941
|
|
@ -12,6 +12,7 @@ package errors
|
|||
|
||||
import (
|
||||
"fmt"
|
||||
"time"
|
||||
|
||||
"github.com/letsencrypt/boulder/identifier"
|
||||
)
|
||||
|
|
@ -56,6 +57,10 @@ type BoulderError struct {
|
|||
Type ErrorType
|
||||
Detail string
|
||||
SubErrors []SubBoulderError
|
||||
|
||||
// RetryAfter the duration a client should wait before retrying the request
|
||||
// which resulted in this error.
|
||||
RetryAfter time.Duration
|
||||
}
|
||||
|
||||
// SubBoulderError represents sub-errors specific to an identifier that are
|
||||
|
|
@ -77,9 +82,10 @@ func (be *BoulderError) Unwrap() error {
|
|||
// provided subErrs to the existing BoulderError.
|
||||
func (be *BoulderError) WithSubErrors(subErrs []SubBoulderError) *BoulderError {
|
||||
return &BoulderError{
|
||||
Type: be.Type,
|
||||
Detail: be.Detail,
|
||||
SubErrors: append(be.SubErrors, subErrs...),
|
||||
Type: be.Type,
|
||||
Detail: be.Detail,
|
||||
SubErrors: append(be.SubErrors, subErrs...),
|
||||
RetryAfter: be.RetryAfter,
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -107,31 +113,35 @@ func NotFoundError(msg string, args ...interface{}) error {
|
|||
return New(NotFound, msg, args...)
|
||||
}
|
||||
|
||||
func RateLimitError(msg string, args ...interface{}) error {
|
||||
func RateLimitError(retryAfter time.Duration, msg string, args ...interface{}) error {
|
||||
return &BoulderError{
|
||||
Type: RateLimit,
|
||||
Detail: fmt.Sprintf(msg+": see https://letsencrypt.org/docs/rate-limits/", args...),
|
||||
Type: RateLimit,
|
||||
Detail: fmt.Sprintf(msg+": see https://letsencrypt.org/docs/rate-limits/", args...),
|
||||
RetryAfter: retryAfter,
|
||||
}
|
||||
}
|
||||
|
||||
func DuplicateCertificateError(msg string, args ...interface{}) error {
|
||||
func DuplicateCertificateError(retryAfter time.Duration, msg string, args ...interface{}) error {
|
||||
return &BoulderError{
|
||||
Type: RateLimit,
|
||||
Detail: fmt.Sprintf(msg+": see https://letsencrypt.org/docs/duplicate-certificate-limit/", args...),
|
||||
Type: RateLimit,
|
||||
Detail: fmt.Sprintf(msg+": see https://letsencrypt.org/docs/duplicate-certificate-limit/", args...),
|
||||
RetryAfter: retryAfter,
|
||||
}
|
||||
}
|
||||
|
||||
func FailedValidationError(msg string, args ...interface{}) error {
|
||||
func FailedValidationError(retryAfter time.Duration, msg string, args ...interface{}) error {
|
||||
return &BoulderError{
|
||||
Type: RateLimit,
|
||||
Detail: fmt.Sprintf(msg+": see https://letsencrypt.org/docs/failed-validation-limit/", args...),
|
||||
Type: RateLimit,
|
||||
Detail: fmt.Sprintf(msg+": see https://letsencrypt.org/docs/failed-validation-limit/", args...),
|
||||
RetryAfter: retryAfter,
|
||||
}
|
||||
}
|
||||
|
||||
func RegistrationsPerIPError(msg string, args ...interface{}) error {
|
||||
func RegistrationsPerIPError(retryAfter time.Duration, msg string, args ...interface{}) error {
|
||||
return &BoulderError{
|
||||
Type: RateLimit,
|
||||
Detail: fmt.Sprintf(msg+": see https://letsencrypt.org/docs/too-many-registrations-for-this-ip/", args...),
|
||||
Type: RateLimit,
|
||||
Detail: fmt.Sprintf(msg+": see https://letsencrypt.org/docs/too-many-registrations-for-this-ip/", args...),
|
||||
RetryAfter: retryAfter,
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -6,6 +6,7 @@ import (
|
|||
"errors"
|
||||
"fmt"
|
||||
"strconv"
|
||||
"time"
|
||||
|
||||
"google.golang.org/grpc"
|
||||
"google.golang.org/grpc/codes"
|
||||
|
|
@ -41,8 +42,13 @@ func wrapError(ctx context.Context, err error) error {
|
|||
"error marshaling json SubErrors, orig error %q",
|
||||
err)
|
||||
}
|
||||
pairs = append(pairs, "suberrors")
|
||||
pairs = append(pairs, string(jsonSubErrs))
|
||||
pairs = append(pairs, "suberrors", string(jsonSubErrs))
|
||||
}
|
||||
|
||||
// If there is a RetryAfter value then extend the metadata pairs to
|
||||
// include the value.
|
||||
if berr.RetryAfter != 0 {
|
||||
pairs = append(pairs, "retryafter", berr.RetryAfter.String())
|
||||
}
|
||||
|
||||
// Ignoring the error return here is safe because if setting the metadata
|
||||
|
|
@ -63,59 +69,75 @@ func unwrapError(err error, md metadata.MD) error {
|
|||
return nil
|
||||
}
|
||||
|
||||
unwrappedErr := status.Convert(err).Message()
|
||||
|
||||
errTypeStrs, ok := md["errortype"]
|
||||
if !ok {
|
||||
return err
|
||||
}
|
||||
|
||||
inErrMsg := status.Convert(err).Message()
|
||||
if len(errTypeStrs) != 1 {
|
||||
return berrors.InternalServerError(
|
||||
"multiple errorType metadata, wrapped error %q",
|
||||
unwrappedErr,
|
||||
"multiple 'errortype' metadata, wrapped error %q",
|
||||
inErrMsg,
|
||||
)
|
||||
}
|
||||
|
||||
errType, decErr := strconv.Atoi(errTypeStrs[0])
|
||||
inErrType, decErr := strconv.Atoi(errTypeStrs[0])
|
||||
if decErr != nil {
|
||||
return berrors.InternalServerError(
|
||||
"failed to decode error type, decoding error %q, wrapped error %q",
|
||||
decErr,
|
||||
unwrappedErr,
|
||||
inErrMsg,
|
||||
)
|
||||
}
|
||||
outErr := berrors.New(berrors.ErrorType(errType), unwrappedErr)
|
||||
|
||||
subErrsJSON, ok := md["suberrors"]
|
||||
if !ok {
|
||||
return outErr
|
||||
}
|
||||
if len(subErrsJSON) != 1 {
|
||||
return berrors.InternalServerError(
|
||||
"multiple suberrors metadata, wrapped error %q",
|
||||
unwrappedErr,
|
||||
)
|
||||
}
|
||||
|
||||
var suberrs []berrors.SubBoulderError
|
||||
err2 := json.Unmarshal([]byte(subErrsJSON[0]), &suberrs)
|
||||
if err2 != nil {
|
||||
return berrors.InternalServerError(
|
||||
"error unmarshaling suberrs JSON %q, wrapped error %q",
|
||||
subErrsJSON[0],
|
||||
unwrappedErr,
|
||||
)
|
||||
}
|
||||
|
||||
var berr *berrors.BoulderError
|
||||
if errors.As(outErr, &berr) {
|
||||
outErr = berr.WithSubErrors(suberrs)
|
||||
} else {
|
||||
inErr := berrors.New(berrors.ErrorType(inErrType), inErrMsg)
|
||||
var outErr *berrors.BoulderError
|
||||
if !errors.As(inErr, &outErr) {
|
||||
return fmt.Errorf(
|
||||
"expected type of outErr to be %T got %T: %q",
|
||||
berr, outErr,
|
||||
outErr.Error(),
|
||||
"expected type of inErr to be %T got %T: %q",
|
||||
outErr,
|
||||
inErr,
|
||||
inErr.Error(),
|
||||
)
|
||||
}
|
||||
|
||||
subErrorsVal, ok := md["suberrors"]
|
||||
if ok {
|
||||
if len(subErrorsVal) != 1 {
|
||||
return berrors.InternalServerError(
|
||||
"multiple 'suberrors' in metadata, wrapped error %q",
|
||||
inErrMsg,
|
||||
)
|
||||
}
|
||||
|
||||
unmarshalErr := json.Unmarshal([]byte(subErrorsVal[0]), &outErr.SubErrors)
|
||||
if unmarshalErr != nil {
|
||||
return berrors.InternalServerError(
|
||||
"JSON unmarshaling 'suberrors' %q, wrapped error %q: %s",
|
||||
subErrorsVal[0],
|
||||
inErrMsg,
|
||||
unmarshalErr,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
retryAfterVal, ok := md["retryafter"]
|
||||
if ok {
|
||||
if len(retryAfterVal) != 1 {
|
||||
return berrors.InternalServerError(
|
||||
"multiple 'retryafter' in metadata, wrapped error %q",
|
||||
inErrMsg,
|
||||
)
|
||||
}
|
||||
var parseErr error
|
||||
outErr.RetryAfter, parseErr = time.ParseDuration(retryAfterVal[0])
|
||||
if parseErr != nil {
|
||||
return berrors.InternalServerError(
|
||||
"parsing 'retryafter' as int64, wrapped error %q, parsing error: %s",
|
||||
inErrMsg,
|
||||
parseErr,
|
||||
)
|
||||
}
|
||||
}
|
||||
return outErr
|
||||
}
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@ package grpc
|
|||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"net"
|
||||
"testing"
|
||||
|
|
@ -47,10 +48,19 @@ func TestErrorWrapping(t *testing.T) {
|
|||
test.AssertNotError(t, err, "Failed to dial grpc test server")
|
||||
client := test_proto.NewChillerClient(conn)
|
||||
|
||||
es.err = berrors.MalformedError("yup")
|
||||
// RateLimitError with a RetryAfter of 500ms.
|
||||
expectRetryAfter := time.Millisecond * 500
|
||||
es.err = berrors.RateLimitError(expectRetryAfter, "yup")
|
||||
_, err = client.Chill(context.Background(), &test_proto.Time{})
|
||||
test.Assert(t, err != nil, fmt.Sprintf("nil error returned, expected: %s", err))
|
||||
test.AssertDeepEquals(t, err, es.err)
|
||||
var bErr *berrors.BoulderError
|
||||
ok := errors.As(err, &bErr)
|
||||
test.Assert(t, ok, "asserting error as boulder error")
|
||||
// Ensure we got a RateLimitError
|
||||
test.AssertErrorIs(t, bErr, berrors.RateLimit)
|
||||
// Ensure our RetryAfter is still 500ms.
|
||||
test.AssertEquals(t, bErr.RetryAfter, expectRetryAfter)
|
||||
|
||||
test.AssertNil(t, wrapError(context.Background(), nil), "Wrapping nil should still be nil")
|
||||
test.AssertNil(t, unwrapError(nil, nil), "Unwrapping nil should still be nil")
|
||||
|
|
|
|||
18
ra/ra.go
18
ra/ra.go
|
|
@ -328,7 +328,7 @@ func (ra *RegistrationAuthorityImpl) checkRegistrationIPLimit(ctx context.Contex
|
|||
}
|
||||
|
||||
if count.Count >= limit.GetThreshold(ip.String(), noRegistrationID) {
|
||||
return berrors.RegistrationsPerIPError("too many registrations for this IP")
|
||||
return berrors.RegistrationsPerIPError(0, "too many registrations for this IP")
|
||||
}
|
||||
|
||||
return nil
|
||||
|
|
@ -365,7 +365,7 @@ func (ra *RegistrationAuthorityImpl) checkRegistrationLimits(ctx context.Context
|
|||
ra.log.Infof("Rate limit exceeded, RegistrationsByIPRange, IP: %s", ip)
|
||||
// For the fuzzyRegLimit we use a new error message that specifically
|
||||
// mentions that the limit being exceeded is applied to a *range* of IPs
|
||||
return berrors.RateLimitError("too many registrations for this IP range")
|
||||
return berrors.RateLimitError(0, "too many registrations for this IP range")
|
||||
}
|
||||
ra.rateLimitCounter.WithLabelValues("registrations_by_ip_range", "pass").Inc()
|
||||
|
||||
|
|
@ -515,7 +515,7 @@ func (ra *RegistrationAuthorityImpl) checkPendingAuthorizationLimit(ctx context.
|
|||
if countPB.Count >= limit.GetThreshold(noKey, regID) {
|
||||
ra.rateLimitCounter.WithLabelValues("pending_authorizations_by_registration_id", "exceeded").Inc()
|
||||
ra.log.Infof("Rate limit exceeded, PendingAuthorizationsByRegID, regID: %d", regID)
|
||||
return berrors.RateLimitError(fmt.Sprintf("too many currently pending authorizations: %d", countPB.Count))
|
||||
return berrors.RateLimitError(0, "too many currently pending authorizations: %d", countPB.Count)
|
||||
}
|
||||
ra.rateLimitCounter.WithLabelValues("pending_authorizations_by_registration_id", "pass").Inc()
|
||||
}
|
||||
|
|
@ -567,7 +567,7 @@ func (ra *RegistrationAuthorityImpl) checkInvalidAuthorizationLimit(ctx context.
|
|||
noKey := ""
|
||||
if count.Count >= limit.GetThreshold(noKey, regID) {
|
||||
ra.log.Infof("Rate limit exceeded, InvalidAuthorizationsByRegID, regID: %d", regID)
|
||||
return berrors.FailedValidationError("too many failed authorizations recently")
|
||||
return berrors.FailedValidationError(0, "too many failed authorizations recently")
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
|
@ -595,7 +595,7 @@ func (ra *RegistrationAuthorityImpl) checkNewOrdersPerAccountLimit(ctx context.C
|
|||
noKey := ""
|
||||
if count.Count >= limit.GetThreshold(noKey, acctID) {
|
||||
ra.rateLimitCounter.WithLabelValues("new_order_by_registration_id", "exceeded").Inc()
|
||||
return berrors.RateLimitError("too many new orders recently")
|
||||
return berrors.RateLimitError(0, "too many new orders recently")
|
||||
}
|
||||
ra.rateLimitCounter.WithLabelValues("new_order_by_registration_id", "pass").Inc()
|
||||
return nil
|
||||
|
|
@ -1347,12 +1347,12 @@ func (ra *RegistrationAuthorityImpl) checkCertificatesPerNameLimit(ctx context.C
|
|||
for _, name := range namesOutOfLimit {
|
||||
subErrors = append(subErrors, berrors.SubBoulderError{
|
||||
Identifier: identifier.DNSIdentifier(name),
|
||||
BoulderError: berrors.RateLimitError("too many certificates already issued").(*berrors.BoulderError),
|
||||
BoulderError: berrors.RateLimitError(0, "too many certificates already issued").(*berrors.BoulderError),
|
||||
})
|
||||
}
|
||||
return berrors.RateLimitError("too many certificates already issued for multiple names (%s and %d others)", namesOutOfLimit[0], len(namesOutOfLimit)).(*berrors.BoulderError).WithSubErrors(subErrors)
|
||||
return berrors.RateLimitError(0, "too many certificates already issued for multiple names (%s and %d others)", namesOutOfLimit[0], len(namesOutOfLimit)).(*berrors.BoulderError).WithSubErrors(subErrors)
|
||||
}
|
||||
return berrors.RateLimitError("too many certificates already issued for: %s", namesOutOfLimit[0])
|
||||
return berrors.RateLimitError(0, "too many certificates already issued for: %s", namesOutOfLimit[0])
|
||||
}
|
||||
ra.rateLimitCounter.WithLabelValues("certificates_for_domain", "pass").Inc()
|
||||
|
||||
|
|
@ -1394,7 +1394,9 @@ func (ra *RegistrationAuthorityImpl) checkCertificatesPerFQDNSetLimit(ctx contex
|
|||
}
|
||||
}
|
||||
retryTime := time.Unix(0, prevIssuances.Timestamps[0]).Add(time.Duration(nsPerToken))
|
||||
retryAfter := retryTime.Sub(now)
|
||||
return berrors.DuplicateCertificateError(
|
||||
retryAfter,
|
||||
"too many certificates (%d) already issued for this exact set of domains in the last %.0f hours: %s, retry after %s",
|
||||
threshold, limit.Window.Duration.Hours(), strings.Join(names, ","), retryTime.Format(time.RFC3339),
|
||||
)
|
||||
|
|
|
|||
|
|
@ -310,8 +310,7 @@ func (va *ValidationAuthorityImpl) extractRequestTarget(req *http.Request) (stri
|
|||
// Check that the request host isn't a bare IP address. We only follow
|
||||
// redirects to hostnames.
|
||||
if net.ParseIP(reqHost) != nil {
|
||||
return "", 0, berrors.ConnectionFailureError(
|
||||
"Invalid host in redirect target %q. Only domain names are supported, not IP addresses", reqHost)
|
||||
return "", 0, berrors.ConnectionFailureError("Invalid host in redirect target %q. Only domain names are supported, not IP addresses", reqHost)
|
||||
}
|
||||
|
||||
// Often folks will misconfigure their webserver to send an HTTP redirect
|
||||
|
|
@ -331,8 +330,7 @@ func (va *ValidationAuthorityImpl) extractRequestTarget(req *http.Request) (stri
|
|||
}
|
||||
|
||||
if _, err := iana.ExtractSuffix(reqHost); err != nil {
|
||||
return "", 0, berrors.ConnectionFailureError(
|
||||
"Invalid hostname in redirect target, must end in IANA registered TLD")
|
||||
return "", 0, berrors.ConnectionFailureError("Invalid hostname in redirect target, must end in IANA registered TLD")
|
||||
}
|
||||
|
||||
return reqHost, reqPort, nil
|
||||
|
|
|
|||
|
|
@ -34,7 +34,7 @@ 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.RateLimitError(detailMsg), 429, probs.RateLimitedProblem, fullDetail + ": see https://letsencrypt.org/docs/rate-limits/"},
|
||||
{berrors.RateLimitError(0, detailMsg), 429, probs.RateLimitedProblem, fullDetail + ": see https://letsencrypt.org/docs/rate-limits/"},
|
||||
{berrors.InvalidEmailError(detailMsg), 400, probs.InvalidEmailProblem, fullDetail},
|
||||
{berrors.RejectedIdentifierError(detailMsg), 400, probs.RejectedIdentifierProblem, fullDetail},
|
||||
}
|
||||
|
|
|
|||
14
wfe2/wfe.go
14
wfe2/wfe.go
|
|
@ -74,6 +74,8 @@ const (
|
|||
|
||||
// Non-ACME paths
|
||||
aiaIssuerPath = "/aia/issuer/"
|
||||
|
||||
headerRetryAfter = "Retry-After"
|
||||
)
|
||||
|
||||
var errIncompleteGRPCResponse = errors.New("incomplete gRPC response message")
|
||||
|
|
@ -584,6 +586,16 @@ func (wfe *WebFrontEndImpl) Nonce(
|
|||
|
||||
// sendError wraps web.SendError
|
||||
func (wfe *WebFrontEndImpl) sendError(response http.ResponseWriter, logEvent *web.RequestEvent, prob *probs.ProblemDetails, ierr error) {
|
||||
var bErr *berrors.BoulderError
|
||||
if errors.As(ierr, &bErr) {
|
||||
retryAfterSeconds := int(bErr.RetryAfter.Round(time.Second).Seconds())
|
||||
if retryAfterSeconds > 0 {
|
||||
response.Header().Add(headerRetryAfter, strconv.Itoa(retryAfterSeconds))
|
||||
if bErr.Type == berrors.RateLimit {
|
||||
response.Header().Add("Link", link("https://letsencrypt.org/docs/rate-limits", "help"))
|
||||
}
|
||||
}
|
||||
}
|
||||
wfe.stats.httpErrorCount.With(prometheus.Labels{"type": string(prob.Type)}).Inc()
|
||||
web.SendError(wfe.log, probs.V2ErrorNS, response, logEvent, prob, ierr)
|
||||
}
|
||||
|
|
@ -2304,7 +2316,7 @@ func (wfe *WebFrontEndImpl) RenewalInfo(ctx context.Context, logEvent *web.Reque
|
|||
beeline.AddFieldToTrace(ctx, "request.serial", serial)
|
||||
|
||||
setDefaultRetryAfterHeader := func(response http.ResponseWriter) {
|
||||
response.Header().Set("Retry-After", fmt.Sprintf("%d", int(6*time.Hour/time.Second)))
|
||||
response.Header().Set(headerRetryAfter, fmt.Sprintf("%d", int(6*time.Hour/time.Second)))
|
||||
}
|
||||
|
||||
// Check if the serial is part of an ongoing incident.
|
||||
|
|
|
|||
|
|
@ -3701,3 +3701,34 @@ func TestOldTLSInbound(t *testing.T) {
|
|||
wfe.Handler(metrics.NoopRegisterer).ServeHTTP(responseWriter, req)
|
||||
test.AssertEquals(t, responseWriter.Code, http.StatusBadRequest)
|
||||
}
|
||||
|
||||
func Test_sendError(t *testing.T) {
|
||||
features.Reset()
|
||||
wfe, _ := setupWFE(t)
|
||||
testResponse := httptest.NewRecorder()
|
||||
|
||||
testErr := berrors.RateLimitError(0, "test")
|
||||
wfe.sendError(testResponse, &web.RequestEvent{Endpoint: "test"}, probs.RateLimited("test"), testErr)
|
||||
// Ensure a 0 value RetryAfter results in no Retry-After header.
|
||||
test.AssertEquals(t, testResponse.Header().Get("Retry-After"), "")
|
||||
// Ensure the Link header isn't populatsed.
|
||||
test.AssertEquals(t, testResponse.Header().Get("Link"), "")
|
||||
|
||||
testErr = berrors.RateLimitError(time.Millisecond*500, "test")
|
||||
wfe.sendError(testResponse, &web.RequestEvent{Endpoint: "test"}, probs.RateLimited("test"), testErr)
|
||||
// Ensure a 500ms RetryAfter is rounded up to a 1s Retry-After header.
|
||||
test.AssertEquals(t, testResponse.Header().Get("Retry-After"), "1")
|
||||
// Ensure the Link header is populated.
|
||||
test.AssertEquals(t, testResponse.Header().Get("Link"), "<https://letsencrypt.org/docs/rate-limits>;rel=\"help\"")
|
||||
|
||||
// Clear headers for the next test.
|
||||
testResponse.Header().Del("Retry-After")
|
||||
testResponse.Header().Del("Link")
|
||||
|
||||
testErr = berrors.RateLimitError(time.Millisecond*499, "test")
|
||||
wfe.sendError(testResponse, &web.RequestEvent{Endpoint: "test"}, probs.RateLimited("test"), testErr)
|
||||
// Ensure a 499ms RetryAfter results in no Retry-After header.
|
||||
test.AssertEquals(t, testResponse.Header().Get("Retry-After"), "")
|
||||
// Ensure the Link header isn't populatsed.
|
||||
test.AssertEquals(t, testResponse.Header().Get("Link"), "")
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue