Skip to content

Commit 80dd18f

Browse files
authored
Merge pull request keybase#59 from keybase/zapu/nested-sigs
Support multiple (nested) signatures
2 parents f637167 + 0947fec commit 80dd18f

File tree

3 files changed

+272
-24
lines changed

3 files changed

+272
-24
lines changed

openpgp/nested_sig_test.go

Lines changed: 181 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,181 @@
1+
package openpgp
2+
3+
import (
4+
"bytes"
5+
"fmt"
6+
"io/ioutil"
7+
"strings"
8+
"testing"
9+
10+
"github.com/keybase/go-crypto/openpgp/armor"
11+
)
12+
13+
func TestMultisig(t *testing.T) {
14+
kring1, err := ReadArmoredKeyRing(bytes.NewBufferString(testKey1))
15+
if err != nil {
16+
t.Error(err)
17+
return
18+
}
19+
20+
kring2, err := ReadArmoredKeyRing(bytes.NewBufferString(testKey2))
21+
if err != nil {
22+
t.Error(err)
23+
return
24+
}
25+
26+
if len(kring1) != 1 && len(kring2) != 1 {
27+
t.Fatalf("Expected both keyrings to only have one key each: len 1: %d len 2: %d", len(kring1), len(kring2))
28+
}
29+
30+
tryWithKey := func(keys EntityList) error {
31+
sig, err := armor.Decode(strings.NewReader(testSignature))
32+
if err != nil {
33+
return err
34+
}
35+
36+
md, err := ReadMessage(sig.Body, keys, nil, nil)
37+
if err != nil {
38+
return err
39+
}
40+
41+
if !md.MultiSig {
42+
return fmt.Errorf("Expected MultiSig to be true")
43+
}
44+
45+
if md.SignedByKeyId == 0 {
46+
return fmt.Errorf("SignedByKeyId should never be zero, even if keyring doesn't contain good key")
47+
}
48+
49+
if md.SignedBy == nil {
50+
return fmt.Errorf("Couldn't find key message was signed with or message wasn't signed (md is %+v)", md)
51+
}
52+
53+
if md.SignedBy.PublicKey != keys[0].PrimaryKey {
54+
return fmt.Errorf("Message wasn't by expected key (SignedBy is %p)", md.SignedBy)
55+
}
56+
57+
_, err = ioutil.ReadAll(md.UnverifiedBody)
58+
if err != nil {
59+
return err
60+
}
61+
62+
if md.SignatureError != nil {
63+
return fmt.Errorf("md.SignatureError: %s", md.SignatureError)
64+
}
65+
66+
if md.Signature == nil && md.SignatureV3 == nil {
67+
return fmt.Errorf("Signature is nil after reading (md is %+v)", md)
68+
}
69+
70+
return nil
71+
}
72+
73+
badkey, err := ReadArmoredKeyRing(bytes.NewBufferString(matthiasuKey))
74+
if err != nil {
75+
t.Error(err)
76+
return
77+
}
78+
79+
t.Logf("Trying keyring 1")
80+
if err := tryWithKey(kring1); err != nil {
81+
t.Error(err)
82+
}
83+
84+
t.Logf("Trying keyring 2")
85+
if err := tryWithKey(kring2); err != nil {
86+
t.Error(err)
87+
}
88+
89+
t.Logf("Trying entirely unrelated key from different test file")
90+
err = tryWithKey(badkey)
91+
if err == nil {
92+
t.Error("Expected an error but got nil when trying unrelated key.")
93+
}
94+
t.Logf("When trying with bad key, error was: %s", err)
95+
}
96+
97+
func TestMultisigMalformed(t *testing.T) {
98+
keys, err := ReadArmoredKeyRing(bytes.NewBufferString(testKey2))
99+
if err != nil {
100+
t.Fatal(err)
101+
return
102+
}
103+
104+
sig, err := armor.Decode(strings.NewReader(testSignatureMalformed))
105+
if err != nil {
106+
t.Fatalf("Got error during decode: %v", err)
107+
}
108+
109+
md, err := ReadMessage(sig.Body, keys, nil, nil)
110+
if err != nil {
111+
t.Fatalf("Got error during ReadMessage: %v", err)
112+
}
113+
114+
if !md.MultiSig {
115+
t.Fatal("Expected MultiSig to be true")
116+
}
117+
118+
_, err = ioutil.ReadAll(md.UnverifiedBody)
119+
if err != nil {
120+
t.Fatalf("Got error during ReadAll: %v", err)
121+
}
122+
123+
if md.SignatureError == nil || md.Signature != nil {
124+
t.Fatalf("Expected SignatureError, got nil")
125+
}
126+
127+
t.Logf("SignatureError is: %v", md.SignatureError)
128+
}
129+
130+
const testKey1 = `-----BEGIN PGP PUBLIC KEY BLOCK-----
131+
132+
mDMEWfcV/BYJKwYBBAHaRw8BAQdAYhAuI4LPgxnu8MDU/XJpSlfCFPelz58v5QpU
133+
9R9MtFa0ClRlc3QgS2V5IDGIeQQTFggAIQUCWfcV/AIbAwULCQgHAgYVCAkKCwIE
134+
FgIDAQIeAQIXgAAKCRCc9gMcYqzhOWz5AP91QOM2xFiy5FZ+suqpP5zbygMNe/PJ
135+
wunDkjryQRaWqgEAjalogSO20NeTEbDBWiglggMvJrTFXMqdsZ5bdUkoSQi4OARZ
136+
9xX8EgorBgEEAZdVAQUBAQdAIXy/6CNrP/Tq6uDnTu9Vra8Qc05uGY18gUqou9/0
137+
m1wDAQgHiGEEGBYIAAkFAln3FfwCGwwACgkQnPYDHGKs4TkTYwEA9JWX8sASAY6u
138+
NSuMuq3f3fKwYVR3kB0hYRd7ffic+aABALBDGedfGTfjKLWAqd+NFO4fKlQJjg0Y
139+
+EnrmcTzas4G
140+
=JK7o
141+
-----END PGP PUBLIC KEY BLOCK-----
142+
`
143+
144+
const testKey2 = `-----BEGIN PGP PUBLIC KEY BLOCK-----
145+
146+
mDMEWfcWjxYJKwYBBAHaRw8BAQdAxsmWcp2FwAiRHylbOrDnoKKUBAa1wgQlE1mJ
147+
fNj4EFS0ClRlc3QgS2V5IDKIeQQTFggAIQUCWfcWjwIbAwULCQgHAgYVCAkKCwIE
148+
FgIDAQIeAQIXgAAKCRCIHHukx/1YbM3dAQDNiAF2ZqxrDvxv5chMeazuvsu9o5J8
149+
mtpPludqpWKsvAD6AsH0fhDeIwKVBk1uigw3ut7VKyyNSSNezy3RczengQy4OARZ
150+
9xaPEgorBgEEAZdVAQUBAQdAF1hhJLcRj77GF+lc9gEVziFZ1yJW/8LYSMZ0AAo9
151+
kkgDAQgHiGEEGBYIAAkFAln3Fo8CGwwACgkQiBx7pMf9WGxlJgD/RriX0jfA3Hjl
152+
pSCtbGRJGm6LZgYEn9XHzfmZ+ZTG9bsA/AxYjMrv4I3Ft4x6ogrqzvxmcga3zgGc
153+
QjcG/YKbNUQJ
154+
=/+FB
155+
-----END PGP PUBLIC KEY BLOCK-----
156+
`
157+
158+
const testSignature = `-----BEGIN PGP MESSAGE-----
159+
160+
owGbwMvMwCHWIVO95PjfiByGCWDunG/MMklrHloynhZLYoj8bmpdgQq4OuJYGMQ4
161+
GNhYmUCyDFycAjAtVeyMDAsPbJoqV7fr73PmkwouPLvPX02Oeye648Tq72GOz+68
162+
lTdiZLigI+QhMS++YiL/9uLLKyZmlnX3iZ89rLYhTKZe2b5JHIv5MBeCzP9zUvnk
163+
xI4vmaJe3MJav1zjZZc67ZSJYVy9+Q33resy8X0M/4Mmhrs7zvyssnTjiTOTPzus
164+
uXp/8adNSepq6kncd9iiU5kB
165+
=QVa3
166+
-----END PGP MESSAGE-----
167+
`
168+
169+
// This signature payload ends prematurely, it claims to be signed by
170+
// both testKey1 (9CF6031C62ACE139) and testKey2 (881C7BA4C7FD586C)
171+
// but it only contains `signature packet` for 9CF6031C62ACE139. If we
172+
// test it with 881C7BA4C7FD586C, it should fail to read.
173+
const testSignatureMalformed = `-----BEGIN PGP MESSAGE-----
174+
175+
kA0DAAgWiBx7pMf9WGwAkA0DAAgWnPYDHGKs4TkByxZiAFn3NTt4eHh4eHh4eHh4
176+
eHh4eHgKiF4EABYIAAYFAln3NTsACgkQnPYDHGKs4Tl6BwEAocCylR5+uv3nA8kg
177+
RAy7z9VjXu4VuMir91ZB5tztHzIBANAsEkgYnl94kQ+3c9OokWl2i44XzcMmsFYc
178+
fyM/ghcK
179+
=R0Rj
180+
-----END PGP ARMORED FILE-----
181+
`

