X Tutup
Skip to content

Commit 8ea5a3d

Browse files
andygabbyandygabby
authored
Handle redis lookup errors when value not found (letsencrypt#5860)
Add a not found error type to rocsp. Handle redis value not found lookup errors in the ocsp-responder different than other redis lookup errors. Add labels to the to ocspLookup metric and delete the source used metric. This can now be determined based on which lookup metric reports success. Fixes letsencrypt#5833
1 parent 3b9f3dc commit 8ea5a3d

File tree

2 files changed

+54
-38
lines changed

2 files changed

+54
-38
lines changed

‎cmd/ocsp-responder/main.go‎

Lines changed: 37 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -78,26 +78,18 @@ func newFilter(issuerCerts []string, serialPrefixes []string) (*ocspFilter, erro
7878
// between redis and mysql.
7979
type sourceMetrics struct {
8080
ocspLookups *prometheus.CounterVec
81-
sourceUsed *prometheus.CounterVec
8281
}
8382

8483
func newSourceMetrics(stats prometheus.Registerer) *sourceMetrics {
8584
// Metrics for response lookups
8685
ocspLookups := prometheus.NewCounterVec(prometheus.CounterOpts{
8786
Name: "ocsp_lookups",
88-
Help: "A counter of ocsp lookups labeled by source_result",
89-
}, []string{"result"})
87+
Help: "A counter of ocsp lookups labeled with source and result",
88+
}, []string{"source", "result"})
9089
stats.MustRegister(ocspLookups)
9190

92-
sourceUsed := prometheus.NewCounterVec(prometheus.CounterOpts{
93-
Name: "lookup_source_used",
94-
Help: "A counter of lookups returned labeled by source used",
95-
}, []string{"source"})
96-
stats.MustRegister(sourceUsed)
97-
9891
metrics := sourceMetrics{
9992
ocspLookups: ocspLookups,
100-
sourceUsed: sourceUsed,
10193
}
10294
return &metrics
10395
}
@@ -233,15 +225,17 @@ func (src *dbSource) Response(ctx context.Context, req *ocsp.Request) ([]byte, h
233225
select {
234226
case <-ctx.Done():
235227
if errors.Is(ctx.Err(), context.Canceled) {
236-
src.metrics.ocspLookups.WithLabelValues("canceled").Inc()
228+
src.metrics.ocspLookups.WithLabelValues("mysql", "canceled").Inc()
229+
} else {
230+
src.metrics.ocspLookups.WithLabelValues("mysql", "deadline_exceeded").Inc()
237231
}
238-
src.metrics.ocspLookups.WithLabelValues("deadline_exceeded").Inc()
239232
return nil, nil, fmt.Errorf("looking up OCSP response for serial: %s err: %w", serialString, ctx.Err())
240233
case primaryResult := <-primaryChan:
241234
if primaryResult.err != nil {
242-
if !errors.Is(primaryResult.err, bocsp.ErrNotFound) {
243-
src.metrics.ocspLookups.WithLabelValues("mysql_failed").Inc()
244-
src.metrics.sourceUsed.WithLabelValues("error_returned").Inc()
235+
if errors.Is(primaryResult.err, bocsp.ErrNotFound) {
236+
src.metrics.ocspLookups.WithLabelValues("mysql", "not_found").Inc()
237+
} else {
238+
src.metrics.ocspLookups.WithLabelValues("mysql", "failed").Inc()
245239
}
246240
return nil, nil, primaryResult.err
247241
}
@@ -250,13 +244,11 @@ func (src *dbSource) Response(ctx context.Context, req *ocsp.Request) ([]byte, h
250244
primaryParsed, err := ocsp.ParseResponse(primaryResult.bytes, nil)
251245
if err != nil {
252246
src.log.AuditErrf("parsing OCSP response: %s", err)
253-
src.metrics.ocspLookups.WithLabelValues("mysql_parse_error").Inc()
254-
src.metrics.sourceUsed.WithLabelValues("error_returned").Inc()
247+
src.metrics.ocspLookups.WithLabelValues("mysql", "parse_error").Inc()
255248
return nil, nil, err
256249
}
257250
src.log.Debugf("returning ocsp from primary source: %v", helper.PrettyResponse(primaryParsed))
258-
src.metrics.ocspLookups.WithLabelValues("mysql_success").Inc()
259-
src.metrics.sourceUsed.WithLabelValues("mysql").Inc()
251+
src.metrics.ocspLookups.WithLabelValues("mysql", "success").Inc()
260252
return primaryResult.bytes, header, nil
261253
case secondaryResult := <-secondaryChan:
262254
// If secondary returns first, wait for primary to return for
@@ -267,18 +259,20 @@ func (src *dbSource) Response(ctx context.Context, req *ocsp.Request) ([]byte, h
267259
select {
268260
case <-ctx.Done():
269261
if errors.Is(ctx.Err(), context.Canceled) {
270-
src.metrics.ocspLookups.WithLabelValues("canceled").Inc()
262+
src.metrics.ocspLookups.WithLabelValues("mysql", "canceled").Inc()
263+
} else {
264+
src.metrics.ocspLookups.WithLabelValues("mysql", "deadline_exceeded").Inc()
271265
}
272-
src.metrics.ocspLookups.WithLabelValues("deadline_exceeded").Inc()
273266
return nil, nil, fmt.Errorf("looking up OCSP response for serial: %s err: %w", serialString, ctx.Err())
274267
case primaryResult = <-primaryChan:
275268
}
276269

277270
// Check for error returned from the mysql lookup, return on error.
278271
if primaryResult.err != nil {
279-
if !errors.Is(primaryResult.err, bocsp.ErrNotFound) {
280-
src.metrics.ocspLookups.WithLabelValues("mysql_failed").Inc()
281-
src.metrics.sourceUsed.WithLabelValues("error_returned").Inc()
272+
if errors.Is(primaryResult.err, bocsp.ErrNotFound) {
273+
src.metrics.ocspLookups.WithLabelValues("mysql", "not_found").Inc()
274+
} else {
275+
src.metrics.ocspLookups.WithLabelValues("mysql", "failed").Inc()
282276
}
283277
return nil, nil, primaryResult.err
284278
}
@@ -288,16 +282,22 @@ func (src *dbSource) Response(ctx context.Context, req *ocsp.Request) ([]byte, h
288282
primaryParsed, err := ocsp.ParseResponse(primaryResult.bytes, nil)
289283
if err != nil {
290284
src.log.AuditErrf("parsing OCSP response: %s", err)
291-
src.metrics.ocspLookups.WithLabelValues("mysql_parse_error").Inc()
292-
src.metrics.sourceUsed.WithLabelValues("error_returned").Inc()
285+
src.metrics.ocspLookups.WithLabelValues("mysql", "parse_error").Inc()
293286
return nil, nil, err
294287
}
295288

296289
// Check for error returned from the redis lookup. If error return
297290
// primary lookup result.
298291
if secondaryResult.err != nil {
299-
src.metrics.ocspLookups.WithLabelValues("redis_failed").Inc()
300-
src.metrics.sourceUsed.WithLabelValues("mysql").Inc()
292+
// If we made it this far then there was a successful lookup
293+
// on mysql but an error on redis. Either the response exists
294+
// in mysql and not in redis or a different error occurred.
295+
if errors.Is(secondaryResult.err, bocsp.ErrNotFound) {
296+
src.metrics.ocspLookups.WithLabelValues("redis", "not_found").Inc()
297+
} else {
298+
src.metrics.ocspLookups.WithLabelValues("redis", "failed").Inc()
299+
}
300+
src.metrics.ocspLookups.WithLabelValues("mysql", "success").Inc()
301301
return primaryResult.bytes, header, nil
302302
}
303303

@@ -306,22 +306,21 @@ func (src *dbSource) Response(ctx context.Context, req *ocsp.Request) ([]byte, h
306306
secondaryParsed, err := ocsp.ParseResponse(secondaryResult.bytes, nil)
307307
if err != nil {
308308
src.log.AuditErrf("parsing secondary OCSP response: %s", err)
309-
src.metrics.ocspLookups.WithLabelValues("redis_parse_error").Inc()
310-
src.metrics.sourceUsed.WithLabelValues("mysql").Inc()
309+
src.metrics.ocspLookups.WithLabelValues("redis", "parse_error").Inc()
310+
src.metrics.ocspLookups.WithLabelValues("mysql", "success").Inc()
311311
return primaryResult.bytes, header, nil
312312
}
313313

314314
// If the secondary response status doesn't match primary return
315315
// primary response.
316316
if primaryParsed.Status != secondaryParsed.Status {
317-
src.metrics.ocspLookups.WithLabelValues("redis_mismatch").Inc()
318-
src.metrics.sourceUsed.WithLabelValues("mysql").Inc()
317+
src.metrics.ocspLookups.WithLabelValues("redis", "mismatch").Inc()
318+
src.metrics.ocspLookups.WithLabelValues("mysql", "success").Inc()
319319
return primaryResult.bytes, header, nil
320320
}
321321

322322
// The secondary response has passed checks, return it.
323-
src.metrics.ocspLookups.WithLabelValues("redis_success").Inc()
324-
src.metrics.sourceUsed.WithLabelValues("redis").Inc()
323+
src.metrics.ocspLookups.WithLabelValues("redis", "success").Inc()
325324
return secondaryResult.bytes, header, nil
326325
}
327326
}
@@ -404,6 +403,10 @@ func (src redisReceiver) getResponse(ctx context.Context, req *ocsp.Request) cha
404403
go func() {
405404
defer close(responseChan)
406405
respBytes, err := src.rocspReader.GetResponse(ctx, serialString)
406+
if errors.Is(err, rocsp.ErrRedisNotFound) {
407+
responseChan <- lookupResponse{nil, bocsp.ErrNotFound}
408+
return
409+
}
407410
responseChan <- lookupResponse{respBytes, err}
408411
}()
409412

‎rocsp/rocsp.go‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package rocsp
33
import (
44
"context"
55
"encoding/binary"
6+
"errors"
67
"fmt"
78
"time"
89

@@ -13,6 +14,8 @@ import (
1314
"golang.org/x/crypto/ocsp"
1415
)
1516

17+
var ErrRedisNotFound = errors.New("redis key not found")
18+
1619
// Metadata represents information stored with the 'm' prefix in the Redis DB:
1720
// information required to maintain or serve the response, but not the response
1821
// itself.
@@ -176,12 +179,17 @@ func (c *Client) GetResponse(ctx context.Context, serial string) ([]byte, error)
176179

177180
responseKey := MakeResponseKey(serial)
178181

179-
val, err := c.rdb.Get(ctx, responseKey).Result()
182+
resp, err := c.rdb.Get(ctx, responseKey).Result()
180183
if err != nil {
184+
// go-redis `Get` returns redis.Nil error when key does not exist. In
185+
// that case return a `ErrRedisNotFound` error.
186+
if errors.Is(err, redis.Nil) {
187+
return nil, ErrRedisNotFound
188+
}
181189
return nil, fmt.Errorf("getting response: %w", err)
182190
}
183191

184-
return []byte(val), nil
192+
return []byte(resp), nil
185193
}
186194

187195
// GetMetadata fetches the metadata for the given serial number.
@@ -191,11 +199,16 @@ func (c *Client) GetMetadata(ctx context.Context, serial string) (*Metadata, err
191199

192200
metadataKey := MakeMetadataKey(serial)
193201

194-
val, err := c.rdb.Get(ctx, metadataKey).Result()
202+
resp, err := c.rdb.Get(ctx, metadataKey).Result()
195203
if err != nil {
204+
// go-redis `Get` returns redis.Nil error when key does not exist. In
205+
// that case return a `ErrRedisNotFound` error.
206+
if errors.Is(err, redis.Nil) {
207+
return nil, ErrRedisNotFound
208+
}
196209
return nil, fmt.Errorf("getting metadata: %w", err)
197210
}
198-
metadata, err := UnmarshalMetadata([]byte(val))
211+
metadata, err := UnmarshalMetadata([]byte(resp))
199212
if err != nil {
200213
return nil, fmt.Errorf("unmarshaling metadata: %w", err)
201214
}

0 commit comments

Comments
 (0)
X Tutup