🌱 Add support for CA/certificate rotation (#1062)

* Add support for CA/certificate rotation

Mounted secrets are automatically updated into pods, but...
* It doesn't work with `subPath` mountings
* When `subPath` is not used, then a bunch of directories are mounted
* And one of those directories is a symlink, so `IsDir()` returns false
* And a watch is needed to notice the change

So, update the certificate volume patch, which requires a change in how
we look for certificates in the CA cert directory.

Add a watch, so when the certs do change, we update the cert pool.

Also look at validity dates of certificates, and error on expired certs.

The default cert-manager certificates have 90 days validities.

Signed-off-by: Todd Short <[email protected]>

* fixup! Add support for CA/certificate rotation

* fixup! Add support for CA/certificate rotation

Signed-off-by: Todd Short <[email protected]>

* fixup! Add support for CA/certificate rotation

Signed-off-by: Todd Short <[email protected]>

---------

Signed-off-by: Todd Short <[email protected]>

Author: tmshort

cmd/manager/main.go

@@ -20,6 +20,7 @@ import (
 	"context"
 	"flag"
 	"fmt"
+	"net/http"
 	"os"
 	"path/filepath"
 	"time"
@@ -206,16 +207,16 @@ func main() {
 		os.Exit(1)
 	}
 
-	certPool, err := httputil.NewCertPool(caCertDir)
+	certPoolWatcher, err := httputil.NewCertPoolWatcher(caCertDir, ctrl.Log.WithName("cert-pool"))
 	if err != nil {
 		setupLog.Error(err, "unable to create CA certificate pool")
 		os.Exit(1)
 	}
 	unpacker := &source.ImageRegistry{
 		BaseCachePath: filepath.Join(cachePath, "unpack"),
 		// TODO: This needs to be derived per extension via ext.Spec.InstallNamespace
-		AuthNamespace: systemNamespace,
-		CaCertPool:    certPool,
+		AuthNamespace:   systemNamespace,
+		CertPoolWatcher: certPoolWatcher,
 	}
 
 	clusterExtensionFinalizers := crfinalizer.NewFinalizers()
@@ -240,18 +241,15 @@ func main() {
 	}
 
 	cl := mgr.GetClient()
-	httpClient, err := httputil.BuildHTTPClient(certPool)
-	if err != nil {
-		setupLog.Error(err, "unable to create catalogd http client")
-		os.Exit(1)
-	}
 
 	catalogsCachePath := filepath.Join(cachePath, "catalogs")
 	if err := os.MkdirAll(catalogsCachePath, 0700); err != nil {
 		setupLog.Error(err, "unable to create catalogs cache directory")
 		os.Exit(1)
 	}
-	catalogClient := catalogclient.New(cache.NewFilesystemCache(catalogsCachePath, httpClient))
+	catalogClient := catalogclient.New(cache.NewFilesystemCache(catalogsCachePath, func() (*http.Client, error) {
+		return httputil.BuildHTTPClient(certPoolWatcher)
+	}))
 
 	resolver := &resolve.CatalogResolver{
 		WalkCatalogsFunc: resolve.CatalogWalker(
@@ -273,7 +271,6 @@ func main() {
 		Unpacker:              unpacker,
 		InstalledBundleGetter: &controllers.DefaultInstalledBundleGetter{ActionClientGetter: acg},
 		Finalizers:            clusterExtensionFinalizers,
-		CaCertPool:            certPool,
 		Preflights:            preflights,
 	}).SetupWithManager(mgr); err != nil {
 		setupLog.Error(err, "unable to create controller", "controller", "ClusterExtension")

config/components/tls/patches/manager_deployment_cert.yaml

@@ -3,7 +3,7 @@
   value: {"name":"olmv1-certificate", "secret":{"secretName":"olmv1-cert", "optional": false, "items": [{"key": "ca.crt", "path": "olm-ca.crt"}]}}
 - op: add
   path: /spec/template/spec/containers/0/volumeMounts/-
-  value: {"name":"olmv1-certificate", "readOnly": true, "mountPath":"/var/certs/olm-ca.crt", "subPath":"olm-ca.crt"}
+  value: {"name":"olmv1-certificate", "readOnly": true, "mountPath":"/var/certs/"}
 - op: add
   path: /spec/template/spec/containers/0/args/-
   value: "--ca-certs-dir=/var/certs"

go.mod

@@ -7,6 +7,7 @@ require (
 	github.com/Masterminds/semver/v3 v3.2.1
 	github.com/blang/semver/v4 v4.0.0
 	github.com/containerd/containerd v1.7.20
+	github.com/fsnotify/fsnotify v1.7.0
 	github.com/go-logr/logr v1.4.2
 	github.com/google/go-cmp v0.6.0
 	github.com/google/go-containerregistry v0.20.1
@@ -112,7 +113,6 @@ require (
 	github.com/exponent-io/jsonpath v0.0.0-20210407135951-1de76d718b3f // indirect
 	github.com/fatih/color v1.15.0 // indirect
 	github.com/felixge/httpsnoop v1.0.4 // indirect
-	github.com/fsnotify/fsnotify v1.7.0 // indirect
 	github.com/go-errors/errors v1.4.2 // indirect
 	github.com/go-git/gcfg v1.5.1-0.20230307220236-3a3c6141e376 // indirect
 	github.com/go-git/go-billy/v5 v5.5.0 // indirect

internal/catalogmetadata/cache/cache.go

@@ -25,11 +25,11 @@ var _ client.Fetcher = &filesystemCache{}
 //   - IF cached it will verify the cache is up to date. If it is up to date it will return
 //     the cached contents, if not it will fetch the new contents from the catalogd HTTP
 //     server and update the cached contents.
-func NewFilesystemCache(cachePath string, client *http.Client) client.Fetcher {
+func NewFilesystemCache(cachePath string, clientFunc func() (*http.Client, error)) client.Fetcher {
 	return &filesystemCache{
 		cachePath:              cachePath,
 		mutex:                  sync.RWMutex{},
-		client:                 client,
+		getClient:              clientFunc,
 		cacheDataByCatalogName: map[string]cacheData{},
 	}
 }
@@ -50,7 +50,7 @@ type cacheData struct {
 type filesystemCache struct {
 	mutex                  sync.RWMutex
 	cachePath              string
-	client                 *http.Client
+	getClient              func() (*http.Client, error)
 	cacheDataByCatalogName map[string]cacheData
 }
 
@@ -95,7 +95,11 @@ func (fsc *filesystemCache) FetchCatalogContents(ctx context.Context, catalog *c
 		return nil, fmt.Errorf("error forming request: %v", err)
 	}
 
-	resp, err := fsc.client.Do(req)
+	client, err := fsc.getClient()
+	if err != nil {
+		return nil, fmt.Errorf("error getting HTTP client: %w", err)
+	}
+	resp, err := client.Do(req)
 	if err != nil {
 		return nil, fmt.Errorf("error performing request: %v", err)
 	}

internal/catalogmetadata/cache/cache_test.go

@@ -214,7 +214,9 @@ func TestFilesystemCache(t *testing.T) {
 			maps.Copy(tt.tripper.content, tt.contents)
 			httpClient := http.DefaultClient
 			httpClient.Transport = tt.tripper
-			c := cache.NewFilesystemCache(cacheDir, httpClient)
+			c := cache.NewFilesystemCache(cacheDir, func() (*http.Client, error) {
+				return httpClient, nil
+			})
 
 			actualFS, err := c.FetchCatalogContents(ctx, tt.catalog)
 			if !tt.wantErr {

internal/controllers/clusterextension_controller.go

@@ -19,7 +19,6 @@ package controllers
 import (
 	"bytes"
 	"context"
-	"crypto/x509"
 	"errors"
 	"fmt"
 	"io"
@@ -89,7 +88,6 @@ type ClusterExtensionReconciler struct {
 	cache                 cache.Cache
 	InstalledBundleGetter InstalledBundleGetter
 	Finalizers            crfinalizer.Finalizers
-	CaCertPool            *x509.CertPool
 	Preflights            []Preflight
 }
 

internal/httputil/certpoolwatcher.go

@@ -0,0 +1,108 @@
+package httputil
+
+import (
+	"crypto/x509"
+	"fmt"
+	"os"
+	"sync"
+	"time"
+
+	"github.com/fsnotify/fsnotify"
+	"github.com/go-logr/logr"
+)
+
+type CertPoolWatcher struct {
+	generation int
+	dir        string
+	mx         sync.RWMutex
+	pool       *x509.CertPool
+	log        logr.Logger
+	watcher    *fsnotify.Watcher
+	done       chan bool
+}
+
+// Returns the current CertPool and the generation number
+func (cpw *CertPoolWatcher) Get() (*x509.CertPool, int, error) {
+	cpw.mx.RLock()
+	defer cpw.mx.RUnlock()
+	if cpw.pool == nil {
+		return nil, 0, fmt.Errorf("no certificate pool available")
+	}
+	return cpw.pool.Clone(), cpw.generation, nil
+}
+
+func (cpw *CertPoolWatcher) Done() {
+	cpw.done <- true
+}
+
+func NewCertPoolWatcher(caDir string, log logr.Logger) (*CertPoolWatcher, error) {
+	pool, err := NewCertPool(caDir, log)
+	if err != nil {
+		return nil, err
+	}
+	watcher, err := fsnotify.NewWatcher()
+	if err != nil {
+		return nil, err
+	}
+	if err = watcher.Add(caDir); err != nil {
+		return nil, err
+	}
+
+	cpw := &CertPoolWatcher{
+		generation: 1,
+		dir:        caDir,
+		pool:       pool,
+		log:        log,
+		watcher:    watcher,
+		done:       make(chan bool),
+	}
+	go func() {
+		for {
+			select {
+			case <-watcher.Events:
+				cpw.drainEvents()
+				cpw.update()
+			case err := <-watcher.Errors:
+				log.Error(err, "error watching certificate dir")
+				os.Exit(1)
+			case <-cpw.done:
+				err := watcher.Close()
+				if err != nil {
+					log.Error(err, "error closing watcher")
+				}
+				return
+			}
+		}
+	}()
+	return cpw, nil
+}
+
+func (cpw *CertPoolWatcher) update() {
+	cpw.log.Info("updating certificate pool")
+	pool, err := NewCertPool(cpw.dir, cpw.log)
+	if err != nil {
+		cpw.log.Error(err, "error updating certificate pool")
+		os.Exit(1)
+	}
+	cpw.mx.Lock()
+	defer cpw.mx.Unlock()
+	cpw.pool = pool
+	cpw.generation++
+}
+
+// Drain as many events as possible before doing anything
+// Otherwise, we will be hit with an event for _every_ entry in the
+// directory, and end up doing an update for each one
+func (cpw *CertPoolWatcher) drainEvents() {
+	for {
+		drainTimer := time.NewTimer(time.Millisecond * 50)
+		select {
+		case <-drainTimer.C:
+			return
+		case <-cpw.watcher.Events:
+		}
+		if !drainTimer.Stop() {
+			<-drainTimer.C
+		}
+	}
+}

internal/httputil/certpoolwatcher_test.go

@@ -0,0 +1,97 @@
+package httputil_test
+
+import (
+	"context"
+	"crypto/ecdsa"
+	"crypto/elliptic"
+	"crypto/rand"
+	"crypto/x509"
+	"crypto/x509/pkix"
+	"encoding/pem"
+	"math/big"
+	"os"
+	"path/filepath"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+	"sigs.k8s.io/controller-runtime/pkg/log"
+
+	"github.com/operator-framework/operator-controller/internal/httputil"
+)
+
+func createCert(t *testing.T, name string) {
+	priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	require.NoError(t, err)
+
+	notBefore := time.Now()
+	notAfter := notBefore.Add(time.Hour)
+	serialNumberLimit := new(big.Int).Lsh(big.NewInt(1), 128)
+	serialNumber, err := rand.Int(rand.Reader, serialNumberLimit)
+	require.NoError(t, err)
+
+	template := x509.Certificate{
+		SerialNumber: serialNumber,
+		Subject: pkix.Name{
+			Organization: []string{name},
+		},
+		NotBefore: notBefore,
+		NotAfter:  notAfter,
+
+		IsCA: true,
+
+		KeyUsage:    x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign,
+		ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth},
+
+		BasicConstraintsValid: true,
+	}
+
+	derBytes, err := x509.CreateCertificate(rand.Reader, &template, &template, &priv.PublicKey, priv)
+	require.NoError(t, err)
+
+	certOut, err := os.Create(name)
+	require.NoError(t, err)
+
+	err = pem.Encode(certOut, &pem.Block{Type: "CERTIFICATE", Bytes: derBytes})
+	require.NoError(t, err)
+
+	err = certOut.Close()
+	require.NoError(t, err)
+
+	// ignore the key
+}
+
+func TestCertPoolWatcher(t *testing.T) {
+	// create a temporary directory
+	tmpDir, err := os.MkdirTemp("", "cert-pool")
+	require.NoError(t, err)
+	defer os.RemoveAll(tmpDir)
+
+	// create the first cert
+	certName := filepath.Join(tmpDir, "test1.pem")
+	t.Logf("Create cert file at %q\n", certName)
+	createCert(t, certName)
+
+	// Create the cert pool watcher
+	cpw, err := httputil.NewCertPoolWatcher(tmpDir, log.FromContext(context.Background()))
+	require.NoError(t, err)
+	defer cpw.Done()
+
+	// Get the original pool
+	firstPool, firstGen, err := cpw.Get()
+	require.NoError(t, err)
+	require.NotNil(t, firstPool)
+
+	// Create a second cert
+	certName = filepath.Join(tmpDir, "test2.pem")
+	t.Logf("Create cert file at %q\n", certName)
+	createCert(t, certName)
+
+	require.Eventually(t, func() bool {
+		secondPool, secondGen, err := cpw.Get()
+		if err != nil {
+			return false
+		}
+		return secondGen != firstGen && !firstPool.Equal(secondPool)
+	}, 30*time.Second, time.Second)
+}