letsencrypt
/
boulder
mirror of https://github.com/letsencrypt/boulder.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

WFE2: Implement draft-13 keyrollover with feature flag. (#3813) 

ACME draft-13 changed the inner JWS' body to contain the old key
that signed the outer JWS. Because this is a backwards incompatible
change with the draft-12 ACME v2 key rollover we introduce a new feature
flag `features.ACME13KeyRollover` and conditionally use the old or new
key rollover semantics based on its value.
This commit is contained in:
Daniel McCarney 2018-08-07 15:27:25 -04:00 committed by GitHub
parent 52e5f20806
commit 0cb28c9e02
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 174 additions and 37 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
features/featureflag_string.go
View File

@ -4,9 +4,9 @@ package features
import "strconv"
const _FeatureFlag_name = "unusedReusePendingAuthzCountCertificatesExactIPv6FirstAllowRenewalFirstRLWildcardDomainsForceConsistentStatusEnforceChallengeDisableRPCHeadroomTLSSNIRevalidationEmbedSCTsCancelCTSubmissionsVAChecksGSBEnforceV2ContentTypeEnforceOverlappingWildcardsOrderReadyStatusCAAValidationMethodsCAAAccountURI"
const _FeatureFlag_name = "unusedReusePendingAuthzCountCertificatesExactIPv6FirstAllowRenewalFirstRLWildcardDomainsForceConsistentStatusEnforceChallengeDisableRPCHeadroomTLSSNIRevalidationEmbedSCTsCancelCTSubmissionsVAChecksGSBEnforceV2ContentTypeEnforceOverlappingWildcardsOrderReadyStatusCAAValidationMethodsCAAAccountURIACME13KeyRollover"
var _FeatureFlag_index = [...]uint16{0, 6, 23, 45, 54, 73, 88, 109, 132, 143, 161, 170, 189, 200, 220, 247, 263, 283, 296}
var _FeatureFlag_index = [...]uint16{0, 6, 23, 45, 54, 73, 88, 109, 132, 143, 161, 170, 189, 200, 220, 247, 263, 283, 296, 313}
func (i FeatureFlag) String() string {
if i < 0 || i >= FeatureFlag(len(_FeatureFlag_index)-1) {

3
features/features.go
View File

@ -42,6 +42,8 @@ const (
CAAValidationMethods
// Check CAA and respect accounturi parameter.
CAAAccountURI
// Honour draft-ietf-acme-13's keyrollover
ACME13KeyRollover
)
// List of features and their default value, protected by fMu
@ -64,6 +66,7 @@ var features = map[FeatureFlag]bool{
OrderReadyStatus: false,
CAAValidationMethods: false,
CAAAccountURI: false,
ACME13KeyRollover: false,
}
var fMu = new(sync.RWMutex)

3
test/config-next/wfe2.json
View File

@ -37,7 +37,8 @@
},
"features": {
"EnforceV2ContentType": true,
"RPCHeadroom": true
"RPCHeadroom": true,
"ACME13KeyRollover": true
}
},

110
wfe2/verify.go
View File

@ -603,12 +603,33 @@ func (wfe *WebFrontEndImpl) validSelfAuthenticatedPOST(
return wfe.validSelfAuthenticatedJWS(jws, request, logEvent)
}
// rolloverRequest is a struct representing an ACME key rollover request
// rolloverOperation is a struct representing a requested rollover operation
// from the specified old key to the new key for the given account ID. It is
// composed based on a rolloverRequest or a rolloverRequestDraft13 based on the
// `features.ACME13KeyRollover` feature flag.
type rolloverOperation struct {
OldKey jose.JSONWebKey
NewKey jose.JSONWebKey
Account string
}
// rolloverRequest is a client request to change the account ID provided to the
// new key provided. This is the legacy format used by ACMEv2 pre-draft13.
// TODO(@cpu): Delete this type once features.ACME13KeyRollover is active
// everywhere.
type rolloverRequest struct {
NewKey jose.JSONWebKey
Account string
}
// rolloverRequestDraft13 is a client request to change the key for the account
// ID provided from the specified old key to a new key (the embedded JWK in the
// inner JWS). This type is used when features.ACME13KeyRollover is enabled.
type rolloverRequestDraft13 struct {
OldKey jose.JSONWebKey
Account string
}
// validKeyRollover checks if the innerJWS is a valid key rollover operation
// given the outer JWS that carried it. It is assumed that the outerJWS has
// already been validated per the normal ACME process using `validPOSTForAccount`.
@ -617,17 +638,26 @@ type rolloverRequest struct {
// 1) the inner JWS is valid and well formed
// 2) the inner JWS has the same "url" header as the outer JWS
// 3) the inner JWS is self-authenticated with an embedded JWK
// 4) the payload of the inner JWS is a valid JSON key change object
// 5) that the key specified in the key change object *also* verifies the inner JWS
// A *rolloverRequest object and is returned if successfully validated,
// otherwise a problem is returned. The caller is left to verify whether the
// new key is appropriate (e.g. isn't being used by another existing account)
// and that the account field of the rollover object matches the account that
// verified the outer JWS.
//
// If features.ACME13KeyRollover is enabled this function verifies that the
// inner JWS' body is a rolloverRequestDraft13 instance that specifies the
// correct oldKey. The returned rolloverOperation's NewKey field will be set to
// the JWK from the inner JWS.
// If features.ACME13KeyRollover is disabled this function verifies that the
// inner JWS' body is a rolloverRequest instance and that the key in newKey
// validates the inner JWS signature. The returned rolloverOperations' NewKey
// field will be set to the JWK from the rolloverRequest.
//
// In both cases A *rolloverOperation object and is returned if successfully
// validated, otherwise a problem is returned. The caller is left to verify
// whether the new key is appropriate (e.g. isn't being used by another existing
// account) and that the account field of the rollover object matches the
// account that verified the outer JWS.
func (wfe *WebFrontEndImpl) validKeyRollover(
outerJWS *jose.JSONWebSignature,
innerJWS *jose.JSONWebSignature,
logEvent *web.RequestEvent) (*rolloverRequest, *probs.ProblemDetails) {
oldKey *jose.JSONWebKey,
logEvent *web.RequestEvent) (*rolloverOperation, *probs.ProblemDetails) {
// Extract the embedded JWK from the inner JWS
jwk, prob := wfe.extractJWK(innerJWS)
@ -664,21 +694,53 @@ func (wfe *WebFrontEndImpl) validKeyRollover(
return nil, prob
}
// Unmarshal the inner JWS' key roll over request
var req rolloverRequest
if json.Unmarshal(innerPayload, &req) != nil {
wfe.stats.joseErrorCount.With(prometheus.Labels{"type": "KeyRolloverUnmarshalFailed"}).Inc()
return nil, probs.Malformed(
"Inner JWS payload did not parse as JSON key rollover object")
result := &rolloverOperation{OldKey: *oldKey}
if !features.Enabled(features.ACME13KeyRollover) {
// Unmarshal the inner JWS' key roll over request
var req rolloverRequest
if json.Unmarshal(innerPayload, &req) != nil {
wfe.stats.joseErrorCount.With(prometheus.Labels{"type": "KeyRolloverUnmarshalFailed"}).Inc()
return nil, probs.Malformed(
"Inner JWS payload did not parse as JSON key rollover object")
}
// Verify that the key roll over request's NewKey *also* validates the inner
// JWS. So far we've only checked that the JWK embedded in the inner JWS valides
// the JWS.
if _, err := innerJWS.Verify(req.NewKey); err != nil {
wfe.stats.joseErrorCount.With(prometheus.Labels{"type": "KeyRolloverJWSNewKeyVerifyFailed"}).Inc()
return nil, probs.Malformed("Inner JWS does not verify with specified new key")
}
result.Account = req.Account
// Use the JWK specified in the key rollover request in pre-draft-13 key rollovers
result.NewKey = req.NewKey
} else {
var req rolloverRequestDraft13
if json.Unmarshal(innerPayload, &req) != nil {
wfe.stats.joseErrorCount.With(prometheus.Labels{"type": "KeyRolloverUnmarshalFailed"}).Inc()
return nil, probs.Malformed(
"Inner JWS payload did not parse as JSON key rollover object")
}
// If there's no oldkey specified fail before trying to use
// core.PublicKeyEqual on a nil argument.
if req.OldKey.Key == nil {
wfe.stats.joseErrorCount.With(prometheus.Labels{"type": "KeyRolloverWrongOldKey"}).Inc()
return nil, probs.Malformed("Inner JWS does not contain old key field matching current account key")
}
// If ACME13KeyRollover is enabled then we must validate that the inner JWS'
// rollover request specifies the correct oldKey.
if keysEqual, err := core.PublicKeysEqual(req.OldKey.Key, oldKey.Key); err != nil {
return nil, probs.Malformed("Unable to compare new and old keys: %s", err.Error())
} else if !keysEqual {
wfe.stats.joseErrorCount.With(prometheus.Labels{"type": "KeyRolloverWrongOldKey"}).Inc()
return nil, probs.Malformed("Inner JWS does not contain old key field matching current account key")
}
result.Account = req.Account
// Use the JWK pulled from the inner JWS as the new key for draft-13 key rollovers
result.NewKey = *jwk
}
// Verify that the key roll over request's NewKey *also* validates the inner
// JWS. So far we've only checked that the JWK embedded in the inner JWS valides
// the JWS.
if _, err := innerJWS.Verify(req.NewKey); err != nil {
wfe.stats.joseErrorCount.With(prometheus.Labels{"type": "KeyRolloverJWSNewKeyVerifyFailed"}).Inc()
return nil, probs.Malformed("Inner JWS does not verify with specified new key")
}
return &req, nil
return result, nil
}

11
wfe2/wfe.go
View File

@ -1373,6 +1373,7 @@ func (wfe *WebFrontEndImpl) KeyRollover(
wfe.sendError(response, logEvent, prob, nil)
return
}
oldKey := acct.Key
// Parse the inner JWS from the validated outer JWS body
innerJWS, prob := wfe.parseJWS(outerBody)
@ -1382,21 +1383,21 @@ func (wfe *WebFrontEndImpl) KeyRollover(
}
// Validate the inner JWS as a key rollover request for the outer JWS
rolloverRequest, prob := wfe.validKeyRollover(outerJWS, innerJWS, logEvent)
rolloverOperation, prob := wfe.validKeyRollover(outerJWS, innerJWS, oldKey, logEvent)
if prob != nil {
wfe.sendError(response, logEvent, prob, nil)
return
}
newKey := rolloverRequest.NewKey
newKey := rolloverOperation.NewKey
// Check that the rollover request's account URL matches the account URL used
// to validate the outer JWS
header := outerJWS.Signatures[0].Header
if rolloverRequest.Account != header.KeyID {
if rolloverOperation.Account != header.KeyID {
wfe.stats.joseErrorCount.With(prometheus.Labels{"type": "KeyRolloverMismatchedAccount"}).Inc()
wfe.sendError(response, logEvent, probs.Malformed(
fmt.Sprintf("Inner key rollover request specified Account %q, but outer JWS has Key ID %q",
rolloverRequest.Account, header.KeyID)), nil)
rolloverOperation.Account, header.KeyID)), nil)
return
}
@ -1405,7 +1406,7 @@ func (wfe *WebFrontEndImpl) KeyRollover(
// will find the old account if its equal to the old account key. We
// check new key against old key explicitly to save an RPC round trip and a DB
// query for this easy rejection case
keysEqual, err := core.PublicKeysEqual(newKey.Key, acct.Key.Key)
keysEqual, err := core.PublicKeysEqual(newKey.Key, oldKey.Key)
if err != nil {
// This should not happen - both the old and new key have been validated by now
wfe.sendError(response, logEvent, probs.ServerInternal("Unable to compare new and old keys"), err)

80
wfe2/wfe_test.go
View File

@ -27,6 +27,7 @@ import (
"github.com/letsencrypt/boulder/core"
corepb "github.com/letsencrypt/boulder/core/proto"
berrors "github.com/letsencrypt/boulder/errors"
"github.com/letsencrypt/boulder/features"
"github.com/letsencrypt/boulder/goodkey"
blog "github.com/letsencrypt/boulder/log"
"github.com/letsencrypt/boulder/metrics"
@ -2052,11 +2053,12 @@ func TestKeyRollover(t *testing.T) {
}`)
testCases := []struct {
Name string
Payload string
ExpectedResponse string
NewKey crypto.Signer
ErrorStatType string
Name string
ACME13KeyRollover bool
Payload string
ExpectedResponse string
NewKey crypto.Signer
ErrorStatType string
}{
{
Name: "Missing account URL",
@ -2125,10 +2127,78 @@ func TestKeyRollover(t *testing.T) {
}`,
NewKey: newKeyPriv,
},
{
Name: "ACME13KeyRollover, legacy rollover request",
ACME13KeyRollover: true,
Payload: `{"newKey":` + string(newJWKJSON) + `,"account":"http://localhost/acme/acct/1"}`,
ExpectedResponse: `{
"type": "` + probs.V2ErrorNS + `malformed",
"detail": "Inner JWS does not contain old key field matching current account key",
"status": 400
}`,
NewKey: newKeyPriv,
ErrorStatType: "KeyRolloverWrongOldKey",
},
{
Name: "ACME13KeyRollover, Missing account URL",
ACME13KeyRollover: true,
Payload: `{"oldKey":` + test1KeyPublicJSON + `}`,
ExpectedResponse: `{
"type": "` + probs.V2ErrorNS + `malformed",
"detail": "Inner key rollover request specified Account \"\", but outer JWS has Key ID \"http://localhost/acme/acct/1\"",
"status": 400
}`,
NewKey: newKeyPriv,
ErrorStatType: "KeyRolloverMismatchedAccount",
},
{
Name: "ACME13KeyRollover, incorrect old key",
ACME13KeyRollover: true,
Payload: `{"oldKey":` + string(newJWKJSON) + `,"account":"http://localhost/acme/acct/1"}`,
ExpectedResponse: `{
"type": "` + probs.V2ErrorNS + `malformed",
"detail": "Inner JWS does not contain old key field matching current account key",
"status": 400
}`,
NewKey: newKeyPriv,
ErrorStatType: "KeyRolloverWrongOldKey",
},
{
Name: "ACME13KeyRollover, Valid key rollover request, key exists",
ACME13KeyRollover: true,
Payload: `{"oldKey":` + test1KeyPublicJSON + `,"account":"http://localhost/acme/acct/1"}`,
ExpectedResponse: `{
"type": "urn:ietf:params:acme:error:malformed",
"detail": "New key is already in use for a different account",
"status": 409
}`,
NewKey: existingKey,
},
{
Name: "ACME13KeyRollover, Valid key rollover request",
ACME13KeyRollover: true,
Payload: `{"oldKey":` + test1KeyPublicJSON + `,"account":"http://localhost/acme/acct/1"}`,
ExpectedResponse: `{
"id": 1,
"key": ` + string(newJWKJSON) + `,
"contact": [
"mailto:person@mail.com"
],
"agreement": "http://example.invalid/terms",
"initialIp": "",
"createdAt": "0001-01-01T00:00:00Z",
"status": "valid"
}`,
NewKey: newKeyPriv,
},
}
for _, tc := range testCases {
t.Run(tc.Name, func(t *testing.T) {
if tc.ACME13KeyRollover {
_ = features.Set(map[string]bool{"ACME13KeyRollover": true})
defer features.Reset()
}
wfe.stats.joseErrorCount.Reset()
responseWriter.Body.Reset()
_, _, inner := signRequestEmbed(t, tc.NewKey, "http://localhost/key-change", tc.Payload, wfe.nonceService)