launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/bionic-gpg into lp:launchpad.

Commit message:
Use gpg1 rather than gpg if it exists.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/bionic-gpg/+merge/364986

On bionic, gpg is gpg2, which we don't support yet.

In the process, I refactored various scattered places where gpg was hardcoded into a single get_gpg_path function, and similarly refactored three places that configured a GPGME context into a single get_gpgme_context function.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/bionic-gpg into lp:launchpad.

=== modified file 'lib/lp/services/gpg/handler.py'
--- lib/lp/services/gpg/handler.py	2019-03-18 10:12:37 +0000
+++ lib/lp/services/gpg/handler.py	2019-03-22 19:58:42 +0000
@@ -31,6 +31,8 @@
 from lp.app.validators.email import valid_email
 from lp.services.config import config
 from lp.services.gpg.interfaces import (
+    get_gpg_path,
+    get_gpgme_context,
     GPGKeyAlgorithm,
     GPGKeyDoesNotExistOnServer,
     GPGKeyExpired,
@@ -116,14 +118,6 @@
 
         atexit.register(removeHome, self.home)
 
-    def _getContext(self):
-        """Return a new appropriately-configured GPGME context."""
-        context = gpgme.Context()
-        # Stick to GnuPG 1.
-        context.set_engine_info(gpgme.PROTOCOL_OpenPGP, "/usr/bin/gpg", None)
-        context.armor = True
-        return context
-
     def sanitizeFingerprint(self, fingerprint):
         """See IGPGHandler."""
         return sanitize_fingerprint(fingerprint)
@@ -230,7 +224,7 @@
         assert not isinstance(content, unicode)
         assert not isinstance(signature, unicode)
 
-        ctx = self._getContext()
+        ctx = get_gpgme_context()
 
         # We may not yet have the public key, so find out the fingerprint we
         # need to fetch.
@@ -273,7 +267,7 @@
     def importPublicKey(self, content):
         """See IGPGHandler."""
         assert isinstance(content, str)
-        context = self._getContext()
+        context = get_gpgme_context()
 
         newkey = StringIO(content)
         with gpgme_timeline("import", "new public key"):
@@ -308,7 +302,7 @@
         if 'GPG_AGENT_INFO' in os.environ:
             del os.environ['GPG_AGENT_INFO']
 
-        context = self._getContext()
+        context = get_gpgme_context()
         newkey = StringIO(content)
         with gpgme_timeline("import", "new secret key"):
             import_result = context.import_(newkey)
@@ -334,7 +328,7 @@
 
     def generateKey(self, name):
         """See `IGPGHandler`."""
-        context = self._getContext()
+        context = get_gpgme_context()
 
         # Make sure that gpg-agent doesn't interfere.
         if 'GPG_AGENT_INFO' in os.environ:
@@ -372,8 +366,7 @@
         if isinstance(content, unicode):
             raise TypeError('Content cannot be Unicode.')
 
-        # setup context
-        ctx = self._getContext()
+        ctx = get_gpgme_context()
 
         # setup containers
         plain = StringIO(content)
@@ -404,7 +397,7 @@
 
         # Find the key and make it the only one allowed to sign content
         # during this session.
-        context = self._getContext()
+        context = get_gpgme_context()
         context.signers = [removeSecurityProxy(key.key)]
 
         # Set up containers.
@@ -432,7 +425,7 @@
         """Get an iterator of the keys this gpg handler
         already knows about.
         """
-        ctx = self._getContext()
+        ctx = get_gpgme_context()
 
         # XXX michaeln 2010-05-07 bug=576405
         # Currently gpgme.Context().keylist fails if passed a unicode
@@ -458,7 +451,7 @@
             pubkey = self._getPubKey(fingerprint)
             key = self.importPublicKey(pubkey)
             if not key.matches(fingerprint):
-                ctx = self._getContext()
+                ctx = get_gpgme_context()
                 with gpgme_timeline("delete", key.fingerprint):
                     ctx.delete(key.key)
                 raise GPGKeyMismatchOnServer(fingerprint, key.fingerprint)
@@ -602,17 +595,9 @@
         self._buildFromGpgmeKey(key)
         return self
 
-    def _getContext(self):
-        """Return a new appropriately-configured GPGME context."""
-        context = gpgme.Context()
-        # Stick to GnuPG 1.
-        context.set_engine_info(gpgme.PROTOCOL_OpenPGP, "/usr/bin/gpg", None)
-        context.armor = True
-        return context
-
     def _buildFromFingerprint(self, fingerprint):
         """Build key information from a fingerprint."""
-        context = self._getContext()
+        context = get_gpgme_context()
         # retrive additional key information
         try:
             with gpgme_timeline("get-key", fingerprint):
@@ -658,11 +643,12 @@
             # XXX cprov 20081014: gpgme_op_export() only supports public keys.
             # See http://www.fifi.org/cgi-bin/info2www?(gpgme)Exporting+Keys
             p = subprocess.Popen(
-                ['gpg', '--export-secret-keys', '-a', self.fingerprint],
+                [get_gpg_path(), '--export-secret-keys', '-a',
+                 self.fingerprint],
                 stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
             return p.stdout.read()
 
-        context = self._getContext()
+        context = get_gpgme_context()
         keydata = StringIO()
         with gpgme_timeline("export", self.fingerprint):
             context.export(self.fingerprint.encode('ascii'), keydata)

=== modified file 'lib/lp/services/gpg/interfaces.py'
--- lib/lp/services/gpg/interfaces.py	2019-03-18 10:12:37 +0000
+++ lib/lp/services/gpg/interfaces.py	2019-03-22 19:58:42 +0000
@@ -2,6 +2,8 @@
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __all__ = [
+    'get_gpg_path',
+    'get_gpgme_context',
     'GPGKeyAlgorithm',
     'GPGKeyDoesNotExistOnServer',
     'GPGKeyExpired',
@@ -22,6 +24,7 @@
     ]
 
 import httplib
+import os.path
 import re
 
 from lazr.enum import (
@@ -56,6 +59,28 @@
         return False
 
 
+def get_gpg_path():
+    """Return the path to the GPG executable we prefer.
+
+    We stick to GnuPG 1 until we've worked out how to get things working
+    with GnuPG 2.
+    """
+    if os.path.exists("/usr/bin/gpg1"):
+        return "/usr/bin/gpg1"
+    else:
+        return "/usr/bin/gpg"
+
+
+def get_gpgme_context():
+    """Return a new appropriately-configured GPGME context."""
+    import gpgme
+
+    context = gpgme.Context()
+    context.set_engine_info(gpgme.PROTOCOL_OpenPGP, get_gpg_path(), None)
+    context.armor = True
+    return context
+
+
 # XXX: cprov 2004-10-04:
 # (gpg+dbschema) the data structure should be rearranged to support 4 field
 # needed: keynumber(1,16,17,20), keyalias(R,g,D,G), title and description

=== modified file 'lib/lp/services/gpg/tests/test_gpghandler.py'
--- lib/lp/services/gpg/tests/test_gpghandler.py	2019-03-18 10:12:37 +0000
+++ lib/lp/services/gpg/tests/test_gpghandler.py	2019-03-22 19:58:42 +0000
@@ -12,6 +12,7 @@
 from zope.security.proxy import removeSecurityProxy
 
 from lp.services.gpg.interfaces import (
+    get_gpg_path,
     GPGKeyDoesNotExistOnServer,
     GPGKeyMismatchOnServer,
     GPGKeyTemporarilyNotFoundError,
@@ -327,7 +328,8 @@
             # plumbing.
             with open(os.devnull, "w") as devnull:
                 gpg_proc = subprocess.Popen(
-                    ["gpg", "--quiet", "--status-fd", "1", "--verify"],
+                    [get_gpg_path(), "--quiet", "--status-fd", "1",
+                     "--verify"],
                     stdin=subprocess.PIPE, stdout=subprocess.PIPE,
                     stderr=devnull, universal_newlines=True)
             status = gpg_proc.communicate(signed_content)[0].splitlines()

=== modified file 'lib/lp/testing/gpgkeys/__init__.py'
--- lib/lp/testing/gpgkeys/__init__.py	2018-05-06 08:52:34 +0000
+++ lib/lp/testing/gpgkeys/__init__.py	2019-03-22 19:58:42 +0000
@@ -28,7 +28,10 @@
 
 from lp.registry.interfaces.gpg import IGPGKeySet
 from lp.registry.interfaces.person import IPersonSet
-from lp.services.gpg.interfaces import IGPGHandler
+from lp.services.gpg.interfaces import (
+    get_gpgme_context,
+    IGPGHandler,
+    )
 
 
 gpgkeysdir = os.path.join(os.path.dirname(__file__), 'data')
@@ -127,11 +130,7 @@
     if isinstance(content, unicode):
         raise TypeError('Content cannot be Unicode.')
 
-    # setup context
-    ctx = gpgme.Context()
-    # Stick to GnuPG 1.
-    ctx.set_engine_info(gpgme.PROTOCOL_OpenPGP, "/usr/bin/gpg", None)
-    ctx.armor = True
+    ctx = get_gpgme_context()
 
     # setup containers
     cipher = StringIO(content)

=== modified file 'utilities/make-lp-user'
--- utilities/make-lp-user	2019-02-23 07:41:11 +0000
+++ utilities/make-lp-user	2019-03-22 19:58:42 +0000
@@ -46,6 +46,7 @@
 from lp.registry.interfaces.ssh import ISSHKeySet
 from lp.registry.interfaces.teammembership import TeamMembershipStatus
 from lp.services.gpg.interfaces import (
+    get_gpg_path,
     GPGKeyAlgorithm,
     IGPGHandler,
     )
@@ -138,7 +139,7 @@
     # Prevent translated gpg output from messing up our parsing.
     env['LC_ALL'] = 'C'
 
-    command_line = ['gpg'] + arguments
+    command_line = [get_gpg_path()] + arguments
     pipe = subprocess.Popen(
         command_line, env=env, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
     stdout, stderr = pipe.communicate()