From 48b33ab5215d2457a8c2ee45a5d05c8a51cc61c5 Mon Sep 17 00:00:00 2001 From: Will Browne Date: Mon, 19 Dec 2022 11:46:27 +0000 Subject: [PATCH] Plugins: Unsigned chromium file should not invalidate signature for Renderer plugin (#59104) * Plugins: Unsigned chromium file should not invalidate signature for Renderer plugin * fix test * re-work solution --- pkg/plugins/manager/signature/manifest.go | 8 ++++++ .../manager/signature/manifest_test.go | 25 +++++++++++++++++ .../renderer-added-file/plugin/MANIFEST.txt | 28 +++++++++++++++++++ .../plugin/chrome-win/debug.log | 1 + .../renderer-added-file/plugin/plugin.json | 11 ++++++++ pkg/plugins/pfs/pfs_test.go | 4 +++ pkg/services/rendering/rendering.go | 11 -------- 7 files changed, 77 insertions(+), 11 deletions(-) create mode 100644 pkg/plugins/manager/testdata/renderer-added-file/plugin/MANIFEST.txt create mode 100644 pkg/plugins/manager/testdata/renderer-added-file/plugin/chrome-win/debug.log create mode 100644 pkg/plugins/manager/testdata/renderer-added-file/plugin/plugin.json diff --git a/pkg/plugins/manager/signature/manifest.go b/pkg/plugins/manager/signature/manifest.go index 8edf7f2d556..8f9053be7ce 100644 --- a/pkg/plugins/manager/signature/manifest.go +++ b/pkg/plugins/manager/signature/manifest.go @@ -12,6 +12,7 @@ import ( "os" "path" "path/filepath" + "runtime" "strings" "github.com/gobwas/glob" @@ -54,6 +55,8 @@ N1c5v9v/4h6qeA== -----END PGP PUBLIC KEY BLOCK----- ` +var runningWindows = runtime.GOOS == "windows" + // pluginManifest holds details for the file manifest type pluginManifest struct { Plugin string `json:"plugin"` @@ -259,6 +262,11 @@ func pluginFilesRequiringVerification(plugin *plugins.Plugin) ([]string, error) return nil } + // Ignoring unsigned Chromium debug.log so it doesn't invalidate the signature for Renderer plugin running on Windows + if runningWindows && plugin.IsRenderer() && strings.HasSuffix(path, filepath.Join("chrome-win", "debug.log")) { + return nil + } + // verify that file is within plugin directory file, err := filepath.Rel(plugin.PluginDir, path) if err != nil { diff --git a/pkg/plugins/manager/signature/manifest_test.go b/pkg/plugins/manager/signature/manifest_test.go index 68aff9538bd..5fa671b362b 100644 --- a/pkg/plugins/manager/signature/manifest_test.go +++ b/pkg/plugins/manager/signature/manifest_test.go @@ -165,6 +165,31 @@ func TestCalculate(t *testing.T) { require.Equal(t, tc.expectedSignature, sig) } }) + + t.Run("Unsigned Chromium file should not invalidate signature for Renderer plugin running on Windows", func(t *testing.T) { + backup := runningWindows + t.Cleanup(func() { + runningWindows = backup + }) + + runningWindows = true + sig, err := Calculate(log.NewNopLogger(), &plugins.Plugin{ + JSONData: plugins.JSONData{ + ID: "test-renderer", + Type: plugins.Renderer, + Info: plugins.Info{ + Version: "1.0.0", + }, + }, + PluginDir: "../testdata/renderer-added-file/plugin", + }) + require.NoError(t, err) + require.Equal(t, plugins.Signature{ + Status: plugins.SignatureValid, + Type: plugins.GrafanaSignature, + SigningOrg: "Grafana Labs", + }, sig) + }) } func fileList(manifest *pluginManifest) []string { diff --git a/pkg/plugins/manager/testdata/renderer-added-file/plugin/MANIFEST.txt b/pkg/plugins/manager/testdata/renderer-added-file/plugin/MANIFEST.txt new file mode 100644 index 00000000000..78e3161df43 --- /dev/null +++ b/pkg/plugins/manager/testdata/renderer-added-file/plugin/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-renderer", + "version": "1.0.0", + "time": 1669116992691, + "keyId": "7e4d0c6a708866e7", + "files": { + "plugin.json": "2f8fc554d0a080b9719ba43c0a8df366a88500eb129dfd4aedd9e3d147178273" + } +} +-----BEGIN PGP SIGNATURE----- +Version: OpenPGP.js v4.10.10 +Comment: https://openpgpjs.org + +wrgEARMKAAYFAmN8tEAAIQkQfk0ManCIZucWIQTzOyW2kQdOhGNlcPN+TQxq +cIhm576UAgiedgIhpOgTi/ypYgg8AtGQqKAwnDuDRDrYOc6LUDuskbSsR+J7 +f2QjcPGpPk3alaqiTYMxixkTgmD01hltaTJ0AwIJAS9XkKHKxC9/ZhSHYemw +7wUeKs6AEvKR6amYZ+TF2pVyiJ9cEnl9J55MmDikqKFrIqC9J0V9r9wFkPAF +kOKVOY+y +=gqF+ +-----END PGP SIGNATURE----- diff --git a/pkg/plugins/manager/testdata/renderer-added-file/plugin/chrome-win/debug.log b/pkg/plugins/manager/testdata/renderer-added-file/plugin/chrome-win/debug.log new file mode 100644 index 00000000000..9ff48768c17 --- /dev/null +++ b/pkg/plugins/manager/testdata/renderer-added-file/plugin/chrome-win/debug.log @@ -0,0 +1 @@ +TEST LOG LINE diff --git a/pkg/plugins/manager/testdata/renderer-added-file/plugin/plugin.json b/pkg/plugins/manager/testdata/renderer-added-file/plugin/plugin.json new file mode 100644 index 00000000000..3160dbb3d7a --- /dev/null +++ b/pkg/plugins/manager/testdata/renderer-added-file/plugin/plugin.json @@ -0,0 +1,11 @@ +{ + "type": "renderer", + "name": "Test", + "id": "test-renderer", + "backend": true, + "executable": "test", + "info": { + "version": "1.0.0", + "description": "Test" + } +} diff --git a/pkg/plugins/pfs/pfs_test.go b/pkg/plugins/pfs/pfs_test.go index eed41673925..ede09039767 100644 --- a/pkg/plugins/pfs/pfs_test.go +++ b/pkg/plugins/pfs/pfs_test.go @@ -69,6 +69,10 @@ func TestParseTreeTestdata(t *testing.T) { rootid: "test-datasource", subpath: "plugin", }, + "renderer-added-file": { + rootid: "test-renderer", + subpath: "plugin", + }, "symbolic-plugin-dirs": { skip: "io/fs-based scanner will not traverse symlinks; caller of ParsePluginFS() must do it", }, diff --git a/pkg/services/rendering/rendering.go b/pkg/services/rendering/rendering.go index eccf441d887..3b68e8be9f2 100644 --- a/pkg/services/rendering/rendering.go +++ b/pkg/services/rendering/rendering.go @@ -7,7 +7,6 @@ import ( "math" "net/url" "os" - "path" "path/filepath" "strings" "sync" @@ -170,16 +169,6 @@ func (rs *RenderingService) Run(ctx context.Context) error { rs.sanitizeSVGAction = rs.sanitizeSVGViaPlugin <-ctx.Done() - // On Windows, Chromium is generating a debug.log file that breaks signature check on next restart - debugFilePath := path.Join(rs.pluginInfo.PluginDir, "chrome-win/debug.log") - if _, err := os.Stat(debugFilePath); err == nil { - err = os.Remove(debugFilePath) - if err != nil { - rs.log.Warn("Couldn't remove debug.log file, the renderer plugin will not be able to pass the signature check until this file is deleted", - "err", err) - } - } - return nil }