[OE-core] [meta-oe,v2] kernel-fitimage: introduce FIT_HASH_ALG

[Luca Boccassi]

On Wed, 2019-06-19 at 10:06 +0100, Alex Kiernan wrote:
> On Wed, Jun 19, 2019 at 9:48 AM Luca Boccassi <
> luca.boccassi at gmail.com
> > wrote:
> > On Tue, 2019-06-18 at 17:11 +0100, Richard Purdie wrote:
> > > On Tue, 2019-06-18 at 08:53 -0700, Khem Raj wrote:
> > > > On Tue, Jun 18, 2019 at 1:26 AM Luca Boccassi <
> > > > luca.boccassi at gmail.com
> > > > 
> > > > > wrote:
> > > > > On Thu, 2017-11-02 at 16:48 +0100, Ayoub Zaki wrote:
> > > > > > sanitize fitImage hash algorithm selection with
> > > > > > FIT_HASH_ALG
> > > > > > switch default hash algorithm from sha1 to sha256
> > > > > > 
> > > > > > Signed-off-by: Ayoub Zaki <
> > > > > > ayoub.zaki at embexus.com
> > > > > > 
> > > > > > 
> > > > > > Acked-by: Denys Dmytriyenko <
> > > > > > denys at ti.com
> > > > > > 
> > > > > > 
> > > > > > ---
> > > > > >  meta/classes/kernel-fitimage.bbclass | 13 ++++++++-----
> > > > > >  1 file changed, 8 insertions(+), 5 deletions(-)
> > > > > > 
> > > > > > diff --git a/meta/classes/kernel-fitimage.bbclass
> > > > > > b/meta/classes/kernel-fitimage.bbclass
> > > > > > index 179185b..3cc3a33 100644
> > > > > > --- a/meta/classes/kernel-fitimage.bbclass
> > > > > > +++ b/meta/classes/kernel-fitimage.bbclass
> > > > > > @@ -36,6 +36,9 @@ python __anonymous () {
> > > > > >  # Options for the device tree compiler passed to mkimage
> > > > > > '-D'
> > > > > > feature:
> > > > > >  UBOOT_MKIMAGE_DTCOPTS ??= ""
> > > > > > 
> > > > > > +# fitImage Hash Algo
> > > > > > +FIT_HASH_ALG ?= "sha256"
> 
> Should this be ??=
> 
> > > > > > +
> > > > > >  #
> > > > > >  # Emit the fitImage ITS header
> > > > > >  #
> > > > > > @@ -95,7 +98,7 @@ EOF
> > > > > >  # $4 ... Compression type
> > > > > >  fitimage_emit_section_kernel() {
> > > > > > 
> > > > > > -     kernel_csum="sha1"
> > > > > > +     kernel_csum="${FIT_HASH_ALG}"
> > > > > > 
> > > > > >       ENTRYPOINT=${UBOOT_ENTRYPOINT}
> > > > > >       if [ -n "${UBOOT_ENTRYSYMBOL}" ]; then
> > > > > > @@ -128,7 +131,7 @@ EOF
> > > > > >  # $3 ... Path to DTB image
> > > > > >  fitimage_emit_section_dtb() {
> > > > > > 
> > > > > > -     dtb_csum="sha1"
> > > > > > +     dtb_csum="${FIT_HASH_ALG}"
> > > > > > 
> > > > > >       cat << EOF >> ${1}
> > > > > >                  fdt@${2} {
> > > > > > @@ -152,7 +155,7 @@ EOF
> > > > > >  # $3 ... Path to setup image
> > > > > >  fitimage_emit_section_setup() {
> > > > > > 
> > > > > > -     setup_csum="sha1"
> > > > > > +     setup_csum="${FIT_HASH_ALG}"
> > > > > > 
> > > > > >       cat << EOF >> ${1}
> > > > > >                  setup@${2} {
> > > > > > @@ -179,7 +182,7 @@ EOF
> > > > > >  # $3 ... Path to ramdisk image
> > > > > >  fitimage_emit_section_ramdisk() {
> > > > > > 
> > > > > > -     ramdisk_csum="sha1"
> > > > > > +     ramdisk_csum="${FIT_HASH_ALG}"
> > > > > >       ramdisk_ctype="none"
> > > > > >       ramdisk_loadline=""
> > > > > >       ramdisk_entryline=""
> > > > > > @@ -237,7 +240,7 @@ EOF
> > > > > >  # $6 ... default flag
> > > > > >  fitimage_emit_section_config() {
> > > > > > 
> > > > > > -     conf_csum="sha1"
> > > > > > +     conf_csum="${FIT_HASH_ALG}"
> > > > > >       if [ -n "${UBOOT_SIGN_ENABLE}" ] ; then
> > > > > >               conf_sign_keyname="${UBOOT_SIGN_KEYNAME}"
> > > > > >       fi
> > > > > 
> > > > > Hi,
> > > > > 
> > > > > Any update on this patch? It was acked almost 2 years ago.
> > > > > 
> > > > > It would be great to have a way to change the hashsum
> > > > > algorithm
> > > > > when
> > > > > building signed images.
> > > > > 
> > > > 
> > > > I agree, but it would be good to resend this patch on top of
> > > > current
> > > > master
> > > 
> > > I managed to apply it to master so its in testing now.
> > > 
> > > My concerns about the lack of tests for this class are the main
> > > reason
> > > patches here get held up.
> > > 
> > > Cheers,
> > > 
> > > Richard
> > 
> > Thanks Richard - in case it helps, we've been using this patch for
> > a
> > few months at $work (on top of sumo) and it seems to work fine.
> > 
> 
> I expect that's true for everyone touching stuff in this class - it
> works fine for them... the problem is everyone else, since it's
> terribly fragile and has no tests.
> 
> I'd not be at all surprised if changing the default from sha1 to
> sha256 doesn't break someone.

Keeping the default to sha1 would be fine by me, it's the ability to
configure it that is useful.

-- 
Kind regards,
Luca Boccassi