python gpg.Context.decrypt(verify=False) no longer works
Closed, ResolvedPublic

Description

in 11403a46358f9b6e98776974f3c70f211d9adf85, the python gpg bindings appear to have radically changed the semantics of the verify= argument to the gpg.Context.decrypt() function.

In particular:

+            if verify is not None:
+                if isinstance(verify, bool) is True:
+                    if verify is False:
+                        verify = True
+                        sink_result = True
+                    else:
+                        pass

This appears to change verify from False to True, which means that if there is no signature, an error will be raised anyway.

See https://bugs.debian.org/911568 as one example of a problem introduced by this change:

  File "/usr/lib/python3/dist-packages/impass/db.py", line 112, in _decryptDB
    data, _, _ = self._gpg.decrypt(try2, verify=False)
  File "/usr/lib/python3/dist-packages/gpg/core.py", line 431, in decrypt
    for s in verify_result.signatures):
AttributeError: 'NoneType' object has no attribute 'signatures'
dkg created this task.Tue, Nov 27, 10:10 PM
dkg added a comment.Tue, Nov 27, 10:14 PM

please add a unit to the test suite to make sure something like this doesn't happen in the future!

i believe i heard from some alot users that they were seeing crashes on encrypted but unsigned messages as well, though i don't have a pointer (they're using python-gpg too)

werner triaged this task as Unbreak Now! priority.Wed, Nov 28, 9:30 AM
werner added a subscriber: werner.

Regression introduced with 1.12.0.

dkg added a comment.Wed, Dec 5, 12:51 AM

I've just pushed a branch dkg/fix-T4271 , currently at ac8d7238dbf165950c9844e5cb41da8eb4d37bc0 that resolves this problem.

dkg added a comment.Wed, Dec 5, 6:34 AM

note that the branch also updates the test suite to make sure the verify=False case is tested.

Needs to be merged. (Note that Phabricator does not show the branch in the tooltip for commit ids.)

aheinecke changed the task status from Open to Testing.Wed, Dec 5, 11:49 AM
aheinecke assigned this task to BenM.
aheinecke added subscribers: BenM, aheinecke.

Ben is not even subscribed to this issue.
With the volatility of gpgme-python I think that this can easily be merged. I did a quick review and it looked good to me.

@BenM do you object?

dkg added a comment.Wed, Dec 5, 2:06 PM

since @aheinecke merged my changes, i think this bug is now resolved. I'll let @BenM close it though :)

BenM added a comment.Wed, Dec 5, 4:14 PM

Ooh, nice catch @dkg, I just stepped through each of your changes and it all looks good. I'll tweak the relevant sections of the HOWTO dealing with this in the next few days (I need to replace a keyboard here before properly diving back in) and then close this case once done.

BenM closed this task as Resolved.Mon, Dec 10, 6:08 AM

Confirmed that this is indeed fixed and made the (rather minor) change to the HOWTO that was needed. No changes were needed for the example script (decrypt-file.py).