Always require signatures for prepackaged plugins (#31785)

* Always require signatures for prepackaged plugins

We have always required signatures for packages installed via the marketplace -- whether remotely satisfied, or sourced from the prepackaged plugin cache.

However, prepackaged plugins discovered and automatically installed on
startup did not require a valid signature. Since we already ship
signatures for all Mattermost-authored prepackaged plugins, it's easy to
simply start requiring this.

Distributions of Mattermost that bundle their own prepackaged plugins
will have to include their own signatures. This in turn requires
distributing and configuring Mattermost with a custom public key via
`PluginSettings.SignaturePublicKeyFiles`.

Note that this enhanced security is neutered with a deployment that uses
a file-based `config.json`, as any exploit that allows appending to the
prepackaged plugins cache probably also allows modifying `config.json`
to register a new public key. A [database-based
config](https://docs.mattermost.com/configure/configuration-in-your-database.html)
is recommended.

Finally, we already support an optional setting
`PluginSettings.RequirePluginSignature` to always require a plugin
signature, although this effectively disables plugin uploads and
requires extra effort to deploy the corresponding signature. In
environments where only prepackaged plugins are used, this setting is
ideal.

Fixes: https://mattermost.atlassian.net/browse/MM-64627

* setup dev key, expect no plugins if sig fails

* Fix shadow variable errors in test helpers

Pre-declare signaturePublicKey variable in loops to avoid shadowing
the outer err variable used in error handling.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* Replace PrepackagedPlugin.Signature with SignaturePath for memory efficiency

- Changed PrepackagedPlugin struct to use SignaturePath string instead of Signature []byte
- Updated buildPrepackagedPlugin to use file descriptor instead of reading signature into memory
- Modified plugin installation and persistence to read from signature file paths
- Updated all tests to check SignaturePath instead of Signature field
- Removed unused bytes import from plugin.go

This change reduces memory usage by storing file paths instead of signature data
in memory while maintaining the same security verification functionality.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

---------

Co-authored-by: Claude <noreply@anthropic.com>

server/channels/api4/apitestlib.go

@@ -127,6 +127,12 @@ func setupTestHelper(tb testing.TB, dbStore store.Store, sqlSettings *model.SqlS
 		updateConfig(memoryConfig)
 	}
 	memoryStore.Set(memoryConfig)
+	for _, signaturePublicKeyFile := range memoryConfig.PluginSettings.SignaturePublicKeyFiles {
+		var signaturePublicKey []byte
+		signaturePublicKey, err = os.ReadFile(signaturePublicKeyFile)
+		require.NoError(tb, err, "failed to read signature public key file %s", signaturePublicKeyFile)
+		memoryStore.SetFile(signaturePublicKeyFile, signaturePublicKey)
+	}
 
 	configStore, err := config.NewStoreFromBacking(memoryStore, nil, false)
 	require.NoError(tb, err)

server/channels/api4/plugin_test.go

@@ -1406,16 +1406,18 @@ func TestGetPrepackagedPlaybooksPluginIn(t *testing.T) {
 }
 
 func TestInstallMarketplacePlugin(t *testing.T) {
-	th := Setup(t).InitBasic()
-	defer th.TearDown()
+	path, _ := fileutils.FindDir("tests")
 
-	th.App.UpdateConfig(func(cfg *model.Config) {
+	th := SetupConfig(t, func(cfg *model.Config) {
 		*cfg.PluginSettings.Enable = true
 		*cfg.PluginSettings.EnableUploads = true
 		*cfg.PluginSettings.EnableMarketplace = false
-	})
+		cfg.PluginSettings.SignaturePublicKeyFiles = []string{
+			filepath.Join(path, "development-private-key.asc"),
+		}
+	}).InitBasic()
+	defer th.TearDown()
 
-	path, _ := fileutils.FindDir("tests")
 	signatureFilename := "testplugin2.tar.gz.sig"
 	signatureFileReader, err := os.Open(filepath.Join(path, signatureFilename))
 	require.NoError(t, err)
@@ -1581,11 +1583,6 @@ func TestInstallMarketplacePlugin(t *testing.T) {
 			*cfg.PluginSettings.MarketplaceURL = testServer.URL
 		})
 
-		key, err := os.Open(filepath.Join(path, "development-private-key.asc"))
-		require.NoError(t, err)
-		appErr := th.App.AddPublicKey("pub_key", key)
-		require.Nil(t, appErr)
-
 		pRequest := &model.InstallMarketplacePluginRequest{Id: "testplugin2"}
 		manifest, _, err := client.InstallMarketplacePlugin(context.Background(), pRequest)
 		require.NoError(t, err)
@@ -1627,11 +1624,6 @@ func TestInstallMarketplacePlugin(t *testing.T) {
 			*cfg.PluginSettings.MarketplaceURL = testServer.URL
 		})
 
-		key, err := os.Open(filepath.Join(path, "development-private-key.asc"))
-		require.NoError(t, err)
-		appErr := th.App.AddPublicKey("pub_key", key)
-		require.Nil(t, appErr)
-
 		pRequest := &model.InstallMarketplacePluginRequest{Id: "testplugin2", Version: "9.9.9"}
 		manifest, _, err := client.InstallMarketplacePlugin(context.Background(), pRequest)
 		require.NoError(t, err)
@@ -1833,24 +1825,14 @@ func TestInstallMarketplacePluginPrepackagedDisabled(t *testing.T) {
 		th := SetupConfig(t, func(cfg *model.Config) {
 			// Disable auto-installing prepackaged plugins
 			*cfg.PluginSettings.AutomaticPrepackagedPlugins = false
+			cfg.PluginSettings.SignaturePublicKeyFiles = []string{
+				filepath.Join(path, "development-private-key.asc"),
+			}
 		}).InitBasic()
 		defer th.TearDown()
 
 		th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) {
-			pluginSignatureFile, err := os.Open(filepath.Join(path, "testplugin.tar.gz.asc"))
-			require.NoError(t, err)
-			pluginSignatureData, err := io.ReadAll(pluginSignatureFile)
-			require.NoError(t, err)
-
-			key, err := os.Open(filepath.Join(path, "development-private-key.asc"))
-			require.NoError(t, err)
-			appErr := th.App.AddPublicKey("pub_key", key)
-			require.Nil(t, appErr)
-
-			t.Cleanup(func() {
-				appErr = th.App.DeletePublicKey("pub_key")
-				require.Nil(t, appErr)
-			})
+			expectedSignaturePath := filepath.Join(prepackagedPluginsDir, "testplugin.tar.gz.sig")
 
 			testServer := httptest.NewServer(http.HandlerFunc(func(res http.ResponseWriter, req *http.Request) {
 				serverVersion := req.URL.Query().Get("server_version")
@@ -1896,7 +1878,7 @@ func TestInstallMarketplacePluginPrepackagedDisabled(t *testing.T) {
 				plugins := env.PrepackagedPlugins()
 				require.Len(t, plugins, 1)
 				require.Equal(t, "testplugin", plugins[0].Manifest.Id)
-				require.Equal(t, pluginSignatureData, plugins[0].Signature)
+				require.Equal(t, expectedSignaturePath, plugins[0].SignaturePath)
 
 				pluginsResp, _, err = client.GetPlugins(context.Background())
 				require.NoError(t, err)
@@ -1956,7 +1938,7 @@ func TestInstallMarketplacePluginPrepackagedDisabled(t *testing.T) {
 				assert.Equal(t, "0.0.1", manifest.Version)
 			})
 
-			t.Run("Install both a prepacked and a Marketplace plugin", func(t *testing.T) {
+			t.Run("Install both a prepackaged and a Marketplace plugin", func(t *testing.T) {
 				pRequest := &model.InstallMarketplacePluginRequest{Id: "testplugin"}
 				manifest1, _, err := client.InstallMarketplacePlugin(context.Background(), pRequest)
 				require.NoError(t, err)
@@ -1993,9 +1975,6 @@ func TestInstallMarketplacePluginPrepackagedDisabled(t *testing.T) {
 					},
 				})
 			})
-
-			appErr = th.App.DeletePublicKey("pub_key")
-			require.Nil(t, appErr)
 		})
 	})
 
@@ -2016,15 +1995,13 @@ func TestInstallMarketplacePluginPrepackagedDisabled(t *testing.T) {
 		th := SetupConfig(t, func(cfg *model.Config) {
 			// Disable auto-installing prepackaged plugins
 			*cfg.PluginSettings.AutomaticPrepackagedPlugins = false
+			cfg.PluginSettings.SignaturePublicKeyFiles = []string{
+				filepath.Join(path, "development-private-key.asc"),
+			}
 		}).InitBasic()
 		defer th.TearDown()
 
 		th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) {
-			key, err := os.Open(filepath.Join(path, "development-private-key.asc"))
-			require.NoError(t, err)
-			appErr := th.App.AddPublicKey("pub_key", key)
-			require.Nil(t, appErr)
-
 			testServer := httptest.NewServer(http.HandlerFunc(func(res http.ResponseWriter, req *http.Request) {
 				serverVersion := req.URL.Query().Get("server_version")
 				require.NotEmpty(t, serverVersion)
@@ -2050,9 +2027,7 @@ func TestInstallMarketplacePluginPrepackagedDisabled(t *testing.T) {
 
 			env := th.App.GetPluginsEnvironment()
 			plugins := env.PrepackagedPlugins()
-			require.Len(t, plugins, 1)
-			require.Equal(t, "testplugin", plugins[0].Manifest.Id)
-			require.Empty(t, plugins[0].Signature)
+			require.Len(t, plugins, 0)
 
 			pluginsResp, _, err := client.GetPlugins(context.Background())
 			require.NoError(t, err)
@@ -2080,10 +2055,6 @@ func TestInstallMarketplacePluginPrepackagedDisabled(t *testing.T) {
 			require.NoError(t, err)
 			require.Len(t, pluginsResp.Active, 0)
 			require.Len(t, pluginsResp.Inactive, 0)
-
-			// Clean up
-			appErr = th.App.DeletePublicKey("pub_key")
-			require.Nil(t, appErr)
 		})
 	})
 }

server/channels/app/helper_test.go

@@ -82,6 +82,13 @@ func setupTestHelper(dbStore store.Store, sqlStore *sqlstore.SqlStore, sqlSettin
 	if updateConfig != nil {
 		updateConfig(memoryConfig)
 	}
+
+	for _, signaturePublicKeyFile := range memoryConfig.PluginSettings.SignaturePublicKeyFiles {
+		var signaturePublicKey []byte
+		signaturePublicKey, err = os.ReadFile(signaturePublicKeyFile)
+		require.NoError(tb, err, "failed to read signature public key file %s", signaturePublicKeyFile)
+		configStore.SetFile(signaturePublicKeyFile, signaturePublicKey)
+	}
 	configStore.Set(memoryConfig)
 
 	buffer := &mlog.Buffer{}

server/channels/app/plugin.go

@@ -4,7 +4,6 @@
 package app
 
 import (
-	"bytes"
 	"encoding/base64"
 	"fmt"
 	"io"
@@ -316,7 +315,11 @@ func (ch *Channels) syncPlugins() *model.AppError {
 		wg.Add(1)
 		go func(plugin *pluginSignaturePath) {
 			defer wg.Done()
-			logger := ch.srv.Log().With(mlog.String("plugin_id", plugin.pluginID), mlog.String("bundle_path", plugin.bundlePath))
+			logger := ch.srv.Log().With(
+				mlog.String("plugin_id", plugin.pluginID),
+				mlog.String("bundle_path", plugin.bundlePath),
+				mlog.String("signature_path", plugin.signaturePath),
+			)
 
 			bundle, appErr := ch.srv.fileReader(plugin.bundlePath)
 			if appErr != nil {
@@ -333,7 +336,7 @@ func (ch *Channels) syncPlugins() *model.AppError {
 				}
 				defer signature.Close()
 
-				if appErr = ch.verifyPlugin(bundle, signature); appErr != nil {
+				if appErr = ch.verifyPlugin(logger, bundle, signature); appErr != nil {
 					logger.Error("Failed to validate plugin signature", mlog.Err(appErr))
 					return
 				}
@@ -923,13 +926,19 @@ func (ch *Channels) getPluginsFromFilePaths(fileStorePaths []string) map[string]
 // If enabled, prepackaged plugins are installed or upgraded locally. A list of transitionally
 // prepackaged plugins is also collected for later persistence to the filestore.
 func (ch *Channels) processPrepackagedPlugins(prepackagedPluginsDir string) error {
+	logger := ch.srv.Log()
+	logger.Info("Processing prepackaged plugin")
+
 	prepackagedPluginsPath, found := fileutils.FindDir(prepackagedPluginsDir)
 	if !found {
-		ch.srv.Log().Debug("No prepackaged plugins directory found")
+		logger.Debug("No prepackaged plugins directory found")
 		return nil
 	}
 
-	ch.srv.Log().Debug("Processing prepackaged plugins in directory", mlog.String("path", prepackagedPluginsPath))
+	logger = logger.With(
+		mlog.String("prepackaged_plugins_path", prepackagedPluginsPath),
+	)
+	ch.srv.Log().Debug("Processing prepackaged plugins in directory")
 
 	var fileStorePaths []string
 	err := filepath.Walk(prepackagedPluginsPath, func(walkPath string, info os.FileInfo, err error) error {
@@ -971,7 +980,7 @@ func (ch *Channels) processPrepackagedPlugins(prepackagedPluginsDir string) erro
 				if errors.As(err, &appErr) && appErr.Id == "app.plugin.skip_installation.app_error" {
 					return
 				}
-				ch.srv.Log().Error("Failed to install prepackaged plugin", mlog.String("bundle_path", psPath.bundlePath), mlog.Err(err))
+				logger.Error("Failed to install prepackaged plugin", mlog.String("bundle_path", psPath.bundlePath), mlog.Err(err))
 				return
 			}
 
@@ -1004,7 +1013,10 @@ var SemVerV2 = semver.MustParse("2.0.0")
 // processPrepackagedPlugin will return the prepackaged plugin metadata and will also
 // install the prepackaged plugin if it had been previously enabled and AutomaticPrepackagedPlugins is true.
 func (ch *Channels) processPrepackagedPlugin(pluginPath *pluginSignaturePath) (*plugin.PrepackagedPlugin, error) {
-	logger := ch.srv.Log().With(mlog.String("bundle_path", pluginPath.bundlePath))
+	logger := ch.srv.Log().With(
+		mlog.String("bundle_path", pluginPath.bundlePath),
+		mlog.String("signature_path", pluginPath.signaturePath),
+	)
 
 	logger.Info("Processing prepackaged plugin")
 
@@ -1020,7 +1032,7 @@ func (ch *Channels) processPrepackagedPlugin(pluginPath *pluginSignaturePath) (*
 	}
 	defer os.RemoveAll(tmpDir)
 
-	plugin, pluginDir, err := ch.buildPrepackagedPlugin(pluginPath, fileReader, tmpDir)
+	plugin, pluginDir, err := ch.buildPrepackagedPlugin(logger, pluginPath, fileReader, tmpDir)
 	if err != nil {
 		return nil, errors.Wrapf(err, "Failed to get prepackaged plugin %s", pluginPath.bundlePath)
 	}
@@ -1204,23 +1216,34 @@ func (ch *Channels) persistTransitionallyPrepackagedPlugins() {
 		go func(p *plugin.PrepackagedPlugin) {
 			defer wg.Done()
 
-			logger := ch.srv.Log().With(mlog.String("plugin_id", p.Manifest.Id), mlog.String("version", p.Manifest.Version))
+			logger := ch.srv.Log().With(
+				mlog.String("plugin_id", p.Manifest.Id),
+				mlog.String("version", p.Manifest.Version),
+				mlog.String("bundle_path", p.Path),
+				mlog.String("signature_path", p.SignaturePath),
+			)
 
 			logger.Info("Persisting transitionally prepackaged plugin")
 
 			bundleReader, err := os.Open(p.Path)
 			if err != nil {
 				logger.Error("Failed to read transitionally prepackaged plugin", mlog.Err(err))
+				return
 			}
 			defer bundleReader.Close()
 
-			signatureReader := bytes.NewReader(p.Signature)
+			signatureReader, err := os.Open(p.SignaturePath)
+			if err != nil {
+				logger.Error("Failed to read transitionally prepackaged plugin signature", mlog.Err(err))
+				return
+			}
+			defer signatureReader.Close()
 
 			// Write the plugin to the filestore, but don't bother notifying the peers,
 			// as there's no reason to reload the plugin to run the same version again.
 			appErr := ch.installPluginToFilestore(p.Manifest, bundleReader, signatureReader)
 			if appErr != nil {
-				logger.Error("Failed to persist transitionally prepackaged plugin", mlog.Err(err))
+				logger.Error("Failed to persist transitionally prepackaged plugin", mlog.Err(appErr))
 			}
 		}(p)
 	}
@@ -1231,7 +1254,31 @@ func (ch *Channels) persistTransitionallyPrepackagedPlugins() {
 }
 
 // buildPrepackagedPlugin builds a PrepackagedPlugin from the plugin at the given path, additionally returning the directory in which it was extracted.
-func (ch *Channels) buildPrepackagedPlugin(pluginPath *pluginSignaturePath, pluginFile io.ReadSeeker, tmpDir string) (*plugin.PrepackagedPlugin, string, error) {
+func (ch *Channels) buildPrepackagedPlugin(logger *mlog.Logger, pluginPath *pluginSignaturePath, pluginFile io.ReadSeeker, tmpDir string) (*plugin.PrepackagedPlugin, string, error) {
+	// Always require signature for prepackaged plugins
+	if pluginPath.signaturePath == "" {
+		return nil, "", errors.Errorf("Prepackaged plugin missing required signature file")
+	}
+
+	// Open signature file
+	signatureFile, sigErr := os.Open(pluginPath.signaturePath)
+	if sigErr != nil {
+		return nil, "", errors.Wrapf(sigErr, "Failed to open prepackaged plugin signature %s", pluginPath.signaturePath)
+	}
+	defer signatureFile.Close()
+
+	// Verify signature extraction
+	if _, err := pluginFile.Seek(0, io.SeekStart); err != nil {
+		return nil, "", errors.Wrapf(err, "Failed to seek to start of plugin file for signature verification: %s", pluginPath.bundlePath)
+	}
+	if appErr := ch.verifyPlugin(logger, pluginFile, signatureFile); appErr != nil {
+		return nil, "", errors.Wrapf(appErr, "Prepackaged plugin signature verification failed for %s using %s", pluginPath.bundlePath, pluginPath.signaturePath)
+	}
+
+	// Extract plugin after signature verification
+	if _, err := pluginFile.Seek(0, io.SeekStart); err != nil {
+		return nil, "", errors.Wrapf(err, "Failed to seek to start of plugin file for extraction: %s", pluginPath.bundlePath)
+	}
 	manifest, pluginDir, appErr := extractPlugin(pluginFile, tmpDir)
 	if appErr != nil {
 		return nil, "", errors.Wrapf(appErr, "Failed to extract plugin with path %s", pluginPath.bundlePath)
@@ -1240,24 +1287,12 @@ func (ch *Channels) buildPrepackagedPlugin(pluginPath *pluginSignaturePath, plug
 	plugin := new(plugin.PrepackagedPlugin)
 	plugin.Manifest = manifest
 	plugin.Path = pluginPath.bundlePath
-
-	if pluginPath.signaturePath != "" {
-		sig := pluginPath.signaturePath
-		sigReader, sigErr := os.Open(sig)
-		if sigErr != nil {
-			return nil, "", errors.Wrapf(sigErr, "Failed to open prepackaged plugin signature %s", sig)
-		}
-		bytes, sigErr := io.ReadAll(sigReader)
-		if sigErr != nil {
-			return nil, "", errors.Wrapf(sigErr, "Failed to read prepackaged plugin signature %s", sig)
-		}
-		plugin.Signature = bytes
-	}
+	plugin.SignaturePath = pluginPath.signaturePath
 
 	if manifest.IconPath != "" {
 		iconData, err := getIcon(filepath.Join(pluginDir, manifest.IconPath))
 		if err != nil {
-			ch.srv.Log().Warn("Error loading local plugin icon", mlog.String("plugin_id", plugin.Manifest.Id), mlog.String("icon_path", plugin.Manifest.IconPath), mlog.Err(err))
+			logger.Warn("Error loading local plugin icon", mlog.String("icon_path", plugin.Manifest.IconPath), mlog.Err(err))
 		}
 		plugin.IconData = iconData
 	}

server/channels/app/plugin_install.go

@@ -123,6 +123,11 @@ func (ch *Channels) installPluginFromClusterMessage(pluginID string) {
 		return
 	}
 
+	logger = logger.With(
+		mlog.String("bundle_path", plugin.bundlePath),
+		mlog.String("signature_path", plugin.signaturePath),
+	)
+
 	bundle, appErr := ch.srv.fileReader(plugin.bundlePath)
 	if appErr != nil {
 		logger.Error("Failed to open plugin bundle from file store.", mlog.Err(appErr))
@@ -139,7 +144,7 @@ func (ch *Channels) installPluginFromClusterMessage(pluginID string) {
 		}
 		defer signature.Close()
 
-		if err := ch.verifyPlugin(bundle, signature); err != nil {
+		if err := ch.verifyPlugin(logger, bundle, signature); err != nil {
 			logger.Error("Failed to validate plugin signature.", mlog.Err(appErr))
 			return
 		}
@@ -270,7 +275,11 @@ func (ch *Channels) installPluginToFilestore(manifest *model.Manifest, bundle, s
 // plugin bundle from the prepackaged folder, if available, or remotely if EnableRemoteMarketplace
 // is true.
 func (ch *Channels) InstallMarketplacePlugin(request *model.InstallMarketplacePluginRequest) (*model.Manifest, *model.AppError) {
-	logger := ch.srv.Log().With(mlog.String("plugin_id", request.Id))
+	logger := ch.srv.Log().With(
+		mlog.String("plugin_id", request.Id),
+		mlog.String("requested_version", request.Version),
+	)
+	logger.Info("Installing plugin from marketplace")
 
 	var pluginFile, signatureFile io.ReadSeeker
 
@@ -285,8 +294,15 @@ func (ch *Channels) InstallMarketplacePlugin(request *model.InstallMarketplacePl
 		}
 		defer fileReader.Close()
 
+		signatureReader, err := os.Open(prepackagedPlugin.SignaturePath)
+		if err != nil {
+			return nil, model.NewAppError("InstallMarketplacePlugin", "app.plugin.install_marketplace_plugin.app_error", nil, fmt.Sprintf("failed to open prepackaged plugin signature %s", prepackagedPlugin.SignaturePath), http.StatusInternalServerError).Wrap(err)
+		}
+		defer signatureReader.Close()
+
 		pluginFile = fileReader
-		signatureFile = bytes.NewReader(prepackagedPlugin.Signature)
+		signatureFile = signatureReader
+		logger.Debug("Found matching pre-packaged plugin", mlog.String("bundle_path", prepackagedPlugin.Path), mlog.String("signature_path", prepackagedPlugin.SignaturePath))
 	}
 
 	if *ch.cfgSvc.Config().PluginSettings.EnableRemoteMarketplace {
@@ -313,6 +329,8 @@ func (ch *Channels) InstallMarketplacePlugin(request *model.InstallMarketplacePl
 			}
 
 			if prepackagedVersion.LT(marketplaceVersion) { // Always true if no prepackaged plugin was found
+				logger.Debug("Found upgraded plugin from remote marketplace", mlog.String("version", plugin.Manifest.Version), mlog.String("download_url", plugin.DownloadURL))
+
 				downloadedPluginBytes, err := ch.srv.downloadFromURL(plugin.DownloadURL)
 				if err != nil {
 					return nil, model.NewAppError("InstallMarketplacePlugin", "app.plugin.install_marketplace_plugin.app_error", nil, "", http.StatusInternalServerError).Wrap(err)
@@ -323,6 +341,8 @@ func (ch *Channels) InstallMarketplacePlugin(request *model.InstallMarketplacePl
 				}
 				pluginFile = bytes.NewReader(downloadedPluginBytes)
 				signatureFile = signature
+			} else {
+				logger.Debug("Preferring pre-packaged plugin over version in remote marketplace", mlog.String("version", plugin.Manifest.Version), mlog.String("download_url", plugin.DownloadURL))
 			}
 		}
 	}
@@ -334,7 +354,7 @@ func (ch *Channels) InstallMarketplacePlugin(request *model.InstallMarketplacePl
 		return nil, model.NewAppError("InstallMarketplacePlugin", "app.plugin.marketplace_plugins.signature_not_found.app_error", nil, "", http.StatusInternalServerError)
 	}
 
-	appErr = ch.verifyPlugin(pluginFile, signatureFile)
+	appErr = ch.verifyPlugin(logger, pluginFile, signatureFile)
 	if appErr != nil {
 		return nil, appErr
 	}

server/channels/app/plugin_prepackaged_test.go

@@ -0,0 +1,176 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See LICENSE.txt for license information.
+
+package app
+
+import (
+	"bytes"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/mattermost/mattermost/server/public/shared/mlog"
+	"github.com/mattermost/mattermost/server/v8/channels/utils/fileutils"
+)
+
+func TestBuildPrepackagedPlugin(t *testing.T) {
+	mainHelper.Parallel(t)
+
+	testsPath, found := fileutils.FindDir("tests")
+	require.True(t, found, "tests directory not found")
+
+	// Read public key file once for all subtests
+	publicKeyData, err := os.ReadFile(filepath.Join(testsPath, "development-public-key.asc"))
+	require.NoError(t, err)
+
+	t.Run("valid plugin with signature and icon data", func(t *testing.T) {
+		th := Setup(t)
+		defer th.TearDown()
+
+		// Import development public key for signature verification
+		appErr := th.App.AddPublicKey("development-public-key.asc", bytes.NewBuffer(publicKeyData))
+		require.Nil(t, appErr)
+
+		// Create test plugin path
+		pluginPath := &pluginSignaturePath{
+			pluginID:      "testplugin",
+			bundlePath:    filepath.Join(testsPath, "testplugin.tar.gz"),
+			signaturePath: filepath.Join(testsPath, "testplugin.tar.gz.sig"),
+		}
+
+		// Open plugin file
+		pluginFile, err := os.Open(pluginPath.bundlePath)
+		require.NoError(t, err)
+		defer pluginFile.Close()
+
+		// Create logger
+		logger := mlog.CreateConsoleTestLogger(t)
+
+		// Test buildPrepackagedPlugin
+		plugin, pluginDir, err := th.App.ch.buildPrepackagedPlugin(logger, pluginPath, pluginFile, t.TempDir())
+		require.NoError(t, err)
+		require.NotNil(t, plugin)
+		require.NotEmpty(t, pluginDir)
+
+		// Verify plugin fields
+		assert.NotNil(t, plugin.Manifest)
+		assert.Equal(t, pluginPath.bundlePath, plugin.Path)
+		assert.Equal(t, pluginPath.signaturePath, plugin.SignaturePath)
+		assert.Equal(t, "testplugin", plugin.Manifest.Id)
+
+		// Verify plugin has icon data loaded
+		assert.Equal(t, "assets/icon.svg", plugin.Manifest.IconPath)
+		assert.NotEmpty(t, plugin.IconData, "Plugin should have icon data loaded")
+	})
+
+	t.Run("missing signature file", func(t *testing.T) {
+		th := Setup(t)
+		defer th.TearDown()
+
+		// Create plugin path with empty signature path
+		pluginPath := &pluginSignaturePath{
+			pluginID:      "testplugin",
+			bundlePath:    filepath.Join(testsPath, "testplugin.tar.gz"),
+			signaturePath: "", // Empty signature path
+		}
+
+		pluginFile, err := os.Open(pluginPath.bundlePath)
+		require.NoError(t, err)
+		defer pluginFile.Close()
+
+		logger := mlog.CreateConsoleTestLogger(t)
+
+		plugin, pluginDir, err := th.App.ch.buildPrepackagedPlugin(logger, pluginPath, pluginFile, t.TempDir())
+		require.Error(t, err)
+		require.Nil(t, plugin)
+		require.Empty(t, pluginDir)
+
+		assert.Contains(t, err.Error(), "Prepackaged plugin missing required signature file")
+	})
+
+	t.Run("nonexistent signature file", func(t *testing.T) {
+		th := Setup(t)
+		defer th.TearDown()
+
+		pluginPath := &pluginSignaturePath{
+			pluginID:      "testplugin",
+			bundlePath:    filepath.Join(testsPath, "testplugin.tar.gz"),
+			signaturePath: "/nonexistent/signature.sig",
+		}
+
+		pluginFile, err := os.Open(pluginPath.bundlePath)
+		require.NoError(t, err)
+		defer pluginFile.Close()
+
+		logger := mlog.CreateConsoleTestLogger(t)
+
+		plugin, pluginDir, err := th.App.ch.buildPrepackagedPlugin(logger, pluginPath, pluginFile, t.TempDir())
+		require.Error(t, err)
+		require.Nil(t, plugin)
+		require.Empty(t, pluginDir)
+
+		assert.Contains(t, err.Error(), "Failed to open prepackaged plugin signature")
+	})
+
+	t.Run("empty signature file", func(t *testing.T) {
+		th := Setup(t)
+		defer th.TearDown()
+
+		// Import development public key
+		appErr := th.App.AddPublicKey("development-public-key.asc", bytes.NewBuffer(publicKeyData))
+		require.Nil(t, appErr)
+
+		// Create empty signature file
+		tmpSig, err := os.CreateTemp("", "*.sig")
+		require.NoError(t, err)
+		tmpSig.Close()
+		defer os.Remove(tmpSig.Name())
+
+		pluginPath := &pluginSignaturePath{
+			pluginID:      "testplugin",
+			bundlePath:    filepath.Join(testsPath, "testplugin.tar.gz"),
+			signaturePath: tmpSig.Name(),
+		}
+
+		pluginFile, err := os.Open(pluginPath.bundlePath)
+		require.NoError(t, err)
+		defer pluginFile.Close()
+
+		logger := mlog.CreateConsoleTestLogger(t)
+
+		plugin, pluginDir, err := th.App.ch.buildPrepackagedPlugin(logger, pluginPath, pluginFile, t.TempDir())
+		require.Error(t, err)
+		require.Nil(t, plugin)
+		require.Empty(t, pluginDir)
+
+		assert.Contains(t, err.Error(), "Prepackaged plugin signature verification failed")
+	})
+
+	t.Run("signature verification failure", func(t *testing.T) {
+		th := Setup(t)
+		defer th.TearDown()
+
+		// Use mismatched plugin and signature (testplugin.tar.gz with testplugin2.tar.gz.sig)
+		pluginPath := &pluginSignaturePath{
+			pluginID:      "testplugin",
+			bundlePath:    filepath.Join(testsPath, "testplugin.tar.gz"),
+			signaturePath: filepath.Join(testsPath, "testplugin2.tar.gz.sig"),
+		}
+
+		pluginFile, err := os.Open(pluginPath.bundlePath)
+		require.NoError(t, err)
+		defer pluginFile.Close()
+
+		logger := mlog.CreateConsoleTestLogger(t)
+
+		plugin, pluginDir, err := th.App.ch.buildPrepackagedPlugin(logger, pluginPath, pluginFile, t.TempDir())
+		require.Error(t, err)
+		require.Nil(t, plugin)
+		require.Empty(t, pluginDir)
+
+		assert.Contains(t, err.Error(), "Prepackaged plugin signature verification failed")
+	})
+}

server/channels/app/plugin_signature.go

@@ -73,35 +73,36 @@ func (a *App) DeletePublicKey(name string) *model.AppError {
 	return nil
 }
 
-// VerifyPlugin checks that the given signature corresponds to the given plugin and matches a trusted certificate.
-func (a *App) VerifyPlugin(plugin, signature io.ReadSeeker) *model.AppError {
-	return a.ch.verifyPlugin(plugin, signature)
-}
-
-func (ch *Channels) verifyPlugin(plugin, signature io.ReadSeeker) *model.AppError {
+func (ch *Channels) verifyPlugin(logger *mlog.Logger, plugin, signature io.ReadSeeker) *model.AppError {
+	// First try verifying using the hard-coded public key.
 	if err := verifySignature(bytes.NewReader(mattermostPluginPublicKey), plugin, signature); err == nil {
+		logger.Debug("Plugin signature verified using hard-coded public key")
 		return nil
 	}
+
+	// If that fails, try any of the admin-configured public keys.
 	publicKeys := ch.srv.Config().PluginSettings.SignaturePublicKeyFiles
 	for _, pk := range publicKeys {
 		pkBytes, appErr := ch.srv.getPublicKey(pk)
 		if appErr != nil {
-			mlog.Warn("Unable to get public key for ", mlog.String("filename", pk))
+			logger.Warn("Unable to read configured signature public key file", mlog.String("public_key_path", pk))
 			continue
 		}
 		publicKey := bytes.NewReader(pkBytes)
 		if _, err := plugin.Seek(0, io.SeekStart); err != nil {
-			mlog.Warn("Unable to seek in public key reader for ", mlog.String("filename", pk))
+			logger.Warn("Unable to seek in public key reader for ", mlog.String("public_key_path", pk))
 			continue
 		}
 		if _, err := signature.Seek(0, io.SeekStart); err != nil {
-			mlog.Warn("Unable to seek in signature for public key ", mlog.String("filename", pk))
+			logger.Warn("Unable to seek in signature for public key ", mlog.String("public_key_path", pk))
 			continue
 		}
 		if err := verifySignature(publicKey, plugin, signature); err == nil {
+			logger.Debug("Plugin signature verified using configured public key", mlog.String("public_key_path", pk))
 			return nil
 		}
 	}
+
 	return model.NewAppError("VerifyPlugin", "api.plugin.verify_plugin.app_error", nil, "", http.StatusInternalServerError)
 }
 

server/channels/app/plugin_test.go

@@ -478,7 +478,13 @@ func TestGetPluginStatuses(t *testing.T) {
 
 func TestPluginSync(t *testing.T) {
 	mainHelper.Parallel(t)
-	th := Setup(t)
+	path, _ := fileutils.FindDir("tests")
+
+	th := SetupConfig(t, func(cfg *model.Config) {
+		cfg.PluginSettings.SignaturePublicKeyFiles = []string{
+			filepath.Join(path, "development-private-key.asc"),
+		}
+	})
 	defer th.TearDown()
 
 	testCases := []struct {
@@ -527,8 +533,6 @@ func TestPluginSync(t *testing.T) {
 			env := th.App.GetPluginsEnvironment()
 			require.NotNil(t, env)
 
-			path, _ := fileutils.FindDir("tests")
-
 			t.Run("new bundle in the file store", func(t *testing.T) {
 				th.App.UpdateConfig(func(cfg *model.Config) {
 					*cfg.PluginSettings.RequirePluginSignature = false
@@ -610,15 +614,10 @@ func TestPluginSync(t *testing.T) {
 					*cfg.PluginSettings.RequirePluginSignature = true
 				})
 
-				key, err := os.Open(filepath.Join(path, "development-private-key.asc"))
-				require.NoError(t, err)
-				appErr := th.App.AddPublicKey("pub_key", key)
-				checkNoError(t, appErr)
-
 				signatureFileReader, err := os.Open(filepath.Join(path, "testplugin.tar.gz.sig"))
 				require.NoError(t, err)
 				defer signatureFileReader.Close()
-				_, appErr = th.App.WriteFile(signatureFileReader, getSignatureStorePath("testplugin"))
+				_, appErr := th.App.WriteFile(signatureFileReader, getSignatureStorePath("testplugin"))
 				checkNoError(t, appErr)
 
 				appErr = th.App.SyncPlugins()
@@ -832,13 +831,17 @@ func (a pluginStatusById) Less(i, j int) bool { return a[i].PluginId < a[j].Plug
 
 func TestProcessPrepackagedPlugins(t *testing.T) {
 	mainHelper.Parallel(t)
-	// Find the tests folder before we change directories to the temporary workspace.
-	testsPath, _ := fileutils.FindDir("tests")
+	testsPath, found := fileutils.FindDir("tests")
+	require.True(t, found, "failed to find tests directory")
 
 	setup := func(t *testing.T) *TestHelper {
 		t.Helper()
 
-		th := Setup(t)
+		th := SetupConfig(t, func(cfg *model.Config) {
+			cfg.PluginSettings.SignaturePublicKeyFiles = []string{
+				filepath.Join(testsPath, "development-private-key.asc"),
+			}
+		})
 		t.Cleanup(th.TearDown)
 
 		// Make a prepackaged_plugins directory for use with the tests.
@@ -892,7 +895,7 @@ func TestProcessPrepackagedPlugins(t *testing.T) {
 		t.Helper()
 
 		require.Equal(t, pluginID, actual.Manifest.Id)
-		require.NotEmpty(t, actual.Signature, "testplugin has no signature")
+		require.NotEmpty(t, actual.SignaturePath, "testplugin has no signature")
 		require.Equal(t, version, actual.Manifest.Version)
 	}
 

server/public/plugin/environment.go

@@ -38,10 +38,10 @@ type registeredPlugin struct {
 
 // PrepackagedPlugin is a plugin prepackaged with the server and found on startup.
 type PrepackagedPlugin struct {
-	Path      string
-	IconData  string
-	Manifest  *model.Manifest
-	Signature []byte
+	Path          string
+	IconData      string
+	Manifest      *model.Manifest
+	SignaturePath string
 }
 
 // Environment represents the execution environment of active plugins.