SecurityInnovation / PGPy

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

Ignore unsupported preferred hash algorithms #322

Open vollkorn1982 opened 4 years ago

vollkorn1982 commented 4 years ago

I receive the below error when importing a key, that has "pref-hash-algos" set to "101".

[...]
# off=892 ctb=89 tag=2 hlen=3 plen=323
:signature packet: algo 1, keyid [redacted]
    version 4, created 1371144648, md5len 0, sigclass 0x13
    digest algo 2, begin of digest 28 a0
    hashed subpkt 2 len 4 (sig created 2013-06-13)
    hashed subpkt 27 len 1 (key flags: 23)
    hashed subpkt 11 len 11 (pref-sym-algos: 3 10 7 8 9 2 4 1 11 12 13)
    hashed subpkt 21 len 9 (pref-hash-algos: 3 8 9 10 11 101 6 2 1)
    hashed subpkt 22 len 4 (pref-zip-algos: 2 1 3 0)
    hashed subpkt 30 len 1 (features: 01)
    hashed subpkt 23 len 1 (keyserver preferences: 80)
    subpkt 16 len 8 (issuer key ID [redacted])
    data: [2047 bits]
[...]

According to the RFC 100 through 110 are reserved for "Private/Experimental algorithm[s]". It is highly unlikely that PGPy will ever be able to support any of these due to the very nature of this definition.

Therefore, I propose to change the behavior such that IDs 100 through 110 are simply ignored, because they are valid, but impossible to support.

Full error message

Traceback (most recent call last):
  File "/home/jan/Projekte/pgpy/venv/lib/python3.6/site-packages/pgpy/types.py", line 556, in __call__
    obj.parse(packet)
  File "/home/jan/Projekte/pgpy/venv/lib/python3.6/site-packages/pgpy/packet/subpackets/signature.py", line 129, in parse
    self.flags = packet[:1]
  File "/home/jan/Projekte/pgpy/venv/lib/python3.6/site-packages/pgpy/decorators.py", line 48, in wrapper
    return sd.dispatch(args[0].__class__)(obj, *args, **kwargs)
  File "/home/jan/Projekte/pgpy/venv/lib/python3.6/site-packages/pgpy/packet/subpackets/signature.py", line 115, in flags_bytearray
    self.flags = self.bytes_to_int(val)
  File "/home/jan/Projekte/pgpy/venv/lib/python3.6/site-packages/pgpy/decorators.py", line 48, in wrapper
    return sd.dispatch(args[0].__class__)(obj, *args, **kwargs)
  File "/home/jan/Projekte/pgpy/venv/lib/python3.6/site-packages/pgpy/packet/subpackets/signature.py", line 111, in flags_int
    self._flags.append(self.__flags__(val))
  File "/usr/lib/python3.6/enum.py", line 293, in __call__
    return cls.__new__(cls, value)
  File "/usr/lib/python3.6/enum.py", line 535, in __new__
    return cls._missing_(value)
  File "/usr/lib/python3.6/enum.py", line 548, in _missing_
    raise ValueError("%r is not a valid %s" % (value, cls.__name__))
ValueError: 101 is not a valid HashAlgorithm

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/home/jan/Projekte/pgpy/venv/lib/python3.6/site-packages/pgpy/types.py", line 556, in __call__
    obj.parse(packet)
  File "/home/jan/Projekte/pgpy/venv/lib/python3.6/site-packages/pgpy/packet/packets.py", line 446, in parse
    self.subpackets.parse(packet)
  File "/home/jan/Projekte/pgpy/venv/lib/python3.6/site-packages/pgpy/packet/fields.py", line 206, in parse
    sp = SignatureSP(packet)
  File "/home/jan/Projekte/pgpy/venv/lib/python3.6/site-packages/pgpy/types.py", line 559, in __call__
    six.raise_from(PGPError(str(ex)), ex)
  File "<string>", line 3, in raise_from
pgpy.errors.PGPError: 101 is not a valid HashAlgorithm

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "./import_key.py", line 6, in <module>
    pgpy.PGPKey.from_file(sys.argv[1])
  File "/home/jan/Projekte/pgpy/venv/lib/python3.6/site-packages/pgpy/types.py", line 189, in from_file
    po = obj.parse(data)
  File "/home/jan/Projekte/pgpy/venv/lib/python3.6/site-packages/pgpy/pgp.py", line 2363, in parse
    [ operator.ior(pgpobj, PGPSignature() | sig) for sig in group if not isinstance(sig, Opaque) ]
  File "/home/jan/Projekte/pgpy/venv/lib/python3.6/site-packages/pgpy/pgp.py", line 2363, in <listcomp>
    [ operator.ior(pgpobj, PGPSignature() | sig) for sig in group if not isinstance(sig, Opaque) ]
  File "/home/jan/Projekte/pgpy/venv/lib/python3.6/site-packages/pgpy/pgp.py", line 2333, in <lambda>
    _getpkt = lambda d: (Packet(d) if d else None)  # flake8: noqa
  File "/home/jan/Projekte/pgpy/venv/lib/python3.6/site-packages/pgpy/types.py", line 559, in __call__
    six.raise_from(PGPError(str(ex)), ex)
  File "<string>", line 3, in raise_from
pgpy.errors.PGPError: 101 is not a valid HashAlgorithm
J-M0 commented 4 years ago

I'm trying to figure out what the best behavior should be here. PGPy can't know what the private algorithms are unless it knows what implementation they come from. Since we don't have support for other implementations' private algorithms right now, I don't see the value in being able to import keys with these settings, but I'm open to hearing how it would be helpful.

vollkorn1982 commented 3 years ago

@J-M0 Same story here as in #329. These fields are just blank fields described by the OpenPGP standard. You can use them or just ignore them, but a key having these fields is a valid key and should IMHO be imported. Just as they are with gnupg. Such keys exist and conform to the standard.

Worst case would be a key, that does not support any of the standardized hash algorithms, but a custom one. That's an easy case though: Just ignore the custom ones and handle this key as if it doesn't have any hash algorithms defined.

I think it boils down to the question: Is PGPy meant to deal with the crazy, yet standard-conforming keys out there in "production" or not? If yes, import the custom fields, but ignore them, don't offer any possible way to interact with them and we're fine.

These two issues are my personal blockers for using PGPy instead of gnupg in a couple of other FOSS projects (e.g. https://github.com/byro/byro). If PGPy won't learn to import such keys I will not use PGPy but instead wait for Sequoia's python bindings. I really prefer to use PGPy, though.

J-M0 commented 3 years ago

Ok, I think this has merit then. Could you provide a PGP key that uses one of these private algorithms please? I need to see how PGPy would behave right now with one.

vollkorn1982 commented 3 years ago

The key 0x1B4FF635AAE6C36A is on the public key servers, for example at http://pool.sks-keyservers.net/pks/lookup?op=get&search=0x1B4FF635AAE6C36A

(Yes, it's old and revoked, but it is a key with this feature that's public)