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

[Jelmer Vernooij]

Add support for passing mode argument to GPGSignature.sign.

Add support for passing mode argument to GPGSignature.sign.

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

[Jelmer Vernooij (jelmer)]

review: Approve

=== modified file 'breezy/bzr/remote.py'
--- breezy/bzr/remote.py	2017-11-23 20:02:55 +0000
+++ breezy/bzr/remote.py	2018-03-18 20:19:21 +0000
@@ -2692,7 +2692,7 @@
 
     def store_revision_signature(self, gpg_strategy, plaintext, revision_id):
         with self.lock_write():
-            signature = gpg_strategy.sign(plaintext)
+            signature = gpg_strategy.sign(plaintext, gpg.MODE_CLEAR)
             self.add_signature_text(revision_id, signature)
 
     def add_signature_text(self, revision_id, signature):
@@ -2737,9 +2737,11 @@
             signature = self.get_signature_text(revision_id)
 
             testament = _mod_testament.Testament.from_revision(self, revision_id)
-            plaintext = testament.as_short_text()
 
-            return gpg_strategy.verify(signature, plaintext)
+            (status, key, signed_plaintext) = gpg_strategy.verify(signature)
+            if testament.as_short_text() != signed_plaintext:
+                return gpg.SIGNATURE_NOT_VALID, None
+            return (status, key)
 
     def item_keys_introduced_by(self, revision_ids, _files_pb=None):
         self._ensure_real()
 
=== modified file 'breezy/gpg.py'
--- breezy/gpg.py	2018-02-24 15:50:23 +0000
+++ breezy/gpg.py	2018-03-18 20:19:21 +0000
@@ -52,6 +52,10 @@
 SIGNATURE_NOT_SIGNED = 3
 SIGNATURE_EXPIRED = 4
 
+MODE_NORMAL = 0
+MODE_DETACH = 1
+MODE_CLEAR = 2
+
 
 class GpgNotInstalled(errors.DependencyNotPresent):
 
@@ -123,10 +127,10 @@
     def __init__(self, ignored):
         """Real strategies take a configuration."""
 
-    def sign(self, content):
+    def sign(self, content, mode):
         raise SigningFailed('Signing is disabled.')
 
-    def verify(self, content, testament):
+    def verify(self, signed_data, signature=None):
         raise SignatureVerificationFailed('Signature verification is \
 disabled.')
 
@@ -146,12 +150,14 @@
     def __init__(self, ignored):
         """Real strategies take a configuration."""
 
-    def sign(self, content):
+    def sign(self, content, mode):
         return ("-----BEGIN PSEUDO-SIGNED CONTENT-----\n" + content +
                 "-----END PSEUDO-SIGNED CONTENT-----\n")
 
-    def verify(self, content, testament):
-        return SIGNATURE_VALID, None
+    def verify(self, signed_data, signature=None):
+        plain_text = signed_data.replace("-----BEGIN PSEUDO-SIGNED CONTENT-----\n", "")
+        plain_text = plain_text.replace("-----END PSEUDO-SIGNED CONTENT-----\n", "")
+        return SIGNATURE_VALID, None, plain_text
 
     def set_acceptable_keys(self, command_line_input):
         if command_line_input is not None:
@@ -187,6 +193,7 @@
         try:
             import gpg
             self.context = gpg.Context()
+            self.context.armor = True
         except ImportError as error:
             pass # can't use verify()
 
@@ -224,7 +231,7 @@
         except ImportError as error:
             return False
 
-    def sign(self, content):
+    def sign(self, content, mode):
         import gpg
         if isinstance(content, unicode):
             raise errors.BzrBadParameterUnicode('content')
@@ -232,29 +239,34 @@
         plain_text = gpg.Data(content)
         try:
             output, result = self.context.sign(
-                plain_text, mode=gpg.constants.sig.mode.CLEAR)
+                plain_text, mode={
+                    MODE_DETACH: gpg.constants.sig.mode.DETACH,
+                    MODE_CLEAR: gpg.constants.sig.mode.CLEAR,
+                    MODE_NORMAL: gpg.constants.sig.mode.NORMAL,
+                    }[mode])
         except gpg.errors.GPGMEError as error:
             raise SigningFailed(str(error))
 
         return output
 
