Skip to content

Commit ea7e5e2

Browse files
authored
Chore: Improve error message in case of a revoked key (#67413)
1 parent 816ba47 commit ea7e5e2

File tree

2 files changed

+110
-1
lines changed

2 files changed

+110
-1
lines changed

pkg/plugins/manager/signature/manifest.go

+6-1
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818

1919
"github.com/ProtonMail/go-crypto/openpgp"
2020
"github.com/ProtonMail/go-crypto/openpgp/clearsign"
21+
openpgpErrors "github.com/ProtonMail/go-crypto/openpgp/errors"
2122
"github.com/ProtonMail/go-crypto/openpgp/packet"
2223
"github.com/gobwas/glob"
2324

@@ -141,7 +142,7 @@ func (s *Signature) Calculate(ctx context.Context, src plugins.PluginSource, plu
141142

142143
manifest, err := s.readPluginManifest(ctx, byteValue)
143144
if err != nil {
144-
s.log.Debug("Plugin signature invalid", "id", plugin.JSONData.ID, "err", err)
145+
s.log.Warn("Plugin signature invalid", "id", plugin.JSONData.ID, "err", err)
145146
return plugins.Signature{
146147
Status: plugins.SignatureInvalid,
147148
}, nil
@@ -341,6 +342,10 @@ func (s *Signature) Verify(ctx context.Context, keyID string, block *clearsign.B
341342
if _, err = openpgp.CheckDetachedSignature(keyring,
342343
bytes.NewBuffer(block.Bytes),
343344
block.ArmoredSignature.Body, &packet.Config{}); err != nil {
345+
// If the key includes revocations, we can assume that the key was revoked
346+
if len(keyring) > 0 && len(keyring[0].Revocations) > 0 {
347+
return fmt.Errorf("%s (KeyID: %s): %w", openpgpErrors.ErrKeyRevoked.Error(), keyID, err)
348+
}
344349
return fmt.Errorf("%v: %w", "failed to check signature", err)
345350
}
346351

pkg/plugins/manager/signature/manifest_test.go

+104
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ import (
88
"strings"
99
"testing"
1010

11+
"github.com/ProtonMail/go-crypto/openpgp/clearsign"
12+
openpgpErrors "github.com/ProtonMail/go-crypto/openpgp/errors"
1113
"github.com/stretchr/testify/assert"
1214
"github.com/stretchr/testify/require"
1315

@@ -756,3 +758,105 @@ func mustNewStaticFSForTests(t *testing.T, dir string) plugins.FS {
756758
require.NoError(t, err)
757759
return sfs
758760
}
761+
762+
type revokedKeyProvider struct{}
763+
764+
func (p *revokedKeyProvider) GetPublicKey(ctx context.Context, keyID string) (string, error) {
765+
// dummy revoked key created locally
766+
const publicKeyText = `-----BEGIN PGP PUBLIC KEY BLOCK-----
767+
768+
mQGNBGRKicsBDADMP7DxjVIj/1gWaaaC+21p7AIXvF6I94FL687fBQLPjFDh9Lrt
769+
iGk58n/OG4hw+5qhEWdVWR9RvhtNP8XB/wXzFJBTEadZZfShkqEwEP+tSSiczxgl
770+
C25LvMmfzUjYXwJdByYRZlFTlP3vBqBZy56QWnz0Q7O/CvjNleGWJ4DfqiMFgDoC
771+
zuCkXLhnpJHMf4HhYqM0qPn4q7SkA+7nJ7LjwU016rIsY+f6iDoe8fLVdqzkg8Ag
772+
Oo7OsqEU0bex6gxP0XJzAUJffj+fqUty5E8+SBJMCxGcwagqEtivhGTR5sERfcbs
773+
hk8cPhHDE0qNZvrVQrOsXQc+CXdPtIZl2BQOTiXcaeOItZ5FIfk5kM+HpB3xFVgX
774+
hu8Ct8r1kKTlRbu5a7BwI8emQJaPrPExr89wALVSFc3SUP6FMsCdCSJZpACMNuro
775+
HTREH+pKktnhdAptye/LJ4G5PXX89utDOe06iTembTuwi/YouSfeFv5/oVWFf3U8
776+
MzbLt6hVC8kuZs0AEQEAAYkBtgQgAQoAIBYhBOyXlK8OTF+dy2fAqF3wd+PYth+G
777+
BQJkSorEAh0AAAoJEF3wd+PYth+GqnwL/0Z8TM+shR8EgoKqXvuytGbyURTL+cz3
778+
34t0jjayXB0rUp4+Q6umlHZ3JIkIJhzgd3rShtIuo/sxFX7GYXqfQJj28Ry+Gfec
779+
8hlW+YvzVOs6UzlpFlHktJAHy8+uEw5Z9364apE1yK6MOzy+LWACu7YWYiH/WCQE
780+
eH4P0R6IiaC/pIUbM4obHtbncL67PnLXn2/350sHdXceInUitLgp9DNZZQvoBA1Y
781+
Y5cGYMuCF5Ji3/p5z8NYuP5l9KVdb2tBfQDYi3e5TrntpRG/0iI4hPmXJbFlwQip
782+
nCb31mZy8AlLupCsC3+F97/Ea/sPJblRRrm+cLxXSvqlVJivH7iHsWz5iraMSG4e
783+
HVyvDc2Cv2uvM6kGDCOTOx/H5w+5FNeFz/AtCE5WQVb8nR66oGWMeV2Dr1RsHKQY
784+
oJL9C+Gv4gUxz/2E+JFnrJwC4dbZYOQBWNagecTYeMbZMO0uv5WQyqra/99b6Tby
785+
XlNekEpRXBExbBY2cucrDNXFiFspbX/2jLQHYW5kcmVzMokB1AQTAQoAPhYhBOyX
786+
lK8OTF+dy2fAqF3wd+PYth+GBQJkSonLAhsDBQkDwmcABQsJCAcCBhUKCQgLAgQW
787+
AgMBAh4BAheAAAoJEF3wd+PYth+G5VEL/14o7ARD5e3YEKqfbaShXUZItT7rPw13
788+
M7lDXdr+XB+hrkRPP2ZZVK54x1S4CsDLSym08WFRGiC2mPx2wWESepisWVvixaDj
789+
EXZm3z76O4pY8NzAymKHKNALev2jgEDIQ22XGFgSxW2MHLLV0OBFAIZBgGLUsR7f
790+
L9QfG7rICIx5W3W9Rd18SI6s64cSknDjzbyiZeETXQHxVODPmd5u8y/SVwPKQx5J
791+
qr5qEb7oHKEALRhO7STCyC+kCkU1gmGrzATjng4SzNegwuHDFbSuwy4YEcFvRSkm
792+
gS4UKEEQBNoZj95I7B8S3hAHYnXWLRAcwg+e3G8JWdBLdYmnuOWa4qsix+GNUXd4
793+
QUpXFmSihCJO1lF7GBcfE8sUXTq+IwzGP690p/ZBpEqO2wn9UbeSZxGbgZ0HCc2H
794+
8CNWflWJsfPGnLz2sPt6JmrNW1124gz1PlgBixV2DUzEVBj/Nnv6aqRxZbEQ7/+V
795+
FYPnNsKV0LVxzDxc0Ob0qFzZ562P/mj9OrkBjQRkSonLAQwAq2L6KSVzLJrtAP12
796+
TJWERNCrjwWB1SjeKctWwgT+0EwEKmTx0Escnf2aELPgcAQ0pBYD2CEutDn12nhr
797+
nZppLmyqv7dtOR5JgOs65BHu//K5LOvY5V5deDo7QHfYWCGhgvEHKk0JY2N2ueRM
798+
iqQwHQPYyLH8rWVueOCfXONSB9I8VqE3HEdug4Wk6jgMNt+9dbGUFl7PvoVDtpcE
799+
ghNSbXJptQnfFL6lpgPLMyuS+d7W37jhqkFSoe5CvCLSFc8UZRPdIVei5jxfh8rx
800+
g4QJ9gdVxIHyY6+dBXQ+ZFxIe3EufmYSiST9LM9uZ75oY6VnTEXpu2e2A/mgT8ke
801+
2Nd/1O7wWV6UndAFruJ732cntT6BLwwHTYHiH2b4km5qjtMrsgY9BWju4WcrDJVD
802+
RtQ0i5jfmuZOYgxFgwr8Y9nA5k5zUVuudShh/DGEpjpTOQ7jbw2XzvlmTIcwpP/a
803+
IrKbXZhMW9X3VhXfCOg9IHiKsnvvBVsZbDD4942dU7+NGSPBABEBAAGJAbYEGAEK
804+
ACAWIQTsl5SvDkxfnctnwKhd8Hfj2LYfhgUCZEqJywIbDAAKCRBd8Hfj2LYfhsqc
805+
C/9/od/rbuiaJ8h9LfVOjcljDnCyf+2W8HXYcdl4MKNG6IOviLZqwfLLxDzsVgYC
806+
3A/HsX10kaJNZWbpDttMLJrUyQ4ZBT8UvQv149iCrRdTcNAv+bllpta73phz3D0u
807+
izMQ7wawOA3pR5VBVGRsYuljwOBR5WuqJ9EDknbE3YCCHFtq1ehHy+VA4BUx9czv
808+
mPHbYPsJVAWDcBrEKZ7WdIF9U3souFa6PplEQfDgjsoBEw8dC+EQhgb7Z4pP9VlG
809+
rVI1vraW0T+hS9csr+0LYR+TQiD24gA4Ec5bLJcPinwHoBvPCE3aqqiX67qcxuhq
810+
jmiiz3S2RrGYAi8vod87xc6k9X8rmv3zir3UeekVq2mPCensQ6+zIK+zyASY/i1d
811+
kYfyUNMj4t2j9+96F8u2Mh3KpaVTfj4Olg5JWcqG9UJXwXGJflk7NuaBiPBbK/W6
812+
LusDoGuEb/CYRKY/bRblEm2YcRGJHqzod+S+mBZmEjEB6OSWz01CABs/hWY9rdtY
813+
YNE=
814+
=U6y9
815+
-----END PGP PUBLIC KEY BLOCK-----
816+
`
817+
return publicKeyText, nil
818+
}
819+
820+
func Test_VerifyRevokedKey(t *testing.T) {
821+
s := ProvideService(&config.Cfg{}, &revokedKeyProvider{})
822+
m := createV2Manifest(t)
823+
txt := `-----BEGIN PGP SIGNED MESSAGE-----
824+
Hash: SHA512
825+
826+
{
827+
"manifestVersion": "2.0.0",
828+
"signatureType": "grafana",
829+
"signedByOrg": "grafana",
830+
"signedByOrgName": "Grafana Labs",
831+
"plugin": "test-app",
832+
"version": "1.0.0",
833+
"time": 1621356785895,
834+
"keyId": "7e4d0c6a708866e7",
835+
"files": {
836+
"plugin.json": "c59a51bf6d7ecd7a99608ccb99353390c8b973672a938a0247164324005c0caf",
837+
"dashboards/connections.json": "bea86da4be970b98dc4681802ab55cdef3441dc3eb3c654cb207948d17b25303",
838+
"dashboards/memory.json": "7c042464941084caa91d0a9a2f188b05315a9796308a652ccdee31ca4fbcbfee",
839+
"dashboards/connections_result.json": "124d85c9c2e40214b83273f764574937a79909cfac3f925276fbb72543c224dc"
840+
}
841+
}
842+
-----BEGIN PGP SIGNATURE-----
843+
844+
iQGzBAEBCgAdFiEE7JeUrw5MX53LZ8CoXfB349i2H4YFAmRKigIACgkQXfB349i2
845+
H4ZdKgwAuVuTjGT7Rn1MfxYRUXRymdnyqsDRYaK8gw5i9OZweBuJBVLtL1eFII0h
846+
tTr+2jM4kGlsCakpJm3sjRG//8sBYoO5GsnOM6g1gv7mgUwo/Pv3A5eFFeOIkF1W
847+
E33nNyF17BlY+YPVJPMQ8Q4uBSz2pDlcdQY8gOleWERWMWvmsHZgobt7wyGgts7Y
848+
hCzKdm+e5/HpWBskW7dRMh1yB+8Ql+IK/Ksy8EDdX+Yv1fGV6ZNNIQxSEBXSily6
849+
uvZlU9zExa0db9rkg53jFpSfSFpQIJJ0Y0yOmHKDA4WLnphroCIBwo2lxIBIwuNH
850+
sXjmTjacvrqk13Af7Gat7XSNLapBfy5rTZwJFOwGWyDP1V0FTrlmt5vmoD0MRskq
851+
gry5NAKktwc2llGaS5uGc5wJ1wTvl5wYQkU8lBevdejntpQSOYNEuICe+OyKQP+h
852+
OOKpUCovEat+3W9JU1PM+z3cb1H/WWQ3hpKEykyzzi/jZMuRnRobW8Jm/4WxFgaY
853+
70RA9/V8
854+
=NUH5
855+
-----END PGP SIGNATURE-----
856+
`
857+
block, _ := clearsign.Decode([]byte(txt))
858+
require.NotNil(t, block, "failed to decode block")
859+
err := s.validateManifest(context.Background(), *m, block)
860+
require.Error(t, err)
861+
require.Contains(t, err.Error(), openpgpErrors.ErrKeyRevoked.Error())
862+
}

0 commit comments

Comments
 (0)