Repository: minio/minio (language: go)

Issue:
Reload of KES mTLS client certificates without restart
Minio watches its TLS server certificate and key files and automatically reloads them if they change, but it does not appear to do the same with the _client_ certificate used when communicating with KES.  With short-lived client certificates the minio server process will often outlive the client certificate lifetime, but a renewed client certificate requires a restart of minio.

The logic to watch and reload client certificates using `certs.Manager` is already in place for webhooks, could the same be applied for KES?  Currently KES client certs are loaded once up front in `handleCommonEnvVars`.



Discussion / hints:
This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 15 days if no further activity occurs. Thank you for your contributions.

Fix patch:
diff --git a/cmd/common-main.go b/cmd/common-main.go
index 8f78c824c7f8c..40fe8e3672bc6 100644
--- a/cmd/common-main.go
+++ b/cmd/common-main.go
@@ -830,43 +830,56 @@ func handleKMSConfig() {
 				endpoints = append(endpoints, strings.Join(lbls, ""))
 			}
 		}
-		// Manually load the certificate and private key into memory.
-		// We need to check whether the private key is encrypted, and
-		// if so, decrypt it using the user-provided password.
-		certBytes, err := os.ReadFile(env.Get(config.EnvKESClientCert, ""))
-		if err != nil {
-			logger.Fatal(err, "Unable to load KES client certificate as specified by the shell environment")
-		}
-		keyBytes, err := os.ReadFile(env.Get(config.EnvKESClientKey, ""))
+		rootCAs, err := certs.GetRootCAs(env.Get(config.EnvKESServerCA, globalCertsCADir.Get()))
 		if err != nil {
-			logger.Fatal(err, "Unable to load KES client private key as specified by the shell environment")
-		}
-		privateKeyPEM, rest := pem.Decode(bytes.TrimSpace(keyBytes))
-		if len(rest) != 0 {
-			logger.Fatal(errors.New("private key contains additional data"), "Unable to load KES client private key as specified by the shell environment")
+			logger.Fatal(err, fmt.Sprintf("Unable to load X.509 root CAs for KES from %q", env.Get(config.EnvKESServerCA, globalCertsCADir.Get())))
 		}
