Locking down U-Boot env with ENV_WRITEABLE_LIST

[Tim Harvey]

On Tue, Apr 6, 2021 at 2:54 PM Sean Anderson <sean.anderson at seco.com> wrote:
>
>
>
> On 4/6/21 4:10 PM, Marek Vasut wrote:
>  > On 4/6/21 9:52 PM, Tim Harvey wrote:
>  >> On Mon, Apr 5, 2021 at 10:36 AM Marek Vasut <marex at denx.de> wrote:
>  >>>
>  >>> On 4/5/21 6:27 PM, Tim Harvey wrote:
>  >>>> On Sat, Apr 3, 2021 at 12:09 PM Marek Vasut <marex at denx.de> wrote:
>  >>>>>
>  >>>>> On 4/3/21 6:43 PM, Tim Harvey wrote:
>  >>>>>
>  >>>>> Hi,
>  >>>>>
>  >>>>> [...]
>  >>>>>
>  >>>>>>>>> And those config options I had enabled in u-boot defconfig:
>  >>>>>>>>>
>  >>>>>>>>> CONFIG_CMD_ENV_CALLBACK=y
>  >>>>>>>>> CONFIG_CMD_ENV_FLAGS=y
>  >>>>>>>>> CONFIG_ENV_IS_NOWHERE=y
>  >>>>>>>>> CONFIG_ENV_IS_IN_MMC=y
>  >>>>>>>>> CONFIG_ENV_APPEND=y
>  >>>>>>>>> CONFIG_ENV_WRITEABLE_LIST=y
>  >>>>>>>>> CONFIG_ENV_ACCESS_IGNORE_FORCE=y
>  >>>>>>>>
>  >>>>>>>> Do you really define both ENV_IS_NOWHERE and ENV_IS_IN_MMC? From what
>  >>>>>>>> I see if you define ENV_IS_NOWHERE none of the others will be used.
>  >>>>>>>
>  >>>>>>> Yes, having two ENV drivers is mandatory. One provides the base env (the
>  >>>>>>> nowhere) and the other is used to import the filtered extras from
>  >>>>>>> external env.
>  >>>>>>
>  >>>>>> Enabling ENV_IS_NOWHERE does not work as you describe in my testing.
>  >>>>>> I'm testing this with imx8mm_venice_defconfig on U-Boot 2021.01 and as
>  >>>>>> soon as I define ENV_IS_NOWHERE then env_load is never called because
>  >>>>>> when env_relocate is called env is not valid yet so env_set_default is
>  >>>>>> called and env_load is 'never' called, thus mmc env is never loaded.
>  >>>>>> This is all from
>  >>>>>> https://elixir.bootlin.com/u-boot/v2021.01/source/env/common.c#L259
>  >>>>>
>  >>>>> Maybe this [PATCH V3] env: Fix invalid env handling in env_init() patch
>  >>>>> is missing ?
>  >>>>>
>  >>>>> [...]
>  >>>>>
>  >>>>>>     /* Link Definitions */
>  >>>>>>     #define CONFIG_LOADADDR                        0x40480000
>  >>>>>>     #define CONFIG_SYS_LOAD_ADDR           CONFIG_LOADADDR''
>  >>>>>>
>  >>>>>> With this configuration a blank env in flash results in the entire
>  >>>>>> default env showing in an 'env print' (ie all the stuff from
>  >>>>>> include/env_default.h 'default_environment') but as soon as I put an
>  >>>>>> env in flash (ie saveenv) and reset now the only env is what was set
>  >>>>>> via running code (ethaddr's fdtcontroladdr stdin/err/out). The only
>  >>>>>> different I expected is that I expected the default env from
>  >>>>>> include/env_default.h to be there on an initial boot with no valid mmc
>  >>>>>> env.
>  >>>>>
>  >>>>> Maybe the aforementioned patch is missing ?
>  >>>>
>  >>>> Marek,
>  >>>>
>  >>>> Yes, that patch fixes the issue I'm seeing of mmc not initializing and
>  >>>> now the default env shows up as expected when MMC env is empty or
>  >>>> populated.
>  >>>>
>  >>>> Thanks - hopefully that patch gets merged soon... I did respond with a
>  >>>> Tested-By.
>  >>>
>  >>> Oh, good. You might want to notify Tom about that, so it would get picked.
>  >>
>  >> Marek,
>  >>
>  >> One additional thing I did get stuck on is that if your writable
>  >> var(s) appears in the default environment the default will take
>  >> precedence over the FLASH env. This does make sense, but requires that
>  >> you create a default FLASH env (ie mkenvimage) and put it in the
>  >> appropriate place. I figured I would mention this for anyone else that
>  >> ends up reading this thread for help.
>  >
>  > Can you maybe write some better env documentation and submit a patch, now that you got it all figured out ?
>  >
>  >> One last thing that I have not yet figured out how to work-around is
>  >> how to best disable the shell completely so that if there is any
>  >> failure in your bootcmd you don't simply drop into a completely
>  >> insecure U-Boot shell. What is your recommendation there?
>  >
>  > set bootdelay=-2 disables the prompt access.
>
> This is insufficient. If the end of the boot command is reached, U-Boot
> will fall back to the shell. One must ensure that the bootcmd never
> exists. This can be done either by placing the whole command in a loop,
> recursively calling another command, or by resetting the board if no
> boot commands work. See e.g. [1] for a good writeup.
>
> --Sean
>
> [1] https://labs.f-secure.com/assets/BlogFiles/2020-05-u-booting-securely-wp-final.pdf

Right, even though bootdelay=-2 if for whatever reason the bootcmd
fails it will drop you in a shell.

I like the idea of adding a reset to the end to handle failures.
Thanks for the reference - its full of great info!

Tim