From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 4D824C433F5 for ; Thu, 25 Nov 2021 00:16:33 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id AAA168379A; Thu, 25 Nov 2021 01:14:28 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=chromium.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (1024-bit key; unprotected) header.d=chromium.org header.i=@chromium.org header.b="HyHdw3VY"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 55535801E7; Thu, 25 Nov 2021 01:13:14 +0100 (CET) Received: from mail-oi1-x22c.google.com (mail-oi1-x22c.google.com [IPv6:2607:f8b0:4864:20::22c]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 1F10F82F91 for ; Thu, 25 Nov 2021 01:13:08 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=chromium.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=sjg@google.com Received: by mail-oi1-x22c.google.com with SMTP id m6so8784063oim.2 for ; Wed, 24 Nov 2021 16:13:08 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=uwvPyMlTghurASFOzg5U+n77HBNqY+xGCqOTd+iWO9I=; b=HyHdw3VYn1GHBR8C7Htg//0ytqR+1VdPm0Z9fhnjIno84FN91dH9ehBkg0fNXwyd+U naGFe8wxtgI3HjMvGcJLTaBTykAlEA5V0TZsbANVa7OnsXLtIyGCXs64Coj4RMuE7qEw KTDuoLydV0A4wQ6LJf3wdDG1hTSkQezkm6WH0= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=uwvPyMlTghurASFOzg5U+n77HBNqY+xGCqOTd+iWO9I=; b=knr929aUrRBTW6cX44Ql6khQ1GMee3FZ3JjdX0szX1ZvM1f+q+ve4n2Vhm9Hu34PBx Ppsv1Dn81Cyovdp9YTQxg0k0B2hDgGASxagJT0hHqAu+31/wV8xxv7tEcK5kkumB/k0j GrYFO0Dj/GejqFDvvEOvWoxs8u2lA/oONvwBg+0/2GOISAhaaQrdLCaLiER34Knvj/9B 6oNMrLWZRFr4tR2S4zQ3ow3PRBLhm5Y7nAN6rR+pgPgBzTx2LDswg95wjuHNsxi0h8cB w5IKElMsNP5du9mqqLoVVmtC0HsCXKSEsLe+noPuJo5cr4h2Rvten/j5aX1ZWwRLJZ7d 2ySw== X-Gm-Message-State: AOAM531XVTV9FG3yWSLqeoitT0AGDBlq32zwKn/BTaNJuA+CWFTGX7JX NVVTNOOMVdSXNO1SWzUZbH55MdLjiyDIOMGi3GtvWg== X-Google-Smtp-Source: ABdhPJwyqZEFAoo6bTDYLUc1/+6H2mEhybvGo1/cBUHFAYQPRFR9bSfk8r3CzxYID+4RgfxzslhFnOQx1NywH9bX/o4= X-Received: by 2002:a05:6808:b0d:: with SMTP id s13mr11199208oij.53.1637799186467; Wed, 24 Nov 2021 16:13:06 -0800 (PST) MIME-Version: 1.0 References: <20211117175215.24262-1-philippe.reynes@softathome.com> <20211117175215.24262-5-philippe.reynes@softathome.com> In-Reply-To: <20211117175215.24262-5-philippe.reynes@softathome.com> From: Simon Glass Date: Wed, 24 Nov 2021 17:12:54 -0700 Message-ID: Subject: Re: [RFC PATCH v3 4/8] boot: image: add a stage pre-load To: Philippe Reynes Cc: mr.nuke.me@gmail.com, joel.peshkin@broadcom.com, u-boot@lists.denx.de Content-Type: text/plain; charset="UTF-8" X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.37 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.2 at phobos.denx.de X-Virus-Status: Clean Hi Philippe, On Wed, 17 Nov 2021 at 10:52, Philippe Reynes wrote: > > This commit adds a stage pre-load that could > check or modify an image. > > For the moment, only a header with a signature is > supported. This header has this format: > - magic : 4 bytes > - image size : 4 bytes > - signature : n bytes > - padding : up to header size > > The stage use a node /image/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 > - signature-size : size of the signature (in the header) > - mandatory : set to yes if this sig is mandatory > - public-key (madatory) : 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 > - ... > > Signed-off-by: Philippe Reynes > --- > boot/Kconfig | 33 +++++ > boot/Makefile | 1 + > boot/image-pre-load.c | 291 ++++++++++++++++++++++++++++++++++++++++++ > include/image.h | 9 ++ > 4 files changed, 334 insertions(+) > create mode 100644 boot/image-pre-load.c Reviewed-by: Simon Glass Please see below. > > diff --git a/boot/Kconfig b/boot/Kconfig > index d3a12be228..3856580af6 100644 > --- a/boot/Kconfig > +++ b/boot/Kconfig > @@ -958,6 +958,39 @@ config AUTOBOOT_MENU_SHOW > > endmenu > > +menu "Image support" > + > +config IMAGE_PRE_LOAD > + bool "Image pre-load support" > + help > + Enable image pre-load support > + > +config SPL_IMAGE_PRE_LOAD > + bool "Image pre-load support within SPL" > + depends on SPL && IMAGE_PRE_LOAD > + help > + Enable image pre-load support in SPL Please expand all your help > + > +config IMAGE_PRE_LOAD_SIG > + bool "Image pre-load signature support" > + depends on IMAGE_PRE_LOAD > + select FIT_SIGNATURE > + select RSA > + select RSA_VERIFY_WITH_PKEY > + help > + Enable image pre-load signature support > + > +config SPL_IMAGE_PRE_LOAD_SIG > + bool "Image pre-load signature support witin SPL" > + depends on SPL_IMAGE_PRE_LOAD && IMAGE_PRE_LOAD_SIG > + select SPL_FIT_SIGNATURE > + select SPL_RSA > + select SPL_RSA_VERIFY_WITH_PKEY > + help > + Enable image pre-load signature support in SPL > + > +endmenu > + > config USE_BOOTARGS > bool "Enable boot arguments" > help > diff --git a/boot/Makefile b/boot/Makefile > index 2938c3f145..59752c65ca 100644 > --- a/boot/Makefile > +++ b/boot/Makefile > @@ -26,6 +26,7 @@ obj-$(CONFIG_$(SPL_TPL_)OF_LIBFDT) += image-fdt.o > obj-$(CONFIG_$(SPL_TPL_)FIT_SIGNATURE) += fdt_region.o > obj-$(CONFIG_$(SPL_TPL_)FIT) += image-fit.o > obj-$(CONFIG_$(SPL_)MULTI_DTB_FIT) += boot_fit.o common_fit.o > +obj-$(CONFIG_$(SPL_TPL_)IMAGE_PRE_LOAD) += image-pre-load.o > obj-$(CONFIG_$(SPL_TPL_)IMAGE_SIGN_INFO) += image-sig.o > obj-$(CONFIG_$(SPL_TPL_)FIT_SIGNATURE) += image-fit-sig.o > obj-$(CONFIG_$(SPL_TPL_)FIT_CIPHER) += image-cipher.o > diff --git a/boot/image-pre-load.c b/boot/image-pre-load.c > new file mode 100644 > index 0000000000..6ed21c3f51 > --- /dev/null > +++ b/boot/image-pre-load.c > @@ -0,0 +1,291 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Copyright (C) 2021 Philippe Reynes > + */ > + > +#include > +#include > +DECLARE_GLOBAL_DATA_PTR; > +#include > +#include > + > +#define IMAGE_PRE_LOAD_SIG_MAGIC 0x55425348 > +#define IMAGE_PRE_LOAD_SIG_OFFSET_MAGIC 0 > +#define IMAGE_PRE_LOAD_SIG_OFFSET_IMG_LEN 4 > +#define IMAGE_PRE_LOAD_SIG_OFFSET_SIG 8 > + > +#define IMAGE_PRE_LOAD_PATH "/image/pre-load/sig" > +#define IMAGE_PRE_LOAD_PROP_HEADER_SIZE "header-size" > +#define IMAGE_PRE_LOAD_PROP_ALGO_NAME "algo-name" > +#define IMAGE_PRE_LOAD_PROP_PADDING_NAME "padding-name" > +#define IMAGE_PRE_LOAD_PROP_SIG_SIZE "signature-size" > +#define IMAGE_PRE_LOAD_PROP_PUBLIC_KEY "public-key" > +#define IMAGE_PRE_LOAD_PROP_MANDATORY "mandatory" > + > +#ifndef CONFIG_SYS_BOOTM_LEN > +/* use 8MByte as default max gunzip size */ > +#define CONFIG_SYS_BOOTM_LEN 0x800000 > +#endif > + > +struct image_sig_header { > + u32 magic; > + u32 size; > + u8 *sig; > +}; > + > +struct image_sig_info { > + ulong header_size; > + char *algo_name; > + char *padding_name; > + u8 *key; > + int key_len; > + u32 sig_size; > + int mandatory; comments > +}; > + > +ulong image_load_offset; > + > +/* > + * This function gathers information about the signature check > + * that could be done before launching the image. > + * > + * return: > + * -1 => an error has occurred Can you use -Exxx instead? > + * 0 => OK > + * 1 => no setup > + */ > +static int image_pre_load_sig_setup(struct image_sig_info *info) > +{ > + const void *algo_name, *padding_name, *key, *mandatory; > + const u32 *header_size, *sig_size; > + int key_len; > + int node, ret = 0; > + > + if (!info) { > + printf("ERROR: info is NULL for image pre-load sig check\n"); log_err() ? > + ret = -1; > + goto out; > + } > + > + memset(info, 0, sizeof(*info)); > + > + node = fdt_path_offset(gd_fdt_blob(), IMAGE_PRE_LOAD_PATH); > + if (node < 0) { > + printf("INFO: no info for image pre-load sig check\n"); > + ret = 1; > + goto out; > + } > + > + header_size = fdt_getprop(gd_fdt_blob(), node, > + IMAGE_PRE_LOAD_PROP_HEADER_SIZE, NULL); > + if (!header_size) { > + printf("ERROR: no header-size for image pre-load sig check\n"); > + ret = -1; > + goto out; > + } > + > + algo_name = fdt_getprop(gd_fdt_blob(), node, > + IMAGE_PRE_LOAD_PROP_ALGO_NAME, NULL); > + if (!algo_name) { > + printf("ERROR: no algo_name for image pre-load sig check\n"); > + ret = -1; > + goto out; > + } > + > + padding_name = fdt_getprop(gd_fdt_blob(), node, > + IMAGE_PRE_LOAD_PROP_PADDING_NAME, NULL); > + if (!padding_name) { > + printf("INFO: no padding_name provided, so using pkcs-1.5\n"); > + padding_name = "pkcs-1.5"; > + } > + > + sig_size = fdt_getprop(gd_fdt_blob(), node, > + IMAGE_PRE_LOAD_PROP_SIG_SIZE, NULL); > + if (!sig_size) { > + printf("ERROR: no signature-size for image pre-load sig check\n"); > + ret = -1; > + goto out; > + } > + > + key = fdt_getprop(gd_fdt_blob(), node, > + IMAGE_PRE_LOAD_PROP_PUBLIC_KEY, &key_len); > + if (!key) { > + printf("ERROR: no key for image 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; > + info->sig_size = fdt32_to_cpu(*sig_size); > + > + mandatory = fdt_getprop(gd_fdt_blob(), node, > + IMAGE_PRE_LOAD_PROP_MANDATORY, NULL); > + if (mandatory && !strcmp((char *)mandatory, "yes")) > + info->mandatory = 1; > + > + out: > + return ret; > +} > + > +static int image_pre_load_sig_get_header_u32(struct image_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 image pre-load sig\n"); > + ret = -1; > + goto out; > + } > + > + tmp = header + offset; > + *value = be32_to_cpu(*tmp); > + > + unmap_sysmem(header); > + > + out: > + return ret; > +} > + > +static int image_pre_load_sig_get_magic(struct image_sig_info *info, > + ulong addr, u32 *magic) > +{ > + int ret; > + > + ret = image_pre_load_sig_get_header_u32(info, addr, > + IMAGE_PRE_LOAD_SIG_OFFSET_MAGIC, magic); > + > + return ret; > +} > + > +static int image_pre_load_sig_get_img_len(struct image_sig_info *info, > + ulong addr, u32 *len) > +{ > + int ret; > + > + ret = image_pre_load_sig_get_header_u32(info, addr, > + IMAGE_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; > + } > + > + if (*len == 0) { > + printf("ERROR: size of image (%u) is zero\n", *len); > + ret = -1; > + goto out; > + } > + > + out: > + return ret; > +} > + > +static int image_pre_load_sig_check(struct image_sig_info *info, ulong addr, int img_len) > +{ > + void *image; > + struct image_sign_info sig_info; > + struct image_region reg; > + u32 sig_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; Please use real error codes throughout. > + goto out; > + } > + > + memset(&sig_info, 0, sizeof(sig_info)); '\0' > + 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; > + > + sig = (uint8_t *)image + IMAGE_PRE_LOAD_SIG_OFFSET_SIG; > + sig_len = info->sig_size; > + > + ret = sig_info.crypto->verify(&sig_info, ®, 1, sig, sig_len); > + if (ret) { > + printf("ERROR: signature check has failed (err=%d)\n", ret); > + ret = -1; > + goto out_unmap; > + } > + > + printf("INFO: signature check has succeed\n"); > + > +out_unmap: > + unmap_sysmem(image); > + > + out: > + return ret; > +} > + > +int image_pre_load_sig(ulong addr) > +{ > + struct image_sig_info info; > + u32 magic, img_len; > + int ret; > + > + ret = image_pre_load_sig_setup(&info); > + if (ret < 0) > + goto out; > + if (ret > 0) { > + ret = 0; > + goto out; > + } > + > + ret = image_pre_load_sig_get_magic(&info, addr, &magic); > + if (ret < 0) > + goto out; > + > + if (magic != IMAGE_PRE_LOAD_SIG_MAGIC) { > + if (info.mandatory) { > + printf("ERROR: signature is mandatory\n"); > + ret = -1; > + } > + goto out; > + } > + > + ret = image_pre_load_sig_get_img_len(&info, addr, &img_len); > + if (ret < 0) > + goto out; > + > + ret = image_pre_load_sig_check(&info, addr, img_len); > + > + if (!ret) > + image_load_offset += info.header_size; > + > + out: > + return ret; > +} > + > +int image_pre_load(ulong addr) > +{ > + int ret = 0; > + > + image_load_offset = 0; > + > + if (CONFIG_IS_ENABLED(IMAGE_PRE_LOAD_SIG)) > + ret = image_pre_load_sig(addr); > + > + return ret; > +} > diff --git a/include/image.h b/include/image.h > index fd662e74b4..5f83e4c747 100644 > --- a/include/image.h > +++ b/include/image.h > @@ -48,6 +48,7 @@ struct fdt_region; > extern ulong image_load_addr; /* Default Load Address */ > extern ulong image_save_addr; /* Default Save Address */ > extern ulong image_save_size; /* Default Save Size */ > +extern ulong image_load_offset; /* Default Load Address Offset */ > > /* An invalid size, meaning that the image size is not known */ > #define IMAGE_SIZE_INVAL (-1UL) > @@ -1289,6 +1290,14 @@ struct crypto_algo *image_get_crypto_algo(const char *full_name); > */ > struct padding_algo *image_get_padding_algo(const char *name); > > +/** > + * image_pre_load() - Manage pre load header > + * ?? Please add a description here. > + * @param addr Address of the image > + * @return: 0 on success, -ve on error > + */ > +int image_pre_load(ulong addr); > + > /** > * fit_image_verify_required_sigs() - Verify signatures marked as 'required' > * > -- > 2.17.1 > Regards, Simon