-		if x509.IsEncryptedPEMBlock(privateKeyPEM) {
-			keyBytes, err = x509.DecryptPEMBlock(privateKeyPEM, []byte(env.Get(config.EnvKESClientPassword, "")))
+
+		loadX509KeyPair := func(certFile, keyFile string) (tls.Certificate, error) {
+			// Manually load the certificate and private key into memory.
+			// We need to check whether the private key is encrypted, and
+			// if so, decrypt it using the user-provided password.
+			certBytes, err := os.ReadFile(certFile)
 			if err != nil {
-				logger.Fatal(err, "Unable to decrypt KES client private key as specified by the shell environment")
+				return tls.Certificate{}, fmt.Errorf("Unable to load KES client certificate as specified by the shell environment: %v", err)
 			}
-			keyBytes = pem.EncodeToMemory(&pem.Block{Type: privateKeyPEM.Type, Bytes: keyBytes})
-		}
-		certificate, err := tls.X509KeyPair(certBytes, keyBytes)
-		if err != nil {
-			logger.Fatal(err, "Unable to load KES client certificate as specified by the shell environment")
+			keyBytes, err := os.ReadFile(keyFile)
+			if err != nil {
+				return tls.Certificate{}, fmt.Errorf("Unable to load KES client private key as specified by the shell environment: %v", err)
+			}
+			privateKeyPEM, rest := pem.Decode(bytes.TrimSpace(keyBytes))
+			if len(rest) != 0 {
+				return tls.Certificate{}, errors.New("Unable to load KES client private key as specified by the shell environment: private key contains additional data")
+			}
+			if x509.IsEncryptedPEMBlock(privateKeyPEM) {
+				keyBytes, err = x509.DecryptPEMBlock(privateKeyPEM, []byte(env.Get(config.EnvKESClientPassword, "")))
+				if err != nil {
+					return tls.Certificate{}, fmt.Errorf("Unable to decrypt KES client private key as specified by the shell environment: %v", err)
+				}
+				keyBytes = pem.EncodeToMemory(&pem.Block{Type: privateKeyPEM.Type, Bytes: keyBytes})
+			}
+			certificate, err := tls.X509KeyPair(certBytes, keyBytes)
+			if err != nil {
+				return tls.Certificate{}, fmt.Errorf("Unable to load KES client certificate as specified by the shell environment: %v", err)
+			}
+			return certificate, nil
 		}
-		rootCAs, err := certs.GetRootCAs(env.Get(config.EnvKESServerCA, globalCertsCADir.Get()))
+
+		reloadCertEvents := make(chan tls.Certificate, 1)
+		certificate, err := certs.NewCertificate(env.Get(config.EnvKESClientCert, ""), env.Get(config.EnvKESClientKey, ""), loadX509KeyPair)
 		if err != nil {
-			logger.Fatal(err, fmt.Sprintf("Unable to load X.509 root CAs for KES from %q", env.Get(config.EnvKESServerCA, globalCertsCADir.Get())))
+			logger.Fatal(err, "Failed to load KES client certificate")
 		}
+		certificate.Watch(context.Background(), 15*time.Minute, syscall.SIGHUP)
+		certificate.Notify(reloadCertEvents)
 
 		defaultKeyID := env.Get(config.EnvKESKeyName, "")
 		KMS, err := kms.NewWithConfig(kms.Config{
-			Endpoints:    endpoints,
-			DefaultKeyID: defaultKeyID,
-			Certificate:  certificate,
-			RootCAs:      rootCAs,
+			Endpoints:        endpoints,
+			DefaultKeyID:     defaultKeyID,
+			Certificate:      certificate,
+			ReloadCertEvents: reloadCertEvents,
+			RootCAs:          rootCAs,
 		})
 		if err != nil {
 			logger.Fatal(err, "Unable to initialize a connection to KES as specified by the shell environment")
diff --git a/go.mod b/go.mod
index 4ee178a8a43d3..8872f943661a4 100644
--- a/go.mod
+++ b/go.mod
@@ -50,7 +50,7 @@ require (
 	github.com/minio/kes v0.20.0
 	github.com/minio/madmin-go v1.4.9
 	github.com/minio/minio-go/v7 v7.0.33
-	github.com/minio/pkg v1.1.26
+	github.com/minio/pkg v1.3.0
 	github.com/minio/selfupdate v0.5.0
 	github.com/minio/sha256-simd v1.0.0
 	github.com/minio/simdjson-go v0.4.2
diff --git a/go.sum b/go.sum
index 15b90b737f879..8d423f52a0bb1 100644
--- a/go.sum
+++ b/go.sum
@@ -635,6 +635,8 @@ github.com/minio/minio-go/v7 v7.0.33/go.mod h1:nCrRzjoSUQh8hgKKtu3Y708OLvRLtuASM
 github.com/minio/pkg v1.1.20/go.mod h1:Xo7LQshlxGa9shKwJ7NzQbgW4s8T/Wc1cOStR/eUiMY=
 github.com/minio/pkg v1.1.26 h1:a8x4sHNBxCiHEkxZ/0EBTLqvV3nMtM2G/A6lXNfXN3U=
 github.com/minio/pkg v1.1.26/go.mod h1:z9PfmEI804KFkF6eY4LoGe8IDVvTCsYGVuaf58Dr0WI=
+github.com/minio/pkg v1.3.0 h1:myG72OiSi1dMN5kTrdfffzC1VHQaoRwC6jefyH3VGrU=
+github.com/minio/pkg v1.3.0/go.mod h1:z9PfmEI804KFkF6eY4LoGe8IDVvTCsYGVuaf58Dr0WI=
 github.com/minio/selfupdate v0.5.0 h1:0UH1HlL49+2XByhovKl5FpYTjKfvrQ2sgL1zEXK6mfI=
 github.com/minio/selfupdate v0.5.0/go.mod h1:mcDkzMgq8PRcpCRJo/NlPY7U45O5dfYl2Y0Rg7IustY=
 github.com/minio/sha256-simd v0.1.1/go.mod h1:B5e1o+1/KgNmWrSQK08Y6Z1Vb5pwIktudl0J58iy0KM=
diff --git a/internal/kms/kes.go b/internal/kms/kes.go
index fa2d8cdc266ff..1ded34ca2b5a5 100644
--- a/internal/kms/kes.go
+++ b/internal/kms/kes.go
@@ -23,8 +23,10 @@ import (
 	"crypto/x509"
 	"errors"
 	"strings"
+	"sync"
 
 	"github.com/minio/kes"
+	"github.com/minio/pkg/certs"
 )
 
 const (
@@ -46,7 +48,11 @@ type Config struct {
 
 	// Certificate is the client TLS certificate
 	// to authenticate to KMS via mTLS.
-	Certificate tls.Certificate
+	Certificate *certs.Certificate
+
+	// ReloadCertEvents is an event channel that receives
+	// the reloaded client certificate.
+	ReloadCertEvents <-chan tls.Certificate
 
 	// RootCAs is a set of root CA certificates
 	// to verify the KMS server TLS certificate.
@@ -64,7 +70,7 @@ func NewWithConfig(config Config) (KMS, error) {
 
 	client := kes.NewClientWithConfig("", &tls.Config{
 		MinVersion:         tls.VersionTLS12,
-		Certificates:       []tls.Certificate{config.Certificate},
+		Certificates:       []tls.Certificate{config.Certificate.Get()},
 		RootCAs:            config.RootCAs,
 		ClientSessionCache: tls.NewLRUClientSessionCache(tlsClientSessionCacheSize),
 	})
@@ -81,14 +87,35 @@ func NewWithConfig(config Config) (KMS, error) {
 			}
 		}
 	}
-	return &kesClient{
+
+	c := &kesClient{
 		client:        client,
 		defaultKeyID:  config.DefaultKeyID,
 		bulkAvailable: bulkAvailable,
-	}, nil
+	}
+	go func() {
+		for {
+			select {
+			case certificate := <-config.ReloadCertEvents:
+				client := kes.NewClientWithConfig("", &tls.Config{
+					MinVersion:         tls.VersionTLS12,
+					Certificates:       []tls.Certificate{certificate},
+					RootCAs:            config.RootCAs,
+					ClientSessionCache: tls.NewLRUClientSessionCache(tlsClientSessionCacheSize),
+				})
+				client.Endpoints = endpoints
+
+				c.lock.Lock()
+				c.client = client
+				c.lock.Unlock()
+			}
+		}
+	}()
+	return c, nil
 }
 
 type kesClient struct {
+	lock         sync.RWMutex
 	defaultKeyID string
 	client       *kes.Client
 
@@ -113,6 +140,9 @@ func (c *kesClient) Stat(ctx context.Context) (Status, error) {
 }
 
 func (c *kesClient) Metrics(ctx context.Context) (kes.Metric, error) {
+	c.lock.RLock()
+	defer c.lock.RUnlock()
+
 	return c.client.Metrics(ctx)
 }
 
@@ -122,6 +152,9 @@ func (c *kesClient) Metrics(ctx context.Context) (kes.Metric, error) {
 // If the a key with the same keyID already exists then
 // CreateKey returns kes.ErrKeyExists.
 func (c *kesClient) CreateKey(ctx context.Context, keyID string) error {
+	c.lock.RLock()
+	defer c.lock.RUnlock()
+
 	return c.client.CreateKey(ctx, keyID)
 }
 
@@ -134,6 +167,9 @@ func (c *kesClient) CreateKey(ctx context.Context, keyID string) error {
 // The same context must be provided when the generated
 // key should be decrypted.
 func (c *kesClient) GenerateKey(ctx context.Context, keyID string, cryptoCtx Context) (DEK, error) {
+	c.lock.RLock()
+	defer c.lock.RUnlock()
+
 	if keyID == "" {
 		keyID = c.defaultKeyID
 	}
@@ -156,6 +192,9 @@ func (c *kesClient) GenerateKey(ctx context.Context, keyID string, cryptoCtx Con
 // server referenced by the key ID. The context must match the
 // context value used to generate the ciphertext.
 func (c *kesClient) DecryptKey(keyID string, ciphertext []byte, ctx Context) ([]byte, error) {
+	c.lock.RLock()
+	defer c.lock.RUnlock()
+
 	ctxBytes, err := ctx.MarshalText()
 	if err != nil {
 		return nil, err
@@ -164,6 +203,9 @@ func (c *kesClient) DecryptKey(keyID string, ciphertext []byte, ctx Context) ([]
 }
 
 func (c *kesClient) DecryptAll(ctx context.Context, keyID string, ciphertexts [][]byte, contexts []Context) ([][]byte, error) {
+	c.lock.RLock()
+	defer c.lock.RUnlock()
+
 	if c.bulkAvailable {
 		CCPs := make([]kes.CCP, 0, len(ciphertexts))
 		for i := range ciphertexts {


Test patch: