isislovecruft / python-gnupg

A modified version of python-gnupg, including security patches, extensive documentation, and extra features.
Other
424 stars 172 forks source link

Signature status is None during "key considered" status, causing TypeError exception #224

Closed ghost closed 4 years ago

ghost commented 6 years ago

I'd like to apologize in advance if this is a duplicate issue, I searched open/closed issues & PRs, but didn't find anything directly addressing this.

I get a TypeError when verifying signed data. I've checked and the signed data is valid as far as I can tell. I already have a properly generated key pair in ./pgp/ (with no password)

Environment:

>>> import gnupg
>>> gpg = gnupg.GPG(homedir='./pgp/')
>>> sign = gpg.sign('test')
>>> verify = gpg.verify(sign.data)
Exception in thread Thread-5:
Traceback (most recent call last):
  File "/usr/lib/python3.6/threading.py", line 916, in _bootstrap_inner
    self.run()
  File "/usr/lib/python3.6/threading.py", line 864, in run
    self._target(*self._args, **self._kwargs)
  File "/usr/local/lib/python3.6/dist-packages/gnupg/_meta.py", line 650, in _read_response
    result._handle_status(keyword, value)
  File "/usr/local/lib/python3.6/dist-packages/gnupg/_parsers.py", line 1504, in _handle_status
    self.status = '\n'.join([self.status, "key considered"])
TypeError: sequence item 0: expected str instance, NoneType found

>>> verify.valid
False

Simply checking if the status is None first seems to fix it. (see patch linked below) verify.valid returns True with my patch (& of course False when the sig is invalid).

I will create a pull request if everything looks good.

key-considered-patch.txt

ghost commented 6 years ago

@isislovecruft Any thoughts on this?

d9pouces commented 6 years ago

maybe:

  diff --git a/gnupg/_parsers.py b/gnupg/_parsers.py
  index 7dd1ed7..69377fa 100644
  --- a/gnupg/_parsers.py
  +++ b/gnupg/_parsers.py
  @@ -1501,7 +1501,8 @@ class Verify(object):
               ):
               pass
           elif key == "KEY_CONSIDERED":
  -            self.status = '\n'.join([self.status, "key considered"])
  +            if self.status is None:
  +                self.status = 'key considered'
  +            else:
  +                self.status = '\n'.join([self.status, "key considered"])
           elif key == "NEWSIG":
               # Reset
               self.status = None
ghost commented 6 years ago

To anyone else having this issue, it's solved correctly by this PR: https://github.com/isislovecruft/python-gnupg/pull/220/commits/9023a6ecc23c8d96c82d4a7d5af6b0c6bbfe8d28

crashvb commented 3 years ago

Hello,

Thank you for working to create / maintain a great module :)

This bug does not appear to be fixed as of v3.1.1; parsing still fails for this error when gpg produces output similar to the following:

[GNUPG:] NEWSIG
gpg: Signature made Mon Mar 23 12:36:40 2020 UTC
gpg:                using RSA key A328C2DC7801F08F
[GNUPG:] KEY_CONSIDERED 1111A943E5D571AC145949A67B4D120726C8BA41 0
[GNUPG:] SIG_ID YnRIfkpi48w7qQmm9smnYhcvEWA 2020-03-23 1584967000
[GNUPG:] KEY_CONSIDERED 1111A943E5D571AC145949A67B4D120726C8BA41 0
[GNUPG:] GOODSIG A328C2DC7801FF9E First Last <First.Last@domain.com>
gpg: Good signature from "First Last <First.Last@domain.com>" [ultimate]
[GNUPG:] VALIDSIG 1111E6B9F947C2F2BA3820B1A328C2DC7801F082 2020-03-23 1584967000 0 4 0 1 10 00 1111A943E5D571AC145949A67B4D120726C8BA41
[GNUPG:] KEY_CONSIDERED 1111A943E5D571AC145949A67B4D120726C8BA41 0
[GNUPG:] TRUST_ULTIMATE 0 pgp
[GNUPG:] VERIFICATION_COMPLIANCE_MODE 23

It does not appear that the handing of KEY_CONSIDERED is correct when it appears in sequence before self.status has been assigned.

Should this bug be re-opened, or a new one created?

Myzel394 commented 1 year ago

I'm also facing this issue. Seems like, the commits didn't change anything :( I also can't simply sign and verify a message. Is there a workaround available? @isislovecruft or @crashvb ?

crashvb commented 1 year ago

@Myzel394 I duck punched it.

This one just one of several issues I found with this library in general. Once you factor in the GnuPG v1 vs v2 compatibility issues (i.e. needing an agent in v2 for pin entry), and the lack of standard parsing practices which @isislovecruft left many comments towards, and the cross-distro issues (such as RHEL rolling their own variants), it's simply too much to try and quantify in a consolidated library.

Personally, I consider this library to be dead in need of new maintainers. For other projects I reverted to invoking the CLI directly using subprocess (the pitfalls include having to configure and manage multiple child processes); also because I can't test this library actually works while also using this library. In the end it was faster, and easier to understand and maintain going forward.

Side Note: I did dive into the source code for GnuPG itself, and GnuPGME library as well looking to hopefully expose it through python, and learned a lot about the challenges these developers did, and would have to continue, to overcome to bring it up to spec with other similar project (pyopenssl, entrust, etc). I don't blame them for reevaluating their participation.