RA: Add PerformValidation RPC to replace UpdateAuthorization. (#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 #3930

core/interfaces.go

@@ -69,8 +69,13 @@ type RegistrationAuthority interface {
 	UpdateRegistration(ctx context.Context, base, updates Registration) (Registration, error)
 
 	// [WebFrontEnd]
+	// TODO(@cpu): Remove UpdateAuthorization - it is replaced by
+	// PerformValidation. See https://github.com/letsencrypt/boulder/issues/3947
 	UpdateAuthorization(ctx context.Context, authz Authorization, challengeIndex int, response Challenge) (Authorization, error)
 
+	// [WebFrontEnd]
+	PerformValidation(ctx context.Context, req *rapb.PerformValidationRequest) (*corepb.Authorization, error)
+
 	// [WebFrontEnd]
 	RevokeCertificateWithReg(ctx context.Context, cert x509.Certificate, code revocation.Reason, regID int64) error
 

features/features.go

@@ -41,6 +41,9 @@ const (
 	// SimplifiedVAHTTP enables the simplified VA http-01 rewrite that doesn't use
 	// a custom dialer.
 	SimplifiedVAHTTP
+	// PerformValidationRPC enables the WFE/WFE2 to use the RA's PerformValidation
+	// RPC instead of the deprecated UpdateAuthorization RPC.
+	PerformValidationRPC
 )
 
 // List of features and their default value, protected by fMu
@@ -66,6 +69,7 @@ var features = map[FeatureFlag]bool{
 	ACME13KeyRollover:           false,
 	ProbeCTLogs:                 false,
 	SimplifiedVAHTTP:            false,
+	PerformValidationRPC:        false,
 }
 
 var fMu = new(sync.RWMutex)

grpc/ra-wrappers.go

@@ -122,6 +122,21 @@ func (rac RegistrationAuthorityClientWrapper) UpdateAuthorization(ctx context.Co
 	return PBToAuthz(response)
 }
 
+func (rac RegistrationAuthorityClientWrapper) PerformValidation(
+	ctx context.Context,
+	req *rapb.PerformValidationRequest) (*corepb.Authorization, error) {
+	authz, err := rac.inner.PerformValidation(ctx, req)
+	if err != nil {
+		return nil, err
+	}
+
+	if authz == nil || !authorizationValid(authz) {
+		return nil, errIncompleteResponse
+	}
+
+	return authz, nil
+}
+
 func (rac RegistrationAuthorityClientWrapper) RevokeCertificateWithReg(ctx context.Context, cert x509.Certificate, code revocation.Reason, regID int64) error {
 	reason := int64(code)
 	_, err := rac.inner.RevokeCertificateWithReg(ctx, &rapb.RevokeCertificateWithRegRequest{
@@ -292,6 +307,15 @@ func (ras *RegistrationAuthorityServerWrapper) UpdateAuthorization(ctx context.C
 	return AuthzToPB(newAuthz)
 }
 
+func (ras *RegistrationAuthorityServerWrapper) PerformValidation(
+	ctx context.Context,
+	request *rapb.PerformValidationRequest) (*corepb.Authorization, error) {
+	if request == nil || !authorizationValid(request.Authz) || request.ChallengeIndex == nil {
+		return nil, errIncompleteRequest
+	}
+	return ras.inner.PerformValidation(ctx, request)
+}
+
 func (ras *RegistrationAuthorityServerWrapper) RevokeCertificateWithReg(ctx context.Context, request *rapb.RevokeCertificateWithRegRequest) (*corepb.Empty, error) {
 	if request == nil || request.Cert == nil || request.Code == nil || request.RegID == nil {
 		return nil, errIncompleteRequest

ra/proto/ra.proto

@@ -10,7 +10,10 @@ service RegistrationAuthority {
         rpc NewAuthorization(NewAuthorizationRequest) returns (core.Authorization) {}
         rpc NewCertificate(NewCertificateRequest) returns (core.Certificate) {}
         rpc UpdateRegistration(UpdateRegistrationRequest) returns (core.Registration) {}
+        // TODO(@cpu): Remove UpdateAuthorization. It is deprecated in favour of
+        // PerformValidation. See https://github.com/letsencrypt/boulder/issues/3947
         rpc UpdateAuthorization(UpdateAuthorizationRequest) returns (core.Authorization) {}
+        rpc PerformValidation(PerformValidationRequest) returns (core.Authorization) {}
         rpc RevokeCertificateWithReg(RevokeCertificateWithRegRequest) returns (core.Empty) {}
         rpc DeactivateRegistration(core.Registration) returns (core.Empty) {}
         rpc DeactivateAuthorization(core.Authorization) returns (core.Empty) {}
@@ -40,6 +43,11 @@ message UpdateAuthorizationRequest {
         optional core.Challenge response = 3;
 }
 
+message PerformValidationRequest {
+        optional core.Authorization authz = 1;
+        optional int64 challengeIndex = 2;
+}
+
 message RevokeCertificateWithRegRequest {
         optional bytes cert = 1;
         optional int64 code = 2;

ra/ra.go

@@ -1409,23 +1409,53 @@ func mergeUpdate(r *core.Registration, input core.Registration) bool {
 	return changed
 }
 
-// UpdateAuthorization updates an authorization with new values.
+// UpdateAuthorization is a legacy function in the process of being replaced by
+// PerformValidation.
+// TODO(@cpu): Remove this. https://github.com/letsencrypt/boulder/issues/3947
 func (ra *RegistrationAuthorityImpl) UpdateAuthorization(
 	ctx context.Context,
 	base core.Authorization,
 	challengeIndex int,
-	response core.Challenge) (core.Authorization, error) {
+	_ core.Challenge) (core.Authorization, error) {
+	authzPB, err := bgrpc.AuthzToPB(base)
+	if err != nil {
+		return core.Authorization{}, err
+	}
+	challIndex := int64(challengeIndex)
+	authzPB, err = ra.PerformValidation(ctx, &rapb.PerformValidationRequest{
+		Authz:          authzPB,
+		ChallengeIndex: &challIndex,
+	})
+	if err != nil {
+		return core.Authorization{}, err
+	}
+	return bgrpc.PBToAuthz(authzPB)
+}
+
+// PerformValidation initiates validation for a specific challenge associated
+// with the given base authorization. The authorization and challenge are
+// updated based on the results.
+func (ra *RegistrationAuthorityImpl) PerformValidation(
+	ctx context.Context,
+	req *rapb.PerformValidationRequest) (*corepb.Authorization, error) {
+	base, err := bgrpc.PBToAuthz(req.Authz)
+	if err != nil {
+		return nil, err
+	}
+
 	// Refuse to update expired authorizations
 	if base.Expires == nil || base.Expires.Before(ra.clk.Now()) {
-		return core.Authorization{}, berrors.MalformedError("expired authorization")
+		return nil, berrors.MalformedError("expired authorization")
 	}
 
 	authz := base
-	if challengeIndex >= len(authz.Challenges) {
-		return core.Authorization{}, berrors.MalformedError("invalid challenge index '%d'", challengeIndex)
+	challIndex := int(*req.ChallengeIndex)
+	if challIndex >= len(authz.Challenges) {
+		return nil,
+			berrors.MalformedError("invalid challenge index '%d'", challIndex)
 	}
 
-	ch := &authz.Challenges[challengeIndex]
+	ch := &authz.Challenges[challIndex]
 
 	// If TLSSNIRevalidation is enabled, find out whether this was a revalidation
 	// (previous certificate existed) or not. If it is a revalidation, we can
@@ -1437,10 +1467,11 @@ func (ra *RegistrationAuthorityImpl) UpdateAuthorization(
 			RegID:  &authz.RegistrationID,
 		})
 		if err != nil {
-			return core.Authorization{}, err
+			return nil, err
 		}
 		if !*existsResp.Exists {
-			return core.Authorization{}, berrors.MalformedError("challenge type %q no longer allowed", ch.Type)
+			return nil,
+				berrors.MalformedError("challenge type %q no longer allowed", ch.Type)
 		}
 	}
 
@@ -1451,34 +1482,23 @@ func (ra *RegistrationAuthorityImpl) UpdateAuthorization(
 	// case and return early.
 	if ra.reuseValidAuthz && authz.Status == core.StatusValid {
 		ra.stats.Inc("ReusedValidAuthzChallenge", 1)
-		return authz, nil
+		return req.Authz, nil
 	}
 
 	if authz.Status != core.StatusPending {
-		return core.Authorization{}, berrors.WrongAuthorizationStateError("authorization must be pending")
+		return nil, berrors.WrongAuthorizationStateError("authorization must be pending")
 	}
 
 	// Look up the account key for this authorization
 	reg, err := ra.SA.GetRegistration(ctx, authz.RegistrationID)
 	if err != nil {
-		return core.Authorization{}, berrors.InternalServerError(err.Error())
+		return nil, berrors.InternalServerError(err.Error())
 	}
 
 	// Compute the key authorization field based on the registration key
 	expectedKeyAuthorization, err := ch.ExpectedKeyAuthorization(reg.Key)
 	if err != nil {
-		return core.Authorization{}, berrors.InternalServerError("could not compute expected key authorization value")
-	}
-
-	// NOTE(@cpu): Historically challenge update required the client to send
-	// a JSON POST body that included a computed KeyAuthorization. The RA would
-	// check this provided authorization against its own computation of the key
-	// authorization and err if they did not match. New ACME specification does
-	// not require this - the client does not need to send the key authorization.
-	// To support this for ACMEv2 we only enforce the provided key authorization
-	// matches expected if the update included it.
-	if response.ProvidedKeyAuthorization != "" && expectedKeyAuthorization != response.ProvidedKeyAuthorization {
-		return core.Authorization{}, berrors.MalformedError("provided key authorization was incorrect")
+		return nil, berrors.InternalServerError("could not compute expected key authorization value")
 	}
 
 	// Populate the ProvidedKeyAuthorization such that the VA can confirm the
@@ -1491,7 +1511,7 @@ func (ra *RegistrationAuthorityImpl) UpdateAuthorization(
 
 	// Double check before sending to VA
 	if cErr := ch.CheckConsistencyForValidation(); cErr != nil {
-		return core.Authorization{}, berrors.MalformedError(cErr.Error())
+		return nil, berrors.MalformedError(cErr.Error())
 	}
 
 	ra.stats.Inc("NewPendingAuthorizations", 1)
@@ -1506,7 +1526,7 @@ func (ra *RegistrationAuthorityImpl) UpdateAuthorization(
 		copy(challenges, authz.Challenges)
 		authz.Challenges = challenges
 
-		records, err := ra.VA.PerformValidation(vaCtx, authz.Identifier.Value, authz.Challenges[challengeIndex], authz)
+		records, err := ra.VA.PerformValidation(vaCtx, authz.Identifier.Value, authz.Challenges[challIndex], authz)
 		var prob *probs.ProblemDetails
 		if p, ok := err.(*probs.ProblemDetails); ok {
 			prob = p
@@ -1516,7 +1536,7 @@ func (ra *RegistrationAuthorityImpl) UpdateAuthorization(
 		}
 
 		// Save the updated records
-		challenge := &authz.Challenges[challengeIndex]
+		challenge := &authz.Challenges[challIndex]
 		challenge.ValidationRecord = records
 
 		if !challenge.RecordsSane() && prob == nil {
@@ -1529,7 +1549,7 @@ func (ra *RegistrationAuthorityImpl) UpdateAuthorization(
 		} else {
 			challenge.Status = core.StatusValid
 		}
-		authz.Challenges[challengeIndex] = *challenge
+		authz.Challenges[challIndex] = *challenge
 
 		err = ra.onValidationUpdate(vaCtx, authz)
 		if err != nil {
@@ -1538,7 +1558,7 @@ func (ra *RegistrationAuthorityImpl) UpdateAuthorization(
 		}
 	}(authz)
 	ra.stats.Inc("UpdatedPendingAuthorizations", 1)
-	return authz, nil
+	return bgrpc.AuthzToPB(authz)
 }
 
 func revokeEvent(state, serial, cn string, names []string, revocationCode revocation.Reason) string {