Add Authz revocation upon Cert revocation, by feature flag.

cmd/cert-checker/main.go

@@ -322,8 +322,8 @@ var expectedExtensionContent = map[string][]byte{
 
 // checkValidations checks the database for matching authorizations that were
 // likely valid at the time the certificate was issued. Authorizations with
-// status = "deactivated" are counted for this, so long as their validatedAt
-// is before the issuance and expiration is after.
+// status = "deactivated" and "revoked" are counted for this, so long as their
+// validatedAt is before the issuance and expiration is after.
 func (c *certChecker) checkValidations(ctx context.Context, cert *corepb.Certificate, idents identifier.ACMEIdentifiers) error {
 	authzs, err := sa.SelectAuthzsMatchingIssuance(ctx, c.dbMap, cert.RegistrationID, cert.Issued.AsTime(), idents)
 	if err != nil {

features/features.go

@@ -68,6 +68,15 @@ type Config struct {
 	// during certificate issuance. This flag must be set to true in the
 	// RA and VA services for full functionality.
 	DNSPersist01Enabled bool
+
+	// RevokeAuthzsUponRevokeCert controls whether the RA will call for
+	// revocation of Authorizations for identifiers in a certificate that is
+	// successfully revoked by a requester that is DIFFERENT than the one that
+	// was originally granted the certificate. In this scenario, the new
+	// requester has demonstrated control over the requisite set of identifiers,
+	// so we can avoid the possibility of Authz re-use by the original
+	// requester via Authz revocation.
+	RevokeAuthzsUponRevokeCert bool
 }
 
 var fMu = new(sync.RWMutex)

ra/ra.go

@@ -1599,6 +1599,32 @@ func (ra *RegistrationAuthorityImpl) updateRevocationForKeyCompromise(ctx contex
 	return nil
 }
 
+// revokeAuthorizations must be called as a background goroutine as it uses a
+// custom context timeout and is not cancelled by its parent. It sends off an
+// asynchronous request to the SA to revoke authorizations for all Identifiers
+// from the provided cert which are held by the provided RegistrationID. It will
+// log each Identifier and RegistrationID pair attempted against the SA, and
+// will include the gRPC error in the logs, if encountered.
+func (ra *RegistrationAuthorityImpl) revokeAuthorizations(ctx context.Context, cert *x509.Certificate, regId int64) {
+	ctx, cancel := context.WithTimeout(context.WithoutCancel(ctx), 5*time.Second)
+	defer cancel()
+
+	if features.Get().RevokeAuthzsUponRevokeCert {
+		idents := identifier.FromCert(cert)
+		for _, ident := range idents {
+			_, err := ra.SA.RevokeAuthorizationsFor(ctx, &sapb.RevokeAuthorizationsForRequest{
+				RegistrationID: regId,
+				Identifier:     ident.ToProto(),
+			})
+			if err != nil {
+				ra.log.Error(ctx, "Authz revocation failed", err, blog.Idents(ident), blog.Acct(regId))
+			} else {
+				ra.log.Info(ctx, "Authz revocation succeeded", blog.Idents(ident), blog.Acct(regId))
+			}
+		}
+	}
+}
+
 // RevokeCertByApplicant revokes the certificate in question. It allows any
 // revocation reason from (0, 1, 3, 4, 5, 9), because Subscribers are allowed to
 // request any revocation reason for their own certificates. However, if the
@@ -1627,6 +1653,10 @@ func (ra *RegistrationAuthorityImpl) RevokeCertByApplicant(ctx context.Context,
 	serialString := core.SerialToString(cert.SerialNumber)
 	ctx = blog.ContextWith(ctx, blog.Acct(req.RegID), blog.Serial(serialString))
 
+	// By default, do not revoke Authorizations held for the revoked-cert
+	// identifiers.
+	requestAuthzRevocation := false
+
 	// Below this point, do not re-declare `err` (i.e. type `err :=`) or `ctx` in
 	// a nested scope. Doing so will create a new variable that is not captured by
 	// this closure.
@@ -1679,6 +1709,14 @@ func (ra *RegistrationAuthorityImpl) RevokeCertByApplicant(ctx context.Context,
 		// circumstances where "the certificate subscriber no longer owns the
 		// domain names in the certificate". Override the reason code to match.
 		reasonCode = revocation.CessationOfOperation
+
+		// We have confirmed that the requester RegistrationID is NOT the same
+		// as the original subscriber. Requester has demonstrated control over
+		// the set of identifiers sufficient for certificate revocation. Given
+		// BOTH, enable this boolean to signal that authorizations held by the
+		// original subscriber RegID should be revoked after certificate
+		// revocation.
+		requestAuthzRevocation = true
 	}
 
 	ctx = blog.ContextWith(ctx, slog.Int64("reason", int64(reasonCode)))
@@ -1687,6 +1725,13 @@ func (ra *RegistrationAuthorityImpl) RevokeCertByApplicant(ctx context.Context,
 		return nil, err
 	}
 
+	// Asynchronously request to revoke authorizations for identifiers from this
+	// revoked certificate which are held by the RegID from cert metadata,
+	// confirmed above to be different than requester ID.
+	if requestAuthzRevocation {
+		go ra.revokeAuthorizations(ctx, cert, metadata.RegistrationID)
+	}
+
 	return &emptypb.Empty{}, nil
 }
 

ra/ra_test.go

@@ -746,7 +746,7 @@ func TestPerformValidation_FailedThenSuccessfulValidationResetsPauseIdentifiersR
 	})
 	test.AssertNotError(t, err, "PerformValidation failed")
 
-	// Wait for the RA to finish processesing the validation, and ensure that
+	// Wait for the RA to finish processing the validation, and ensure that
 	// the reset bucket key is what we expect.
 	reset := <-keyChan
 	test.AssertEquals(t, reset, bucketKey)
@@ -3649,6 +3649,62 @@ func TestRevokeCertByApplicant_Controller(t *testing.T) {
 	test.AssertEquals(t, mockSA.revoked[core.SerialToString(cert.SerialNumber)].RevokedReason, int64(revocation.CessationOfOperation))
 }
 
+// mockSARecordAuthzRevocation is a mock sapb.StorageAuthorityClient that simply
+// maps identifier strings to RegistrationIDs for received RevokeAuthorizationsFor
+// requests.
+type mockSARecordAuthzRevocation struct {
+	sapb.StorageAuthorityClient
+	recv map[string]int64
+}
+
+func (msa *mockSARecordAuthzRevocation) RevokeAuthorizationsFor(ctx context.Context, req *sapb.RevokeAuthorizationsForRequest, _ ...grpc.CallOption) (*emptypb.Empty, error) {
+	msa.recv[req.Identifier.Value] = req.RegistrationID
+	return &emptypb.Empty{}, nil
+}
+
+func TestRevokeAuthorizations_FeatureDisabled(t *testing.T) {
+	_, _, ra, _, clk, _, cleanUp := initAuthorities(t)
+	defer cleanUp()
+
+	features.Set(features.Config{RevokeAuthzsUponRevokeCert: false})
+	defer features.Reset()
+
+	mockSA := mockSARecordAuthzRevocation{}
+	mockSA.recv = make(map[string]int64)
+	ra.SA = &mockSA
+
+	_, cert := test.ThrowAwayCert(t, clk)
+
+	meta := &sapb.SerialMetadata{RegistrationID: 333}
+
+	ra.revokeAuthorizations(context.Background(), cert, meta.RegistrationID)
+	// mockSA should not have received ANY requests
+	test.AssertEquals(t, len(mockSA.recv), 0)
+}
+
+func TestRevokeAuthorizations_FeatureEnabled(t *testing.T) {
+	_, _, ra, _, clk, _, cleanUp := initAuthorities(t)
+	defer cleanUp()
+
+	features.Set(features.Config{RevokeAuthzsUponRevokeCert: true})
+	defer features.Reset()
+
+	mockSA := mockSARecordAuthzRevocation{}
+	mockSA.recv = make(map[string]int64)
+	ra.SA = &mockSA
+
+	_, cert := test.ThrowAwayCert(t, clk)
+	idents := identifier.FromCert(cert)
+
+	meta := &sapb.SerialMetadata{RegistrationID: 333}
+
+	ra.revokeAuthorizations(context.Background(), cert, meta.RegistrationID)
+	// mockSA should have received requests for each of the certificate identifiers
+	for _, ident := range idents {
+		test.AssertEquals(t, mockSA.recv[ident.Value], meta.RegistrationID)
+	}
+}
+
 func TestRevokeCertByKey(t *testing.T) {
 	_, _, ra, _, clk, _, cleanUp := initAuthorities(t)
 	defer cleanUp()

sa/model.go

@@ -541,7 +541,7 @@ func SelectAuthzsMatchingIssuance(
 	identConditions, identArgs := buildIdentifierQueryConditions(idents)
 	query := fmt.Sprintf(`SELECT %s FROM authz2 WHERE
 			registrationID = ? AND
-			status IN (?, ?) AND
+			status IN (?, ?, ?) AND
 			expires >= ? AND
 			attemptedAt <= ? AND
 			(%s)`,
@@ -550,7 +550,7 @@ func SelectAuthzsMatchingIssuance(
 	var args []any
 	args = append(args,
 		regID,
-		statusToUint[core.StatusValid], statusToUint[core.StatusDeactivated],
+		statusToUint[core.StatusValid], statusToUint[core.StatusDeactivated], statusToUint[core.StatusRevoked],
 		issued.Add(-1*time.Second), // leeway for clock skew
 		issued.Add(1*time.Second),  // leeway for clock skew
 	)