[RFC][PATCH 1/2] cmd: bootm: add a stage pre-load

Alex G. mr.nuke.me at gmail.com
Tue Mar 30 19:50:06 CEST 2021


Hi Phillipe,

On 3/30/21 11:26 AM, Philippe Reynes wrote:
> This commit adds a stage pre-load that could
> check or modify the image provided to the bootm
> command.
> 
> For the moment, only a header with a signature is
> supported. This header has this format:
> - magic : 4 bytes
> - image size : 4 bytes
> - sig size : 4 bytes
> - signature : n bytes
> - padding : up to header size
> 
> The stage use a node /bootm/pre-load/sig to
> get some information:
> - header-size (mandatory) : size of the header
> - algo-name (mandatory) : name of the algo used to sign
> - padding-name : name of padding used to sign
> - mandatory : set to yes if this sig is mandatory
> - public-key : value of the public key
> 
> Before running the image, the stage pre-load check
> the signature provided in the header.
> 
> This is an initial support, later we could add the
> support of:
> - ciphering
> - uncompressing
> - ...

You're on the right path of what I had in mind.

One thing that we could improve is dropping the dependency on bootm. A 
FIT image could also be loaded with CONFIG_SPL_LOAD_FIT or 
CONFIG_SPL_LOAD_FIT_FULL. It would be nice to have the signature 
verification code shared with image-fit-sig.c

The decision to verify the "header signature" is done at kconfig time. 
For distinguishing between image or config node signing, the "required" 
property in the u-boot FDT is used. So it seems odd to introduce another 
mechanism instead of leveraging "required".

A nice to have: how does mkimage insert this header signature into the FIT?

Alex



