From c6e017092eb4472694e182c8291d169363b73e5f Mon Sep 17 00:00:00 2001 From: Charles Duffy Date: Sat, 30 May 2020 13:55:42 -0500 Subject: [PATCH] pgp: Recognize issuer subpackets in either hashed or unhashed sections To: grub-devel@gnu.org Currently, GRUB's OpenPGP signature parsing searches for the issuer field (specifying the key to use) only in the unhashed portion of the signature. RFC 4880 permits almost all fields (with the sole exception of signature creation time, which MUST be recognized only in the hashed area) to be present either in hashed or unhashed areas; and specifies that implementations should use the unhashed area only for "advisory information". While GnuPG's decision to consider issuer ID advisory is defensible (after all, one could simply do an exhaustive search of known public keys in its absence), it is not the only valid decision; in particular, the Go x/crypto/openpgp library chooses to store issuer ID in the hashed area. Without this patch, trying to verify a valid signature made by x/crypto/openpgp results in `error: public key 00000000 not found.`, because the `keyid` variable is unpopulated. This patch, originally written by Ignat Korchagin and ported to GRUB 2.04 by Daniel Axtens, remedies this. I (Charles Duffy) have tried to address review comments on the original requesting that named constants be used to enhance readability. There are still outstanding compatibility issues parsing public keys not serialized by GnuPG; these may be addressed in a later patch. Signed-off-by: Ignat Korchagin Signed-off-by: Daniel Axtens Signed-off-by: Charles Duffy --- grub-core/commands/pgp.c | 137 +++++++++++++++++++++++++++------------ 1 file changed, 97 insertions(+), 40 deletions(-) diff --git a/grub-core/commands/pgp.c b/grub-core/commands/pgp.c index bbf6871fe..b49b997c4 100644 --- a/grub-core/commands/pgp.c +++ b/grub-core/commands/pgp.c @@ -39,6 +39,17 @@ enum OPTION_SKIP_SIG = 0 }; +enum + { + OPENPGP_SIG_SUBPACKET_TYPE_ISSUER = 16 /* subpacket contains 8-octet key id, see RFC4880 5.2.3.5 */ + }; + +enum + { + OPENPGP_SIG_SUBPACKET_LEN_2BYTE_LIMIT = 192, + OPENPGP_SIG_SUBPACKET_LEN_5BYTE_CONST = 255 + }; + static const struct grub_arg_option options[] = { {"skip-sig", 's', 0, @@ -448,6 +459,47 @@ struct grub_pubkey_context void *hash_context; }; +static grub_uint64_t +grub_subpacket_keyid_search (const grub_uint8_t * sub, grub_ssize_t sub_len) +{ + const grub_uint8_t *ptr; + grub_uint32_t l; + grub_uint64_t keyid = 0; + + for (ptr = sub; ptr < sub + sub_len; ptr += l) + { + /* this algorithm is expressely given in RFC 4880 5.2.3.1 to parse length + * specifications, which may be 1-byte, 2-byte or 5-bytes long */ + if (*ptr < OPENPGP_SIG_SUBPACKET_LEN_2BYTE_LIMIT) + l = *ptr++; + /* 2-octet length field; the two high bits used to specify this format + * are not part of the data, and the value as a whole is offset to avoid + * having multiple ways to specify values that would fit in the 1-byte + * form */ + else if (*ptr < OPENPGP_SIG_SUBPACKET_LEN_5BYTE_CONST) + { + if (ptr + 1 >= sub + sub_len) + break; + l = (((ptr[0] - OPENPGP_SIG_SUBPACKET_LEN_2BYTE_LIMIT) << GRUB_CHAR_BIT) | ptr[1]) + + OPENPGP_SIG_SUBPACKET_LEN_2BYTE_LIMIT; + ptr += 2; + } + /* 5-octet length field, 0xff constant followed by 4-byte value */ + else + { + if (ptr + 5 >= sub + sub_len) + break; + l = grub_be_to_cpu32 (grub_get_unaligned32 (ptr + 1)); + ptr += 5; + } + /* determine whether we found the packet we're looking for */ + if (*ptr == OPENPGP_SIG_SUBPACKET_TYPE_ISSUER && l >= 8) + keyid = grub_get_unaligned64 (ptr + 1); + } + + return keyid; +} + static grub_err_t grub_verify_signature_init (struct grub_pubkey_context *ctxt, grub_file_t sig) { @@ -520,7 +572,7 @@ grub_verify_signature_real (struct grub_pubkey_context *ctxt, gcry_mpi_t mpis[10]; grub_uint8_t pk = ctxt->v4.pkeyalgo; grub_size_t i; - grub_uint8_t *readbuf = NULL; + grub_uint8_t *readbuf = NULL, *subpacket_buf = NULL; unsigned char *hval; grub_ssize_t rem = grub_be_to_cpu16 (ctxt->v4.hashed_sub); grub_uint32_t headlen = grub_cpu_to_be32 (rem + 6); @@ -538,17 +590,29 @@ grub_verify_signature_real (struct grub_pubkey_context *ctxt, ctxt->hash->write (ctxt->hash_context, &ctxt->v, sizeof (ctxt->v)); ctxt->hash->write (ctxt->hash_context, &ctxt->v4, sizeof (ctxt->v4)); - while (rem) + + subpacket_buf = grub_malloc (rem); + if (!subpacket_buf) + goto fail; + + r = 0; + while (r < rem) { - r = grub_file_read (ctxt->sig, readbuf, - rem < READBUF_SIZE ? rem : READBUF_SIZE); - if (r < 0) - goto fail; - if (r == 0) + grub_ssize_t rr = grub_file_read (ctxt->sig, subpacket_buf + r, rem - r); + if (rr < 0) + goto fail; + if (rr == 0) break; - ctxt->hash->write (ctxt->hash_context, readbuf, r); - rem -= r; + r += rr; } + if (r != rem) + goto fail; + ctxt->hash->write (ctxt->hash_context, subpacket_buf, rem); + + keyid = grub_subpacket_keyid_search (subpacket_buf, rem); + grub_free (subpacket_buf); + subpacket_buf = NULL; + ctxt->hash->write (ctxt->hash_context, &ctxt->v, sizeof (ctxt->v)); s = 0xff; ctxt->hash->write (ctxt->hash_context, &s, sizeof (s)); @@ -556,37 +620,27 @@ grub_verify_signature_real (struct grub_pubkey_context *ctxt, r = grub_file_read (ctxt->sig, &unhashed_sub, sizeof (unhashed_sub)); if (r != sizeof (unhashed_sub)) goto fail; - { - grub_uint8_t *ptr; - grub_uint32_t l; - rem = grub_be_to_cpu16 (unhashed_sub); - if (rem > READBUF_SIZE) - goto fail; - r = grub_file_read (ctxt->sig, readbuf, rem); - if (r != rem) - goto fail; - for (ptr = readbuf; ptr < readbuf + rem; ptr += l) - { - if (*ptr < 192) - l = *ptr++; - else if (*ptr < 255) - { - if (ptr + 1 >= readbuf + rem) - break; - l = (((ptr[0] & ~192) << GRUB_CHAR_BIT) | ptr[1]) + 192; - ptr += 2; - } - else - { - if (ptr + 5 >= readbuf + rem) - break; - l = grub_be_to_cpu32 (grub_get_unaligned32 (ptr + 1)); - ptr += 5; - } - if (*ptr == 0x10 && l >= 8) - keyid = grub_get_unaligned64 (ptr + 1); - } - } + + rem = grub_be_to_cpu16 (unhashed_sub); + subpacket_buf = grub_malloc (rem); + if (!subpacket_buf) + goto fail; + + r = 0; + while (r < rem) + { + grub_ssize_t rr = grub_file_read (ctxt->sig, subpacket_buf + r, rem - r); + if (rr < 0) + goto fail; + if (rr == 0) + break; + r += rr; + } + if (r != rem) + goto fail; + + if (keyid == 0) + keyid = grub_subpacket_keyid_search (subpacket_buf, rem); ctxt->hash->final (ctxt->hash_context); @@ -656,10 +710,13 @@ grub_verify_signature_real (struct grub_pubkey_context *ctxt, goto fail; grub_free (readbuf); + grub_free (subpacket_buf); return GRUB_ERR_NONE; fail: + if (subpacket_buf) + grub_free (subpacket_buf); grub_free (readbuf); if (!grub_errno) return grub_error (GRUB_ERR_BAD_SIGNATURE, N_("bad signature")); -- 2.25.4