From 71dd44ebdb689925a430e6e5df45391ac68d722d Mon Sep 17 00:00:00 2001 From: Camila Macedo <7708031+camilamacedo86@users.noreply.github.com> Date: Tue, 26 Nov 2024 12:49:45 +0000 Subject: [PATCH] Replace kube-rbac-proxy to ensure the same level of protection with controller-runtime feature Utilise Controller-Runtime's WithAuthenticationAndAuthorization feature to protect the metrics endpoint. This approach provides access control, similar to the functionality of kube-rbac-proxy. kube-rbac-proxy image from gcr.io/kubebuilder/kube-rbac-proxy is deprecated and should no longer be used More info: https://github.com/kubernetes-sigs/kubebuilder/discussions/3907 --- cmd/manager/main.go | 66 ++++++++++++++++++- config/base/manager/manager.yaml | 27 ++------ config/base/rbac/kustomization.yaml | 10 ++- .../coverage/manager_e2e_coverage_patch.yaml | 1 - .../manager_e2e_registries_conf_patch.yaml | 1 - .../tls/patches/manager_deployment_cert.yaml | 8 ++- .../tls/resources/manager_cert.yaml | 2 + testdata/build-test-registry.sh | 2 + 8 files changed, 87 insertions(+), 30 deletions(-) diff --git a/cmd/manager/main.go b/cmd/manager/main.go index 38d7534f3..08d33f8bd 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -18,8 +18,10 @@ package main import ( "context" + "crypto/tls" "flag" "fmt" + "log" "net/http" "os" "path/filepath" @@ -41,9 +43,11 @@ import ( "k8s.io/klog/v2/textlogger" ctrl "sigs.k8s.io/controller-runtime" crcache "sigs.k8s.io/controller-runtime/pkg/cache" + "sigs.k8s.io/controller-runtime/pkg/certwatcher" "sigs.k8s.io/controller-runtime/pkg/client" crfinalizer "sigs.k8s.io/controller-runtime/pkg/finalizer" "sigs.k8s.io/controller-runtime/pkg/healthz" + "sigs.k8s.io/controller-runtime/pkg/metrics/filters" "sigs.k8s.io/controller-runtime/pkg/metrics/server" catalogd "github.com/operator-framework/catalogd/api/v1" @@ -70,6 +74,7 @@ import ( var ( setupLog = ctrl.Log.WithName("setup") defaultSystemNamespace = "olmv1-system" + certWatcher *certwatcher.CertWatcher ) const authFilePrefix = "operator-controller-global-pull-secrets" @@ -89,6 +94,8 @@ func podNamespace() string { func main() { var ( metricsAddr string + certFile string + keyFile string enableLeaderElection bool probeAddr string cachePath string @@ -97,9 +104,11 @@ func main() { caCertDir string globalPullSecret string ) - flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "The address the metric endpoint binds to.") + flag.StringVar(&metricsAddr, "metrics-bind-address", "0", "The address for the metrics endpoint. Use :8443 for HTTPS or set to 0 to disable. Requires both tls-cert and tls-key.") 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(&certFile, "tls-cert", "", "The certificate file used for serving metrics contents over HTTPS. Requires tls-key.") + flag.StringVar(&keyFile, "tls-key", "", "The key file used for serving metrics contents over HTTPS. Requires tls-cert.") flag.BoolVar(&enableLeaderElection, "leader-elect", false, "Enable leader election for controller manager. "+ "Enabling this will ensure there is only one active controller manager.") @@ -119,6 +128,11 @@ func main() { os.Exit(0) } + if (certFile != "" && keyFile == "") || (certFile == "" && keyFile != "") { + setupLog.Error(nil, "unable to configure TLS certificates: tls-cert and tls-key flags must be used together") + os.Exit(1) + } + ctrl.SetLogger(textlogger.NewLogger(textlogger.NewConfig())) setupLog.Info("starting up the controller", "version info", version.String()) @@ -161,9 +175,49 @@ func main() { }, } } + + // Force serving be disabled by default if no certs are provided + metricsServerOptions := server.Options{ + BindAddress: "0", + } + + if len(certFile) > 0 && len(keyFile) > 0 { + setupLog.Info("Starting metrics server with TLS enabled.") + + metricsServerOptions.BindAddress = metricsAddr + metricsServerOptions.SecureServing = true + metricsServerOptions.FilterProvider = filters.WithAuthenticationAndAuthorization + + setupLog.Info("Using provided TLS certificate and key files for the metrics server.", + "certFile", certFile, "keyFile", keyFile) + + // If the certificate files change, the watcher will reload them. + var err error + certWatcher, err = certwatcher.New(certFile, keyFile) + if err != nil { + log.Fatalf("Failed to initialize certificate watcher: %v", err) + } + metricsServerOptions.TLSOpts = append(metricsServerOptions.TLSOpts, func(config *tls.Config) { + config.GetCertificate = certWatcher.GetCertificate + + // If the enable-http2 flag is false (the default), http/2 should be disabled + // due to its vulnerabilities. More specifically, disabling http/2 will + // prevent from being vulnerable to the HTTP/2 Stream Cancellation and + // Rapid Reset CVEs. For more information see: + // - https://github.com/advisories/GHSA-qppj-fm5r-hxr3 + // - https://github.com/advisories/GHSA-4374-p667-p6c8 + // Besides, those CVEs are solved already; the solution is still insufficient, and we need to mitigate + // the risks. More info https://github.com/golang/go/issues/63417 + setupLog.Info("disabling http/2") + config.NextProtos = []string{"http/1.1"} + }) + } else { + setupLog.Info("WARNING: Metrics Server will not be serving") + } + mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{ Scheme: scheme.Scheme, - Metrics: server.Options{BindAddress: metricsAddr}, + Metrics: metricsServerOptions, HealthProbeBindAddress: probeAddr, LeaderElection: enableLeaderElection, LeaderElectionID: "9c4404e7.operatorframework.io", @@ -220,6 +274,14 @@ func main() { os.Exit(1) } + if certWatcher != nil { + setupLog.Info("Adding certificate watcher to manager") + if err := mgr.Add(certWatcher); err != nil { + setupLog.Error(err, "unable to add certificate watcher to manager") + os.Exit(1) + } + } + unpacker := &source.ContainersImageRegistry{ BaseCachePath: filepath.Join(cachePath, "unpack"), SourceContextFunc: func(logger logr.Logger) (*types.SystemContext, error) { diff --git a/config/base/manager/manager.yaml b/config/base/manager/manager.yaml index e261c5c3e..51119161f 100644 --- a/config/base/manager/manager.yaml +++ b/config/base/manager/manager.yaml @@ -52,8 +52,12 @@ spec: - /manager args: - "--health-probe-bind-address=:8081" - - "--metrics-bind-address=127.0.0.1:8080" + - "--metrics-bind-address=:8443" - "--leader-elect" + ports: + - containerPort: 8443 + protocol: TCP + name: https image: controller:latest imagePullPolicy: IfNotPresent name: manager @@ -84,27 +88,6 @@ spec: cpu: 10m memory: 64Mi terminationMessagePolicy: FallbackToLogsOnError - - name: kube-rbac-proxy - securityContext: - allowPrivilegeEscalation: false - capabilities: - drop: - - "ALL" - image: gcr.io/kubebuilder/kube-rbac-proxy:v0.15.0 - args: - - --secure-listen-address=0.0.0.0:8443 - - --http2-disable - - --upstream=http://127.0.0.1:8080/ - - --logtostderr=true - ports: - - containerPort: 8443 - protocol: TCP - name: https - resources: - requests: - cpu: 5m - memory: 64Mi - terminationMessagePolicy: FallbackToLogsOnError serviceAccountName: operator-controller-controller-manager terminationGracePeriodSeconds: 10 volumes: diff --git a/config/base/rbac/kustomization.yaml b/config/base/rbac/kustomization.yaml index 33b8765d5..1736277f3 100644 --- a/config/base/rbac/kustomization.yaml +++ b/config/base/rbac/kustomization.yaml @@ -17,10 +17,14 @@ resources: - extension_editor_role.yaml - extension_viewer_role.yaml -# Comment the following 4 lines if you want to disable -# the auth proxy (https://github.com/brancz/kube-rbac-proxy) -# which protects your /metrics endpoint. +# The following RBAC configurations are used to protect +# the metrics endpoint with authn/authz. These configurations +# ensure that only authorized users and service accounts +# can access the metrics endpoint. Comment the following +# permissions if you want to disable this protection. +# More info: https://book.kubebuilder.io/reference/metrics.html - auth_proxy_service.yaml - auth_proxy_role.yaml - auth_proxy_role_binding.yaml - auth_proxy_client_clusterrole.yaml + diff --git a/config/components/coverage/manager_e2e_coverage_patch.yaml b/config/components/coverage/manager_e2e_coverage_patch.yaml index bda011daf..f2be3a19a 100644 --- a/config/components/coverage/manager_e2e_coverage_patch.yaml +++ b/config/components/coverage/manager_e2e_coverage_patch.yaml @@ -7,7 +7,6 @@ spec: template: spec: containers: - - name: kube-rbac-proxy - name: manager env: - name: GOCOVERDIR diff --git a/config/components/registries-conf/manager_e2e_registries_conf_patch.yaml b/config/components/registries-conf/manager_e2e_registries_conf_patch.yaml index 7530f9b08..42012d697 100644 --- a/config/components/registries-conf/manager_e2e_registries_conf_patch.yaml +++ b/config/components/registries-conf/manager_e2e_registries_conf_patch.yaml @@ -7,7 +7,6 @@ spec: template: spec: containers: - - name: kube-rbac-proxy - name: manager volumeMounts: - name: e2e-registries-conf diff --git a/config/components/tls/patches/manager_deployment_cert.yaml b/config/components/tls/patches/manager_deployment_cert.yaml index 747979321..18afac59d 100644 --- a/config/components/tls/patches/manager_deployment_cert.yaml +++ b/config/components/tls/patches/manager_deployment_cert.yaml @@ -1,9 +1,15 @@ - op: add path: /spec/template/spec/volumes/- - value: {"name":"olmv1-certificate", "secret":{"secretName":"olmv1-cert", "optional": false, "items": [{"key": "ca.crt", "path": "olm-ca.crt"}]}} + value: {"name":"olmv1-certificate", "secret":{"secretName":"olmv1-cert", "optional": false, "items": [{"key": "ca.crt", "path": "olm-ca.crt"}, {"key": "tls.crt", "path": "tls.cert"}, {"key": "tls.key", "path": "tls.key"}]}} - op: add path: /spec/template/spec/containers/0/volumeMounts/- value: {"name":"olmv1-certificate", "readOnly": true, "mountPath":"/var/certs/"} - op: add path: /spec/template/spec/containers/0/args/- value: "--ca-certs-dir=/var/certs" +- op: add + path: /spec/template/spec/containers/0/args/- + value: "--tls-cert=/var/certs/tls.cert" +- op: add + path: /spec/template/spec/containers/0/args/- + value: "--tls-key=/var/certs/tls.key" diff --git a/config/components/tls/resources/manager_cert.yaml b/config/components/tls/resources/manager_cert.yaml index a7a19f4dd..2c3ee8b17 100644 --- a/config/components/tls/resources/manager_cert.yaml +++ b/config/components/tls/resources/manager_cert.yaml @@ -7,6 +7,8 @@ spec: dnsNames: - operator-controller.olmv1-system.svc - operator-controller.olmv1-system.svc.cluster.local + - operator-controller-controller-manager-metrics-service.olmv1-system.svc + - operator-controller-controller-manager-metrics-service.olmv1-system.svc.cluster.local privateKey: algorithm: ECDSA size: 256 diff --git a/testdata/build-test-registry.sh b/testdata/build-test-registry.sh index 3ea0e65b0..8b1372021 100755 --- a/testdata/build-test-registry.sh +++ b/testdata/build-test-registry.sh @@ -42,6 +42,8 @@ spec: dnsNames: - ${name}.${namespace}.svc - ${name}.${namespace}.svc.cluster.local + - ${name}-controller-manager-metrics-service.${namespace}.svc + - ${name}-controller-manager-metrics-service.${namespace}.svc.cluster.local privateKey: algorithm: ECDSA size: 256