Plugins: Enforce signing for all plugins (#34364)

* enforce non-backend plugin signing

* fix tests

* add tests

* add signatures

* apply PR feedback

* update upgrading docs

docs/sources/installation/upgrading.md

@@ -243,7 +243,7 @@ A global minimum dashboard refresh interval is now enforced and defaults to 5 se
 
 ### Backend plugins
 
-Grafana now requires backend plugins to be signed. If a backend plugin is not signed Grafana will not load/start it. This is an additional security measure to make sure backend plugin binaries and files haven't been tampered with.  All Grafana Labs authored backend plugins, including Enterprise plugins, are now signed. It's possible to allow unsigned plugins using a configuration setting, but is something we strongly advise against doing. For more information about this setting, refer to [allow loading unsigned plugins]({{< relref "../administration/#allow-loading-unsigned-plugins" >}}).
+Grafana now requires backend plugins to be signed. If a backend plugin is not signed Grafana will not load/start it. This is an additional security measure to make sure backend plugin binaries and files haven't been tampered with.  All Grafana Labs authored backend plugins, including Enterprise plugins, are now signed. It's possible to allow unsigned plugins using a configuration setting, but is something we strongly advise against doing. For more information about this setting, refer to [allow loading unsigned plugins]({{< relref "../administration/#allow_loading_unsigned_plugins" >}}).
 
 ### Cookie path
 
@@ -320,3 +320,9 @@ The Grafana Docker images use the `root` group instead of the `grafana` group. T
 The VictorOps alert notifier now accepts a `severity` tag, in a similar vein to the PagerDuty alert notifier. The possible values are outlined in the [VictorOps docs](https://help.victorops.com/knowledge-base/incident-fields-glossary/).
 
 For example, if you want an alert to be `INFO`-level in VictorOps, create a tag `severity=info` (case-insensitive) in your alert.
+
+## Upgrading to v8.0
+
+### Plugins
+
+Grafana now requires all plugins to be signed. If a plugin is not signed Grafana will not load/start it. This is an additional security measure to make sure plugin files and binaries haven't been tampered with. All Grafana Labs authored plugins, including Enterprise plugins, are now signed. It's possible to allow unsigned plugins using a configuration setting, but is something we strongly advise against doing. For more information about this setting, refer to [allow loading unsigned plugins]({{< relref "../administration/#allow_loading_unsigned_plugins" >}}).

docs/sources/plugins/plugin-signatures.md

@@ -42,7 +42,7 @@ EROR[06-01|16:45:59] Failed to load plugin   error=plugin <plugin id> is unsigne
 
 ## Allow unsigned plugins
 
-We strongly recommend that you don't run unsigned plugins in your Grafana installation. If you're aware of the risks and you still want to load an unsigned plugin, refer to [Configuration]({{< relref "../administration/configuration.md#allow-loading-unsigned-plugins" >}}).
+We strongly recommend that you don't run unsigned plugins in your Grafana installation. If you're aware of the risks and you still want to load an unsigned plugin, refer to [Configuration]({{< relref "../administration/configuration.md#allow_loading_unsigned_plugins" >}}).
 
 If you've allowed loading of an unsigned backend plugin, then Grafana writes a warning message to the server log:
 

pkg/plugins/manager/manager.go

@@ -454,7 +454,11 @@ func (pm *PluginManager) scan(pluginDir string, requireSigned bool) error {
 	}
 
 	if len(scanner.errors) > 0 {
-		pm.log.Warn("Some plugin scanning errors were found", "errors", scanner.errors)
+		var errStr []string
+		for _, err := range scanner.errors {
+			errStr = append(errStr, err.Error())
+		}
+		pm.log.Warn("Some plugin scanning errors were found", "errors", strings.Join(errStr, ", "))
 		pm.scanningErrors = scanner.errors
 	}
 
@@ -623,9 +627,7 @@ func (s *PluginScanner) validateSignature(plugin *plugins.PluginBase) *plugins.P
 			"state", plugin.Signature)
 	}
 
