Heinrich,
You are discussing two different issues:
1. if we should remove "-K/-D" options from mkeficapsule
2. if it is safe or not to store a key in device tree
It makes the discussion in this thread confusing.
On Thu, May 13, 2021 at 07:42:23PM +0200, Heinrich Schuchardt wrote:
> On 5/13/21 9:13 AM, AKASHI Takahiro wrote:
> > On Thu, May 13, 2021 at 07:08:12AM +0200, Heinrich Schuchardt wrote:
> > > On 5/13/21 4:33 AM, AKASHI Takahiro wrote:
> > > > On Wed, May 12, 2021 at 12:01:32PM +0200, Heinrich Schuchardt wrote:
> > > > > On 12.05.21 10:01, Ilias Apalodimas wrote:
> > > > > > On Wed, May 12, 2021 at 04:49:02PM +0900, Masami Hiramatsu wrote:
> > > > > > > Hi Ilias,
> > > > > > >
> > > > > > > 2021年5月12日(水) 16:21 Ilias Apalodimas
> > > > > > > <ilias.apalodi...@linaro.org>:
> > > > > > > >
> > > > > > > > Akashi-san,
> > > > > > > >
> > > > > > > > On Wed, May 12, 2021 at 01:57:51PM +0900, AKASHI Takahiro wrote:
> > > > > > > > > As we discussed, "-K" and "-D" options have nothing to do with
> > > > > > > > > creating a capsule file. The same result can be obtained by
> > > > > > > > > using standard commands like:
> > > > > > > > > === signature.dts ===
> > > > > > > > > /dts-v1/;
> > > > > > > > > /plugin/;
> > > > > > > > >
> > > > > > > > > &{/} {
> > > > > > > > > signature {
> > > > > > > > > capsule-key = /incbin/("SIGNER.esl");
> > > > > > > > > };
> > > > > > > > > };
> > > > > > > > > ===
> > > > > > > > > $ dtc -@ -I dts -O dtb -o signature.dtbo signature.dts
> > > > > > > > > $ fdtoverlay -i test.dtb -o test_sig.dtb -v signature.dtbo
> > > > > > > > >
> > > > > > > > > So just remove this feature.
> > > > > > > > > (Effectively revert the commit 322c813f4bec ("mkeficapsule:
> > > > > > > > > Add support
> > > > > > > > > for embedding public key in a dtb").)
> > > > > > > > >
> > > > > > > > > The same feature is implemented by a shell script
> > > > > > > > > (tools/fdtsig.sh).
> > > > > > > >
> > > > > > > >
> > > > > > > > The only reason I can see to keep this, is if mkeficapsule gets
> > > > > > > > included
> > > > > > > > intro distro packages in the future. That would make end users
> > > > > > > > life a bit
> > > > > > > > easier, since they would need a single binary to create the
> > > > > > > > whole
> > > > > > > > CapsuleUpdate sequence.
> > > > > > >
> > > > > > > Hmm, I think it is better to write a manpage of mkeficapsule which
> > > > > > > also describes
> > > > > > > how to embed the key into dtb as in the above example if it is so
> > > > > > > short.
> > > > > > > Or, distros can package the above shell script with mkeficapsule.
> > > > > > >
> > > > > > > Embedding a key and signing a capsule are different operations but
> > > > > > > using the same tool may confuse users (at least me).
> > > > > >
> > > > > > Sure fair enough. I am merely pointing out we need a way to
> > > > > > explain all of
> > > > > > those to users.
> > > > >
> > > > > This is currently our only documentation:
> > > > >
> > > > > https://u-boot.readthedocs.io/en/latest/board/emulation/qemu_capsule_update.html?highlight=mkeficapsule
> > > >
> > > > As I mentioned several times (and TODO in the cover letter),
> > > > this text must be reviewed, revised and generalized
> > > > as a platform-independent document.
> > > > It contains a couple of errors.
> > > >
> > > > > For mkimage we have a man-page ./doc/mkimage.1 that is packaged with
> > > > > Debians u-boot-tools package. Please, provide a similar man-page as
> > > > > ./doc/mkeficapsule.1.
> > > >
> > > > So after all do you agree to removing "-K/-D"?
Regarding (1), you should clarify your opinion here first.
> > > I see no need to replicate in U-Boot what is already in the device tree
> > > compiler package.
> >
> > This is another reason that we should remove Sughosh's change.
Hereafter, you are talking about (2).
> > > In the current workflow the fdt command is used to load the public key.
> > > This is insecure and not usable for production.
> >
> > I totally disagree.
> > Why is using fdt command (what do you mean by fdt command, dtc/fdtoverlay?)
> > insecure?
>
> A user can load an insecure capsule.
>
> The fdt command is in /cmd/fdt.c and you are referring to it in
> board/emulation/qemu_capsule_update.rst.
OK, you meant U-Boot's fdt command.
> >
> > > The public key used to verify the capsule must be built into the U-Boot
> > > binary. This will supplant the -K and -D options.
> >
> > I don't get your point. You don't understand my code.
> >
> > Even with Sughosh's original patch, the public key (as I said,
> > it is not a public key but a X509 certificate in ESL format) is
> > embedded in the U-Boot's "control device tree".
>
> No, the ESL file it is not built into U-Boot's control device tree.
What I meant by "control device tree" is the feature
provided by OF_CONTROL+OF_EMBED with Sughosh's patch[1]
which is also a prerequisite for my patch series.
!config OF_EMBED
! bool "Embedded DTB for DT control"
! help
! If this option is enabled, the device tree will be picked up and
! built into the U-Boot image.
> A user is loading it and updating the control device tree.
>
> You shouldn't trust anything a user has loaded. You need at least the
> public key of the root CA built somewhere into U-Boot.
>
> The 'fdt resize' command may overwrite code. This is not what you want
> to do with the control device tree.
>
> If CONFIG_OF_LIVE=y, the active device tree is not at $fdtcontroladdr
> but in a hierarchical structure. You cannot update it via the fdt command.
>
> >
> > Even after applying my patch, this is true.
> >
> > Or are you insisting that the key should not be in the device tree?
>
> The public key of the root CA must not be in a place where it can be
> changed by a user while the device is in deployed mode.
UEFI secure boot and capsule update are totally independent concepts
as we discussed a long time ago. The notion of "deployed mode" is
only valid for secure boot.
> The device-tree based design is a good feasibility study but not
> suitable for production.
Nevertheless,
I agree, even I have already mentioned the similar concern in [2],
additionally saying we should turn off "fdt" command.
[1] https://lists.denx.de/pipermail/u-boot/2021-April/447183.html
[2] (oops, my message seems to have been lost.)
-Takahiro Akashi
> Best regards
>
> Heinrich
