From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.71) id 1aokUp-0002Qe-Qv for mharc-grub-devel@gnu.org; Sat, 09 Apr 2016 00:27:43 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38645) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aokUn-0002NB-7d for grub-devel@gnu.org; Sat, 09 Apr 2016 00:27:42 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aokUk-0000Kf-0U for grub-devel@gnu.org; Sat, 09 Apr 2016 00:27:41 -0400 Received: from mail-lf0-x235.google.com ([2a00:1450:4010:c07::235]:33250) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aokUj-0000KT-Jp for grub-devel@gnu.org; Sat, 09 Apr 2016 00:27:37 -0400 Received: by mail-lf0-x235.google.com with SMTP id e190so97044721lfe.0 for ; Fri, 08 Apr 2016 21:27:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=subject:to:references:from:message-id:date:user-agent:mime-version :in-reply-to:content-transfer-encoding; bh=f6Ou6DFuGQGx5ztuY8t5cv/L3VuZAVht5rpFNIIr9y0=; b=nQFEtRxXHavMxrYTaCxRWol96gbm6KRUDh/86KUdRcwE+7Ne3nzpS4m1eOlQLcPctV Tpzezk9ztblmpWqJ3j7xO1rYbgW460WPmpyWXYOq0dU7T8iWe9KwkuVJzeKPp0Zwfsvj 6vS5tlJva+PSAof3Fvy8nuJ691iK3e5criiY2FAJsmewI2O8GhKCc1bcGBMXbpifpB0b lL6AkUTIjprNgIZUF9+kAiR0zGrvHmINk5VwgAePNYzi+R6g1PeGUt1caM23bluA9XNp jfiIAG/VUdWaxR2Fjov7cTT0l99UDtS9jUlqng4v+X5VQHRZBtjZGXjm1FvB3B0UUOXE w4Xw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding; bh=f6Ou6DFuGQGx5ztuY8t5cv/L3VuZAVht5rpFNIIr9y0=; b=Qn+CYFvLZISr1acFMyjklLl7+gYDheJ+n+84Mwryb5QQkXHGkDuJCnZfYvUiXJeph5 ur6fg/3BlR1AHZhIuGcX4nkyMQxIunublGK+DtjZkZtTpzuU4GQlXTMovuG3sOJKSfhl GieZJUIcgEyV5QZQHEjtZAsB3K5k7iiHWE8ZQzlCC03/2sXX87CT5vLsEVeCKMk7qFds 8jMTJ7dOWNfgiYEUH9cgCMwYbz4VjMpZTzMhqGotuH9F5yms7uwKvg8JLcopKYjwCN7j f4CMXj7un4CDv5LooVcyDFCP1WkcJodNnz7fnWY4sjUyR3WISFq02Fc/GhMCRP6C8Rou 8rfA== X-Gm-Message-State: AD7BkJKN3O6EiJlbL/JUFCnnFlq+RKvL5THERofNcMwRstn+eGDtiboO4drIfEwRPAGJGA== X-Received: by 10.25.166.140 with SMTP id p134mr4019076lfe.29.1460176056786; Fri, 08 Apr 2016 21:27:36 -0700 (PDT) Received: from [192.168.1.42] (ppp109-252-90-50.pppoe.spdop.ru. [109.252.90.50]) by smtp.gmail.com with ESMTPSA id nv8sm2510050lbb.7.2016.04.08.21.27.35 for (version=TLSv1/SSLv3 cipher=OTHER); Fri, 08 Apr 2016 21:27:35 -0700 (PDT) Subject: Re: [PATCH] verify: search keyid in hashed signature subpackets To: grub-devel@gnu.org References: <56FB59AD.9020804@gmail.com> From: Andrei Borzenkov X-Enigmail-Draft-Status: N1110 Message-ID: <570884B6.2000302@gmail.com> Date: Sat, 9 Apr 2016 07:27:34 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 2a00:1450:4010:c07::235 X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 09 Apr 2016 04:27:42 -0000 30.03.2016 17:09, Ignat Korchagin пишет: > Implemented as a separate function which should process arbitrary length data. TBH I still think that simply setting READBUF_SIZE to 64K is the simplest solution. > As for tests, it seems that the easiest way is to add this signature to tests/file_filter. Not sure how should I send you the patch with binary data though. > Just sign files and send new signatures and keys, I will commit them. > diff --git a/grub-core/commands/verify.c b/grub-core/commands/verify.c > index 166d0aa..cc8fa39 100644 > --- a/grub-core/commands/verify.c > +++ b/grub-core/commands/verify.c > @@ -445,6 +445,88 @@ rsa_pad (gcry_mpi_t *hmpi, grub_uint8_t *hval, > return ret; > } > > +static grub_uint64_t > +grub_subpacket_keyid_search (grub_file_t f, grub_ssize_t sub_len, > + const gcry_md_spec_t *hash, void *context) This does more than just searching for keyid, it also hashes content, so name is misleading. > +{ > + grub_uint64_t keyid = 0; > + grub_uint8_t *readbuf = NULL; > + > + while (sub_len > 0) > + { > + grub_uint8_t szid[5]; > + grub_size_t sz_len; > + grub_size_t l; > + > + grub_ssize_t r = grub_file_read (f, szid, 1); > + if (r != 1) > + return 0; > + > + if (szid[0] < 192) > + { > + l = szid[0]; > + sz_len = 1; > + } > + else if (szid[0] < 255) > + { > + r = grub_file_read (f, szid + 1, 1); > + if (r != 1) > + return 0; > + > + l = (((szid[0] & ~192) << GRUB_CHAR_BIT) | szid[1]) + 192; > + sz_len = 2; > + } > + else > + { > + r = grub_file_read (f, szid + 1, 4); > + if (r != 4) > + return 0; > + > + l = grub_be_to_cpu32 (grub_get_unaligned32 (szid + 1)); > + sz_len = 5; > + } > + > + readbuf = grub_zalloc (l); So you allocate full subpacket length anyway. Why not set READBUF_SIZE to max size then from the very start? > + if (!readbuf) > + return 0; > + > + r = grub_file_read (f, readbuf, l); > + if (r <= 0) > + goto fail; > + > + while ((grub_size_t)r < l) > + { > + grub_ssize_t rr = grub_file_read (f, readbuf + r, l - (grub_size_t)r); > + if (rr <= 0) > + goto fail; > + r += rr; > + } > + > + if (*readbuf == 0x10 && l >= 8) > + keyid = grub_get_unaligned64 (readbuf + 1); > + > + if (hash && context) > + { > + hash->write (context, szid, sz_len); > + hash->write (context, readbuf, l); > + } > + > + grub_free (readbuf); > + readbuf = NULL; > + > + sub_len -= sz_len + l; > + } > + > +fail: > + if (readbuf) > + { > + grub_free (readbuf); > + return 0; > + } > + > + return keyid; > +} > + > static grub_err_t > grub_verify_signature_real (char *buf, grub_size_t size, > grub_file_t f, grub_file_t sig, > @@ -532,17 +614,7 @@ grub_verify_signature_real (char *buf, grub_size_t size, > > hash->write (context, &v, sizeof (v)); > hash->write (context, &v4, sizeof (v4)); > - while (rem) > - { > - r = grub_file_read (sig, readbuf, > - rem < READBUF_SIZE ? rem : READBUF_SIZE); > - if (r < 0) > - goto fail; > - if (r == 0) > - break; > - hash->write (context, readbuf, r); > - rem -= r; > - } > + keyid = grub_subpacket_keyid_search (sig, rem, hash, context); > hash->write (context, &v, sizeof (v)); > s = 0xff; > hash->write (context, &s, sizeof (s)); > @@ -550,37 +622,11 @@ grub_verify_signature_real (char *buf, grub_size_t size, > r = grub_file_read (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 (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); > + if (keyid == 0) > + keyid = grub_subpacket_keyid_search (sig, rem, NULL, NULL); > + else > + grub_subpacket_keyid_search (sig, rem, NULL, NULL); > > hash->final (context); > > > > _______________________________________________ > Grub-devel mailing list > Grub-devel@gnu.org > https://lists.gnu.org/mailman/listinfo/grub-devel >