> Signed-off-by: Philippe Reynes <philippe.reynes at softathome.com>
> ---
>   cmd/Kconfig     |   9 ++
>   cmd/bootm.c     |   2 +-
>   common/bootm.c  | 258 ++++++++++++++++++++++++++++++++++++++++++++++++
>   include/image.h |   1 +
>   4 files changed, 269 insertions(+), 1 deletion(-)
> 
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index eff238cb38..086d2b7b74 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -194,6 +194,15 @@ config CMD_BOOTM
>   	help
>   	  Boot an application image from the memory.
>   
> +config CMD_BOOTM_PRE_LOAD
> +       bool "enable pre-load on bootm"
> +       depends on CMD_BOOTM
> +       default n
> +       help
> +         Enable support of stage pre-load for the bootm command.
> +	 This stage allow to check of modifty the image provided
> +	 to the bootm command.
> +
>   config BOOTM_EFI
>   	bool "Support booting UEFI FIT images"
>   	depends on CMD_BOOTEFI && CMD_BOOTM && FIT
> diff --git a/cmd/bootm.c b/cmd/bootm.c
> index 81c6b93978..7a6299d8d8 100644
> --- a/cmd/bootm.c
> +++ b/cmd/bootm.c
> @@ -126,7 +126,7 @@ int do_bootm(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
>   	}
>   
>   	return do_bootm_states(cmdtp, flag, argc, argv, BOOTM_STATE_START |
> -		BOOTM_STATE_FINDOS | BOOTM_STATE_FINDOTHER |
> +		BOOTM_STATE_FINDOS | BOOTM_STATE_PRE_LOAD | BOOTM_STATE_FINDOTHER |
>   		BOOTM_STATE_LOADOS |
>   #ifdef CONFIG_SYS_BOOT_RAMDISK_HIGH
>   		BOOTM_STATE_RAMDISK |
> diff --git a/common/bootm.c b/common/bootm.c
> index defaed8426..37b1340023 100644
> --- a/common/bootm.c
> +++ b/common/bootm.c
> @@ -42,6 +42,12 @@
>   
>   #define IH_INITRD_ARCH IH_ARCH_DEFAULT
>   
> +#define BOOTM_PRE_LOAD_SIG_MAGIC		0x55425348
> +#define BOOTM_PRE_LOAD_SIG_OFFSET_MAGIC		0
> +#define BOOTM_PRE_LOAD_SIG_OFFSET_IMG_LEN	4
> +#define BOOTM_PRE_LOAD_SIG_OFFSET_SIG_LEN	8
> +#define BOOTM_PRE_LOAD_SIG_OFFSET_SIG		12
> +
>   #ifndef USE_HOSTCC
>   
>   DECLARE_GLOBAL_DATA_PTR;
> @@ -87,6 +93,255 @@ static int bootm_start(struct cmd_tbl *cmdtp, int flag, int argc,
>   	return 0;
>   }
>   
> +static ulong bootm_data_addr(int argc, char *const argv[])
> +{
> +	ulong addr;
> +
> +	if (argc > 0)
> +		addr = simple_strtoul(argv[0], NULL, 16);
> +	else
> +		addr = image_load_addr;
> +
> +	return addr;
> +}
> +
> +struct bootm_sig_header {
> +	u32 magic;
> +	u32 size;
> +	u8 *sig;
> +};
> +
> +struct bootm_sig_info {
> +	ulong header_size;
> +	char *algo_name;
> +	char *padding_name;
> +	u8 *key;
> +	int key_len;
> +	int mandatory;
> +};
> +
> +/*
> + * This function gather information about the signature check
> + * that could be done in the pre-load stage of bootm.
> + *
> + * return:
> + * -1 => an error has occurred
> + *  0 => OK
> + *  1 => no setup
> + */
> +static int bootm_pre_load_sig_setup(struct bootm_sig_info *info)
> +{
> +	const void *algo_name, *padding_name, *key, *mandatory;
> +	const u32 *header_size;
> +	int key_len;
> +	int node, ret = 0;
> +
> +	if (!info) {
> +		printf("ERROR: info is NULL for bootm pre-load sig check\n");
> +		ret = -1;
> +		goto out;
> +	}
> +
> +	memset(info, 0, sizeof(*info));
> +
> +	node = fdt_path_offset(gd->fdt_blob, "/bootm/pre-load/sig");
> +	if (node < 0) {
> +		printf("INFO: no info for bootm pre-load sig check\n");
> +		ret = 1;
> +		goto out;
> +	}
> +
> +	header_size = fdt_getprop(gd->fdt_blob, node, "header-size", NULL);
> +	if (!header_size) {
> +		printf("ERROR: no header-size for bootm pre-load sig check\n");
> +		ret = -1;
> +		goto out;
> +	}
> +
> +	algo_name = fdt_getprop(gd->fdt_blob, node, "algo-name", NULL);
> +	if (!algo_name) {
> +		printf("ERROR: no algo_name for bootm pre-load sig check\n");
> +		ret = -1;
> +		goto out;
> +	}
> +
> +	padding_name = fdt_getprop(gd->fdt_blob, node, "padding-name", NULL);
> +	if (!padding_name) {
> +		printf("INFO: no padding_name provided, so using pkcs-1.5\n");
> +		padding_name = "pkcs-1.5";
> +	}
> +
> +	key = fdt_getprop(gd->fdt_blob, node, "public-key", &key_len);
> +	if (!key) {
> +		printf("ERROR: no key for bootm pre-load sig check\n");
> +		ret = -1;
> +		goto out;
> +	}
> +
> +	info->header_size	= fdt32_to_cpu(*header_size);
> +	info->algo_name		= (char *)algo_name;
> +	info->padding_name	= (char *)padding_name;
> +	info->key		= (uint8_t *)key;
> +	info->key_len		= key_len;
> +
> +	mandatory = fdt_getprop(gd->fdt_blob, node, "mandatory", NULL);
> +	if (mandatory && !strcmp((char *)mandatory, "yes"))
> +		info->mandatory = 1;
> +
> + out:
> +	return ret;
> +}
> +
> +static int bootm_pre_load_sig_get_header_u32(struct bootm_sig_info *info,
> +					     ulong addr,  u32 offset,
> +					     u32 *value)
> +{
> +	void *header;
> +	u32 *tmp;
> +	int ret = 0;
> +
> +	header = map_sysmem(addr, info->header_size);
> +	if (!header) {
> +		printf("ERROR: can't map header bootm pre-load sig\n");
> +		ret = -1;
> +		goto out;
> +	}
> +
> +	tmp = header + offset;
> +	*value = be32_to_cpu(*tmp);
> +
> +	unmap_sysmem(header);
> +
> + out:
> +	return ret;
> +}
> +
> +static int bootm_pre_load_sig_get_magic(struct bootm_sig_info *info,
> +					ulong addr, u32 *magic)
> +{
> +	int ret;
> +
> +	ret = bootm_pre_load_sig_get_header_u32(info, addr,
> +						BOOTM_PRE_LOAD_SIG_OFFSET_MAGIC, magic);
> +
> +	return ret;
> +}
> +
> +static int bootm_pre_load_sig_get_img_len(struct bootm_sig_info *info,
> +					  ulong addr, u32 *len)
> +{
> +	int ret;
> +
> +	ret = bootm_pre_load_sig_get_header_u32(info, addr,
> +						BOOTM_PRE_LOAD_SIG_OFFSET_IMG_LEN, len);
> +	if (ret < 0)
> +		goto out;
> +
> +	if (*len > CONFIG_SYS_BOOTM_LEN) {
> +		printf("ERROR: size of image (%u) bigger than CONFIG_SYS_BOOTM_LEN (%u)\n",
> +		       *len, CONFIG_SYS_BOOTM_LEN);
> +		ret = -1;
> +		goto out;
> +	}
> +
> + out:
> +	return ret;
> +}
> +
> +static int bootm_pre_load_sig_check(struct bootm_sig_info *info, ulong addr, int img_len)
> +{
> +	void *image;
> +	struct image_sign_info sig_info;
> +	struct image_region reg;
> +	u32 sig_len, *psig_len;
> +	u8 *sig;
> +	int ret = 0;
> +
> +	image = (void *)map_sysmem(addr, info->header_size + img_len);
> +	if (!image) {
> +		printf("ERROR: can't map full image\n");
> +		ret = -1;
> +		goto out;
> +	}
> +
> +	memset(&sig_info, '\0', sizeof(sig_info));
> +	sig_info.name = info->algo_name;
> +	sig_info.padding = image_get_padding_algo(info->padding_name);
> +	sig_info.checksum = image_get_checksum_algo(sig_info.name);
> +	sig_info.crypto = image_get_crypto_algo(sig_info.name);
> +	sig_info.key = info->key;
> +	sig_info.keylen = info->key_len;
> +
> +	reg.data = image + info->header_size;
> +	reg.size = img_len;
> +
> +	psig_len = (uint32_t *)((uint8_t *)image + BOOTM_PRE_LOAD_SIG_OFFSET_SIG_LEN);
> +	sig_len = be32_to_cpu(*psig_len);
> +	sig = (uint8_t *)image + BOOTM_PRE_LOAD_SIG_OFFSET_SIG;
> +
> +	ret = sig_info.crypto->verify(&sig_info, &reg, 1, sig, sig_len);
> +	if (ret < 0)
> +		printf("ERROR: signature check has failed (err=%d)\n", ret);
> +
> +	memmove(image, image + info->header_size, img_len);
> +
> +	unmap_sysmem(image);
> +
> + out:
> +	return ret;
> +}
> +
> +static int bootm_pre_load_sig(ulong addr)
> +{
> +	struct bootm_sig_info info;
> +	u32 magic, img_len;
> +	int ret;
> +
> +	ret = bootm_pre_load_sig_setup(&info);
> +	if (ret < 0)
> +		goto out;
> +	if (ret > 0) {
> +		ret = 0;
> +		goto out;
> +	}
> +
> +	ret = bootm_pre_load_sig_get_magic(&info, addr, &magic);
> +	if (ret < 0)
> +		goto out;
> +
> +	if (magic != BOOTM_PRE_LOAD_SIG_MAGIC) {
> +		if (info.mandatory) {
> +			printf("ERROR: signature is mandatory\n");
> +			ret = -1;
> +		}
> +		goto out;
> +	}
> +
> +	ret = bootm_pre_load_sig_get_img_len(&info, addr, &img_len);
> +	if (ret < 0)
> +		goto out;
> +
> +	ret = bootm_pre_load_sig_check(&info, addr, img_len);
> +
> + out:
> +	return ret;
> +}
> +
> +static int bootm_pre_load(struct cmd_tbl *cmdtp, int flag, int argc,
> +			  char *const argv[])
> +{
> +	ulong data_addr = bootm_data_addr(argc, argv);
> +	int ret = 0;
> +
> +	if (CONFIG_IS_ENABLED(CMD_BOOTM_PRE_LOAD))
> +		ret = bootm_pre_load_sig(data_addr);
> +
> +	if (ret)
> +		ret = CMD_RET_FAILURE;
> +
> +	return ret;
> +}
> +
>   static int bootm_find_os(struct cmd_tbl *cmdtp, int flag, int argc,
>   			 char *const argv[])
>   {
> @@ -676,6 +931,9 @@ int do_bootm_states(struct cmd_tbl *cmdtp, int flag, int argc,
>   	if (states & BOOTM_STATE_START)
>   		ret = bootm_start(cmdtp, flag, argc, argv);
>   
> +	if (!ret && (states & BOOTM_STATE_PRE_LOAD))
> +		ret = bootm_pre_load(cmdtp, flag, argc, argv);
> +
>   	if (!ret && (states & BOOTM_STATE_FINDOS))
>   		ret = bootm_find_os(cmdtp, flag, argc, argv);
>   
> diff --git a/include/image.h b/include/image.h
> index b4b284d52b..bd72b49913 100644
> --- a/include/image.h
> +++ b/include/image.h
> @@ -432,6 +432,7 @@ typedef struct bootm_headers {
>   #define	BOOTM_STATE_OS_PREP	(0x00000100)
>   #define	BOOTM_STATE_OS_FAKE_GO	(0x00000200)	/* 'Almost' run the OS */
>   #define	BOOTM_STATE_OS_GO	(0x00000400)
> +#define	BOOTM_STATE_PRE_LOAD	(0x00000800)
>   	int		state;
>   
>   #ifdef CONFIG_LMB
> 


More information about the U-Boot mailing list