Add test for Authz revocation. Count "revoked" authz in cert-checker.

Author: ezekiel

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 {

ra/ra.go

@@ -1600,6 +1600,9 @@ func (ra *RegistrationAuthorityImpl) updateRevocationForKeyCompromise(ctx contex
 }
 
 func (ra *RegistrationAuthorityImpl) revokeAuthorizations(ctx context.Context, cert *x509.Certificate, smeta *sapb.SerialMetadata) {
+	ctx, cancel := context.WithTimeout(context.WithoutCancel(ctx), 5*time.Second)
+	defer cancel()
+
 	if features.Get().RevokeAuthzsUponRevokeCert {
 		idents := identifier.FromCert(cert)
 		for _, ident := range idents {
@@ -1608,7 +1611,9 @@ func (ra *RegistrationAuthorityImpl) revokeAuthorizations(ctx context.Context, c
 				Identifier:     ident.ToProto(),
 			})
 			if err != nil {
-				ra.log.Infof("Authz revocation failed: %s", err)
+				ra.log.Error(ctx, "Authz revocation failed", err, blog.Idents(ident), blog.Acct(smeta.RegistrationID))
+			} else {
+				ra.log.Info(ctx, "Authz revocation succeeded", blog.Idents(ident), blog.Acct(smeta.RegistrationID))
 			}
 		}
 	}

ra/ra_test.go

@@ -3681,6 +3681,7 @@ func TestRevokeAuthorizations_FeatureDisabled(t *testing.T) {
 	// 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()

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
 	)

test/integration/revocation_test.go

@@ -17,6 +17,7 @@ import (
 	"os"
 	"os/exec"
 	"path"
+	"slices"
 	"strings"
 	"sync"
 	"testing"
@@ -699,3 +700,91 @@ func TestBadKeyRevokerByAccount(t *testing.T) {
 		checkUnrevoked(t, allCRLs, bundle.certs[0])
 	}
 }
+
+// waitAndCheckRevoked uses an acme client to poll some(a slice of)
+// authorizations for a desired status. It is willing to repeatedly fetch the
+// authorizations up to four times, and wait up to 5 seconds, before reporting
+// failure.
+func waitAndCheckAuthzStatus(t *testing.T, aClient *client, authzs []string, wantStatus core.AcmeStatus) {
+	t.Helper()
+
+	for try := range 4 {
+		time.Sleep(core.RetryBackoff(try, time.Second, 2*time.Second, 1.5))
+
+		var allStatus []string
+
+		for authz := range authzs {
+			respAuth, err := aClient.FetchAuthorization(aClient.Account, authzs[authz])
+			t.Logf("Authorization fetched: %v", respAuth)
+			test.AssertNotError(t, err, "FetchAuthorization Failed")
+
+			allStatus = append(allStatus, respAuth.Status)
+		}
+
+		slices.Sort(allStatus)
+		uniqStatus := slices.Compact(allStatus)
+
+		if len(uniqStatus) == 1 && uniqStatus[0] == string(wantStatus) {
+			// Success, all authorizations match our desired result
+			return
+		}
+	}
+
+	t.Errorf("exhausted authz polling attempts, status values still not as desired")
+}
+
+func TestRevokeAuthzUponRevokeCert(t *testing.T) {
+	if !strings.Contains(os.Getenv("BOULDER_CONFIG_DIR"), "test/config-next") {
+		t.Skip("RevokeAuthzUponRevokeCert is only configured in config-next")
+	}
+
+	// Two acme clients, both alike in dignity
+	clientRed, err := makeClient("mailto:example@letsencrypt.org")
+	test.AssertNotError(t, err, "creating acme client")
+	clientBlue, err := makeClient("mailto:example@letsencrypt.org")
+	test.AssertNotError(t, err, "creating acme client")
+
+	// With different keys
+	certKeyRed, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	test.AssertNotError(t, err, "failed to generate cert key")
+	certKeyBlue, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	test.AssertNotError(t, err, "failed to generate cert key")
+
+	// Share one set of domains for BOTH client certs
+	redVsBlueIdents := []acme.Identifier{
+		{Type: "dns", Value: random_domain()},
+		{Type: "dns", Value: random_domain()},
+		{Type: "dns", Value: random_domain()},
+	}
+
+	// Both clients issue certs for the shared-control set of domains
+	res, err := authAndIssue(clientRed, certKeyRed, redVsBlueIdents, true, "")
+	test.AssertNotError(t, err, "authAndIssue failed")
+	certRed := res.certs[0]
+	authzRed := res.Order.Authorizations
+
+	res, err = authAndIssue(clientBlue, certKeyBlue, redVsBlueIdents, true, "")
+	test.AssertNotError(t, err, "authAndIssue failed")
+	certBlue := res.certs[0]
+	authzBlue := res.Order.Authorizations
+
+	// Red client revokes the Blue client cert with reason "Unspecified"
+	err = clientRed.RevokeCertificate(clientRed.Account, certBlue, clientRed.PrivateKey, int(revocation.Unspecified))
+	test.AssertNotError(t, err, "failed to revoke certificate")
+
+	// Authorizations for shared-control domains held by Blue client should be revoked
+	t.Logf("Blue cert revoked by Red client, poll authz for Idents: %v", redVsBlueIdents)
+	waitAndCheckAuthzStatus(t, clientBlue, authzBlue, core.StatusRevoked)
+
+	// Red client revokes its own certificate with reason "Unspecified"
+	err = clientRed.RevokeCertificate(clientRed.Account, certRed, clientRed.PrivateKey, int(revocation.Unspecified))
+	test.AssertNotError(t, err, "failed to revoke certificate")
+
+	// Authorizations for single-client domains should not be revoked
+	t.Logf("Red cert revoked by Red client, poll authz for Idents: %v", redVsBlueIdents)
+	// re-using waitAndCheckAuthzStatus() here is basically only telling us that
+	// the authorizations did not _immediately_ change from valid to something
+	// else. Another function might exhaust more wall clock time to test beyond
+	// the context timeout.
+	waitAndCheckAuthzStatus(t, clientRed, authzRed, core.StatusValid)
+}