From 6a634a4bd250ffe685ec24c3e602aa43c8b91b87 Mon Sep 17 00:00:00 2001 From: Aaron Gable Date: Fri, 19 Jul 2024 09:53:16 -0700 Subject: [PATCH] Remove CAA-rechecking dead code (#7606) 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. --- ra/ra.go | 46 ++++++++++++---------------------------------- ra/ra_test.go | 4 ---- 2 files changed, 12 insertions(+), 38 deletions(-) diff --git a/ra/ra.go b/ra/ra.go index 60fdd4349..106c5e169 100644 --- a/ra/ra.go +++ b/ra/ra.go @@ -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) @@ -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", @@ -281,7 +274,6 @@ func NewRegistrationAuthorityImpl( recheckCAACounter: recheckCAACounter, newCertCounter: newCertCounter, revocationReasonCounter: revocationReasonCounter, - recheckCAAUsedAuthzLifetime: recheckCAAUsedAuthzLifetime, authzAges: authzAges, orderAges: orderAges, inflightFinalizes: inflightFinalizes, @@ -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 { @@ -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) } } diff --git a/ra/ra_test.go b/ra/ra_test.go index d056bb676..3bf241c2b 100644 --- a/ra/ra_test.go +++ b/ra/ra_test.go @@ -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 {