Launchpad itself

Merge lp:~cjwatson/launchpad/isolate-gpgme into lp:launchpad

  1. isolate-gpgme
  2. Merge into devel
Proposed by Colin Watson
Status: Rejected
Rejected by: Colin Watson
Proposed branch: lp:~cjwatson/launchpad/isolate-gpgme
Merge into: lp:launchpad
Diff against target: 197 lines (+60/-17)
2 files modified
lib/lp/services/gpg/handler.py (+54/-14)
lib/lp/testing/gpgkeys/__init__.py (+6/-3)
To merge this branch: bzr merge lp:~cjwatson/launchpad/isolate-gpgme
Reviewer Review Type Date Requested Status
Launchpad code reviewers Pending
Review via email: mp+310906@code.launchpad.net

Commit message

Isolate gpgme from its terminal environment, if any.

Description of the change

I've been having problems for ages with spurious gpgme-related test failures in my local environment that went away when I piped the output through cat, and this was particularly annoying any time I needed to apply pdb to one of them. This isolates things so that that isn't a problem any more.

To post a comment you must log in.
Revision history for this message
William Grant (wgrant) wrote :

This seems like a pretty big hammer to use outside the test suite. The appservers are single-threaded today, but it still feels quite off. Have you examined whether we can force a null pinentry with gpg1? IIRC from my gpg2 work it's certainly possible there -- maybe it's worth just living with the problem until we're on xenial.

Revision history for this message
Colin Watson (cjwatson) wrote :

I'm withdrawing this at least for now, as it doesn't appear to be a problem in xenial test runs.

Unmerged revisions

18270. By Colin Watson

