Merge lp:~jelmer/brz/gpg-detached-sign into lp:~brz/brz/base

Proposed by Jelmer Vernooij
Status: Merged
Approved by: Jelmer Vernooij
Approved revision: no longer in the source branch.
Merged at revision: 6892
Proposed branch: lp:~jelmer/brz/gpg-detached-sign
Merge into: lp:~brz/brz/base
Diff against target: 442 lines (+72/-109)
8 files modified
breezy/bzr/remote.py (+5/-3)
breezy/gpg.py (+38/-29)
breezy/merge_directive.py (+1/-1)
breezy/repository.py (+6/-5)
breezy/tests/per_repository/test_signatures.py (+1/-1)
breezy/tests/test_commit.py (+2/-1)
breezy/tests/test_gpg.py (+13/-54)
breezy/tests/test_log.py (+6/-15)
To merge this branch: bzr merge lp:~jelmer/brz/gpg-detached-sign
Reviewer Review Type Date Requested Status
Jelmer Vernooij Approve
Review via email: mp+341598@code.launchpad.net

Commit message

Add support for passing mode argument to GPGSignature.sign.

Description of the change

Add support for passing mode argument to GPGSignature.sign.

This is necessary for brz-git, which needs to create detached signatures.

To post a comment you must log in.
Revision history for this message
Jelmer Vernooij (jelmer) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'breezy/bzr/remote.py'
2--- breezy/bzr/remote.py 2017-11-23 20:02:55 +0000
3+++ breezy/bzr/remote.py 2018-03-18 20:19:21 +0000
4@@ -2692,7 +2692,7 @@
5
6 def store_revision_signature(self, gpg_strategy, plaintext, revision_id):
7 with self.lock_write():
8- signature = gpg_strategy.sign(plaintext)
9+ signature = gpg_strategy.sign(plaintext, gpg.MODE_CLEAR)
10 self.add_signature_text(revision_id, signature)
11
12 def add_signature_text(self, revision_id, signature):
13@@ -2737,9 +2737,11 @@
14 signature = self.get_signature_text(revision_id)
15
16 testament = _mod_testament.Testament.from_revision(self, revision_id)
17- plaintext = testament.as_short_text()
18
19- return gpg_strategy.verify(signature, plaintext)
20+ (status, key, signed_plaintext) = gpg_strategy.verify(signature)
21+ if testament.as_short_text() != signed_plaintext:
22+ return gpg.SIGNATURE_NOT_VALID, None
23+ return (status, key)
24
25 def item_keys_introduced_by(self, revision_ids, _files_pb=None):
26 self._ensure_real()
27
28=== modified file 'breezy/gpg.py'
29--- breezy/gpg.py 2018-02-24 15:50:23 +0000
30+++ breezy/gpg.py 2018-03-18 20:19:21 +0000
31@@ -52,6 +52,10 @@
32 SIGNATURE_NOT_SIGNED = 3
33 SIGNATURE_EXPIRED = 4
34
35+MODE_NORMAL = 0
36+MODE_DETACH = 1
37+MODE_CLEAR = 2
38+
39
40 class GpgNotInstalled(errors.DependencyNotPresent):
41
42@@ -123,10 +127,10 @@
43 def __init__(self, ignored):
44 """Real strategies take a configuration."""
45
46- def sign(self, content):
47+ def sign(self, content, mode):
48 raise SigningFailed('Signing is disabled.')
49
50- def verify(self, content, testament):
51+ def verify(self, signed_data, signature=None):
52 raise SignatureVerificationFailed('Signature verification is \
53 disabled.')
54
55@@ -146,12 +150,14 @@
56 def __init__(self, ignored):
57 """Real strategies take a configuration."""
58
59- def sign(self, content):
60+ def sign(self, content, mode):
61 return ("-----BEGIN PSEUDO-SIGNED CONTENT-----\n" + content +
62 "-----END PSEUDO-SIGNED CONTENT-----\n")
63
64- def verify(self, content, testament):
65- return SIGNATURE_VALID, None
66+ def verify(self, signed_data, signature=None):
67+ plain_text = signed_data.replace("-----BEGIN PSEUDO-SIGNED CONTENT-----\n", "")
68+ plain_text = plain_text.replace("-----END PSEUDO-SIGNED CONTENT-----\n", "")
69+ return SIGNATURE_VALID, None, plain_text
70
71 def set_acceptable_keys(self, command_line_input):
72 if command_line_input is not None:
73@@ -187,6 +193,7 @@
74 try:
75 import gpg
76 self.context = gpg.Context()
77+ self.context.armor = True
78 except ImportError as error:
79 pass # can't use verify()
80
81@@ -224,7 +231,7 @@
82 except ImportError as error:
83 return False
84
85- def sign(self, content):
86+ def sign(self, content, mode):
87 import gpg
88 if isinstance(content, unicode):
89 raise errors.BzrBadParameterUnicode('content')
90@@ -232,29 +239,34 @@
91 plain_text = gpg.Data(content)
92 try:
93 output, result = self.context.sign(
94- plain_text, mode=gpg.constants.sig.mode.CLEAR)
95+ plain_text, mode={
96+ MODE_DETACH: gpg.constants.sig.mode.DETACH,
97+ MODE_CLEAR: gpg.constants.sig.mode.CLEAR,
98+ MODE_NORMAL: gpg.constants.sig.mode.NORMAL,
99+ }[mode])
100 except gpg.errors.GPGMEError as error:
101 raise SigningFailed(str(error))
102
103 return output
104
105- def verify(self, content, testament):
106+ def verify(self, signed_data, signature=None):
107 """Check content has a valid signature.
108
109- :param content: the commit signature
110- :param testament: the valid testament string for the commit
111+ :param signed_data; Signed data
112+ :param signature: optional signature (if detached)
113
114- :return: SIGNATURE_VALID or a failed SIGNATURE_ value, key uid if valid
115+ :return: SIGNATURE_VALID or a failed SIGNATURE_ value, key uid if valid, plain text
116 """
117 try:
118 import gpg
119 except ImportError as error:
120 raise errors.GpgNotInstalled(error)
121
122- signature = gpg.Data(content)
123- sink = gpg.Data()
124+ signed_data = gpg.Data(signed_data)
125+ if signature:
126+ signature = gpg.Data(signature)
127 try:
128- plain_output, result = self.context.verify(signature)
129+ plain_output, result = self.context.verify(signed_data, signature)
130 except gpg.errors.BadSignatures as error:
131 fingerprint = error.result.signatures[0].fpr
132 if error.result.signatures[0].summary & gpg.constants.SIGSUM_KEY_EXPIRED:
133@@ -262,35 +274,32 @@
134 if expires > error.result.signatures[0].timestamp:
135 # The expired key was not expired at time of signing.
136 # test_verify_expired_but_valid()
137- return SIGNATURE_EXPIRED, fingerprint[-8:]
138+ return SIGNATURE_EXPIRED, fingerprint[-8:], None
139 else:
140 # I can't work out how to create a test where the signature
141 # was expired at the time of signing.
142- return SIGNATURE_NOT_VALID, None
143+ return SIGNATURE_NOT_VALID, None, None
144
145 # GPG does not know this key.
146 # test_verify_unknown_key()
147 if error.result.signatures[0].summary & gpg.constants.SIGSUM_KEY_MISSING:
148- return SIGNATURE_KEY_MISSING, fingerprint[-8:]
149+ return SIGNATURE_KEY_MISSING, fingerprint[-8:], None
150
151- return SIGNATURE_NOT_VALID, None
152+ return SIGNATURE_NOT_VALID, None, None
153 except gpg.errors.GPGMEError as error:
154- raise SignatureVerificationFailed(error[2])
155+ raise SignatureVerificationFailed(error)
156
157 # No result if input is invalid.
158 # test_verify_invalid()
159 if len(result.signatures) == 0:
160- return SIGNATURE_NOT_VALID, None
161+ return SIGNATURE_NOT_VALID, None, plain_output
162+
163 # User has specified a list of acceptable keys, check our result is in
164 # it. test_verify_unacceptable_key()
165 fingerprint = result.signatures[0].fpr
166 if self.acceptable_keys is not None:
167 if not fingerprint in self.acceptable_keys:
168- return SIGNATURE_KEY_MISSING, fingerprint[-8:]
169- # Check the signature actually matches the testament.
170- # test_verify_bad_testament()
171- if testament != plain_output:
172- return SIGNATURE_NOT_VALID, None
173+ return SIGNATURE_KEY_MISSING, fingerprint[-8:], plain_output
174 # Yay gpg set the valid bit.
175 # Can't write a test for this one as you can't set a key to be
176 # trusted using gpg.
177@@ -298,20 +307,20 @@
178 key = self.context.get_key(fingerprint)
179 name = key.uids[0].name
180 email = key.uids[0].email
181- return SIGNATURE_VALID, name + " <" + email + ">"
182+ return SIGNATURE_VALID, name.decode('utf-8') + u" <" + email.decode('utf-8') + u">", plain_output
183 # Sigsum_red indicates a problem, unfortunatly I have not been able
184 # to write any tests which actually set this.
185 if result.signatures[0].summary & gpg.constants.SIGSUM_RED:
186- return SIGNATURE_NOT_VALID, None
187+ return SIGNATURE_NOT_VALID, None, plain_output
188 # Summary isn't set if sig is valid but key is untrusted but if user
189 # has explicity set the key as acceptable we can validate it.
190 if result.signatures[0].summary == 0 and self.acceptable_keys is not None:
191 if fingerprint in self.acceptable_keys:
192 # test_verify_untrusted_but_accepted()
193- return SIGNATURE_VALID, None
194+ return SIGNATURE_VALID, None, plain_output
195 # test_verify_valid_but_untrusted()
196 if result.signatures[0].summary == 0 and self.acceptable_keys is None:
197- return SIGNATURE_NOT_VALID, None
198+ return SIGNATURE_NOT_VALID, None, plain_output
199 # Other error types such as revoked keys should (I think) be caught by
200 # SIGSUM_RED so anything else means something is buggy.
201 raise SignatureVerificationFailed(
202
203=== modified file 'breezy/merge_directive.py'
204--- breezy/merge_directive.py 2017-10-27 00:18:42 +0000
205+++ breezy/merge_directive.py 2018-03-18 20:19:21 +0000
206@@ -251,7 +251,7 @@
207 :return: a string
208 """
209 my_gpg = gpg.GPGStrategy(branch.get_config_stack())
210- return my_gpg.sign(''.join(self.to_lines()))
211+ return my_gpg.sign(''.join(self.to_lines()), gpg.MODE_CLEAR)
212
213 def to_email(self, mail_to, branch, sign=False):
214 """Serialize as an email message.
215
216=== modified file 'breezy/repository.py'
217--- breezy/repository.py 2018-02-24 15:50:23 +0000
218+++ breezy/repository.py 2018-03-18 20:19:21 +0000
219@@ -887,7 +887,7 @@
220
221 def store_revision_signature(self, gpg_strategy, plaintext, revision_id):
222 with self.lock_write():
223- signature = gpg_strategy.sign(plaintext)
224+ signature = gpg_strategy.sign(plaintext, gpg.MODE_CLEAR)
225 self.add_signature_text(revision_id, signature)
226
227 def add_signature_text(self, revision_id, signature):
228@@ -1104,11 +1104,12 @@
229 return gpg.SIGNATURE_NOT_SIGNED, None
230 signature = self.get_signature_text(revision_id)
231
232- testament = _mod_testament.Testament.from_revision(
233- self, revision_id)
234- plaintext = testament.as_short_text()
235+ testament = _mod_testament.Testament.from_revision(self, revision_id)
236
237- return gpg_strategy.verify(signature, plaintext)
238+ (status, key, signed_plaintext) = gpg_strategy.verify(signature)
239+ if testament.as_short_text() != signed_plaintext:
240+ return gpg.SIGNATURE_NOT_VALID, None
241+ return (status, key)
242
243 def verify_revision_signatures(self, revision_ids, gpg_strategy):
244 """Verify revision signatures for a number of revisions.
245
246=== modified file 'breezy/tests/per_repository/test_signatures.py'
247--- breezy/tests/per_repository/test_signatures.py 2017-08-07 19:09:47 +0000
248+++ breezy/tests/per_repository/test_signatures.py 2018-03-18 20:19:21 +0000
249@@ -123,7 +123,7 @@
250 '-----END PSEUDO-SIGNED CONTENT-----\n',
251 repo.get_signature_text(a))
252 self.assertEqual(
253- (gpg.SIGNATURE_VALID, None, ),
254+ (gpg.SIGNATURE_VALID, None),
255 repo.verify_revision_signature(a, strategy))
256
257 def test_verify_revision_signatures(self):
258
259=== modified file 'breezy/tests/test_commit.py'
260--- breezy/tests/test_commit.py 2018-02-15 19:38:33 +0000
261+++ breezy/tests/test_commit.py 2018-03-18 20:19:21 +0000
262@@ -435,7 +435,8 @@
263 message="base", allow_pointless=True, rev_id='B',
264 working_tree=wt)
265 def sign(text):
266- return breezy.gpg.LoopbackGPGStrategy(None).sign(text)
267+ return breezy.gpg.LoopbackGPGStrategy(None).sign(
268+ text, breezy.gpg.MODE_CLEAR)
269 self.assertEqual(sign(Testament.from_revision(branch.repository,
270 'B').as_short_text()),
271 branch.repository.get_signature_text('B'))
272
273=== modified file 'breezy/tests/test_gpg.py'
274--- breezy/tests/test_gpg.py 2017-07-04 20:03:11 +0000
275+++ breezy/tests/test_gpg.py 2018-03-18 20:19:21 +0000
276@@ -224,8 +224,7 @@
277 """
278 my_gpg = gpg.GPGStrategy(FakeConfig())
279 my_gpg.set_acceptable_keys("bazaar@example.com")
280- self.assertEqual((gpg.SIGNATURE_VALID, None), my_gpg.verify(content,
281- plain))
282+ self.assertEqual((gpg.SIGNATURE_VALID, None, plain), my_gpg.verify(content))
283
284 def test_verify_unacceptable_key(self):
285 self.requireFeature(features.gpg)
286@@ -255,8 +254,8 @@
287 """
288 my_gpg = gpg.GPGStrategy(FakeConfig())
289 my_gpg.set_acceptable_keys("foo@example.com")
290- self.assertEqual((gpg.SIGNATURE_KEY_MISSING, u'E3080E45'),
291- my_gpg.verify(content, plain))
292+ self.assertEqual((gpg.SIGNATURE_KEY_MISSING, u'E3080E45', plain),
293+ my_gpg.verify(content))
294
295 def test_verify_valid_but_untrusted(self):
296 self.requireFeature(features.gpg)
297@@ -285,40 +284,7 @@
298 sha1: 6411f9bdf6571200357140c9ce7c0f50106ac9a4
299 """
300 my_gpg = gpg.GPGStrategy(FakeConfig())
301- self.assertEqual((gpg.SIGNATURE_NOT_VALID, None), my_gpg.verify(content,
302- plain))
303-
304- def test_verify_bad_testament(self):
305- self.requireFeature(features.gpg)
306- self.import_keys()
307-
308- content = """-----BEGIN PGP SIGNED MESSAGE-----
309-Hash: SHA1
310-
311-bazaar-ng testament short form 1
312-revision-id: amy@example.com-20110527185938-hluafawphszb8dl1
313-sha1: 6411f9bdf6571200357140c9ce7c0f50106ac9a4
314------BEGIN PGP SIGNATURE-----
315-Version: GnuPG v1.4.11 (GNU/Linux)
316-
317-iQEcBAEBAgAGBQJN+ekFAAoJEIdoGx7jCA5FGtEH/i+XxJRvqU6wdBtLVrGBMAGk
318-FZ5VP+KyXYtymSbgSstj/vM12NeMIeFs3xGnNnYuX1MIcY6We5TKtCH0epY6ym5+
319-6g2Q2QpQ5/sT2d0mWzR0K4uVngmxVQaXTdk5PdZ40O7ULeDLW6CxzxMHyUL1rsIx
320-7UBUTBh1O/1n3ZfD99hUkm3hVcnsN90uTKH59zV9NWwArU0cug60+5eDKJhSJDbG
321-rIwlqbFAjDZ7L/48e+IaYIJwBZFzMBpJKdCxzALLtauMf+KK8hGiL2hrRbWm7ty6
322-NgxfkMYOB4rDPdSstT35N+5uBG3n/UzjxHssi0svMfVETYYX40y57dm2eZQXFp8=
323-=iwsn
324------END PGP SIGNATURE-----
325-"""
326- plain = """bazaar-ng testament short form 1
327-revision-id: doctor@example.com-20110527185938-hluafawphszb8dl1
328-sha1: 6411f9bdf6571200357140c9ce7c0f50106ac9a4
329-"""
330- my_gpg = gpg.GPGStrategy(FakeConfig())
331- my_gpg.set_acceptable_keys("bazaar@example.com")
332- self.assertEqual((gpg.SIGNATURE_NOT_VALID, None), my_gpg.verify(content,
333- plain))
334-
335+ self.assertEqual((gpg.SIGNATURE_NOT_VALID, None, plain), my_gpg.verify(content))
336
337 def test_verify_revoked_signature(self):
338 self.requireFeature(features.gpg)
339@@ -341,8 +307,7 @@
340 plain = """asdf\n"""
341 my_gpg = gpg.GPGStrategy(FakeConfig())
342 my_gpg.set_acceptable_keys("test@example.com")
343- self.assertEqual((gpg.SIGNATURE_NOT_VALID, None), my_gpg.verify(content,
344- plain))
345+ self.assertEqual((gpg.SIGNATURE_NOT_VALID, None, None), my_gpg.verify(content))
346
347 def test_verify_invalid(self):
348 self.requireFeature(features.gpg)
349@@ -366,8 +331,8 @@
350 sha1: 6411f9bdf6571200357140c9ce7c0f50106ac9a4
351 """
352 my_gpg = gpg.GPGStrategy(FakeConfig())
353- self.assertEqual((gpg.SIGNATURE_NOT_VALID, None),
354- my_gpg.verify(content, plain))
355+ self.assertEqual((gpg.SIGNATURE_NOT_VALID, None, plain),
356+ my_gpg.verify(content))
357
358 def test_verify_expired_but_valid(self):
359 self.requireFeature(features.gpg)
360@@ -388,13 +353,9 @@
361 =uHen
362 -----END PGP SIGNATURE-----
363 """
364- plain = """bazaar-ng testament short form 1
365-revision-id: test@example.com-20110801100657-f1dr1nompeex723z
366-sha1: 59ab434be4c2d5d646dee84f514aa09e1b72feeb
367-"""
368 my_gpg = gpg.GPGStrategy(FakeConfig())
369- self.assertEqual((gpg.SIGNATURE_EXPIRED, u'4F8D1513'),
370- my_gpg.verify(content, plain))
371+ self.assertEqual((gpg.SIGNATURE_EXPIRED, u'4F8D1513', None),
372+ my_gpg.verify(content))
373
374 def test_verify_unknown_key(self):
375 self.requireFeature(features.gpg)
376@@ -415,10 +376,9 @@
377 =RNR5
378 -----END PGP SIGNATURE-----
379 """
380- plain = "asdf\n"
381 my_gpg = gpg.GPGStrategy(FakeConfig())
382- self.assertEqual((gpg.SIGNATURE_KEY_MISSING, u'5D51E56F'),
383- my_gpg.verify(content, plain))
384+ self.assertEqual((gpg.SIGNATURE_KEY_MISSING, u'5D51E56F', None),
385+ my_gpg.verify(content))
386
387 def test_set_acceptable_keys(self):
388 self.requireFeature(features.gpg)
389@@ -454,9 +414,8 @@
390
391 def test_sign(self):
392 self.assertRaises(gpg.SigningFailed,
393- gpg.DisabledGPGStrategy(None).sign, 'content')
394+ gpg.DisabledGPGStrategy(None).sign, 'content', gpg.MODE_CLEAR)
395
396 def test_verify(self):
397 self.assertRaises(gpg.SignatureVerificationFailed,
398- gpg.DisabledGPGStrategy(None).verify, 'content',
399- 'testament')
400+ gpg.DisabledGPGStrategy(None).verify, 'content')
401
402=== modified file 'breezy/tests/test_log.py'
403--- breezy/tests/test_log.py 2017-12-04 00:25:02 +0000
404+++ breezy/tests/test_log.py 2018-03-18 20:19:21 +0000
405@@ -317,31 +317,22 @@
406
407
408 class TestFormatSignatureValidity(tests.TestCaseWithTransport):
409- class UTFLoopbackGPGStrategy(gpg.LoopbackGPGStrategy):
410- def verify(self, content, testament):
411- return (gpg.SIGNATURE_VALID,
412- u'UTF8 Test \xa1\xb1\xc1\xd1\xe1\xf1 <jrandom@example.com>')
413-
414- def has_signature_for_revision_id(self, revision_id):
415- return True
416-
417- def get_signature_text(self, revision_id):
418- return ''
419+
420+ def verify_revision_signature(self, revid, gpg_strategy):
421+ return (gpg.SIGNATURE_VALID,
422+ u'UTF8 Test \xa1\xb1\xc1\xd1\xe1\xf1 <jrandom@example.com>')
423
424 def test_format_signature_validity_utf(self):
425 """Check that GPG signatures containing UTF-8 names are formatted
426 correctly."""
427- # Monkey patch to use our UTF-8 generating GPGStrategy
428- self.overrideAttr(gpg, 'GPGStrategy', self.UTFLoopbackGPGStrategy)
429 wt = self.make_branch_and_tree('.')
430 revid = wt.commit('empty commit')
431 repo = wt.branch.repository
432 # Monkey patch out checking if this rev is actually signed, since we
433 # can't sign it without a heavier TestCase and LoopbackGPGStrategy
434 # doesn't care anyways.
435- self.overrideAttr(repo, 'has_signature_for_revision_id',
436- self.has_signature_for_revision_id)
437- self.overrideAttr(repo, 'get_signature_text', self.get_signature_text)
438+ self.overrideAttr(repo, 'verify_revision_signature',
439+ self.verify_revision_signature)
440 out = log.format_signature_validity(revid, wt.branch)
441 self.assertEqual(
442 u'valid signature from UTF8 Test \xa1\xb1\xc1\xd1\xe1\xf1 <jrandom@example.com>',

Subscribers

People subscribed via source and target branches