Use new RA methods from WFE revocation path

akamai/cache-client.go

@@ -273,7 +273,7 @@ func (cpc *CachePurgeClient) authedRequest(endpoint string, body v3PurgeRequest)
 	}
 
 	cpc.log.AuditInfof("Purge request sent successfully (ID %s) (body %s). Purge expected in %ds",
-		purgeInfo.PurgeID, purgeInfo.EstimatedSeconds, reqBody)
+		purgeInfo.PurgeID, reqBody, purgeInfo.EstimatedSeconds)
 	return nil
 }
 

errors/errors.go

@@ -146,5 +146,5 @@ func AlreadyRevokedError(msg string, args ...interface{}) error {
 }
 
 func BadRevocationReasonError(reason int64) error {
-	return New(AlreadyRevoked, "disallowed revocation reason: %d", reason)
+	return New(BadRevocationReason, "disallowed revocation reason: %d", reason)
 }

features/features.go

@@ -59,6 +59,11 @@ const (
 	GetAuthzUseIndex
 	// Check the failed authorization limit before doing authz reuse.
 	CheckFailedAuthorizationsFirst
+	// AllowReRevocation causes the RA to allow the revocation reason of an
+	// already-revoked certificate to be updated to `keyCompromise` from any
+	// other reason if that compromise is demonstrated by making the second
+	// revocation request signed by the certificate keypair.
+	AllowReRevocation
 	// MozRevocationReasons causes the RA to enforce the following upcoming
 	// Mozilla policies regarding revocation:
 	// - A subscriber can request that their certificate be revoked with reason
@@ -97,6 +102,7 @@ var features = map[FeatureFlag]bool{
 	GetAuthzReadOnly:               false,
 	GetAuthzUseIndex:               false,
 	CheckFailedAuthorizationsFirst: false,
+	AllowReRevocation:              false,
 	MozRevocationReasons:           false,
 }
 

ra/ra.go

@@ -2111,7 +2111,17 @@ func (ra *RegistrationAuthorityImpl) RevokeCertByKey(ctx context.Context, req *r
 	// to the blocked keys list is a worse failure than failing to revoke in the
 	// first place, because it means that bad-key-revoker won't revoke the cert
 	// anyway.
-	if reason == ocsp.KeyCompromise {
+	var shouldBlock bool
+	if features.Enabled(features.AllowReRevocation) {
+		// If we're allowing re-revocation, then block the key for all keyCompromise
+		// requests, no matter whether the revocation itself succeeded or failed.
+		shouldBlock = reason == ocsp.KeyCompromise
+	} else {
+		// Otherwise, only block the key if the revocation above succeeded, or
+		// failed for a reason other than "already revoked".
+		shouldBlock = (reason == ocsp.KeyCompromise && !errors.Is(revokeErr, berrors.AlreadyRevoked))
+	}
+	if shouldBlock {
 		var digest core.Sha256Digest
 		digest, err = core.KeyDigest(cert.PublicKey)
 		if err != nil {
@@ -2132,7 +2142,12 @@ func (ra *RegistrationAuthorityImpl) RevokeCertByKey(ctx context.Context, req *r
 	// than keyCompromise.
 	err = revokeErr
 	if err != nil {
-		if !errors.Is(err, berrors.AlreadyRevoked) || reason != ocsp.KeyCompromise {
+		// Immediately error out, rather than trying re-revocation, if the error was
+		// anything other than AlreadyRevoked, if the requested reason is anything
+		// other than keyCompromise, or if we're not yet using the new logic.
+		if !errors.Is(err, berrors.AlreadyRevoked) ||
+			reason != ocsp.KeyCompromise ||
+			!features.Enabled(features.AllowReRevocation) {
 			return nil, err
 		}
 		err = ra.updateRevocationForKeyCompromise(ctx, cert.SerialNumber, int64(issuerID))

ra/ra_test.go

@@ -3872,6 +3872,19 @@ func TestRevokeCertByKey(t *testing.T) {
 	test.AssertEquals(t, len(mockSA.blocked), 0)
 	test.AssertEquals(t, mockSA.revoked[core.SerialToString(cert.SerialNumber)], int64(ocsp.Unspecified))
 
+	// Re-revoking for any reason should fail, because it isn't enabled.
+	_, err = ra.RevokeCertByKey(context.Background(), &rapb.RevokeCertByKeyRequest{
+		Cert: cert.Raw,
+		Code: ocsp.KeyCompromise,
+	})
+	test.AssertError(t, err, "should have failed")
+
+	// Enable re-revocation.
+	_ = features.Set(map[string]bool{
+		features.MozRevocationReasons.String(): false,
+		features.AllowReRevocation.String():    true,
+	})
+
 	// Re-revoking for the same reason should fail.
 	_, err = ra.RevokeCertByKey(context.Background(), &rapb.RevokeCertByKeyRequest{
 		Cert: cert.Raw,
@@ -3950,13 +3963,26 @@ func TestRevokeCertByKey_Moz(t *testing.T) {
 	test.AssertEquals(t, len(mockSA.blocked[0].Comment), 0)
 	test.AssertEquals(t, mockSA.revoked[core.SerialToString(cert.SerialNumber)], int64(ocsp.KeyCompromise))
 
+	// Re-revoking should fail, because re-revocation is not allowed.
+	_, err = ra.RevokeCertByKey(context.Background(), &rapb.RevokeCertByKeyRequest{
+		Cert: cert.Raw,
+	})
+	test.AssertError(t, err, "should have failed")
+
+	// Enable re-revocation.
+	_ = features.Set(map[string]bool{
+		features.MozRevocationReasons.String(): true,
+		features.AllowReRevocation.String():    true,
+	})
+
 	// Re-revoking should fail, because it is already revoked for keyCompromise.
 	_, err = ra.RevokeCertByKey(context.Background(), &rapb.RevokeCertByKeyRequest{
 		Cert: cert.Raw,
 	})
 	test.AssertError(t, err, "should have failed")
 
-	// Reset, revoke for some other reason, and try again.
+	// Reset and have the Subscriber revoke for a different reason.
+	// Then re-revoking using the key should work.
 	mockSA.revoked = make(map[string]int64)
 	_, err = ra.RevokeCertByApplicant(context.Background(), &rapb.RevokeCertByApplicantRequest{
 		Cert:  cert.Raw,
@@ -3968,10 +3994,6 @@ func TestRevokeCertByKey_Moz(t *testing.T) {
 		Cert: cert.Raw,
 	})
 	test.AssertNotError(t, err, "should have succeeded")
-	_, err = ra.RevokeCertByKey(context.Background(), &rapb.RevokeCertByKeyRequest{
-		Cert: cert.Raw,
-	})
-	test.AssertError(t, err, "should have failed")
 }
 
 func TestAdministrativelyRevokeCertificate(t *testing.T) {

test/config-next/ra.json

@@ -58,6 +58,7 @@
       "StoreRevokerInfo": true,
       "RestrictRSAKeySizes": true,
       "StreamlineOrderAndAuthzs": true,
+      "AllowReRevocation": true,
       "MozRevocationReasons": true
     },
     "CTLogGroups2": [

test/helpers.py

@@ -96,7 +96,7 @@ def ocsp_verify(cert_file, issuer_file, ocsp_response):
         raise(Exception("OCSP verify failure"))
     return output
 
-def verify_ocsp(cert_file, issuer_file, url, status="revoked"):
+def verify_ocsp(cert_file, issuer_file, url, status="revoked", reason=None):
     ocsp_request = make_ocsp_req(cert_file, issuer_file)
     responses = fetch_ocsp(ocsp_request, url)
 
@@ -114,6 +114,10 @@ def verify_ocsp(cert_file, issuer_file, url, status="revoked"):
         if not re.search("%s: %s" % (cert_file, status), verify_output):
             print(verify_output)
             raise(Exception("OCSP response wasn't '%s'" % status))
+    if reason is not None:
+        if not re.search("Reason: %s" % reason, verify_output):
+            print(verify_output)
+            raise(Exception("OCSP response wasn't '%s'" % reason))
     return verify_output
 
 def reset_akamai_purges():