Isolate gpgme from its terminal environment, if any.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/services/gpg/handler.py'
--- lib/lp/services/gpg/handler.py 2016-11-03 15:07:36 +0000
+++ lib/lp/services/gpg/handler.py 2016-11-15 17:42:52 +0000
@@ -5,12 +5,14 @@
55
6__all__ = [6__all__ = [
7 'GPGHandler',7 'GPGHandler',
8 'isolate_gpgme',
8 'PymeKey',9 'PymeKey',
9 'PymeSignature',10 'PymeSignature',
10 'PymeUserId',11 'PymeUserId',
11 ]12 ]
1213
13import atexit14import atexit
15from contextlib import contextmanager
14import httplib16import httplib
15import os17import os
16import shutil18import shutil
@@ -65,6 +67,34 @@
65"""67"""
6668
6769
70@contextmanager
71def isolate_gpgme():
72 """Isolate gpgme from its terminal environment, if any.
73
74 Ensure that stdout is not a tty. gpgme tries to enable interactive
75 behaviour if this is the case. We never want that, and it can cause
76 test failures in some environments, so make sure it's not the case while
77 calling into gpgme.
78
79 Make sure that gpgme does not have a DISPLAY.
80 """
81 if os.isatty(1):
82 old_stdout = os.dup(1)
83 devnull = os.open(os.devnull, os.O_WRONLY)
84 os.dup2(devnull, 1)
85 else:
86 old_stdout = None
87 old_display = os.environ.pop('DISPLAY', None)
88 try:
89 yield
90 finally:
91 if old_display is not None:
92 os.environ['DISPLAY'] = old_display
93 if old_stdout is not None:
94 os.dup2(old_stdout, 1)
95 os.close(old_stdout)
96
97
68@implementer(IGPGHandler)98@implementer(IGPGHandler)
69class GPGHandler:99class GPGHandler:
70 """See IGPGHandler."""100 """See IGPGHandler."""
@@ -178,7 +208,8 @@
178208
179 # process it209 # process it
180 try:210 try:
181 signatures = ctx.verify(*args)211 with isolate_gpgme():
212 signatures = ctx.verify(*args)
182 except gpgme.GpgmeError as e:213 except gpgme.GpgmeError as e:
183 error = GPGVerificationError(e.strerror)214 error = GPGVerificationError(e.strerror)
184 for attr in ("args", "code", "signatures", "source"):215 for attr in ("args", "code", "signatures", "source"):
@@ -230,7 +261,8 @@
230 context.armor = True261 context.armor = True
231262
232 newkey = StringIO(content)263 newkey = StringIO(content)
233 result = context.import_(newkey)264 with isolate_gpgme():
265 result = context.import_(newkey)
234266
235 if len(result.imports) == 0:267 if len(result.imports) == 0:
236 raise GPGKeyNotFoundError(content)268 raise GPGKeyNotFoundError(content)
@@ -264,7 +296,8 @@
264 context = gpgme.Context()296 context = gpgme.Context()
265 context.armor = True297 context.armor = True
266 newkey = StringIO(content)298 newkey = StringIO(content)
267 import_result = context.import_(newkey)299 with isolate_gpgme():
300 import_result = context.import_(newkey)
268301
269 secret_imports = [302 secret_imports = [
270 fingerprint303 fingerprint
@@ -277,7 +310,8 @@
277310
278 fingerprint, result, status = import_result.imports[0]311 fingerprint, result, status = import_result.imports[0]
279 try:312 try:
280 key = context.get_key(fingerprint, True)313 with isolate_gpgme():
314 key = context.get_key(fingerprint, True)
281 except gpgme.GpgmeError:315 except gpgme.GpgmeError:
282 return None316 return None
283317
@@ -296,8 +330,9 @@
296 # Only 'utf-8' encoding is supported by gpgme.330 # Only 'utf-8' encoding is supported by gpgme.
297 # See more information at:331 # See more information at:
298 # http://pyme.sourceforge.net/doc/gpgme/Generating-Keys.html332 # http://pyme.sourceforge.net/doc/gpgme/Generating-Keys.html
299 result = context.genkey(333 with isolate_gpgme():
300 signing_only_param % {'name': name.encode('utf-8')})334 result = context.genkey(
335 signing_only_param % {'name': name.encode('utf-8')})
301336
302 # Right, it might seem paranoid to have this many assertions,337 # Right, it might seem paranoid to have this many assertions,
303 # but we have to take key generation very seriously.338 # but we have to take key generation very seriously.
@@ -340,9 +375,10 @@
340 % key.fingerprint)375 % key.fingerprint)
341376
342 # encrypt content377 # encrypt content
343 ctx.encrypt(378 with isolate_gpgme():
344 [removeSecurityProxy(key.key)], gpgme.ENCRYPT_ALWAYS_TRUST,379 ctx.encrypt(
345 plain, cipher)380 [removeSecurityProxy(key.key)], gpgme.ENCRYPT_ALWAYS_TRUST,
381 plain, cipher)
346382
347 return cipher.getvalue()383 return cipher.getvalue()
348384
@@ -375,7 +411,8 @@
375411
376 # Sign the text.412 # Sign the text.
377 try:413 try:
378 context.sign(plaintext, signature, mode)414 with isolate_gpgme():
415 context.sign(plaintext, signature, mode)
379 except gpgme.GpgmeError:416 except gpgme.GpgmeError:
380 return None417 return None
381418
@@ -393,8 +430,9 @@
393 if type(filter) == unicode:430 if type(filter) == unicode:
394 filter = filter.encode('utf-8')431 filter = filter.encode('utf-8')
395432
396 for key in ctx.keylist(filter, secret):433 with isolate_gpgme():
397 yield PymeKey.newFromGpgmeKey(key)434 for key in ctx.keylist(filter, secret):
435 yield PymeKey.newFromGpgmeKey(key)
398436
399 def retrieveKey(self, fingerprint):437 def retrieveKey(self, fingerprint):
400 """See IGPGHandler."""438 """See IGPGHandler."""
@@ -549,7 +587,8 @@
549 context = gpgme.Context()587 context = gpgme.Context()
550 # retrive additional key information588 # retrive additional key information
551 try:589 try:
552 key = context.get_key(fingerprint, False)590 with isolate_gpgme():
591 key = context.get_key(fingerprint, False)
553 except gpgme.GpgmeError:592 except gpgme.GpgmeError:
554 key = None593 key = None
555594
@@ -597,7 +636,8 @@
597 context = gpgme.Context()636 context = gpgme.Context()
598 context.armor = True637 context.armor = True
599 keydata = StringIO()638 keydata = StringIO()
600 context.export(self.fingerprint.encode('ascii'), keydata)639 with isolate_gpgme():
640 context.export(self.fingerprint.encode('ascii'), keydata)
601641
602 return keydata.getvalue()642 return keydata.getvalue()
603643
604644
=== modified file 'lib/lp/testing/gpgkeys/__init__.py'
--- lib/lp/testing/gpgkeys/__init__.py 2016-11-03 15:07:36 +0000
+++ lib/lp/testing/gpgkeys/__init__.py 2016-11-15 17:42:52 +0000
@@ -1,4 +1,4 @@
1# Copyright 2009 Canonical Ltd. This software is licensed under the1# Copyright 2009-2016 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).2# GNU Affero General Public License version 3 (see the file LICENSE).
33
4"""OpenPGP keys used for testing.4"""OpenPGP keys used for testing.
@@ -27,11 +27,13 @@
2727
28from lp.registry.interfaces.gpg import IGPGKeySet28from lp.registry.interfaces.gpg import IGPGKeySet
29from lp.registry.interfaces.person import IPersonSet29from lp.registry.interfaces.person import IPersonSet
30from lp.services.gpg.handler import isolate_gpgme
30from lp.services.gpg.interfaces import (31from lp.services.gpg.interfaces import (
31 GPGKeyAlgorithm,32 GPGKeyAlgorithm,
32 IGPGHandler,33 IGPGHandler,
33 )34 )
3435
36
35gpgkeysdir = os.path.join(os.path.dirname(__file__), 'data')37gpgkeysdir = os.path.join(os.path.dirname(__file__), 'data')
3638
3739
@@ -141,9 +143,10 @@
141143
142 ctx.passphrase_cb = passphrase_cb144 ctx.passphrase_cb = passphrase_cb
143145
144 # Do the deecryption.146 # Do the decryption.
145 try:147 try:
146 ctx.decrypt(cipher, plain)148 with isolate_gpgme():
149 ctx.decrypt(cipher, plain)
147 except gpgme.GpgmeError:150 except gpgme.GpgmeError:
148 return None151 return None
149152