wfe: return proper error for goodkey timeout (#6938)

In WFE, we do a goodkey check when validating a self-authenticated POST
(i.e. when creating an account). For a while, that was a purely local
check, looking at a list of bad keys or bad moduluses, or checking for
factorability. At some point we also added a backend check, querying the
SA to see if a key was blocked. However, we did not update this one code
path to distinguish "bad key" from "timeout querying SA." That meant
that sometimes we would give a badPublicKey Problem Document when we
should have given an internalServerError.

Related:
https://github.com/letsencrypt/boulder/issues/6795#issuecomment-1574217398
This commit is contained in:
Jacob Hoffman-Andrews 2023-06-20 12:42:21 -07:00 committed by GitHub
parent a2b2e53045
commit cde4b9c90f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 41 additions and 2 deletions

View File

@ -18,6 +18,7 @@ import (
"github.com/letsencrypt/boulder/core"
berrors "github.com/letsencrypt/boulder/errors"
"github.com/letsencrypt/boulder/goodkey"
"github.com/letsencrypt/boulder/grpc"
"github.com/letsencrypt/boulder/nonce"
noncepb "github.com/letsencrypt/boulder/nonce/proto"
@ -682,8 +683,11 @@ func (wfe *WebFrontEndImpl) validSelfAuthenticatedPOST(
// If the key doesn't meet the GoodKey policy return a problem
err := wfe.keyPolicy.GoodKey(ctx, pubKey.Key)
if err != nil {
wfe.stats.joseErrorCount.With(prometheus.Labels{"type": "JWKRejectedByGoodKey"}).Inc()
return nil, nil, probs.BadPublicKey(err.Error())
if errors.Is(err, goodkey.ErrBadKey) {
wfe.stats.joseErrorCount.With(prometheus.Labels{"type": "JWKRejectedByGoodKey"}).Inc()
return nil, nil, probs.BadPublicKey(err.Error())
}
return nil, nil, probs.ServerInternal("error checking key quality")
}
return payload, pubKey, nil

View File

@ -14,6 +14,7 @@ import (
"github.com/letsencrypt/boulder/core"
corepb "github.com/letsencrypt/boulder/core/proto"
"github.com/letsencrypt/boulder/goodkey"
bgrpc "github.com/letsencrypt/boulder/grpc"
"github.com/letsencrypt/boulder/mocks"
"github.com/letsencrypt/boulder/probs"
@ -1495,6 +1496,40 @@ func TestValidPOSTForAccountSwappedKey(t *testing.T) {
test.AssertEquals(t, prob.Detail, "JWS verification error")
}
func TestValidSelfAuthenticatedPOSTGoodKeyErrors(t *testing.T) {
wfe, _, signer := setupWFE(t)
timeoutErrCheckFunc := func(ctx context.Context, keyHash []byte) (bool, error) {
return false, context.DeadlineExceeded
}
kp, err := goodkey.NewKeyPolicy(&goodkey.Config{}, timeoutErrCheckFunc)
test.AssertNotError(t, err, "making key policy")
wfe.keyPolicy = kp
_, _, validJWSBody := signer.embeddedJWK(nil, "http://localhost/test", `{"test":"passed"}`)
request := makePostRequestWithPath("test", validJWSBody)
_, _, prob := wfe.validSelfAuthenticatedPOST(context.Background(), request)
test.AssertEquals(t, prob.Type, probs.ServerInternalProblem)
badKeyCheckFunc := func(ctx context.Context, keyHash []byte) (bool, error) {
return false, fmt.Errorf("oh no: %w", goodkey.ErrBadKey)
}
kp, err = goodkey.NewKeyPolicy(&goodkey.Config{}, badKeyCheckFunc)
test.AssertNotError(t, err, "making key policy")
wfe.keyPolicy = kp
_, _, validJWSBody = signer.embeddedJWK(nil, "http://localhost/test", `{"test":"passed"}`)
request = makePostRequestWithPath("test", validJWSBody)
_, _, prob = wfe.validSelfAuthenticatedPOST(context.Background(), request)
test.AssertEquals(t, prob.Type, probs.BadPublicKeyProblem)
}
func TestValidSelfAuthenticatedPOST(t *testing.T) {
wfe, _, signer := setupWFE(t)