Re: [PATCH] Revert "efi_capsule: Move signature from DTB to .rodata"

[KASHI Takahiro <takahiro.akashi@linaro.org>]

On Mon, Aug 02, 2021 at 08:44:41AM -0600, Simon Glass wrote:
> Hi,
> 
> On Mon, 2 Aug 2021 at 01:15, KASHI Takahiro <takahiro.akashi@linaro.org> wrote:
> >
> > On Sun, Aug 01, 2021 at 08:47:15PM -0600, Simon Glass wrote:
> > > Hi Ilias,
> > >
> > > On Sun, 1 Aug 2021 at 20:28, Ilias Apalodimas
> > > <ilias.apalodimas@linaro.org> wrote:
> > > >
> > > > Hi Simon,
> > > >
> > > > On Sun, Aug 01, 2021 at 07:46:21PM -0600, Simon Glass wrote:
> > > > > This was unfortunately applied despite much discussion about it being
> > > > > the wrong way to implement this feature.
> > > >
> > > > No this was applied *before* the discussion, not despite.
> > >
> > > Oh sorry...I didn't notice either way. Normally there is an email on
> > > the patch saying it was applied. Perhaps I missed it.
> > >
> > > >
> > > > >
> > > > > Revert it before too many other things are built on top of it.
> > > >
> > > > I don't really mind if this gets reverted but there's things that haven't
> > > > been answered on that discussion [1] and my concern is what happens if
> > > > CONFIG_OF_EMBED is not selected.
> > >
> > > Can we start a new discussion perhaps? Or use one of the contributor
> > > calls to talk about it?
> > >
> > > We should not be using OF_EMBED except for testing.
> > >
> > > >
> > > > Also you need to revert the entire series, not just one of the patches,
> > > > as it changes the QEMU documentation for enabling authenticated capsule
> > > > updates, as well as the mkeficapsule app.
> > >
> > > Heinrich, do you have any thoughts on this?
> >
> > # I'm not Heinrich :)
> 
> Perhaps you could impersonate him :-) I ask because he had been doing
> a lot of EFI work.

I know. I was just kidding :)

> >
> > As far as the authentication logic itself is concerned,
> > it is utterly generic except how and from where a public key is
> > retrieved. (It can potentially be platform-specific.)
> > Moreover, mkeficapsule really doesn't care where the key is.
> >
> > So I don't think we need revert all those changes.
> 
> I agree. Having another look, I think perhaps three patches is enough.
> I will try again.
> 
> >
> > For testing, we can run a test on sandbox by having sandbox-specific
> > efi_get_public_key_data() function, i.e. we may want to contain
> > the key in a file on ESP or just in a specific flash partition.
> >
> > Obviously, it's not safe, but it's just a test to verify that the logic
> > is sane.
> >
> > If the discussion goes on for an unexpected spell of time,
> > I would like to take this workaround for now.
> 
> I think this effort should go back to before the change to putting
> things in rodata. That was when things went really off the rails.
> 
> With things back in the DT, you should be able to write a test with
> the existing sandbox build without any special-case code.

Well, that is the way I have adopted in my v1 patch[1].
I hope that the discussion be settled first.

-Takahiro Akashi

[1] https://lists.denx.de/pipermail/u-boot/2021-May/449575.html


> [..]
> 
> Regards,
> Simon