Plugins: Always validate root URL if specified in signature manfiest (#52332)

* always validate root URL if specified in signature

* tidy imports

pkg/plugins/manager/loader/loader_test.go

@@ -343,6 +343,38 @@ func TestLoader_Load(t *testing.T) {
 				},
 			},
 		},
+		{
+			name:  "Load a plugin with manifest which has a file not found in plugin folder",
+			class: plugins.External,
+			cfg: &plugins.Cfg{
+				PluginsPath:          filepath.Join(parentDir),
+				PluginsAllowUnsigned: []string{"test"},
+			},
+			pluginPaths: []string{"../testdata/invalid-v2-missing-file"},
+			want:        []*plugins.Plugin{},
+			pluginErrors: map[string]*plugins.Error{
+				"test": {
+					PluginID:  "test",
+					ErrorCode: "signatureModified",
+				},
+			},
+		},
+		{
+			name:  "Load a plugin with file which is missing from the manifest",
+			class: plugins.External,
+			cfg: &plugins.Cfg{
+				PluginsPath:          filepath.Join(parentDir),
+				PluginsAllowUnsigned: []string{"test"},
+			},
+			pluginPaths: []string{"../testdata/invalid-v2-extra-file"},
+			want:        []*plugins.Plugin{},
+			pluginErrors: map[string]*plugins.Error{
+				"test": {
+					PluginID:  "test",
+					ErrorCode: "signatureModified",
+				},
+			},
+		},
 		{
 			name:  "Load an app with includes",
 			class: plugins.External,

pkg/plugins/manager/signature/manifest.go

@@ -145,7 +145,7 @@ func Calculate(mlog log.Logger, plugin *plugins.Plugin) (plugins.Signature, erro
 	}
 
 	// Validate that private is running within defined root URLs
-	if manifest.SignatureType == plugins.PrivateSignature {
+	if manifest.SignatureType == plugins.PrivateSignature || len(manifest.RootURLs) > 0 {
 		appURL, err := url.Parse(setting.AppUrl)
 		if err != nil {
 			return plugins.Signature{}, err

pkg/plugins/manager/signature/manifest_test.go

@@ -1,11 +1,14 @@
 package signature
 
 import (
+	"path/filepath"
 	"sort"
 	"strings"
 	"testing"
 
+	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/plugins"
+	"github.com/grafana/grafana/pkg/setting"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -113,6 +116,57 @@ khdr/tZ1PDgRxMqB/u+Vtbpl0xSxgblnrDOYMSI=
 	})
 }
 
+func TestCalculate(t *testing.T) {
+	t.Run("Validate root URL against App URL for non-private plugin if is specified in manifest", func(t *testing.T) {
+		tcs := []struct {
+			appURL            string
+			expectedSignature plugins.Signature
+		}{
+			{
+				appURL: "https://dev.grafana.com",
+				expectedSignature: plugins.Signature{
+					Status:     plugins.SignatureValid,
+					Type:       plugins.GrafanaSignature,
+					SigningOrg: "Grafana Labs",
+				},
+			},
+			{
+				appURL: "https://non.matching.url.com",
+				expectedSignature: plugins.Signature{
+					Status: plugins.SignatureInvalid,
+				},
+			},
+		}
+
+		parentDir, err := filepath.Abs("../")
+		if err != nil {
+			t.Errorf("could not construct absolute path of current dir")
+			return
+		}
+
+		for _, tc := range tcs {
+			origAppURL := setting.AppUrl
+			t.Cleanup(func() {
+				setting.AppUrl = origAppURL
+			})
+			setting.AppUrl = tc.appURL
+
+			sig, err := Calculate(log.NewNopLogger(), &plugins.Plugin{
+				JSONData: plugins.JSONData{
+					ID: "test",
+					Info: plugins.Info{
+						Version: "1.0.0",
+					},
+				},
+				PluginDir: filepath.Join(parentDir, "testdata/non-pvt-with-root-url/plugin"),
+				Class:     plugins.External,
+			})
+			require.NoError(t, err)
+			require.Equal(t, tc.expectedSignature, sig)
+		}
+	})
+}
+
 func fileList(manifest *pluginManifest) []string {
 	var keys []string
 	for k := range manifest.Files {

pkg/plugins/manager/testdata/non-pvt-with-root-url/plugin/MANIFEST.txt

@@ -0,0 +1,31 @@
+
+-----BEGIN PGP SIGNED MESSAGE-----
+Hash: SHA512
+
+{
+  "manifestVersion": "2.0.0",
+  "signatureType": "grafana",
+  "signedByOrg": "grafana",
+  "signedByOrgName": "Grafana Labs",
+  "rootUrls": [
+    "https://dev.grafana.com/"
+  ],
+  "plugin": "test",
+  "version": "1.0.0",
+  "time": 1657888677250,
+  "keyId": "7e4d0c6a708866e7",
+  "files": {
+    "plugin.json": "2bb467c0bfd6c454551419efe475b8bf8573734e73c7bab52b14842adb62886f"
+  }
+}
+-----BEGIN PGP SIGNATURE-----
+Version: OpenPGP.js v4.10.10
+Comment: https://openpgpjs.org
+
+wrgEARMKAAYFAmLRX6UAIQkQfk0ManCIZucWIQTzOyW2kQdOhGNlcPN+TQxq
+cIhm5wu9Agjhh5II2OyqsYDUqajO9KtwMzAnEMwaT5Kj0oCOsjJruoT/jLz6
+HO7ioenfCwqNxaJswuFkvpN+5BnrrbIwXDo1mgIJARFtKuRg1t4TK2DPcMiQ
+IiEWNrFGK0jCFaofroH1sGnhjNqUy6JAIUQlUn17BHwiJdBqpsihW1HvPhMa
+8KOdLWED
+=D70r
+-----END PGP SIGNATURE-----

pkg/plugins/manager/testdata/non-pvt-with-root-url/plugin/plugin.json

@@ -0,0 +1,16 @@
+{
+  "type": "datasource",
+  "name": "Test",
+  "id": "test",
+  "backend": true,
+  "executable": "test",
+  "state": "alpha",
+  "info": {
+    "version": "1.0.0",
+    "description": "Test",
+    "author": {
+      "name": "Will Browne",
+      "url": "https://willbrowne.com"
+    }
+  }
+}