OCPBUGS-49896: [release-4.18]: Certificate updates

cmd/manager/main.go

@@ -95,12 +95,14 @@ func main() {
 		cachePath                 string
 		operatorControllerVersion bool
 		systemNamespace           string
-		caCertDir                 string
+		catalogdCasDir            string
+		pullCasDir                string
 		globalPullSecret          string
 	)
 	flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "The address the metric endpoint binds to.")
 	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.BoolVar(&enableLeaderElection, "leader-elect", false,
 		"Enable leader election for controller manager. "+
 			"Enabling this will ensure there is only one active controller manager.")
@@ -221,7 +223,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)
@@ -231,8 +233,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,4 +6,7 @@
   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"

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"
 
 	catalogd "github.com/operator-framework/catalogd/api/v1"
 	"github.com/operator-framework/operator-registry/alpha/declcfg"
@@ -110,6 +113,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)
@@ -132,6 +146,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/kustomize/overlays/openshift/olmv1-ns/patches/manager_deployment_certs.yaml

@@ -1,15 +1,9 @@
 - 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"}]}}
-- 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" }
+  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":"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: "--ca-certs-dir=/var/trusted-cas"
+  path: /spec/template/spec/containers/0/env
+  value: [{"name":"SSL_CERT_DIR", "value":"/var/ca-certs"}]

openshift/kustomize/overlays/openshift/olmv1-ns/patches/manager_deployment_mount_etc_containers.yaml

@@ -4,3 +4,9 @@
 - op: add
   path: /spec/template/spec/containers/0/volumeMounts/-
   value: {"name":"etc-containers", "readOnly": true, "mountPath":"/etc/containers"}
+- op: add
+  path: /spec/template/spec/volumes/-
+  value: {"name":"etc-docker", "hostPath":{"path":"/etc/docker", "type": "Directory"}}
+- op: add
+  path: /spec/template/spec/containers/0/volumeMounts/-
+  value: {"name":"etc-docker", "readOnly": true, "mountPath":"/etc/docker"}

openshift/manifests/20-deployment-openshift-operator-controller-operator-controller-controller-manager.yml

@@ -43,11 +43,13 @@ spec:
             - --health-probe-bind-address=:8081
             - --metrics-bind-address=127.0.0.1:8080
             - --leader-elect
-            - --ca-certs-dir=/var/trusted-cas
             - --v=${LOG_VERBOSITY}
             - --global-pull-secret=openshift-config/pull-secret
           command:
             - /manager
+          env:
+            - name: SSL_CERT_DIR
+              value: /var/ca-certs
           image: ${OPERATOR_CONTROLLER_IMAGE}
           imagePullPolicy: IfNotPresent
           livenessProbe:
@@ -76,15 +78,15 @@ spec:
           volumeMounts:
             - mountPath: /var/cache
               name: cache
-            - mountPath: /var/trusted-cas/ca-bundle.crt
-              name: trusted-ca-bundle
-              subPath: ca-bundle.crt
-            - mountPath: /var/trusted-cas/service-ca.crt
-              name: service-ca
-              subPath: service-ca.crt
+            - mountPath: /var/ca-certs
+              name: ca-certs
+              readOnly: true
             - mountPath: /etc/containers
               name: etc-containers
               readOnly: true
+            - mountPath: /etc/docker
+              name: etc-docker
+              readOnly: true
         - args:
             - --secure-listen-address=0.0.0.0:8443
             - --http2-disable
@@ -131,22 +133,27 @@ spec:
       volumes:
         - emptyDir: {}
           name: cache
-        - configMap:
-            items:
-              - key: ca-bundle.crt
-                path: ca-bundle.crt
-            name: operator-controller-trusted-ca-bundle
-            optional: false
-          name: trusted-ca-bundle
-        - configMap:
-            items:
-              - key: service-ca.crt
-                path: service-ca.crt
-            name: openshift-service-ca.crt
-            optional: false
-          name: service-ca
+        - name: ca-certs
+          projected:
+            sources:
+              - configMap:
+                  items:
+                    - key: ca-bundle.crt
+                      path: ca-bundle.crt
+                  name: operator-controller-trusted-ca-bundle
+                  optional: false
+              - configMap:
+                  items:
+                    - key: service-ca.crt
+                      path: service-ca.crt
+                  name: openshift-service-ca.crt
+                  optional: false
         - hostPath:
             path: /etc/containers
             type: Directory
           name: etc-containers
+        - hostPath:
+            path: /etc/docker
+            type: Directory
+          name: etc-docker
       priorityClassName: system-cluster-critical