Skip to content

Commit

Permalink
Remove CAA-rechecking dead code (#7606)
Browse files Browse the repository at this point in the history
This code path was a safety net to ensure that CAA got rechecked if the
authorization was going to expire less than 30d+7h from now, i.e. if the
authorization had originally been checked more than 7h ago. The metrics
show that, as expected, this code path has not been executed in living
memory, because all situations in which it might be hit instead hit the
preceding `if staleCAA` clause.
  • Loading branch information
aarongable authored Jul 19, 2024
1 parent a3e9943 commit 6a634a4
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 38 deletions.
46 changes: 12 additions & 34 deletions ra/ra.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,19 +112,18 @@ type RegistrationAuthorityImpl struct {

ctpolicy *ctpolicy.CTPolicy

ctpolicyResults *prometheus.HistogramVec
revocationReasonCounter *prometheus.CounterVec
namesPerCert *prometheus.HistogramVec
rlCheckLatency *prometheus.HistogramVec
rlOverrideUsageGauge *prometheus.GaugeVec
newRegCounter prometheus.Counter
recheckCAACounter prometheus.Counter
newCertCounter *prometheus.CounterVec
recheckCAAUsedAuthzLifetime prometheus.Counter
authzAges *prometheus.HistogramVec
orderAges *prometheus.HistogramVec
inflightFinalizes prometheus.Gauge
certCSRMismatch prometheus.Counter
ctpolicyResults *prometheus.HistogramVec
revocationReasonCounter *prometheus.CounterVec
namesPerCert *prometheus.HistogramVec
rlCheckLatency *prometheus.HistogramVec
rlOverrideUsageGauge *prometheus.GaugeVec
newRegCounter prometheus.Counter
recheckCAACounter prometheus.Counter
newCertCounter *prometheus.CounterVec
authzAges *prometheus.HistogramVec
orderAges *prometheus.HistogramVec
inflightFinalizes prometheus.Gauge
certCSRMismatch prometheus.Counter
}

var _ rapb.RegistrationAuthorityServer = (*RegistrationAuthorityImpl)(nil)
Expand Down Expand Up @@ -196,12 +195,6 @@ func NewRegistrationAuthorityImpl(
})
stats.MustRegister(recheckCAACounter)

recheckCAAUsedAuthzLifetime := prometheus.NewCounter(prometheus.CounterOpts{
Name: "recheck_caa_used_authz_lifetime",
Help: "A counter times the old codepath was used for CAA recheck time",
})
stats.MustRegister(recheckCAAUsedAuthzLifetime)

newCertCounter := prometheus.NewCounterVec(prometheus.CounterOpts{
Name: "new_certificates",
Help: "A counter of new certificates including the certificate profile name and hexadecimal certificate profile hash",
Expand Down Expand Up @@ -281,7 +274,6 @@ func NewRegistrationAuthorityImpl(
recheckCAACounter: recheckCAACounter,
newCertCounter: newCertCounter,
revocationReasonCounter: revocationReasonCounter,
recheckCAAUsedAuthzLifetime: recheckCAAUsedAuthzLifetime,
authzAges: authzAges,
orderAges: orderAges,
inflightFinalizes: inflightFinalizes,
Expand Down Expand Up @@ -863,13 +855,6 @@ func (ra *RegistrationAuthorityImpl) checkAuthorizationsCAA(
// Set the recheck time to 7 hours ago.
caaRecheckAfter := now.Add(caaRecheckDuration)

// Set a CAA recheck time based on the assumption of a 30 day authz
// lifetime. This has been deprecated in favor of a new check based
// off the Validated time stored in the database, but we want to check
// both for a time and increment a stat if this code path is hit for
// compliance safety.
caaRecheckTime := now.Add(ra.authorizationLifetime).Add(caaRecheckDuration)

for _, name := range names {
authz := authzs[name]
if authz == nil {
Expand All @@ -883,13 +868,6 @@ func (ra *RegistrationAuthorityImpl) checkAuthorizationsCAA(
} else if staleCAA {
// Ensure that CAA is rechecked for this name
recheckAuthzs = append(recheckAuthzs, authz)
} else if authz.Expires.Before(caaRecheckTime) {
// Ensure that CAA is rechecked for this name
recheckAuthzs = append(recheckAuthzs, authz)
// This codepath should not be used, but is here as a safety
// net until the new codepath is proven. Increment metric if
// it is used.
ra.recheckCAAUsedAuthzLifetime.Add(1)
}
}

Expand Down
4 changes: 0 additions & 4 deletions ra/ra_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1896,10 +1896,6 @@ func TestRecheckCAADates(t *testing.T) {
err = ra.checkAuthorizationsCAA(context.Background(), Registration.Id, []string{"novalidationtime.com"}, authzs, fc.Now())
test.AssertEquals(t, err.Error(), "authorization's challenge has no validated timestamp for: id noval")

// Test to make sure the authorization lifetime codepath was not used
// to determine if CAA needed recheck.
test.AssertMetricWithLabelsEquals(t, ra.recheckCAAUsedAuthzLifetime, prometheus.Labels{}, 0)

// We expect that "recent.com" is not checked because its mock authorization
// isn't expired
if _, present := recorder.names["recent.com"]; present {
Expand Down

0 comments on commit 6a634a4

Please sign in to comment.