* [U-Boot] [PATCH v2 0/4] Allow disabling non-FIT image loading from SPL @ 2016-11-14 19:14 Andrew F. Davis 2016-11-14 19:14 ` [U-Boot] [PATCH v2 1/4] Introduce CONFIG_SPL_ABORT_ON_NON_FIT_IMAGE Andrew F. Davis ` (3 more replies) 0 siblings, 4 replies; 13+ messages in thread From: Andrew F. Davis @ 2016-11-14 19:14 UTC (permalink / raw) To: u-boot Hello all, To address a needed feature brought up by Andreas[0], we need a way to disable SPL from loading non-FIT images. The function spl_parse_image_header is common to all SPL loading paths (common/spl/spl_(nand|net|nor|etc..)) so we add the check here, much like the existing CONFIG_SPL_ABORT_ON_RAW_IMAGE. My original attempt was to add CONFIG_SPL_PANIC_ON_MKIMAGE, but then if other formats are added, flaws in restricting image types may be introduced, so we would like a single option to restrict all non-FIT types vs disabling types individually. Thanks, Andrew [0] https://www.mail-archive.com/u-boot at lists.denx.de/msg219253.html Changes from v1: - Changed to abort on non-FIT so other boot methods can be attempted. - Made this a default if SPL_FIT_SIGNATURE is selected. Andrew F. Davis (4): Introduce CONFIG_SPL_ABORT_ON_NON_FIT_IMAGE ARM: AM57xx: Disable non-FIT based image loading for HS devices ARM: AM437x: Disable non-FIT based image loading for HS devices ARM: DRA7xx: Disable non-FIT based image loading for HS devices Kconfig | 9 +++++++++ common/spl/spl.c | 5 +++++ configs/am43xx_hs_evm_defconfig | 1 + configs/am57xx_hs_evm_defconfig | 1 + configs/dra7xx_hs_evm_defconfig | 1 + 5 files changed, 17 insertions(+) -- 2.10.1 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v2 1/4] Introduce CONFIG_SPL_ABORT_ON_NON_FIT_IMAGE 2016-11-14 19:14 [U-Boot] [PATCH v2 0/4] Allow disabling non-FIT image loading from SPL Andrew F. Davis @ 2016-11-14 19:14 ` Andrew F. Davis 2016-11-14 20:44 ` Simon Glass 2016-11-14 19:14 ` [U-Boot] [PATCH v2 2/4] ARM: AM57xx: Disable non-FIT based image loading for HS devices Andrew F. Davis ` (2 subsequent siblings) 3 siblings, 1 reply; 13+ messages in thread From: Andrew F. Davis @ 2016-11-14 19:14 UTC (permalink / raw) To: u-boot Introduce CONFIG_SPL_ABORT_ON_NON_FIT_IMAGE. An SPL which define this will abort image loading if the image is not a FIT image. Signed-off-by: Andrew F. Davis <afd@ti.com> --- Kconfig | 9 +++++++++ common/spl/spl.c | 5 +++++ 2 files changed, 14 insertions(+) diff --git a/Kconfig b/Kconfig index 1263d0b..eefebef 100644 --- a/Kconfig +++ b/Kconfig @@ -291,6 +291,15 @@ config FIT_IMAGE_POST_PROCESS injected into the FIT creation (i.e. the blobs would have been pre- processed before being added to the FIT image). +config SPL_ABORT_ON_NON_FIT_IMAGE + bool "Disable SPL loading of non-FIT images" + default y if SPL_FIT_SIGNATURE + help + SPL will not load and image if it is not a FIT image. This is + useful for devices that only support authentication/encryption + through SPL FIT loading paths and do not want SPL falling back + to legacy image loading when a non-FIT image is present. + config SPL_DFU_SUPPORT bool "Enable SPL with DFU to load binaries to memory device" depends on USB diff --git a/common/spl/spl.c b/common/spl/spl.c index bdb165a..3d8bee9 100644 --- a/common/spl/spl.c +++ b/common/spl/spl.c @@ -93,6 +93,10 @@ void spl_set_header_raw_uboot(struct spl_image_info *spl_image) int spl_parse_image_header(struct spl_image_info *spl_image, const struct image_header *header) { +#ifdef CONFIG_SPL_ABORT_ON_NON_FIT_IMAGE + /* non-FIT image found, proceed to other boot methods. */ + return -EINVAL; +#else u32 header_size = sizeof(struct image_header); if (image_get_magic(header) == IH_MAGIC) { @@ -156,6 +160,7 @@ int spl_parse_image_header(struct spl_image_info *spl_image, spl_set_header_raw_uboot(spl_image); #endif } +#endif return 0; } -- 2.10.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v2 1/4] Introduce CONFIG_SPL_ABORT_ON_NON_FIT_IMAGE 2016-11-14 19:14 ` [U-Boot] [PATCH v2 1/4] Introduce CONFIG_SPL_ABORT_ON_NON_FIT_IMAGE Andrew F. Davis @ 2016-11-14 20:44 ` Simon Glass 2016-11-14 22:05 ` Andrew F. Davis 0 siblings, 1 reply; 13+ messages in thread From: Simon Glass @ 2016-11-14 20:44 UTC (permalink / raw) To: u-boot Hi Andrew, On 14 November 2016 at 12:14, Andrew F. Davis <afd@ti.com> wrote: > Introduce CONFIG_SPL_ABORT_ON_NON_FIT_IMAGE. An SPL which define > this will abort image loading if the image is not a FIT image. > > Signed-off-by: Andrew F. Davis <afd@ti.com> > --- > Kconfig | 9 +++++++++ > common/spl/spl.c | 5 +++++ > 2 files changed, 14 insertions(+) > > diff --git a/Kconfig b/Kconfig > index 1263d0b..eefebef 100644 > --- a/Kconfig > +++ b/Kconfig > @@ -291,6 +291,15 @@ config FIT_IMAGE_POST_PROCESS > injected into the FIT creation (i.e. the blobs would have been pre- > processed before being added to the FIT image). > > +config SPL_ABORT_ON_NON_FIT_IMAGE We already have CONFIG_IMAGE_FORMAT_LEGACY so how about CONFIG_SPL_IMAGE_FORMAT_LEGACY instead? It can default to y if secure boot is disabled. > + bool "Disable SPL loading of non-FIT images" > + default y if SPL_FIT_SIGNATURE > + help > + SPL will not load and image if it is not a FIT image. This is > + useful for devices that only support authentication/encryption > + through SPL FIT loading paths and do not want SPL falling back > + to legacy image loading when a non-FIT image is present. > + > config SPL_DFU_SUPPORT > bool "Enable SPL with DFU to load binaries to memory device" > depends on USB > diff --git a/common/spl/spl.c b/common/spl/spl.c > index bdb165a..3d8bee9 100644 > --- a/common/spl/spl.c > +++ b/common/spl/spl.c > @@ -93,6 +93,10 @@ void spl_set_header_raw_uboot(struct spl_image_info *spl_image) > int spl_parse_image_header(struct spl_image_info *spl_image, > const struct image_header *header) > { > +#ifdef CONFIG_SPL_ABORT_ON_NON_FIT_IMAGE > + /* non-FIT image found, proceed to other boot methods. */ > + return -EINVAL; How about -EPROTONOSUPPORT since the request is not really invalid. > +#else > u32 header_size = sizeof(struct image_header); > > if (image_get_magic(header) == IH_MAGIC) { > @@ -156,6 +160,7 @@ int spl_parse_image_header(struct spl_image_info *spl_image, > spl_set_header_raw_uboot(spl_image); > #endif > } > +#endif > return 0; > } > > -- > 2.10.1 > Regards, Simon ^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v2 1/4] Introduce CONFIG_SPL_ABORT_ON_NON_FIT_IMAGE 2016-11-14 20:44 ` Simon Glass @ 2016-11-14 22:05 ` Andrew F. Davis 2016-11-15 0:33 ` Simon Glass 0 siblings, 1 reply; 13+ messages in thread From: Andrew F. Davis @ 2016-11-14 22:05 UTC (permalink / raw) To: u-boot On 11/14/2016 02:44 PM, Simon Glass wrote: > Hi Andrew, > > On 14 November 2016 at 12:14, Andrew F. Davis <afd@ti.com> wrote: >> Introduce CONFIG_SPL_ABORT_ON_NON_FIT_IMAGE. An SPL which define >> this will abort image loading if the image is not a FIT image. >> >> Signed-off-by: Andrew F. Davis <afd@ti.com> >> --- >> Kconfig | 9 +++++++++ >> common/spl/spl.c | 5 +++++ >> 2 files changed, 14 insertions(+) >> >> diff --git a/Kconfig b/Kconfig >> index 1263d0b..eefebef 100644 >> --- a/Kconfig >> +++ b/Kconfig >> @@ -291,6 +291,15 @@ config FIT_IMAGE_POST_PROCESS >> injected into the FIT creation (i.e. the blobs would have been pre- >> processed before being added to the FIT image). >> >> +config SPL_ABORT_ON_NON_FIT_IMAGE > > We already have CONFIG_IMAGE_FORMAT_LEGACY so how about > CONFIG_SPL_IMAGE_FORMAT_LEGACY instead? It can default to y if secure > boot is disabled. > We also already have CONFIG_SPL_ABORT_ON_RAW_IMAGE on which this is based. If we only disable legacy image support then RAW images should still be allowed, but we will fail early anyway, we will start to need an unmaintainable amount of pre-processor logic to to handle the different image types and what is allowed/not allowed. Even worse some boot modes don't seem to support FIT images (net, onenand) so these will alway expect legacy to work. Right now we simply have to disable these modes. >> + bool "Disable SPL loading of non-FIT images" >> + default y if SPL_FIT_SIGNATURE >> + help >> + SPL will not load and image if it is not a FIT image. This is >> + useful for devices that only support authentication/encryption >> + through SPL FIT loading paths and do not want SPL falling back >> + to legacy image loading when a non-FIT image is present. >> + >> config SPL_DFU_SUPPORT >> bool "Enable SPL with DFU to load binaries to memory device" >> depends on USB >> diff --git a/common/spl/spl.c b/common/spl/spl.c >> index bdb165a..3d8bee9 100644 >> --- a/common/spl/spl.c >> +++ b/common/spl/spl.c >> @@ -93,6 +93,10 @@ void spl_set_header_raw_uboot(struct spl_image_info *spl_image) >> int spl_parse_image_header(struct spl_image_info *spl_image, >> const struct image_header *header) >> { >> +#ifdef CONFIG_SPL_ABORT_ON_NON_FIT_IMAGE >> + /* non-FIT image found, proceed to other boot methods. */ >> + return -EINVAL; > > How about -EPROTONOSUPPORT since the request is not really invalid. > >> +#else >> u32 header_size = sizeof(struct image_header); >> >> if (image_get_magic(header) == IH_MAGIC) { >> @@ -156,6 +160,7 @@ int spl_parse_image_header(struct spl_image_info *spl_image, >> spl_set_header_raw_uboot(spl_image); >> #endif >> } >> +#endif >> return 0; >> } >> >> -- >> 2.10.1 >> > > Regards, > Simon > ^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v2 1/4] Introduce CONFIG_SPL_ABORT_ON_NON_FIT_IMAGE 2016-11-14 22:05 ` Andrew F. Davis @ 2016-11-15 0:33 ` Simon Glass 2016-12-05 22:37 ` Andrew F. Davis 0 siblings, 1 reply; 13+ messages in thread From: Simon Glass @ 2016-11-15 0:33 UTC (permalink / raw) To: u-boot Hi Andrew, On 14 November 2016 at 15:05, Andrew F. Davis <afd@ti.com> wrote: > On 11/14/2016 02:44 PM, Simon Glass wrote: >> Hi Andrew, >> >> On 14 November 2016 at 12:14, Andrew F. Davis <afd@ti.com> wrote: >>> Introduce CONFIG_SPL_ABORT_ON_NON_FIT_IMAGE. An SPL which define >>> this will abort image loading if the image is not a FIT image. >>> >>> Signed-off-by: Andrew F. Davis <afd@ti.com> >>> --- >>> Kconfig | 9 +++++++++ >>> common/spl/spl.c | 5 +++++ >>> 2 files changed, 14 insertions(+) >>> >>> diff --git a/Kconfig b/Kconfig >>> index 1263d0b..eefebef 100644 >>> --- a/Kconfig >>> +++ b/Kconfig >>> @@ -291,6 +291,15 @@ config FIT_IMAGE_POST_PROCESS >>> injected into the FIT creation (i.e. the blobs would have been pre- >>> processed before being added to the FIT image). >>> >>> +config SPL_ABORT_ON_NON_FIT_IMAGE >> >> We already have CONFIG_IMAGE_FORMAT_LEGACY so how about >> CONFIG_SPL_IMAGE_FORMAT_LEGACY instead? It can default to y if secure >> boot is disabled. >> > > We also already have CONFIG_SPL_ABORT_ON_RAW_IMAGE on which this is > based. If we only disable legacy image support then RAW images should > still be allowed, but we will fail early anyway, we will start to need > an unmaintainable amount of pre-processor logic to to handle the > different image types and what is allowed/not allowed. > > Even worse some boot modes don't seem to support FIT images (net, > onenand) so these will alway expect legacy to work. Right now we simply > have to disable these modes. IMO CONFIG_SPL_ABORT_ON_RAW_IMAGE should become a positive option, to fit in with the legacy format. Otherwise we'll get very confused I think. > >>> + bool "Disable SPL loading of non-FIT images" >>> + default y if SPL_FIT_SIGNATURE >>> + help >>> + SPL will not load and image if it is not a FIT image. This is >>> + useful for devices that only support authentication/encryption >>> + through SPL FIT loading paths and do not want SPL falling back >>> + to legacy image loading when a non-FIT image is present. >>> + >>> config SPL_DFU_SUPPORT >>> bool "Enable SPL with DFU to load binaries to memory device" >>> depends on USB >>> diff --git a/common/spl/spl.c b/common/spl/spl.c >>> index bdb165a..3d8bee9 100644 >>> --- a/common/spl/spl.c >>> +++ b/common/spl/spl.c >>> @@ -93,6 +93,10 @@ void spl_set_header_raw_uboot(struct spl_image_info *spl_image) >>> int spl_parse_image_header(struct spl_image_info *spl_image, >>> const struct image_header *header) >>> { >>> +#ifdef CONFIG_SPL_ABORT_ON_NON_FIT_IMAGE >>> + /* non-FIT image found, proceed to other boot methods. */ >>> + return -EINVAL; >> >> How about -EPROTONOSUPPORT since the request is not really invalid. >> >>> +#else >>> u32 header_size = sizeof(struct image_header); >>> >>> if (image_get_magic(header) == IH_MAGIC) { >>> @@ -156,6 +160,7 @@ int spl_parse_image_header(struct spl_image_info *spl_image, >>> spl_set_header_raw_uboot(spl_image); >>> #endif >>> } >>> +#endif >>> return 0; >>> } >>> >>> -- >>> 2.10.1 >>> Regards, Simon ^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v2 1/4] Introduce CONFIG_SPL_ABORT_ON_NON_FIT_IMAGE 2016-11-15 0:33 ` Simon Glass @ 2016-12-05 22:37 ` Andrew F. Davis 2016-12-07 3:47 ` Simon Glass 0 siblings, 1 reply; 13+ messages in thread From: Andrew F. Davis @ 2016-12-05 22:37 UTC (permalink / raw) To: u-boot On 11/14/2016 06:33 PM, Simon Glass wrote: > Hi Andrew, > > On 14 November 2016 at 15:05, Andrew F. Davis <afd@ti.com> wrote: >> On 11/14/2016 02:44 PM, Simon Glass wrote: >>> Hi Andrew, >>> >>> On 14 November 2016 at 12:14, Andrew F. Davis <afd@ti.com> wrote: >>>> Introduce CONFIG_SPL_ABORT_ON_NON_FIT_IMAGE. An SPL which define >>>> this will abort image loading if the image is not a FIT image. >>>> >>>> Signed-off-by: Andrew F. Davis <afd@ti.com> >>>> --- >>>> Kconfig | 9 +++++++++ >>>> common/spl/spl.c | 5 +++++ >>>> 2 files changed, 14 insertions(+) >>>> >>>> diff --git a/Kconfig b/Kconfig >>>> index 1263d0b..eefebef 100644 >>>> --- a/Kconfig >>>> +++ b/Kconfig >>>> @@ -291,6 +291,15 @@ config FIT_IMAGE_POST_PROCESS >>>> injected into the FIT creation (i.e. the blobs would have been pre- >>>> processed before being added to the FIT image). >>>> >>>> +config SPL_ABORT_ON_NON_FIT_IMAGE >>> >>> We already have CONFIG_IMAGE_FORMAT_LEGACY so how about >>> CONFIG_SPL_IMAGE_FORMAT_LEGACY instead? It can default to y if secure >>> boot is disabled. >>> >> >> We also already have CONFIG_SPL_ABORT_ON_RAW_IMAGE on which this is >> based. If we only disable legacy image support then RAW images should >> still be allowed, but we will fail early anyway, we will start to need >> an unmaintainable amount of pre-processor logic to to handle the >> different image types and what is allowed/not allowed. >> >> Even worse some boot modes don't seem to support FIT images (net, >> onenand) so these will alway expect legacy to work. Right now we simply >> have to disable these modes. > > IMO CONFIG_SPL_ABORT_ON_RAW_IMAGE should become a positive option, to > fit in with the legacy format. Otherwise we'll get very confused I > think. > I'm not sure what you are suggesting here, would you like CONFIG_SPL_SUPPORT_RAW_IMAGE CONFIG_SPL_SUPPORT_LEGACY_IMAGE CONFIG_SPL_SUPPORT_FIT_IMAGE And then we disable as needed? I'm not sure this will work in our case, as a new image type may be introduced and enabled by default, this will break our board security until we discover this and disabled it. The benefit of a negative option for us is that we can specify we *only* allow FIT, then it will be obvious to someone adding a new image type they will not meet this check and should not put code outside this block. >> >>>> + bool "Disable SPL loading of non-FIT images" >>>> + default y if SPL_FIT_SIGNATURE >>>> + help >>>> + SPL will not load and image if it is not a FIT image. This is >>>> + useful for devices that only support authentication/encryption >>>> + through SPL FIT loading paths and do not want SPL falling back >>>> + to legacy image loading when a non-FIT image is present. >>>> + >>>> config SPL_DFU_SUPPORT >>>> bool "Enable SPL with DFU to load binaries to memory device" >>>> depends on USB >>>> diff --git a/common/spl/spl.c b/common/spl/spl.c >>>> index bdb165a..3d8bee9 100644 >>>> --- a/common/spl/spl.c >>>> +++ b/common/spl/spl.c >>>> @@ -93,6 +93,10 @@ void spl_set_header_raw_uboot(struct spl_image_info *spl_image) >>>> int spl_parse_image_header(struct spl_image_info *spl_image, >>>> const struct image_header *header) >>>> { >>>> +#ifdef CONFIG_SPL_ABORT_ON_NON_FIT_IMAGE >>>> + /* non-FIT image found, proceed to other boot methods. */ >>>> + return -EINVAL; >>> >>> How about -EPROTONOSUPPORT since the request is not really invalid. >>> >>>> +#else >>>> u32 header_size = sizeof(struct image_header); >>>> >>>> if (image_get_magic(header) == IH_MAGIC) { >>>> @@ -156,6 +160,7 @@ int spl_parse_image_header(struct spl_image_info *spl_image, >>>> spl_set_header_raw_uboot(spl_image); >>>> #endif >>>> } >>>> +#endif >>>> return 0; >>>> } >>>> >>>> -- >>>> 2.10.1 >>>> > > Regards, > Simon > ^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v2 1/4] Introduce CONFIG_SPL_ABORT_ON_NON_FIT_IMAGE 2016-12-05 22:37 ` Andrew F. Davis @ 2016-12-07 3:47 ` Simon Glass 2017-02-08 15:18 ` Andrew F. Davis 0 siblings, 1 reply; 13+ messages in thread From: Simon Glass @ 2016-12-07 3:47 UTC (permalink / raw) To: u-boot Hi Andrew, On 5 December 2016 at 17:37, Andrew F. Davis <afd@ti.com> wrote: > On 11/14/2016 06:33 PM, Simon Glass wrote: >> Hi Andrew, >> >> On 14 November 2016 at 15:05, Andrew F. Davis <afd@ti.com> wrote: >>> On 11/14/2016 02:44 PM, Simon Glass wrote: >>>> Hi Andrew, >>>> >>>> On 14 November 2016 at 12:14, Andrew F. Davis <afd@ti.com> wrote: >>>>> Introduce CONFIG_SPL_ABORT_ON_NON_FIT_IMAGE. An SPL which define >>>>> this will abort image loading if the image is not a FIT image. >>>>> >>>>> Signed-off-by: Andrew F. Davis <afd@ti.com> >>>>> --- >>>>> Kconfig | 9 +++++++++ >>>>> common/spl/spl.c | 5 +++++ >>>>> 2 files changed, 14 insertions(+) >>>>> >>>>> diff --git a/Kconfig b/Kconfig >>>>> index 1263d0b..eefebef 100644 >>>>> --- a/Kconfig >>>>> +++ b/Kconfig >>>>> @@ -291,6 +291,15 @@ config FIT_IMAGE_POST_PROCESS >>>>> injected into the FIT creation (i.e. the blobs would have been pre- >>>>> processed before being added to the FIT image). >>>>> >>>>> +config SPL_ABORT_ON_NON_FIT_IMAGE >>>> >>>> We already have CONFIG_IMAGE_FORMAT_LEGACY so how about >>>> CONFIG_SPL_IMAGE_FORMAT_LEGACY instead? It can default to y if secure >>>> boot is disabled. >>>> >>> >>> We also already have CONFIG_SPL_ABORT_ON_RAW_IMAGE on which this is >>> based. If we only disable legacy image support then RAW images should >>> still be allowed, but we will fail early anyway, we will start to need >>> an unmaintainable amount of pre-processor logic to to handle the >>> different image types and what is allowed/not allowed. >>> >>> Even worse some boot modes don't seem to support FIT images (net, >>> onenand) so these will alway expect legacy to work. Right now we simply >>> have to disable these modes. >> >> IMO CONFIG_SPL_ABORT_ON_RAW_IMAGE should become a positive option, to >> fit in with the legacy format. Otherwise we'll get very confused I >> think. >> > > I'm not sure what you are suggesting here, would you like > > CONFIG_SPL_SUPPORT_RAW_IMAGE > CONFIG_SPL_SUPPORT_LEGACY_IMAGE > CONFIG_SPL_SUPPORT_FIT_IMAGE > > And then we disable as needed? I'm not sure this will work in our case, > as a new image type may be introduced and enabled by default, this will > break our board security until we discover this and disabled it. The > benefit of a negative option for us is that we can specify we *only* > allow FIT, then it will be obvious to someone adding a new image type > they will not meet this check and should not put code outside this block. I don't think we add new image types often, and they should default to n just as we do for U-Boot proper. Although something odd has happened with DISABLE_IMAGE_LEGACY. his should of thing should be caught in a security review. Also you could add something to print a warning if secure boot is enabled but insecure boot options are available? Perhaps that should be another option, and default to y? It's just that it is really hard to deal with multiple negative options, so best avoided if we can. Regards, Simon ^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v2 1/4] Introduce CONFIG_SPL_ABORT_ON_NON_FIT_IMAGE 2016-12-07 3:47 ` Simon Glass @ 2017-02-08 15:18 ` Andrew F. Davis 2017-02-10 16:23 ` Simon Glass 0 siblings, 1 reply; 13+ messages in thread From: Andrew F. Davis @ 2017-02-08 15:18 UTC (permalink / raw) To: u-boot On 12/06/2016 09:47 PM, Simon Glass wrote: > Hi Andrew, > > On 5 December 2016 at 17:37, Andrew F. Davis <afd@ti.com> wrote: >> On 11/14/2016 06:33 PM, Simon Glass wrote: >>> Hi Andrew, >>> >>> On 14 November 2016 at 15:05, Andrew F. Davis <afd@ti.com> wrote: >>>> On 11/14/2016 02:44 PM, Simon Glass wrote: >>>>> Hi Andrew, >>>>> >>>>> On 14 November 2016 at 12:14, Andrew F. Davis <afd@ti.com> wrote: >>>>>> Introduce CONFIG_SPL_ABORT_ON_NON_FIT_IMAGE. An SPL which define >>>>>> this will abort image loading if the image is not a FIT image. >>>>>> >>>>>> Signed-off-by: Andrew F. Davis <afd@ti.com> >>>>>> --- >>>>>> Kconfig | 9 +++++++++ >>>>>> common/spl/spl.c | 5 +++++ >>>>>> 2 files changed, 14 insertions(+) >>>>>> >>>>>> diff --git a/Kconfig b/Kconfig >>>>>> index 1263d0b..eefebef 100644 >>>>>> --- a/Kconfig >>>>>> +++ b/Kconfig >>>>>> @@ -291,6 +291,15 @@ config FIT_IMAGE_POST_PROCESS >>>>>> injected into the FIT creation (i.e. the blobs would have been pre- >>>>>> processed before being added to the FIT image). >>>>>> >>>>>> +config SPL_ABORT_ON_NON_FIT_IMAGE >>>>> >>>>> We already have CONFIG_IMAGE_FORMAT_LEGACY so how about >>>>> CONFIG_SPL_IMAGE_FORMAT_LEGACY instead? It can default to y if secure >>>>> boot is disabled. >>>>> >>>> >>>> We also already have CONFIG_SPL_ABORT_ON_RAW_IMAGE on which this is >>>> based. If we only disable legacy image support then RAW images should >>>> still be allowed, but we will fail early anyway, we will start to need >>>> an unmaintainable amount of pre-processor logic to to handle the >>>> different image types and what is allowed/not allowed. >>>> >>>> Even worse some boot modes don't seem to support FIT images (net, >>>> onenand) so these will alway expect legacy to work. Right now we simply >>>> have to disable these modes. >>> >>> IMO CONFIG_SPL_ABORT_ON_RAW_IMAGE should become a positive option, to >>> fit in with the legacy format. Otherwise we'll get very confused I >>> think. >>> >> >> I'm not sure what you are suggesting here, would you like >> >> CONFIG_SPL_SUPPORT_RAW_IMAGE >> CONFIG_SPL_SUPPORT_LEGACY_IMAGE >> CONFIG_SPL_SUPPORT_FIT_IMAGE >> >> And then we disable as needed? I'm not sure this will work in our case, >> as a new image type may be introduced and enabled by default, this will >> break our board security until we discover this and disabled it. The >> benefit of a negative option for us is that we can specify we *only* >> allow FIT, then it will be obvious to someone adding a new image type >> they will not meet this check and should not put code outside this block. > > I don't think we add new image types often, and they should default to > n just as we do for U-Boot proper. Although something odd has happened > with DISABLE_IMAGE_LEGACY. > > his should of thing should be caught in a security review. > > Also you could add something to print a warning if secure boot is > enabled but insecure boot options are available? Perhaps that should > be another option, and default to y? > > It's just that it is really hard to deal with multiple negative > options, so best avoided if we can. > I agree in general, but this time I think this is hard to properly avoid. All that would be different with a positive option only case would be a bunch of checks that all other image modes are off, then block undefining the same code I am here. Andrew > Regards, > Simon > ^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v2 1/4] Introduce CONFIG_SPL_ABORT_ON_NON_FIT_IMAGE 2017-02-08 15:18 ` Andrew F. Davis @ 2017-02-10 16:23 ` Simon Glass 2017-02-10 16:57 ` Andrew F. Davis 0 siblings, 1 reply; 13+ messages in thread From: Simon Glass @ 2017-02-10 16:23 UTC (permalink / raw) To: u-boot Hi Andrew, On 8 February 2017 at 08:18, Andrew F. Davis <afd@ti.com> wrote: > On 12/06/2016 09:47 PM, Simon Glass wrote: >> Hi Andrew, >> >> On 5 December 2016 at 17:37, Andrew F. Davis <afd@ti.com> wrote: >>> On 11/14/2016 06:33 PM, Simon Glass wrote: >>>> Hi Andrew, >>>> >>>> On 14 November 2016 at 15:05, Andrew F. Davis <afd@ti.com> wrote: >>>>> On 11/14/2016 02:44 PM, Simon Glass wrote: >>>>>> Hi Andrew, >>>>>> >>>>>> On 14 November 2016 at 12:14, Andrew F. Davis <afd@ti.com> wrote: >>>>>>> Introduce CONFIG_SPL_ABORT_ON_NON_FIT_IMAGE. An SPL which define >>>>>>> this will abort image loading if the image is not a FIT image. >>>>>>> >>>>>>> Signed-off-by: Andrew F. Davis <afd@ti.com> >>>>>>> --- >>>>>>> Kconfig | 9 +++++++++ >>>>>>> common/spl/spl.c | 5 +++++ >>>>>>> 2 files changed, 14 insertions(+) >>>>>>> >>>>>>> diff --git a/Kconfig b/Kconfig >>>>>>> index 1263d0b..eefebef 100644 >>>>>>> --- a/Kconfig >>>>>>> +++ b/Kconfig >>>>>>> @@ -291,6 +291,15 @@ config FIT_IMAGE_POST_PROCESS >>>>>>> injected into the FIT creation (i.e. the blobs would have been pre- >>>>>>> processed before being added to the FIT image). >>>>>>> >>>>>>> +config SPL_ABORT_ON_NON_FIT_IMAGE >>>>>> >>>>>> We already have CONFIG_IMAGE_FORMAT_LEGACY so how about >>>>>> CONFIG_SPL_IMAGE_FORMAT_LEGACY instead? It can default to y if secure >>>>>> boot is disabled. >>>>>> >>>>> >>>>> We also already have CONFIG_SPL_ABORT_ON_RAW_IMAGE on which this is >>>>> based. If we only disable legacy image support then RAW images should >>>>> still be allowed, but we will fail early anyway, we will start to need >>>>> an unmaintainable amount of pre-processor logic to to handle the >>>>> different image types and what is allowed/not allowed. >>>>> >>>>> Even worse some boot modes don't seem to support FIT images (net, >>>>> onenand) so these will alway expect legacy to work. Right now we simply >>>>> have to disable these modes. >>>> >>>> IMO CONFIG_SPL_ABORT_ON_RAW_IMAGE should become a positive option, to >>>> fit in with the legacy format. Otherwise we'll get very confused I >>>> think. >>>> >>> >>> I'm not sure what you are suggesting here, would you like >>> >>> CONFIG_SPL_SUPPORT_RAW_IMAGE >>> CONFIG_SPL_SUPPORT_LEGACY_IMAGE >>> CONFIG_SPL_SUPPORT_FIT_IMAGE >>> >>> And then we disable as needed? I'm not sure this will work in our case, >>> as a new image type may be introduced and enabled by default, this will >>> break our board security until we discover this and disabled it. The >>> benefit of a negative option for us is that we can specify we *only* >>> allow FIT, then it will be obvious to someone adding a new image type >>> they will not meet this check and should not put code outside this block. >> >> I don't think we add new image types often, and they should default to >> n just as we do for U-Boot proper. Although something odd has happened >> with DISABLE_IMAGE_LEGACY. >> >> his should of thing should be caught in a security review. >> >> Also you could add something to print a warning if secure boot is >> enabled but insecure boot options are available? Perhaps that should >> be another option, and default to y? >> >> It's just that it is really hard to deal with multiple negative >> options, so best avoided if we can. >> > > I agree in general, but this time I think this is hard to properly > avoid. All that would be different with a positivoption only case > would be a bunch of checks that all other image modes are off, then > block undefining the same code I am here. But why is SPL different from U-Boot proper on this point? It seems like we could use the same scheme in SPL as we do in U-Boot proper? Positive options are easier to understand and combine. If we end up adding another image format it wouldn't be hard to default it to n if we are using secure boot. Regards, Simon ^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v2 1/4] Introduce CONFIG_SPL_ABORT_ON_NON_FIT_IMAGE 2017-02-10 16:23 ` Simon Glass @ 2017-02-10 16:57 ` Andrew F. Davis 0 siblings, 0 replies; 13+ messages in thread From: Andrew F. Davis @ 2017-02-10 16:57 UTC (permalink / raw) To: u-boot On 02/10/2017 10:23 AM, Simon Glass wrote: > Hi Andrew, > > On 8 February 2017 at 08:18, Andrew F. Davis <afd@ti.com> wrote: >> On 12/06/2016 09:47 PM, Simon Glass wrote: >>> Hi Andrew, >>> >>> On 5 December 2016 at 17:37, Andrew F. Davis <afd@ti.com> wrote: >>>> On 11/14/2016 06:33 PM, Simon Glass wrote: >>>>> Hi Andrew, >>>>> >>>>> On 14 November 2016 at 15:05, Andrew F. Davis <afd@ti.com> wrote: >>>>>> On 11/14/2016 02:44 PM, Simon Glass wrote: >>>>>>> Hi Andrew, >>>>>>> >>>>>>> On 14 November 2016 at 12:14, Andrew F. Davis <afd@ti.com> wrote: >>>>>>>> Introduce CONFIG_SPL_ABORT_ON_NON_FIT_IMAGE. An SPL which define >>>>>>>> this will abort image loading if the image is not a FIT image. >>>>>>>> >>>>>>>> Signed-off-by: Andrew F. Davis <afd@ti.com> >>>>>>>> --- >>>>>>>> Kconfig | 9 +++++++++ >>>>>>>> common/spl/spl.c | 5 +++++ >>>>>>>> 2 files changed, 14 insertions(+) >>>>>>>> >>>>>>>> diff --git a/Kconfig b/Kconfig >>>>>>>> index 1263d0b..eefebef 100644 >>>>>>>> --- a/Kconfig >>>>>>>> +++ b/Kconfig >>>>>>>> @@ -291,6 +291,15 @@ config FIT_IMAGE_POST_PROCESS >>>>>>>> injected into the FIT creation (i.e. the blobs would have been pre- >>>>>>>> processed before being added to the FIT image). >>>>>>>> >>>>>>>> +config SPL_ABORT_ON_NON_FIT_IMAGE >>>>>>> >>>>>>> We already have CONFIG_IMAGE_FORMAT_LEGACY so how about >>>>>>> CONFIG_SPL_IMAGE_FORMAT_LEGACY instead? It can default to y if secure >>>>>>> boot is disabled. >>>>>>> >>>>>> >>>>>> We also already have CONFIG_SPL_ABORT_ON_RAW_IMAGE on which this is >>>>>> based. If we only disable legacy image support then RAW images should >>>>>> still be allowed, but we will fail early anyway, we will start to need >>>>>> an unmaintainable amount of pre-processor logic to to handle the >>>>>> different image types and what is allowed/not allowed. >>>>>> >>>>>> Even worse some boot modes don't seem to support FIT images (net, >>>>>> onenand) so these will alway expect legacy to work. Right now we simply >>>>>> have to disable these modes. >>>>> >>>>> IMO CONFIG_SPL_ABORT_ON_RAW_IMAGE should become a positive option, to >>>>> fit in with the legacy format. Otherwise we'll get very confused I >>>>> think. >>>>> >>>> >>>> I'm not sure what you are suggesting here, would you like >>>> >>>> CONFIG_SPL_SUPPORT_RAW_IMAGE >>>> CONFIG_SPL_SUPPORT_LEGACY_IMAGE >>>> CONFIG_SPL_SUPPORT_FIT_IMAGE >>>> >>>> And then we disable as needed? I'm not sure this will work in our case, >>>> as a new image type may be introduced and enabled by default, this will >>>> break our board security until we discover this and disabled it. The >>>> benefit of a negative option for us is that we can specify we *only* >>>> allow FIT, then it will be obvious to someone adding a new image type >>>> they will not meet this check and should not put code outside this block. >>> >>> I don't think we add new image types often, and they should default to >>> n just as we do for U-Boot proper. Although something odd has happened >>> with DISABLE_IMAGE_LEGACY. >>> >>> his should of thing should be caught in a security review. >>> >>> Also you could add something to print a warning if secure boot is >>> enabled but insecure boot options are available? Perhaps that should >>> be another option, and default to y? >>> >>> It's just that it is really hard to deal with multiple negative >>> options, so best avoided if we can. >>> >> >> I agree in general, but this time I think this is hard to properly >> avoid. All that would be different with a positivoption only case >> would be a bunch of checks that all other image modes are off, then >> block undefining the same code I am here. > > But why is SPL different from U-Boot proper on this point? It seems > like we could use the same scheme in SPL as we do in U-Boot proper? > > Positive options are easier to understand and combine. If we end up > adding another image format it wouldn't be hard to default it to n if > we are using secure boot. > Right after I send this response I caved and decided to do it your way in v3: https://www.mail-archive.com/u-boot at lists.denx.de/msg238520.html Sorry I forgot to express that here. Thanks, Andrew > Regards, > Simon > ^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v2 2/4] ARM: AM57xx: Disable non-FIT based image loading for HS devices 2016-11-14 19:14 [U-Boot] [PATCH v2 0/4] Allow disabling non-FIT image loading from SPL Andrew F. Davis 2016-11-14 19:14 ` [U-Boot] [PATCH v2 1/4] Introduce CONFIG_SPL_ABORT_ON_NON_FIT_IMAGE Andrew F. Davis @ 2016-11-14 19:14 ` Andrew F. Davis 2016-11-14 19:14 ` [U-Boot] [PATCH v2 3/4] ARM: AM437x: " Andrew F. Davis 2016-11-14 19:14 ` [U-Boot] [PATCH v2 4/4] ARM: DRA7xx: " Andrew F. Davis 3 siblings, 0 replies; 13+ messages in thread From: Andrew F. Davis @ 2016-11-14 19:14 UTC (permalink / raw) To: u-boot Disable support for loading non-FIT images for AM57xx platforms using the high-security (HS) device variant. Signed-off-by: Andrew F. Davis <afd@ti.com> --- configs/am57xx_hs_evm_defconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/configs/am57xx_hs_evm_defconfig b/configs/am57xx_hs_evm_defconfig index 6631bb2..5812dfe 100644 --- a/configs/am57xx_hs_evm_defconfig +++ b/configs/am57xx_hs_evm_defconfig @@ -11,6 +11,7 @@ CONFIG_FIT=y CONFIG_OF_BOARD_SETUP=y CONFIG_SPL_LOAD_FIT=y CONFIG_SPL_FIT_IMAGE_POST_PROCESS=y +CONFIG_SPL_ABORT_ON_NON_FIT_IMAGE=y CONFIG_FIT_IMAGE_POST_PROCESS=y CONFIG_SYS_CONSOLE_INFO_QUIET=y CONFIG_VERSION_VARIABLE=y -- 2.10.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v2 3/4] ARM: AM437x: Disable non-FIT based image loading for HS devices 2016-11-14 19:14 [U-Boot] [PATCH v2 0/4] Allow disabling non-FIT image loading from SPL Andrew F. Davis 2016-11-14 19:14 ` [U-Boot] [PATCH v2 1/4] Introduce CONFIG_SPL_ABORT_ON_NON_FIT_IMAGE Andrew F. Davis 2016-11-14 19:14 ` [U-Boot] [PATCH v2 2/4] ARM: AM57xx: Disable non-FIT based image loading for HS devices Andrew F. Davis @ 2016-11-14 19:14 ` Andrew F. Davis 2016-11-14 19:14 ` [U-Boot] [PATCH v2 4/4] ARM: DRA7xx: " Andrew F. Davis 3 siblings, 0 replies; 13+ messages in thread From: Andrew F. Davis @ 2016-11-14 19:14 UTC (permalink / raw) To: u-boot Disable support for loading non-FIT images for AM437x platforms using the high-security (HS) device variant. Signed-off-by: Andrew F. Davis <afd@ti.com> --- configs/am43xx_hs_evm_defconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/configs/am43xx_hs_evm_defconfig b/configs/am43xx_hs_evm_defconfig index 1c53877..9c00e8b 100644 --- a/configs/am43xx_hs_evm_defconfig +++ b/configs/am43xx_hs_evm_defconfig @@ -10,6 +10,7 @@ CONFIG_FIT=y CONFIG_SYS_EXTRA_OPTIONS="CONS_INDEX=1, NAND" CONFIG_SPL_LOAD_FIT=y CONFIG_SPL_FIT_IMAGE_POST_PROCESS=y +CONFIG_SPL_ABORT_ON_NON_FIT_IMAGE=y CONFIG_FIT_IMAGE_POST_PROCESS=y CONFIG_SYS_CONSOLE_INFO_QUIET=y CONFIG_VERSION_VARIABLE=y -- 2.10.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v2 4/4] ARM: DRA7xx: Disable non-FIT based image loading for HS devices 2016-11-14 19:14 [U-Boot] [PATCH v2 0/4] Allow disabling non-FIT image loading from SPL Andrew F. Davis ` (2 preceding siblings ...) 2016-11-14 19:14 ` [U-Boot] [PATCH v2 3/4] ARM: AM437x: " Andrew F. Davis @ 2016-11-14 19:14 ` Andrew F. Davis 3 siblings, 0 replies; 13+ messages in thread From: Andrew F. Davis @ 2016-11-14 19:14 UTC (permalink / raw) To: u-boot Disable support for loading non-FIT images for DRA7xx platforms using the high-security (HS) device variant. Signed-off-by: Andrew F. Davis <afd@ti.com> --- configs/dra7xx_hs_evm_defconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/configs/dra7xx_hs_evm_defconfig b/configs/dra7xx_hs_evm_defconfig index 838de5c..75befb5 100644 --- a/configs/dra7xx_hs_evm_defconfig +++ b/configs/dra7xx_hs_evm_defconfig @@ -12,6 +12,7 @@ CONFIG_FIT=y CONFIG_OF_BOARD_SETUP=y CONFIG_SPL_LOAD_FIT=y CONFIG_SPL_FIT_IMAGE_POST_PROCESS=y +CONFIG_SPL_ABORT_ON_NON_FIT_IMAGE=y CONFIG_FIT_IMAGE_POST_PROCESS=y CONFIG_SYS_CONSOLE_INFO_QUIET=y CONFIG_VERSION_VARIABLE=y -- 2.10.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
end of thread, other threads:[~2017-02-10 16:57 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-11-14 19:14 [U-Boot] [PATCH v2 0/4] Allow disabling non-FIT image loading from SPL Andrew F. Davis 2016-11-14 19:14 ` [U-Boot] [PATCH v2 1/4] Introduce CONFIG_SPL_ABORT_ON_NON_FIT_IMAGE Andrew F. Davis 2016-11-14 20:44 ` Simon Glass 2016-11-14 22:05 ` Andrew F. Davis 2016-11-15 0:33 ` Simon Glass 2016-12-05 22:37 ` Andrew F. Davis 2016-12-07 3:47 ` Simon Glass 2017-02-08 15:18 ` Andrew F. Davis 2017-02-10 16:23 ` Simon Glass 2017-02-10 16:57 ` Andrew F. Davis 2016-11-14 19:14 ` [U-Boot] [PATCH v2 2/4] ARM: AM57xx: Disable non-FIT based image loading for HS devices Andrew F. Davis 2016-11-14 19:14 ` [U-Boot] [PATCH v2 3/4] ARM: AM437x: " Andrew F. Davis 2016-11-14 19:14 ` [U-Boot] [PATCH v2 4/4] ARM: DRA7xx: " Andrew F. Davis
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.