Bug 1809195: Send CVO metrics over https

CVO metrics are currently being sent insecurely over http. Change
metrics configuration to use https and TLS encryption. CVO will
continue to use the same metrics port and will continue to support
http so a connection mux is setup on the existing port.

Author: jottofar

cmd/start.go

@@ -30,5 +30,7 @@ func init() {
 	cmd.PersistentFlags().BoolVar(&opts.EnableAutoUpdate, "enable-auto-update", opts.EnableAutoUpdate, "Enables the autoupdate controller.")
 	cmd.PersistentFlags().BoolVar(&opts.EnableDefaultClusterVersion, "enable-default-cluster-version", opts.EnableDefaultClusterVersion, "Allows the operator to create a ClusterVersion object if one does not already exist.")
 	cmd.PersistentFlags().StringVar(&opts.ReleaseImage, "release-image", opts.ReleaseImage, "The Openshift release image url.")
+	cmd.PersistentFlags().StringVar(&opts.ServingCertFile, "serving-cert-file", opts.ServingCertFile, "The X.509 certificate file for serving metrics over HTTPS.  You must set both --serving-cert-file and --serving-key-file, or neither.")
+	cmd.PersistentFlags().StringVar(&opts.ServingKeyFile, "serving-key-file", opts.ServingKeyFile, "The X.509 key file for serving metrics over HTTPS.  You must set both --serving-cert-file and --serving-key-file, or neither.")
 	rootCmd.AddCommand(cmd)
 }

install/0000_00_cluster-version-operator_03_deployment.yaml

@@ -26,6 +26,8 @@ spec:
           - "--release-image={{.ReleaseImage}}"
           - "--enable-auto-update=false"
           - "--enable-default-cluster-version=true"
+          - "--serving-cert-file=/etc/tls/serving-cert/tls.crt"
+          - "--serving-key-file=/etc/tls/serving-cert/tls.key"
           - "--v=4"
         resources:
           requests:
@@ -39,6 +41,9 @@ spec:
           - mountPath: /etc/cvo/updatepayloads
             name: etc-cvo-updatepayloads
             readOnly: true
+          - mountPath: /etc/tls/serving-cert
+            name: serving-cert
+            readOnly: true
         env:
           - name: KUBERNETES_SERVICE_PORT # allows CVO to communicate with apiserver directly on same host.
             value: "6443"
@@ -80,3 +85,6 @@ spec:
         - name: etc-cvo-updatepayloads
           hostPath:
             path: /etc/cvo/updatepayloads
+        - name: serving-cert
+          secret:
+            secretName: cluster-version-operator-serving-cert

install/0000_90_cluster-version-operator_02_servicemonitor.yaml

@@ -9,9 +9,13 @@ metadata:
     exclude.release.openshift.io/internal-openshift-hosted: "true"
 spec:
   endpoints:
+  - bearerTokenFile: /var/run/secrets/kubernetes.io/serviceaccount/token
   - interval: 30s
     port: metrics
-    scheme: http
+    scheme: https
+    tlsConfig:
+      caFile: /etc/prometheus/configmaps/serving-certs-ca-bundle/service-ca.crt
+      serverName: cluster-version-operator.openshift-cluster-version.svc
   namespaceSelector:
     matchNames:
     - openshift-cluster-version

pkg/start/start.go

