Skip to content
4 changes: 2 additions & 2 deletions cmd/cert-checker/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
9 changes: 9 additions & 0 deletions features/features.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
38 changes: 38 additions & 0 deletions ra/ra.go
Original file line number Diff line number Diff line change
Expand Up @@ -1599,6 +1599,26 @@ func (ra *RegistrationAuthorityImpl) updateRevocationForKeyCompromise(ctx contex
return nil
}

func (ra *RegistrationAuthorityImpl) revokeAuthorizations(ctx context.Context, cert *x509.Certificate, smeta *sapb.SerialMetadata) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only thing this function uses smeta for is the regID, so just pass the bare regID instead.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a doc comment describing this function and the fact that its calling contract requires that it be called as a background goroutine (otherwise the context timeout change will cause problems).

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.RevokeAuthorizationFor(ctx, &sapb.RevokeAuthorizationForRequest{
RegistrationID: smeta.RegistrationID,
Identifier: ident.ToProto(),
})
if err != nil {
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))
}
}
}
}

// 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
Expand Down Expand Up @@ -1627,6 +1647,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.
Expand Down Expand Up @@ -1679,6 +1703,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)))
Expand All @@ -1687,6 +1719,12 @@ func (ra *RegistrationAuthorityImpl) RevokeCertByApplicant(ctx context.Context,
return nil, err
}

// Asynchronously request to revoke authorizations held by the RegID from
// cert metadata, confirmed above to be different than requester ID.
if requestAuthzRevocation {
go ra.revokeAuthorizations(ctx, cert, metadata)
}

return &emptypb.Empty{}, nil
}

Expand Down
56 changes: 56 additions & 0 deletions ra/ra_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 RevokeAuthorizationFor
// requests.
type mockSARecordAuthzRevocation struct {
sapb.StorageAuthorityClient
recv map[string]int64
}

func (msa *mockSARecordAuthzRevocation) RevokeAuthorizationFor(ctx context.Context, req *sapb.RevokeAuthorizationForRequest, _ ...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)
// 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)
// 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()
Expand Down
4 changes: 2 additions & 2 deletions sa/model.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)`,
Expand All @@ -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
)
Expand Down
Loading
Loading