Re: [PATCH 3/3] environment: add explicit option to allow searching for environment devices

[Marco Felsch <m.felsch@pengutronix.de>]

Hi Sascha,

On 26-04-28, Sascha Hauer wrote:
> Add an explicit Kconfig option to allow searching the environment storage path
> based on the barebox environment partition GUID.
> 
> So far this depended on CONFIG_INSECURE being set. First of all loading the
> barebox environment from storage is always insecure as the barebox environment
> doesn't have any security measures. The difference that comes with loading
> the environment from an explicitly specified storage device and autoprobing
> it from the available block devices is that with the former an attacker would
> need access to the internal storage whereas with the latter barebox could
> be tricked into loading an environment from an external SD card.
> 
> Whether or not this is acceptable depends on the case, so ask the user for it.
> 
> Real security can only be provided by not loading an environment from storage
> at all, but that can be controlled at compile time by disabling CONFIG_ENV_HANDLING
> or at runtime by security policies.

TBH I actually don't see why this option can't follow the
CONFIG_INSECURE.

Since ENV handling is enabled you do pull the HAS_INSECURE_DEFAULTS=y.
As you written above env handling is always insecure as of now.

So it seems that you want to get rid of the CONFIG_INSECURE=y in your
setup. The only users of this CONFIG switch are global_env_autoprobe and
lib/random.c. Therefore my question, that I don't see why we can't stick
with the CONFIG_INSECURE switch.

Regards,
  Marco


> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  common/Kconfig       | 14 ++++++++++++++
>  common/environment.c |  2 +-
>  2 files changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/common/Kconfig b/common/Kconfig
> index fd422714d5..f74f06b4ad 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -987,6 +987,20 @@ config ENV_HANDLING
>  	  A safe use of the mutable environment may be possible if board code only
>  	  mounts it after verifying a JSON Web Token that enables a debug mode.
>  
> +config ENV_HANDLING_AUTOPROBE
> +	depends on ENV_HANDLING
> +	bool "Autoprobe for environment devices"
> +	default y
> +	help
> +	  There are two ways for specifying where the barebox environment is. The first one
> +	  specifies the path explicitly in the device tree or board code. The other one is
> +	  to automatically search it on block devices which is enabled with this option.
> +	  Here a partition with the barebox environment GUID is searched. If this option
> +	  is enabled and no environment is chosen by device tree or board code then a block
> +	  device containing a partition with the barebox environment GUID is used for the
> +	  environment. A block device barebox has booted from is preferred over other block
> +	  devices.
> +
>  config DEFAULT_ENVIRONMENT
>  	select CRC32
>  	bool
> diff --git a/common/environment.c b/common/environment.c
> index f883a520c6..a3281feca2 100644
> --- a/common/environment.c
> +++ b/common/environment.c
> @@ -53,7 +53,7 @@ struct action_data {
>  
>  #define TMPDIR "/.defaultenv"
>  
> -static int global_env_autoprobe = IS_ENABLED(CONFIG_INSECURE);
> +static int global_env_autoprobe = IS_ENABLED(CONFIG_ENV_HANDLING_AUTOPROBE);
>  static char *default_environment_path;
>  
>  void default_environment_path_set(const char *path)
> 
> -- 
> 2.47.3
> 
> 
> 

-- 
#gernperDu 
#CallMeByMyFirstName

Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | https://www.pengutronix.de/ |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-9    |