TLS Certificate Validation Bypass (Insecure TLS in development)

PoC: Demonstrating TLS certificate validation bypass in Development prior to this fix 1) Setup a local TLS server with a self-signed certificate (no trusted CA installed). - Generate a cert (example using OpenSSL): openssl req -x509 -newkey rsa:2048 -days 1 -nodes -keyout server.key -out server.crt -subj '/CN=annotation.local' - Start a simple TLS server (example Python snippet): import http.server, ssl, socketserver PORT = 8443 Handler = http.server.SimpleHTTPRequestHandler with socketserver.TCPServer(("", PORT), Handler) as httpd: httpd.socket = ssl.wrap_socket(httpd.socket, certfile='server.crt', keyfile='server.key', server_side=True) print("Serving at https://localhost:8443/") httpd.serve_forever() 2) Demonstrate a TLS client that does not verify certificates (simulating Dev Insecure behavior). - Go example client (InsecureSkipVerify): package main import ( "crypto/tls" "fmt" "io/ioutil" "net/http" ) func main() { tr := &http.Transport{TLSClientConfig: &tls.Config{InsecureSkipVerify: true}} client := &http.Client{Transport: tr} resp, err := client.Get("https://localhost:8443/") if err != nil { fmt.Println("ERROR:", err) return } defer resp.Body.Close() body, _ := ioutil.ReadAll(resp.Body) fmt.Println(string(body)) } - Run this client while the server above is listening. The client will succeed despite the server using a self-signed cert because verification is skipped. 3) How this maps to the Grafana change: - Before this patch, in Development, the annotation API client could set Insecure: true and bypass TLS verification, making MITM feasible. - After this patch, TLS verification can be enforced via a CA bundle (or system trust); In Development, verification can still be skipped if desired, but in Production it will default to verification using CAFile or system CAs. Attack vector (illustrative): An attacker on the same network could deploy a TLS MITM proxy presenting a self-signed certificate for the annotation API host. With InsecureSkipVerify (or equivalent Dev config), Grafana would accept the cert, allowing the attacker to read/modify annotation API traffic.

