| Message ID | 1391924096-13253-4-git-send-email-hs@denx.de |
|---|---|
| State | Deferred |
| Delegated to: | Tom Rini |
| Headers | show |
Hi Heiko, On 8 February 2014 22:34, Heiko Schocher <hs@denx.de> wrote: > add sha256 support to fit images > > Signed-off-by: Heiko Schocher <hs@denx.de> > Acked-by: Simon Glass <sjg@chromium.org> Sorry I spotted a few things since. > > --- > changes for v2: > - add Acked-by from Simon Glass > > common/image-fit.c | 5 +++++ > include/image.h | 9 +++++++++ > lib/sha256.c | 2 +- > tools/Makefile | 3 +++ > 4 files changed, 18 insertions(+), 1 deletion(-) > > diff --git a/common/image-fit.c b/common/image-fit.c > index cf4b67e..f32feb6 100644 > --- a/common/image-fit.c > +++ b/common/image-fit.c > @@ -22,6 +22,7 @@ DECLARE_GLOBAL_DATA_PTR; > > #include <bootstage.h> > #include <sha1.h> > +#include <sha256.h> > #include <u-boot/crc.h> > #include <u-boot/md5.h> > > @@ -882,6 +883,10 @@ int calculate_hash(const void *data, int data_len, const char *algo, > sha1_csum_wd((unsigned char *)data, data_len, > (unsigned char *)value, CHUNKSZ_SHA1); > *value_len = 20; > + } else if (IMAGE_ENABLE_SHA256 && strcmp(algo, "sha256") == 0) { > + sha256_csum_wd((unsigned char *)data, data_len, > + (unsigned char *)value, CHUNKSZ_SHA256); > + *value_len = SHA256_SUM_LEN; > } else if (IMAGE_ENABLE_MD5 && strcmp(algo, "md5") == 0) { > md5_wd((unsigned char *)data, data_len, value, CHUNKSZ_MD5); > *value_len = 16; > diff --git a/include/image.h b/include/image.h > index 7de2bb2..f001a5f 100644 > --- a/include/image.h > +++ b/include/image.h > @@ -57,13 +57,18 @@ struct lmb; > # ifdef CONFIG_SPL_SHA1_SUPPORT > # define IMAGE_ENABLE_SHA1 1 > # endif > +# ifdef CONFIG_SPL_SHA256_SUPPORT > +# define IMAGE_ENABLE_SHA256 1 > +# endif > # else > # define CONFIG_CRC32 /* FIT images need CRC32 support */ > # define CONFIG_MD5 /* and MD5 */ > # define CONFIG_SHA1 /* and SHA1 */ > +# define CONFIG_SHA256 /* and SHA256 */ Thinking about this again, I wonder if we want to force SHA256 to be enabled when FIT is used? Should we just hold the existing CONFIG_SHA256 setting in the board file and change: > # define IMAGE_ENABLE_CRC32 1 > # define IMAGE_ENABLE_MD5 1 > # define IMAGE_ENABLE_SHA1 1 > +# define IMAGE_ENABLE_SHA256 1 this to: #ifdef CONFIG_SHA256 +# define IMAGE_ENABLE_SHA256 0 #endif ? > # endif > > #ifndef IMAGE_ENABLE_CRC32 > @@ -78,6 +83,10 @@ struct lmb; > #define IMAGE_ENABLE_SHA1 0 > #endif > > +#ifndef IMAGE_ENABLE_SHA256 > +#define IMAGE_ENABLE_SHA256 0 > +#endif > + > #endif /* CONFIG_FIT */ > > #ifdef CONFIG_SYS_BOOT_RAMDISK_HIGH > diff --git a/lib/sha256.c b/lib/sha256.c > index 7348162..5766de2 100644 > --- a/lib/sha256.c > +++ b/lib/sha256.c > @@ -258,7 +258,7 @@ void sha256_csum_wd(const unsigned char *input, unsigned int ilen, > { > sha256_context ctx; > #if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG) > - unsigned char *end, *curr; > + const unsigned char *end, *curr; Why remove the const here? > int chunk; > #endif > > diff --git a/tools/Makefile b/tools/Makefile > index 328cea3..5e36e5e 100644 > --- a/tools/Makefile > +++ b/tools/Makefile > @@ -71,6 +71,7 @@ EXT_OBJ_FILES-y += common/image-sig.o > EXT_OBJ_FILES-y += lib/crc32.o > EXT_OBJ_FILES-y += lib/md5.o > EXT_OBJ_FILES-y += lib/sha1.o > +EXT_OBJ_FILES-y += lib/sha256.o > > # Source files located in the tools directory > NOPED_OBJ_FILES-y += aisimage.o > @@ -223,6 +224,7 @@ $(obj)dumpimage$(SFX): $(obj)aisimage.o \ > $(obj)os_support.o \ > $(obj)pblimage.o \ > $(obj)sha1.o \ > + $(obj)sha256.o \ > $(obj)ublimage.o \ > $(LIBFDT_OBJS) \ > $(RSA_OBJS) > @@ -252,6 +254,7 @@ $(obj)mkimage$(SFX): $(obj)aisimage.o \ > $(obj)os_support.o \ > $(obj)pblimage.o \ > $(obj)sha1.o \ > + $(obj)sha256.o \ > $(obj)ublimage.o \ > $(LIBFDT_OBJS) \ > $(RSA_OBJS) > -- > 1.8.3.1 > > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot Regards, Simon
Hello Simon, Am 15.02.2014 23:47, schrieb Simon Glass: > Hi Heiko, > > On 8 February 2014 22:34, Heiko Schocher<hs@denx.de> wrote: >> add sha256 support to fit images >> >> Signed-off-by: Heiko Schocher<hs@denx.de> >> Acked-by: Simon Glass<sjg@chromium.org> > > Sorry I spotted a few things since. No problem! Thanks for your review. >> --- >> changes for v2: >> - add Acked-by from Simon Glass >> >> common/image-fit.c | 5 +++++ >> include/image.h | 9 +++++++++ >> lib/sha256.c | 2 +- >> tools/Makefile | 3 +++ >> 4 files changed, 18 insertions(+), 1 deletion(-) >> [...] >> diff --git a/include/image.h b/include/image.h >> index 7de2bb2..f001a5f 100644 >> --- a/include/image.h >> +++ b/include/image.h >> @@ -57,13 +57,18 @@ struct lmb; >> # ifdef CONFIG_SPL_SHA1_SUPPORT >> # define IMAGE_ENABLE_SHA1 1 >> # endif >> +# ifdef CONFIG_SPL_SHA256_SUPPORT >> +# define IMAGE_ENABLE_SHA256 1 >> +# endif >> # else >> # define CONFIG_CRC32 /* FIT images need CRC32 support */ >> # define CONFIG_MD5 /* and MD5 */ >> # define CONFIG_SHA1 /* and SHA1 */ >> +# define CONFIG_SHA256 /* and SHA256 */ > > Thinking about this again, I wonder if we want to force SHA256 to be > enabled when FIT is used? Should we just hold the existing > CONFIG_SHA256 setting in the board file and change: I can do this, but why are the others fix? >> # define IMAGE_ENABLE_CRC32 1 >> # define IMAGE_ENABLE_MD5 1 >> # define IMAGE_ENABLE_SHA1 1 >> +# define IMAGE_ENABLE_SHA256 1 > > this to: > > #ifdef CONFIG_SHA256 > +# define IMAGE_ENABLE_SHA256 0 > #endif > > ? Ok, changed, into: #ifdef CONFIG_SHA256 # define IMAGE_ENABLE_SHA256 1 #endif >> # endif >> >> #ifndef IMAGE_ENABLE_CRC32 >> @@ -78,6 +83,10 @@ struct lmb; >> #define IMAGE_ENABLE_SHA1 0 >> #endif >> >> +#ifndef IMAGE_ENABLE_SHA256 >> +#define IMAGE_ENABLE_SHA256 0 >> +#endif >> + >> #endif /* CONFIG_FIT */ >> >> #ifdef CONFIG_SYS_BOOT_RAMDISK_HIGH >> diff --git a/lib/sha256.c b/lib/sha256.c >> index 7348162..5766de2 100644 >> --- a/lib/sha256.c >> +++ b/lib/sha256.c >> @@ -258,7 +258,7 @@ void sha256_csum_wd(const unsigned char *input, unsigned int ilen, >> { >> sha256_context ctx; >> #if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG) >> - unsigned char *end, *curr; >> + const unsigned char *end, *curr; > > Why remove the const here? I add const here ... I remve the "const" for the "curr" pointer. >> int chunk; >> #endif >> >> diff --git a/tools/Makefile b/tools/Makefile >> index 328cea3..5e36e5e 100644 >> --- a/tools/Makefile >> +++ b/tools/Makefile >> @@ -71,6 +71,7 @@ EXT_OBJ_FILES-y += common/image-sig.o >> EXT_OBJ_FILES-y += lib/crc32.o >> EXT_OBJ_FILES-y += lib/md5.o >> EXT_OBJ_FILES-y += lib/sha1.o >> +EXT_OBJ_FILES-y += lib/sha256.o >> >> # Source files located in the tools directory >> NOPED_OBJ_FILES-y += aisimage.o >> @@ -223,6 +224,7 @@ $(obj)dumpimage$(SFX): $(obj)aisimage.o \ >> $(obj)os_support.o \ >> $(obj)pblimage.o \ >> $(obj)sha1.o \ >> + $(obj)sha256.o \ >> $(obj)ublimage.o \ >> $(LIBFDT_OBJS) \ >> $(RSA_OBJS) >> @@ -252,6 +254,7 @@ $(obj)mkimage$(SFX): $(obj)aisimage.o \ >> $(obj)os_support.o \ >> $(obj)pblimage.o \ >> $(obj)sha1.o \ >> + $(obj)sha256.o \ >> $(obj)ublimage.o \ >> $(LIBFDT_OBJS) \ >> $(RSA_OBJS) >> -- >> 1.8.3.1 >> >> _______________________________________________ >> U-Boot mailing list >> U-Boot@lists.denx.de >> http://lists.denx.de/mailman/listinfo/u-boot > > Regards, > Simon > > bye, Heiko
Hello Simon, I was a little to fast with my answer ... Am 17.02.2014 07:33, schrieb Heiko Schocher: > Hello Simon, > > Am 15.02.2014 23:47, schrieb Simon Glass: >> Hi Heiko, >> >> On 8 February 2014 22:34, Heiko Schocher<hs@denx.de> wrote: >>> add sha256 support to fit images >>> >>> Signed-off-by: Heiko Schocher<hs@denx.de> >>> Acked-by: Simon Glass<sjg@chromium.org> >> >> Sorry I spotted a few things since. > > No problem! Thanks for your review. > >>> --- >>> changes for v2: >>> - add Acked-by from Simon Glass >>> >>> common/image-fit.c | 5 +++++ >>> include/image.h | 9 +++++++++ >>> lib/sha256.c | 2 +- >>> tools/Makefile | 3 +++ >>> 4 files changed, 18 insertions(+), 1 deletion(-) >>> > [...] >>> diff --git a/include/image.h b/include/image.h >>> index 7de2bb2..f001a5f 100644 >>> --- a/include/image.h >>> +++ b/include/image.h >>> @@ -57,13 +57,18 @@ struct lmb; >>> # ifdef CONFIG_SPL_SHA1_SUPPORT >>> # define IMAGE_ENABLE_SHA1 1 >>> # endif >>> +# ifdef CONFIG_SPL_SHA256_SUPPORT >>> +# define IMAGE_ENABLE_SHA256 1 >>> +# endif >>> # else >>> # define CONFIG_CRC32 /* FIT images need CRC32 support */ >>> # define CONFIG_MD5 /* and MD5 */ >>> # define CONFIG_SHA1 /* and SHA1 */ >>> +# define CONFIG_SHA256 /* and SHA256 */ >> >> Thinking about this again, I wonder if we want to force SHA256 to be >> enabled when FIT is used? Should we just hold the existing >> CONFIG_SHA256 setting in the board file and change: > > I can do this, but why are the others fix? Hmm.. if I do this, mkimage does not work, as missing this "IMAGE_ENABLE_SHA256" define ... I get the following error message: "Unsupported hash algorithm (sha256) for 'hash@1' hash node in 'U-BOOT@1' image node tools/mkimage Can't add hashes to FIT blob" I think, when compiling mkimage for the host, we have no board config header availiable, as mkimage is not compiled (and not intent to be) board specific ... Seems that this is the reason, why the other algorithms are also always enabled ... bye, Heiko
Hi Heiko, On 16 February 2014 23:49, Heiko Schocher <hs@denx.de> wrote: > Hello Simon, > > I was a little to fast with my answer ... > > Am 17.02.2014 07:33, schrieb Heiko Schocher: > >> Hello Simon, >> >> Am 15.02.2014 23:47, schrieb Simon Glass: >>> >>> Hi Heiko, >>> >>> On 8 February 2014 22:34, Heiko Schocher<hs@denx.de> wrote: >>>> >>>> add sha256 support to fit images >>>> >>>> Signed-off-by: Heiko Schocher<hs@denx.de> >>>> Acked-by: Simon Glass<sjg@chromium.org> >>> >>> >>> Sorry I spotted a few things since. >> >> >> No problem! Thanks for your review. >> >>>> --- >>>> changes for v2: >>>> - add Acked-by from Simon Glass >>>> >>>> common/image-fit.c | 5 +++++ >>>> include/image.h | 9 +++++++++ >>>> lib/sha256.c | 2 +- >>>> tools/Makefile | 3 +++ >>>> 4 files changed, 18 insertions(+), 1 deletion(-) >>>> >> [...] >>>> >>>> diff --git a/include/image.h b/include/image.h >>>> index 7de2bb2..f001a5f 100644 >>>> --- a/include/image.h >>>> +++ b/include/image.h >>>> @@ -57,13 +57,18 @@ struct lmb; >>>> # ifdef CONFIG_SPL_SHA1_SUPPORT >>>> # define IMAGE_ENABLE_SHA1 1 >>>> # endif >>>> +# ifdef CONFIG_SPL_SHA256_SUPPORT >>>> +# define IMAGE_ENABLE_SHA256 1 >>>> +# endif >>>> # else >>>> # define CONFIG_CRC32 /* FIT images need CRC32 support */ >>>> # define CONFIG_MD5 /* and MD5 */ >>>> # define CONFIG_SHA1 /* and SHA1 */ >>>> +# define CONFIG_SHA256 /* and SHA256 */ >>> >>> >>> Thinking about this again, I wonder if we want to force SHA256 to be >>> enabled when FIT is used? Should we just hold the existing >>> CONFIG_SHA256 setting in the board file and change: >> >> >> I can do this, but why are the others fix? > > > Hmm.. if I do this, mkimage does not work, as missing > this "IMAGE_ENABLE_SHA256" define ... I get the following > error message: > > "Unsupported hash algorithm (sha256) for 'hash@1' hash node in 'U-BOOT@1' > image node > tools/mkimage Can't add hashes to FIT blob" > > I think, when compiling mkimage for the host, we have no board config > header availiable, as mkimage is not compiled (and not intent to be) > board specific ... > > Seems that this is the reason, why the other algorithms are also > always enabled ... Ah yes, I remember this problem - I wanted mkimage to support everything rather than be dependent on what particular board you build with. In the end I dropped the signing part from this idea. So yes your code is correct and I'm sorry for the trouble. Regards, Simon
diff --git a/common/image-fit.c b/common/image-fit.c index cf4b67e..f32feb6 100644 --- a/common/image-fit.c +++ b/common/image-fit.c @@ -22,6 +22,7 @@ DECLARE_GLOBAL_DATA_PTR; #include <bootstage.h> #include <sha1.h> +#include <sha256.h> #include <u-boot/crc.h> #include <u-boot/md5.h> @@ -882,6 +883,10 @@ int calculate_hash(const void *data, int data_len, const char *algo, sha1_csum_wd((unsigned char *)data, data_len, (unsigned char *)value, CHUNKSZ_SHA1); *value_len = 20; + } else if (IMAGE_ENABLE_SHA256 && strcmp(algo, "sha256") == 0) { + sha256_csum_wd((unsigned char *)data, data_len, + (unsigned char *)value, CHUNKSZ_SHA256); + *value_len = SHA256_SUM_LEN; } else if (IMAGE_ENABLE_MD5 && strcmp(algo, "md5") == 0) { md5_wd((unsigned char *)data, data_len, value, CHUNKSZ_MD5); *value_len = 16; diff --git a/include/image.h b/include/image.h index 7de2bb2..f001a5f 100644 --- a/include/image.h +++ b/include/image.h @@ -57,13 +57,18 @@ struct lmb; # ifdef CONFIG_SPL_SHA1_SUPPORT # define IMAGE_ENABLE_SHA1 1 # endif +# ifdef CONFIG_SPL_SHA256_SUPPORT +# define IMAGE_ENABLE_SHA256 1 +# endif # else # define CONFIG_CRC32 /* FIT images need CRC32 support */ # define CONFIG_MD5 /* and MD5 */ # define CONFIG_SHA1 /* and SHA1 */ +# define CONFIG_SHA256 /* and SHA256 */ # define IMAGE_ENABLE_CRC32 1 # define IMAGE_ENABLE_MD5 1 # define IMAGE_ENABLE_SHA1 1 +# define IMAGE_ENABLE_SHA256 1 # endif #ifndef IMAGE_ENABLE_CRC32 @@ -78,6 +83,10 @@ struct lmb; #define IMAGE_ENABLE_SHA1 0 #endif +#ifndef IMAGE_ENABLE_SHA256 +#define IMAGE_ENABLE_SHA256 0 +#endif + #endif /* CONFIG_FIT */ #ifdef CONFIG_SYS_BOOT_RAMDISK_HIGH diff --git a/lib/sha256.c b/lib/sha256.c index 7348162..5766de2 100644 --- a/lib/sha256.c +++ b/lib/sha256.c @@ -258,7 +258,7 @@ void sha256_csum_wd(const unsigned char *input, unsigned int ilen, { sha256_context ctx; #if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG) - unsigned char *end, *curr; + const unsigned char *end, *curr; int chunk; #endif diff --git a/tools/Makefile b/tools/Makefile index 328cea3..5e36e5e 100644 --- a/tools/Makefile +++ b/tools/Makefile @@ -71,6 +71,7 @@ EXT_OBJ_FILES-y += common/image-sig.o EXT_OBJ_FILES-y += lib/crc32.o EXT_OBJ_FILES-y += lib/md5.o EXT_OBJ_FILES-y += lib/sha1.o +EXT_OBJ_FILES-y += lib/sha256.o # Source files located in the tools directory NOPED_OBJ_FILES-y += aisimage.o @@ -223,6 +224,7 @@ $(obj)dumpimage$(SFX): $(obj)aisimage.o \ $(obj)os_support.o \ $(obj)pblimage.o \ $(obj)sha1.o \ + $(obj)sha256.o \ $(obj)ublimage.o \ $(LIBFDT_OBJS) \ $(RSA_OBJS) @@ -252,6 +254,7 @@ $(obj)mkimage$(SFX): $(obj)aisimage.o \ $(obj)os_support.o \ $(obj)pblimage.o \ $(obj)sha1.o \ + $(obj)sha256.o \ $(obj)ublimage.o \ $(LIBFDT_OBJS) \ $(RSA_OBJS)