R: Fit Signature booting without public key

[Manorit Chawdhry]

Hi,

On 16:51-20230530, Pegorer Massimo wrote:
> Hi,
> 
> > Inviato: venerdì 26 maggio 2023 15:00
> > Oggetto: Re: Fit Signature booting without public key
> > 
> > On Fri, May 26, 2023 at 02:22:38PM +0530, Manorit Chawdhry wrote:
> > > Hi Tom,
> > >
> > > On 10:05-20230525, Tom Rini wrote:
> > > > On Thu, May 25, 2023 at 11:21:44AM +0530, Manorit Chawdhry wrote:
> > > > > Hi Tom,
> > > > >
> > > > > On 11:30-20230516, Tom Rini wrote:
> > > > > > On Tue, May 16, 2023 at 12:11:24PM +0530, Manorit Chawdhry wrote:
> > > > > >
> > > > > > > Hi All,
> > > > > > >
> > > > > > > I recently came upon a discussion that had happened a while back
> > [0].
> > > > > > > I want to continue the discussion as I believe the issue still
> > > > > > > persists and the checks around fit signature booting are still
> > > > > > > the same, that allows booting the fit without changing the uboot
> > dtb.
> > > > > > >
> > > > > > > Allowing the signed fit image without this seems to be a
> > > > > > > bypass that is available and should not be allowed without any
> > > > > > > gate to it for people who'd like to enforce these signing
> > > > > > > checks. Let me know if there is a config already available for
> > > > > > > it and if not, are there any plans to enable such a config in
> > > > > > > future. Would like to hear your opinions on this as I believe this
> > should be fixed as soon as possible.
> > > > > > >
> > > > > > > [0]:
> > > > > > > https://u-boot.denx.narkive.com/dEClg9dW/signed-fit-image-boot
> > > > > > > s-without-public-key
> > > > > >
> > > > > > Yes, can you please reproduce the issue in question on the
> > > > > > current tree, with a supported platform and provide the
> > > > > > defconfig and steps you used for this issue? Thanks.
> > > > > >
> > > > > > --
> > > > >
> > > > > I've created a branch with some custom patches to make the
> > > > > fitimage booting currently, please try with the branch and the
> > > > > fitimage that are also committed [0].
> > > > >
> > > > > The devices that I've tested this with is j721e-hs-evm, the
> > > > > defconfig to use for the builds are j721e_evm_r5_defconfig and
> > > > > j721e_evm_a72_defconfig. Although not synced up with the latest
> > > > > changes but for reference the SDK documentation can help if required
> > [1].
> > > > >
> > > > > Attached the logs for reference with the signed fitimage and an
> > > > > unsigned uboot without any modifications[2].
> > > > >
> > > > > [0]: https://github.com/manorit2001/u-boot/tree/fit-image-poc
> > > > > [1]:
> > > > > https://software-dl.ti.com/jacinto7/esd/processor-sdk-linux-jacint
> > > > > o7/08_06_00_11/exports/docs/linux/Foundational_Components/U-
> > Boot/U
> > > > > G-General-Info.html
> > > > > [2]:
> > > > > https://gist.github.com/manorit2001/3c49cfc19bf937783efb75fd4cddc5
> > > > > 8f
> > > >
> > > > I don't see the problem there, but please go and investigate what
> > > > problem you're seeing.  The intention is that yes, with the
> > > > appropriate CONFIG settings, you can set U-Boot to only boot signed
> > > > FIT configurations and unsigned ones should not boot.
> > > >
> > >
> > > It boots with an unsigned fit too btw for reference [0]. The whole
> > > logic behind the authentication is that uboot stores the public key
> > > that is used for verification of the signed fit but apparantely if I
> > > haven't kept the public key in uboot dtb the fit images still boot
> > > whether signed or unsigned as that check is not guarded by anything if
> > > you see in the codebase[1].
> 
> Which uboot behaviour would you expect? If uboot is missing of the public key, it has no way to check signatures. Therefore either it boots everything (signed and unsigned) or it boots nothing (signed and unsigned). The second means it would be totally useless. So the first seems the only reasonable behaviour, IMO.
> 

I would actually expect it to through some errors telling about all this
instead of silently booting it, if the configurations of the dtb and the
signing ain't proper I don't believe there is any way to know about any
of this as it just boots up without any hint about it whatsoever, I
believe if this is enabled then there should atleast be an optional
config that allows people to get their environment right for validating
all this.

Regards,
Manorit

> Massimo
> 
> > > [0]:
> > > https://gist.github.com/manorit2001/80c9242641141ac5bce0e335ea8a9f1a
> > > [1]:
> > > https://github.com/u-boot/u-
> > boot/blob/6dcee70692601bd3296c86ac07d0317b
> > > f06d2b7b/boot/image-fit-sig.c#L491-L496
> > 
> > Alright, please let us know what you find as indeed if you've set all the right
> > options it's not supposed to boot unsigned images.
> > 
> > --
> > Tom