Add script for re-signing final firmware images with the correct keys.

Add script for re-signing final firmware images with the correct keys.

Also add a script for splitting a firmware image into component firmware data, vblocks and the GBB.

Note: The script uses fmap_decode, a utility to parse flashmap of a firmware image, and a part of the flashmap project:
http://code.google.com/p/flashmap/

BUG=3496
TEST=Tested with newer builds of firmware images with flashmaps enabled. Steps to verify:

1) Use script to re-sign an existing image with a new set of keys.
2) Use unpack_firmwarefd.sh to get individual firmware data and vblocks.
3) Use vbutil_firmware with the new keys. Verification should succeed with
   the newer keys but fail with the older ones.

[gauravsh, 2010-07-23 17:31:35]

ping?

[gauravsh, 2010-07-23 18:35:08]

On Fri, Jul 23, 2010 at 11:30 AM, Vadim Bendebury (вб)
<vbendeb@google.com> wrote:
> On Fri, Jul 23, 2010 at 10:31 AM,  <gauravsh@chromium.org> wrote:
>> ping?
>>
>> http://codereview.chromium.org/3026018/show
>>
> Sorry, I got sidetracked. I am looking into it now and have some
> comments coming up.
>
> In the meantime it would be great to see some info on fmap_decode
> included in the CL description - maybe  a pointer to a doc describing
> it, or to the CL which introduced it, etc.

Done.

>
> cheers,
> /v
>



-- 
-g

[vb, 2010-07-27 00:13:26]

Gaurav, sorry for the delay.

Some comments are duplicated as they seemed to be lost, but they actually were
attached to the earlier versions.

Most of the comments are FYI, but I you should replace curly brackets when
grouping bash commands.

http://codereview.chromium.org/3026018/diff/3001/4003
File scripts/image_signing/resign_firmwarefd.sh (right):

http://codereview.chromium.org/3026018/diff/3001/4003#newcode7
scripts/image_signing/resign_firmwarefd.sh:7: # Script to resign an firmware
image using a different set of keys
s/an f/a f/

Also, it would be good to describe the expected command parameters here if not
in the usage output.

http://codereview.chromium.org/3026018/diff/3001/4003#newcode17
scripts/image_signing/resign_firmwarefd.sh:17: echo "Usage: $0 src_fd dst_fd
firmware_datakey firmware_keyblock kernel_subkey"
80 col limit

http://codereview.chromium.org/3026018/diff/3001/4003#newcode22
scripts/image_signing/resign_firmwarefd.sh:22: type -P fmap_decode &>/dev/null
|| \
Just a thought:

could be compacted using

for p in fmap_decode vbutil_firmware; do
  type -P "${p}" &>/dev/null  || \
  { echo "${p} tool not found."; exit 1; }
done

http://codereview.chromium.org/3026018/diff/2002/7003#newcode17
scripts/image_signing/resign_firmwarefd.sh:17: echo "Usage: $0 src_fd dst_fd
firmware_datakey firmware_keyblock kernel_subkey"
80 col limit

http://codereview.chromium.org/3026018/diff/2002/7003#newcode22
scripts/image_signing/resign_firmwarefd.sh:22: type -P fmap_decode &>/dev/null
|| \
This could be made easily expendable if expressed through 

for f in fmap_decode vbutil_firmware; do
  type -P $f &>/dev/null || \
   ( echo "$f tool not found."; exit 1; )
done

