From 932136807b611ecc1722f3c33f60c4661346efe5 Mon Sep 17 00:00:00 2001 From: Andres Martinez Gotor Date: Fri, 28 Apr 2023 16:00:48 +0200 Subject: [PATCH] Chore: Allow to force the download of the public key (#67486) --- conf/defaults.ini | 2 + conf/sample.ini | 2 + .../keyretriever/dynamic/dynamic_retriever.go | 22 +++++---- .../dynamic/dynamic_retriever_test.go | 46 +++++++++++++------ .../keyretriever/retriever_test.go | 8 ++-- pkg/setting/setting.go | 1 + pkg/setting/setting_plugins.go | 1 + 7 files changed, 54 insertions(+), 28 deletions(-) diff --git a/conf/defaults.ini b/conf/defaults.ini index 85a43de5f58..3e585172afb 100644 --- a/conf/defaults.ini +++ b/conf/defaults.ini @@ -1342,6 +1342,8 @@ plugin_catalog_url = https://grafana.com/grafana/plugins/ plugin_catalog_hidden_plugins = # Log all backend requests for core and external plugins. log_backend_requests = false +# Force download of public key for verifying plugin signature on startup. +enforce_public_key_download = false #################################### Grafana Live ########################################## [live] diff --git a/conf/sample.ini b/conf/sample.ini index fed965002a4..ced98a3a829 100644 --- a/conf/sample.ini +++ b/conf/sample.ini @@ -1281,6 +1281,8 @@ ;plugin_catalog_hidden_plugins = # Log all backend requests for core and external plugins. ;log_backend_requests = false +# Force download of public key for verifying plugin signature on startup. +;enforce_public_key_download = false #################################### Grafana Live ########################################## [live] diff --git a/pkg/services/pluginsintegration/keyretriever/dynamic/dynamic_retriever.go b/pkg/services/pluginsintegration/keyretriever/dynamic/dynamic_retriever.go index 4459de59c40..17948954fef 100644 --- a/pkg/services/pluginsintegration/keyretriever/dynamic/dynamic_retriever.go +++ b/pkg/services/pluginsintegration/keyretriever/dynamic/dynamic_retriever.go @@ -12,10 +12,10 @@ import ( "time" "github.com/grafana/grafana/pkg/plugins" - "github.com/grafana/grafana/pkg/plugins/config" "github.com/grafana/grafana/pkg/plugins/log" "github.com/grafana/grafana/pkg/plugins/manager/signature/statickey" "github.com/grafana/grafana/pkg/services/featuremgmt" + "github.com/grafana/grafana/pkg/setting" ) const publicKeySyncInterval = 10 * 24 * time.Hour // 10 days @@ -29,8 +29,9 @@ type ManifestKeys struct { } type KeyRetriever struct { - cfg *config.Cfg - log log.Logger + cfg *setting.Cfg + log log.Logger + flags featuremgmt.FeatureToggles lock sync.Mutex cli http.Client @@ -40,19 +41,20 @@ type KeyRetriever struct { var _ plugins.KeyRetriever = (*KeyRetriever)(nil) -func ProvideService(cfg *config.Cfg, kv plugins.KeyStore) *KeyRetriever { +func ProvideService(cfg *setting.Cfg, kv plugins.KeyStore, flags featuremgmt.FeatureToggles) *KeyRetriever { kr := &KeyRetriever{ - cfg: cfg, - log: log.New("plugin.signature.key_retriever"), - cli: makeHttpClient(), - kv: kv, + cfg: cfg, + flags: flags, + log: log.New("plugin.signature.key_retriever"), + cli: makeHttpClient(), + kv: kv, } return kr } // IsDisabled disables dynamic retrieval of public keys from the API server. func (kr *KeyRetriever) IsDisabled() bool { - return !kr.cfg.Features.IsEnabled(featuremgmt.FlagPluginsAPIManifestKey) + return !kr.flags.IsEnabled(featuremgmt.FlagPluginsAPIManifestKey) } func (kr *KeyRetriever) Run(ctx context.Context) error { @@ -101,7 +103,7 @@ func (kr *KeyRetriever) updateKeys(ctx context.Context) error { if err != nil { return err } - if time.Since(*lastUpdated) < publicKeySyncInterval { + if !kr.cfg.PluginForcePublicKeyDownload && time.Since(*lastUpdated) < publicKeySyncInterval { // Cache is still valid return nil } diff --git a/pkg/services/pluginsintegration/keyretriever/dynamic/dynamic_retriever_test.go b/pkg/services/pluginsintegration/keyretriever/dynamic/dynamic_retriever_test.go index acd46e7db69..f395e61ee00 100644 --- a/pkg/services/pluginsintegration/keyretriever/dynamic/dynamic_retriever_test.go +++ b/pkg/services/pluginsintegration/keyretriever/dynamic/dynamic_retriever_test.go @@ -9,9 +9,9 @@ import ( "time" "github.com/grafana/grafana/pkg/infra/kvstore" - "github.com/grafana/grafana/pkg/plugins/config" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/pluginsintegration/keystore" + "github.com/grafana/grafana/pkg/setting" "github.com/stretchr/testify/require" ) @@ -43,13 +43,11 @@ func setFakeAPIServer(t *testing.T, publicKey string, keyID string) (*httptest.S } func Test_PublicKeyUpdate(t *testing.T) { t.Run("it should retrieve an API key", func(t *testing.T) { - cfg := &config.Cfg{ - Features: featuremgmt.WithFeatures([]interface{}{featuremgmt.FlagPluginsAPIManifestKey}...), - } + cfg := &setting.Cfg{} expectedKey := "fake" s, done := setFakeAPIServer(t, expectedKey, "7e4d0c6a708866e7") cfg.GrafanaComURL = s.URL - v := ProvideService(cfg, keystore.ProvideService(kvstore.NewFakeKVStore())) + v := ProvideService(cfg, keystore.ProvideService(kvstore.NewFakeKVStore()), featuremgmt.WithFeatures(featuremgmt.FlagPluginsAPIManifestKey)) go func() { err := v.Run(context.Background()) require.NoError(t, err) @@ -66,13 +64,11 @@ func Test_PublicKeyUpdate(t *testing.T) { }) t.Run("it should update the latest update date", func(t *testing.T) { - cfg := &config.Cfg{ - Features: featuremgmt.WithFeatures([]interface{}{featuremgmt.FlagPluginsAPIManifestKey}...), - } + cfg := &setting.Cfg{} expectedKey := "fake" s, done := setFakeAPIServer(t, expectedKey, "7e4d0c6a708866e7") cfg.GrafanaComURL = s.URL - v := ProvideService(cfg, keystore.ProvideService(kvstore.NewFakeKVStore())) + v := ProvideService(cfg, keystore.ProvideService(kvstore.NewFakeKVStore()), featuremgmt.WithFeatures(featuremgmt.FlagPluginsAPIManifestKey)) go func() { err := v.Run(context.Background()) require.NoError(t, err) @@ -88,13 +84,11 @@ func Test_PublicKeyUpdate(t *testing.T) { }) t.Run("it should remove old keys", func(t *testing.T) { - cfg := &config.Cfg{ - Features: featuremgmt.WithFeatures([]interface{}{featuremgmt.FlagPluginsAPIManifestKey}...), - } + cfg := &setting.Cfg{} expectedKey := "fake" s, done := setFakeAPIServer(t, expectedKey, "other") cfg.GrafanaComURL = s.URL - v := ProvideService(cfg, keystore.ProvideService(kvstore.NewFakeKVStore())) + v := ProvideService(cfg, keystore.ProvideService(kvstore.NewFakeKVStore()), featuremgmt.WithFeatures(featuremgmt.FlagPluginsAPIManifestKey)) go func() { err := v.Run(context.Background()) require.NoError(t, err) @@ -113,4 +107,30 @@ func Test_PublicKeyUpdate(t *testing.T) { require.Equal(t, true, found) require.Equal(t, expectedKey, res) }) + + t.Run("it should force-download the key", func(t *testing.T) { + cfg := &setting.Cfg{ + PluginForcePublicKeyDownload: true, + } + expectedKey := "fake" + s, done := setFakeAPIServer(t, expectedKey, "7e4d0c6a708866e7") + cfg.GrafanaComURL = s.URL + v := ProvideService(cfg, keystore.ProvideService(kvstore.NewFakeKVStore()), featuremgmt.WithFeatures(featuremgmt.FlagPluginsAPIManifestKey)) + // Simulate an updated key + err := v.kv.SetLastUpdated(context.Background()) + require.NoError(t, err) + go func() { + err := v.Run(context.Background()) + require.NoError(t, err) + }() + <-done + + // wait for the lock to be free + v.lock.Lock() + defer v.lock.Unlock() + res, found, err := v.kv.Get(context.Background(), "7e4d0c6a708866e7") + require.NoError(t, err) + require.Equal(t, true, found) + require.Equal(t, expectedKey, res) + }) } diff --git a/pkg/services/pluginsintegration/keyretriever/retriever_test.go b/pkg/services/pluginsintegration/keyretriever/retriever_test.go index 5a35674d81a..283c688fa71 100644 --- a/pkg/services/pluginsintegration/keyretriever/retriever_test.go +++ b/pkg/services/pluginsintegration/keyretriever/retriever_test.go @@ -5,20 +5,18 @@ import ( "testing" "github.com/grafana/grafana/pkg/infra/kvstore" - "github.com/grafana/grafana/pkg/plugins/config" "github.com/grafana/grafana/pkg/plugins/manager/signature/statickey" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/pluginsintegration/keyretriever/dynamic" "github.com/grafana/grafana/pkg/services/pluginsintegration/keystore" + "github.com/grafana/grafana/pkg/setting" "github.com/stretchr/testify/require" ) func Test_GetPublicKey(t *testing.T) { t.Run("it should return a static key", func(t *testing.T) { - cfg := &config.Cfg{ - Features: featuremgmt.WithFeatures(), - } - kr := ProvideService(dynamic.ProvideService(cfg, keystore.ProvideService(kvstore.NewFakeKVStore()))) + cfg := &setting.Cfg{} + kr := ProvideService(dynamic.ProvideService(cfg, keystore.ProvideService(kvstore.NewFakeKVStore()), featuremgmt.WithFeatures())) key, err := kr.GetPublicKey(context.Background(), statickey.GetDefaultKeyID()) require.NoError(t, err) require.Equal(t, statickey.GetDefaultKey(), key) diff --git a/pkg/setting/setting.go b/pkg/setting/setting.go index b7020ed8d87..97f7a39e78b 100644 --- a/pkg/setting/setting.go +++ b/pkg/setting/setting.go @@ -236,6 +236,7 @@ type Cfg struct { PluginCatalogHiddenPlugins []string PluginAdminEnabled bool PluginAdminExternalManageEnabled bool + PluginForcePublicKeyDownload bool PluginsCDNURLTemplate string PluginLogBackendRequests bool diff --git a/pkg/setting/setting_plugins.go b/pkg/setting/setting_plugins.go index 86201eccb7b..9d95154895d 100644 --- a/pkg/setting/setting_plugins.go +++ b/pkg/setting/setting_plugins.go @@ -30,6 +30,7 @@ func (cfg *Cfg) readPluginSettings(iniFile *ini.File) error { cfg.PluginsEnableAlpha = pluginsSection.Key("enable_alpha").MustBool(false) cfg.PluginsAppsSkipVerifyTLS = pluginsSection.Key("app_tls_skip_verify_insecure").MustBool(false) cfg.PluginSettings = extractPluginSettings(iniFile.Sections()) + cfg.PluginForcePublicKeyDownload = pluginsSection.Key("enforce_public_key_download").MustBool(false) pluginsAllowUnsigned := pluginsSection.Key("allow_loading_unsigned_plugins").MustString("")