Repository: snowflakedb/gosnowflake
Pull request #1472 (2025-07-21 09:44:30)

SNOW-2117152 Add CRL cache cleanup
### Description

SNOW-2117152 Adds the mechanism to cleanup CRL caches.

### Checklist
- [x] Created tests which fail without the change (if possible)
- [x] Extended the README / documentation, if necessary


codecov[bot]: ### :x: 4 Tests Failed:
| Tests completed | Failed | Passed | Skipped |
|---|---|---|---|
| 2548 | 4 | 2544 | 74 |
<details><summary>View the top 3 failed test(s) by shortest run time</summary>

> <pre><code class="language-python">github.com/snowflakedb/gosnowflake::TestCrlModes/checkMode=CERT_REVOCATION_CHECK_ENABLED/CacheTests/should_clean_up_cache</code></pre>
> <details><summary>Stack Traces | 1.07s run time</summary>
> 
> > <pre><code class="language-python">Failed</code></pre>
> 
> </details>


> <pre><code class="language-python">github.com/snowflakedb/gosnowflake::TestCrlModes/checkMode=CERT_REVOCATION_CHECK_ENABLED/CacheTests</code></pre>
> <details><summary>Stack Traces | 4.51s run time</summary>
> 
> > <pre><code class="language-python">Failed</code></pre>
> 
> </details>


> <pre><code class="language-python">github.com/snowflakedb/gosnowflake::TestCrlModes/checkMode=CERT_REVOCATION_CHECK_ENABLED</code></pre>
> <details><summary>Stack Traces | 13.4s run time</summary>
> 
> > <pre><code class="language-python">Failed</code></pre>
> 
> </details>

</details>

