[PATCH 1/4] tools: mkeficapsule: add firmwware image signing

[Ilias Apalodimas <ilias.apalodimas@linaro.org>]

On Fri, May 14, 2021 at 10:09:46PM +0900, Masami Hiramatsu wrote:
> Hi all,
> 
> I think it's time to summarize the topics on this thread.
> 
> 1. tools/mkeficapsule, config options dependency
>   - The tools, especially useful and distributable tools like
> mkeficapsule should not be changed by the target board configuration.
>   - Since there are target boards which don't need capsule
> authentication, it should be configurable. That also can optimize the
> library dependency.
> 
> 2. tools/mkeficapsule, revert -K/-D options
>   - Since these options are for embedding a public key in the
> devicetree, that is not related to the capsule file. Also, the same
> feature can be provided by a simple shell script.
> 
> 3. capsule authentication, key embedding method
>   - Embedding key in the devicetree is too fragile, especially, the
> document says overwriting new device tree including key with fdt
> command. That is not for the product, only for proof of concept.
>   - Such a key should be embedded in the U-Boot, or hardware secure
> storage so that the user can not change it.
>     (BTW, I think there are more options, like embedding keys in SCP
> firmware, TF-A, or OP-TEE, outside of U-Boot)
> 
> 4. capsule authentication, authentication enablement
>   - The UEFI spec said IMAGE_ATTRIBUTE_AUTHENTICATION_REQUIRED can be
> supported but cleared (for the current running firmware). This means
> it is possible that the authentication feature is supported, but not
> enabled.
>   - For ensuring security, if U-Boot is compiled with
> CONFIG_EFI_CAPSULE_AUTHETICATE=y,
> IMAGE_ATTRIBUTE_AUTHENTICATION_REQUIRED must always be set.
> 
> Are there any other topics on this thread? and any other comments on
> these topics?

I think you already mentioned that, but the key retrieval, should be a
callback of some sort that each vendor/hardware can define to his own
special function and we keep the default as 'key is embedded in U-Boot'.
All of the above sound reasonable. I still think (1) is kinda useless, but
I'll leave up to the maintainers.

Thanks
/Ilias

> 
> Thank you,
> 
> 
> 2021?5?14?(?) 18:51 AKASHI Takahiro <takahiro.akashi@linaro.org>:
> 
> >
> > Heinrich,
> >
> > Can you please reply to each of my replies?
> > Otherwise, I don't know which one of my comments/opinions you agree to
> > and which one not.
> >
> > On Fri, May 14, 2021 at 10:45:48AM +0200, Heinrich Schuchardt wrote:
> > > On 5/14/21 9:13 AM, AKASHI Takahiro wrote:
> > > > > E.g for IMAGE_ATTRIBUTE_IN_USE
> > > > >
> > > > > AttributesSupported | AttributesSetting | Meaning
> > > > > --------------------+-------------------+--------------------
> > > > > 0                   | 0                 | state is unknown
> > > > > 0                   | 1                 | state is unknown
> > > > > 1                   | 0                 | image is not in use
> > > > > 1                   | 1                 | image is in use
> > > > We are discussing *_REQUIRED.
> > > > Can you give me the same table for *_REQUIRED?
> > > >
> > > > -Takahiro Akashi
> > > >
> > > >
> > >
> > > IMAGE_ATTRIBUTE_RESET_REQUIRED
> > >
> > > AttributesSupported | AttributesSetting | Meaning
> > > --------------------+-------------------+--------------------
> > > 0                   | 0                 | state is unknown
> > > 0                   | 1                 | state is unknown
> > > 1                   | 0                 | reset is not needed
> > >                     |                   | to complete upgrade
> > > 1                   | 1                 | reset is needed
> > >                     |                   | to complete upgrade
> > >
> > >
> > > IMAGE_ATTRIBUTE_AUTHENTICATION_REQUIRED
> > >
> > > AttributesSupported | AttributesSetting | Meaning
> > > --------------------+-------------------+--------------------
> > > 0                   | 0                 | state is unknown
> > > 0                   | 1                 | state is unknown
> > > 1                   | 0                 | signed and unsigned
> > >                     |                   | capsules are accepted
> > > 1                   | 1                 | capsules are only
> > >                     |                   | accepted after
> > >                     |                   | checking the signature
> >
> > So what?
> > This table shows there is a case where the authentication will be
> > skipped even if CONFIG_EFI_CAPSULE_AUTHETICATE is on and
> > it is completely compliant with UEFI specification.
> >
> > That is what I and Masami was discussing.
> >
> > > > > > > But as I mentioned in my comment against Sughosh's patch,
> > > > > > > the authentication process will be enforced only if the capsule has
> > > > > > > an attribute, IMAGE_ATTRIBUTE_AUTHENTICATION_REQUIRED.
> > > > > > >
> > > > > >
> > > > > > That would be a security desaster.
> >
> > So I said that you should discuss the topic in UEFI forum first
> > if you think so.
> >
> > -Takahiro Akashi
> >
> >
> > > For both bits AttributesSupported=0 does not make much sense.
> > >
> > > IMAGE_ATTRIBUTE_AUTHENTICATION_REQUIRED is a property of the current
> > > image and should only be deleted by installing a new capsule.
> > >
> > > A vendor might send you a special firmware image for unlocking your
> > > device after registering as a developer. Xiaomi handled it like this for
> > > one of my routers.
> > >
> > > Best regards
> > >
> > > Heinrich
> 
> 
> 
> --
> Masami Hiramatsu