Plugins: Require signing of external back-end plugins (#24075)

* PluginManager: Require signing of external plugins

Co-authored-by: Marcus Efraimsson <marcus.efraimsson@gmail.com>
Co-authored-by: Diana Payton <52059945+oddlittlebird@users.noreply.github.com>

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]

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]

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
 

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
+}

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)
+}

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:"-"`

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
 	}
 

pkg/plugins/plugins_test.go

@@ -1,67 +1,145 @@
 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) {
+func TestPluginManager_Init(t *testing.T) {
+	origRootPath := setting.StaticRootPath
+	origRaw := setting.Raw
+	t.Cleanup(func() {
+		setting.StaticRootPath = origRootPath
+		setting.Raw = origRaw
+	})
 
-	Convey("When scanning for plugins", t, func() {
-		setting.StaticRootPath, _ = filepath.Abs("../../public/")
-		setting.Raw = ini.Empty()
+	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",
+					},
+				},
 			},
 		}
 		err := pm.Init()
+		require.NoError(t, err)
 
-		So(err, ShouldBeNil)
-		So(len(DataSources), ShouldBeGreaterThan, 1)
-		So(len(Panels), ShouldBeGreaterThan, 1)
+		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("Should set module automatically", func() {
-			So(DataSources["graphite"].Module, ShouldEqual, "app/plugins/datasource/graphite/module")
+	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"
+
+		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 reading app plugin definition", t, func() {
+	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"
+
 		pm := &PluginManager{
 			Cfg: &setting.Cfg{
-				FeatureToggles: map[string]bool{},
-				PluginSettings: setting.PluginSettings{
-					"nginx-app": map[string]string{
-						"path": "testdata/test-app",
-					},
-				},
+				PluginsAllowUnsigned: []string{"test"},
 			},
+			BackendPluginManager: fakeBackendPluginManager{},
 		}
 		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)
 	})
 
-	Convey("When checking if renderer is backend only plugin", t, func() {
-		pluginScanner := &PluginScanner{}
-		result := pluginScanner.IsBackendOnlyPlugin("renderer")
+	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)
 
-		So(result, ShouldEqual, true)
+		assert.Equal(t, []error{fmt.Errorf(`plugin "test" has an invalid signature`)}, pm.scanningErrors)
 	})
+}
 
-	Convey("When checking if app is backend only plugin", t, func() {
-		pluginScanner := &PluginScanner{}
-		result := pluginScanner.IsBackendOnlyPlugin("app")
+func TestPluginManager_IsBackendOnlyPlugin(t *testing.T) {
+	pluginScanner := &PluginScanner{}
 
-		So(result, ShouldEqual, false)
-	})
+	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) {
 }

pkg/plugins/testdata/invalid-signature/plugin/MANIFEST.txt

@@ -0,0 +1 @@
+Invalid manifest

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"
+    }
+  }
+}

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"
+    }
+  }
+}

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")