openpgp/read.go

Lines changed: 59 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,9 @@ type MessageDetails struct {
6161
Signature *packet.Signature // the signature packet itself, if v4 (default)
6262
SignatureV3 *packet.SignatureV3 // the signature packet if it is a v2 or v3 signature
6363

64+
// Does the Message include multiple signatures? Also called "nested signatures".
65+
MultiSig bool
66+
6467
decrypted io.ReadCloser
6568
}
6669

@@ -244,8 +247,17 @@ FindLiteralData:
244247
return nil, err
245248
}
246249
case *packet.OnePassSignature:
247-
if !p.IsLast {
248-
return nil, errors.UnsupportedError("nested signatures")
250+
if md.IsSigned {
251+
// If IsSigned is set, it means we have multiple
252+
// OnePassSignature packets.
253+
md.MultiSig = true
254+
if md.SignedBy != nil {
255+
// We've already found the signature we were looking
256+
// for, made by key that we had in keyring and can
257+
// check signature against. Continue with that instead
258+
// of trying to find another.
259+
continue FindLiteralData
260+
}
249261
}
250262

251263
h, wrappedHash, err = hashForSignature(p.Hash, p.SigType)
@@ -329,29 +341,54 @@ func (scr *signatureCheckReader) Read(buf []byte) (n int, err error) {
329341
n, err = scr.md.LiteralData.Body.Read(buf)
330342
scr.wrappedHash.Write(buf[:n])
331343
if err == io.EOF {
332-
var p packet.Packet
333-
p, scr.md.SignatureError = scr.packets.Next()
334-
if scr.md.SignatureError != nil {
335-
return
336-
}
337-
338-
var ok bool
339-
if scr.md.Signature, ok = p.(*packet.Signature); ok {
340-
var err error
341-
if fingerprint := scr.md.Signature.IssuerFingerprint; fingerprint != nil {
342-
if !hmac.Equal(fingerprint, scr.md.SignedBy.PublicKey.Fingerprint[:]) {
343-
err = errors.StructuralError("bad key fingerprint")
344+
for {
345+
var p packet.Packet
346+
p, scr.md.SignatureError = scr.packets.Next()
347+
if scr.md.SignatureError != nil {
348+
if scr.md.MultiSig {
349+
// If we are in MultiSig, we might have found other
350+
// signature that cannot be verified using our key.
351+
// Clear Signature field so it's clear for consumers
352+
// that this message failed to verify.
353+
scr.md.Signature = nil
344354
}
355+
return
345356
}
346-
if err == nil {
347-
err = scr.md.SignedBy.PublicKey.VerifySignature(scr.h, scr.md.Signature)
357+
358+
var ok bool
359+
if scr.md.Signature, ok = p.(*packet.Signature); ok {
360+
var err error
361+
if keyID := scr.md.Signature.IssuerKeyId; keyID != nil {
362+
if *keyID != scr.md.SignedBy.PublicKey.KeyId {
363+
if scr.md.MultiSig {
364+
continue // try again to find a sig we can verify
365+
}
366+
err = errors.StructuralError("bad key id")
367+
}
368+
}
369+
if fingerprint := scr.md.Signature.IssuerFingerprint; fingerprint != nil {
370+
if !hmac.Equal(fingerprint, scr.md.SignedBy.PublicKey.Fingerprint[:]) {
371+
if scr.md.MultiSig {
372+
continue // try again to find a sig we can verify
373+
}
374+
err = errors.StructuralError("bad key fingerprint")
375+
}
376+
}
377+
if err == nil {
378+
err = scr.md.SignedBy.PublicKey.VerifySignature(scr.h, scr.md.Signature)
379+
}
380+
scr.md.SignatureError = err
381+
} else if scr.md.SignatureV3, ok = p.(*packet.SignatureV3); ok {
382+
scr.md.SignatureError = scr.md.SignedBy.PublicKey.VerifySignatureV3(scr.h, scr.md.SignatureV3)
383+
} else {
384+
scr.md.SignatureError = errors.StructuralError("LiteralData not followed by Signature")
385+
return
348386
}
349-
scr.md.SignatureError = err
350-
} else if scr.md.SignatureV3, ok = p.(*packet.SignatureV3); ok {
351-
scr.md.SignatureError = scr.md.SignedBy.PublicKey.VerifySignatureV3(scr.h, scr.md.SignatureV3)
352-
} else {
353-
scr.md.SignatureError = errors.StructuralError("LiteralData not followed by Signature")
354-
return
387+
388+
// Parse only one packet by default, unless message is MultiSig. Then
389+
// we ask for more packets after discovering non-matching signature,
390+
// until we find one that we can verify.
391+
break
355392
}
356393

357394
// The SymmetricallyEncrypted packet, if any, might have an

openpgp/read_test.go

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ func checkSignedMessage(t *testing.T, signedHex, expected string) {
116116
return
117117
}
118118

119-
if !md.IsSigned || md.SignedByKeyId != 0xa34d7e18c20c31bb || md.SignedBy == nil || md.IsEncrypted || md.IsSymmetricallyEncrypted || len(md.EncryptedToKeyIds) != 0 || md.IsSymmetricallyEncrypted {
119+
if !md.IsSigned || md.SignedByKeyId != 0xa34d7e18c20c31bb || md.SignedBy == nil || md.IsEncrypted || md.IsSymmetricallyEncrypted || len(md.EncryptedToKeyIds) != 0 || md.IsSymmetricallyEncrypted || md.MultiSig {
120120
t.Errorf("bad MessageDetails: %#v", md)
121121
}
122122

@@ -209,7 +209,7 @@ func TestSignedEncryptedMessage(t *testing.T) {
209209
return
210210
}
211211

212-
if !md.IsSigned || md.SignedByKeyId != test.signedByKeyId || md.SignedBy == nil || !md.IsEncrypted || md.IsSymmetricallyEncrypted || len(md.EncryptedToKeyIds) == 0 || md.EncryptedToKeyIds[0] != test.encryptedToKeyId {
212+
if !md.IsSigned || md.SignedByKeyId != test.signedByKeyId || md.SignedBy == nil || !md.IsEncrypted || md.IsSymmetricallyEncrypted || len(md.EncryptedToKeyIds) == 0 || md.EncryptedToKeyIds[0] != test.encryptedToKeyId || md.MultiSig {
213213
t.Errorf("#%d: bad MessageDetails: %#v", i, md)
214214
}
215215

@@ -897,6 +897,31 @@ func TestIllformedArmors(t *testing.T) {
897897
}
898898
}
899899

900+
func TestSignedTextMessageNoSignature(t *testing.T) {
901+
kring, _ := ReadKeyRing(readerFromHex(testKeys1And2Hex))
902+
903+
md, err := ReadMessage(readerFromHex(signedTextMessageMalformed), kring, nil, nil)
904+
if err != nil {
905+
t.Fatal(err)
906+
}
907+
908+
if !md.IsSigned || md.SignedByKeyId != 0xa34d7e18c20c31bb || md.SignedBy == nil || md.IsEncrypted || md.IsSymmetricallyEncrypted || len(md.EncryptedToKeyIds) != 0 || md.IsSymmetricallyEncrypted || md.MultiSig {
909+
t.Errorf("bad MessageDetails: %#v", md)
910+
}
911+
912+
contents, err := ioutil.ReadAll(md.UnverifiedBody)
913+
if err != nil {
914+
t.Errorf("error reading UnverifiedBody: %s", err)
915+
}
916+
if string(contents) != signedTextInput {
917+
t.Errorf("bad UnverifiedBody got:%s want:%s", string(contents), signedTextInput)
918+
}
919+
if md.SignatureError == nil || md.Signature != nil {
920+
t.Fatalf("Expected SignatureError, got nil")
921+
}
922+
t.Logf("SignatureError is: %s", md.SignatureError)
923+
}
924+
900925
const testKey1KeyId = 0xA34D7E18C20C31BB
901926
const testKey3KeyId = 0x338934250CCC0360
902927

@@ -923,6 +948,11 @@ const signedMessageHex = "a3019bc0cbccc0c4b8d8b74ee2108fe16ec6d3ca490cbe362d3f83
923948

924949
const signedTextMessageHex = "a3019bc0cbccc8c4b8d8b74ee2108fe16ec6d36a250cbece0c178233d3f352531472538b8b13d35379b97232f352158ca0b4312f57c71c1646462606365626906a062e4e019811591798ff99bf8afee860b0d8a8c2a85c3387e3bcf0bb3b17987f2bbcfab2aa526d930cbfd3d98757184df3995c9f3e7790e36e3e9779f06089d4c64e9e47dd6202cb6e9bc73c5d11bb59fbaf89d22d8dc7cf199ddf17af96e77c5f65f9bbed56f427bd8db7af37f6c9984bf9385efaf5f184f986fb3e6adb0ecfe35bbf92d16a7aa2a344fb0bc52fb7624f0200"
925950

951+
// Same message as signedTextMessageHex but "signature packet" is
952+
// dropped from the end. So this message claims to be signed but it
953+
// cannot be verified, so verification should fail.
954+
const signedTextMessageMalformed = "900d03010201a34d7e18c20c31bb01cb2674004d4300d05369676e6564206d6573736167650d0a6c696e6520320d0a6c696e6520330d0a"
955+
926956
const signedEncryptedMessageHex = "848c032a67d68660df41c70103ff5789d0de26b6a50c985a02a13131ca829c413a35d0e6fa8d6842599252162808ac7439c72151c8c6183e76923fe3299301414d0c25a2f06a2257db3839e7df0ec964773f6e4c4ac7ff3b48c444237166dd46ba8ff443a5410dc670cb486672fdbe7c9dfafb75b4fea83af3a204fe2a7dfa86bd20122b4f3d2646cbeecb8f7be8d2c03b018bd210b1d3791e1aba74b0f1034e122ab72e760492c192383cf5e20b5628bd043272d63df9b923f147eb6091cd897553204832aba48fec54aa447547bb16305a1024713b90e77fd0065f1918271947549205af3c74891af22ee0b56cd29bfec6d6e351901cd4ab3ece7c486f1e32a792d4e474aed98ee84b3f591c7dff37b64e0ecd68fd036d517e412dcadf85840ce184ad7921ad446c4ee28db80447aea1ca8d4f574db4d4e37688158ddd19e14ee2eab4873d46947d65d14a23e788d912cf9a19624ca7352469b72a83866b7c23cb5ace3deab3c7018061b0ba0f39ed2befe27163e5083cf9b8271e3e3d52cc7ad6e2a3bd81d4c3d7022f8d"
927957

928958
const signedEncryptedMessage2Hex = "85010e03cf6a7abcd43e36731003fb057f5495b79db367e277cdbe4ab90d924ddee0c0381494112ff8c1238fb0184af35d1731573b01bc4c55ecacd2aafbe2003d36310487d1ecc9ac994f3fada7f9f7f5c3a64248ab7782906c82c6ff1303b69a84d9a9529c31ecafbcdb9ba87e05439897d87e8a2a3dec55e14df19bba7f7bd316291c002ae2efd24f83f9e3441203fc081c0c23dc3092a454ca8a082b27f631abf73aca341686982e8fbda7e0e7d863941d68f3de4a755c2964407f4b5e0477b3196b8c93d551dd23c8beef7d0f03fbb1b6066f78907faf4bf1677d8fcec72651124080e0b7feae6b476e72ab207d38d90b958759fdedfc3c6c35717c9dbfc979b3cfbbff0a76d24a5e57056bb88acbd2a901ef64bc6e4db02adc05b6250ff378de81dca18c1910ab257dff1b9771b85bb9bbe0a69f5989e6d1710a35e6dfcceb7d8fb5ccea8db3932b3d9ff3fe0d327597c68b3622aec8e3716c83a6c93f497543b459b58ba504ed6bcaa747d37d2ca746fe49ae0a6ce4a8b694234e941b5159ff8bd34b9023da2814076163b86f40eed7c9472f81b551452d5ab87004a373c0172ec87ea6ce42ccfa7dbdad66b745496c4873d8019e8c28d6b3"

0 commit comments

Comments
 (0)