-	// For the time being, we choose to only require back-end plugins to be signed
+	if !s.requireSigned {
-	// NOTE: the state is calculated again when setting metadata on the object
-	if !plugin.Backend || !s.requireSigned {
 		return nil
 	}
 
@@ -633,28 +635,28 @@ func (s *PluginScanner) validateSignature(plugin *plugins.PluginBase) *plugins.P
 	case plugins.PluginSignatureUnsigned:
 		if allowed := s.allowUnsigned(plugin); !allowed {
 			s.log.Debug("Plugin is unsigned", "id", plugin.Id)
-			s.errors = append(s.errors, fmt.Errorf("plugin %q is unsigned", plugin.Id))
+			s.errors = append(s.errors, fmt.Errorf("plugin '%s' is unsigned", plugin.Id))
 			return &plugins.PluginError{
 				ErrorCode: signatureMissing,
 			}
 		}
-		s.log.Warn("Running an unsigned backend plugin", "pluginID", plugin.Id, "pluginDir",
+		s.log.Warn("Running an unsigned plugin", "pluginID", plugin.Id, "pluginDir",
 			plugin.PluginDir)
 		return nil
 	case plugins.PluginSignatureInvalid:
-		s.log.Debug("Plugin %q has an invalid signature", plugin.Id)
+		s.log.Debug("Plugin '%s' has an invalid signature", plugin.Id)
-		s.errors = append(s.errors, fmt.Errorf("plugin %q has an invalid signature", plugin.Id))
+		s.errors = append(s.errors, fmt.Errorf("plugin '%s' has an invalid signature", plugin.Id))
 		return &plugins.PluginError{
 			ErrorCode: signatureInvalid,
 		}
 	case plugins.PluginSignatureModified:
-		s.log.Debug("Plugin %q has a modified signature", plugin.Id)
+		s.log.Debug("Plugin '%s' has a modified signature", plugin.Id)
-		s.errors = append(s.errors, fmt.Errorf("plugin %q's signature has been modified", plugin.Id))
+		s.errors = append(s.errors, fmt.Errorf("plugin '%s' has a modified signature", plugin.Id))
 		return &plugins.PluginError{
 			ErrorCode: signatureModified,
 		}
 	default:
-		panic(fmt.Sprintf("Plugin %q has unrecognized plugin signature state %q", plugin.Id, plugin.Signature))
+		panic(fmt.Sprintf("Plugin '%s' has an unrecognized plugin signature state '%s'", plugin.Id, plugin.Signature))
 	}
 }
 

pkg/plugins/manager/manager_test.go

@@ -61,17 +61,71 @@ func TestPluginManager_Init(t *testing.T) {
 		assert.Equal(t, "public/plugins/test-app/img/screenshot2.png", pm.apps["test-app"].Info.Screenshots[1].Path)
 	})
 
