From 96ffcaa134d44b8565b6d2faa3cc9da48b3762c0 Mon Sep 17 00:00:00 2001 From: Arve Knudsen Date: Mon, 4 May 2020 10:57:55 +0200 Subject: [PATCH] Plugins: Require signing of external back-end plugins (#24075) * PluginManager: Require signing of external plugins Co-authored-by: Marcus Efraimsson Co-authored-by: Diana Payton <52059945+oddlittlebird@users.noreply.github.com> --- conf/defaults.ini | 2 + conf/sample.ini | 2 + docs/sources/installation/configuration.md | 4 + pkg/infra/fs/exists.go | 18 +++ pkg/infra/fs/exists_test.go | 26 +++ pkg/plugins/models.go | 1 + pkg/plugins/plugins.go | 109 +++++++++---- pkg/plugins/plugins_test.go | 148 +++++++++++++----- .../invalid-signature/plugin/MANIFEST.txt | 1 + .../invalid-signature/plugin/plugin.json | 14 ++ .../testdata/unsigned/plugin/plugin.json | 14 ++ pkg/setting/setting.go | 14 +- 12 files changed, 287 insertions(+), 66 deletions(-) create mode 100644 pkg/infra/fs/exists.go create mode 100644 pkg/infra/fs/exists_test.go create mode 100644 pkg/plugins/testdata/invalid-signature/plugin/MANIFEST.txt create mode 100644 pkg/plugins/testdata/invalid-signature/plugin/plugin.json create mode 100644 pkg/plugins/testdata/unsigned/plugin/plugin.json diff --git a/conf/defaults.ini b/conf/defaults.ini index 8574ab35498..7bdbb97a47d 100644 --- a/conf/defaults.ini +++ b/conf/defaults.ini @@ -695,6 +695,8 @@ disable_sanitize_html = false [plugins] enable_alpha = false app_tls_skip_verify_insecure = false +# Enter a comma-separated list of plugin identifiers to identify plugins that are allowed to be loaded even if they lack a valid signature. +allow_loading_unsigned_plugins = #################################### Grafana Image Renderer Plugin ########################## [plugin.grafana-image-renderer] diff --git a/conf/sample.ini b/conf/sample.ini index 6d180436415..d0a1fffdff9 100644 --- a/conf/sample.ini +++ b/conf/sample.ini @@ -684,6 +684,8 @@ [plugins] ;enable_alpha = false ;app_tls_skip_verify_insecure = false +# Enter a comma-separated list of plugin identifiers to identify plugins that are allowed to be loaded even if they lack a valid signature. +;allow_loading_unsigned_plugins = #################################### Grafana Image Renderer Plugin ########################## [plugin.grafana-image-renderer] diff --git a/docs/sources/installation/configuration.md b/docs/sources/installation/configuration.md index 3a9a9611f48..be7c3a5fc95 100644 --- a/docs/sources/installation/configuration.md +++ b/docs/sources/installation/configuration.md @@ -840,6 +840,10 @@ is false. This settings was introduced in Grafana v6.0. Set to true if you want to test alpha plugins that are not yet ready for general usage. +### allow_loading_unsigned_plugins + +Enter a comma-separated list of plugin identifiers to identify plugins that are allowed to be loaded even if they lack a valid signature. + ## [feature_toggles] ### enable diff --git a/pkg/infra/fs/exists.go b/pkg/infra/fs/exists.go new file mode 100644 index 00000000000..f5574ebe1e9 --- /dev/null +++ b/pkg/infra/fs/exists.go @@ -0,0 +1,18 @@ +package fs + +import ( + "os" +) + +// Exists determines whether a file/directory exists or not. +func Exists(fpath string) (bool, error) { + _, err := os.Stat(fpath) + if err != nil { + if !os.IsNotExist(err) { + return false, err + } + return false, nil + } + + return true, nil +} diff --git a/pkg/infra/fs/exists_test.go b/pkg/infra/fs/exists_test.go new file mode 100644 index 00000000000..55e1b351ec7 --- /dev/null +++ b/pkg/infra/fs/exists_test.go @@ -0,0 +1,26 @@ +package fs + +import ( + "github.com/stretchr/testify/require" + "io/ioutil" + "os" + "testing" +) + +func TestExists_NonExistent(t *testing.T) { + exists, err := Exists("non-existent") + require.NoError(t, err) + + require.False(t, exists) +} + +func TestExists_Existent(t *testing.T) { + f, err := ioutil.TempFile("", "") + require.NoError(t, err) + defer os.Remove(f.Name()) + + exists, err := Exists(f.Name()) + require.NoError(t, err) + + require.True(t, exists) +} diff --git a/pkg/plugins/models.go b/pkg/plugins/models.go index 0286be7abfe..d1d47bb8a32 100644 --- a/pkg/plugins/models.go +++ b/pkg/plugins/models.go @@ -60,6 +60,7 @@ type PluginBase struct { Preload bool `json:"preload"` State PluginState `json:"state,omitempty"` Signature PluginSignature `json:"signature"` + Backend bool `json:"backend"` IncludedInAppId string `json:"-"` PluginDir string `json:"-"` diff --git a/pkg/plugins/plugins.go b/pkg/plugins/plugins.go index 72f76dbbdc2..923429cb38b 100644 --- a/pkg/plugins/plugins.go +++ b/pkg/plugins/plugins.go @@ -13,6 +13,7 @@ import ( "strings" "time" + "github.com/grafana/grafana/pkg/infra/fs" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/metrics" "github.com/grafana/grafana/pkg/plugins/backendplugin" @@ -43,12 +44,15 @@ type PluginScanner struct { errors []error backendPluginManager backendplugin.Manager cfg *setting.Cfg + requireSigned bool + log log.Logger } type PluginManager struct { BackendPluginManager backendplugin.Manager `inject:""` Cfg *setting.Cfg `inject:""` log log.Logger + scanningErrors []error } func init() { @@ -73,33 +77,40 @@ func (pm *PluginManager) Init() error { } pm.log.Info("Starting plugin search") + plugDir := path.Join(setting.StaticRootPath, "app/plugins") - if err := pm.scan(plugDir); err != nil { - return errutil.Wrapf(err, "Failed to scan main plugin directory '%s'", plugDir) + pm.log.Debug("Scanning core plugin directory", "dir", plugDir) + if err := pm.scan(plugDir, false); err != nil { + return errutil.Wrapf(err, "failed to scan core plugin directory '%s'", plugDir) } - pm.log.Info("Checking Bundled Plugins") - plugDir = path.Join(setting.HomePath, "plugins-bundled") - if _, err := os.Stat(plugDir); !os.IsNotExist(err) { - if err := pm.scan(plugDir); err != nil { - return errutil.Wrapf(err, "failed to scan bundled plugin directory '%s'", plugDir) + plugDir = pm.Cfg.BundledPluginsPath + pm.log.Debug("Scanning bundled plugins directory", "dir", plugDir) + exists, err := fs.Exists(plugDir) + if err != nil { + return err + } + if exists { + if err := pm.scan(plugDir, false); err != nil { + return errutil.Wrapf(err, "failed to scan bundled plugins directory '%s'", plugDir) } } // check if plugins dir exists - if _, err := os.Stat(setting.PluginsPath); os.IsNotExist(err) { + exists, err = fs.Exists(setting.PluginsPath) + if err != nil { + return err + } + if !exists { if err = os.MkdirAll(setting.PluginsPath, os.ModePerm); err != nil { - plog.Error("Failed to create plugin dir", "dir", setting.PluginsPath, "error", err) + pm.log.Error("failed to create external plugins directory", "dir", setting.PluginsPath, "error", err) } else { - plog.Info("Plugin dir created", "dir", setting.PluginsPath) - if err := pm.scan(setting.PluginsPath); err != nil { - return errutil.Wrapf(err, "Failed to scan configured plugin directory '%s'", - setting.PluginsPath) - } + pm.log.Info("External plugins directory created", "directory", setting.PluginsPath) } } else { - if err := pm.scan(setting.PluginsPath); err != nil { - return errutil.Wrapf(err, "Failed to scan configured plugin directory '%s'", + pm.log.Debug("Scanning external plugins directory", "dir", setting.PluginsPath) + if err := pm.scan(setting.PluginsPath, true); err != nil { + return errutil.Wrapf(err, "failed to scan external plugins directory '%s'", setting.PluginsPath) } } @@ -163,8 +174,8 @@ func (pm *PluginManager) checkPluginPaths() error { continue } - if err := pm.scan(path); err != nil { - return errutil.Wrapf(err, "Failed to scan directory configured for plugin '%s': '%s'", pluginID, path) + if err := pm.scan(path, false); err != nil { + return errutil.Wrapf(err, "failed to scan directory configured for plugin '%s': '%s'", pluginID, path) } } @@ -172,11 +183,13 @@ func (pm *PluginManager) checkPluginPaths() error { } // scan a directory for plugins. -func (pm *PluginManager) scan(pluginDir string) error { +func (pm *PluginManager) scan(pluginDir string, requireSigned bool) error { scanner := &PluginScanner{ pluginPath: pluginDir, backendPluginManager: pm.BackendPluginManager, cfg: pm.Cfg, + requireSigned: requireSigned, + log: pm.log, } if err := util.Walk(pluginDir, true, true, scanner.walker); err != nil { @@ -196,6 +209,7 @@ func (pm *PluginManager) scan(pluginDir string) error { if len(scanner.errors) > 0 { pm.log.Warn("Some plugins failed to load", "errors", scanner.errors) + pm.scanningErrors = scanner.errors } return nil @@ -229,7 +243,7 @@ func (scanner *PluginScanner) walker(currentPath string, f os.FileInfo, err erro if f.Name() == "plugin.json" { err := scanner.loadPluginJson(currentPath) if err != nil { - log.Error(3, "Plugins: Failed to load plugin json file: %v, err: %v", currentPath, err) + scanner.log.Error("Failed to load plugin", "error", err, "pluginPath", filepath.Dir(currentPath)) scanner.errors = append(scanner.errors, err) } } @@ -252,21 +266,51 @@ func (scanner *PluginScanner) loadPluginJson(pluginJsonFilePath string) error { } if pluginCommon.Id == "" || pluginCommon.Type == "" { - return errors.New("Did not find type and id property in plugin.json") + return errors.New("did not find type or id properties in plugin.json") + } + + pluginCommon.PluginDir = filepath.Dir(pluginJsonFilePath) + + // For the time being, we choose to only require back-end plugins to be signed + if pluginCommon.Backend && scanner.requireSigned { + scanner.log.Debug("Plugin signature required, validating", "pluginID", pluginCommon.Id, + "pluginDir", pluginCommon.PluginDir) + allowUnsigned := false + for _, plug := range scanner.cfg.PluginsAllowUnsigned { + if plug == pluginCommon.Id { + allowUnsigned = true + break + } + } + if sig := GetPluginSignatureState(&pluginCommon); sig != PluginSignatureValid && !allowUnsigned { + switch sig { + case PluginSignatureUnsigned: + return fmt.Errorf("plugin %q is unsigned", pluginCommon.Id) + case PluginSignatureInvalid: + return fmt.Errorf("plugin %q has an invalid signature", pluginCommon.Id) + case PluginSignatureModified: + return fmt.Errorf("plugin %q's signature has been modified", pluginCommon.Id) + default: + return fmt.Errorf("unrecognized plugin signature state %v", sig) + } + } } - var loader PluginLoader pluginGoType, exists := PluginTypes[pluginCommon.Type] if !exists { - return errors.New("Unknown plugin type " + pluginCommon.Type) + return fmt.Errorf("unknown plugin type %q", pluginCommon.Type) } - loader = reflect.New(reflect.TypeOf(pluginGoType)).Interface().(PluginLoader) + loader := reflect.New(reflect.TypeOf(pluginGoType)).Interface().(PluginLoader) // External plugins need a module.js file for SystemJS to load if !strings.HasPrefix(pluginJsonFilePath, setting.StaticRootPath) && !scanner.IsBackendOnlyPlugin(pluginCommon.Type) { module := filepath.Join(filepath.Dir(pluginJsonFilePath), "module.js") - if _, err := os.Stat(module); os.IsNotExist(err) { - plog.Warn("Plugin missing module.js", + exists, err := fs.Exists(module) + if err != nil { + return err + } + if !exists { + scanner.log.Warn("Plugin missing module.js", "name", pluginCommon.Name, "warning", "Missing module.js, If you loaded this plugin from git, make sure to compile it.", "path", module) @@ -276,6 +320,7 @@ func (scanner *PluginScanner) loadPluginJson(pluginJsonFilePath string) error { if _, err := reader.Seek(0, 0); err != nil { return err } + return loader.Load(jsonParser, currentDir, scanner.backendPluginManager) } @@ -290,11 +335,19 @@ func GetPluginMarkdown(pluginId string, name string) ([]byte, error) { } path := filepath.Join(plug.PluginDir, fmt.Sprintf("%s.md", strings.ToUpper(name))) - if _, err := os.Stat(path); os.IsNotExist(err) { + exists, err := fs.Exists(path) + if err != nil { + return nil, err + } + if !exists { path = filepath.Join(plug.PluginDir, fmt.Sprintf("%s.md", strings.ToLower(name))) } - if _, err := os.Stat(path); os.IsNotExist(err) { + exists, err = fs.Exists(path) + if err != nil { + return nil, err + } + if !exists { return make([]byte, 0), nil } diff --git a/pkg/plugins/plugins_test.go b/pkg/plugins/plugins_test.go index 8403ab8b609..8d33c628273 100644 --- a/pkg/plugins/plugins_test.go +++ b/pkg/plugins/plugins_test.go @@ -1,40 +1,36 @@ package plugins import ( + "context" + "fmt" "path/filepath" "testing" + "github.com/grafana/grafana-plugin-sdk-go/backend" + "github.com/grafana/grafana/pkg/models" + "github.com/grafana/grafana/pkg/plugins/backendplugin" "github.com/grafana/grafana/pkg/setting" - . "github.com/smartystreets/goconvey/convey" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "gopkg.in/ini.v1" ) -func TestPluginScans(t *testing.T) { - - Convey("When scanning for plugins", t, func() { - setting.StaticRootPath, _ = filepath.Abs("../../public/") - setting.Raw = ini.Empty() - - pm := &PluginManager{ - Cfg: &setting.Cfg{ - FeatureToggles: map[string]bool{}, - }, - } - err := pm.Init() - - So(err, ShouldBeNil) - So(len(DataSources), ShouldBeGreaterThan, 1) - So(len(Panels), ShouldBeGreaterThan, 1) - - Convey("Should set module automatically", func() { - So(DataSources["graphite"].Module, ShouldEqual, "app/plugins/datasource/graphite/module") - }) +func TestPluginManager_Init(t *testing.T) { + origRootPath := setting.StaticRootPath + origRaw := setting.Raw + t.Cleanup(func() { + setting.StaticRootPath = origRootPath + setting.Raw = origRaw }) - Convey("When reading app plugin definition", t, func() { + var err error + setting.StaticRootPath, err = filepath.Abs("../../public/") + require.NoError(t, err) + setting.Raw = ini.Empty() + + t.Run("Base case", func(t *testing.T) { pm := &PluginManager{ Cfg: &setting.Cfg{ - FeatureToggles: map[string]bool{}, PluginSettings: setting.PluginSettings{ "nginx-app": map[string]string{ "path": "testdata/test-app", @@ -43,25 +39,107 @@ func TestPluginScans(t *testing.T) { }, } err := pm.Init() - So(err, ShouldBeNil) + require.NoError(t, err) - So(len(Apps), ShouldBeGreaterThan, 0) - So(Apps["test-app"].Info.Logos.Large, ShouldEqual, "public/plugins/test-app/img/logo_large.png") - So(Apps["test-app"].Info.Screenshots[1].Path, ShouldEqual, "public/plugins/test-app/img/screenshot2.png") + assert.Empty(t, pm.scanningErrors) + assert.Greater(t, len(DataSources), 1) + assert.Greater(t, len(Panels), 1) + assert.Equal(t, "app/plugins/datasource/graphite/module", DataSources["graphite"].Module) + assert.NotEmpty(t, Apps) + assert.Equal(t, "public/plugins/test-app/img/logo_large.png", Apps["test-app"].Info.Logos.Large) + assert.Equal(t, "public/plugins/test-app/img/screenshot2.png", Apps["test-app"].Info.Screenshots[1].Path) }) - Convey("When checking if renderer is backend only plugin", t, func() { - pluginScanner := &PluginScanner{} - result := pluginScanner.IsBackendOnlyPlugin("renderer") + t.Run("With external back-end plugin lacking signature", func(t *testing.T) { + origPluginsPath := setting.PluginsPath + t.Cleanup(func() { + setting.PluginsPath = origPluginsPath + }) + setting.PluginsPath = "testdata/unsigned" - So(result, ShouldEqual, true) + pm := &PluginManager{ + Cfg: &setting.Cfg{}, + } + err := pm.Init() + require.NoError(t, err) + + assert.Equal(t, []error{fmt.Errorf(`plugin "test" is unsigned`)}, pm.scanningErrors) }) - Convey("When checking if app is backend only plugin", t, func() { - pluginScanner := &PluginScanner{} - result := pluginScanner.IsBackendOnlyPlugin("app") + t.Run("With external unsigned back-end plugin and configuration disabling signature check of this plugin", func(t *testing.T) { + origPluginsPath := setting.PluginsPath + t.Cleanup(func() { + setting.PluginsPath = origPluginsPath + }) + setting.PluginsPath = "testdata/unsigned" - So(result, ShouldEqual, false) + pm := &PluginManager{ + Cfg: &setting.Cfg{ + PluginsAllowUnsigned: []string{"test"}, + }, + BackendPluginManager: fakeBackendPluginManager{}, + } + err := pm.Init() + require.NoError(t, err) + + assert.Empty(t, pm.scanningErrors) }) + t.Run("With external back-end plugin with invalid signature", func(t *testing.T) { + origPluginsPath := setting.PluginsPath + t.Cleanup(func() { + setting.PluginsPath = origPluginsPath + }) + setting.PluginsPath = "testdata/invalid-signature" + + pm := &PluginManager{ + Cfg: &setting.Cfg{}, + } + err := pm.Init() + require.NoError(t, err) + + assert.Equal(t, []error{fmt.Errorf(`plugin "test" has an invalid signature`)}, pm.scanningErrors) + }) +} + +func TestPluginManager_IsBackendOnlyPlugin(t *testing.T) { + pluginScanner := &PluginScanner{} + + type testCase struct { + name string + isBackendOnly bool + } + + for _, c := range []testCase{ + {name: "renderer", isBackendOnly: true}, + {name: "app", isBackendOnly: false}, + } { + t.Run(fmt.Sprintf("Plugin %s", c.name), func(t *testing.T) { + result := pluginScanner.IsBackendOnlyPlugin(c.name) + + assert.Equal(t, c.isBackendOnly, result) + }) + } +} + +type fakeBackendPluginManager struct { +} + +func (f fakeBackendPluginManager) Register(descriptor backendplugin.PluginDescriptor) error { + return nil +} + +func (f fakeBackendPluginManager) StartPlugin(ctx context.Context, pluginID string) error { + return nil +} + +func (f fakeBackendPluginManager) CollectMetrics(ctx context.Context, pluginID string) (*backendplugin.CollectMetricsResult, error) { + return nil, nil +} + +func (f fakeBackendPluginManager) CheckHealth(ctx context.Context, pCtx backend.PluginContext) (*backendplugin.CheckHealthResult, error) { + return nil, nil +} + +func (f fakeBackendPluginManager) CallResource(pluginConfig backend.PluginContext, ctx *models.ReqContext, path string) { } diff --git a/pkg/plugins/testdata/invalid-signature/plugin/MANIFEST.txt b/pkg/plugins/testdata/invalid-signature/plugin/MANIFEST.txt new file mode 100644 index 00000000000..3e57ccab0d1 --- /dev/null +++ b/pkg/plugins/testdata/invalid-signature/plugin/MANIFEST.txt @@ -0,0 +1 @@ +Invalid manifest diff --git a/pkg/plugins/testdata/invalid-signature/plugin/plugin.json b/pkg/plugins/testdata/invalid-signature/plugin/plugin.json new file mode 100644 index 00000000000..3e62b3fd5c0 --- /dev/null +++ b/pkg/plugins/testdata/invalid-signature/plugin/plugin.json @@ -0,0 +1,14 @@ +{ + "type": "datasource", + "name": "Test", + "id": "test", + "backend": true, + "state": "alpha", + "info": { + "description": "Test", + "author": { + "name": "Grafana Labs", + "url": "https://grafana.com" + } + } +} diff --git a/pkg/plugins/testdata/unsigned/plugin/plugin.json b/pkg/plugins/testdata/unsigned/plugin/plugin.json new file mode 100644 index 00000000000..3e62b3fd5c0 --- /dev/null +++ b/pkg/plugins/testdata/unsigned/plugin/plugin.json @@ -0,0 +1,14 @@ +{ + "type": "datasource", + "name": "Test", + "id": "test", + "backend": true, + "state": "alpha", + "info": { + "description": "Test", + "author": { + "name": "Grafana Labs", + "url": "https://grafana.com" + } + } +} diff --git a/pkg/setting/setting.go b/pkg/setting/setting.go index f4183a84e30..cc6d44c277f 100644 --- a/pkg/setting/setting.go +++ b/pkg/setting/setting.go @@ -230,9 +230,10 @@ type Cfg struct { ServeFromSubPath bool // Paths - ProvisioningPath string - DataPath string - LogsPath string + ProvisioningPath string + DataPath string + LogsPath string + BundledPluginsPath string // SMTP email settings Smtp SmtpSettings @@ -258,6 +259,7 @@ type Cfg struct { PluginsEnableAlpha bool PluginsAppsSkipVerifyTLS bool PluginSettings PluginSettings + PluginsAllowUnsigned []string DisableSanitizeHtml bool EnterpriseLicensePath string @@ -636,6 +638,7 @@ func (cfg *Cfg) Load(args *CommandLineArgs) error { return err } PluginsPath = makeAbsolute(plugins, HomePath) + cfg.BundledPluginsPath = makeAbsolute("plugins-bundled", HomePath) provisioning, err := valueAsString(iniFile.Section("paths"), "provisioning", "") if err != nil { return err @@ -988,6 +991,11 @@ func (cfg *Cfg) Load(args *CommandLineArgs) 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()) + pluginsAllowUnsigned := pluginsSection.Key("allow_loading_unsigned_plugins").MustString("") + for _, plug := range strings.Split(pluginsAllowUnsigned, ",") { + plug = strings.TrimSpace(plug) + cfg.PluginsAllowUnsigned = append(cfg.PluginsAllowUnsigned, plug) + } // Read and populate feature toggles list featureTogglesSection := iniFile.Section("feature_toggles")