-    def verify(self, content, testament):
+    def verify(self, signed_data, signature=None):
         """Check content has a valid signature.
 
-        :param content: the commit signature
-        :param testament: the valid testament string for the commit
+        :param signed_data; Signed data
+        :param signature: optional signature (if detached)
 
-        :return: SIGNATURE_VALID or a failed SIGNATURE_ value, key uid if valid
+        :return: SIGNATURE_VALID or a failed SIGNATURE_ value, key uid if valid, plain text
         """
         try:
             import gpg
         except ImportError as error:
             raise errors.GpgNotInstalled(error)
 
-        signature = gpg.Data(content)
-        sink = gpg.Data()
+        signed_data = gpg.Data(signed_data)
+        if signature:
+            signature = gpg.Data(signature)
         try:
-            plain_output, result = self.context.verify(signature)
+            plain_output, result = self.context.verify(signed_data, signature)
         except gpg.errors.BadSignatures as error:
             fingerprint = error.result.signatures[0].fpr
             if error.result.signatures[0].summary & gpg.constants.SIGSUM_KEY_EXPIRED:
@@ -262,35 +274,32 @@
                 if expires > error.result.signatures[0].timestamp:
                     # The expired key was not expired at time of signing.
                     # test_verify_expired_but_valid()
-                    return SIGNATURE_EXPIRED, fingerprint[-8:]
+                    return SIGNATURE_EXPIRED, fingerprint[-8:], None
                 else:
                     # I can't work out how to create a test where the signature
                     # was expired at the time of signing.
-                    return SIGNATURE_NOT_VALID, None
+                    return SIGNATURE_NOT_VALID, None, None
 
             # GPG does not know this key.
             # test_verify_unknown_key()
             if error.result.signatures[0].summary & gpg.constants.SIGSUM_KEY_MISSING:
-                return SIGNATURE_KEY_MISSING, fingerprint[-8:]
+                return SIGNATURE_KEY_MISSING, fingerprint[-8:], None
 
-            return SIGNATURE_NOT_VALID, None
+            return SIGNATURE_NOT_VALID, None, None
         except gpg.errors.GPGMEError as error:
-            raise SignatureVerificationFailed(error[2])
+            raise SignatureVerificationFailed(error)
 
         # No result if input is invalid.
         # test_verify_invalid()
         if len(result.signatures) == 0:
-            return SIGNATURE_NOT_VALID, None
+            return SIGNATURE_NOT_VALID, None, plain_output
+
         # User has specified a list of acceptable keys, check our result is in
         # it.  test_verify_unacceptable_key()
         fingerprint = result.signatures[0].fpr
         if self.acceptable_keys is not None:
             if not fingerprint in self.acceptable_keys:
-                return SIGNATURE_KEY_MISSING, fingerprint[-8:]
-        # Check the signature actually matches the testament.
-        # test_verify_bad_testament()
-        if testament != plain_output:
-            return SIGNATURE_NOT_VALID, None
+                return SIGNATURE_KEY_MISSING, fingerprint[-8:], plain_output
         # Yay gpg set the valid bit.
         # Can't write a test for this one as you can't set a key to be
         # trusted using gpg.
@@ -298,20 +307,20 @@
             key = self.context.get_key(fingerprint)
             name = key.uids[0].name
             email = key.uids[0].email
-            return SIGNATURE_VALID, name + " <" + email + ">"
+            return SIGNATURE_VALID, name.decode('utf-8') + u" <" + email.decode('utf-8') + u">", plain_output
         # Sigsum_red indicates a problem, unfortunatly I have not been able
         # to write any tests which actually set this.
         if result.signatures[0].summary & gpg.constants.SIGSUM_RED:
-            return SIGNATURE_NOT_VALID, None
+            return SIGNATURE_NOT_VALID, None, plain_output
         # Summary isn't set if sig is valid but key is untrusted but if user
         # has explicity set the key as acceptable we can validate it.
         if result.signatures[0].summary == 0 and self.acceptable_keys is not None:
             if fingerprint in self.acceptable_keys:
                 # test_verify_untrusted_but_accepted()
-                return SIGNATURE_VALID, None
+                return SIGNATURE_VALID, None, plain_output
         # test_verify_valid_but_untrusted()
         if result.signatures[0].summary == 0 and self.acceptable_keys is None:
