From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marek Vasut Date: Wed, 5 Jun 2019 03:30:56 +0200 Subject: [U-Boot] [EXT] Re: [PATCH 4/6] spl: mmc: support loading i.MX container format file In-Reply-To: References: <20190507130554.4598-1-peng.fan@nxp.com> <20190522080252.732e5b20@jawa> <20190522084622.7d63c32d@jawa> <20190522093407.4b941de6@jawa> <9faa828b-9ebc-8bc7-9232-6ce1ff8f75a8@denx.de> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 6/5/19 3:18 AM, Peng Fan wrote: >> Subject: Re: [EXT] Re: [U-Boot] [PATCH 4/6] spl: mmc: support loading i.MX >> container format file >> >> On 6/4/19 5:27 AM, Peng Fan wrote: >>>> Subject: Re: [EXT] Re: [U-Boot] [PATCH 4/6] spl: mmc: support loading >>>> i.MX container format file >>>> >>>> On 5/30/19 9:06 AM, Ye Li wrote: >>>>> On 2019/5/27 19:31, Marek Vasut wrote: >>>>>> Caution: EXT Email >>>>>> >>>>>> On 5/27/19 11:49 AM, Peng Fan wrote: >>>>>>> Hi Marek, Lukasz, >>>>>>> >>>>>>>> Subject: Re: [EXT] Re: [U-Boot] [PATCH 4/6] spl: mmc: support >>>>>>>> loading i.MX container format file >>>>>>>> >>>>>>>> Hi Marek, >>>>>>>> >>>>>>>> On 2019/5/22 19:41, Marek Vasut wrote: >>>>>>>>> Caution: EXT Email >>>>>>>>> >>>>>>>>> On 5/22/19 9:34 AM, Lukasz Majewski wrote: >>>>>>>>> [...] >>>>>>>>>>>>>>>> By using above approach we do have the NXP's "container" >>>>>>>>>>>>>>>> format only seen in the SPL (which is OK, as for example >>>>>>>>>>>>>>>> Samsung does similar thing with FBL/BL1). When SPL is >>>> "trused" >>>>>>>>>>>>>>>> we may use available facilities. >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> The issue to me is that sc_seco_authenticate could not >>>>>>>>>>>>>>> take a FIT image as input. >>>>>>>>>>>>>> >>>>>>>>>>>>>> Is the sc_seco_authenticate an API accessible from SPL, >>>>>>>>>>>>>> U-Boot proper or Linux crypro engine driver? >>>>>>>>>>>>> >>>>>>>>>>>>> Yes, it is an API accessible in SPL/U-Boot stage. I do not >>>>>>>>>>>>> know about Linux crypto driver. >>>>>>>>>>>> >>>>>>>>>>>> Maybe it would be worth to check how Linux handle this? Maybe >>>>>>>>>>>> it would shed some more light on it? >>>>>>>>>>> >>>>>>>>>>> I am not familiar with that, so might be stupid question below. >>>>>>>>>>> Does it really matter? >>>>>>>>>> >>>>>>>>>> I would check it just out of curiosity. >>>>>>>>> >>>>>>>>> Yes, it matters, because there should be such API. How would >>>>>>>>> Linux authenticate e.g. userspace binaries if there wasn't one, >>>>>>>>> surely not by wrapping every single object into the custom >>>>>>>>> vendor-specific >>>> container ? >>>>>>>>> And if there is one, you can use it to authenticate raw binaries >>>>>>>>> from U-Boot SPL too, e.g. fitImage blobs with an associated >> signature. >>>>>>>>> >>>>>>>> >>>>>>>> iMX8 AHAB uses RSA key pair for authentication, the on-chip thing >>>>>>>> we called SRK is a array of public key hash which is dedicated >>>>>>>> for AHAB. It is not a real key. The real public key is in container. >>>>>>>> AHAB will check the public key with the on-chip SRK before using >>>>>>>> it to authenticate the image. >>>>>>>> Seco which contains the crypto engine on imx8 does not allow to >>>>>>>> use the SRK by user. No such API exported. >>>>>>>> And the fuse of SRK is locked, can't be read directly. >>>>>>>> >>>>>>>> Actually on imx6/imx7/imx8m, the SPL and u-boot are already using >>>>>>>> ROM HAB to implement the trust chain, like SPL authenticates >>>>>>>> u-boot, u-boot authenticatse kernel. We just follow this same way >>>>>>>> on imx8, the difference is >>>>>>>> imx8 needs container format for signed image. We prefer directly >>>>>>>> loading container image than fit image. >>>>>>>> If we pack fit image into container, obviously this will cause >>>>>>>> one more >>>> copy. >>>>>>>> As a boot loader, isn't it better to have more image format supported? >>>>>> >>>>>> If the functionality of the new image format is a subset of already >>>>>> present image format, then no, that's called a duplication. >>>>>> >>>>>>>> We >>>>>>>> don't force to use container, just set it as default. Users still >>>>>>>> can choose fit or raw image. >>>>>> >>>>>> They can. however they cannot authenticate the fitImage because the >>>>>> firmware doesn't provide the necessary API for that ? >>>>>> >>>>>>> >>>>>>> Do you have more comment? >>>>>> >>>>>> How could Linux use this iMX8 chain of trust stuff to authenticate e.g. >>>>>> userspace binaries ? It's the same thing as authenticating blob in >>>>>> a fitImage. >>>>>> >>>>> >>>>> Userspace binaries don't use the same key pair. They are signed by >>>>> software vendors' key. The private key for chip secure boot is only >>>>> hold by >>>> device manufacturer. >>>>> For example, android needs to authenticate a signed APK. Its public >>>>> key (Key >>>> A) is in system FS. >>>>> iMX trust chain only reaches to kernel + ramdisk. Then the chain >>>>> hands over >>>> to android. >>>>> In ramdisk, android puts another public Key (Key B) used by >>>>> dm-verify for system FS. So once system FS is verified ok, then the >>>>> public key A becomes trusted. Finally we can use public key A for APK >> authentication. >>>> >>>> So can we put a public key into the SPL and use it to verify a fitImage ? >>> >>> Technically doable. But compared with the current approach that reuse >>> ROM public API, Using crypto driver in SPL will be complicated and >>> code size larger without calling ROM API. >>> >>> I do not understand the problem the SPL loading NXP i.MX8 container >> format. >>> SPL should only support raw and fit format? vendor format is not >>> allowed and not accepted? >> >> The problem I have is with the duplication of functionality -- the iMX8 custom >> format does exactly the same as fitImage, except differently and with smaller >> user base, thus with less users and reviewers and thus with less potential >> bugfixes, which I think in crypto code is important. > > The change to spl mmc common code is as below: > diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c > index bf53a1dadf..6320af055b 100644 > --- a/common/spl/spl_mmc.c > +++ b/common/spl/spl_mmc.c > @@ -79,6 +79,16 @@ int mmc_load_image_raw_sector(struct spl_image_info *spl_image, > load.bl_len = mmc->read_bl_len; > load.read = h_spl_load_read; > ret = spl_load_simple_fit(spl_image, &load, sector, header); > + } else if (IS_ENABLED(CONFIG_SPL_LOAD_IMX_CONTAINER)) { > + struct spl_load_info load; > + > + load.dev = mmc; > + load.priv = NULL; > + load.filename = NULL; > + load.bl_len = mmc->read_bl_len; > + load.read = h_spl_load_read; > + > + ret = spl_load_imx_container(spl_image, &load, sector); > } else { > > If IMX_CONTAINER is not preferred I never implied that. >, I'll change it to CONFIG_SPL_LOAD_VENDOR_FORMAT. > In this way, only i.MX users need to take care container format, non i.MX users no need > to care about that. That would make it even worse, since if we follow this course of development, I suspect iMX9 will have another different container format and the list will grow on. > It is not duplication of FIT. Container support the similar function of FIT image, but it is not > only that. So what is it ? I don't think I get it. Why would I, as an iMX8 user, want to pick custom new vendor-specific format over years-proven generic fitImage? What is the selling point here ? -- Best regards, Marek Vasut