> -----Original Message-----
> From: linux-tegra-ow...@vger.kernel.org [mailto:linux-tegra-
> ow...@vger.kernel.org] On Behalf Of Stephen Warren
> Sent: Thursday, October 08, 2015 1:58 PM
> To: Jimmy Zhang
> Cc: Allen Martin; Stephen Warren; linux-tegra@vger.kernel.org
> Subject: Re: [cbootimage PATCH v3 5/5] Add two sample scripts to do rsa
> signing for T210 bootimage
> 
> On 10/08/2015 01:38 PM, Jimmy Zhang wrote:
> > sign.sh runs openssl and other linux utilities to generate rsa-pss
> > signatures for a prebuilt bootimage and inject signatures and rsa
> > modulus into bct directly.
> >
> > Syntax: sign.sh <bootimage> <rsa_key.pem>
> >
> > sign-by-update.sh is similar to sign.sh. The difference is the
> > signatures update are done by cbootimage with configuration keywords
> > "RsaKeyModulusFile", "RsaPssSigBlFile", and "RsaPssSigBctFile".
> > Comparing to sign.sh, this script is relatively simple to be ported to
> > T124/T114.
> >
> > Syntax: sign-by-update.sh <bootimage> <rsa_key.pem>
> 
> > diff --git a/rsa_priv.pem b/rsa_priv.pem
> 
> I hope this is some random private key you generated just for the purposes
> of demonstration...
> 

This key is generated by openssl. It is used on my fused board. I will replace 
it with another random generated 2048 bit long modulus private key file.

> > diff --git a/sign-by-update.sh b/sign-by-update.sh
> 
> Let's put these example files in an examples directory or something like that.
> 
> Should we update the Makefile to install the examples into some doc
> directory?
> 

I am not sure how to change Makefile.

> > new file mode 100755
> > index 000000000000..b3f010a41d0e
> > --- /dev/null
> > +++ b/sign-by-update.sh
> > @@ -0,0 +1,59 @@
> > +IMAGE_FILE=$1
> > +KEY_FILE=$2
> 
> There's no #! line here.
> 

Will do.

> I'd suggest adding "set -e" so there is some simple error-checking.
> 
> > +echo " Get rid of all temporary files: *.sig, *.tosig, *.tmp *.mod *.rev"
> 
> Why a space at the start of the echo'd data? (Or the end in other
> commands) Quotes aren't needed either, at least for this command.
> Similar comments for all the other echo statements.
> 

OK. Will clean it up.

> > +echo " Reverse bl signature to meet tegra soc signature ordering"
> > +$OBJCOPY -I binary --reverse-bytes=256 $IMAGE_FILE.bl.sig
> > +$IMAGE_FILE.bl.sig.rev
> 
> Should cbootimage do this itself; this feels like an issue related to packing 
> the
> data into the BCT which is what cbootimage handles...
> 


OK. I will add a function to handle this issue.

> > +echo " Create public key modulus from key file $KEY_FILE and save to
> $KEY_FILE.mod"
> > +$OPENSSL rsa -in $KEY_FILE -noout -modulus -out $KEY_FILE.mod #
> > +remove prefix and LF
> 
> -noout then -out?
> 

No. They are different options. Without -noout, private key is printed to 
output file as well.

> > +$DD bs=1 if=$KEY_FILE.mod of=$KEY_FILE.mod.tmp skip=8 count=512
> 
> I'd suggest using cut for that in case the prefix changes; `cut -d= f2`.
> 

Not sure how to use 'cut'. Instead, will use 'sed'

> > diff --git a/sign.sh b/sign.sh
> 
> Likely all the comments for sign-by-update.sh apply here too.
> 
> I expect these scripts are very similar. Can the script take a cmdline 
> argument
> to request the update type (dd vs. a all to cbootimage -u) so that all the
> common logic isn't duplicated?
> 
> > +echo " Copy the signed binary to the target file $TARGET_IMAGE"
> > +$MV $IMAGE_FILE.tmp $TARGET_IMAGE
> > +
> 
> There's a blank line at EOF there.
> 

Will remove it.

> --
> To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the
> body of a message to majord...@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to