New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 689371 link

Starred by 1 user

Issue metadata

Status: Archived
Owner:
Closed: Feb 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug

Blocking:
issue 684354



Sign in to add a comment

vboot_reference: gen_test_cases.sh does not generate correct signatures anymore

Project Member Reported by drinkcat@chromium.org, Feb 7 2017

Issue description

In order to add RSA exponent 3 support, I need to regenerate keys and signatures. However, it seems like I cannot regenerate the signatures anymore.

In src/platform/vboot_reference:

 1. Cherry-pick https://chromium-review.googlesource.com/439064 (this adds make gentestcases target, and prevents test_file from being regenerated if it already exists).
 2. make runtests => All tests pass
 3. make gentestcases

At this point, git status should show no change (test_file has not changed, nor the keys), but all the signatures have now changed (and are incorrect):

Changes not staged for commit:
	modified:   tests/testcases/test_file.rsa1024_sha1.sig
...
	modified:   tests/testcases/test_file.rsa8192_sha512.sig

And now, make runtests fails:

tests/vb2_rsa_tests.sh
Testing signature verification...
For RSA-1024 and sha1:
Signature Verification FAILED
 
Blocking: 684354
Just picking out one case (algo 0: RSA-1024 with SHA-1), gen_test_cases.sh does this:

build/install_for_test/bin/signature_digest_utility 0 tests/testcases/test_file | openssl rsautl -sign -pkcs -inkey tests/testkeys/key_rsa1024.pem > tests/testcases/test_file.rsa1024_sha1.sig

And vb2_rsa_tests.sh does this to verify the signature:

build/install_for_test/bin/verify_data 0 tests/testkeys/key_rsa1024.keyb tests/testcases/test_file.rsa1024_sha1.sig tests/testcases/test_file

I did a bisect and this turns out to be the first bad commit:

commit 46a382d6136f2fd206fd8c95180dbb816c9ad5ce
Author: Randall Spangler <rspangler@chromium.org>
Date:   Tue Oct 18 12:00:07 2016 -0700

    vboot: Remove vboot1 cryptolib padding source
    
    The old vboot1 cryptolib hard-coded many of its padding arrays in a
    padding.c file.  Use the equivalent vboot2 apis instead.
    
    This change is almost exclusively on the host and test side; the only
    firmware impact is on a single line of debug output.
    
    BUG=chromium:611535
    BRANCH=none
    TEST=make runtests; emerge-kevin coreboot depthcharge
    
    Change-Id: If689ffd92f0255847bea2424950da4547b2c0df3
    Signed-off-by: Randall Spangler <rspangler@chromium.org>
    Reviewed-on: https://chromium-review.googlesource.com/400902
    Reviewed-by: Daisuke Nojiri <dnojiri@chromium.org>

Basically, signature_digest_utility now returns nothing:
build/install_for_test/bin/signature_digest_utility 0 tests/testcases/test_file | wc
      0       0       0
(returns error code 255, actually)

While, before that commit, it would output something sensible:
build/install_for_test/bin/signature_digest_utility 0 tests/testcases/test_file | hexdump -C
00000000  30 21 30 09 06 05 2b 0e  03 02 1a 05 00 04 14 bb  |0!0...+.........|
00000010  bc 03 28 c7 5c 1a 04 bb  f1 6d 80 25 7e 6c 38 4b  |..(.\....m.%~l8K|
00000020  21 4a 6f                                          |!Jo|

Anyway, fix is here: https://chromium-review.googlesource.com/439086 .

I'm not sure what's the impact of this bug, but it seems like it only affects signature_digest_utility, which may accidentally include the wrong digestinfo if VB2 algorithm happens to be a valid hash_halg, e.g. build/install_for_test/bin/signature_digest_utility 3 tests/testcases/test_file does output something.
Owner: rspangler@chromium.org
Status: Assigned (was: Available)
Project Member

Comment 3 by bugdroid1@chromium.org, Feb 11 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/vboot_reference/+/0bde10afbfa8753521b1d10d89b5409d4d1ad1f2

commit 0bde10afbfa8753521b1d10d89b5409d4d1ad1f2
Author: Nicolas Boichat <drinkcat@google.com>
Date: Sat Feb 11 10:42:01 2017

signature_digest/SignatureDigest: convert vb2_crypto to hash algorithm

We were passing the wrong value to PrependDigestInfo. Let's also refactor
the function a little bit.

BRANCH=none
BUG= chromium:689371 
TEST=make gentestcases; git status => no change

Change-Id: I0244c3f3de05b33b7ddd21e93a266faf34f2c239
Reviewed-on: https://chromium-review.googlesource.com/439086
Commit-Ready: Nicolas Boichat <drinkcat@chromium.org>
Tested-by: Nicolas Boichat <drinkcat@chromium.org>
Reviewed-by: Randall Spangler <rspangler@chromium.org>
Reviewed-by: Vincent Palatin <vpalatin@chromium.org>

[modify] https://crrev.com/0bde10afbfa8753521b1d10d89b5409d4d1ad1f2/host/lib/signature_digest.c

Owner: drinkcat@chromium.org
Status: Fixed (was: Assigned)
Fixed. I can't find any uses of signature_digest_utility in our code, expect for that specific case generation shell script, so I believe this bug had no impact.

Comment 5 by dchan@google.com, Apr 17 2017

Labels: VerifyIn-59

Comment 6 by dchan@google.com, May 30 2017

Labels: VerifyIn-60

Comment 7 by dchan@chromium.org, Aug 1 2017

Labels: VerifyIn-61

Comment 8 by dchan@chromium.org, Oct 14 2017

Status: Archived (was: Fixed)

Sign in to add a comment