To view more test analytics, go to the [Test Analytics Dashboard](https://app.codecov.io/gh/snowflakedb/gosnowflake/tests/SNOW-2117152-crl-cache-cleanup)
<sub>📋 Got 3 mins? [Take this short survey](https://forms.gle/BpocVj23nhr2Y45G7) to help us improve Test Analytics.</sub>

Patch:
diff --git a/crl.go b/crl.go
index 21177ef..3f11804 100644
--- a/crl.go
+++ b/crl.go
@@ -32,8 +32,11 @@ type crlValidator struct {
 	inMemoryCacheMutex             sync.Mutex
 	onDiskCacheDisabled            bool
 	onDiskCacheDir                 string
+	onDiskCacheRemovalDelay        time.Duration
 	crlURLMutexes                  map[string]*sync.Mutex
 	httpClient                     *http.Client
+	cleanupStopChan                chan struct{}
+	cleanupDoneChan                chan struct{}
 }
 
 type crlInMemoryCacheValueType struct {
@@ -54,8 +57,11 @@ func newCrlValidator(certRevocationCheckMode CertRevocationCheckMode, allowCerti
 		inMemoryCache:                  inMemoryCache,
 		onDiskCacheDisabled:            onDiskCacheDisabled,
 		onDiskCacheDir:                 onDiskCacheDir,
+		onDiskCacheRemovalDelay:        7 * 24 * time.Hour, // 7 days
 		crlURLMutexes:                  make(map[string]*sync.Mutex),
 		httpClient:                     httpClient,
+		cleanupStopChan:                make(chan struct{}),
+		cleanupDoneChan:                make(chan struct{}),
 	}
 }
 
@@ -421,3 +427,84 @@ func isShortLivedCertificate(cert *x509.Certificate) bool {
 	certValidityPeriod := cert.NotAfter.Sub(cert.NotBefore)
 	return maximumValidityPeriod > certValidityPeriod
 }
+
+func (cv *crlValidator) startPeriodicCacheCleanup(tickRate time.Duration) {
+	logger.Debugf("starting periodic CRL cache cleanup with tick rate %v", tickRate)
+	cv.cleanupStopChan = make(chan struct{})
+	cv.cleanupDoneChan = make(chan struct{})
+	go func() {
+		ticker := time.NewTicker(tickRate)
+		defer ticker.Stop()
+		for {
+			select {
+			case <-ticker.C:
+				cv.cleanupInMemoryCache()
+				cv.cleanupOnDiskCache()
+			case <-cv.cleanupStopChan:
+				close(cv.cleanupDoneChan)
+				return
+			}
+		}
+	}()
+}
+
+func (cv *crlValidator) stopPeriodicCacheCleanup() {
+	logger.Debug("stopping periodic CRL cache cleanup")
+	if cv.cleanupStopChan != nil {
+		close(cv.cleanupStopChan)
+		<-cv.cleanupDoneChan
+	}
+}
+
+func (cv *crlValidator) cleanupInMemoryCache() {
+	if cv.inMemoryCacheDisabled {
+		return
+	}
+	now := time.Now()
+	logger.Debugf("cleaning up in-memory CRL cache at %v", now)
+	cv.inMemoryCacheMutex.Lock()
+	for k, v := range cv.inMemoryCache {
+		expired := v.crl.NextUpdate.Before(now)
+		evicted := v.downloadTime.Add(cv.cacheValidityTime).Before(now)
+		logger.Debugf("testing CRL for %v (nextUpdate=%v, downloadTime=%v) from in-memory cache (expired: %v, evicted: %v)", k, v.crl.NextUpdate, v.downloadTime, expired, evicted)
+		if expired || evicted {
+			delete(cv.inMemoryCache, k)
+		}
+	}
+	cv.inMemoryCacheMutex.Unlock()
+}
+
+func (cv *crlValidator) cleanupOnDiskCache() {
+	if cv.onDiskCacheDisabled {
+		return
+	}
+	now := time.Now()
+	logger.Debugf("cleaning up on-disk CRL cache at %v", now)
+	entries, err := os.ReadDir(cv.onDiskCacheDir)
+	if err != nil {
+		logger.Warnf("failed to read CRL cache dir: %v", err)
+		return
+	}
+	for _, entry := range entries {
+		if !entry.Type().IsRegular() {
+			continue
+		}
+		path := filepath.Join(cv.onDiskCacheDir, entry.Name())
+		crlBytes, err := os.ReadFile(path)
+		if err != nil {
+			logger.Warnf("failed to read CRL file %v: %v", path, err)
+			continue
+		}
+		crl, err := x509.ParseRevocationList(crlBytes)
+		if err != nil {
+			logger.Warnf("failed to parse CRL file %v: %v", path, err)
+			continue
+		}
+		if crl.NextUpdate.Add(cv.onDiskCacheRemovalDelay).Before(now) {
+			logger.Debugf("CRL file %v is expired, removing", path)
+			if err := os.Remove(path); err != nil {
+				logger.Warnf("failed to remove expired CRL file %v: %v", path, err)
+			}
+		}
+	}
+}


Test patch:
diff --git a/crl_test.go b/crl_test.go
index 96c60e7..f506aab 100644
--- a/crl_test.go
+++ b/crl_test.go
@@ -526,6 +526,37 @@ func TestCrlModes(t *testing.T) {
 					_, err = os.Open(cv.crlURLToPath(fullCrlURL("/rootCrl")))
 					assertErrIsE(t, err, os.ErrNotExist, "CRL file should not be created in the cache directory when on-disk cache is disabled")
 				})
+
+				t.Run("should clean up cache", func(t *testing.T) {
+					caPrivateKey, caCert := createCa(t, nil, nil, "root CA", "")
+					_, leafCert := createLeafCert(t, caCert, caPrivateKey, "/rootCrl")
+					crl := createCrl(t, caCert, caPrivateKey, nextUpdateType(time.Now().Add(10*time.Millisecond)))
+
+					server := createCrlServer(t, newCrlEndpointDef("/rootCrl", crl))
+					defer closeServer(t, server)
+
+					cv := newTestCrlValidator(t, checkMode, cacheValidityTimeType(100*time.Millisecond))
+					cv.onDiskCacheRemovalDelay = 200 * time.Millisecond
+					cv.startPeriodicCacheCleanup(10 * time.Millisecond)
+					defer cv.stopPeriodicCacheCleanup()
+
+					err := cv.verifyPeerCertificates(nil, [][]*x509.Certificate{{leafCert, caCert}})
+					assertNilE(t, err)
+					cv.inMemoryCacheMutex.Lock()
+					assertNotNilE(t, cv.inMemoryCache[fullCrlURL("/rootCrl")], "in-memory cache should be populated")
+					cv.inMemoryCacheMutex.Unlock()
+					fd, err := os.Open(cv.crlURLToPath(fullCrlURL("/rootCrl")))
+					assertNilE(t, err, "CRL file should be created in the cache directory")
+					fd.Close()
+
+					time.Sleep(500 * time.Millisecond) // wait for cleanup to happen
+
+					cv.inMemoryCacheMutex.Lock()
+					assertNilE(t, cv.inMemoryCache[fullCrlURL("/rootCrl")], "in-memory cache should be cleaned up")
+					cv.inMemoryCacheMutex.Unlock()
+					_, err = os.Open(cv.crlURLToPath(fullCrlURL("/rootCrl")))
+					assertErrIsE(t, err, os.ErrNotExist, "CRL file should be removed from the cache directory")
+				})
 			})
 		})
 	}