From 6df8c3afb2ec84a34b127bff53523d914ca01d76 Mon Sep 17 00:00:00 2001 From: Daniel Kahn Gillmor Date: Fri, 2 Mar 2018 09:38:13 +0100 Subject: [PATCH 01/11] rfc4880bis identifies public key algorithm 22 as EdDSA --- pgpdump/packet.py | 1 + 1 file changed, 1 insertion(+) diff --git a/pgpdump/packet.py b/pgpdump/packet.py index cf83e4e..f8a6f49 100644 --- a/pgpdump/packet.py +++ b/pgpdump/packet.py @@ -46,6 +46,7 @@ class AlgoLookup(object): 19: "ECDSA", 20: "Formerly ElGamal Encrypt or Sign", 21: "Diffie-Hellman", + 22: "EdDSA", } @classmethod From 9afac97b6b55215926bcba5ea98ffb03b819d1c4 Mon Sep 17 00:00:00 2001 From: Daniel Kahn Gillmor Date: Fri, 2 Mar 2018 09:41:06 +0100 Subject: [PATCH 02/11] pass on unknown pubkey algorithms rather than raising an Exception If we're OK with "passing" on experimental algorithms, we should be ok with passing on known algorithms that we know the name of but don't implement. --- pgpdump/packet.py | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/pgpdump/packet.py b/pgpdump/packet.py index f8a6f49..58e79cc 100644 --- a/pgpdump/packet.py +++ b/pgpdump/packet.py @@ -421,12 +421,9 @@ def parse_key_material(self, offset): self.prime, offset = get_mpi(self.data, offset) self.group_gen, offset = get_mpi(self.data, offset) self.key_value, offset = get_mpi(self.data, offset) - elif 100 <= self.raw_pub_algorithm <= 110: - # Private/Experimental algorithms, just move on - pass else: - raise PgpdumpException("Unsupported public key algorithm %d" % - self.raw_pub_algorithm) + # If we don't know how to handle the algorithm, just move on + pass return offset @@ -610,12 +607,9 @@ def parse_private_key_material(self, offset): self.pub_algorithm_type = "elg" # x self.exponent_x, offset = get_mpi(self.data, offset) - elif 100 <= self.raw_pub_algorithm <= 110: - # Private/Experimental algorithms, just move on - pass else: - raise PgpdumpException("Unsupported public key algorithm %d" % - self.raw_pub_algorithm) + # If we don't know how to handle the algorithm, just move on + pass return offset From 7cc89da7d44ecfd85cb9a33e503c387e081dd687 Mon Sep 17 00:00:00 2001 From: Daniel Kahn Gillmor Date: Fri, 2 Mar 2018 09:42:36 +0100 Subject: [PATCH 03/11] Correct copy/pasted error message --- pgpdump/packet.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pgpdump/packet.py b/pgpdump/packet.py index 58e79cc..bd6c363 100644 --- a/pgpdump/packet.py +++ b/pgpdump/packet.py @@ -576,7 +576,7 @@ def parse(self): "Unsupported GnuPG S2K extension, encountered mode %d" % mode) else: raise PgpdumpException( - "Unsupported public key algorithm %d" % s2k_type_id) + "Unsupported S2K algorithm %d" % s2k_type_id) if s2k_length != (offset - offset_before_s2k): raise PgpdumpException( From dbc1539d9f42ad61e6b982f1e9d17bab20ad5cba Mon Sep 17 00:00:00 2001 From: Daniel Kahn Gillmor Date: Tue, 8 Jan 2019 15:15:39 -0500 Subject: [PATCH 04/11] rfc4880bis includes embedded issuer fingerprint The issuer fingerprint effectively supercedes issuer keyID as a more robust signal of the issuer (though it's possible and in some cases likely to include both in a signature, to ensure that it's understood by old and new code). --- pgpdump/packet.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pgpdump/packet.py b/pgpdump/packet.py index bd6c363..f29c5cd 100644 --- a/pgpdump/packet.py +++ b/pgpdump/packet.py @@ -144,6 +144,7 @@ def __init__(self, raw, hashed, data): 30: "Features", 31: "Signature Target", 32: "Embedded Signature", + 33: "Issuer Fingerprint", } @property @@ -173,6 +174,8 @@ def __init__(self, *args, **kwargs): self.raw_expiration_time = None self.expiration_time = None self.key_id = None + self.fingerprint = None + self.fingerprint_version = None self.hash2 = None self.subpackets = [] super(SignaturePacket, self).__init__(*args, **kwargs) @@ -276,6 +279,9 @@ def parse_subpackets(self, outer_offset, outer_length, hashed=False): self.raw_expiration_time = get_int4(subpacket.data, 0) elif subpacket.subtype == 16: self.key_id = get_key_id(subpacket.data, 0) + elif subpacket.subtype == 33: + self.fingerprint_version = int(subpacket.data[0]) + self.fingerprint = get_hex_data(subpacket.data, 1, len(subpacket.data)) offset += sub_len self.subpackets.append(subpacket) From 25cc2738b872eb3cf48acdb193a7a21cf2478679 Mon Sep 17 00:00:00 2001 From: Daniel Kahn Gillmor Date: Thu, 12 Sep 2019 02:14:05 -0400 Subject: [PATCH 05/11] Raise PgpdumpException when MPI parsing fails Some malformed MPIs will cause a failure in packet parsing; this is an OpenPGP formatting error, so we should raise it explicitly. Below is an example packet (found on the SKS keyserver network) that has a malformed MPI. -----BEGIN PGP PUBLIC KEY BLOCK----- mQGiBEhFwD0RBAC8mLriGM4hMjV4ePeBD+FoqB/ENCXbLgl1vAvB3ha/HxqVeH1o w8HdYiSHykreflEqH31fxJv1xFgQ87sf21jv/8TeR0LeXnx7eEIF+uhDHWiZIr4q +EfrzIkpC9+yyo5XaveqNqiSOcnBH2aq334L80/2LR++7J2ffZHgWVfKDtsofE6L XfmOPAwT7R06+po2VnFjlwsD/jXgG6rkjwJZn/VhibcifIms66Fp/RYCXZp9mCeS pADN7i+7VwwCjaME3ndQu/KMdMqSzfHsWJFJbycjN5MK49vxXZyz+cGblcKmZH3V CKwC2iVVRP5FJe+ZLaqmEt/qbyjstXimWUYaJkQkD3lPz+7NfVhIXa0tlE2UcL+7 A5ghA/9+GZu0eFtV7OokUuyPHcfQDQWLh+nuJP7bVSepo9Gy4c0SCYyKlHf+e+A3 qIF76+1Gj7IvVEY8fcC0APzGad9Y0WELSTNcZ4+UqW3Ezag1k7Yq5F0VycZ2LTIC 9nF3UsjKgUxWehbkrNz9NRx9v7ktxS0f2N43dEQIffab3rPqMw== =V948 -----END PGP PUBLIC KEY BLOCK----- Signed-off-by: Daniel Kahn Gillmor fix up mpi error Signed-off-by: Daniel Kahn Gillmor --- pgpdump/utils.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pgpdump/utils.py b/pgpdump/utils.py index 960d183..fa209ca 100644 --- a/pgpdump/utils.py +++ b/pgpdump/utils.py @@ -92,6 +92,8 @@ def get_mpi(data, offset): mpi_len = get_int2(data, offset) offset += 2 to_process = (mpi_len + 7) // 8 + if to_process > (len(data) - offset): + raise PgpdumpException("MPI wants %s octets, but buffer has only %s left"%(to_process, len(data) - offset)) mpi = 0 i = -4 for i in range(0, to_process - 3, 4): From 79574abe5b5865964ae93031319e6dc93a102556 Mon Sep 17 00:00:00 2001 From: Daniel Kahn Gillmor Date: Thu, 12 Sep 2019 04:09:47 -0400 Subject: [PATCH 06/11] raise a PgpdumpException on malformed User Attribute packets --- pgpdump/packet.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pgpdump/packet.py b/pgpdump/packet.py index f29c5cd..bb4a2c3 100644 --- a/pgpdump/packet.py +++ b/pgpdump/packet.py @@ -669,6 +669,9 @@ def parse(self): sub_offset, sub_len, sub_part = new_tag_length(self.data, offset) # sub_len includes the subtype single byte, knock that off sub_len -= 1 + if offset + sub_offset >= len(self.data): + raise PgpdumpException("Attribute at position %d wants another %d octets, but only %d octets remain"%( + offset, sub_offset, len(self.data) - offset)) # initial length bytes offset += sub_offset From 63e1c4eae2aaa1912c009ce69dfd6367e404484b Mon Sep 17 00:00:00 2001 From: Daniel Kahn Gillmor Date: Thu, 12 Sep 2019 04:08:54 -0400 Subject: [PATCH 07/11] add optional skip= parameter to data.BinaryData.packets() Each malformed packet skipped will generate a warning with the python logging module, but the stream will continue to return any subsequent well-formed packets as it is encountered. Signed-off-by: Daniel Kahn Gillmor --- pgpdump/data.py | 10 ++++++---- pgpdump/packet.py | 21 ++++++++++++++++++--- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/pgpdump/data.py b/pgpdump/data.py index f13c4bf..f73d25e 100644 --- a/pgpdump/data.py +++ b/pgpdump/data.py @@ -23,13 +23,15 @@ def __init__(self, data): self.data = data self.length = len(data) - def packets(self): - '''A generator function returning PGP data packets.''' + def packets(self, skip=False): + '''A generator function returning PGP data packets. + if skip=True, failing packets will log an error instead of raising an exception.''' offset = 0 while offset < self.length: - total_length, packet = construct_packet(self.data, offset) + total_length, packet = construct_packet(self.data, offset, skip) offset += total_length - yield packet + if packet is not None: + yield packet def __repr__(self): return "<%s: length %d>" % ( diff --git a/pgpdump/packet.py b/pgpdump/packet.py index bb4a2c3..8624252 100644 --- a/pgpdump/packet.py +++ b/pgpdump/packet.py @@ -2,6 +2,7 @@ import hashlib from math import ceil, log import re +import logging from .utils import (PgpdumpException, get_int2, get_int4, get_mpi, get_key_id, get_hex_data, get_int_bytes, pack_data) @@ -821,10 +822,16 @@ def old_tag_length(data, start): return (offset, length) -def construct_packet(data, header_start): +def construct_packet(data, header_start, skip=False): '''Returns a (length, packet) tuple constructed from 'data' at index 'header_start'. If there is a next packet, it will be found at - header_start + length.''' + header_start + length. + + If skip=True, then a packet with an error will emit a warning (via + the logging module) and return None as the packet; otherwise the + error will be raised directly. + + ''' # tag encoded in bits 5-0 (new packet format) # 0x3f == 111111b @@ -871,5 +878,13 @@ def construct_packet(data, header_start): data, header_start) else: break - packet = PacketType(tag, name, new, packet_data) + packet = None + try: + packet = PacketType(tag, name, new, packet_data) + except PgpdumpException as e: + if skip: + # FIXME: assmeble the packet structure and add it to the warning in ascii-armored form + logging.warning(e) + else: + raise return (consumed, packet) From dfe29ee543bce8546f068b87b5a86786883da198 Mon Sep 17 00:00:00 2001 From: Daniel Kahn Gillmor Date: Thu, 12 Sep 2019 08:34:21 -0400 Subject: [PATCH 08/11] raise errors for wrong-size ints --- pgpdump/utils.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/pgpdump/utils.py b/pgpdump/utils.py index fa209ca..eb47340 100644 --- a/pgpdump/utils.py +++ b/pgpdump/utils.py @@ -71,17 +71,26 @@ def crc24(data): def get_int2(data, offset): '''Pull two bytes from data at offset and return as an integer.''' + if offset+2 > len(data): + raise PgpdumpException("Wants a 2-byte integer at position %d of packet of size %d"%( + offset, len(data))) return (data[offset] << 8) + data[offset + 1] def get_int4(data, offset): '''Pull four bytes from data at offset and return as an integer.''' + if offset+4 > len(data): + raise PgpdumpException("Wants a 4-byte integer at position %d of packet of size %d"%( + offset, len(data))) return ((data[offset] << 24) + (data[offset + 1] << 16) + (data[offset + 2] << 8) + data[offset + 3]) def get_int8(data, offset): '''Pull eight bytes from data at offset and return as an integer.''' + if offset+8 > len(data): + raise PgpdumpException("Wants an 8-byte integer at position %d of packet of size %d"%( + offset, len(data))) return (get_int4(data, offset) << 32) + get_int4(data, offset + 4) From 3ae66d5de1f7dc32febab48a9e983b59ed43b5b0 Mon Sep 17 00:00:00 2001 From: Daniel Kahn Gillmor Date: Sat, 14 Sep 2019 21:24:47 -0400 Subject: [PATCH 09/11] add encode_packet, and dump encoded packet to log when malformed parsing --- pgpdump/packet.py | 6 +++--- pgpdump/utils.py | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 3 deletions(-) diff --git a/pgpdump/packet.py b/pgpdump/packet.py index 8624252..2ada765 100644 --- a/pgpdump/packet.py +++ b/pgpdump/packet.py @@ -4,7 +4,8 @@ import re import logging -from .utils import (PgpdumpException, get_int2, get_int4, get_mpi, +from .utils import (PgpdumpException, encode_packet, + get_int2, get_int4, get_mpi, get_key_id, get_hex_data, get_int_bytes, pack_data) @@ -883,8 +884,7 @@ def construct_packet(data, header_start, skip=False): packet = PacketType(tag, name, new, packet_data) except PgpdumpException as e: if skip: - # FIXME: assmeble the packet structure and add it to the warning in ascii-armored form - logging.warning(e) + logging.warning(str(e) + '\n' + encode_packet(tag, new, packet_data, armored=True)) else: raise return (consumed, packet) diff --git a/pgpdump/utils.py b/pgpdump/utils.py index eb47340..d8c1223 100644 --- a/pgpdump/utils.py +++ b/pgpdump/utils.py @@ -1,5 +1,7 @@ import binascii import sys +import codecs +from base64 import b64encode PY26 = sys.version_info[0] == 2 and sys.version_info[1] <= 6 @@ -94,6 +96,51 @@ def get_int8(data, offset): return (get_int4(data, offset) << 32) + get_int4(data, offset + 4) +def encode_packet(tag, new, data, armored=False): + if tag > 0x0f and not new: + raise PgpdumpException("cannot make new packet with tag %d"%(tag)) + hdr = bytearray() + if new: + hdr += bytearray([0xc0|tag]) + if len(data) < 192: + hdr += bytearray([len(data)]) + elif len(data) < 8384: + i = len(data) - 192 + hdr += bytearray([i//256, i%256]) + else: + i = len(data) + hdr += bytearray([256, i >> 24, (i>>16)&0xff, (i>>8)&0xff, i&0xff]) + else: # old-style packet format + if len(data) < (1<<8): + hdr += bytearray([0x80|(tag <<2), len(data)]) + elif len(data) < (1<<16): + hdr += bytearray([0x80|(tag <<2)|1, + (len(data)>>8)&0xff, + len(data)&0xff]) + elif len(data) < (1<<32): + hdr += bytearray([0x80|(tag <<2)|2, + (len(data)>>24)&0xff, + (len(data)>>16)&0xff, + (len(data)>>8)&0xff, + len(data)&0xff]) + else: + raise NotImplementedError('packet of length %d, but we do not generate indeterminate-sized packets'%(len(data),)) + frame = hdr + data + if not armored: + return frame + else: + strdata = codecs.decode(b64encode(frame), 'ascii') + return '''-----BEGIN PGP {block}----- + +{body} +={crc} +-----END PGP {block}----- +'''.format(block={2: 'SIGNATURE', 6: 'KEY BLOCK'}.get(tag, 'MESSAGE'), + body='\n'.join([strdata[i:i+64] for i in range(0, len(strdata), 64)]), + crc=codecs.decode(b64encode(crc24(frame).to_bytes(3, 'big')), 'ascii')) + + + def get_mpi(data, offset): '''Gets a multi-precision integer as per RFC-4880. Returns the MPI and the new offset. From 90718e8ce05e6b8bdb0b275f81e37e89d0d0b004 Mon Sep 17 00:00:00 2001 From: Daniel Kahn Gillmor Date: Sun, 15 Sep 2019 04:33:02 -0400 Subject: [PATCH 10/11] catch errors in new_tag_length Signed-off-by: Daniel Kahn Gillmor --- pgpdump/packet.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pgpdump/packet.py b/pgpdump/packet.py index 2ada765..26b1a28 100644 --- a/pgpdump/packet.py +++ b/pgpdump/packet.py @@ -773,6 +773,8 @@ def new_tag_length(data, start): look. Returns a derived (offset, length, partial) tuple. Reference: http://tools.ietf.org/html/rfc4880#section-4.2.2 ''' + if len(data) <= start: + raise PgpdumpException("new_tag_length at start %d of packet of length %d"%(start, len(data))) first = data[start] offset = length = 0 partial = False From 8fcc7b913825362e01ee9ff3b2490d2853aa48d0 Mon Sep 17 00:00:00 2001 From: Daniel Kahn Gillmor Date: Sun, 15 Sep 2019 09:36:42 -0400 Subject: [PATCH 11/11] throw a PgpdumpException on malformed image attributes Signed-off-by: Daniel Kahn Gillmor --- pgpdump/packet.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pgpdump/packet.py b/pgpdump/packet.py index 26b1a28..3145442 100644 --- a/pgpdump/packet.py +++ b/pgpdump/packet.py @@ -683,10 +683,16 @@ def parse(self): # there is only one currently known type- images (1) if sub_type == 1: # the only little-endian encoded value in OpenPGP + if len(self.data) <= (offset + 3): + raise PgpdumpException("Needs 4-octet attribute header at position %d of packet size %d"%(offset, len(self.data))) hdr_size = self.data[offset] + (self.data[offset + 1] << 8) hdr_version = self.data[offset + 2] self.raw_image_format = self.data[offset + 3] + if len(self.data) <= (offset + hdr_size): + raise PgpdumpException("Claimed attribute header has %d octets at position %d of packet size %d"%(hdr_size, offset, len(self.data))) offset += hdr_size + # FIXME: ensure that the reserved octets of the header are all-zeros + # (see https://tools.ietf.org/html/rfc4880#section-5.12.1) self.image_data = self.data[offset:] if self.raw_image_format == 1: