Merge pull request #1151 from jpeeler/cert-rotation

bug 1771811: make certificate updates live upon update

Author: openshift-merge-robot

cmd/catalog/main.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"context"
+	"crypto/tls"
 	"flag"
 	"fmt"
 	"net/http"
@@ -18,6 +19,7 @@ import (
 
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/api/client"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/catalog"
+	"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/filemonitor"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/operatorclient"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/operatorstatus"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/profile"
@@ -138,8 +140,20 @@ func main() {
 	metricsMux := http.NewServeMux()
 	metricsMux.Handle("/metrics", promhttp.Handler())
 	if useTLS {
+		tlsGetCertFn, err := filemonitor.OLMGetCertRotationFn(logger, *tlsCertPath, *tlsKeyPath)
+		if err != nil {
+			logger.Errorf("Certificate monitoring for metrics (https) failed: %v", err)
+		}
+
 		go func() {
-			err := http.ListenAndServeTLS(":8081", *tlsCertPath, *tlsKeyPath, metricsMux)
+			httpsServer := &http.Server{
+				Addr:    ":8081",
+				Handler: metricsMux,
+				TLSConfig: &tls.Config{
+					GetCertificate: tlsGetCertFn,
+				},
+			}
+			err := httpsServer.ListenAndServeTLS("", "")
 			if err != nil {
 				logger.Errorf("Metrics (https) serving failed: %v", err)
 			}

cmd/olm/main.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"context"
+	"crypto/tls"
 	"flag"
 	"fmt"
 	"net/http"
@@ -18,6 +19,7 @@ import (
 
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/api/client"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/olm"
+	"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/filemonitor"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/operatorclient"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/operatorstatus"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/profile"
@@ -139,8 +141,20 @@ func main() {
 	metricsMux := http.NewServeMux()
 	metricsMux.Handle("/metrics", promhttp.Handler())
 	if useTLS {
+		tlsGetCertFn, err := filemonitor.OLMGetCertRotationFn(logger, *tlsCertPath, *tlsKeyPath)
+		if err != nil {
+			logger.Errorf("Certificate monitoring for metrics (https) failed: %v", err)
+		}
+
 		go func() {
-			err := http.ListenAndServeTLS(":8081", *tlsCertPath, *tlsKeyPath, metricsMux)
+			httpsServer := &http.Server{
+				Addr:    ":8081",
+				Handler: metricsMux,
+				TLSConfig: &tls.Config{
+					GetCertificate: tlsGetCertFn,
+				},
+			}
+			err := httpsServer.ListenAndServeTLS("", "")
 			if err != nil {
 				logger.Errorf("Metrics (https) serving failed: %v", err)
 			}

go.mod

@@ -10,6 +10,7 @@ require (
 	github.com/coreos/pkg v0.0.0-20180928190104-399ea9e2e55f // indirect
 	github.com/emicklei/go-restful v2.9.6+incompatible // indirect
 	github.com/evanphx/json-patch v4.5.0+incompatible // indirect
+	github.com/fsnotify/fsnotify v1.4.7
 	github.com/ghodss/yaml v1.0.0
 	github.com/go-openapi/spec v0.19.2
 	github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b

go.sum

@@ -424,8 +424,6 @@ github.com/openshift/client-go v0.0.0-20190923180330-3b6373338c9b h1:E++qQ7W1/Ed
 github.com/openshift/client-go v0.0.0-20190923180330-3b6373338c9b/go.mod h1:6rzn+JTr7+WYS2E1TExP4gByoABxMznR6y2SnUIkmxk=
 github.com/opentracing/opentracing-go v1.1.0/go.mod h1:UkNAQd3GIcIGf0SeVgPpRdFStlNbqXla1AfSYxPUl2o=
 github.com/openzipkin/zipkin-go v0.1.6/go.mod h1:QgAqvLzwWbR/WpD4A3cGpPtJrZXNIiJc5AZX7/PBEpw=
-github.com/operator-framework/operator-registry v1.4.0 h1:yhI7ZeF6d7G588i2egoFjgGi1eXhaU1aL0GMEMDuS/4=
-github.com/operator-framework/operator-registry v1.4.0/go.mod h1:3nJcOSX6TKPpAbmGOAVvDxakZWgccyF0WgiJsKjPAzQ=
 github.com/operator-framework/operator-registry v1.5.1 h1:8ruUOG6IBDVTAXYWKsv6hwr4yv/0SFPFPAYGCpcv97E=
 github.com/operator-framework/operator-registry v1.5.1/go.mod h1:agrQlkWOo1q8U1SAaLSS2WQ+Z9vswNT2M2HFib9iuLY=
 github.com/otiai10/copy v1.0.1 h1:gtBjD8aq4nychvRZ2CyJvFWAw0aja+VHazDdruZKGZA=
@@ -461,15 +459,11 @@ github.com/prometheus/client_model v0.0.0-20190115171406-56726106282f h1:BVwpUVJ
 github.com/prometheus/client_model v0.0.0-20190115171406-56726106282f/go.mod h1:MbSGuTsp3dbXC40dX6PRTWyKYBIrTGTE9sqQNg2J8bo=
 github.com/prometheus/common v0.0.0-20181126121408-4724e9255275 h1:PnBWHBf+6L0jOqq0gIVUe6Yk0/QMZ640k6NvkxcBf+8=
 github.com/prometheus/common v0.0.0-20181126121408-4724e9255275/go.mod h1:daVV7qP5qjZbuso7PdcryaAu0sAZbrN9i7WWcTMWvro=
-github.com/prometheus/common v0.0.0-20190104105734-b1c43a6df3ae h1:iq3e1tH4dCzdqscIkWimcnzYt6Pkz0zOzHSgV9cb5DE=
-github.com/prometheus/common v0.0.0-20190104105734-b1c43a6df3ae/go.mod h1:TNfzLD0ON7rHzMJeJkieUDPYmFC7Snx/y86RQel1bk4=
 github.com/prometheus/common v0.2.0 h1:kUZDBDTdBVBYBj5Tmh2NZLlF60mfjA27rM34b+cVwNU=
 github.com/prometheus/common v0.2.0/go.mod h1:TNfzLD0ON7rHzMJeJkieUDPYmFC7Snx/y86RQel1bk4=
 github.com/prometheus/procfs v0.0.0-20181005140218-185b4288413d/go.mod h1:c3At6R/oaqEKCNdg8wHV1ftS6bRYblBhIjjI8uT2IGk=
 github.com/prometheus/procfs v0.0.0-20181204211112-1dc9a6cbc91a h1:9a8MnZMP0X2nLJdBg+pBmGgkJlSaKC2KaQmTCk1XDtE=
 github.com/prometheus/procfs v0.0.0-20181204211112-1dc9a6cbc91a/go.mod h1:c3At6R/oaqEKCNdg8wHV1ftS6bRYblBhIjjI8uT2IGk=
-github.com/prometheus/procfs v0.0.0-20190104112138-b1a0a9a36d74 h1:d1Xoc24yp/pXmWl2leBiBA+Tptce6cQsA+MMx/nOOcY=
-github.com/prometheus/procfs v0.0.0-20190104112138-b1a0a9a36d74/go.mod h1:c3At6R/oaqEKCNdg8wHV1ftS6bRYblBhIjjI8uT2IGk=
 github.com/prometheus/procfs v0.0.0-20190117184657-bf6a532e95b1 h1:/K3IL0Z1quvmJ7X0A1AwNEK7CRkVK3YwfOU/QAL4WGg=
 github.com/prometheus/procfs v0.0.0-20190117184657-bf6a532e95b1/go.mod h1:c3At6R/oaqEKCNdg8wHV1ftS6bRYblBhIjjI8uT2IGk=
 github.com/quobyte/api v0.1.2/go.mod h1:jL7lIHrmqQ7yh05OJ+eEEdHr0u/kmT1Ff9iHd+4H6VI=

pkg/lib/filemonitor/cert_updater.go

@@ -0,0 +1,90 @@
+package filemonitor
+
+import (
+	"context"
+	"crypto/tls"
+	"crypto/x509"
+	"fmt"
+	"path/filepath"
+	"sync"
+
+	"github.com/fsnotify/fsnotify"
+	"github.com/sirupsen/logrus"
+)
+
+type keystore struct {
+	mutex      sync.RWMutex
+	cert       *tls.Certificate
+	tlsCrtPath string
+	tlsKeyPath string
+}
+
+type getCertFn = func(*tls.ClientHelloInfo) (*tls.Certificate, error)
+
+// NewKeystore returns a store for storing the certificate data and the ability to retrieve it safely
+func NewKeystore(tlsCrt, tlsKey string) *keystore {
+	cert, err := tls.LoadX509KeyPair(tlsCrt, tlsKey)
+	if err != nil {
+		panic(err)
+	}
+	return &keystore{
+		mutex:      sync.RWMutex{},
+		cert:       &cert,
+		tlsCrtPath: tlsCrt,
+		tlsKeyPath: tlsKey,
+	}
+}
+
+// HandleFilesystemUpdate is intended to be used as the OnUpdateFn for a watcher
+// and expects the certificate files to be in the same directory.
+func (k *keystore) HandleFilesystemUpdate(logger *logrus.Logger, event fsnotify.Event) {
+	switch op := event.Op; op {
+	case fsnotify.Create:
+		logger.Debugf("got fs event for %v", event.Name)
+
+		if err := k.storeCertificate(k.tlsCrtPath, k.tlsKeyPath); err != nil {
+			// this can happen if both certificates aren't updated at the same
+			// time, but it's okay as replacement only occurs with a valid key pair
+			logger.Debugf("certificates not in sync: %v", err)
+		} else {
+			info, err := x509.ParseCertificate(k.cert.Certificate[0])
+			if err != nil {
+				logger.Debugf("certificates refreshed, but parsing returned error: %v", err)
+			} else {
+				logger.Debugf("certificates refreshed: Subject=%v NotBefore=%v NotAfter=%v", info.Subject, info.NotBefore, info.NotAfter)
+			}
+		}
+	}
+}
+
+func (k *keystore) storeCertificate(tlsCrt, tlsKey string) error {
+	cert, err := tls.LoadX509KeyPair(tlsCrt, tlsKey)
+	if err == nil {
+		k.mutex.Lock()
+		defer k.mutex.Unlock()
+		k.cert = &cert
+	}
+	return err
+}
+
+func (k *keystore) GetCertificate(h *tls.ClientHelloInfo) (*tls.Certificate, error) {
+	k.mutex.RLock()
+	defer k.mutex.RUnlock()
+	return k.cert, nil
+}
+
+// OLMGetCertRotationFn is a convenience function for OLM use only, but serves as an example for monitoring file system events
+func OLMGetCertRotationFn(logger *logrus.Logger, tlsCertPath, tlsKeyPath string) (getCertFn, error) {
+	if filepath.Dir(tlsCertPath) != filepath.Dir(tlsKeyPath) {
+		return nil, fmt.Errorf("certificates expected to be in same directory %v vs %v", tlsCertPath, tlsKeyPath)
+	}
+
+	keystore := NewKeystore(tlsCertPath, tlsKeyPath)
+	watcher, err := NewWatch(logger, []string{filepath.Dir(tlsCertPath)}, keystore.HandleFilesystemUpdate)
+	if err != nil {
+		return nil, err
+	}
+	watcher.Run(context.Background())
+
+	return keystore.GetCertificate, nil
+}

pkg/lib/filemonitor/cert_updater_test.go

@@ -0,0 +1,135 @@
+package filemonitor
+
+import (
+	"crypto/tls"
+	"crypto/x509"
+	"fmt"
+	"html"
+	"io/ioutil"
+	"net"
+	"net/http"
+	"os"
+	"path/filepath"
+	"strconv"
+	"testing"
+	"time"
+
+	"k8s.io/apimachinery/pkg/util/wait"
+
+	"github.com/sirupsen/logrus"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestOLMGetCertRotationFn(t *testing.T) {
+	logger := logrus.New()
+	logger.SetLevel(logrus.DebugLevel)
+	logger.SetFormatter(&logrus.TextFormatter{
+		TimestampFormat: time.RFC3339Nano,
+	})
+
+	testData := "testdata"
+	monitorDir := "monitor"
+	caCrt := filepath.Join(testData, "ca.crt")
+	oldCrt := filepath.Join(testData, "server-old.crt")
+	oldKey := filepath.Join(testData, "server-old.key")
+	newCrt := filepath.Join(testData, "server-new.crt")
+	newKey := filepath.Join(testData, "server-new.key")
+	loadCrt := filepath.Join(monitorDir, "loaded.crt")
+	loadKey := filepath.Join(monitorDir, "loaded.key")
+
+	// these values must match values specified in the testdata generation script
+	expectedOldCN := "CN=127.0.0.1,OU=OpenShift,O=Red Hat,L=Columbia,ST=SC,C=US"
+	expectedNewCN := "CN=127.0.0.1,OU=OpenShift,O=Red Hat,L=New York City,ST=NY,C=US"
+
+	// the directory is expected to contain exactly one keypair, so create an empty directory to swap the keys in
+	err := os.RemoveAll(monitorDir) // this is for test development, shouldn't ever exist beforehand otherwise
+	require.NoError(t, err)
+	err = os.Mkdir(monitorDir, 0777)
+	require.NoError(t, err)
+
+	// symlink old files to loading files
+	err = os.Symlink(filepath.Join("..", oldCrt), loadCrt)
+	require.NoError(t, err)
+	err = os.Symlink(filepath.Join("..", oldKey), loadKey)
+	require.NoError(t, err)
+
+	tlsGetCertFn, err := OLMGetCertRotationFn(logger, loadCrt, loadKey)
+	require.NoError(t, err)
+
+	// find a free port to listen on and start server
+	listener, err := net.Listen("tcp", ":0")
+	require.NoError(t, err)
+	freePort := listener.Addr().(*net.TCPAddr).Port
+	http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
+		fmt.Fprintf(w, "Path: %q", html.EscapeString(r.URL.Path))
+	})
+	httpsServer := &http.Server{
+		Addr: ":" + strconv.Itoa(freePort),
+		TLSConfig: &tls.Config{
+			GetCertificate: tlsGetCertFn,
+		},
+	}
+	go func() {
+		if err := httpsServer.ServeTLS(listener, "", ""); err != nil {
+			panic(err)
+		}
+	}()
+
+	caCert, err := ioutil.ReadFile(caCrt)
+	require.NoError(t, err)
+	caCertPool := x509.NewCertPool()
+	caCertPool.AppendCertsFromPEM(caCert)
+
+	client := &http.Client{
+		Transport: &http.Transport{
+			TLSClientConfig: &tls.Config{
+				RootCAs: caCertPool,
+			},
+		},
+	}
+
+	resp, err := client.Get(fmt.Sprintf("https://localhost:%v", freePort))
+	require.NoError(t, err)
+	assert.Equal(t, resp.StatusCode, http.StatusOK)
+	assert.Equal(t, expectedOldCN, resp.TLS.PeerCertificates[0].Subject.String())
+	resp.Body.Close()
+	client.CloseIdleConnections()
+
+	// atomically switch out the symlink so the file contents are always seen in a consistent state
+	// (the same idea is used in the atomic writer in kubernetes)
+	atomicCrt := loadCrt + ".atomic-op"
+	atomicKey := loadKey + ".atomic-op"
+	err = os.Symlink(filepath.Join("..", newCrt), atomicCrt)
+	require.NoError(t, err)
+	err = os.Symlink(filepath.Join("..", newKey), atomicKey)
+	require.NoError(t, err)
+
+	err = os.Rename(atomicCrt, loadCrt)
+	require.NoError(t, err)
+	err = os.Rename(atomicKey, loadKey)
+	require.NoError(t, err)
+
+	// sometimes the the filesystem operations need time to catch up so the server cert is updated
+	err = wait.PollImmediate(500*time.Millisecond, 10*time.Second, func() (bool, error) {
+		currentCert, _ := tlsGetCertFn(nil)
+		info, err := x509.ParseCertificate(currentCert.Certificate[0])
+		if err != nil {
+			return false, err
+		}
+		if info.Subject.String() == expectedNewCN {
+			return true, nil
+		}
+
+		return false, nil
+	})
+	require.NoError(t, err)
+
+	resp, err = client.Get(fmt.Sprintf("https://localhost:%v", freePort))
+	require.NoError(t, err)
+	assert.Equal(t, resp.StatusCode, http.StatusOK)
+	assert.Equal(t, expectedNewCN, resp.TLS.PeerCertificates[0].Subject.String())
+
+	os.RemoveAll(monitorDir)
+}

pkg/lib/filemonitor/testdata/ca.crt

@@ -0,0 +1,19 @@
+-----BEGIN CERTIFICATE-----
+MIIDCTCCAfGgAwIBAgIUZSqEGDsDVte9dO9YVGNd3n9/EakwDQYJKoZIhvcNAQEL
+BQAwFDESMBAGA1UEAwwJMTI3LjAuMC4xMB4XDTE5MTEyMzAyMDM1OFoXDTQ3MDQx
+MDAyMDM1OFowFDESMBAGA1UEAwwJMTI3LjAuMC4xMIIBIjANBgkqhkiG9w0BAQEF
+AAOCAQ8AMIIBCgKCAQEA4Gwb6+HM2IJn/ZZIERVA/ngKQzk+Lk+36Yrn1ka8HOum
+vmWhozle3FwczCOQks5Jc2v5W6ulZHg0uQtNutw3lWkinVqGZzI34E0bxKchPniy
+egGnW3n9/dgIVd9ooPnOLJJXEwV4ZVIhubfB/EHib9rha2nIKVyYxihhrkCMU8aq
+qhYqOzJYk9eWzwB+mXAxW5AMmUAmVF0J9JvpG/FHliG+WgJNrxunuGuIa4XLw50H
+V5bJ5pXUdX/L0EPTQ1KWpmjByFD/dcOmFGc+pdw+x84CYUAH2DkxewUHAe0e6hlO
+RY3tuOXEigtfU6F0jUxD2RjC2pBp1WoH+UyyUiRQTQIDAQABo1MwUTAdBgNVHQ4E
+FgQU0jPw9F7SBqdbDbws33KFGPqKqb8wHwYDVR0jBBgwFoAU0jPw9F7SBqdbDbws
+33KFGPqKqb8wDwYDVR0TAQH/BAUwAwEB/zANBgkqhkiG9w0BAQsFAAOCAQEAjDnW
+CiQ8X+Ncm09bF6SXjtTzwbSduJ7VEkVjVm+nW0/Rh56Lgw0Z/ceWy/GyqYzuhsUz
+6uQOrS3drlg+p2fn4ZqtKWmDByYaUQS709I/qDkBE9mXxjojOhnQEqgjdQ5yoIP7
+mXeJQlFlrSaf4yEW264XQjbasKGQi2QBejhHK+aHw6PsszaNxqeqvTr0K95OTPPv
+vl2L0A4grMKtXnws8LL6zazz9eqib0K9iyHOLxvtWnPjc1y3XmFPr8mjWmbc9pl3
+5UGohvZy/wBjPS0YPLMDoGGu0jajvbNqcoEMFJ6Zo6oPGJIpwpqCjgLAgwMpx9oL
+EhMnEidqcop3+Z28kA==
+-----END CERTIFICATE-----

pkg/lib/filemonitor/testdata/cert-config/Makefile

@@ -0,0 +1,14 @@
+DESTINATION:=../
+
+all:
+	./gen-certs.sh
+
+clean:
+	rm -f *.csr *.crt *.key *.srl
+
+copy:
+	cp ca.crt $(DESTINATION)
+	cp server-old.crt $(DESTINATION)
+	cp server-old.key $(DESTINATION)
+	cp server-new.crt $(DESTINATION)
+	cp server-new.key $(DESTINATION)

pkg/lib/filemonitor/testdata/cert-config/csr-new.conf

@@ -0,0 +1,28 @@
+[ req ]
+default_bits = 2048
+prompt = no
+default_md = sha256
+req_extensions = req_ext
+distinguished_name = dn
+
+[ dn ]
+C = US
+ST = NY
+L = New York City
+O = Red Hat
+OU = OpenShift
+CN = 127.0.0.1
+
+[ req_ext ]
+subjectAltName = @alt_names
+
+[ alt_names ]
+DNS.1 = localhost
+IP.1 = 127.0.0.1
+
+[ v3_ext ]
+authorityKeyIdentifier=keyid,issuer:always
+basicConstraints=CA:FALSE
+keyUsage=keyEncipherment,dataEncipherment
+extendedKeyUsage=serverAuth,clientAuth
+subjectAltName=@alt_names