-            return SIGNATURE_NOT_VALID, None
+            return SIGNATURE_NOT_VALID, None, plain_output
         # Other error types such as revoked keys should (I think) be caught by
         # SIGSUM_RED so anything else means something is buggy.
         raise SignatureVerificationFailed(
 
=== modified file 'breezy/merge_directive.py'
--- breezy/merge_directive.py	2017-10-27 00:18:42 +0000
+++ breezy/merge_directive.py	2018-03-18 20:19:21 +0000
@@ -251,7 +251,7 @@
         :return: a string
         """
         my_gpg = gpg.GPGStrategy(branch.get_config_stack())
-        return my_gpg.sign(''.join(self.to_lines()))
+        return my_gpg.sign(''.join(self.to_lines()), gpg.MODE_CLEAR)
 
     def to_email(self, mail_to, branch, sign=False):
         """Serialize as an email message.
 
=== modified file 'breezy/repository.py'
--- breezy/repository.py	2018-02-24 15:50:23 +0000
+++ breezy/repository.py	2018-03-18 20:19:21 +0000
@@ -887,7 +887,7 @@
 
     def store_revision_signature(self, gpg_strategy, plaintext, revision_id):
         with self.lock_write():
-            signature = gpg_strategy.sign(plaintext)
+            signature = gpg_strategy.sign(plaintext, gpg.MODE_CLEAR)
             self.add_signature_text(revision_id, signature)
 
     def add_signature_text(self, revision_id, signature):
@@ -1104,11 +1104,12 @@
                 return gpg.SIGNATURE_NOT_SIGNED, None
             signature = self.get_signature_text(revision_id)
 
-            testament = _mod_testament.Testament.from_revision(
-                    self, revision_id)
-            plaintext = testament.as_short_text()
+            testament = _mod_testament.Testament.from_revision(self, revision_id)
 
-            return gpg_strategy.verify(signature, plaintext)
+            (status, key, signed_plaintext) = gpg_strategy.verify(signature)
+            if testament.as_short_text() != signed_plaintext:
+                return gpg.SIGNATURE_NOT_VALID, None
+            return (status, key)
 
     def verify_revision_signatures(self, revision_ids, gpg_strategy):
         """Verify revision signatures for a number of revisions.
 
=== modified file 'breezy/tests/per_repository/test_signatures.py'
--- breezy/tests/per_repository/test_signatures.py	2017-08-07 19:09:47 +0000
+++ breezy/tests/per_repository/test_signatures.py	2018-03-18 20:19:21 +0000
@@ -123,7 +123,7 @@
                          '-----END PSEUDO-SIGNED CONTENT-----\n',
                          repo.get_signature_text(a))
         self.assertEqual(
-            (gpg.SIGNATURE_VALID, None, ),
+            (gpg.SIGNATURE_VALID, None),
             repo.verify_revision_signature(a, strategy))
 
     def test_verify_revision_signatures(self):
 
=== modified file 'breezy/tests/test_commit.py'
--- breezy/tests/test_commit.py	2018-02-15 19:38:33 +0000
+++ breezy/tests/test_commit.py	2018-03-18 20:19:21 +0000
@@ -435,7 +435,8 @@
                 message="base", allow_pointless=True, rev_id='B',
                 working_tree=wt)
             def sign(text):
-                return breezy.gpg.LoopbackGPGStrategy(None).sign(text)
+                return breezy.gpg.LoopbackGPGStrategy(None).sign(
+                        text, breezy.gpg.MODE_CLEAR)
             self.assertEqual(sign(Testament.from_revision(branch.repository,
                                                           'B').as_short_text()),
                              branch.repository.get_signature_text('B'))
 
=== modified file 'breezy/tests/test_gpg.py'
--- breezy/tests/test_gpg.py	2017-07-04 20:03:11 +0000
+++ breezy/tests/test_gpg.py	2018-03-18 20:19:21 +0000
@@ -224,8 +224,7 @@
 """
         my_gpg = gpg.GPGStrategy(FakeConfig())
         my_gpg.set_acceptable_keys("[e-mail]")
-        self.assertEqual((gpg.SIGNATURE_VALID, None), my_gpg.verify(content,
-                            plain))
+        self.assertEqual((gpg.SIGNATURE_VALID, None, plain), my_gpg.verify(content))
 
     def test_verify_unacceptable_key(self):
         self.requireFeature(features.gpg)
@@ -255,8 +254,8 @@
 """
         my_gpg = gpg.GPGStrategy(FakeConfig())
         my_gpg.set_acceptable_keys("[e-mail]")
-        self.assertEqual((gpg.SIGNATURE_KEY_MISSING, u'E3080E45'),
-                         my_gpg.verify(content, plain))
+        self.assertEqual((gpg.SIGNATURE_KEY_MISSING, u'E3080E45', plain),
+                         my_gpg.verify(content))
 
     def test_verify_valid_but_untrusted(self):
         self.requireFeature(features.gpg)
@@ -285,40 +284,7 @@
 sha1: 6411f9bdf6571200357140c9ce7c0f50106ac9a4
 """
         my_gpg = gpg.GPGStrategy(FakeConfig())
-        self.assertEqual((gpg.SIGNATURE_NOT_VALID, None), my_gpg.verify(content,
-                            plain))
-
-    def test_verify_bad_testament(self):
-        self.requireFeature(features.gpg)
-        self.import_keys()
-
-        content = """-----BEGIN PGP SIGNED MESSAGE-----
-Hash: SHA1
-
-bazaar-ng testament short form 1
-revision-id: [e-mail]-20110527185938-hluafawphszb8dl1
-sha1: 6411f9bdf6571200357140c9ce7c0f50106ac9a4
------BEGIN PGP SIGNATURE-----
-Version: GnuPG v1.4.11 (GNU/Linux)
-
-iQEcBAEBAgAGBQJN+ekFAAoJEIdoGx7jCA5FGtEH/i+XxJRvqU6wdBtLVrGBMAGk
-FZ5VP+KyXYtymSbgSstj/vM12NeMIeFs3xGnNnYuX1MIcY6We5TKtCH0epY6ym5+
-6g2Q2QpQ5/sT2d0mWzR0K4uVngmxVQaXTdk5PdZ40O7ULeDLW6CxzxMHyUL1rsIx
-7UBUTBh1O/1n3ZfD99hUkm3hVcnsN90uTKH59zV9NWwArU0cug60+5eDKJhSJDbG
-rIwlqbFAjDZ7L/48e+IaYIJwBZFzMBpJKdCxzALLtauMf+KK8hGiL2hrRbWm7ty6
-NgxfkMYOB4rDPdSstT35N+5uBG3n/UzjxHssi0svMfVETYYX40y57dm2eZQXFp8=
-=iwsn
------END PGP SIGNATURE-----
-"""
-        plain = """bazaar-ng testament short form 1
-revision-id: [e-mail]-20110527185938-hluafawphszb8dl1
-sha1: 6411f9bdf6571200357140c9ce7c0f50106ac9a4
-"""
-        my_gpg = gpg.GPGStrategy(FakeConfig())
-        my_gpg.set_acceptable_keys("[e-mail]")
-        self.assertEqual((gpg.SIGNATURE_NOT_VALID, None), my_gpg.verify(content,
-                            plain))
-
+        self.assertEqual((gpg.SIGNATURE_NOT_VALID, None, plain), my_gpg.verify(content))
 
     def test_verify_revoked_signature(self):
         self.requireFeature(features.gpg)
@@ -341,8 +307,7 @@
         plain = """asdf\n"""
         my_gpg = gpg.GPGStrategy(FakeConfig())
         my_gpg.set_acceptable_keys("[e-mail]")
-        self.assertEqual((gpg.SIGNATURE_NOT_VALID, None), my_gpg.verify(content,
-                            plain))
+        self.assertEqual((gpg.SIGNATURE_NOT_VALID, None, None), my_gpg.verify(content))
 
     def test_verify_invalid(self):
         self.requireFeature(features.gpg)
@@ -366,8 +331,8 @@
 sha1: 6411f9bdf6571200357140c9ce7c0f50106ac9a4
 """
         my_gpg = gpg.GPGStrategy(FakeConfig())
-        self.assertEqual((gpg.SIGNATURE_NOT_VALID, None),
-                            my_gpg.verify(content, plain))
+        self.assertEqual((gpg.SIGNATURE_NOT_VALID, None, plain),
+                            my_gpg.verify(content))
 
     def test_verify_expired_but_valid(self):
         self.requireFeature(features.gpg)
@@ -388,13 +353,9 @@
 =uHen
 -----END PGP SIGNATURE-----
 """
-        plain = """bazaar-ng testament short form 1
-revision-id: [e-mail]-20110801100657-f1dr1nompeex723z
-sha1: 59ab434be4c2d5d646dee84f514aa09e1b72feeb
-"""
         my_gpg = gpg.GPGStrategy(FakeConfig())
-        self.assertEqual((gpg.SIGNATURE_EXPIRED, u'4F8D1513'),
-                            my_gpg.verify(content, plain))
+        self.assertEqual((gpg.SIGNATURE_EXPIRED, u'4F8D1513', None),
+                            my_gpg.verify(content))
 
     def test_verify_unknown_key(self):
         self.requireFeature(features.gpg)
@@ -415,10 +376,9 @@
 =RNR5
 -----END PGP SIGNATURE-----
 """
-        plain = "asdf\n"
         my_gpg = gpg.GPGStrategy(FakeConfig())
-        self.assertEqual((gpg.SIGNATURE_KEY_MISSING, u'5D51E56F'),
-                            my_gpg.verify(content, plain))
+        self.assertEqual((gpg.SIGNATURE_KEY_MISSING, u'5D51E56F', None),
+                            my_gpg.verify(content))
 
     def test_set_acceptable_keys(self):
         self.requireFeature(features.gpg)
@@ -454,9 +414,8 @@
 
     def test_sign(self):
         self.assertRaises(gpg.SigningFailed,
-                          gpg.DisabledGPGStrategy(None).sign, 'content')
+                          gpg.DisabledGPGStrategy(None).sign, 'content', gpg.MODE_CLEAR)
 
     def test_verify(self):
         self.assertRaises(gpg.SignatureVerificationFailed,
-                          gpg.DisabledGPGStrategy(None).verify, 'content',
-                          'testament')
+                          gpg.DisabledGPGStrategy(None).verify, 'content')
 
=== modified file 'breezy/tests/test_log.py'
--- breezy/tests/test_log.py	2017-12-04 00:25:02 +0000
+++ breezy/tests/test_log.py	2018-03-18 20:19:21 +0000
@@ -317,31 +317,22 @@
 
 
 class TestFormatSignatureValidity(tests.TestCaseWithTransport):
-    class UTFLoopbackGPGStrategy(gpg.LoopbackGPGStrategy):
-        def verify(self, content, testament):
-            return (gpg.SIGNATURE_VALID,
-                u'UTF8 Test \xa1\xb1\xc1\xd1\xe1\xf1 <[e-mail]>')
-
-    def has_signature_for_revision_id(self, revision_id):
-        return True
-
-    def get_signature_text(self, revision_id):
-        return ''
+
+    def verify_revision_signature(self, revid, gpg_strategy):
+        return (gpg.SIGNATURE_VALID,
+            u'UTF8 Test \xa1\xb1\xc1\xd1\xe1\xf1 <[e-mail]>')
 
     def test_format_signature_validity_utf(self):
         """Check that GPG signatures containing UTF-8 names are formatted
         correctly."""
-        # Monkey patch to use our UTF-8 generating GPGStrategy
-        self.overrideAttr(gpg, 'GPGStrategy', self.UTFLoopbackGPGStrategy)
         wt = self.make_branch_and_tree('.')
         revid = wt.commit('empty commit')
         repo = wt.branch.repository
         # Monkey patch out checking if this rev is actually signed, since we
         # can't sign it without a heavier TestCase and LoopbackGPGStrategy
         # doesn't care anyways.
-        self.overrideAttr(repo, 'has_signature_for_revision_id',
-                self.has_signature_for_revision_id)
-        self.overrideAttr(repo, 'get_signature_text', self.get_signature_text)
+        self.overrideAttr(repo, 'verify_revision_signature',
+                self.verify_revision_signature)
         out = log.format_signature_validity(revid, wt.branch)
         self.assertEqual(
 u'valid signature from UTF8 Test \xa1\xb1\xc1\xd1\xe1\xf1 <[e-mail]>',