diff --git a/CHANGELOG.md b/CHANGELOG.md index 048e35cf0bf..c8ce4812b13 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -91,6 +91,7 @@ To learn more about active deprecations, we recommend checking [GitHub Discussio - **General**: Check updated status for Fallback condition instead of ScaledObject ([#7488](https://github.com/kedacore/keda/issues/7488)) - **General**: Fix int64 overflow in milli-quantity conversion for very large metric values ([#7441](https://github.com/kedacore/keda/issues/7441)) - **General**: Fix ScaledObject admission webhook to return validation error from `verifyReplicaCount`, preventing invalid ScaledObjects from being created ([#5954](https://github.com/kedacore/keda/issues/5954)) +- **General**: Rebuild CA cert pool from scratch on each rotation to prevent monotonic memory growth ([#7691](https://github.com/kedacore/keda/issues/7691)) - **Azure Data Explorer Scaler**: Remove clientSecretFromEnv support ([#7554](https://github.com/kedacore/keda/pull/7554)) - **Cron Scaler**: Fix metric name generation so cron expressions with comma-separated values no longer produce invalid metric names ([#7448](https://github.com/kedacore/keda/issues/7448)) - **Forgejo Scaler**: Limit HTTP error response logging ([#7469](https://github.com/kedacore/keda/pull/7469)) diff --git a/pkg/metricsservice/utils/tls.go b/pkg/metricsservice/utils/tls.go index 4c37ede1338..35794a2198b 100644 --- a/pkg/metricsservice/utils/tls.go +++ b/pkg/metricsservice/utils/tls.go @@ -29,33 +29,40 @@ import ( "github.com/fsnotify/fsnotify" "google.golang.org/grpc/credentials" logf "sigs.k8s.io/controller-runtime/pkg/log" - - kedautil "github.com/kedacore/keda/v2/pkg/util" ) var log = logf.Log.WithName("grpc_server_certificates") +// buildCertPool creates a fresh x509.CertPool seeded from the system pool +// and appends the PEM-encoded CA bundle at caPath. +// A new pool is returned on every call so that stale entries never accumulate. +func buildCertPool(caPath string) (*x509.CertPool, error) { + pemClientCA, err := os.ReadFile(caPath) + if err != nil { + return nil, err + } + pool, _ := x509.SystemCertPool() + if pool == nil { + pool = x509.NewCertPool() + } + if !pool.AppendCertsFromPEM(pemClientCA) { + return nil, fmt.Errorf("failed to add client CA's certificate") + } + return pool, nil +} + // LoadGrpcTLSCredentials reads the certificate from the given path and returns TLS transport credentials func LoadGrpcTLSCredentials(ctx context.Context, certDir string, server bool) (credentials.TransportCredentials, error) { caPath := path.Join(certDir, "ca.crt") certPath := path.Join(certDir, "tls.crt") keyPath := path.Join(certDir, "tls.key") - // Load certificate of the CA who signed client's certificate - pemClientCA, err := os.ReadFile(caPath) + // Build the initial cert pool + initialPool, err := buildCertPool(caPath) if err != nil { return nil, err } - // Get the SystemCertPool, continue with an empty pool on error - certPool, _ := x509.SystemCertPool() - if certPool == nil { - certPool = x509.NewCertPool() - } - if !certPool.AppendCertsFromPEM(pemClientCA) { - return nil, fmt.Errorf("failed to add client CA's certificate") - } - // Load initial certificate and private key mTLSCertificate, err := tls.LoadX509KeyPair(certPath, keyPath) if err != nil { @@ -73,6 +80,9 @@ func LoadGrpcTLSCredentials(ctx context.Context, certDir string, server bool) (c } certMutex := sync.RWMutex{} + // certPool is guarded by certMutex and replaced (never mutated) on rotation + certPool := initialPool + go func() { log.V(1).Info("starting mTLS certificates monitoring") for { @@ -92,27 +102,24 @@ func LoadGrpcTLSCredentials(ctx context.Context, certDir string, server bool) (c } log.V(1).Info("detected change on certificates, reloading") - pemClientCA, err := os.ReadFile(caPath) + // Rebuild the CA pool from scratch on every rotation so that + // the pool never grows monotonically with duplicate entries. + newPool, err := buildCertPool(caPath) if err != nil { - log.Error(err, "error reading grpc ca certificate") + log.Error(err, "error rebuilding grpc ca certificate pool") continue } - if !certPool.AppendCertsFromPEM(pemClientCA) { - log.Error(err, "failed to add client CA's certificate") - continue - } - log.V(1).Info("grpc ca certificate has been updated") - // Load certificate of the CA who signed client's certificate cert, err := tls.LoadX509KeyPair(certPath, keyPath) if err != nil { log.Error(err, "error reading grpc certificate") continue } certMutex.Lock() + certPool = newPool mTLSCertificate = cert certMutex.Unlock() - log.V(1).Info("grpc mTLS certificate has been updated") + log.V(1).Info("grpc mTLS certificate and CA pool have been updated") case err, ok := <-watcher.Errors: if !ok { // Channel was closed (i.e. Watcher.Close() was called). @@ -127,10 +134,12 @@ func LoadGrpcTLSCredentials(ctx context.Context, certDir string, server bool) (c } }() - // Create the credentials and return it + // GetConfigForClient is invoked on every TLS handshake, ensuring each + // handshake uses the current certPool rather than the one captured at + // startup. This prevents the CA pool from growing monotonically across + // certificate rotations (fixes #7691). config := &tls.Config{ - MinVersion: kedautil.GetServiceMinTLSVersion(), - CipherSuites: kedautil.GetServiceTLSCipherList(), + MinVersion: tls.VersionTLS13, GetCertificate: func(_ *tls.ClientHelloInfo) (*tls.Certificate, error) { certMutex.RLock() defer certMutex.RUnlock() @@ -141,12 +150,29 @@ func LoadGrpcTLSCredentials(ctx context.Context, certDir string, server bool) (c defer certMutex.RUnlock() return &mTLSCertificate, nil }, + GetConfigForClient: func(_ *tls.ClientHelloInfo) (*tls.Config, error) { + certMutex.RLock() + pool := certPool + cert := mTLSCertificate + certMutex.RUnlock() + cfg := &tls.Config{ + MinVersion: tls.VersionTLS13, + Certificates: []tls.Certificate{cert}, + } + if server { + cfg.ClientAuth = tls.RequireAndVerifyClientCert + cfg.ClientCAs = pool + } else { + cfg.RootCAs = pool + } + return cfg, nil + }, } if server { config.ClientAuth = tls.RequireAndVerifyClientCert - config.ClientCAs = certPool + config.ClientCAs = initialPool } else { - config.RootCAs = certPool + config.RootCAs = initialPool } return credentials.NewTLS(config), nil