VBoot Reference: Fix splicing bugs in Firmware and Kernel verification.

VBoot Reference: Fix splicing bugs in Firmware and Kernel verification.

BUG=670
TESTS=Adds new tests which verify this doesn't occur anymore. Existing tests still pass.

The existing code computes and verifies signatures on firmware/kernel data and firmware/kernel versions separately. This causes a image splicing bug where it is possible to combine together a version signature from a valid new firmware with firmware data and signature from an older version. The same problem exists with kernel verification.

This CL fixes this by changing the firmware/kernel signatures to also include the version information.

For the Firmware, there's a separate signature on the preamble (which contains the version) but the firmware signature now also includes this preamble in addition to the firmware data.

For the Kernel, there's a separate signature on the kernel config/options (wich also contains the version), but the kernel signature now also includes these config/options in addition to the kernel data.

[gauravsh, 2010-03-29 17:50:16]

ping?

On Fri, Mar 26, 2010 at 10:46 AM,  <gauravsh@chromium.org> wrote:
> Reviewers: Will Drewry,
>
> Description:
> VBoot Reference: Fix splicing bugs in Firmware and Kernel verification.
>
> BUG=670
> TESTS=Adds new tests which verify this doesn't occur anymore. Existing tests
> still pass.
>
> The existing code computes and verifies signatures on firmware/kernel data
> and
> firmware/kernel versions separately. This causes a image splicing bug where
> it
> is possible to combine together a version signature from a valid new
> firmware
> with firmware data and signature from an older version. The same problem
> exists
> with kernel verification.
>
> This CL fixes this by changing the firmware/kernel signatures to also
> include
> the version information.
>
> For the Firmware, there's a separate signature on the preamble (which
> contains
> the version) but the firmware signature now also includes this preamble in
> addition to the firmware data.
>
> For the Kernel, there's a separate signature on the kernel config/options
> (wich
> also contains the version), but the kernel signature now also includes these
> config/options in addition to the kernel data.
>
> Please review this at http://codereview.chromium.org/1430001
>
> Affected files:
>  M src/platform/vboot_reference/crypto/rsa_utility.c
>  M src/platform/vboot_reference/include/firmware_image.h
>  M src/platform/vboot_reference/include/kernel_image.h
>  M src/platform/vboot_reference/include/rsa_utility.h
>  M src/platform/vboot_reference/tests/Makefile
>  M src/platform/vboot_reference/tests/firmware_image_tests.c
>  A src/platform/vboot_reference/tests/firmware_splicing_tests.c
>  M src/platform/vboot_reference/tests/kernel_image_tests.c
>  A src/platform/vboot_reference/tests/kernel_splicing_tests.c
>  M src/platform/vboot_reference/tests/test_common.h
>  M src/platform/vboot_reference/tests/test_common.c
>  M src/platform/vboot_reference/utils/firmware_image.c
>  M src/platform/vboot_reference/utils/kernel_image.c
>
>
>



-- 
-g

[Will Drewry, 2010-03-29 23:03:40]

LGTM

Feel free to submit - I may take a look through all of this again(!) (including
code not changed) to make sure I understand all the checks.

Thanks!!

http://codereview.chromium.org/1430001/diff/2001/3009
File src/platform/vboot_reference/tests/kernel_splicing_tests.c (right):

http://codereview.chromium.org/1430001/diff/2001/3009#newcode28
src/platform/vboot_reference/tests/kernel_splicing_tests.c:28: &len);
spacing ;)