Skip to content

Commit 8f5de53

Browse files
cpucpu
authored
RA: Add PerformValidation RPC to replace UpdateAuthorization. (letsencrypt#3942)
The existing RA `UpdateAuthorization` RPC needs replacing for two reasons: 1. The name isn't accurate - `PerformValidation` better captures the purpose of the RPC. 2. The `core.Challenge` argument is superfluous since Key Authorizations are not sent in the initiation POST from the client anymore. The corresponding unmarshal and verification is now removed. Notably this means broken clients that were POSTing the wrong thing and failing pre-validation will now likely fail post-validation. To remove `UpdateAuthorization` the new `PerformValidation` RPC is added alongside the old one. WFE and WFE2 are updated to use the new RPC when the perform validation feature flag is enabled. We can remove `UpdateAuthorization` and its associated wrappers once all WFE instances have been updated. Resolves letsencrypt#3930
1 parent ba7a8e8 commit 8f5de53

15 files changed

Lines changed: 362 additions & 122 deletions

File tree

‎core/interfaces.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,13 @@ type RegistrationAuthority interface {
6969
UpdateRegistration(ctx context.Context, base, updates Registration) (Registration, error)
7070

7171
// [WebFrontEnd]
72+
// TODO(@cpu): Remove UpdateAuthorization - it is replaced by
73+
// PerformValidation. See https://github.com/letsencrypt/boulder/issues/3947
7274
UpdateAuthorization(ctx context.Context, authz Authorization, challengeIndex int, response Challenge) (Authorization, error)
7375

76+
// [WebFrontEnd]
77+
PerformValidation(ctx context.Context, req *rapb.PerformValidationRequest) (*corepb.Authorization, error)
78+
7479
// [WebFrontEnd]
7580
RevokeCertificateWithReg(ctx context.Context, cert x509.Certificate, code revocation.Reason, regID int64) error
7681

‎features/featureflag_string.go‎

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎features/features.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,9 @@ const (
4141
// SimplifiedVAHTTP enables the simplified VA http-01 rewrite that doesn't use
4242
// a custom dialer.
4343
SimplifiedVAHTTP
44+
// PerformValidationRPC enables the WFE/WFE2 to use the RA's PerformValidation
45+
// RPC instead of the deprecated UpdateAuthorization RPC.
46+
PerformValidationRPC
4447
)
4548

4649
// List of features and their default value, protected by fMu
@@ -66,6 +69,7 @@ var features = map[FeatureFlag]bool{
6669
ACME13KeyRollover: false,
6770
ProbeCTLogs: false,
6871
SimplifiedVAHTTP: false,
72+
PerformValidationRPC: false,
6973
}
7074

7175
var fMu = new(sync.RWMutex)

‎grpc/ra-wrappers.go‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,21 @@ func (rac RegistrationAuthorityClientWrapper) UpdateAuthorization(ctx context.Co
122122
return PBToAuthz(response)
123123
}
124124

125+
func (rac RegistrationAuthorityClientWrapper) PerformValidation(
126+
ctx context.Context,
127+
req *rapb.PerformValidationRequest) (*corepb.Authorization, error) {
128+
authz, err := rac.inner.PerformValidation(ctx, req)
129+
if err != nil {
130+
return nil, err
131+
}
132+
133+
if authz == nil || !authorizationValid(authz) {
134+
return nil, errIncompleteResponse
135+
}
136+
137+
return authz, nil
138+
}
139+
125140
func (rac RegistrationAuthorityClientWrapper) RevokeCertificateWithReg(ctx context.Context, cert x509.Certificate, code revocation.Reason, regID int64) error {
126141
reason := int64(code)
127142
_, err := rac.inner.RevokeCertificateWithReg(ctx, &rapb.RevokeCertificateWithRegRequest{
@@ -292,6 +307,15 @@ func (ras *RegistrationAuthorityServerWrapper) UpdateAuthorization(ctx context.C
292307
return AuthzToPB(newAuthz)
293308
}
294309

310+
func (ras *RegistrationAuthorityServerWrapper) PerformValidation(
311+
ctx context.Context,
312+
request *rapb.PerformValidationRequest) (*corepb.Authorization, error) {
313+
if request == nil || !authorizationValid(request.Authz) || request.ChallengeIndex == nil {
314+
return nil, errIncompleteRequest
315+
}
316+
return ras.inner.PerformValidation(ctx, request)
317+
}
318+
295319
func (ras *RegistrationAuthorityServerWrapper) RevokeCertificateWithReg(ctx context.Context, request *rapb.RevokeCertificateWithRegRequest) (*corepb.Empty, error) {
296320
if request == nil || request.Cert == nil || request.Code == nil || request.RegID == nil {
297321
return nil, errIncompleteRequest

‎ra/proto/ra.pb.go‎

Lines changed: 108 additions & 42 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎ra/proto/ra.proto‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,10 @@ service RegistrationAuthority {
1010
rpc NewAuthorization(NewAuthorizationRequest) returns (core.Authorization) {}
1111
rpc NewCertificate(NewCertificateRequest) returns (core.Certificate) {}
1212
rpc UpdateRegistration(UpdateRegistrationRequest) returns (core.Registration) {}
13+
// TODO(@cpu): Remove UpdateAuthorization. It is deprecated in favour of
14+
// PerformValidation. See https://github.com/letsencrypt/boulder/issues/3947
1315
rpc UpdateAuthorization(UpdateAuthorizationRequest) returns (core.Authorization) {}
16+
rpc PerformValidation(PerformValidationRequest) returns (core.Authorization) {}
1417
rpc RevokeCertificateWithReg(RevokeCertificateWithRegRequest) returns (core.Empty) {}
1518
rpc DeactivateRegistration(core.Registration) returns (core.Empty) {}
1619
rpc DeactivateAuthorization(core.Authorization) returns (core.Empty) {}
@@ -40,6 +43,11 @@ message UpdateAuthorizationRequest {
4043
optional core.Challenge response = 3;
4144
}
4245

46+
message PerformValidationRequest {
47+
optional core.Authorization authz = 1;
48+
optional int64 challengeIndex = 2;
49+
}
50+
4351
message RevokeCertificateWithRegRequest {
4452
optional bytes cert = 1;
4553
optional int64 code = 2;

‎ra/ra.go‎

Lines changed: 48 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1409,23 +1409,53 @@ func mergeUpdate(r *core.Registration, input core.Registration) bool {
14091409
return changed
14101410
}
14111411

1412-
// UpdateAuthorization updates an authorization with new values.
1412+
// UpdateAuthorization is a legacy function in the process of being replaced by
1413+
// PerformValidation.
1414+
// TODO(@cpu): Remove this. https://github.com/letsencrypt/boulder/issues/3947
14131415
func (ra *RegistrationAuthorityImpl) UpdateAuthorization(
14141416
ctx context.Context,
14151417
base core.Authorization,
14161418
challengeIndex int,
1417-
response core.Challenge) (core.Authorization, error) {
1419+
_ core.Challenge) (core.Authorization, error) {
1420+
authzPB, err := bgrpc.AuthzToPB(base)
1421+
if err != nil {
1422+
return core.Authorization{}, err
1423+
}
1424+
challIndex := int64(challengeIndex)
1425+
authzPB, err = ra.PerformValidation(ctx, &rapb.PerformValidationRequest{
1426+
Authz: authzPB,
1427+
ChallengeIndex: &challIndex,
1428+
})
1429+
if err != nil {
1430+
return core.Authorization{}, err
1431+
}
1432+
return bgrpc.PBToAuthz(authzPB)
1433+
}
1434+
1435+
// PerformValidation initiates validation for a specific challenge associated
1436+
// with the given base authorization. The authorization and challenge are
1437+
// updated based on the results.
1438+
func (ra *RegistrationAuthorityImpl) PerformValidation(
1439+
ctx context.Context,
1440+
req *rapb.PerformValidationRequest) (*corepb.Authorization, error) {
1441+
base, err := bgrpc.PBToAuthz(req.Authz)
1442+
if err != nil {
1443+
return nil, err
1444+
}
1445+
14181446
// Refuse to update expired authorizations
14191447
if base.Expires == nil || base.Expires.Before(ra.clk.Now()) {
1420-
return core.Authorization{}, berrors.MalformedError("expired authorization")
1448+
return nil, berrors.MalformedError("expired authorization")
14211449
}
14221450

14231451
authz := base
1424-
if challengeIndex >= len(authz.Challenges) {
1425-
return core.Authorization{}, berrors.MalformedError("invalid challenge index '%d'", challengeIndex)
1452+
challIndex := int(*req.ChallengeIndex)
1453+
if challIndex >= len(authz.Challenges) {
1454+
return nil,
1455+
berrors.MalformedError("invalid challenge index '%d'", challIndex)
14261456
}
14271457

1428-
ch := &authz.Challenges[challengeIndex]
1458+
ch := &authz.Challenges[challIndex]
14291459

14301460
// If TLSSNIRevalidation is enabled, find out whether this was a revalidation
14311461
// (previous certificate existed) or not. If it is a revalidation, we can
@@ -1437,10 +1467,11 @@ func (ra *RegistrationAuthorityImpl) UpdateAuthorization(
14371467
RegID: &authz.RegistrationID,
14381468
})
14391469
if err != nil {
1440-
return core.Authorization{}, err
1470+
return nil, err
14411471
}
14421472
if !*existsResp.Exists {
1443-
return core.Authorization{}, berrors.MalformedError("challenge type %q no longer allowed", ch.Type)
1473+
return nil,
1474+
berrors.MalformedError("challenge type %q no longer allowed", ch.Type)
14441475
}
14451476
}
14461477

@@ -1451,34 +1482,23 @@ func (ra *RegistrationAuthorityImpl) UpdateAuthorization(
14511482
// case and return early.
14521483
if ra.reuseValidAuthz && authz.Status == core.StatusValid {
14531484
ra.stats.Inc("ReusedValidAuthzChallenge", 1)
1454-
return authz, nil
1485+
return req.Authz, nil
14551486
}
14561487

14571488
if authz.Status != core.StatusPending {
1458-
return core.Authorization{}, berrors.WrongAuthorizationStateError("authorization must be pending")
1489+
return nil, berrors.WrongAuthorizationStateError("authorization must be pending")
14591490
}
14601491

14611492
// Look up the account key for this authorization
14621493
reg, err := ra.SA.GetRegistration(ctx, authz.RegistrationID)
14631494
if err != nil {
1464-
return core.Authorization{}, berrors.InternalServerError(err.Error())
1495+
return nil, berrors.InternalServerError(err.Error())
14651496
}
14661497

14671498
// Compute the key authorization field based on the registration key
14681499
expectedKeyAuthorization, err := ch.ExpectedKeyAuthorization(reg.Key)
14691500
if err != nil {
1470-
return core.Authorization{}, berrors.InternalServerError("could not compute expected key authorization value")
1471-
}
1472-
1473-
// NOTE(@cpu): Historically challenge update required the client to send
1474-
// a JSON POST body that included a computed KeyAuthorization. The RA would
1475-
// check this provided authorization against its own computation of the key
1476-
// authorization and err if they did not match. New ACME specification does
1477-
// not require this - the client does not need to send the key authorization.
1478-
// To support this for ACMEv2 we only enforce the provided key authorization
1479-
// matches expected if the update included it.
1480-
if response.ProvidedKeyAuthorization != "" && expectedKeyAuthorization != response.ProvidedKeyAuthorization {
1481-
return core.Authorization{}, berrors.MalformedError("provided key authorization was incorrect")
1501+
return nil, berrors.InternalServerError("could not compute expected key authorization value")
14821502
}
14831503

14841504
// Populate the ProvidedKeyAuthorization such that the VA can confirm the
@@ -1491,7 +1511,7 @@ func (ra *RegistrationAuthorityImpl) UpdateAuthorization(
14911511

14921512
// Double check before sending to VA
14931513
if cErr := ch.CheckConsistencyForValidation(); cErr != nil {
1494-
return core.Authorization{}, berrors.MalformedError(cErr.Error())
1514+
return nil, berrors.MalformedError(cErr.Error())
14951515
}
14961516

14971517
ra.stats.Inc("NewPendingAuthorizations", 1)
@@ -1506,7 +1526,7 @@ func (ra *RegistrationAuthorityImpl) UpdateAuthorization(
15061526
copy(challenges, authz.Challenges)
15071527
authz.Challenges = challenges
15081528

1509-
records, err := ra.VA.PerformValidation(vaCtx, authz.Identifier.Value, authz.Challenges[challengeIndex], authz)
1529+
records, err := ra.VA.PerformValidation(vaCtx, authz.Identifier.Value, authz.Challenges[challIndex], authz)
15101530
var prob *probs.ProblemDetails
15111531
if p, ok := err.(*probs.ProblemDetails); ok {
15121532
prob = p
@@ -1516,7 +1536,7 @@ func (ra *RegistrationAuthorityImpl) UpdateAuthorization(
15161536
}
15171537

15181538
// Save the updated records
1519-
challenge := &authz.Challenges[challengeIndex]
1539+
challenge := &authz.Challenges[challIndex]
15201540
challenge.ValidationRecord = records
15211541

15221542
if !challenge.RecordsSane() && prob == nil {
@@ -1529,7 +1549,7 @@ func (ra *RegistrationAuthorityImpl) UpdateAuthorization(
15291549
} else {
15301550
challenge.Status = core.StatusValid
15311551
}
1532-
authz.Challenges[challengeIndex] = *challenge
1552+
authz.Challenges[challIndex] = *challenge
15331553

15341554
err = ra.onValidationUpdate(vaCtx, authz)
15351555
if err != nil {
@@ -1538,7 +1558,7 @@ func (ra *RegistrationAuthorityImpl) UpdateAuthorization(
15381558
}
15391559
}(authz)
15401560
ra.stats.Inc("UpdatedPendingAuthorizations", 1)
1541-
return authz, nil
1561+
return bgrpc.AuthzToPB(authz)
15421562
}
15431563

15441564
func revokeEvent(state, serial, cn string, names []string, revocationCode revocation.Reason) string {

0 commit comments

Comments
 (0)