diff --git a/pkg/api/plugin_resource_test.go b/pkg/api/plugin_resource_test.go index 3aeb39a8bd6..a2d3c93111f 100644 --- a/pkg/api/plugin_resource_test.go +++ b/pkg/api/plugin_resource_test.go @@ -14,6 +14,7 @@ import ( "github.com/stretchr/testify/require" "github.com/grafana/grafana/pkg/infra/db" + "github.com/grafana/grafana/pkg/infra/kvstore" "github.com/grafana/grafana/pkg/infra/localcache" "github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/plugins/backendplugin/coreplugin" @@ -35,6 +36,7 @@ import ( "github.com/grafana/grafana/pkg/services/oauthtoken/oauthtokentest" "github.com/grafana/grafana/pkg/services/pluginsintegration" "github.com/grafana/grafana/pkg/services/pluginsintegration/config" + "github.com/grafana/grafana/pkg/services/pluginsintegration/keystore" "github.com/grafana/grafana/pkg/services/pluginsintegration/pluginaccesscontrol" "github.com/grafana/grafana/pkg/services/pluginsintegration/plugincontext" pluginSettings "github.com/grafana/grafana/pkg/services/pluginsintegration/pluginsettings/service" @@ -65,7 +67,7 @@ func TestCallResource(t *testing.T) { reg := registry.ProvideService() l := loader.ProvideService(pCfg, fakes.NewFakeLicensingService(), signature.NewUnsignedAuthorizer(pCfg), reg, provider.ProvideService(coreRegistry), finder.NewLocalFinder(), fakes.NewFakeRoleRegistry(), - assetpath.ProvideService(pluginscdn.ProvideService(pCfg)), signature.ProvideService(pCfg)) + assetpath.ProvideService(pluginscdn.ProvideService(pCfg)), signature.ProvideService(pCfg, keystore.ProvideService(kvstore.NewFakeKVStore()))) srcs := sources.ProvideService(cfg, pCfg) ps, err := store.ProvideService(reg, srcs, l) require.NoError(t, err) diff --git a/pkg/plugins/ifaces.go b/pkg/plugins/ifaces.go index 9a523686310..a4ce3c6f736 100644 --- a/pkg/plugins/ifaces.go +++ b/pkg/plugins/ifaces.go @@ -148,3 +148,12 @@ type FeatureToggles interface { type SignatureCalculator interface { Calculate(ctx context.Context, src PluginSource, plugin FoundPlugin) (Signature, error) } + +type KeyStore interface { + Get(ctx context.Context, key string) (string, bool, error) + Set(ctx context.Context, key string, value string) error + Del(ctx context.Context, key string) error + ListKeys(ctx context.Context) ([]string, error) + GetLastUpdated(ctx context.Context) (*time.Time, error) + SetLastUpdated(ctx context.Context) error +} diff --git a/pkg/plugins/manager/loader/loader_test.go b/pkg/plugins/manager/loader/loader_test.go index d0dda4d4c51..0fa139bd86b 100644 --- a/pkg/plugins/manager/loader/loader_test.go +++ b/pkg/plugins/manager/loader/loader_test.go @@ -2,10 +2,7 @@ package loader import ( "context" - "encoding/json" "fmt" - "net/http" - "net/http/httptest" "os" "path/filepath" "sort" @@ -15,6 +12,7 @@ import ( "github.com/google/go-cmp/cmp/cmpopts" "github.com/stretchr/testify/require" + "github.com/grafana/grafana/pkg/infra/kvstore" "github.com/grafana/grafana/pkg/plugins" "github.com/grafana/grafana/pkg/plugins/backendplugin" "github.com/grafana/grafana/pkg/plugins/config" @@ -24,10 +22,9 @@ import ( "github.com/grafana/grafana/pkg/plugins/manager/loader/finder" "github.com/grafana/grafana/pkg/plugins/manager/loader/initializer" "github.com/grafana/grafana/pkg/plugins/manager/signature" - "github.com/grafana/grafana/pkg/plugins/manager/signature/manifestverifier" "github.com/grafana/grafana/pkg/plugins/manager/sources" "github.com/grafana/grafana/pkg/plugins/pluginscdn" - "github.com/grafana/grafana/pkg/services/featuremgmt" + "github.com/grafana/grafana/pkg/services/pluginsintegration/keystore" "github.com/grafana/grafana/pkg/setting" ) @@ -1110,115 +1107,6 @@ func TestLoader_Load_SkipUninitializedPlugins(t *testing.T) { }) } -func TestLoader_Load_UseAPIForManifestPublicKey(t *testing.T) { - t.Run("Load plugin using API manifest", func(t *testing.T) { - pluginDir, err := filepath.Abs("../testdata/test-app") - if err != nil { - t.Errorf("could not construct absolute path of plugin dir") - return - } - expected := []*plugins.Plugin{ - { - JSONData: plugins.JSONData{ - ID: "test-app", - Type: "app", - Name: "Test App", - Info: plugins.Info{ - Author: plugins.InfoLink{ - Name: "Test Inc.", - URL: "http://test.com", - }, - Description: "Official Grafana Test App & Dashboard bundle", - Version: "1.0.0", - Links: []plugins.InfoLink{ - {Name: "Project site", URL: "http://project.com"}, - {Name: "License & Terms", URL: "http://license.com"}, - }, - Logos: plugins.Logos{ - Small: "public/plugins/test-app/img/logo_small.png", - Large: "public/plugins/test-app/img/logo_large.png", - }, - Screenshots: []plugins.Screenshots{ - {Path: "public/plugins/test-app/img/screenshot1.png", Name: "img1"}, - {Path: "public/plugins/test-app/img/screenshot2.png", Name: "img2"}, - }, - Updated: "2015-02-10", - }, - Dependencies: plugins.Dependencies{ - GrafanaVersion: "3.x.x", - Plugins: []plugins.Dependency{ - {Type: "datasource", ID: "graphite", Name: "Graphite", Version: "1.0.0"}, - {Type: "panel", ID: "graph", Name: "Graph", Version: "1.0.0"}, - }, - }, - Includes: []*plugins.Includes{ - {Name: "Nginx Connections", Path: "dashboards/connections.json", Type: "dashboard", Role: "Viewer", Slug: "nginx-connections"}, - {Name: "Nginx Memory", Path: "dashboards/memory.json", Type: "dashboard", Role: "Viewer", Slug: "nginx-memory"}, - {Name: "Nginx Panel", Type: "panel", Role: "Viewer", Slug: "nginx-panel"}, - {Name: "Nginx Datasource", Type: "datasource", Role: "Viewer", Slug: "nginx-datasource"}, - }, - Backend: false, - }, - FS: plugins.NewLocalFS(filesInDir(t, pluginDir), pluginDir), - Class: plugins.External, - Signature: plugins.SignatureValid, - SignatureType: plugins.GrafanaSignature, - SignatureOrg: "Grafana Labs", - Module: "plugins/test-app/module", - BaseURL: "public/plugins/test-app", - }, - } - - reg := fakes.NewFakePluginRegistry() - procPrvdr := fakes.NewFakeBackendProcessProvider() - procMgr := fakes.NewFakeProcessManager() - apiCalled := false - cfg := &config.Cfg{Features: featuremgmt.WithFeatures([]interface{}{"pluginsAPIManifestKey"}...)} - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.URL.Path == "/api/plugins/ci/keys" { - w.WriteHeader(http.StatusOK) - // Use the hardcoded key - k, err := manifestverifier.New(&config.Cfg{}, log.New("test")).GetPublicKey("7e4d0c6a708866e7") - require.NoError(t, err) - data := struct { - Items []manifestverifier.ManifestKeys `json:"items"` - }{ - Items: []manifestverifier.ManifestKeys{{PublicKey: k, KeyID: "7e4d0c6a708866e7"}}, - } - b, err := json.Marshal(data) - require.NoError(t, err) - _, err = w.Write(b) - require.NoError(t, err) - apiCalled = true - return - } - w.WriteHeader(http.StatusNotFound) - })) - cfg.GrafanaComURL = s.URL - l := newLoader(cfg, func(l *Loader) { - l.pluginRegistry = reg - l.processManager = procMgr - l.pluginInitializer = initializer.New(cfg, procPrvdr, fakes.NewFakeLicensingService()) - }) - got, err := l.Load(context.Background(), &fakes.FakePluginSource{ - PluginClassFunc: func(ctx context.Context) plugins.Class { - return plugins.External - }, - PluginURIsFunc: func(ctx context.Context) []string { - return []string{pluginDir} - }, - }) - require.NoError(t, err) - require.True(t, apiCalled) - - if !cmp.Equal(got, expected, compareOpts...) { - t.Fatalf("Result mismatch (-want +got):\n%s", cmp.Diff(got, expected, compareOpts...)) - } - - verifyState(t, expected, reg, procPrvdr, procMgr) - }) -} - func TestLoader_Load_NestedPlugins(t *testing.T) { rootDir, err := filepath.Abs("../") if err != nil { @@ -1533,7 +1421,8 @@ func Test_setPathsBasedOnApp(t *testing.T) { func newLoader(cfg *config.Cfg, cbs ...func(loader *Loader)) *Loader { l := New(cfg, &fakes.FakeLicensingService{}, signature.NewUnsignedAuthorizer(cfg), fakes.NewFakePluginRegistry(), fakes.NewFakeBackendProcessProvider(), fakes.NewFakeProcessManager(), fakes.NewFakeRoleRegistry(), - assetpath.ProvideService(pluginscdn.ProvideService(cfg)), finder.NewLocalFinder(), signature.ProvideService(cfg)) + assetpath.ProvideService(pluginscdn.ProvideService(cfg)), finder.NewLocalFinder(), + signature.ProvideService(cfg, keystore.ProvideService(kvstore.NewFakeKVStore()))) for _, cb := range cbs { cb(l) diff --git a/pkg/plugins/manager/manager_integration_test.go b/pkg/plugins/manager/manager_integration_test.go index d98e34ccfb6..1e20c169614 100644 --- a/pkg/plugins/manager/manager_integration_test.go +++ b/pkg/plugins/manager/manager_integration_test.go @@ -14,6 +14,7 @@ import ( "gopkg.in/ini.v1" "github.com/grafana/grafana/pkg/infra/db" + "github.com/grafana/grafana/pkg/infra/kvstore" "github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/plugins" "github.com/grafana/grafana/pkg/plugins/backendplugin/coreplugin" @@ -31,6 +32,7 @@ import ( "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/licensing" "github.com/grafana/grafana/pkg/services/pluginsintegration/config" + "github.com/grafana/grafana/pkg/services/pluginsintegration/keystore" plicensing "github.com/grafana/grafana/pkg/services/pluginsintegration/licensing" "github.com/grafana/grafana/pkg/services/searchV2" "github.com/grafana/grafana/pkg/setting" @@ -117,7 +119,7 @@ func TestIntegrationPluginManager(t *testing.T) { lic := plicensing.ProvideLicensing(cfg, &licensing.OSSLicensingService{Cfg: cfg}) l := loader.ProvideService(pCfg, lic, signature.NewUnsignedAuthorizer(pCfg), reg, provider.ProvideService(coreRegistry), finder.NewLocalFinder(), fakes.NewFakeRoleRegistry(), - assetpath.ProvideService(pluginscdn.ProvideService(pCfg)), signature.ProvideService(pCfg)) + assetpath.ProvideService(pluginscdn.ProvideService(pCfg)), signature.ProvideService(pCfg, keystore.ProvideService(kvstore.NewFakeKVStore()))) srcs := sources.ProvideService(cfg, pCfg) ps, err := store.ProvideService(reg, srcs, l) require.NoError(t, err) diff --git a/pkg/plugins/manager/signature/manifest.go b/pkg/plugins/manager/signature/manifest.go index 7fe263ed9ed..2f4bbcf853f 100644 --- a/pkg/plugins/manager/signature/manifest.go +++ b/pkg/plugins/manager/signature/manifest.go @@ -58,17 +58,25 @@ type Signature struct { var _ plugins.SignatureCalculator = &Signature{} -func ProvideService(cfg *config.Cfg) *Signature { +func ProvideService(cfg *config.Cfg, kv plugins.KeyStore) *Signature { log := log.New("plugin.signature") return &Signature{ - verifier: manifestverifier.New(cfg, log), + verifier: manifestverifier.New(cfg, log, kv), mlog: log, } } +func (s *Signature) IsDisabled() bool { + return s.verifier.IsDisabled() +} + +func (s *Signature) Run(ctx context.Context) error { + return s.verifier.Run(ctx) +} + // readPluginManifest attempts to read and verify the plugin manifest // if any error occurs or the manifest is not valid, this will return an error -func (s *Signature) readPluginManifest(body []byte) (*PluginManifest, error) { +func (s *Signature) readPluginManifest(ctx context.Context, body []byte) (*PluginManifest, error) { block, _ := clearsign.Decode(body) if block == nil { return nil, errors.New("unable to decode manifest") @@ -81,7 +89,7 @@ func (s *Signature) readPluginManifest(body []byte) (*PluginManifest, error) { return nil, fmt.Errorf("%v: %w", "Error parsing manifest JSON", err) } - if err = s.validateManifest(manifest, block); err != nil { + if err = s.validateManifest(ctx, manifest, block); err != nil { return nil, err } @@ -131,7 +139,7 @@ func (s *Signature) Calculate(ctx context.Context, src plugins.PluginSource, plu }, nil } - manifest, err := s.readPluginManifest(byteValue) + manifest, err := s.readPluginManifest(ctx, byteValue) if err != nil { s.mlog.Debug("Plugin signature invalid", "id", plugin.JSONData.ID, "err", err) return plugins.Signature{ @@ -286,7 +294,7 @@ func (r invalidFieldErr) Error() string { return fmt.Sprintf("valid manifest field %s is required", r.field) } -func (s *Signature) validateManifest(m PluginManifest, block *clearsign.Block) error { +func (s *Signature) validateManifest(ctx context.Context, m PluginManifest, block *clearsign.Block) error { if len(m.Plugin) == 0 { return invalidFieldErr{field: "plugin"} } @@ -314,5 +322,5 @@ func (s *Signature) validateManifest(m PluginManifest, block *clearsign.Block) e } } - return s.verifier.Verify(m.KeyID, block) + return s.verifier.Verify(ctx, m.KeyID, block) } diff --git a/pkg/plugins/manager/signature/manifest_test.go b/pkg/plugins/manager/signature/manifest_test.go index 3a1e887ffa8..5c9d78f7d37 100644 --- a/pkg/plugins/manager/signature/manifest_test.go +++ b/pkg/plugins/manager/signature/manifest_test.go @@ -10,9 +10,11 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/grafana/grafana/pkg/infra/kvstore" "github.com/grafana/grafana/pkg/plugins" "github.com/grafana/grafana/pkg/plugins/config" "github.com/grafana/grafana/pkg/plugins/manager/fakes" + "github.com/grafana/grafana/pkg/services/pluginsintegration/keystore" "github.com/grafana/grafana/pkg/setting" ) @@ -49,8 +51,8 @@ NR7DnB0CCQHO+4FlSPtXFTzNepoc+CytQyDAeOLMLmf2Tqhk2YShk+G/YlVX -----END PGP SIGNATURE-----` t.Run("valid manifest", func(t *testing.T) { - s := ProvideService(&config.Cfg{}) - manifest, err := s.readPluginManifest([]byte(txt)) + s := ProvideService(&config.Cfg{}, keystore.ProvideService(kvstore.NewFakeKVStore())) + manifest, err := s.readPluginManifest(context.Background(), []byte(txt)) require.NoError(t, err) require.NotNil(t, manifest) @@ -66,8 +68,8 @@ NR7DnB0CCQHO+4FlSPtXFTzNepoc+CytQyDAeOLMLmf2Tqhk2YShk+G/YlVX t.Run("invalid manifest", func(t *testing.T) { modified := strings.ReplaceAll(txt, "README.md", "xxxxxxxxxx") - s := ProvideService(&config.Cfg{}) - _, err := s.readPluginManifest([]byte(modified)) + s := ProvideService(&config.Cfg{}, keystore.ProvideService(kvstore.NewFakeKVStore())) + _, err := s.readPluginManifest(context.Background(), []byte(modified)) require.Error(t, err) }) } @@ -104,8 +106,8 @@ khdr/tZ1PDgRxMqB/u+Vtbpl0xSxgblnrDOYMSI= -----END PGP SIGNATURE-----` t.Run("valid manifest", func(t *testing.T) { - s := ProvideService(&config.Cfg{}) - manifest, err := s.readPluginManifest([]byte(txt)) + s := ProvideService(&config.Cfg{}, keystore.ProvideService(kvstore.NewFakeKVStore())) + manifest, err := s.readPluginManifest(context.Background(), []byte(txt)) require.NoError(t, err) require.NotNil(t, manifest) @@ -158,7 +160,7 @@ func TestCalculate(t *testing.T) { setting.AppUrl = tc.appURL basePath := filepath.Join(parentDir, "testdata/non-pvt-with-root-url/plugin") - s := ProvideService(&config.Cfg{}) + s := ProvideService(&config.Cfg{}, keystore.ProvideService(kvstore.NewFakeKVStore())) sig, err := s.Calculate(context.Background(), &fakes.FakePluginSource{ PluginClassFunc: func(ctx context.Context) plugins.Class { return plugins.External @@ -189,7 +191,7 @@ func TestCalculate(t *testing.T) { basePath := "../testdata/renderer-added-file/plugin" runningWindows = true - s := ProvideService(&config.Cfg{}) + s := ProvideService(&config.Cfg{}, keystore.ProvideService(kvstore.NewFakeKVStore())) sig, err := s.Calculate(context.Background(), &fakes.FakePluginSource{ PluginClassFunc: func(ctx context.Context) plugins.Class { return plugins.External @@ -238,7 +240,7 @@ func TestCalculate(t *testing.T) { basePath := "../testdata/app-with-child/dist" - s := ProvideService(&config.Cfg{}) + s := ProvideService(&config.Cfg{}, keystore.ProvideService(kvstore.NewFakeKVStore())) sig, err := s.Calculate(context.Background(), &fakes.FakePluginSource{ PluginClassFunc: func(ctx context.Context) plugins.Class { return plugins.External @@ -683,8 +685,8 @@ func Test_validateManifest(t *testing.T) { } for _, tc := range tcs { t.Run(tc.name, func(t *testing.T) { - s := ProvideService(&config.Cfg{}) - err := s.validateManifest(*tc.manifest, nil) + s := ProvideService(&config.Cfg{}, keystore.ProvideService(kvstore.NewFakeKVStore())) + err := s.validateManifest(context.Background(), *tc.manifest, nil) require.Errorf(t, err, tc.expectedErr) }) } diff --git a/pkg/plugins/manager/signature/manifestverifier/verifier.go b/pkg/plugins/manager/signature/manifestverifier/verifier.go index a5cd6ce57cb..4424b8a8e42 100644 --- a/pkg/plugins/manager/signature/manifestverifier/verifier.go +++ b/pkg/plugins/manager/signature/manifestverifier/verifier.go @@ -2,6 +2,7 @@ package manifestverifier import ( "bytes" + "context" "encoding/json" "errors" "fmt" @@ -14,10 +15,16 @@ import ( "github.com/ProtonMail/go-crypto/openpgp" "github.com/ProtonMail/go-crypto/openpgp/clearsign" "github.com/ProtonMail/go-crypto/openpgp/packet" + "github.com/grafana/grafana/pkg/plugins" "github.com/grafana/grafana/pkg/plugins/config" "github.com/grafana/grafana/pkg/plugins/log" + + // Only used for getting the feature flag value + "github.com/grafana/grafana/pkg/services/featuremgmt" ) +const publicKeySyncInterval = 10 * 24 * time.Hour // 10 days + // ManifestKeys is the database representation of public keys // used to verify plugin manifests. type ManifestKeys struct { @@ -30,20 +37,82 @@ type ManifestVerifier struct { cfg *config.Cfg mlog log.Logger - lock sync.Mutex - cli http.Client - publicKeys map[string]ManifestKeys + lock sync.Mutex + cli http.Client + kv plugins.KeyStore + hasKeys bool } -func New(cfg *config.Cfg, mlog log.Logger) *ManifestVerifier { - return &ManifestVerifier{ - cfg: cfg, - publicKeys: map[string]ManifestKeys{}, - mlog: mlog, - cli: makeHttpClient(), +func New(cfg *config.Cfg, mlog log.Logger, kv plugins.KeyStore) *ManifestVerifier { + pmv := &ManifestVerifier{ + cfg: cfg, + mlog: mlog, + cli: makeHttpClient(), + kv: kv, } + return pmv } +// IsDisabled disables dynamic retrieval of public keys from the API server. +func (pmv *ManifestVerifier) IsDisabled() bool { + return pmv.cfg == nil || pmv.cfg.Features == nil || !pmv.cfg.Features.IsEnabled(featuremgmt.FlagPluginsAPIManifestKey) +} + +func (pmv *ManifestVerifier) Run(ctx context.Context) error { + // do an initial update if necessary + err := pmv.updateKeys(ctx) + if err != nil { + pmv.mlog.Error("Error downloading plugin manifest keys", "error", err) + } + + // calculate initial send delay + lastUpdated, err := pmv.kv.GetLastUpdated(ctx) + if err != nil { + return err + } + nextSendInterval := time.Until(lastUpdated.Add(publicKeySyncInterval)) + if nextSendInterval < time.Minute { + nextSendInterval = time.Minute + } + + downloadKeysTicker := time.NewTicker(nextSendInterval) + defer downloadKeysTicker.Stop() + + select { + case <-downloadKeysTicker.C: + err = pmv.updateKeys(ctx) + if err != nil { + pmv.mlog.Error("Error downloading plugin manifest keys", "error", err) + } + + if nextSendInterval != publicKeySyncInterval { + nextSendInterval = publicKeySyncInterval + downloadKeysTicker.Reset(nextSendInterval) + } + case <-ctx.Done(): + return ctx.Err() + } + + return ctx.Err() +} + +func (pmv *ManifestVerifier) updateKeys(ctx context.Context) error { + pmv.lock.Lock() + defer pmv.lock.Unlock() + + lastUpdated, err := pmv.kv.GetLastUpdated(ctx) + if err != nil { + return err + } + if time.Since(*lastUpdated) < publicKeySyncInterval { + // Cache is still valid + return nil + } + + return pmv.downloadKeys(ctx) +} + +const publicKeyID = "7e4d0c6a708866e7" const publicKeyText = `-----BEGIN PGP PUBLIC KEY BLOCK----- Version: OpenPGP.js v4.10.1 Comment: https://openpgpjs.org @@ -68,40 +137,24 @@ N1c5v9v/4h6qeA== -----END PGP PUBLIC KEY BLOCK----- ` -// getPublicKey loads public keys from: -// - The hard-coded value if the feature flag is not enabled. -// - A cached value from memory if it has been already retrieved. -// - The Grafana.com API if the database is empty. -func (pmv *ManifestVerifier) GetPublicKey(keyID string) (string, error) { - if pmv.cfg == nil || pmv.cfg.Features == nil || !pmv.cfg.Features.IsEnabled("pluginsAPIManifestKey") { - return publicKeyText, nil - } - - pmv.lock.Lock() - defer pmv.lock.Unlock() - - key, exist := pmv.publicKeys[keyID] - if exist { - return key.PublicKey, nil - } - - // Retrieve the key from the API and store it in the database +// Retrieve the key from the API and store it in the database +func (pmv *ManifestVerifier) downloadKeys(ctx context.Context) error { var data struct { Items []ManifestKeys } url, err := url.JoinPath(pmv.cfg.GrafanaComURL, "/api/plugins/ci/keys") // nolint:gosec URL is provided by config if err != nil { - return "", err + return err } - req, err := http.NewRequest(http.MethodGet, url, nil) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) if err != nil { - return "", err + return err } resp, err := pmv.cli.Do(req) if err != nil { - return "", err + return err } defer func() { err := resp.Body.Close() @@ -111,27 +164,89 @@ func (pmv *ManifestVerifier) GetPublicKey(keyID string) (string, error) { }() if err := json.NewDecoder(resp.Body).Decode(&data); err != nil { - return "", err + return err } if len(data.Items) == 0 { - return "", errors.New("missing public key") + return errors.New("missing public key") } + cachedKeys, err := pmv.kv.ListKeys(ctx) + if err != nil { + return err + } + + shouldKeep := make(map[string]bool) for _, key := range data.Items { - pmv.publicKeys[key.KeyID] = key + err = pmv.kv.Set(ctx, key.KeyID, key.PublicKey) + if err != nil { + return err + } + shouldKeep[key.KeyID] = true } - key, exist = pmv.publicKeys[keyID] + // Delete keys that are no longer in the API + for _, key := range cachedKeys { + if !shouldKeep[key] { + err = pmv.kv.Del(ctx, key) + if err != nil { + return err + } + } + } + + // Update the last updated timestamp + return pmv.kv.SetLastUpdated(ctx) +} + +func (pmv *ManifestVerifier) ensureKeys(ctx context.Context) error { + if pmv.hasKeys { + return nil + } + keys, err := pmv.kv.ListKeys(ctx) + if err != nil { + return err + } + if len(keys) == 0 { + // Populate with the default key + err := pmv.kv.Set(ctx, publicKeyID, publicKeyText) + if err != nil { + return err + } + } + pmv.hasKeys = true + return nil +} + +// getPublicKey loads public keys from: +// - The hard-coded value if the feature flag is not enabled. +// - A cached value from kv storage if it has been already retrieved. This cache is populated from the grafana.com API. +func (pmv *ManifestVerifier) getPublicKey(ctx context.Context, keyID string) (string, error) { + if pmv.IsDisabled() { + return publicKeyText, nil + } + + pmv.lock.Lock() + defer pmv.lock.Unlock() + + err := pmv.ensureKeys(ctx) + if err != nil { + return "", err + } + + key, exist, err := pmv.kv.Get(ctx, keyID) + if err != nil { + return "", err + } if exist { - return key.PublicKey, nil + return key, nil } return "", fmt.Errorf("missing public key for %s", keyID) } -func (pmv *ManifestVerifier) Verify(keyID string, block *clearsign.Block) error { - publicKey, err := pmv.GetPublicKey(keyID) +func (pmv *ManifestVerifier) Verify(ctx context.Context, keyID string, block *clearsign.Block) error { + publicKey, err := pmv.getPublicKey(ctx, keyID) if err != nil { return err } diff --git a/pkg/plugins/manager/signature/manifestverifier/verifier_test.go b/pkg/plugins/manager/signature/manifestverifier/verifier_test.go index 116d447b3d2..2d5ed092d22 100644 --- a/pkg/plugins/manager/signature/manifestverifier/verifier_test.go +++ b/pkg/plugins/manager/signature/manifestverifier/verifier_test.go @@ -1,22 +1,26 @@ package manifestverifier import ( + "context" "encoding/json" "net/http" "net/http/httptest" "os" "testing" + "time" "github.com/ProtonMail/go-crypto/openpgp/clearsign" + "github.com/grafana/grafana/pkg/infra/kvstore" "github.com/grafana/grafana/pkg/plugins/config" "github.com/grafana/grafana/pkg/plugins/log" "github.com/grafana/grafana/pkg/services/featuremgmt" + "github.com/grafana/grafana/pkg/services/pluginsintegration/keystore" "github.com/stretchr/testify/require" ) func Test_Verify(t *testing.T) { t.Run("it should verify a manifest with the default key", func(t *testing.T) { - v := New(&config.Cfg{}, log.New("test")) + v := New(&config.Cfg{}, log.New("test"), keystore.ProvideService(kvstore.NewFakeKVStore())) body, err := os.ReadFile("../../testdata/test-app/MANIFEST.txt") if err != nil { @@ -28,47 +32,107 @@ func Test_Verify(t *testing.T) { t.Fatal("failed to decode") } - err = v.Verify("7e4d0c6a708866e7", block) + err = v.Verify(context.Background(), "7e4d0c6a708866e7", block) require.NoError(t, err) }) - - t.Run("it should verify a manifest with the API key", func(t *testing.T) { - cfg := &config.Cfg{ - Features: featuremgmt.WithFeatures([]interface{}{"pluginsAPIManifestKey"}...), - } - v := New(cfg, log.New("test")) - apiCalled := false - s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.URL.Path == "/api/plugins/ci/keys" { - w.WriteHeader(http.StatusOK) - data := struct { - Items []ManifestKeys `json:"items"` - }{ - Items: []ManifestKeys{{PublicKey: publicKeyText, KeyID: "7e4d0c6a708866e7"}}, - } - b, err := json.Marshal(data) - require.NoError(t, err) - _, err = w.Write(b) - require.NoError(t, err) - apiCalled = true - return - } - w.WriteHeader(http.StatusNotFound) - })) - cfg.GrafanaComURL = s.URL - - body, err := os.ReadFile("../../testdata/test-app/MANIFEST.txt") - if err != nil { - t.Fatal(err) - } - - block, _ := clearsign.Decode(body) - if block == nil { - t.Fatal("failed to decode") - } - - err = v.Verify("7e4d0c6a708866e7", block) - require.NoError(t, err) - require.Equal(t, true, apiCalled) - }) +} + +func setFakeAPIServer(t *testing.T, publicKey string, keyID string) (*httptest.Server, chan bool) { + done := make(chan bool) + return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/api/plugins/ci/keys" { + w.WriteHeader(http.StatusOK) + data := struct { + Items []ManifestKeys `json:"items"` + }{ + Items: []ManifestKeys{{PublicKey: publicKey, KeyID: keyID}}, + } + b, err := json.Marshal(data) + if err != nil { + t.Fatal(err) + } + _, err = w.Write(b) + if err != nil { + t.Fatal(err) + } + require.NoError(t, err) + done <- true + return + } + w.WriteHeader(http.StatusNotFound) + done <- true + })), done +} +func Test_PublicKeyUpdate(t *testing.T) { + t.Run("it should verify a manifest with the API key", func(t *testing.T) { + cfg := &config.Cfg{ + Features: featuremgmt.WithFeatures([]interface{}{featuremgmt.FlagPluginsAPIManifestKey}...), + } + expectedKey := "fake" + s, done := setFakeAPIServer(t, expectedKey, "7e4d0c6a708866e7") + cfg.GrafanaComURL = s.URL + v := New(cfg, log.New("test"), keystore.ProvideService(kvstore.NewFakeKVStore())) + 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) + }) + + t.Run("it should update the latest update date", func(t *testing.T) { + cfg := &config.Cfg{ + Features: featuremgmt.WithFeatures([]interface{}{featuremgmt.FlagPluginsAPIManifestKey}...), + } + expectedKey := "fake" + s, done := setFakeAPIServer(t, expectedKey, "7e4d0c6a708866e7") + cfg.GrafanaComURL = s.URL + v := New(cfg, log.New("test"), keystore.ProvideService(kvstore.NewFakeKVStore())) + 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() + ti, err := v.kv.GetLastUpdated(context.Background()) + require.NoError(t, err) + require.Less(t, time.Time{}, *ti) + }) + + t.Run("it should remove old keys", func(t *testing.T) { + cfg := &config.Cfg{ + Features: featuremgmt.WithFeatures([]interface{}{featuremgmt.FlagPluginsAPIManifestKey}...), + } + expectedKey := "fake" + s, done := setFakeAPIServer(t, expectedKey, "other") + cfg.GrafanaComURL = s.URL + v := New(cfg, log.New("test"), keystore.ProvideService(kvstore.NewFakeKVStore())) + 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() + _, found, err := v.kv.Get(context.Background(), "7e4d0c6a708866e7") + require.NoError(t, err) + require.Equal(t, false, found) + + res, found, err := v.kv.Get(context.Background(), "other") + require.NoError(t, err) + require.Equal(t, true, found) + require.Equal(t, expectedKey, res) + }) } diff --git a/pkg/server/backgroundsvcs/background_services.go b/pkg/server/backgroundsvcs/background_services.go index 70e25fb1465..1900d9e455f 100644 --- a/pkg/server/backgroundsvcs/background_services.go +++ b/pkg/server/backgroundsvcs/background_services.go @@ -8,6 +8,7 @@ import ( uss "github.com/grafana/grafana/pkg/infra/usagestats/service" "github.com/grafana/grafana/pkg/infra/usagestats/statscollector" "github.com/grafana/grafana/pkg/plugins/manager/process" + "github.com/grafana/grafana/pkg/plugins/manager/signature" "github.com/grafana/grafana/pkg/registry" "github.com/grafana/grafana/pkg/services/alerting" "github.com/grafana/grafana/pkg/services/auth" @@ -50,6 +51,7 @@ func ProvideBackgroundServiceRegistry( grpcServerProvider grpcserver.Provider, secretMigrationProvider secretsMigrations.SecretMigrationProvider, loginAttemptService *loginattemptimpl.Service, bundleService *supportbundlesimpl.Service, publicDashboardsMetric *publicdashboardsmetric.Service, + signature *signature.Signature, // Need to make sure these are initialized, is there a better place to put them? _ dashboardsnapshots.Service, _ *alerting.AlertNotificationService, _ serviceaccounts.Service, _ *guardian.Provider, @@ -86,6 +88,7 @@ func ProvideBackgroundServiceRegistry( loginAttemptService, bundleService, publicDashboardsMetric, + signature, ) } diff --git a/pkg/services/pluginsintegration/keystore/keystore.go b/pkg/services/pluginsintegration/keystore/keystore.go new file mode 100644 index 00000000000..80b5bf5c4ff --- /dev/null +++ b/pkg/services/pluginsintegration/keystore/keystore.go @@ -0,0 +1,74 @@ +package keystore + +import ( + "context" + "fmt" + "time" + + "github.com/grafana/grafana/pkg/infra/kvstore" + "github.com/grafana/grafana/pkg/plugins" +) + +// Service is a service for storing and retrieving public keys. +type Service struct { + kv *kvstore.NamespacedKVStore +} + +const ( + prefix = "key-" + lastUpdatedKey = "last_updated" +) + +var _ plugins.KeyStore = (*Service)(nil) + +func ProvideService(kv kvstore.KVStore) *Service { + return &Service{ + kv: kvstore.WithNamespace(kv, 0, "plugin.publickeys"), + } +} + +func (s *Service) Get(ctx context.Context, key string) (string, bool, error) { + return s.kv.Get(ctx, prefix+key) +} + +func (s *Service) Set(ctx context.Context, key string, value string) error { + return s.kv.Set(ctx, prefix+key, value) +} + +func (s *Service) Del(ctx context.Context, key string) error { + return s.kv.Del(ctx, prefix+key) +} + +func (s *Service) GetLastUpdated(ctx context.Context) (*time.Time, error) { + lastUpdated := &time.Time{} + if val, ok, err := s.kv.Get(ctx, lastUpdatedKey); err != nil { + return nil, fmt.Errorf("failed to get last updated time: %v", err) + } else if ok { + if parsed, err := time.Parse(time.RFC3339, val); err != nil { + return nil, fmt.Errorf("failed to parse last updated time: %v", err) + } else { + lastUpdated = &parsed + } + } + return lastUpdated, nil +} + +func (s *Service) SetLastUpdated(ctx context.Context) error { + lastUpdated := time.Now() + if err := s.kv.Set(ctx, lastUpdatedKey, lastUpdated.Format(time.RFC3339)); err != nil { + return fmt.Errorf("failed to update last updated time: %v", err) + } + return nil +} + +func (s *Service) ListKeys(ctx context.Context) ([]string, error) { + keys, err := s.kv.Keys(ctx, prefix) + if err != nil { + return nil, err + } + res := make([]string, 0, len(keys)) + for _, key := range keys { + res = append(res, key.Key) + } + return res, nil +} diff --git a/pkg/services/pluginsintegration/pluginsintegration.go b/pkg/services/pluginsintegration/pluginsintegration.go index bc4ab99eb47..12770e78a81 100644 --- a/pkg/services/pluginsintegration/pluginsintegration.go +++ b/pkg/services/pluginsintegration/pluginsintegration.go @@ -26,6 +26,7 @@ import ( "github.com/grafana/grafana/pkg/services/oauthtoken" "github.com/grafana/grafana/pkg/services/pluginsintegration/clientmiddleware" "github.com/grafana/grafana/pkg/services/pluginsintegration/config" + "github.com/grafana/grafana/pkg/services/pluginsintegration/keystore" "github.com/grafana/grafana/pkg/services/pluginsintegration/licensing" "github.com/grafana/grafana/pkg/services/pluginsintegration/plugincontext" "github.com/grafana/grafana/pkg/services/pluginsintegration/pluginsettings" @@ -68,6 +69,8 @@ var WireSet = wire.NewSet( wire.Bind(new(plugins.FileStore), new(*filestore.Service)), wire.Bind(new(plugins.SignatureCalculator), new(*signature.Signature)), signature.ProvideService, + wire.Bind(new(plugins.KeyStore), new(*keystore.Service)), + keystore.ProvideService, ) // WireExtensionSet provides a wire.ProviderSet of plugin providers that can be