Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(10)

Issue 660161: Vboot Reference: Add functions to verify signed kernel images. (Closed)

Created:
10 years, 10 months ago by gauravsh
Modified:
9 years, 6 months ago
CC:
chromium-os-reviews_chromium.org
Visibility:
Public.

Description

Vboot Reference: Add functions to verify signed kernel images. BUG=670 TEST=Adds kernel_image_test which tests the new functions. The kernel image verification pretty much exactly mirror the already existing firmware image verification functions except with a few different/additional fields in a signed kernel image. The firmware signing key is the root key equivalent for kernel images. This CL also moves the image verification tests to a different script. There's some additional cleanup of the code that I will be submitting separately after this and another pending patches get LGTMed and land.

Patch Set 1 #

Total comments: 16

Patch Set 2 : Clean up the patch. #

Patch Set 3 : Change field ordering/types based on the review. #

Total comments: 17

Patch Set 4 : Fixes based on the review. #

Patch Set 5 : Fix comment. #

Total comments: 2

Patch Set 6 : Fix comment. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1199 lines, -31 lines) Patch
A src/platform/vboot_reference/include/kernel_image.h View 1 2 3 4 5 1 chunk +200 lines, -0 lines 0 comments Download
M src/platform/vboot_reference/tests/Makefile View 2 chunks +10 lines, -6 lines 0 comments Download
M src/platform/vboot_reference/tests/firmware_image_tests.c View 1 1 chunk +7 lines, -2 lines 0 comments Download
A src/platform/vboot_reference/tests/kernel_image_tests.c View 1 2 3 4 1 chunk +257 lines, -0 lines 0 comments Download
A src/platform/vboot_reference/tests/run_image_verification_tests.sh View 1 chunk +100 lines, -0 lines 0 comments Download
M src/platform/vboot_reference/tests/run_rsa_tests.sh View 3 chunks +7 lines, -21 lines 0 comments Download
M src/platform/vboot_reference/utils/Makefile View 1 2 chunks +4 lines, -2 lines 0 comments Download
A src/platform/vboot_reference/utils/kernel_image.c View 1 2 3 1 chunk +614 lines, -0 lines 0 comments Download

Messages

Total messages: 9 (0 generated)
gauravsh
Randall: Can you have a look at the KernelImage structure and let me know if ...
10 years, 10 months ago (2010-02-26 04:56:55 UTC) #1
gauravsh
Ugh, i just realized that git cl upload also picked up diffs from changes that ...
10 years, 10 months ago (2010-02-26 05:03:05 UTC) #2
Randall Spangler
http://codereview.chromium.org/660161/diff/1/5 File src/platform/vboot_reference/include/kernel_image.h (right): http://codereview.chromium.org/660161/diff/1/5#newcode23 src/platform/vboot_reference/include/kernel_image.h:23: uint32_t kernel_load_addr; /* Load address in memory for the ...
10 years, 10 months ago (2010-02-26 22:38:40 UTC) #3
gauravsh
Cleaned up the patch to remove all the leakage from the other CL. Git interactive ...
10 years, 10 months ago (2010-02-26 23:21:56 UTC) #4
gauravsh
http://codereview.chromium.org/660161/diff/1/5 File src/platform/vboot_reference/include/kernel_image.h (right): http://codereview.chromium.org/660161/diff/1/5#newcode23 src/platform/vboot_reference/include/kernel_image.h:23: uint32_t kernel_load_addr; /* Load address in memory for the ...
10 years, 10 months ago (2010-02-27 01:25:07 UTC) #5
Chris Masone
Mostly just nits, except the one in the destructor in kernel_image.c http://codereview.chromium.org/660161/diff/1/5 File src/platform/vboot_reference/include/kernel_image.h (right): ...
10 years, 9 months ago (2010-02-28 23:02:51 UTC) #6
gauravsh
http://codereview.chromium.org/660161/diff/1/5 File src/platform/vboot_reference/include/kernel_image.h (right): http://codereview.chromium.org/660161/diff/1/5#newcode36 src/platform/vboot_reference/include/kernel_image.h:36: * signing key. */ On 2010/02/28 23:02:52, cmasone wrote: ...
10 years, 9 months ago (2010-03-01 02:48:05 UTC) #7
Chris Masone
lgtm once you fix the comment. http://codereview.chromium.org/660161/diff/51/1023 File src/platform/vboot_reference/include/kernel_image.h (right): http://codereview.chromium.org/660161/diff/51/1023#newcode43 src/platform/vboot_reference/include/kernel_image.h:43: * [header_len | ...
10 years, 9 months ago (2010-03-01 03:12:17 UTC) #8
gauravsh
10 years, 9 months ago (2010-03-01 03:18:12 UTC) #9
Fixed the comment. Submitting...

http://codereview.chromium.org/660161/diff/51/1023
File src/platform/vboot_reference/include/kernel_image.h (right):

http://codereview.chromium.org/660161/diff/51/1023#newcode43
src/platform/vboot_reference/include/kernel_image.h:43: * [header_len |
On 2010/03/01 03:12:17, cmasone wrote:
> yeah, I think that bitwise OR could be a legit interpretation of this
> syntax...if you can make it clear that you mean concatenation here, that'd be
> helpful

Ok, replaced | with a comma and made it clear it's concatenation.

Powered by Google App Engine
This is Rietveld 408576698