SecurityInnovation / PGPy

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

Handle key revocation certificates. #202

Open J08nY opened 7 years ago

J08nY commented 7 years ago

Let's say that I have the following example public key:

pub_key = """\
-----BEGIN PGP PUBLIC KEY BLOCK-----

mI0EWUpapAEEANjx0Ii5vrHi1wVLdKZZbWvly3CV0WLGQvPwukF2Clz7O3uJZkhe
9VYf0ZiBPOAT6oQBUly9AMbv9xfrNOimSQYRSafAGAQWlzteLQP78ZgLbdeV5CQg
X9HYx/QBbcUHH9TErSQiRh9N6WS5kGtqDzeIuHfIpeRwubVPTXR4ZtNZABEBAAG0
KVJTQSAxMDI0YiBleGFtcGxlIDxSU0EtMTAyNGJAZXhhbXBsZS5vcmc+iM4EEwEI
ADgWIQTUqUiGscoglqZFjlxH8QwIeIS3WAUCWUpapAIbAwULCQgHAgYVCAkKCwIE
FgIDAQIeAQIXgAAKCRBH8QwIeIS3WG0CBACwcCVMZARiWJT+ZDmWAIEB8Ovq8CIH
VwKD5JXcHBj4b5QRKHEWMfuSlGK8e1t+dnb/gIMUmefdCS+yRr1iQ7zMaGRgiYkb
ye/lXpbikR9eu8E/B+IcX7QSas6K01OHsj8xvGxZ3rmoOXcPvNQ3Z4F41kQqKxBx
HWkI3LH71RJp4biNBFlKWqQBBADXLjeg+K0ZAuchhNkvMnfb8TlKoc1t1lH94uno
qoj3L5Srf3XSo1fmuEBNr1nc5BLyH0tPDJZyuVCypfY4en9uaGvWO0U4t/OUiIWq
PjBmXbjZWDdGKksb0cQNy13cvE57EmirbuVcXCE0OdU8lcbopN2wnecKOxLKK9Pa
LBZ69wARAQABiLYEGAEIACAWIQTUqUiGscoglqZFjlxH8QwIeIS3WAUCWUpapAIb
DAAKCRBH8QwIeIS3WA7BA/sG7Ifagq5Y8BdxWsl2Xc/d8iq+bgnMpGNcJUD2PlA5
2KEfVbmtkULHcn5tS8prVAnU24ehT2Bi9HY0+z4KkyVMnbCDuSyC112UL2WxWLWK
O+MkK6C94GFr8JAm3sKXq4OSJ8+wPmTHJRN5yadV9hkHoVv+etnEnkJvV0o8aLEO
nA==
=4olM
-----END PGP PUBLIC KEY BLOCK-----
"""

For which, gpg generated this key revocation:

revoc_sig = """\
-----BEGIN PGP PUBLIC KEY BLOCK-----
Comment: This is a revocation certificate

iLYEIAEIACAWIQTUqUiGscoglqZFjlxH8QwIeIS3WAUCWYB5cwIdAwAKCRBH8QwI
eIS3WBk7A/oCv4Rq/CN7zk3zMA1lE5CC0WAFecHPhvBnDWG4xkxL5shvoBaEV72s
BI+XAP6zdz08i+JxpKzu/jSCD1Dc0jmyNbrv7DqI2kzO9KQQSP6HLwECrAy1gmiT
7sRpNwNlfPGhUwLLJpA9xLtFBMfY9ldxaM0SHQaEBqWTXKLDUXYGMQ==
=+x1V
-----END PGP PUBLIC KEY BLOCK-----
"""

PGPy should be able to parse and verify this revocation signature. As currently doing:

s = PGPSignature.from_blob(revoc_sig)
Traceback (most recent call last):
  File "<stdin>", line 11, in <module>
  File "/home/johny/dev/PGPy/pgpy/types.py", line 202, in from_blob
    po = obj.parse(bytearray(blob, 'latin-1'))
  File "/home/johny/dev/PGPy/pgpy/pgp.py", line 521, in parse
    raise ValueError('Expected: SIGNATURE. Got: {}'.format(str(unarmored['magic'])))
ValueError: Expected: SIGNATURE. Got: PUBLIC KEY BLOCK

m = PGPMessage.from_blob(revoc_sig)
Traceback (most recent call last):
  File "<stdin>", line 11, in <module>
  File "/home/johny/dev/PGPy/pgpy/types.py", line 202, in from_blob
    po = obj.parse(bytearray(blob, 'latin-1'))
  File "/home/johny/dev/PGPy/pgpy/pgp.py", line 2464, in parse
    for group in iter(group for _, group in itertools.groupby(getpkt, key=pktgrouper()) if not _.endswith('Opaque')):
  File "/home/johny/dev/PGPy/pgpy/pgp.py", line 2464, in <genexpr>
    for group in iter(group for _, group in itertools.groupby(getpkt, key=pktgrouper()) if not _.endswith('Opaque')):
AttributeError: 'NoneType' object has no attribute 'endswith'

doesn't work.

J08nY commented 7 years ago

What gpg generates as a PGP PUBLIC KEY BLOCK is not really a valid transferable public key, so it makes sense that PGPKey.from_blob errors out. However there should be a way to get at least the signature packet out of an ASCII-Armored blob such as that one.

>>> dearm = Armorable.ascii_unarmor(revoc_sig)
>>> data = bytearray(dearm['body'])
>>> p = Packet(data)
>>> p
<SignatureV4 [tag 02][v4] at 0x....>
>>> sig = PGPSignature()
>>> sig |= p
>>> sig
<PGPSignature [KeyRevocation] object at 0x....>
Commod0re commented 7 years ago

Interesting that it's wrapped in magic claiming it to be a PUBLIC KEY BLOCK when the only packet contained within is a signature.

A slightly less obnoxious short-term workaround, if you already know ahead of time that it's going to be a revocation signature, could be:

s = PGPSignature.from_blob(revoc_sig.replace('PUBLIC KEY BLOCK', 'SIGNATURE'))

I guess we'll have to take the armor magic as a suggestion in the future 8)

J08nY commented 7 years ago

Nice workaround! Definitely using that. 👍

Might be also worthwhile to report this as a bug to GPG, as what they are generating under the PUBLIC KEY BLOCK magic is definitely not a valid transferable public key and should maybe change that to put it in SIGNATURE magic. Although the PUBLIC KEY BLOCK magic is understandable as key revocation sigs are part of keys when they are imported, so I guess that is their logic.