Add Authz revocation upon Cert revocation, by feature flag.

cmd/cert-checker/main.go

@@ -317,8 +317,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

@@ -73,6 +73,15 @@ type Config struct {
 	// to find and revoke accounts using keys which have been added to the
 	// blockedKeys table.
 	RevokeBadKeyAccounts 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

@@ -1682,6 +1682,36 @@ 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. The
+// logged line will include the affected row count from the gRPC response when
+// successful, or an error.
+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 {
+			// We expect a limit of 100 to be be rarely, if ever, reached. We
+			// can add re-fire logic if we see evidence otherwise.
+			response, err := ra.SA.RevokeAuthorizationsFor(ctx, &sapb.RevokeAuthorizationsForRequest{
+				RegistrationID: regId,
+				Identifier:     ident.ToProto(),
+				RevokeLimit:    100,
+			})
+			if err != nil {
+				ra.log.Errf("Authz revocation error encountered for identifier %q, held by regId %d: %v", ident, regId, err)
+			} else {
+				ra.log.Infof("Authz revocation succeeded with %d affected rows for identifier %q, held by regId %d", response.RevokedCount, ident, 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
@@ -1717,6 +1747,10 @@ func (ra *RegistrationAuthorityImpl) RevokeCertByApplicant(ctx context.Context,
 		Requester:    req.RegID,
 	}
 
+	// 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 :=`) in a
 	// nested scope. Doing so will create a new `err` variable that is not
 	// captured by this closure.
@@ -1769,13 +1803,28 @@ func (ra *RegistrationAuthorityImpl) RevokeCertByApplicant(ctx context.Context,
 		// domain names in the certificate". Override the reason code to match.
 		reasonCode = revocation.CessationOfOperation
 		logEvent.Reason = reasonCode
+
+		// 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
 	}
 
 	err = ra.revokeCertificate(ctx, cert, reasonCode)
 	if err != nil {
 		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

@@ -3658,6 +3658,65 @@ 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) (*sapb.RevokeAuthorizationsForResponse, error) {
+	msa.recv[req.Identifier.Value] = req.RegistrationID
+	// always return an affected rows count of 3
+	fauxResp := &sapb.RevokeAuthorizationsForResponse{}
+	fauxResp.RevokedCount = 3
+	return fauxResp, 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

@@ -504,7 +504,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)`,
@@ -513,7 +513,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
 	)