From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.90_1) id 1jf8tq-0004t3-5t for mharc-grub-devel@gnu.org; Sat, 30 May 2020 17:20:14 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:35778) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1jf8to-0004sY-P2 for grub-devel@gnu.org; Sat, 30 May 2020 17:20:12 -0400 Received: from mail-io1-xd42.google.com ([2607:f8b0:4864:20::d42]:41759) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1jf8tm-0004Ol-Ud for grub-devel@gnu.org; Sat, 30 May 2020 17:20:12 -0400 Received: by mail-io1-xd42.google.com with SMTP id o5so3051717iow.8 for ; Sat, 30 May 2020 14:20:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=dyfis.net; s=google; h=date:from:to:subject:message-id:mime-version:content-disposition; bh=KxZfPfrHyvQMUB9PWp53NHoEAHZIY7tI+cb6syccwvw=; b=d+HiBBIeJz5pAQcKuBteHoEPCPP9PqEgj6tt/C4qHtX9mXjUlWBhfD4gqp5KGlgiJ6 jhv5pYqp9hPuuh0ysyzl6vmHvDF4MuBuWvRzn1ziqme6V6Xh3im513CjCs6eSfxBztD8 TBItCcDHMRV2HZf3Xt/ShyqeNjEa0LCc1SKlw= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:subject:message-id:mime-version :content-disposition; bh=KxZfPfrHyvQMUB9PWp53NHoEAHZIY7tI+cb6syccwvw=; b=BF8ATf3dwP209i/jo3WWKOPQwaDE9Kc5HRpbRAvDFA01cRaxmUDbnUybaGBcQ6ZMbY xCuosIEfQhz57PC38ipRp9f1lJcgUsbyRlY7bO6fCXQ6o9b36EgJsFm2Dy4ejvjfGro8 SRe7ThZs2880aZs8om37kjBaPOIqV12d0oIaWG86rwXg0OtqYLA/j/n8L03snhJQaJp1 g1A5h+MGzL+vPv5Y1OHtXduuOlV/8Svc4v+ooSIP0/lyQc2y1krLnzXg+jhijEqnXPhv SWfK/IvwsxPOucfYjjfvs4c+IxyvoV1hZjVju+Giyb9LNFmeA5p0M0IuPaO//Uh+mBs5 cT0A== X-Gm-Message-State: AOAM53152VLNcRpI7fIz9fDN5dDxILbxpoSW/owIf/+F7FZHV7Mxdw8x loLUakUMddlXJxLWgjWv9VcNWu3OKbE= X-Google-Smtp-Source: ABdhPJztMstZ0lUmPSv6a+rJ3JB2jBKOTTwzKxHK+e4UYKXyOuVStlSNneg0Do3M5keyzJo22C0vSg== X-Received: by 2002:a05:6638:10b:: with SMTP id x11mr12725041jao.109.1590873608241; Sat, 30 May 2020 14:20:08 -0700 (PDT) Received: from shiny5l ([198.54.107.169]) by smtp.gmail.com with ESMTPSA id 18sm5494022ion.17.2020.05.30.14.20.07 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 30 May 2020 14:20:07 -0700 (PDT) Date: Sat, 30 May 2020 16:20:06 -0500 From: Charles Duffy To: grub-devel@gnu.org Subject: [PATCH] pgp: Recognize issuer subpackets in either hashed or unhashed sections Message-ID: <20200530211727.GA9870@shiny5l> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Received-SPF: pass client-ip=2607:f8b0:4864:20::d42; envelope-from=charles@dyfis.net; helo=mail-io1-xd42.google.com X-detected-operating-system: by eggs.gnu.org: No matching host in p0f cache. That's all we know. X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, SPF_PASS=-0.001, URIBL_BLOCKED=0.001 autolearn=_AUTOLEARN X-Spam_action: no action X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 30 May 2020 21:20:13 -0000 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