http://codereview.chromium.org/3026018/diff/2002/7003#newcode23
scripts/image_signing/resign_firmwarefd.sh:23: { echo "fmap_decode tool not
found."; exit 1; }
these should be regular parens, not curly brackets

http://codereview.chromium.org/3026018/diff/2002/7003#newcode58
scripts/image_signing/resign_firmwarefd.sh:58: vbutil_firmware \
why do you need two traps here?

http://codereview.chromium.org/3026018/diff/2002/7003#newcode70
scripts/image_signing/resign_firmwarefd.sh:70: 
You might want at least define the version as a file scope variable to make it
easier to modify it later (or maybe make a command line parameter, etc.)

http://codereview.chromium.org/3026018/diff/2002/7003#newcode79
scripts/image_signing/resign_firmwarefd.sh:79: --keyblock "${firmware_keyblock}"
\
just out of curiosity: aren't FW A and B supposed to be identical?

[gauravsh, 2010-07-27 01:28:19]

On Mon, Jul 26, 2010 at 5:13 PM,  <vbendeb@chromium.org> wrote:
> Gaurav, sorry for the delay.
>
> Some comments are duplicated as they seemed to be lost, but they actually
> were
> attached to the earlier versions.
>
> Most of the comments are FYI, but I you should replace curly brackets when
> grouping bash commands.
>
>
> http://codereview.chromium.org/3026018/diff/3001/4003
> File scripts/image_signing/resign_firmwarefd.sh (right):
>
> http://codereview.chromium.org/3026018/diff/3001/4003#newcode7
> scripts/image_signing/resign_firmwarefd.sh:7: # Script to resign an
> firmware image using a different set of keys
> s/an f/a f/
>
Done.

> Also, it would be good to describe the expected command parameters here
> if not in the usage output.

Done.

>
> http://codereview.chromium.org/3026018/diff/3001/4003#newcode17
> scripts/image_signing/resign_firmwarefd.sh:17: echo "Usage: $0 src_fd
> dst_fd firmware_datakey firmware_keyblock kernel_subkey"
> 80 col limit

Done.

>
> http://codereview.chromium.org/3026018/diff/3001/4003#newcode22
> scripts/image_signing/resign_firmwarefd.sh:22: type -P fmap_decode
> &>/dev/null || \
> Just a thought:
>
> could be compacted using
>
> for p in fmap_decode vbutil_firmware; do
>  type -P "${p}" &>/dev/null  || \
>  { echo "${p} tool not found."; exit 1; }
> done
>

Done.
> http://codereview.chromium.org/3026018/diff/2002/7003#newcode17
> scripts/image_signing/resign_firmwarefd.sh:17: echo "Usage: $0 src_fd
> dst_fd firmware_datakey firmware_keyblock kernel_subkey"
> 80 col limit

Done.

>
> http://codereview.chromium.org/3026018/diff/2002/7003#newcode22
> scripts/image_signing/resign_firmwarefd.sh:22: type -P fmap_decode
> &>/dev/null || \
> This could be made easily expendable if expressed through
>
> for f in fmap_decode vbutil_firmware; do
>  type -P $f &>/dev/null || \
>   ( echo "$f tool not found."; exit 1; )
> done

Done.

>
> http://codereview.chromium.org/3026018/diff/2002/7003#newcode23
> scripts/image_signing/resign_firmwarefd.sh:23: { echo "fmap_decode tool
> not found."; exit 1; }
> these should be regular parens, not curly brackets
>

Ok, this is something I am confused about. () would cause exit 1 to be
invoked in a separate subshell. Because of "set -e", we will still
exit the top level script, but why is using {} bad form?


> http://codereview.chromium.org/3026018/diff/2002/7003#newcode58
> scripts/image_signing/resign_firmwarefd.sh:58: vbutil_firmware \
> why do you need two traps here?

Replaced with a single trap.

>
> http://codereview.chromium.org/3026018/diff/2002/7003#newcode70
> scripts/image_signing/resign_firmwarefd.sh:70:
> You might want at least define the version as a file scope variable to
> make it easier to modify it later (or maybe make a command line
> parameter, etc.)
>

Done.

> http://codereview.chromium.org/3026018/diff/2002/7003#newcode79
> scripts/image_signing/resign_firmwarefd.sh:79: --keyblock
> "${firmware_keyblock}" \
> just out of curiosity: aren't FW A and B supposed to be identical?

Logically, they are. But there is a possibility they may contain
separate firmware ids (to distinguish between them), or some other
data - so the script makes no assumptions.

>
> http://codereview.chromium.org/3026018/show
>



-- 
-g

[vb, 2010-07-27 17:40:15]

LGTM

Thank you for addressing my concerns. A few more nits, and you were right, curly
brackets are just fine in this case.

http://codereview.chromium.org/3026018/diff/15001/14004
File scripts/image_signing/resign_firmwarefd.sh (right):

http://codereview.chromium.org/3026018/diff/15001/14004#newcode30
scripts/image_signing/resign_firmwarefd.sh:30: # area_offset="0x001c0000"
area_size="0x00040000" area_name="Boot Stub" area_flags_raw="0x01"
area_flags="static"
please wrap the long lines.

http://codereview.chromium.org/3026018/diff/15001/14004#newcode50
scripts/image_signing/resign_firmwarefd.sh:50: "kernel_subkey"
a space got lost in transition