From 843ff932b70049619671cfc571f26e841f40c605 Mon Sep 17 00:00:00 2001 From: Will Browne Date: Fri, 22 Jul 2022 10:48:16 +0200 Subject: [PATCH] Plugins: Add signature wildcard globbing for dedicated private plugin type (#52163) * it's glob time * add more tests * fix warn log * support new sig type --- pkg/plugins/manager/signature/manifest.go | 63 ++-- .../manager/signature/manifest_test.go | 272 ++++++++++++++++++ pkg/plugins/models.go | 5 +- 3 files changed, 314 insertions(+), 26 deletions(-) diff --git a/pkg/plugins/manager/signature/manifest.go b/pkg/plugins/manager/signature/manifest.go index 12cd45ebe2a..9643b7e778a 100644 --- a/pkg/plugins/manager/signature/manifest.go +++ b/pkg/plugins/manager/signature/manifest.go @@ -15,6 +15,7 @@ import ( "path/filepath" "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" @@ -144,32 +145,14 @@ func Calculate(mlog log.Logger, plugin *plugins.Plugin) (plugins.Signature, erro }, nil } - // Validate that private is running within defined root URLs - if manifest.SignatureType == plugins.PrivateSignature || len(manifest.RootURLs) > 0 { - appURL, err := url.Parse(setting.AppUrl) - if err != nil { + // Validate that plugin is running within defined root URLs + if len(manifest.RootURLs) > 0 { + if match, err := urlMatch(manifest.RootURLs, setting.AppUrl, manifest.SignatureType); err != nil { + mlog.Warn("Could not verify if root URLs match", "plugin", plugin.ID, "rootUrls", manifest.RootURLs) return plugins.Signature{}, err - } - - foundMatch := false - for _, u := range manifest.RootURLs { - rootURL, err := url.Parse(u) - if err != nil { - mlog.Warn("Could not parse plugin root URL", "plugin", plugin.ID, "rootUrl", rootURL) - return plugins.Signature{}, err - } - - if rootURL.Scheme == appURL.Scheme && - rootURL.Host == appURL.Host && - path.Clean(rootURL.RequestURI()) == path.Clean(appURL.RequestURI()) { - foundMatch = true - break - } - } - - if !foundMatch { + } else if !match { mlog.Warn("Could not find root URL that matches running application URL", "plugin", plugin.ID, - "appUrl", appURL, "rootUrls", manifest.RootURLs) + "appUrl", setting.AppUrl, "rootUrls", manifest.RootURLs) return plugins.Signature{ Status: plugins.SignatureInvalid, }, nil @@ -299,3 +282,35 @@ func pluginFilesRequiringVerification(plugin *plugins.Plugin) ([]string, error) return files, err } + +func urlMatch(specs []string, target string, signatureType plugins.SignatureType) (bool, error) { + targetURL, err := url.Parse(target) + if err != nil { + return false, err + } + + for _, spec := range specs { + specURL, err := url.Parse(spec) + if err != nil { + return false, err + } + + if specURL.Scheme == targetURL.Scheme && specURL.Host == targetURL.Host && + path.Clean(specURL.RequestURI()) == path.Clean(targetURL.RequestURI()) { + return true, nil + } + + if signatureType != plugins.PrivateGlobSignature { + continue + } + + sp, err := glob.Compile(spec, '/', '.') + if err != nil { + return false, err + } + if match := sp.Match(target); match { + return true, nil + } + } + return false, nil +} diff --git a/pkg/plugins/manager/signature/manifest_test.go b/pkg/plugins/manager/signature/manifest_test.go index c4c2b13be5f..9852b087a22 100644 --- a/pkg/plugins/manager/signature/manifest_test.go +++ b/pkg/plugins/manager/signature/manifest_test.go @@ -175,3 +175,275 @@ func fileList(manifest *pluginManifest) []string { sort.Strings(keys) return keys } + +func Test_urlMatch_privateGlob(t *testing.T) { + type args struct { + specs []string + target string + } + tests := []struct { + name string + args args + shouldMatch bool + }{ + { + name: "Support single wildcard matching single subdomain", + args: args{ + specs: []string{"https://*.example.com"}, + target: "https://test.example.com", + }, + shouldMatch: true, + }, + { + name: "Do not support single wildcard matching multiple subdomains", + args: args{ + specs: []string{"https://*.example.com"}, + target: "https://more.test.example.com", + }, + shouldMatch: false, + }, + { + name: "Support multiple wildcards matching multiple subdomains", + args: args{ + specs: []string{"https://**.example.com"}, + target: "https://test.example.com", + }, + shouldMatch: true, + }, + { + name: "Support multiple wildcards matching multiple subdomains", + args: args{ + specs: []string{"https://**.example.com"}, + target: "https://more.test.example.com", + }, + shouldMatch: true, + }, + { + name: "Support single wildcard matching single paths", + args: args{ + specs: []string{"https://www.example.com/*"}, + target: "https://www.example.com/grafana1", + }, + shouldMatch: true, + }, + { + name: "Do not support single wildcard matching multiple paths", + args: args{ + specs: []string{"https://www.example.com/*"}, + target: "https://www.example.com/other/grafana", + }, + shouldMatch: false, + }, + { + name: "Support double wildcard matching multiple paths", + args: args{ + specs: []string{"https://www.example.com/**"}, + target: "https://www.example.com/other/grafana", + }, + shouldMatch: true, + }, + { + name: "Do not support subdomain mismatch", + args: args{ + specs: []string{"https://www.test.example.com/grafana/docs"}, + target: "https://www.dev.example.com/grafana/docs", + }, + shouldMatch: false, + }, + { + name: "Support single wildcard matching single path", + args: args{ + specs: []string{"https://www.example.com/grafana*"}, + target: "https://www.example.com/grafana1", + }, + shouldMatch: true, + }, + { + name: "Do not support single wildcard matching different path prefix", + args: args{ + specs: []string{"https://www.example.com/grafana*"}, + target: "https://www.example.com/somethingelse", + }, + shouldMatch: false, + }, + { + name: "Do not support path mismatch", + args: args{ + specs: []string{"https://example.com/grafana"}, + target: "https://example.com/grafana1", + }, + shouldMatch: false, + }, + { + name: "Support both domain and path wildcards", + args: args{ + specs: []string{"https://*.example.com/*"}, + target: "https://www.example.com/grafana1", + }, + shouldMatch: true, + }, + { + name: "Do not support wildcards without TLDs", + args: args{ + specs: []string{"https://example.*"}, + target: "https://www.example.com/grafana1", + }, + shouldMatch: false, + }, + { + name: "Support exact match", + args: args{ + specs: []string{"https://example.com/test"}, + target: "https://example.com/test", + }, + shouldMatch: true, + }, + { + name: "Does not support scheme mismatch", + args: args{ + specs: []string{"https://test.example.com/grafana"}, + target: "http://test.example.com/grafana", + }, + shouldMatch: false, + }, + { + name: "Support trailing slash in spec", + args: args{ + specs: []string{"https://example.com/"}, + target: "https://example.com", + }, + shouldMatch: true, + }, + { + name: "Support trailing slash in target", + args: args{ + specs: []string{"https://example.com"}, + target: "https://example.com/", + }, + shouldMatch: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := urlMatch(tt.args.specs, tt.args.target, plugins.PrivateGlobSignature) + require.NoError(t, err) + require.Equal(t, tt.shouldMatch, got) + }) + } +} + +func Test_urlMatch_private(t *testing.T) { + type args struct { + specs []string + target string + } + tests := []struct { + name string + args args + shouldMatch bool + }{ + { + name: "Support exact match", + args: args{ + specs: []string{"https://example.com/test"}, + target: "https://example.com/test", + }, + shouldMatch: true, + }, + { + name: "Support trailing slash in spec", + args: args{ + specs: []string{"https://example.com/test/"}, + target: "https://example.com/test", + }, + shouldMatch: true, + }, + { + name: "Support trailing slash in target", + args: args{ + specs: []string{"https://example.com/test"}, + target: "https://example.com/test/", + }, + shouldMatch: true, + }, + { + name: "Do not support single wildcard matching single subdomain", + args: args{ + specs: []string{"https://*.example.com"}, + target: "https://test.example.com", + }, + shouldMatch: false, + }, + { + name: "Do not support multiple wildcards matching multiple subdomains", + args: args{ + specs: []string{"https://**.example.com"}, + target: "https://more.test.example.com", + }, + shouldMatch: false, + }, + { + name: "Do not support single wildcard matching single paths", + args: args{ + specs: []string{"https://www.example.com/*"}, + target: "https://www.example.com/grafana1", + }, + shouldMatch: false, + }, + { + name: "Do not support double wildcard matching multiple paths", + args: args{ + specs: []string{"https://www.example.com/**"}, + target: "https://www.example.com/other/grafana", + }, + shouldMatch: false, + }, + { + name: "Do not support subdomain mismatch", + args: args{ + specs: []string{"https://www.test.example.com/grafana/docs"}, + target: "https://www.dev.example.com/grafana/docs", + }, + shouldMatch: false, + }, + { + name: "Do not support path mismatch", + args: args{ + specs: []string{"https://example.com/grafana"}, + target: "https://example.com/grafana1", + }, + shouldMatch: false, + }, + { + name: "Do not support both domain and path wildcards", + args: args{ + specs: []string{"https://*.example.com/*"}, + target: "https://www.example.com/grafana1", + }, + shouldMatch: false, + }, + { + name: "Do not support wildcards without TLDs", + args: args{ + specs: []string{"https://example.*"}, + target: "https://www.example.com/grafana1", + }, + shouldMatch: false, + }, + { + name: "Do not support scheme mismatch", + args: args{ + specs: []string{"https://test.example.com/grafana"}, + target: "http://test.example.com/grafana", + }, + shouldMatch: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := urlMatch(tt.args.specs, tt.args.target, plugins.PrivateSignature) + require.NoError(t, err) + require.Equal(t, tt.shouldMatch, got) + }) + } +} diff --git a/pkg/plugins/models.go b/pkg/plugins/models.go index 3da073d8e21..4f4f7d5536d 100644 --- a/pkg/plugins/models.go +++ b/pkg/plugins/models.go @@ -176,8 +176,9 @@ const ( type SignatureType string const ( - GrafanaSignature SignatureType = "grafana" - PrivateSignature SignatureType = "private" + GrafanaSignature SignatureType = "grafana" + PrivateSignature SignatureType = "private" + PrivateGlobSignature SignatureType = "private-glob" ) type PluginFiles map[string]struct{}