-	t.Run("With external back-end plugin lacking signature", func(t *testing.T) {
+	t.Run("With external back-end plugin lacking signature (production)", func(t *testing.T) {
 		pm := createManager(t, func(pm *PluginManager) {
-			pm.Cfg.PluginsPath = "testdata/unsigned"
+			pm.Cfg.PluginsPath = "testdata/unsigned-datasource"
+			pm.Cfg.Env = setting.Prod
 		})
 		err := pm.Init()
 		require.NoError(t, err)
+		const pluginID = "test"
+
+		assert.Equal(t, []error{fmt.Errorf(`plugin '%s' is unsigned`, pluginID)}, pm.scanningErrors)
+		assert.Nil(t, pm.GetDataSource(pluginID))
+		assert.Nil(t, pm.GetPlugin(pluginID))
+	})
+
+	t.Run("With external back-end plugin lacking signature (development)", func(t *testing.T) {
+		pm := createManager(t, func(pm *PluginManager) {
+			pm.Cfg.PluginsPath = "testdata/unsigned-datasource"
+			pm.Cfg.Env = setting.Dev
+		})
+		err := pm.Init()
+		require.NoError(t, err)
+		const pluginID = "test"
+
+		assert.Empty(t, pm.scanningErrors)
+		assert.NotNil(t, pm.GetDataSource(pluginID))
+
+		plugin := pm.GetPlugin(pluginID)
+		assert.NotNil(t, plugin)
+		assert.Equal(t, plugins.PluginSignatureUnsigned, plugin.Signature)
+	})
+
+	t.Run("With external panel plugin lacking signature (production)", func(t *testing.T) {
+		pm := createManager(t, func(pm *PluginManager) {
+			pm.Cfg.PluginsPath = "testdata/unsigned-panel"
+			pm.Cfg.Env = setting.Prod
+		})
+		err := pm.Init()
+		require.NoError(t, err)
+		const pluginID = "test-panel"
+
+		assert.Equal(t, []error{fmt.Errorf(`plugin '%s' is unsigned`, pluginID)}, pm.scanningErrors)
+		assert.Nil(t, pm.panels[pluginID])
+		assert.Nil(t, pm.GetPlugin(pluginID))
+	})
+
+	t.Run("With external panel plugin lacking signature (development)", func(t *testing.T) {
+		pm := createManager(t, func(pm *PluginManager) {
+			pm.Cfg.PluginsPath = "testdata/unsigned-panel"
+			pm.Cfg.Env = setting.Dev
+		})
+		err := pm.Init()
+		require.NoError(t, err)
+		pluginID := "test-panel"
+
+		assert.Empty(t, pm.scanningErrors)
+		assert.NotNil(t, pm.panels[pluginID])
+
+		plugin := pm.GetPlugin(pluginID)
+		assert.NotNil(t, plugin)
+		assert.Equal(t, plugins.PluginSignatureUnsigned, plugin.Signature)
 	})
 
 	t.Run("With external unsigned back-end plugin and configuration disabling signature check of this plugin", func(t *testing.T) {
 		pm := createManager(t, func(pm *PluginManager) {
-			pm.Cfg.PluginsPath = "testdata/unsigned"
+			pm.Cfg.PluginsPath = "testdata/unsigned-datasource"
 			pm.Cfg.PluginsAllowUnsigned = []string{"test"}
 		})
 		err := pm.Init()
@@ -87,7 +141,10 @@ func TestPluginManager_Init(t *testing.T) {
 		err := pm.Init()
 		require.NoError(t, err)
 
-		assert.Equal(t, []error{fmt.Errorf(`plugin "test" has an invalid signature`)}, pm.scanningErrors)
+		const pluginID = "test"
+		assert.Equal(t, []error{fmt.Errorf(`plugin '%s' has an invalid signature`, pluginID)}, pm.scanningErrors)
+		assert.Nil(t, pm.GetDataSource(pluginID))
+		assert.Nil(t, pm.GetPlugin(pluginID))
 	})
 
 	t.Run("With external back-end plugin lacking files listed in manifest", func(t *testing.T) {
@@ -99,7 +156,7 @@ func TestPluginManager_Init(t *testing.T) {
 		err := pm.Init()
 		require.NoError(t, err)
 
-		assert.Equal(t, []error{fmt.Errorf(`plugin "test"'s signature has been modified`)}, pm.scanningErrors)
+		assert.Equal(t, []error{fmt.Errorf(`plugin 'test' has a modified signature`)}, pm.scanningErrors)
 	})
 
 	t.Run("Transform plugins should be ignored when expressions feature is off", func(t *testing.T) {
@@ -221,7 +278,7 @@ func TestPluginManager_Init(t *testing.T) {
 		err := pm.Init()
 		require.NoError(t, err)
 
-		assert.Equal(t, []error{fmt.Errorf(`plugin "test" has an invalid signature`)}, pm.scanningErrors)
+		assert.Equal(t, []error{fmt.Errorf(`plugin 'test' has an invalid signature`)}, pm.scanningErrors)
 		assert.Nil(t, pm.plugins[("test")])
 	})
 
@@ -263,7 +320,7 @@ func TestPluginManager_Init(t *testing.T) {
 		})
 		err := pm.Init()
 		require.NoError(t, err)
-		assert.Equal(t, []error{fmt.Errorf(`plugin "test"'s signature has been modified`)}, pm.scanningErrors)
+		assert.Equal(t, []error{fmt.Errorf(`plugin 'test' has a modified signature`)}, pm.scanningErrors)
 		assert.Nil(t, pm.plugins[("test")])
 	})
 
@@ -279,7 +336,7 @@ func TestPluginManager_Init(t *testing.T) {
 		})
 		err := pm.Init()
 		require.NoError(t, err)
-		assert.Equal(t, []error{fmt.Errorf(`plugin "test"'s signature has been modified`)}, pm.scanningErrors)
+		assert.Equal(t, []error{fmt.Errorf(`plugin 'test' has a modified signature`)}, pm.scanningErrors)
 		assert.Nil(t, pm.plugins[("test")])
 	})
 }

pkg/plugins/manager/testdata/duplicate-plugins/nested/MANIFEST.txt

