OCPBUGS-49860: Fix certificate issues

catalogd/cmd/catalogd/main.go

@@ -97,7 +97,7 @@ func main() {
 		certFile             string
 		keyFile              string
 		webhookPort          int
-		caCertDir            string
+		pullCasDir           string
 		globalPullSecret     string
 	)
 	flag.StringVar(&metricsAddr, "metrics-bind-address", "", "The address for the metrics endpoint. Requires tls-cert and tls-key. (Default: ':7443')")
@@ -115,7 +115,7 @@ func main() {
 	flag.StringVar(&certFile, "tls-cert", "", "The certificate file used for serving catalog and metrics. Required to enable the metrics server. Requires tls-key.")
 	flag.StringVar(&keyFile, "tls-key", "", "The key file used for serving catalog contents and metrics. Required to enable the metrics server. Requires tls-cert.")
 	flag.IntVar(&webhookPort, "webhook-server-port", 9443, "The port that the mutating webhook server serves at.")
-	flag.StringVar(&caCertDir, "ca-certs-dir", "", "The directory of CA certificate to use for verifying HTTPS connections to image registries.")
+	flag.StringVar(&pullCasDir, "pull-cas-dir", "", "The directory of TLS certificate authoritiess to use for verifying HTTPS connections to image registries.")
 	flag.StringVar(&globalPullSecret, "global-pull-secret", "", "The <namespace>/<name> of the global pull secret that is going to be used to pull bundle images.")
 
 	klog.InitFlags(flag.CommandLine)
@@ -271,8 +271,8 @@ func main() {
 		BaseCachePath: unpackCacheBasePath,
 		SourceContextFunc: func(logger logr.Logger) (*types.SystemContext, error) {
 			srcContext := &types.SystemContext{
-				DockerCertPath: caCertDir,
-				OCICertPath:    caCertDir,
+				DockerCertPath: pullCasDir,
+				OCICertPath:    pullCasDir,
 			}
 			if _, err := os.Stat(authFilePath); err == nil && globalPullSecretKey != nil {
 				logger.Info("using available authentication information for pulling image")

catalogd/config/components/ca/patches/manager_deployment_cacerts.yaml

@@ -6,4 +6,4 @@
   value: {"name":"olmv1-certificate", "readOnly": true, "mountPath":"/var/ca-certs/"}
 - op: add
   path: /spec/template/spec/containers/0/args/-
-  value: "--ca-certs-dir=/var/ca-certs"
+  value: "--pull-cas-dir=/var/ca-certs"

cmd/operator-controller/main.go

@@ -101,12 +101,14 @@ func main() {
 		cachePath                 string
 		operatorControllerVersion bool
 		systemNamespace           string
-		caCertDir                 string
+		catalogdCasDir            string
+		pullCasDir                string
 		globalPullSecret          string
 	)
 	flag.StringVar(&metricsAddr, "metrics-bind-address", "", "The address for the metrics endpoint. Requires tls-cert and tls-key. (Default: ':8443')")
 	flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.")
-	flag.StringVar(&caCertDir, "ca-certs-dir", "", "The directory of TLS certificate to use for verifying HTTPS connections to the Catalogd and docker-registry web servers.")
+	flag.StringVar(&catalogdCasDir, "catalogd-cas-dir", "", "The directory of TLS certificate authorities to use for verifying HTTPS connections to the Catalogd web service.")
+	flag.StringVar(&pullCasDir, "pull-cas-dir", "", "The directory of TLS certificate authorities to use for verifying HTTPS connections to image registries.")
 	flag.StringVar(&certFile, "tls-cert", "", "The certificate file used for the metrics server. Required to enable the metrics server. Requires tls-key.")
 	flag.StringVar(&keyFile, "tls-key", "", "The key file used for the metrics server. Required to enable the metrics server. Requires tls-cert")
 	flag.BoolVar(&enableLeaderElection, "leader-elect", false,
@@ -283,7 +285,7 @@ func main() {
 		os.Exit(1)
 	}
 
-	certPoolWatcher, err := httputil.NewCertPoolWatcher(caCertDir, ctrl.Log.WithName("cert-pool"))
+	certPoolWatcher, err := httputil.NewCertPoolWatcher(catalogdCasDir, ctrl.Log.WithName("cert-pool"))
 	if err != nil {
 		setupLog.Error(err, "unable to create CA certificate pool")
 		os.Exit(1)
@@ -301,8 +303,8 @@ func main() {
 		BaseCachePath: filepath.Join(cachePath, "unpack"),
 		SourceContextFunc: func(logger logr.Logger) (*types.SystemContext, error) {
 			srcContext := &types.SystemContext{
-				DockerCertPath: caCertDir,
-				OCICertPath:    caCertDir,
+				DockerCertPath: pullCasDir,
+				OCICertPath:    pullCasDir,
 			}
 			if _, err := os.Stat(authFilePath); err == nil && globalPullSecretKey != nil {
 				logger.Info("using available authentication information for pulling image")

config/components/tls/patches/manager_deployment_cert.yaml

@@ -6,7 +6,10 @@
   value: {"name":"olmv1-certificate", "readOnly": true, "mountPath":"/var/certs/"}
 - op: add
   path: /spec/template/spec/containers/0/args/-
-  value: "--ca-certs-dir=/var/certs"
+  value: "--catalogd-cas-dir=/var/certs"
+- op: add
+  path: /spec/template/spec/containers/0/args/-
+  value: "--pull-cas-dir=/var/certs"
 - op: add
   path: /spec/template/spec/containers/0/args/-
   value: "--tls-cert=/var/certs/tls.cert"

internal/catalogmetadata/client/client.go

@@ -2,15 +2,18 @@ package client
 
 import (
 	"context"
+	"crypto/tls"
 	"errors"
 	"fmt"
 	"io"
 	"io/fs"
 	"net/http"
 	"net/url"
 
+	"github.com/go-logr/logr"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	ctrl "sigs.k8s.io/controller-runtime"
 
 	"github.com/operator-framework/operator-registry/alpha/declcfg"
 
@@ -111,6 +114,17 @@ func (c *Client) PopulateCache(ctx context.Context, catalog *catalogd.ClusterCat
 	return c.cache.Put(catalog.Name, catalog.Status.ResolvedSource.Image.Ref, resp.Body, nil)
 }
 
+func logX509Error(err error, l logr.Logger) {
+	var cvErr *tls.CertificateVerificationError
+	if errors.As(err, &cvErr) {
+		n := 1
+		for _, cert := range cvErr.UnverifiedCertificates {
+			l.Error(err, "unverified cert", "n", n, "subject", cert.Subject, "issuer", cert.Issuer, "DNSNames", cert.DNSNames, "serial", cert.SerialNumber)
+			n = n + 1
+		}
+	}
+}
+
 func (c *Client) doRequest(ctx context.Context, catalog *catalogd.ClusterCatalog) (*http.Response, error) {
 	if catalog.Status.URLs == nil {
 		return nil, fmt.Errorf("error: catalog %q has a nil status.urls value", catalog.Name)
@@ -133,6 +147,7 @@ func (c *Client) doRequest(ctx context.Context, catalog *catalogd.ClusterCatalog
 
 	resp, err := client.Do(req)
 	if err != nil {
+		logX509Error(err, ctrl.Log.WithName("catalog-client"))
 		return nil, fmt.Errorf("error performing request: %v", err)
 	}
 

internal/httputil/certlog.go

@@ -0,0 +1,83 @@
+package httputil
+
+import (
+	"crypto/x509"
+	"encoding/pem"
+	"fmt"
+	"os"
+	"path/filepath"
+
+	"github.com/go-logr/logr"
+)
+
+func logPath(path, action string, log logr.Logger) {
+	fi, err := os.Stat(path)
+	if err != nil {
+		log.Error(err, "error in os.Stat()", "path", path)
+		return
+	}
+	if !fi.IsDir() {
+		logFile(path, "", fmt.Sprintf("%s file", action), log)
+		return
+	}
+	action = fmt.Sprintf("%s directory", action)
+	dirEntries, err := os.ReadDir(path)
+	if err != nil {
+		log.Error(err, "error in os.ReadDir()", "path", path)
+		return
+	}
+	for _, e := range dirEntries {
+		file := filepath.Join(path, e.Name())
+		fi, err := os.Stat(file)
+		if err != nil {
+			log.Error(err, "error in os.Stat()", "file", file)
+			continue
+		}
+		if fi.IsDir() {
+			log.Info("ignoring subdirectory", "directory", file)
+			continue
+		}
+		logFile(e.Name(), path, action, log)
+	}
+}
+
+func logFile(filename, path, action string, log logr.Logger) {
+	filepath := filepath.Join(path, filename)
+	data, err := os.ReadFile(filepath)
+	if err != nil {
+		log.Error(err, "error in os.ReadFile()", "file", filename)
+		return
+	}
+	logPem(data, filename, path, action, log)
+}
+
+func logPem(data []byte, filename, path, action string, log logr.Logger) {
+	for len(data) > 0 {
+		var block *pem.Block
+		block, data = pem.Decode(data)
+		if block == nil {
+			log.Info("error: no block returned from pem.Decode()", "file", filename)
+			return
+		}
+		crt, err := x509.ParseCertificate(block.Bytes)
+		if err != nil {
+			log.Error(err, "error in x509.ParseCertificate()", "file", filename)
+			return
+		}
+
+		args := []any{}
+		if path != "" {
+			args = append(args, "directory", path)
+		}
+		// Find an appopriate certificate identifier
+		args = append(args, "file", filename)
+		if s := crt.Subject.String(); s != "" {
+			args = append(args, "subject", s)
+		} else if crt.DNSNames != nil {
+			args = append(args, "DNSNames", crt.DNSNames)
+		} else if s := crt.SerialNumber.String(); s != "" {
+			args = append(args, "serial", s)
+		}
+		log.Info(action, args...)
+	}
+}

internal/httputil/certpoolwatcher.go

@@ -4,6 +4,8 @@ import (
 	"crypto/x509"
 	"fmt"
 	"os"
+	"slices"
+	"strings"
 	"sync"
 	"time"
 
@@ -44,8 +46,31 @@ func NewCertPoolWatcher(caDir string, log logr.Logger) (*CertPoolWatcher, error)
 	if err != nil {
 		return nil, err
 	}
-	if err = watcher.Add(caDir); err != nil {
-		return nil, err
+
+	// If the SSL_CERT_DIR or SSL_CERT_FILE environment variables are
+	// specified, this means that we have some control over the system root
+	// location, thus they may change, thus we should watch those locations.
+	sslCertDir := os.Getenv("SSL_CERT_DIR")
+	sslCertFile := os.Getenv("SSL_CERT_FILE")
+	log.Info("SSL environment", "SSL_CERT_DIR", sslCertDir, "SSL_CERT_FILE", sslCertFile)
+
+	watchPaths := strings.Split(sslCertDir, ":")
+	watchPaths = append(watchPaths, caDir, sslCertFile)
+	watchPaths = slices.DeleteFunc(watchPaths, func(p string) bool {
+		if p == "" {
+			return true
+		}
+		if _, err := os.Stat(p); err != nil {
+			return true
+		}
+		return false
+	})
+
+	for _, p := range watchPaths {
+		if err := watcher.Add(p); err != nil {
+			return nil, err
+		}
+		logPath(p, "watching certificate", log)
 	}
 
 	cpw := &CertPoolWatcher{

internal/httputil/certpoolwatcher_test.go

@@ -72,6 +72,10 @@ func TestCertPoolWatcher(t *testing.T) {
 	t.Logf("Create cert file at %q\n", certName)
 	createCert(t, certName)
 
+	// Update environment variables for the watcher - some of these should not exist
+	os.Setenv("SSL_CERT_DIR", tmpDir+":/tmp/does-not-exist.dir")
+	os.Setenv("SSL_CERT_FILE", "/tmp/does-not-exist.file")
+
 	// Create the cert pool watcher
 	cpw, err := httputil.NewCertPoolWatcher(tmpDir, log.FromContext(context.Background()))
 	require.NoError(t, err)

internal/httputil/certutil.go

@@ -5,7 +5,6 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
-	"time"
 
 	"github.com/go-logr/logr"
 )
@@ -24,7 +23,6 @@ func NewCertPool(caDir string, log logr.Logger) (*x509.CertPool, error) {
 		return nil, err
 	}
 	count := 0
-	firstExpiration := time.Time{}
 
 	for _, e := range dirEntries {
 		file := filepath.Join(caDir, e.Name())
@@ -46,13 +44,13 @@ func NewCertPool(caDir string, log logr.Logger) (*x509.CertPool, error) {
 		if caCertPool.AppendCertsFromPEM(data) {
 			count++
 		}
+		logPem(data, e.Name(), caDir, "loading certificate file", log)
 	}
 
 	// Found no certs!
 	if count == 0 {
 		return nil, fmt.Errorf("no certificates found in %q", caDir)
 	}
 
-	log.Info("first expiration", "time", firstExpiration.Format(time.RFC3339))
 	return caCertPool, nil
 }

openshift/catalogd/kustomize/overlays/openshift/olmv1-ns/patches/manager_deployment_certs.yaml

@@ -1,27 +1,21 @@
 - op: add
   path: /spec/template/spec/volumes/-
-  value: {"name":"catalogserver-certs", "secret":{"optional":false,"secretName":"catalogserver-cert"}}
+  value: {"name":"catalogserver-certs", "secret":{"optional":false,"secretName":"catalogserver-cert","items":[{"key":"tls.crt","path":"tls.crt"},{"key":"tls.key","path":"tls.key"}]}}
 - op: add
   path: /spec/template/spec/volumes/-
-  value: {"name":"trusted-ca-bundle", "configMap":{"optional":false,"name":"trusted-ca-bundle", "items":[{"key":"ca-bundle.crt","path":"ca-bundle.crt"}]}}
-- op: add
-  path: /spec/template/spec/volumes/-
-  value: {"name":"service-ca", "configMap":{"optional":false,"name":"openshift-service-ca.crt", "items":[{"key":"service-ca.crt","path":"service-ca.crt"}]}}
+  value: {"name":"ca-certs", "projected": {"sources":[{"configMap":{"optional":false,"name":"trusted-ca-bundle", "items":[{"key":"ca-bundle.crt","path":"ca-bundle.crt"}]}},{"configMap":{"optional":false,"name":"openshift-service-ca.crt", "items":[{"key":"service-ca.crt","path":"service-ca.crt"}]}}]}}
 - op: add
   path: /spec/template/spec/containers/0/volumeMounts/-
   value: {"name":"catalogserver-certs", "mountPath":"/var/certs"}
 - op: add
   path: /spec/template/spec/containers/0/volumeMounts/-
-  value: {"name":"trusted-ca-bundle", "mountPath":"/var/trusted-cas/ca-bundle.crt", "subPath":"ca-bundle.crt"}
-- op: add
-  path: /spec/template/spec/containers/0/volumeMounts/-
-  value: {"name":"service-ca", "mountPath":"/var/trusted-cas/service-ca.crt", "subPath":"service-ca.crt"}
+  value: {"name":"ca-certs", "mountPath":"/var/ca-certs", "readOnly": true}
 - op: add
   path: /spec/template/spec/containers/0/args/-
   value: "--tls-cert=/var/certs/tls.crt"
 - op: add
   path: /spec/template/spec/containers/0/args/-
   value: "--tls-key=/var/certs/tls.key"
 - op: add
-  path: /spec/template/spec/containers/0/args/-
-  value: "--ca-certs-dir=/var/trusted-cas"
+  path: /spec/template/spec/containers/0/env
+  value: [{"name":"SSL_CERT_DIR", "value":"/var/ca-certs"}]