Plugins: Add validation for plugin manifest (#52787)

* add validation for plugin manifest

* no more semver checking

* undo go.mod changes

* undo go.mod changes

* only validate v2 fields where necessary

* remove manifest version field check

pkg/plugins/manager/signature/manifest.go

@@ -16,6 +16,7 @@ import (
 	"strings"
 
 	"github.com/gobwas/glob"
+
 	// TODO: replace deprecated `golang.org/x/crypto` package https://github.com/grafana/grafana/issues/46050
 	// nolint:staticcheck
 	"golang.org/x/crypto/openpgp"
@@ -88,15 +89,8 @@ func readPluginManifest(body []byte) (*pluginManifest, error) {
 		return nil, fmt.Errorf("%v: %w", "Error parsing manifest JSON", err)
 	}
 
-	keyring, err := openpgp.ReadArmoredKeyRing(bytes.NewBufferString(publicKeyText))
-	if err != nil {
-		return nil, fmt.Errorf("%v: %w", "failed to parse public key", err)
-	}
-
-	if _, err := openpgp.CheckDetachedSignature(keyring,
-		bytes.NewBuffer(block.Bytes),
-		block.ArmoredSignature.Body); err != nil {
-		return nil, fmt.Errorf("%v: %w", "failed to check signature", err)
+	if err = validateManifest(manifest, block); err != nil {
+		return nil, err
 	}
 
 	return &manifest, nil
@@ -132,7 +126,7 @@ func Calculate(mlog log.Logger, plugin *plugins.Plugin) (plugins.Signature, erro
 
 	manifest, err := readPluginManifest(byteValue)
 	if err != nil {
-		mlog.Debug("Plugin signature invalid", "id", plugin.ID)
+		mlog.Debug("Plugin signature invalid", "id", plugin.ID, "err", err)
 		return plugins.Signature{
 			Status: plugins.SignatureInvalid,
 		}, nil
@@ -314,3 +308,52 @@ func urlMatch(specs []string, target string, signatureType plugins.SignatureType
 	}
 	return false, nil
 }
+
+type invalidFieldErr struct {
+	field string
+}
+
+func (r invalidFieldErr) Error() string {
+	return fmt.Sprintf("valid manifest field %s is required", r.field)
+}
+
+func validateManifest(m pluginManifest, block *clearsign.Block) error {
+	if len(m.Plugin) == 0 {
+		return invalidFieldErr{field: "plugin"}
+	}
+	if len(m.Version) == 0 {
+		return invalidFieldErr{field: "version"}
+	}
+	if len(m.KeyID) == 0 {
+		return invalidFieldErr{field: "keyId"}
+	}
+	if m.Time == 0 {
+		return invalidFieldErr{field: "time"}
+	}
+	if len(m.Files) == 0 {
+		return invalidFieldErr{field: "files"}
+	}
+	if m.isV2() {
+		if len(m.SignedByOrg) == 0 {
+			return invalidFieldErr{field: "signedByOrg"}
+		}
+		if len(m.SignedByOrgName) == 0 {
+			return invalidFieldErr{field: "signedByOrgName"}
+		}
+		if !m.SignatureType.IsValid() {
+			return fmt.Errorf("%s is not a valid signature type", m.SignatureType)
+		}
+	}
+	keyring, err := openpgp.ReadArmoredKeyRing(bytes.NewBufferString(publicKeyText))
+	if err != nil {
+		return fmt.Errorf("%v: %w", "failed to parse public key", err)
+	}
+
+	if _, err = openpgp.CheckDetachedSignature(keyring,
+		bytes.NewBuffer(block.Bytes),
+		block.ArmoredSignature.Body); err != nil {
+		return fmt.Errorf("%v: %w", "failed to check signature", err)
+	}
+
+	return nil
+}

pkg/plugins/manager/signature/manifest_test.go

@@ -447,3 +447,87 @@ func Test_urlMatch_private(t *testing.T) {
 		})
 	}
 }
+
+func Test_validateManifest(t *testing.T) {
+	tcs := []struct {
+		name        string
+		manifest    *pluginManifest
+		expectedErr string
+	}{
+		{
+			name:        "Empty plugin field",
+			manifest:    createV2Manifest(t, func(m *pluginManifest) { m.Plugin = "" }),
+			expectedErr: "valid manifest field plugin is required",
+		},
+		{
+			name:        "Empty keyId field",
+			manifest:    createV2Manifest(t, func(m *pluginManifest) { m.KeyID = "" }),
+			expectedErr: "valid manifest field keyId is required",
+		},
+		{
+			name:        "Empty signedByOrg field",
+			manifest:    createV2Manifest(t, func(m *pluginManifest) { m.SignedByOrg = "" }),
+			expectedErr: "valid manifest field signedByOrg is required",
+		},
+		{
+			name:        "Empty signedByOrgName field",
+			manifest:    createV2Manifest(t, func(m *pluginManifest) { m.SignedByOrgName = "" }),
+			expectedErr: "valid manifest field SignedByOrgName is required",
+		},
+		{
+			name:        "Empty signatureType field",
+			manifest:    createV2Manifest(t, func(m *pluginManifest) { m.SignatureType = "" }),
+			expectedErr: "valid manifest field signatureType is required",
+		},
+		{
+			name:        "Invalid signatureType field",
+			manifest:    createV2Manifest(t, func(m *pluginManifest) { m.SignatureType = "invalidSignatureType" }),
+			expectedErr: "valid manifest field signatureType is required",
+		},
+		{
+			name:        "Empty files field",
+			manifest:    createV2Manifest(t, func(m *pluginManifest) { m.Files = map[string]string{} }),
+			expectedErr: "valid manifest field files is required",
+		},
+		{
+			name:        "Empty time field",
+			manifest:    createV2Manifest(t, func(m *pluginManifest) { m.Time = 0 }),
+			expectedErr: "valid manifest field time is required",
+		},
+		{
+			name:        "Empty version field",
+			manifest:    createV2Manifest(t, func(m *pluginManifest) { m.Version = "" }),
+			expectedErr: "valid manifest field version is required",
+		},
+	}
+	for _, tc := range tcs {
+		t.Run(tc.name, func(t *testing.T) {
+			err := validateManifest(*tc.manifest, nil)
+			require.Errorf(t, err, tc.expectedErr)
+		})
+	}
+}
+
+func createV2Manifest(t *testing.T, cbs ...func(*pluginManifest)) *pluginManifest {
+	t.Helper()
+
+	m := &pluginManifest{
+		Plugin:  "grafana-test-app",
+		Version: "2.5.3",
+		KeyID:   "7e4d0c6a708866e7",
+		Time:    1586817677115,
+		Files: map[string]string{
+			"plugin.json": "55556b845e91935cc48fae3aa67baf0f22694c3f",
+		},
+		ManifestVersion: "2.0.0",
+		SignatureType:   plugins.GrafanaSignature,
+		SignedByOrg:     "grafana",
+		SignedByOrgName: "grafana",
+	}
+
+	for _, cb := range cbs {
+		cb(m)
+	}
+
+	return m
+}

pkg/plugins/models.go

@@ -177,10 +177,20 @@ type SignatureType string
 
 const (
 	GrafanaSignature     SignatureType = "grafana"
+	CommercialSignature  SignatureType = "commercial"
+	CommunitySignature   SignatureType = "community"
 	PrivateSignature     SignatureType = "private"
 	PrivateGlobSignature SignatureType = "private-glob"
 )
 
+func (s SignatureType) IsValid() bool {
+	switch s {
+	case GrafanaSignature, CommercialSignature, CommunitySignature, PrivateSignature, PrivateGlobSignature:
+		return true
+	}
+	return false
+}
+
 type PluginFiles map[string]struct{}
 
 type Signature struct {