@@ -0,0 +1,28 @@
+
+-----BEGIN PGP SIGNED MESSAGE-----
+Hash: SHA512
+
+{
+  "manifestVersion": "2.0.0",
+  "signatureType": "grafana",
+  "signedByOrg": "grafana",
+  "signedByOrgName": "Grafana Labs",
+  "plugin": "test-app",
+  "version": "1.0.0",
+  "time": 1621412405893,
+  "keyId": "7e4d0c6a708866e7",
+  "files": {
+    "plugin.json": "e2c9f711796252bdde63b19691b248aaabea361f521fff6de8ded8d95a333609",
+    "nested/plugin.json": "d4aee2052f5f9aaa3eecc90e5c5d9568efcd2d97595cd77fdcd1de0ada922638"
+  }
+}
+-----BEGIN PGP SIGNATURE-----
+Version: OpenPGP.js v4.10.1
+Comment: https://openpgpjs.org
+
+wqIEARMKAAYFAmCkyjYACgkQfk0ManCIZufMrgIJAWouS5oNJixNsold48Jw
+BuCFtAT7cP0rqxiyu/Z1c06IIVcmEJg/KngcUDhP8bEN4xAunP7KfZktmGp9
++8OqVbd/AgkBg9tWWgnMWln8XENca0ou1PTd/y24embsK3UNweqBAJPDL9el
+nnmA5UWS7pFiHQTLp/daE08o2FGclRbgHcOtFBI=
+=cSQ0
+-----END PGP SIGNATURE-----

pkg/plugins/manager/testdata/duplicate-plugins/nested/nested/MANIFEST.txt

@@ -0,0 +1,27 @@
+
+-----BEGIN PGP SIGNED MESSAGE-----
+Hash: SHA512
+
+{
+  "manifestVersion": "2.0.0",
+  "signatureType": "grafana",
+  "signedByOrg": "grafana",
+  "signedByOrgName": "Grafana Labs",
+  "plugin": "test-app",
+  "version": "1.0.0",
+  "time": 1621411638406,
+  "keyId": "7e4d0c6a708866e7",
+  "files": {
+    "plugin.json": "d4aee2052f5f9aaa3eecc90e5c5d9568efcd2d97595cd77fdcd1de0ada922638"
+  }
+}
+-----BEGIN PGP SIGNATURE-----
+Version: OpenPGP.js v4.10.1
+Comment: https://openpgpjs.org
+
+wqIEARMKAAYFAmCkxzYACgkQfk0ManCIZucKRAIJAUqsvNDA1GaHdMSQ4h+3
+lOXkvN7xMbzOpRvC3Wu7agfsNgmaQtctL/502jUpH94J6aItg7Wmx+mtvVGj
+5i456DitAgkBWDQU7KMAYRhAPNToRZhAdIBr0UXEOS6P9sM+xDuQ/gjZ2J+/
+Iy8j85zhl//0hC/RLspYVxgbZFIHmEto/y+3bbs=
+=1s40
+-----END PGP SIGNATURE-----

pkg/plugins/manager/testdata/test-app/MANIFEST.txt

@@ -0,0 +1,30 @@
+
+-----BEGIN PGP SIGNED MESSAGE-----
+Hash: SHA512
+
+{
+  "manifestVersion": "2.0.0",
+  "signatureType": "grafana",
+  "signedByOrg": "grafana",
+  "signedByOrgName": "Grafana Labs",
+  "plugin": "test-app",
+  "version": "1.0.0",
+  "time": 1621356785895,
+  "keyId": "7e4d0c6a708866e7",
+  "files": {
+    "plugin.json": "c59a51bf6d7ecd7a99608ccb99353390c8b973672a938a0247164324005c0caf",
+    "dashboards/connections.json": "bea86da4be970b98dc4681802ab55cdef3441dc3eb3c654cb207948d17b25303",
+    "dashboards/memory.json": "7c042464941084caa91d0a9a2f188b05315a9796308a652ccdee31ca4fbcbfee",
+    "dashboards/connections_result.json": "124d85c9c2e40214b83273f764574937a79909cfac3f925276fbb72543c224dc"
+  }
+}
+-----BEGIN PGP SIGNATURE-----
+Version: OpenPGP.js v4.10.1
+Comment: https://openpgpjs.org
+
+wqAEARMKAAYFAmCj8PIACgkQfk0ManCIZueQJQII8f8za5CkX59o9A0uG/fH
+imSSFN0YxGfDXSOC1VjNwVQQWSSQ7S08cOX16cQ6huNpxUKCNUGSQnRnQIGg
+CcMMRU0CB3kZB1y8mcgq8Lcy2wiIjGhWRLmvckRSPotnyPM5hGkxQCIAX/k4
+VXEh4FM7q7uTiktRjRgrUMNXnszq4FJInkU8
+=DVFL
+-----END PGP SIGNATURE-----

pkg/plugins/manager/testdata/unsigned-panel/plugin/plugin.json

@@ -0,0 +1,12 @@
+{
+  "type": "panel",
+  "name": "Test",
+  "id": "test-panel",
+  "info": {
+    "description": "Test panel",
+    "author": {
+      "name": "Grafana Labs",
+      "url": "https://grafana.com"
+    }
+  }
+}