diff --git a/catalogd/cmd/catalogd/main.go b/catalogd/cmd/catalogd/main.go index 35854aeae..b4e58c6a9 100644 --- a/catalogd/cmd/catalogd/main.go +++ b/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 / 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") diff --git a/catalogd/config/components/ca/patches/manager_deployment_cacerts.yaml b/catalogd/config/components/ca/patches/manager_deployment_cacerts.yaml index b5b03633e..6b0816706 100644 --- a/catalogd/config/components/ca/patches/manager_deployment_cacerts.yaml +++ b/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" diff --git a/cmd/operator-controller/main.go b/cmd/operator-controller/main.go index b7b8551a4..556bfed65 100644 --- a/cmd/operator-controller/main.go +++ b/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") diff --git a/config/components/tls/patches/manager_deployment_cert.yaml b/config/components/tls/patches/manager_deployment_cert.yaml index 18afac59d..8fbdb5592 100644 --- a/config/components/tls/patches/manager_deployment_cert.yaml +++ b/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" diff --git a/internal/catalogmetadata/client/client.go b/internal/catalogmetadata/client/client.go index 7daddaaec..3774efd1c 100644 --- a/internal/catalogmetadata/client/client.go +++ b/internal/catalogmetadata/client/client.go @@ -2,6 +2,7 @@ package client import ( "context" + "crypto/tls" "errors" "fmt" "io" @@ -9,8 +10,10 @@ import ( "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) } diff --git a/internal/httputil/certlog.go b/internal/httputil/certlog.go new file mode 100644 index 000000000..4ea076022 --- /dev/null +++ b/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...) + } +} diff --git a/internal/httputil/certpoolwatcher.go b/internal/httputil/certpoolwatcher.go index 2a250d069..adbb3b0fa 100644 --- a/internal/httputil/certpoolwatcher.go +++ b/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{ diff --git a/internal/httputil/certpoolwatcher_test.go b/internal/httputil/certpoolwatcher_test.go index bfebebd28..2ea3f862a 100644 --- a/internal/httputil/certpoolwatcher_test.go +++ b/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) diff --git a/internal/httputil/certutil.go b/internal/httputil/certutil.go index a6cd9f98e..53af8fe71 100644 --- a/internal/httputil/certutil.go +++ b/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,6 +44,7 @@ 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! @@ -53,6 +52,5 @@ func NewCertPool(caDir string, log logr.Logger) (*x509.CertPool, error) { return nil, fmt.Errorf("no certificates found in %q", caDir) } - log.Info("first expiration", "time", firstExpiration.Format(time.RFC3339)) return caCertPool, nil } diff --git a/openshift/catalogd/kustomize/overlays/openshift/olmv1-ns/patches/manager_deployment_certs.yaml b/openshift/catalogd/kustomize/overlays/openshift/olmv1-ns/patches/manager_deployment_certs.yaml index 2a8207da6..2d5ece67b 100644 --- a/openshift/catalogd/kustomize/overlays/openshift/olmv1-ns/patches/manager_deployment_certs.yaml +++ b/openshift/catalogd/kustomize/overlays/openshift/olmv1-ns/patches/manager_deployment_certs.yaml @@ -1,21 +1,15 @@ - 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" @@ -23,5 +17,5 @@ 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"}] diff --git a/openshift/catalogd/manifests/14-deployment-openshift-catalogd-catalogd-controller-manager.yml b/openshift/catalogd/manifests/14-deployment-openshift-catalogd-catalogd-controller-manager.yml index f2297bc3c..2859102f7 100644 --- a/openshift/catalogd/manifests/14-deployment-openshift-catalogd-catalogd-controller-manager.yml +++ b/openshift/catalogd/manifests/14-deployment-openshift-catalogd-catalogd-controller-manager.yml @@ -46,11 +46,13 @@ spec: - --external-address=catalogd-service.openshift-catalogd.svc - --tls-cert=/var/certs/tls.crt - --tls-key=/var/certs/tls.key - - --ca-certs-dir=/var/trusted-cas - --v=${LOG_VERBOSITY} - --global-pull-secret=openshift-config/pull-secret command: - ./catalogd + env: + - name: SSL_CERT_DIR + value: /var/ca-certs image: ${CATALOGD_IMAGE} imagePullPolicy: IfNotPresent livenessProbe: @@ -81,12 +83,9 @@ spec: name: cache - mountPath: /var/certs name: catalogserver-certs - - 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 @@ -119,22 +118,28 @@ spec: name: cache - name: catalogserver-certs secret: - optional: false - secretName: catalogserver-cert - - configMap: items: - - key: ca-bundle.crt - path: ca-bundle.crt - name: catalogd-trusted-ca-bundle + - key: tls.crt + path: tls.crt + - key: tls.key + path: tls.key 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 + secretName: catalogserver-cert + - name: ca-certs + projected: + sources: + - configMap: + items: + - key: ca-bundle.crt + path: ca-bundle.crt + name: catalogd-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 diff --git a/openshift/operator-controller/kustomize/overlays/openshift/olmv1-ns/patches/manager_deployment_certs.yaml b/openshift/operator-controller/kustomize/overlays/openshift/olmv1-ns/patches/manager_deployment_certs.yaml index 4100ff569..dda3b8c24 100644 --- a/openshift/operator-controller/kustomize/overlays/openshift/olmv1-ns/patches/manager_deployment_certs.yaml +++ b/openshift/operator-controller/kustomize/overlays/openshift/olmv1-ns/patches/manager_deployment_certs.yaml @@ -1,21 +1,15 @@ - op: add path: /spec/template/spec/volumes/- - value: {"name":"operator-controller-certs", "secret":{"optional":false,"secretName":"operator-controller-cert"}} + value: {"name":"operator-controller-certs", "secret":{"optional":false,"secretName":"operator-controller-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":"operator-controller-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" @@ -23,5 +17,5 @@ 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"}] diff --git a/openshift/operator-controller/manifests/20-deployment-openshift-operator-controller-operator-controller-controller-manager.yml b/openshift/operator-controller/manifests/20-deployment-openshift-operator-controller-operator-controller-controller-manager.yml index 3cb5f9ad0..a0604b26b 100644 --- a/openshift/operator-controller/manifests/20-deployment-openshift-operator-controller-operator-controller-controller-manager.yml +++ b/openshift/operator-controller/manifests/20-deployment-openshift-operator-controller-operator-controller-controller-manager.yml @@ -45,11 +45,13 @@ spec: - --leader-elect - --tls-cert=/var/certs/tls.crt - --tls-key=/var/certs/tls.key - - --ca-certs-dir=/var/trusted-cas - --v=${LOG_VERBOSITY} - --global-pull-secret=openshift-config/pull-secret command: - /operator-controller + env: + - name: SSL_CERT_DIR + value: /var/ca-certs image: ${OPERATOR_CONTROLLER_IMAGE} imagePullPolicy: IfNotPresent livenessProbe: @@ -80,12 +82,9 @@ spec: name: cache - mountPath: /var/certs name: operator-controller-certs - - 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 @@ -118,22 +117,28 @@ spec: name: cache - name: operator-controller-certs secret: - optional: false - secretName: operator-controller-cert - - configMap: items: - - key: ca-bundle.crt - path: ca-bundle.crt - name: operator-controller-trusted-ca-bundle + - key: tls.crt + path: tls.crt + - key: tls.key + path: tls.key 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 + secretName: operator-controller-cert + - 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