Skip to content

Commit d878510

Browse files
cpucpu
authored
Migrate WFE2 to use Prometheus stats. (letsencrypt#3002)
Per letsencrypt#3001 we should not be adding new StatsD code for metrics anymore. This commit updates all of the WFE2 to use 1st class Prometheus stats. Unit tests are updated accordingly. I have broken the error stats into two counts: 1. httpErrorCount for all of the http layer client request errors (e.g. no POST body, no content-length) 2. joseErrorCount, for all of the JOSE layer client request errors (e.g. malformed JWS, broken signature, invalid JWK) This commit also removes the stubbed out `TestValidKeyRollover` function from `wfe2/verify_test.go`. This was committed accidentally and the same functionality is covered by the `wfe2/wfe_test.go` `TestKeyRollover` function.
1 parent eadbc19 commit d878510

File tree

5 files changed

+191
-78
lines changed

5 files changed

+191
-78
lines changed

‎wfe2/stats.go‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
package wfe2
2+
3+
import (
4+
"github.com/letsencrypt/boulder/metrics"
5+
"github.com/prometheus/client_golang/prometheus"
6+
)
7+
8+
type wfe2Stats struct {
9+
// httpErrorCount counts client errors at the HTTP level
10+
// e.g. failure to provide a Content-Length header, no POST body, etc
11+
httpErrorCount *prometheus.CounterVec
12+
// joseErrorCount counts client errors at the JOSE level
13+
// e.g. bad JWS, broken JWS signature, invalid JWK, etc
14+
joseErrorCount *prometheus.CounterVec
15+
}
16+
17+
func initStats(scope metrics.Scope) wfe2Stats {
18+
httpErrorCount := prometheus.NewCounterVec(
19+
prometheus.CounterOpts{
20+
Name: "httpErrors",
21+
Help: "client request errors at the HTTP level",
22+
},
23+
[]string{"type"})
24+
scope.MustRegister(httpErrorCount)
25+
26+
joseErrorCount := prometheus.NewCounterVec(
27+
prometheus.CounterOpts{
28+
Name: "joseErrors",
29+
Help: "client request errors at the JOSE level",
30+
},
31+
[]string{"type"})
32+
scope.MustRegister(joseErrorCount)
33+
34+
return wfe2Stats{
35+
httpErrorCount: httpErrorCount,
36+
joseErrorCount: joseErrorCount,
37+
}
38+
}

‎wfe2/verify.go‎

Lines changed: 39 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,12 @@ import (
1414
"strconv"
1515
"strings"
1616

17+
"github.com/prometheus/client_golang/prometheus"
18+
"gopkg.in/square/go-jose.v2"
19+
1720
"github.com/letsencrypt/boulder/core"
1821
berrors "github.com/letsencrypt/boulder/errors"
1922
"github.com/letsencrypt/boulder/probs"
20-
21-
"gopkg.in/square/go-jose.v2"
2223
)
2324

2425
var sigAlgErr = errors.New("no signature algorithms suitable for given key type")
@@ -113,13 +114,13 @@ func (wfe *WebFrontEndImpl) enforceJWSAuthType(
113114
// Check the auth type for the provided JWS
114115
authType, prob := checkJWSAuthType(jws)
115116
if prob != nil {
116-
wfe.stats.Inc("Errors.InvalidJWSAuth", 1)
117+
wfe.stats.joseErrorCount.With(prometheus.Labels{"type": "JWSAuthTypeInvalid"}).Inc()
117118
return prob
118119
}
119120
// If the auth type isn't the one expected return a sensible problem based on
120121
// what was expected
121122
if authType != expectedAuthType {
122-
wfe.stats.Inc("Errors.WrongJWSAuthType", 1)
123+
wfe.stats.joseErrorCount.With(prometheus.Labels{"type": "JWSAuthTypeWrong"}).Inc()
123124
switch expectedAuthType {
124125
case embeddedKeyID:
125126
return probs.Malformed("No Key ID in JWS header")
@@ -136,20 +137,20 @@ func (wfe *WebFrontEndImpl) enforceJWSAuthType(
136137
func (wfe *WebFrontEndImpl) validPOSTRequest(request *http.Request) *probs.ProblemDetails {
137138
// All POSTs should have an accompanying Content-Length header
138139
if _, present := request.Header["Content-Length"]; !present {
139-
wfe.stats.Inc("HTTP.ClientErrors.LengthRequiredError", 1)
140+
wfe.stats.httpErrorCount.With(prometheus.Labels{"type": "ContentLengthRequired"}).Inc()
140141
return probs.ContentLengthRequired()
141142
}
142143

143144
// Per 6.4.1 "Replay-Nonce" clients should not send a Replay-Nonce header in
144145
// the HTTP request, it needs to be part of the signed JWS request body
145146
if _, present := request.Header["Replay-Nonce"]; present {
146-
wfe.stats.Inc("HTTP.ClientErrors.ReplayNonceOutsideJWSError", 1)
147+
wfe.stats.httpErrorCount.With(prometheus.Labels{"type": "ReplayNonceOutsideJWS"}).Inc()
147148
return probs.Malformed("HTTP requests should NOT contain Replay-Nonce header. Use JWS nonce field")
148149
}
149150

150151
// All POSTs should have a non-nil body
151152
if request.Body == nil {
152-
wfe.stats.Inc("HTTP.ClientErrors.NoPOSTBody", 1)
153+
wfe.stats.httpErrorCount.With(prometheus.Labels{"type": "NoPOSTBody"}).Inc()
153154
return probs.Malformed("No body on POST")
154155
}
155156

@@ -168,10 +169,10 @@ func (wfe *WebFrontEndImpl) validNonce(jws *jose.JSONWebSignature, logEvent *req
168169
nonce := header.Nonce
169170
logEvent.RequestNonce = nonce
170171
if len(nonce) == 0 {
171-
wfe.stats.Inc("Errors.JWSMissingNonce", 1)
172+
wfe.stats.joseErrorCount.With(prometheus.Labels{"type": "JWSMissingNonce"}).Inc()
172173
return probs.BadNonce("JWS has no anti-replay nonce")
173174
} else if !wfe.nonceService.Valid(nonce) {
174-
wfe.stats.Inc("Errors.JWSInvalidNonce", 1)
175+
wfe.stats.joseErrorCount.With(prometheus.Labels{"type": "JWSInvalidNonce"}).Inc()
175176
return probs.BadNonce(fmt.Sprintf("JWS has an invalid anti-replay nonce: %q", nonce))
176177
}
177178
return nil
@@ -190,13 +191,13 @@ func (wfe *WebFrontEndImpl) validPOSTURL(
190191
extraHeaders := header.ExtraHeaders
191192
// Check that there is at least one Extra Header
192193
if len(extraHeaders) == 0 {
193-
wfe.stats.Inc("Errors.MissingURLinJWS", 1)
194+
wfe.stats.joseErrorCount.With(prometheus.Labels{"type": "JWSNoExtraHeaders"}).Inc()
194195
return probs.Malformed("JWS header parameter 'url' required")
195196
}
196197
// Try to read a 'url' Extra Header as a string
197198
headerURL, ok := extraHeaders[jose.HeaderKey("url")].(string)
198199
if !ok || len(headerURL) == 0 {
199-
wfe.stats.Inc("Errors.MissingURLinJWS", 1)
200+
wfe.stats.joseErrorCount.With(prometheus.Labels{"type": "JWSMissingURL"}).Inc()
200201
return probs.Malformed("JWS header parameter 'url' required")
201202
}
202203
// Compute the URL we expect to be in the JWS based on the HTTP request
@@ -208,6 +209,7 @@ func (wfe *WebFrontEndImpl) validPOSTURL(
208209
// Check that the URL we expect is the one that was found in the signed JWS
209210
// header
210211
if expectedURL.String() != headerURL {
212+
wfe.stats.joseErrorCount.With(prometheus.Labels{"type": "JWSMismatchedURL"}).Inc()
211213
return probs.Malformed(fmt.Sprintf(
212214
"JWS header parameter 'url' incorrect. Expected %q got %q",
213215
expectedURL.String(), headerURL))
@@ -218,25 +220,28 @@ func (wfe *WebFrontEndImpl) validPOSTURL(
218220
// matchJWSURLs checks two JWS' URL headers are equal. This is used during key
219221
// rollover to check that the inner JWS URL matches the outer JWS URL. If the
220222
// JWS URLs do not match a problem is returned.
221-
func matchJWSURLs(outer, inner *jose.JSONWebSignature) *probs.ProblemDetails {
223+
func (wfe *WebFrontEndImpl) matchJWSURLs(outer, inner *jose.JSONWebSignature) *probs.ProblemDetails {
222224
// Verify that the outer JWS has a non-empty URL header. This is strictly
223225
// defensive since the expectation is that endpoints using `matchJWSURLs`
224226
// have received at least one of their JWS from calling validPOSTForAccount(),
225227
// which checks the outer JWS has the expected URL header before processing
226228
// the inner JWS.
227229
outerURL, ok := outer.Signatures[0].Header.ExtraHeaders[jose.HeaderKey("url")].(string)
228230
if !ok || len(outerURL) == 0 {
231+
wfe.stats.joseErrorCount.With(prometheus.Labels{"type": "KeyRolloverOuterJWSNoURL"}).Inc()
229232
return probs.Malformed("Outer JWS header parameter 'url' required")
230233
}
231234

232235
// Verify the inner JWS has a non-empty URL header.
233236
innerURL, ok := inner.Signatures[0].Header.ExtraHeaders[jose.HeaderKey("url")].(string)
234237
if !ok || len(innerURL) == 0 {
238+
wfe.stats.joseErrorCount.With(prometheus.Labels{"type": "KeyRolloverInnerJWSNoURL"}).Inc()
235239
return probs.Malformed("Inner JWS header parameter 'url' required")
236240
}
237241

238242
// Verify that the outer URL matches the inner URL
239243
if outerURL != innerURL {
244+
wfe.stats.joseErrorCount.With(prometheus.Labels{"type": "KeyRolloverMismatchedURLs"}).Inc()
240245
return probs.Malformed(fmt.Sprintf(
241246
"Outer JWS 'url' value %q does not match inner JWS 'url' value %q",
242247
outerURL, innerURL))
@@ -261,22 +266,22 @@ func (wfe *WebFrontEndImpl) parseJWS(body []byte) (*jose.JSONWebSignature, *prob
261266
Signatures []interface{}
262267
}
263268
if err := json.Unmarshal(body, &unprotected); err != nil {
264-
wfe.stats.Inc("Errors.JWSParseError", 1)
269+
wfe.stats.joseErrorCount.With(prometheus.Labels{"type": "JWSUnmarshalFailed"}).Inc()
265270
return nil, probs.Malformed("Parse error reading JWS")
266271
}
267272

268273
// ACME v2 never uses values from the unprotected JWS header. Reject JWS that
269274
// include unprotected headers.
270275
if unprotected.Header != nil {
271-
wfe.stats.Inc("Errors.JWSUnprotectedHeaders", 1)
276+
wfe.stats.joseErrorCount.With(prometheus.Labels{"type": "JWSUnprotectedHeaders"}).Inc()
272277
return nil, probs.Malformed(
273278
"JWS \"header\" field not allowed. All headers must be in \"protected\" field")
274279
}
275280

276281
// ACME v2 never uses the "signatures" array of JSON serialized JWS, just the
277282
// mandatory "signature" field. Reject JWS that include the "signatures" array.
278283
if len(unprotected.Signatures) > 0 {
279-
wfe.stats.Inc("Errors.JWSMultiSigField", 1)
284+
wfe.stats.joseErrorCount.With(prometheus.Labels{"type": "JWSMultiSig"}).Inc()
280285
return nil, probs.Malformed(
281286
"JWS \"signatures\" field not allowed. Only the \"signature\" field should contain a signature")
282287
}
@@ -286,19 +291,19 @@ func (wfe *WebFrontEndImpl) parseJWS(body []byte) (*jose.JSONWebSignature, *prob
286291
bodyStr := string(body)
287292
parsedJWS, err := jose.ParseSigned(bodyStr)
288293
if err != nil {
289-
wfe.stats.Inc("Errors.JWSParseError", 1)
294+
wfe.stats.joseErrorCount.With(prometheus.Labels{"type": "JWSParseError"}).Inc()
290295
return nil, probs.Malformed("Parse error reading JWS")
291296
}
292297
if len(parsedJWS.Signatures) > 1 {
293-
wfe.stats.Inc("Errors.TooManySignaturesInJWS", 1)
298+
wfe.stats.joseErrorCount.With(prometheus.Labels{"type": "JWSTooManySignatures"}).Inc()
294299
return nil, probs.Malformed("Too many signatures in POST body")
295300
}
296301
if len(parsedJWS.Signatures) == 0 {
297-
wfe.stats.Inc("Errors.NoSignaturesInJWS", 1)
302+
wfe.stats.joseErrorCount.With(prometheus.Labels{"type": "JWSNoSignatures"}).Inc()
298303
return nil, probs.Malformed("POST JWS not signed")
299304
}
300305
if len(parsedJWS.Signatures) == 1 && len(parsedJWS.Signatures[0].Signature) == 0 {
301-
wfe.stats.Inc("Errors.EmptySignatureInJWS", 1)
306+
wfe.stats.joseErrorCount.With(prometheus.Labels{"type": "JWSEmptySignature"}).Inc()
302307
return nil, probs.Malformed("POST JWS not signed")
303308
}
304309

@@ -316,7 +321,7 @@ func (wfe *WebFrontEndImpl) parseJWSRequest(request *http.Request) (*jose.JSONWe
316321
// that the body is non-nil
317322
bodyBytes, err := ioutil.ReadAll(request.Body)
318323
if err != nil {
319-
wfe.stats.Inc("Errors.UnableToReadRequestBody", 1)
324+
wfe.stats.httpErrorCount.With(prometheus.Labels{"type": "UnableToReadReqBody"}).Inc()
320325
return nil, probs.ServerInternal("unable to read request body")
321326
}
322327

@@ -344,7 +349,7 @@ func (wfe *WebFrontEndImpl) extractJWK(jws *jose.JSONWebSignature) (*jose.JSONWe
344349

345350
// If the key isn't considered valid by go-jose return a problem immediately
346351
if !key.Valid() {
347-
wfe.stats.Inc("Errors.InvalidJWK", 1)
352+
wfe.stats.joseErrorCount.With(prometheus.Labels{"type": "JWKInvalid"}).Inc()
348353
return nil, probs.Malformed("Invalid JWK in JWS header")
349354
}
350355

@@ -376,7 +381,7 @@ func (wfe *WebFrontEndImpl) lookupJWK(
376381
// GetRegistration RPC
377382
accountID, err := strconv.ParseInt(accountIDStr, 10, 64)
378383
if err != nil {
379-
wfe.stats.Inc("Errors.InvalidKeyID", 1)
384+
wfe.stats.joseErrorCount.With(prometheus.Labels{"type": "JWSInvalidKeyID"}).Inc()
380385
return nil, nil, probs.Malformed(fmt.Sprintf("Malformed account ID in KeyID header"))
381386
}
382387

@@ -385,14 +390,14 @@ func (wfe *WebFrontEndImpl) lookupJWK(
385390
if err != nil {
386391
// If the account isn't found, return a suitable problem
387392
if berrors.Is(err, berrors.NotFound) {
388-
wfe.stats.Inc("Errors.KeyIDNotFound", 1)
393+
wfe.stats.joseErrorCount.With(prometheus.Labels{"type": "JWSKeyIDNotFound"}).Inc()
389394
return nil, nil, probs.AccountDoesNotExist(fmt.Sprintf(
390395
"Account %q not found", accountURL))
391396
}
392397

393398
// If there was an error and it isn't a "Not Found" error, return
394399
// a ServerInternal problem since this is unexpected.
395-
wfe.stats.Inc("Errors.UnableToGetAccountByID", 1)
400+
wfe.stats.joseErrorCount.With(prometheus.Labels{"type": "JWSKeyIDLookupFailed"}).Inc()
396401
// Add an error to the log event with the internal error message
397402
logEvent.AddError(fmt.Sprintf("Error calling SA.GetRegistration: %s", err.Error()))
398403
return nil, nil, probs.ServerInternal(fmt.Sprintf(
@@ -401,7 +406,7 @@ func (wfe *WebFrontEndImpl) lookupJWK(
401406

402407
// Verify the account is not deactivated
403408
if account.Status != core.StatusValid {
404-
wfe.stats.Inc("Errors.AccountIsNotValid", 1)
409+
wfe.stats.joseErrorCount.With(prometheus.Labels{"type": "JWSKeyIDAccountInvalid"}).Inc()
405410
return nil, nil, probs.Unauthorized(
406411
fmt.Sprintf("Account is not valid, has status %q", account.Status))
407412
}
@@ -426,6 +431,7 @@ func (wfe *WebFrontEndImpl) validJWSForKey(
426431

427432
// Check that the public key and JWS algorithms match expected
428433
if err := checkAlgorithm(jwk, jws); err != nil {
434+
wfe.stats.joseErrorCount.With(prometheus.Labels{"type": "JWSAlgorithmCheckFailed"}).Inc()
429435
return nil, probs.Malformed(err.Error())
430436
}
431437

@@ -437,7 +443,7 @@ func (wfe *WebFrontEndImpl) validJWSForKey(
437443
// the signature itself.
438444
payload, err := jws.Verify(jwk)
439445
if err != nil {
440-
wfe.stats.Inc("Errors.JWSVerificationFailed", 1)
446+
wfe.stats.joseErrorCount.With(prometheus.Labels{"type": "JWSVerifyFailed"}).Inc()
441447
return nil, probs.Malformed("JWS verification error")
442448
}
443449
// Store the verified payload in the logEvent
@@ -460,7 +466,7 @@ func (wfe *WebFrontEndImpl) validJWSForKey(
460466
// early if it isn't valid JSON.
461467
var parsedBody struct{}
462468
if err := json.Unmarshal(payload, &parsedBody); err != nil {
463-
wfe.stats.Inc("Errors.UnparseableJWSPayload", 1)
469+
wfe.stats.joseErrorCount.With(prometheus.Labels{"type": "JWSBodyUnmarshalFailed"}).Inc()
464470
return nil, probs.Malformed("Request payload did not parse as JSON")
465471
}
466472

@@ -529,7 +535,7 @@ func (wfe *WebFrontEndImpl) validSelfAuthenticatedPOST(
529535

530536
// If the key doesn't meet the GoodKey policy return a problem immediately
531537
if err := wfe.keyPolicy.GoodKey(pubKey.Key); err != nil {
532-
wfe.stats.Inc("Errors.JWKRejectedByGoodKey", 1)
538+
wfe.stats.joseErrorCount.With(prometheus.Labels{"type": "JWKRejectedByGoodKey"}).Inc()
533539
return nil, nil, probs.Malformed(err.Error())
534540
}
535541

@@ -576,7 +582,7 @@ func (wfe *WebFrontEndImpl) validKeyRollover(
576582

577583
// If the key doesn't meet the GoodKey policy return a problem immediately
578584
if err := wfe.keyPolicy.GoodKey(jwk.Key); err != nil {
579-
wfe.stats.Inc("Errors.InnerJWKRejectedByGoodKey", 1)
585+
wfe.stats.joseErrorCount.With(prometheus.Labels{"type": "KeyRolloverJWKRejectedByGoodKey"}).Inc()
580586
return nil, probs.Malformed(err.Error())
581587
}
582588

@@ -591,21 +597,22 @@ func (wfe *WebFrontEndImpl) validKeyRollover(
591597
// HTTP request to match the URL to)
592598
innerPayload, err := innerJWS.Verify(jwk)
593599
if err != nil {
594-
wfe.stats.Inc("Errors.InnerJWSVerificationFailed", 1)
600+
wfe.stats.joseErrorCount.With(prometheus.Labels{"type": "KeyRolloverJWSVerifyFailed"}).Inc()
595601
return nil, probs.Malformed("Inner JWS does not verify with embedded JWK")
596602
}
597603
// NOTE(@cpu): we do not stomp the requestEvent's payload here since that is set
598604
// from the outerJWS in validPOSTForAccount and contains the inner JWS and inner
599605
// payload already.
600606

601607
// Verify that the outer and inner JWS protected URL headers match
602-
if matchJWSURLs(outerJWS, innerJWS) != nil {
608+
if wfe.matchJWSURLs(outerJWS, innerJWS) != nil {
603609
return nil, prob
604610
}
605611

606612
// Unmarshal the inner JWS' key roll over request
607613
var req rolloverRequest
608614
if json.Unmarshal(innerPayload, &req) != nil {
615+
wfe.stats.joseErrorCount.With(prometheus.Labels{"type": "KeyRolloverUnmarshalFailed"}).Inc()
609616
return nil, probs.Malformed(
610617
"Inner JWS payload did not parse as JSON key rollover object")
611618
}
@@ -614,7 +621,7 @@ func (wfe *WebFrontEndImpl) validKeyRollover(
614621
// JWS. So far we've only checked that the JWK embedded in the inner JWS valides
615622
// the JWS.
616623
if _, err := innerJWS.Verify(req.NewKey); err != nil {
617-
wfe.stats.Inc("Errors.InnerJWSVerificationFailed", 1)
624+
wfe.stats.joseErrorCount.With(prometheus.Labels{"type": "KeyRolloverJWSNewKeyVerifyFailed"}).Inc()
618625
return nil, probs.Malformed("Inner JWS does not verify with specified new key")
619626
}
620627

0 commit comments

Comments
 (0)