From f31e02272270a646562dd518596ad829a5a3034e Mon Sep 17 00:00:00 2001 From: sarankumar1325 <120080287+sarankumar1325@users.noreply.github.com> Date: Sat, 31 Oct 2026 06:24:25 +0000 Subject: [PATCH] tls: return an error when the CA file holds no certificates GetTLSConfig ignored the result of AppendCertsFromPEM. A tls_ca_path file with no PEM certificates (empty, wrong file, a key) produced an empty root pool, so every TLS connection failed later with "certificate signed by unknown authority" and no hint about the file. Return an error naming the file instead. Signed-off-by: sarankumar1325 <120080287+sarankumar1325@users.noreply.github.com> --- CHANGELOG.md | 1 + pkg/util/tls/tls.go | 4 +++- pkg/util/tls/tls_test.go | 17 +++++++++++++++++ 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9daa87d2f2c..74af505659e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ * [BUGFIX] Store Gateway: Fix the Parquet bucket store ignoring query sharding, which made every shard return all series. #7892 * [BUGFIX] Querier: Fix the querier crashing instead of reporting the error when a blocks store set subservice fails. `blocksStoreReplicationSet` never initialised its `FailureWatcher`, so the failure callback dereferenced a nil pointer on the goroutine the service manager runs listeners on, and the resulting panic's stack trace buried the error logged for the actual cause. #7896 * [BUGFIX] Compactor: Delete block deletion markers left in the global `markers/` location of a tenant after the block they refer to was removed. They were skipped at every bucket index update and kept the tenant location in the bucket forever. #7890 +* [BUGFIX] TLS: Fail client TLS setup with an error when the file set by a `-.tls-ca-path` flag holds no PEM-encoded certificates. Before, the CA pool was left empty and every connection failed later with `certificate signed by unknown authority`. #7908 ## 1.22.0 in progress * [CHANGE] Ruler: Remove the deprecated `-ruler.evaluation-delay-duration` flag and its `ruler_evaluation_delay_duration` per-tenant limit. Use `-ruler.query-offset` / `ruler_query_offset`, which no longer takes the higher of the two values. Cortex decodes the runtime config strictly, so a leftover `ruler_evaluation_delay_duration` override makes the runtime config fail to load: Cortex **exits at startup** (`module failed`, `module=runtime-config`), and on an already-running process every reload fails, pinning the last good overrides and dropping `cortex_runtime_config_last_reload_successful` to 0. Run `grep -r ruler_evaluation_delay_duration` over your runtime configs before upgrading. #7792 diff --git a/pkg/util/tls/tls.go b/pkg/util/tls/tls.go index b89f1c6d6fb..a27d0a3b3dd 100644 --- a/pkg/util/tls/tls.go +++ b/pkg/util/tls/tls.go @@ -50,7 +50,9 @@ func (cfg *ClientConfig) GetTLSConfig() (*tls.Config, error) { return nil, errors.Wrapf(err, "error loading ca cert: %s", cfg.CAPath) } caCertPool = x509.NewCertPool() - caCertPool.AppendCertsFromPEM(caCert) + if !caCertPool.AppendCertsFromPEM(caCert) { + return nil, errors.Errorf("no valid PEM-encoded CA certificates found in ca cert: %s", cfg.CAPath) + } config.RootCAs = caCertPool } diff --git a/pkg/util/tls/tls_test.go b/pkg/util/tls/tls_test.go index 0430cad98f7..7b8f0e4e4d4 100644 --- a/pkg/util/tls/tls_test.go +++ b/pkg/util/tls/tls_test.go @@ -165,6 +165,23 @@ func TestGetTLSConfig_CA(t *testing.T) { _, err = c.GetTLSConfig() assert.Error(t, err) assert.Contains(t, err.Error(), "error loading ca cert") + + // expect an error when the CA file holds no PEM certificates + for name, content := range map[string][]byte{ + "empty": {}, + "not PEM": []byte("not a certificate"), + "key only": []byte(keyPEM), + } { + t.Run(name, func(t *testing.T) { + paths := newTestX509Files(t, nil, nil, content) + c := &ClientConfig{ + CAPath: paths.ca, + } + _, err := c.GetTLSConfig() + require.Error(t, err) + assert.Contains(t, err.Error(), "no valid PEM-encoded CA certificates found") + }) + } } func TestGetTLSConfig_InsecureSkipVerify(t *testing.T) {