@@ -4,15 +4,20 @@ package start
 
 import (
 	"context"
+	"crypto/tls"
 	"fmt"
+	"io/ioutil"
 	"math/rand"
+	"net"
 	"net/http"
 	"os"
 	"os/signal"
 	"sync"
 	"syscall"
 	"time"
 
+	"github.com/cockroachdb/cmux"
+
 	"github.com/google/uuid"
 	"github.com/prometheus/client_golang/prometheus/promhttp"
 	v1 "k8s.io/api/core/v1"
@@ -34,6 +39,7 @@ import (
 	"github.com/openshift/cluster-version-operator/pkg/autoupdate"
 	"github.com/openshift/cluster-version-operator/pkg/cvo"
 	"github.com/openshift/cluster-version-operator/pkg/internal"
+	"github.com/openshift/library-go/pkg/crypto"
 )
 
 const (
@@ -49,7 +55,9 @@ const (
 
 // Options are the valid inputs to starting the CVO.
 type Options struct {
-	ReleaseImage string
+	ReleaseImage    string
+	ServingCertFile string
+	ServingKeyFile  string
 
 	Kubeconfig string
 	NodeName   string
@@ -103,6 +111,12 @@ func (o *Options) Run() error {
 	if o.ReleaseImage == "" {
 		return fmt.Errorf("missing --release-image flag, it is required")
 	}
+	if o.ServingCertFile == "" && o.ServingKeyFile != "" {
+		return fmt.Errorf("--serving-key-file was set, so --serving-cert-file must also be set")
+	}
+	if o.ServingKeyFile == "" && o.ServingCertFile != "" {
+		return fmt.Errorf("--serving-cert-file was set, so --serving-key-file must also be set")
+	}
 	if len(o.PayloadOverride) > 0 {
 		klog.Warningf("Using an override payload directory for testing only: %s", o.PayloadOverride)
 	}
@@ -153,16 +167,79 @@ func (o *Options) Run() error {
 	return nil
 }
 
+func (o *Options) makeTLSConfig() (*tls.Config, error) {
+	// Load the initial certificate contents.
+	certBytes, err := ioutil.ReadFile(o.ServingCertFile)
+	if err != nil {
+		return nil, err
+	}
+	keyBytes, err := ioutil.ReadFile(o.ServingKeyFile)
+	if err != nil {
+		return nil, err
+	}
+	certificate, err := tls.X509KeyPair(certBytes, keyBytes)
+	if err != nil {
+		return nil, err
+	}
+
+	return crypto.SecureTLSConfig(&tls.Config{
+		GetCertificate: func(_ *tls.ClientHelloInfo) (*tls.Certificate, error) {
+			return &certificate, nil
+		},
+	}), nil
+}
+
 func (o *Options) run(ctx context.Context, controllerCtx *Context, lock *resourcelock.ConfigMapLock) {
 	// listen on metrics
 	if len(o.ListenAddr) > 0 {
-		mux := http.NewServeMux()
-		mux.Handle("/metrics", promhttp.Handler())
+		handler := http.NewServeMux()
+		handler.Handle("/metrics", promhttp.Handler())
+		tcpl, err := net.Listen("tcp", o.ListenAddr)
+		if err != nil {
+			klog.Fatalf("Listen error: %v", err)
+		}
+
+		// if a TLS connection was requested, set up a connection mux that will send TLS requests to
+		// the TLS server but send HTTP requests to the HTTP server. Preserves the ability for legacy
+		// HTTP, needed during upgrade, while still allowing TLS certs and end to end metrics protection.
+		m := cmux.New(tcpl)
+
+		// match HTTP first
+		httpl := m.Match(cmux.HTTP1())
 		go func() {
-			if err := http.ListenAndServe(o.ListenAddr, mux); err != nil {
-				klog.Fatalf("Unable to start metrics server: %v", err)
+			s := &http.Server{
+				Handler: handler,
+			}
+			if err := s.Serve(httpl); err != cmux.ErrListenerClosed {
+				klog.Fatalf("HTTP serve error: %v", err)
 			}
 		}()
+
+		if o.ServingCertFile != "" || o.ServingKeyFile != "" {
+			tlsConfig, err := o.makeTLSConfig()
+			if err != nil {
+				klog.Fatalf("Failed to create TLS config: %v", err)
+			}
+
+			tlsListener := tls.NewListener(m.Match(cmux.Any()), tlsConfig)
+			klog.Infof("Metrics port listening for HTTP and HTTPS on %v", o.ListenAddr)
+			go func() {
+				s := &http.Server{
+					Handler: handler,
+				}
+				if err := s.Serve(tlsListener); err != cmux.ErrListenerClosed {
+					klog.Fatalf("HTTPS serve error: %v", err)
+				}
+			}()
+
+			go func() {
+				if err := m.Serve(); err != nil {
+					klog.Errorf("CMUX serve error: %v", err)
+				}
+			}()
+		} else {
+			klog.Infof("Metrics port listening for HTTP on %v", o.ListenAddr)
+		}
 	}
 
 	exit := make(chan struct{})