diff --git a/pkg/services/annotations/annotationsapi/api_client.go b/pkg/services/annotations/annotationsapi/api_client.go index 364d9db98d1f2..a816f324cf0b8 100644 --- a/pkg/services/annotations/annotationsapi/api_client.go +++ b/pkg/services/annotations/annotationsapi/api_client.go @@ -50,7 +50,7 @@ func newAnnotationAPIClient(cfg *setting.Cfg, userSvc user.Service, exchanger au } nsMapper := request.GetNamespaceMapper(cfg) - restCfg := buildRESTConfig(url, exchanger, nsMapper, cfg.Env == setting.Dev) + restCfg := buildRESTConfig(url, exchanger, nsMapper, cfg.AnnotationAppPlatform.TLSClientConfig) return &annotationAPIClient{ k8sClient: client.NewK8sHandler( @@ -239,13 +239,11 @@ func newTokenExchangeClient(token, tokenExchangeURL string, allowInsecure bool) return tc, nil } -func buildRESTConfig(url string, exchanger authnlib.TokenExchanger, nsMapper request.NamespaceMapper, allowInsecure bool) *rest.Config { +func buildRESTConfig(url string, exchanger authnlib.TokenExchanger, nsMapper request.NamespaceMapper, tlsConfig rest.TLSClientConfig) *rest.Config { return &rest.Config{ - Host: url, - WrapTransport: newBearerTokenExchangeWrapper(exchanger, nsMapper), - TLSClientConfig: rest.TLSClientConfig{ - Insecure: allowInsecure, - }, + Host: url, + WrapTransport: newBearerTokenExchangeWrapper(exchanger, nsMapper), + TLSClientConfig: tlsConfig, } } diff --git a/pkg/setting/setting.go b/pkg/setting/setting.go index f114020e800a7..269527a80fece 100644 --- a/pkg/setting/setting.go +++ b/pkg/setting/setting.go @@ -1125,7 +1125,7 @@ func (cfg *Cfg) readAnnotationSettings() error { section := cfg.Raw.Section("annotations") cfg.AnnotationCleanupJobBatchSize = section.Key("cleanupjob_batchsize").MustInt64(100) cfg.AnnotationMaximumTagsLength = section.Key("tags_length").MustInt64(500) - annotationAppPlatformSettings, err := loadAnnotationAppPlatformSettings(cfg.Raw) + annotationAppPlatformSettings, err := loadAnnotationAppPlatformSettings(cfg) if err != nil { return err } diff --git a/pkg/setting/setting_annotations.go b/pkg/setting/setting_annotations.go index cb6b14138fa6e..8a24e78fe37ed 100644 --- a/pkg/setting/setting_annotations.go +++ b/pkg/setting/setting_annotations.go @@ -2,9 +2,10 @@ package setting import ( "fmt" + "path/filepath" "time" - "gopkg.in/ini.v1" + "k8s.io/client-go/rest" ) type AnnotationAppPlatformSettings struct { @@ -41,6 +42,9 @@ type AnnotationAppPlatformSettings struct { // APIServerURL is the URL of the standalone annotation API server. // Empty means proxy is disabled regardless of APIMigrationPhase. APIServerURL string + + // TLSClientConfig configures TLS for the connection to the annotation API server. + TLSClientConfig rest.TLSClientConfig } const ( @@ -57,8 +61,9 @@ func (s AnnotationAppPlatformSettings) ProxyAll() bool { return s.APIMigrationPhase == AnnotationAPIMigrationPhaseProxyAll } -func loadAnnotationAppPlatformSettings(cfg *ini.File) (AnnotationAppPlatformSettings, error) { - appPlatformSection := cfg.Section("annotations.app_platform") +func loadAnnotationAppPlatformSettings(cfg *Cfg) (AnnotationAppPlatformSettings, error) { + appPlatformSection := cfg.Raw.Section("annotations.app_platform") + settings := AnnotationAppPlatformSettings{ Enabled: appPlatformSection.Key("enabled").MustBool(false), StoreBackend: appPlatformSection.Key("store_backend").MustString("legacy-sql"), @@ -67,6 +72,7 @@ func loadAnnotationAppPlatformSettings(cfg *ini.File) (AnnotationAppPlatformSett MaxScopeCount: appPlatformSection.Key("max_scope_count").MustInt(5), APIMigrationPhase: appPlatformSection.Key("api_migration_phase").MustString(AnnotationAPIMigrationPhaseOff), APIServerURL: appPlatformSection.Key("api_server_url").MustString(""), + TLSClientConfig: loadTLSClientConfig(cfg), GRPCAddress: appPlatformSection.Key("grpc_address").MustString("localhost:9090"), GRPCUseTLS: appPlatformSection.Key("grpc_use_tls").MustBool(false), @@ -88,3 +94,14 @@ func loadAnnotationAppPlatformSettings(cfg *ini.File) (AnnotationAppPlatformSett return settings, nil } + +// loadTLSClientConfig builds the TLS configuration for the annotation API server client. +// When no CA bundle is configured, it falls back to the system trust store. +// In development, verification is skipped so local self-signed serving certs work without extra setup. +func loadTLSClientConfig(cfg *Cfg) rest.TLSClientConfig { + caCertPath := cfg.SectionWithEnvOverrides("grafana-apiserver").Key("apiservice_ca_bundle_file").MustString("") + if caCertPath == "" { + return rest.TLSClientConfig{Insecure: cfg.Env == Dev} + } + return rest.TLSClientConfig{CAFile: filepath.Clean(caCertPath)} +} diff --git a/pkg/setting/setting_annotations_test.go b/pkg/setting/setting_annotations_test.go index 1554a8a7316be..d2c2cfc5773d8 100644 --- a/pkg/setting/setting_annotations_test.go +++ b/pkg/setting/setting_annotations_test.go @@ -8,38 +8,73 @@ import ( "gopkg.in/ini.v1" ) -func TestLoadAnnotationAppPlatformSettings_MaxScopeCount(t *testing.T) { - cases := []struct { - name string - iniValue *string // nil means no key set - expectedMaxScopeCount int - expectErr bool - }{ - {name: "default when key absent", expectedMaxScopeCount: 5}, - {name: "explicit positive", iniValue: new("10"), expectedMaxScopeCount: 10}, - {name: "zero is accepted", iniValue: new("0"), expectedMaxScopeCount: 0}, - {name: "negative is rejected", iniValue: new("-1"), expectErr: true}, - } - - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - iniFile := ini.Empty() - if tc.iniValue != nil { - section, err := iniFile.NewSection("annotations.app_platform") - require.NoError(t, err) +func TestLoadAnnotationAppPlatformSettings(t *testing.T) { + t.Run("MaxScopeCount", func(t *testing.T) { + cases := []struct { + name string + iniValue *string // nil means no key set + expectedMaxScopeCount int + expectErr bool + }{ + {name: "default when key absent", expectedMaxScopeCount: 5}, + {name: "explicit positive", iniValue: new("10"), expectedMaxScopeCount: 10}, + {name: "zero is accepted", iniValue: new("0"), expectedMaxScopeCount: 0}, + {name: "negative is rejected", iniValue: new("-1"), expectErr: true}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + iniFile := ini.Empty() + if tc.iniValue != nil { + section, err := iniFile.NewSection("annotations.app_platform") + require.NoError(t, err) + + _, err = section.NewKey("max_scope_count", *tc.iniValue) + require.NoError(t, err) + } + + settings, err := loadAnnotationAppPlatformSettings(&Cfg{Raw: iniFile}) + if tc.expectErr { + assert.Error(t, err) + return + } - _, err = section.NewKey("max_scope_count", *tc.iniValue) require.NoError(t, err) - } + assert.Equal(t, tc.expectedMaxScopeCount, settings.MaxScopeCount) + }) + } + }) + + t.Run("TLSClientConfig", func(t *testing.T) { + t.Run("configured CA bundle sets CAFile", func(t *testing.T) { + const caPath = "/etc/grafana/ca.crt" + + iniFile := ini.Empty() + section, err := iniFile.NewSection("grafana-apiserver") + require.NoError(t, err) + _, err = section.NewKey("apiservice_ca_bundle_file", caPath) + require.NoError(t, err) - settings, err := loadAnnotationAppPlatformSettings(iniFile) - if tc.expectErr { - assert.Error(t, err) - return - } + settings, err := loadAnnotationAppPlatformSettings(&Cfg{Raw: iniFile}) + require.NoError(t, err) + assert.Equal(t, caPath, settings.TLSClientConfig.CAFile) + assert.Nil(t, settings.TLSClientConfig.CAData) + assert.False(t, settings.TLSClientConfig.Insecure) + }) + t.Run("no CA bundle verifies against system trust", func(t *testing.T) { + settings, err := loadAnnotationAppPlatformSettings(&Cfg{Raw: ini.Empty(), Env: Prod}) + require.NoError(t, err) + assert.False(t, settings.TLSClientConfig.Insecure) + assert.Empty(t, settings.TLSClientConfig.CAFile) + assert.Nil(t, settings.TLSClientConfig.CAData) + }) + t.Run("no CA bundle in development skips verification", func(t *testing.T) { + settings, err := loadAnnotationAppPlatformSettings(&Cfg{Raw: ini.Empty(), Env: Dev}) require.NoError(t, err) - assert.Equal(t, tc.expectedMaxScopeCount, settings.MaxScopeCount) + assert.True(t, settings.TLSClientConfig.Insecure) + assert.Empty(t, settings.TLSClientConfig.CAFile) + assert.Nil(t, settings.TLSClientConfig.CAData) }) - } + }) }