Skip to content

Commit bdd9ad9

Browse files
beautifulentropybeautifulentropy
authored
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
1 parent e7919df commit bdd9ad9

File tree

8 files changed

+150
-65
lines changed

8 files changed

+150
-65
lines changed

‎errors/errors.go‎

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ package errors
1212

1313
import (
1414
"fmt"
15+
"time"
1516

1617
"github.com/letsencrypt/boulder/identifier"
1718
)
@@ -56,6 +57,10 @@ type BoulderError struct {
5657
Type ErrorType
5758
Detail string
5859
SubErrors []SubBoulderError
60+
61+
// RetryAfter the duration a client should wait before retrying the request
62+
// which resulted in this error.
63+
RetryAfter time.Duration
5964
}
6065

6166
// SubBoulderError represents sub-errors specific to an identifier that are
@@ -77,9 +82,10 @@ func (be *BoulderError) Unwrap() error {
7782
// provided subErrs to the existing BoulderError.
7883
func (be *BoulderError) WithSubErrors(subErrs []SubBoulderError) *BoulderError {
7984
return &BoulderError{
80-
Type: be.Type,
81-
Detail: be.Detail,
82-
SubErrors: append(be.SubErrors, subErrs...),
85+
Type: be.Type,
86+
Detail: be.Detail,
87+
SubErrors: append(be.SubErrors, subErrs...),
88+
RetryAfter: be.RetryAfter,
8389
}
8490
}
8591

@@ -107,31 +113,35 @@ func NotFoundError(msg string, args ...interface{}) error {
107113
return New(NotFound, msg, args...)
108114
}
109115

110-
func RateLimitError(msg string, args ...interface{}) error {
116+
func RateLimitError(retryAfter time.Duration, msg string, args ...interface{}) error {
111117
return &BoulderError{
112-
Type: RateLimit,
113-
Detail: fmt.Sprintf(msg+": see https://letsencrypt.org/docs/rate-limits/", args...),
118+
Type: RateLimit,
119+
Detail: fmt.Sprintf(msg+": see https://letsencrypt.org/docs/rate-limits/", args...),
120+
RetryAfter: retryAfter,
114121
}
115122
}
116123

117-
func DuplicateCertificateError(msg string, args ...interface{}) error {
124+
func DuplicateCertificateError(retryAfter time.Duration, msg string, args ...interface{}) error {
118125
return &BoulderError{
119-
Type: RateLimit,
120-
Detail: fmt.Sprintf(msg+": see https://letsencrypt.org/docs/duplicate-certificate-limit/", args...),
126+
Type: RateLimit,
127+
Detail: fmt.Sprintf(msg+": see https://letsencrypt.org/docs/duplicate-certificate-limit/", args...),
128+
RetryAfter: retryAfter,
121129
}
122130
}
123131

124-
func FailedValidationError(msg string, args ...interface{}) error {
132+
func FailedValidationError(retryAfter time.Duration, msg string, args ...interface{}) error {
125133
return &BoulderError{
126-
Type: RateLimit,
127-
Detail: fmt.Sprintf(msg+": see https://letsencrypt.org/docs/failed-validation-limit/", args...),
134+
Type: RateLimit,
135+
Detail: fmt.Sprintf(msg+": see https://letsencrypt.org/docs/failed-validation-limit/", args...),
136+
RetryAfter: retryAfter,
128137
}
129138
}
130139

131-
func RegistrationsPerIPError(msg string, args ...interface{}) error {
140+
func RegistrationsPerIPError(retryAfter time.Duration, msg string, args ...interface{}) error {
132141
return &BoulderError{
133-
Type: RateLimit,
134-
Detail: fmt.Sprintf(msg+": see https://letsencrypt.org/docs/too-many-registrations-for-this-ip/", args...),
142+
Type: RateLimit,
143+
Detail: fmt.Sprintf(msg+": see https://letsencrypt.org/docs/too-many-registrations-for-this-ip/", args...),
144+
RetryAfter: retryAfter,
135145
}
136146
}
137147

‎grpc/errors.go‎

Lines changed: 57 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"errors"
77
"fmt"
88
"strconv"
9+
"time"
910

1011
"google.golang.org/grpc"
1112
"google.golang.org/grpc/codes"
@@ -41,8 +42,13 @@ func wrapError(ctx context.Context, err error) error {
4142
"error marshaling json SubErrors, orig error %q",
4243
err)
4344
}
44-
pairs = append(pairs, "suberrors")
45-
pairs = append(pairs, string(jsonSubErrs))
45+
pairs = append(pairs, "suberrors", string(jsonSubErrs))
46+
}
47+
48+
// If there is a RetryAfter value then extend the metadata pairs to
49+
// include the value.
50+
if berr.RetryAfter != 0 {
51+
pairs = append(pairs, "retryafter", berr.RetryAfter.String())
4652
}
4753

4854
// Ignoring the error return here is safe because if setting the metadata
@@ -63,59 +69,75 @@ func unwrapError(err error, md metadata.MD) error {
6369
return nil
6470
}
6571

66-
unwrappedErr := status.Convert(err).Message()
67-
6872
errTypeStrs, ok := md["errortype"]
6973
if !ok {
7074
return err
7175
}
76+
77+
inErrMsg := status.Convert(err).Message()
7278
if len(errTypeStrs) != 1 {
7379
return berrors.InternalServerError(
74-
"multiple errorType metadata, wrapped error %q",
75-
unwrappedErr,
80+
"multiple 'errortype' metadata, wrapped error %q",
81+
inErrMsg,
7682
)
7783
}
7884

79-
errType, decErr := strconv.Atoi(errTypeStrs[0])
85+
inErrType, decErr := strconv.Atoi(errTypeStrs[0])
8086
if decErr != nil {
8187
return berrors.InternalServerError(
8288
"failed to decode error type, decoding error %q, wrapped error %q",
8389
decErr,
84-
unwrappedErr,
90+
inErrMsg,
8591
)
8692
}
87-
outErr := berrors.New(berrors.ErrorType(errType), unwrappedErr)
88-
89-
subErrsJSON, ok := md["suberrors"]
90-
if !ok {
91-
return outErr
92-
}
93-
if len(subErrsJSON) != 1 {
94-
return berrors.InternalServerError(
95-
"multiple suberrors metadata, wrapped error %q",
96-
unwrappedErr,
93+
inErr := berrors.New(berrors.ErrorType(inErrType), inErrMsg)
94+
var outErr *berrors.BoulderError
95+
if !errors.As(inErr, &outErr) {
96+
return fmt.Errorf(
97+
"expected type of inErr to be %T got %T: %q",
98+
outErr,
99+
inErr,
100+
inErr.Error(),
97101
)
98102
}
99103

100-
var suberrs []berrors.SubBoulderError
101-
err2 := json.Unmarshal([]byte(subErrsJSON[0]), &suberrs)
102-
if err2 != nil {
103-
return berrors.InternalServerError(
104-
"error unmarshaling suberrs JSON %q, wrapped error %q",
105-
subErrsJSON[0],
106-
unwrappedErr,
107-
)
104+
subErrorsVal, ok := md["suberrors"]
105+
if ok {
106+
if len(subErrorsVal) != 1 {
107+
return berrors.InternalServerError(
108+
"multiple 'suberrors' in metadata, wrapped error %q",
109+
inErrMsg,
110+
)
111+
}
112+
113+
unmarshalErr := json.Unmarshal([]byte(subErrorsVal[0]), &outErr.SubErrors)
114+
if unmarshalErr != nil {
115+
return berrors.InternalServerError(
116+
"JSON unmarshaling 'suberrors' %q, wrapped error %q: %s",
117+
subErrorsVal[0],
118+
inErrMsg,
119+
unmarshalErr,
120+
)
121+
}
108122
}
109123

110-
var berr *berrors.BoulderError
111-
if errors.As(outErr, &berr) {
112-
outErr = berr.WithSubErrors(suberrs)
113-
} else {
114-
return fmt.Errorf(
115-
"expected type of outErr to be %T got %T: %q",
116-
berr, outErr,
117-
outErr.Error(),
118-
)
124+
retryAfterVal, ok := md["retryafter"]
125+
if ok {
126+
if len(retryAfterVal) != 1 {
127+
return berrors.InternalServerError(
128+
"multiple 'retryafter' in metadata, wrapped error %q",
129+
inErrMsg,
130+
)
131+
}
132+
var parseErr error
133+
outErr.RetryAfter, parseErr = time.ParseDuration(retryAfterVal[0])
134+
if parseErr != nil {
135+
return berrors.InternalServerError(
136+
"parsing 'retryafter' as int64, wrapped error %q, parsing error: %s",
137+
inErrMsg,
138+
parseErr,
139+
)
140+
}
119141
}
120142
return outErr
121143
}

‎grpc/errors_test.go‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package grpc
22

33
import (
44
"context"
5+
"errors"
56
"fmt"
67
"net"
78
"testing"
@@ -47,10 +48,19 @@ func TestErrorWrapping(t *testing.T) {
4748
test.AssertNotError(t, err, "Failed to dial grpc test server")
4849
client := test_proto.NewChillerClient(conn)
4950

50-
es.err = berrors.MalformedError("yup")
51+
// RateLimitError with a RetryAfter of 500ms.
52+
expectRetryAfter := time.Millisecond * 500
53+
es.err = berrors.RateLimitError(expectRetryAfter, "yup")
5154
_, err = client.Chill(context.Background(), &test_proto.Time{})
5255
test.Assert(t, err != nil, fmt.Sprintf("nil error returned, expected: %s", err))
5356
test.AssertDeepEquals(t, err, es.err)
57+
var bErr *berrors.BoulderError
58+
ok := errors.As(err, &bErr)
59+
test.Assert(t, ok, "asserting error as boulder error")
60+
// Ensure we got a RateLimitError
61+
test.AssertErrorIs(t, bErr, berrors.RateLimit)
62+
// Ensure our RetryAfter is still 500ms.
63+
test.AssertEquals(t, bErr.RetryAfter, expectRetryAfter)
5464

5565
test.AssertNil(t, wrapError(context.Background(), nil), "Wrapping nil should still be nil")
5666
test.AssertNil(t, unwrapError(nil, nil), "Unwrapping nil should still be nil")

‎ra/ra.go‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,7 @@ func (ra *RegistrationAuthorityImpl) checkRegistrationIPLimit(ctx context.Contex
328328
}
329329

330330
if count.Count >= limit.GetThreshold(ip.String(), noRegistrationID) {
331-
return berrors.RegistrationsPerIPError("too many registrations for this IP")
331+
return berrors.RegistrationsPerIPError(0, "too many registrations for this IP")
332332
}
333333

334334
return nil
@@ -365,7 +365,7 @@ func (ra *RegistrationAuthorityImpl) checkRegistrationLimits(ctx context.Context
365365
ra.log.Infof("Rate limit exceeded, RegistrationsByIPRange, IP: %s", ip)
366366
// For the fuzzyRegLimit we use a new error message that specifically
367367
// mentions that the limit being exceeded is applied to a *range* of IPs
368-
return berrors.RateLimitError("too many registrations for this IP range")
368+
return berrors.RateLimitError(0, "too many registrations for this IP range")
369369
}
370370
ra.rateLimitCounter.WithLabelValues("registrations_by_ip_range", "pass").Inc()
371371

@@ -515,7 +515,7 @@ func (ra *RegistrationAuthorityImpl) checkPendingAuthorizationLimit(ctx context.
515515
if countPB.Count >= limit.GetThreshold(noKey, regID) {
516516
ra.rateLimitCounter.WithLabelValues("pending_authorizations_by_registration_id", "exceeded").Inc()
517517
ra.log.Infof("Rate limit exceeded, PendingAuthorizationsByRegID, regID: %d", regID)
518-
return berrors.RateLimitError(fmt.Sprintf("too many currently pending authorizations: %d", countPB.Count))
518+
return berrors.RateLimitError(0, "too many currently pending authorizations: %d", countPB.Count)
519519
}
520520
ra.rateLimitCounter.WithLabelValues("pending_authorizations_by_registration_id", "pass").Inc()
521521
}
@@ -567,7 +567,7 @@ func (ra *RegistrationAuthorityImpl) checkInvalidAuthorizationLimit(ctx context.
567567
noKey := ""
568568
if count.Count >= limit.GetThreshold(noKey, regID) {
569569
ra.log.Infof("Rate limit exceeded, InvalidAuthorizationsByRegID, regID: %d", regID)
570-
return berrors.FailedValidationError("too many failed authorizations recently")
570+
return berrors.FailedValidationError(0, "too many failed authorizations recently")
571571
}
572572
return nil
573573
}
@@ -595,7 +595,7 @@ func (ra *RegistrationAuthorityImpl) checkNewOrdersPerAccountLimit(ctx context.C
595595
noKey := ""
596596
if count.Count >= limit.GetThreshold(noKey, acctID) {
597597
ra.rateLimitCounter.WithLabelValues("new_order_by_registration_id", "exceeded").Inc()
598-
return berrors.RateLimitError("too many new orders recently")
598+
return berrors.RateLimitError(0, "too many new orders recently")
599599
}
600600
ra.rateLimitCounter.WithLabelValues("new_order_by_registration_id", "pass").Inc()
601601
return nil
@@ -1347,12 +1347,12 @@ func (ra *RegistrationAuthorityImpl) checkCertificatesPerNameLimit(ctx context.C
13471347
for _, name := range namesOutOfLimit {
13481348
subErrors = append(subErrors, berrors.SubBoulderError{
13491349
Identifier: identifier.DNSIdentifier(name),
1350-
BoulderError: berrors.RateLimitError("too many certificates already issued").(*berrors.BoulderError),
1350+
BoulderError: berrors.RateLimitError(0, "too many certificates already issued").(*berrors.BoulderError),
13511351
})
13521352
}
1353-
return berrors.RateLimitError("too many certificates already issued for multiple names (%s and %d others)", namesOutOfLimit[0], len(namesOutOfLimit)).(*berrors.BoulderError).WithSubErrors(subErrors)
1353+
return berrors.RateLimitError(0, "too many certificates already issued for multiple names (%s and %d others)", namesOutOfLimit[0], len(namesOutOfLimit)).(*berrors.BoulderError).WithSubErrors(subErrors)
13541354
}
1355-
return berrors.RateLimitError("too many certificates already issued for: %s", namesOutOfLimit[0])
1355+
return berrors.RateLimitError(0, "too many certificates already issued for: %s", namesOutOfLimit[0])
13561356
}
13571357
ra.rateLimitCounter.WithLabelValues("certificates_for_domain", "pass").Inc()
13581358

@@ -1394,7 +1394,9 @@ func (ra *RegistrationAuthorityImpl) checkCertificatesPerFQDNSetLimit(ctx contex
13941394
}
13951395
}
13961396
retryTime := time.Unix(0, prevIssuances.Timestamps[0]).Add(time.Duration(nsPerToken))
1397+
retryAfter := retryTime.Sub(now)
13971398
return berrors.DuplicateCertificateError(
1399+
retryAfter,
13981400
"too many certificates (%d) already issued for this exact set of domains in the last %.0f hours: %s, retry after %s",
13991401
threshold, limit.Window.Duration.Hours(), strings.Join(names, ","), retryTime.Format(time.RFC3339),
14001402
)

‎va/http.go‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -310,8 +310,7 @@ func (va *ValidationAuthorityImpl) extractRequestTarget(req *http.Request) (stri
310310
// Check that the request host isn't a bare IP address. We only follow
311311
// redirects to hostnames.
312312
if net.ParseIP(reqHost) != nil {
313-
return "", 0, berrors.ConnectionFailureError(
314-
"Invalid host in redirect target %q. Only domain names are supported, not IP addresses", reqHost)
313+
return "", 0, berrors.ConnectionFailureError("Invalid host in redirect target %q. Only domain names are supported, not IP addresses", reqHost)
315314
}
316315

317316
// Often folks will misconfigure their webserver to send an HTTP redirect
@@ -331,8 +330,7 @@ func (va *ValidationAuthorityImpl) extractRequestTarget(req *http.Request) (stri
331330
}
332331

333332
if _, err := iana.ExtractSuffix(reqHost); err != nil {
334-
return "", 0, berrors.ConnectionFailureError(
335-
"Invalid hostname in redirect target, must end in IANA registered TLD")
333+
return "", 0, berrors.ConnectionFailureError("Invalid hostname in redirect target, must end in IANA registered TLD")
336334
}
337335

338336
return reqHost, reqPort, nil

‎web/probs_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ func TestProblemDetailsFromError(t *testing.T) {
3434
{berrors.MalformedError(detailMsg), 400, probs.MalformedProblem, fullDetail},
3535
{berrors.UnauthorizedError(detailMsg), 403, probs.UnauthorizedProblem, fullDetail},
3636
{berrors.NotFoundError(detailMsg), 404, probs.MalformedProblem, fullDetail},
37-
{berrors.RateLimitError(detailMsg), 429, probs.RateLimitedProblem, fullDetail + ": see https://letsencrypt.org/docs/rate-limits/"},
37+
{berrors.RateLimitError(0, detailMsg), 429, probs.RateLimitedProblem, fullDetail + ": see https://letsencrypt.org/docs/rate-limits/"},
3838
{berrors.InvalidEmailError(detailMsg), 400, probs.InvalidEmailProblem, fullDetail},
3939
{berrors.RejectedIdentifierError(detailMsg), 400, probs.RejectedIdentifierProblem, fullDetail},
4040
}

‎wfe2/wfe.go‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,8 @@ const (
7474

7575
// Non-ACME paths
7676
aiaIssuerPath = "/aia/issuer/"
77+
78+
headerRetryAfter = "Retry-After"
7779
)
7880

7981
var errIncompleteGRPCResponse = errors.New("incomplete gRPC response message")
@@ -584,6 +586,16 @@ func (wfe *WebFrontEndImpl) Nonce(
584586

585587
// sendError wraps web.SendError
586588
func (wfe *WebFrontEndImpl) sendError(response http.ResponseWriter, logEvent *web.RequestEvent, prob *probs.ProblemDetails, ierr error) {
589+
var bErr *berrors.BoulderError
590+
if errors.As(ierr, &bErr) {
591+
retryAfterSeconds := int(bErr.RetryAfter.Round(time.Second).Seconds())
592+
if retryAfterSeconds > 0 {
593+
response.Header().Add(headerRetryAfter, strconv.Itoa(retryAfterSeconds))
594+
if bErr.Type == berrors.RateLimit {
595+
response.Header().Add("Link", link("https://letsencrypt.org/docs/rate-limits", "help"))
596+
}
597+
}
598+
}
587599
wfe.stats.httpErrorCount.With(prometheus.Labels{"type": string(prob.Type)}).Inc()
588600
web.SendError(wfe.log, probs.V2ErrorNS, response, logEvent, prob, ierr)
589601
}
@@ -2304,7 +2316,7 @@ func (wfe *WebFrontEndImpl) RenewalInfo(ctx context.Context, logEvent *web.Reque
23042316
beeline.AddFieldToTrace(ctx, "request.serial", serial)
23052317

23062318
setDefaultRetryAfterHeader := func(response http.ResponseWriter) {
2307-
response.Header().Set("Retry-After", fmt.Sprintf("%d", int(6*time.Hour/time.Second)))
2319+
response.Header().Set(headerRetryAfter, fmt.Sprintf("%d", int(6*time.Hour/time.Second)))
23082320
}
23092321

23102322
// Check if the serial is part of an ongoing incident.

0 commit comments

Comments
 (0)