caddypki,caddytls: fix data races in CA accessors and TLS storage cleanup

modules/caddypki/ca.go

@@ -161,7 +161,9 @@ func (ca *CA) Provision(ctx caddy.Context, id string, log *zap.Logger) error {
 }
 
 // RootCertificate returns the CA's root certificate (public key).
-func (ca CA) RootCertificate() *x509.Certificate {
+// Note: This method uses a pointer receiver to prevent data races.
+// Using a value receiver would copy the struct before acquiring the lock.
+func (ca *CA) RootCertificate() *x509.Certificate {
 	ca.mu.RLock()
 	defer ca.mu.RUnlock()
 	return ca.root
@@ -170,21 +172,25 @@ func (ca CA) RootCertificate() *x509.Certificate {
 // RootKey returns the CA's root private key. Since the root key is
 // not cached in memory long-term, it needs to be loaded from storage,
 // which could yield an error.
-func (ca CA) RootKey() (any, error) {
+func (ca *CA) RootKey() (any, error) {
 	_, rootKey, err := ca.loadOrGenRoot()
 	return rootKey, err
 }
 
 // IntermediateCertificate returns the CA's intermediate
 // certificate (public key).
-func (ca CA) IntermediateCertificate() *x509.Certificate {
+// Note: This method uses a pointer receiver to prevent data races.
+// Using a value receiver would copy the struct before acquiring the lock.
+func (ca *CA) IntermediateCertificate() *x509.Certificate {
 	ca.mu.RLock()
 	defer ca.mu.RUnlock()
 	return ca.inter
 }
 
 // IntermediateKey returns the CA's intermediate private key.
-func (ca CA) IntermediateKey() any {
+// Note: This method uses a pointer receiver to prevent data races.
+// Using a value receiver would copy the struct before acquiring the lock.
+func (ca *CA) IntermediateKey() any {
 	ca.mu.RLock()
 	defer ca.mu.RUnlock()
 	return ca.interKey

modules/caddytls/tls.go

@@ -130,6 +130,7 @@ type TLS struct {
 	ctx                caddy.Context
 	storageCleanTicker *time.Ticker
 	storageCleanStop   chan struct{}
+	storageCleanMu     sync.Mutex // protects storageCleanTicker and storageCleanStop
 	logger             *zap.Logger
 	events             *caddyevents.App
 
@@ -433,11 +434,18 @@ func (t *TLS) Start() error {
 // Stop stops the TLS module and cleans up any allocations.
 func (t *TLS) Stop() error {
 	// stop the storage cleaner goroutine and ticker
-	if t.storageCleanStop != nil {
-		close(t.storageCleanStop)
+	t.storageCleanMu.Lock()
+	stopChan := t.storageCleanStop
+	ticker := t.storageCleanTicker
+	t.storageCleanStop = nil
+	t.storageCleanTicker = nil
+	t.storageCleanMu.Unlock()
+
+	if stopChan != nil {
+		close(stopChan)
 	}
-	if t.storageCleanTicker != nil {
-		t.storageCleanTicker.Stop()
+	if ticker != nil {
+		ticker.Stop()
 	}
 	return nil
 }
@@ -786,8 +794,10 @@ func (t *TLS) HasCertificateForSubject(subject string) bool {
 // known storage units if it was not recently done, and then runs the
 // operation at every tick from t.storageCleanTicker.
 func (t *TLS) keepStorageClean() {
+	t.storageCleanMu.Lock()
 	t.storageCleanTicker = time.NewTicker(t.storageCleanInterval())
 	t.storageCleanStop = make(chan struct{})
+	t.storageCleanMu.Unlock()
 	go func() {
 		defer func() {
 			if err := recover(); err != nil {