* spl: allow board_spl_fit_post_load() to fail @ 2020-05-09 16:13 Patrick Wildt 2020-05-09 16:38 ` Heinrich Schuchardt 0 siblings, 1 reply; 5+ messages in thread From: Patrick Wildt @ 2020-05-09 16:13 UTC (permalink / raw) To: u-boot On i.MX platforms board_spl_fit_post_load() can check the loaded SPL image for authenticity using its HAB engine. U-Boot's SPL mechanism allows booting images from other sources as well, but in the current setup the SPL would just hang if it encounters an image that does not pass scrutiny. Allowing the function to return an error, allows the SPL to try booting from another source as a fallback instead of ending up as a brick. Signed-off-by: Patrick Wildt <patrick@blueri.se> diff --git a/arch/arm/mach-imx/spl.c b/arch/arm/mach-imx/spl.c index fd3fa04600..b8f6fcb4df 100644 --- a/arch/arm/mach-imx/spl.c +++ b/arch/arm/mach-imx/spl.c @@ -311,7 +311,7 @@ ulong board_spl_fit_size_align(ulong size) return size; } -void board_spl_fit_post_load(ulong load_addr, size_t length) +int board_spl_fit_post_load(ulong load_addr, size_t length) { u32 offset = length - CONFIG_CSF_SIZE; @@ -319,8 +319,10 @@ void board_spl_fit_post_load(ulong load_addr, size_t length) offset + IVT_SIZE + CSF_PAD_SIZE, offset)) { puts("spl: ERROR: image authentication unsuccessful\n"); - hang(); + return -1; } + + return 0; } #endif diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c index c51e4beb1c..21c873c5fb 100644 --- a/common/spl/spl_fit.c +++ b/common/spl/spl_fit.c @@ -24,8 +24,9 @@ DECLARE_GLOBAL_DATA_PTR; #define CONFIG_SYS_BOOTM_LEN (64 << 20) #endif -__weak void board_spl_fit_post_load(ulong load_addr, size_t length) +__weak int board_spl_fit_post_load(ulong load_addr, size_t length) { + return 0; } __weak ulong board_spl_fit_size_align(ulong size) @@ -678,7 +679,9 @@ int spl_load_simple_fit(struct spl_image_info *spl_image, spl_image->flags |= SPL_FIT_FOUND; #ifdef CONFIG_IMX_HAB - board_spl_fit_post_load((ulong)fit, size); + ret = board_spl_fit_post_load((ulong)fit, size); + if (ret) + return ret; #endif return 0; diff --git a/include/spl.h b/include/spl.h index 6bf9fd8beb..93d5a5a1f3 100644 --- a/include/spl.h +++ b/include/spl.h @@ -560,7 +560,7 @@ int board_return_to_bootrom(struct spl_image_info *spl_image, * board_spl_fit_post_load - allow process images after loading finished * */ -void board_spl_fit_post_load(ulong load_addr, size_t length); +int board_spl_fit_post_load(ulong load_addr, size_t length); /** * board_spl_fit_size_align - specific size align before processing payload ^ permalink raw reply related [flat|nested] 5+ messages in thread
* spl: allow board_spl_fit_post_load() to fail 2020-05-09 16:13 spl: allow board_spl_fit_post_load() to fail Patrick Wildt @ 2020-05-09 16:38 ` Heinrich Schuchardt 2020-05-09 17:45 ` Patrick Wildt 0 siblings, 1 reply; 5+ messages in thread From: Heinrich Schuchardt @ 2020-05-09 16:38 UTC (permalink / raw) To: u-boot On 5/9/20 6:13 PM, Patrick Wildt wrote: > On i.MX platforms board_spl_fit_post_load() can check the loaded > SPL image for authenticity using its HAB engine. U-Boot's SPL > mechanism allows booting images from other sources as well, but > in the current setup the SPL would just hang if it encounters an > image that does not pass scrutiny. Allowing the function to return > an error, allows the SPL to try booting from another source as a > fallback instead of ending up as a brick. > > Signed-off-by: Patrick Wildt <patrick@blueri.se> Could an intruder abuse this by destroying a signed image and providing an unsigned image on a source under his control? Best regards Heinrich ^ permalink raw reply [flat|nested] 5+ messages in thread
* spl: allow board_spl_fit_post_load() to fail 2020-05-09 16:38 ` Heinrich Schuchardt @ 2020-05-09 17:45 ` Patrick Wildt 2020-05-09 18:09 ` Heinrich Schuchardt 0 siblings, 1 reply; 5+ messages in thread From: Patrick Wildt @ 2020-05-09 17:45 UTC (permalink / raw) To: u-boot On Sat, May 09, 2020 at 06:38:33PM +0200, Heinrich Schuchardt wrote: > On 5/9/20 6:13 PM, Patrick Wildt wrote: > > On i.MX platforms board_spl_fit_post_load() can check the loaded > > SPL image for authenticity using its HAB engine. U-Boot's SPL > > mechanism allows booting images from other sources as well, but > > in the current setup the SPL would just hang if it encounters an > > image that does not pass scrutiny. Allowing the function to return > > an error, allows the SPL to try booting from another source as a > > fallback instead of ending up as a brick. > > > > Signed-off-by: Patrick Wildt <patrick@blueri.se> > > Could an intruder abuse this by destroying a signed image and providing > an unsigned image on a source under his control? > > Best regards > > Heinrich Sure, let's think about it here. Maybe you have some more thoughts to add. First of all, the SPL goes through all the boot devices, and if there's none to find with an image, it will hang. It will hang like it does before the diff. The only difference is that it tries additional sources before hanging. Thus the attacker, unless he can exploit it in his first trial, or is able to force a reset, must have some access to reset the machine to have it boot and try again. This seems like he must have some kind of local or remote phyiscal access. Let's assume the image is on the network or on another remote medium. Then I guess the attacker will just try to attack that medium, and the alternate boot sources won't make a difference. I guess that means we should focus on local sources. I think we can also ignore a removable SD card, since he can just put in another one and try again. So maybe let's think about SPI flash and eMMC, soldered on, not directly accessible. If he has physical access, I guess he could open up the box and desolder a few pins, or add some voltage here and there to try and disrupt the bootup. But, then maybe it's easier to just desolder the whole SPI/eMMC and add his own. But what if he doesn't have that access? If he's remote? Ok, he will probably have to exploit the daemon (webserver or whatever) to gain some code execution. Then he'll try and become root, so he can access the disks. Then I figure he'll try and overwrite or remove the image. With this, on the next reboot it will (hopefully) fail to boot, unless he already has an exploit, then my patch won't make a difference. I figure the real issue could be that when the attacker has physical access, manages to remove/replace the image with a fallback to load from a device like an SD card, that it's now easier for him to try and find an exploit. Am I missing something? Best regards, Patrick ^ permalink raw reply [flat|nested] 5+ messages in thread
* spl: allow board_spl_fit_post_load() to fail 2020-05-09 17:45 ` Patrick Wildt @ 2020-05-09 18:09 ` Heinrich Schuchardt 2020-05-09 18:34 ` Patrick Wildt 0 siblings, 1 reply; 5+ messages in thread From: Heinrich Schuchardt @ 2020-05-09 18:09 UTC (permalink / raw) To: u-boot On 5/9/20 7:45 PM, Patrick Wildt wrote: > On Sat, May 09, 2020 at 06:38:33PM +0200, Heinrich Schuchardt wrote: >> On 5/9/20 6:13 PM, Patrick Wildt wrote: >>> On i.MX platforms board_spl_fit_post_load() can check the loaded >>> SPL image for authenticity using its HAB engine. U-Boot's SPL >>> mechanism allows booting images from other sources as well, but >>> in the current setup the SPL would just hang if it encounters an >>> image that does not pass scrutiny. Allowing the function to return >>> an error, allows the SPL to try booting from another source as a >>> fallback instead of ending up as a brick. >>> >>> Signed-off-by: Patrick Wildt <patrick@blueri.se> >> >> Could an intruder abuse this by destroying a signed image and providing >> an unsigned image on a source under his control? >> >> Best regards >> >> Heinrich > > Sure, let's think about it here. Maybe you have some more thoughts to > add. > > First of all, the SPL goes through all the boot devices, and if there's > none to find with an image, it will hang. It will hang like it does > before the diff. The only difference is that it tries additional > sources before hanging. Thus the attacker, unless he can exploit it > in his first trial, or is able to force a reset, must have some access > to reset the machine to have it boot and try again. This seems like he > must have some kind of local or remote phyiscal access. > > Let's assume the image is on the network or on another remote medium. > Then I guess the attacker will just try to attack that medium, and the > alternate boot sources won't make a difference. > > I guess that means we should focus on local sources. I think we can > also ignore a removable SD card, since he can just put in another one > and try again. > > So maybe let's think about SPI flash and eMMC, soldered on, not directly > accessible. If he has physical access, I guess he could open up the box > and desolder a few pins, or add some voltage here and there to try and > disrupt the bootup. But, then maybe it's easier to just desolder the > whole SPI/eMMC and add his own. > > But what if he doesn't have that access? If he's remote? Ok, he will > probably have to exploit the daemon (webserver or whatever) to gain some > code execution. Then he'll try and become root, so he can access the > disks. Then I figure he'll try and overwrite or remove the image. > With this, on the next reboot it will (hopefully) fail to boot, unless > he already has an exploit, then my patch won't make a difference. > > I figure the real issue could be that when the attacker has physical > access, manages to remove/replace the image with a fallback to load from > a device like an SD card, that it's now easier for him to try and find > an exploit. This last scenario is what I was thinking of. So if HAB is used it may be ok to search all devices for signed images but non-signed images should be rejected. Is that given with your patch? Best regards Heinrich > > Am I missing something? > > Best regards, > Patrick > ^ permalink raw reply [flat|nested] 5+ messages in thread
* spl: allow board_spl_fit_post_load() to fail 2020-05-09 18:09 ` Heinrich Schuchardt @ 2020-05-09 18:34 ` Patrick Wildt 0 siblings, 0 replies; 5+ messages in thread From: Patrick Wildt @ 2020-05-09 18:34 UTC (permalink / raw) To: u-boot On Sat, May 09, 2020 at 08:09:39PM +0200, Heinrich Schuchardt wrote: > On 5/9/20 7:45 PM, Patrick Wildt wrote: > > On Sat, May 09, 2020 at 06:38:33PM +0200, Heinrich Schuchardt wrote: > >> On 5/9/20 6:13 PM, Patrick Wildt wrote: > >>> On i.MX platforms board_spl_fit_post_load() can check the loaded > >>> SPL image for authenticity using its HAB engine. U-Boot's SPL > >>> mechanism allows booting images from other sources as well, but > >>> in the current setup the SPL would just hang if it encounters an > >>> image that does not pass scrutiny. Allowing the function to return > >>> an error, allows the SPL to try booting from another source as a > >>> fallback instead of ending up as a brick. > >>> > >>> Signed-off-by: Patrick Wildt <patrick@blueri.se> > >> > >> Could an intruder abuse this by destroying a signed image and providing > >> an unsigned image on a source under his control? > >> > >> Best regards > >> > >> Heinrich > > > > Sure, let's think about it here. Maybe you have some more thoughts to > > add. > > > > First of all, the SPL goes through all the boot devices, and if there's > > none to find with an image, it will hang. It will hang like it does > > before the diff. The only difference is that it tries additional > > sources before hanging. Thus the attacker, unless he can exploit it > > in his first trial, or is able to force a reset, must have some access > > to reset the machine to have it boot and try again. This seems like he > > must have some kind of local or remote phyiscal access. > > > > Let's assume the image is on the network or on another remote medium. > > Then I guess the attacker will just try to attack that medium, and the > > alternate boot sources won't make a difference. > > > > I guess that means we should focus on local sources. I think we can > > also ignore a removable SD card, since he can just put in another one > > and try again. > > > > So maybe let's think about SPI flash and eMMC, soldered on, not directly > > accessible. If he has physical access, I guess he could open up the box > > and desolder a few pins, or add some voltage here and there to try and > > disrupt the bootup. But, then maybe it's easier to just desolder the > > whole SPI/eMMC and add his own. > > > > But what if he doesn't have that access? If he's remote? Ok, he will > > probably have to exploit the daemon (webserver or whatever) to gain some > > code execution. Then he'll try and become root, so he can access the > > disks. Then I figure he'll try and overwrite or remove the image. > > With this, on the next reboot it will (hopefully) fail to boot, unless > > he already has an exploit, then my patch won't make a difference. > > > > I figure the real issue could be that when the attacker has physical > > access, manages to remove/replace the image with a fallback to load from > > a device like an SD card, that it's now easier for him to try and find > > an exploit. > > This last scenario is what I was thinking of. > > So if HAB is used it may be ok to search all devices for signed images > but non-signed images should be rejected. Is that given with your patch? > > Best regards > > Heinrich I think you have a very good point there. No, with that diff I think there's an issue, but I have added a second diff here. I'll explain. i.MX's jump_to_image_no_args() has a check: if (spl_image->flags & SPL_FIT_FOUND) { image_entry(); } else { [...] if (!imx_hab_authenticate_image(spl_image->load_addr, That means that it will jump right away if the flags say SPL_FIT_FOUND. Unfortunately my diff sets that flag before returning an error, and that means if the fallback tried to boot a non-FIT, it will skip the HAB check and jump right away. I have also not (yet) seen code that resets the flag. I figure we should just set the flag after doing the post load. Since it's a "fit_post_load"-function, we can assume that that function does not depend on the SPL_FIT_FOUND flag to do its work. I mean, it's only called because it parses a FIT in the first place. Thanks for making me think about this again! Best regards, Patrick diff --git a/arch/arm/mach-imx/spl.c b/arch/arm/mach-imx/spl.c index fd3fa04600..b8f6fcb4df 100644 --- a/arch/arm/mach-imx/spl.c +++ b/arch/arm/mach-imx/spl.c @@ -311,7 +311,7 @@ ulong board_spl_fit_size_align(ulong size) return size; } -void board_spl_fit_post_load(ulong load_addr, size_t length) +int board_spl_fit_post_load(ulong load_addr, size_t length) { u32 offset = length - CONFIG_CSF_SIZE; @@ -319,8 +319,10 @@ void board_spl_fit_post_load(ulong load_addr, size_t length) offset + IVT_SIZE + CSF_PAD_SIZE, offset)) { puts("spl: ERROR: image authentication unsuccessful\n"); - hang(); + return -1; } + + return 0; } #endif diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c index c51e4beb1c..7e9aff9240 100644 --- a/common/spl/spl_fit.c +++ b/common/spl/spl_fit.c @@ -24,8 +24,9 @@ DECLARE_GLOBAL_DATA_PTR; #define CONFIG_SYS_BOOTM_LEN (64 << 20) #endif -__weak void board_spl_fit_post_load(ulong load_addr, size_t length) +__weak int board_spl_fit_post_load(ulong load_addr, size_t length) { + return 0; } __weak ulong board_spl_fit_size_align(ulong size) @@ -675,11 +676,12 @@ int spl_load_simple_fit(struct spl_image_info *spl_image, if (spl_image->entry_point == FDT_ERROR || spl_image->entry_point == 0) spl_image->entry_point = spl_image->load_addr; - spl_image->flags |= SPL_FIT_FOUND; - #ifdef CONFIG_IMX_HAB - board_spl_fit_post_load((ulong)fit, size); + ret = board_spl_fit_post_load((ulong)fit, size); + if (ret) + return ret; #endif + spl_image->flags |= SPL_FIT_FOUND; return 0; } diff --git a/include/spl.h b/include/spl.h index 6bf9fd8beb..93d5a5a1f3 100644 --- a/include/spl.h +++ b/include/spl.h @@ -560,7 +560,7 @@ int board_return_to_bootrom(struct spl_image_info *spl_image, * board_spl_fit_post_load - allow process images after loading finished * */ -void board_spl_fit_post_load(ulong load_addr, size_t length); +int board_spl_fit_post_load(ulong load_addr, size_t length); /** * board_spl_fit_size_align - specific size align before processing payload ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-05-09 18:34 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-05-09 16:13 spl: allow board_spl_fit_post_load() to fail Patrick Wildt 2020-05-09 16:38 ` Heinrich Schuchardt 2020-05-09 17:45 ` Patrick Wildt 2020-05-09 18:09 ` Heinrich Schuchardt 2020-05-09 18:34 ` Patrick Wildt
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.