SecurityInnovation / PGPy

Pretty Good Privacy for Python
BSD 3-Clause "New" or "Revised" License
313 stars 98 forks source link

Can't access older signatures: `AttributeError: 'NoneType' object has no attribute 'signer'` #433

Open woodruffw opened 1 year ago

woodruffw commented 1 year ago

Hi there! Thanks for maintaining this library.

I'm doing a bit of software spelunking/archaeology, and I'd like to be able to parse old PGP signatures and access their signer information (specifically, Key IDs).

For example, a signature like this:

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)

iD8DBQBLQcxSAfqZj7rGN0oRAsPWAJ9RA18LvpgZgW/8duIfhTCQNJ7nAQCglMJA
HFbko58OnywVkhewv77NVqo=
=Uy85
-----END PGP SIGNATURE-----

When I try to load that with PGPSignature.from_blob(...), I don't get an exception:

>>> import pgpy
>>> s = pgpy.PGPSignature.from_blob(sig)

However, as soon as I try to access s (even just its repr), I get an AttributeError:

>>> s
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/william/personal/devel/pypi-pgp-statistics/env/lib/python3.11/site-packages/pgpy/pgp.py", line 362, in __repr__
    return "<PGPSignature [{:s}] object at 0x{:02x}>".format(self.type.name, id(self))
                                                             ^^^^^^^^^
  File "/Users/william/personal/devel/pypi-pgp-statistics/env/lib/python3.11/site-packages/pgpy/pgp.py", line 322, in type
    return self._signature.sigtype
           ^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'sigtype'

I did a quick check, and it looks like this is coming from the following packet check:

https://github.com/SecurityInnovation/PGPy/blob/30a757181ab02f918a94f8549f354d93639b95e6/pgpy/pgp.py#L585-L592

This looks wrong to me: rather than pass-ing, this should probably either raise an exception or (longer-term) handler whatever older/legacy packet type appears in these older signatures. That way, the PGPSignature object itself is never constructed in an invalid/inaccessible state.

I'm happy to make the changes for the quick fix (raising an exception); the more detailed fix should probably be done by someone who understands PGP better 🙂