Improvements to FIT ciphering

[Patrick Oppenlander <patrick.oppenlander@gmail.com>]

Hi Philippe,

On Fri, Sep 11, 2020 at 2:08 AM Philippe REYNES
<philippe.reynes@softathome.com> wrote:
>
> Hi Patrick,
>
> Sorry for the late answer, I was very busy in the beginning of september

No problem at all. I have a product deadline at the end of September
rapidly approaching so I am also extremely busy at the moment.

> >>
> >> I agree that IV should be set in the FIT.
> >>
> >> So in the dts, we may have:
> >> cipher {
> >> algo = "aes256";
> >> key-name-hint = "aeskey";
> >> iv = "aesiv";
> >> };
> >> or (I propose) :
> >> cipher {
> >> algo = "aes256";
> >> key-name-hint = "aeskey";
> >> iv-name-hint = "aesiv";
> >> iv-in-fit;
> >> };
> >>
> >> I think that both solution should work ...
> >>
> >> Have you planned to implement this change/feature ?
> >> (otherwise I will try to found some time for it,
> >> it is a really nice improvement).
> >
> > Hi Philippe,
> >
> > here is what I had in mind, in the .its we would put:
> >
> > cipher {
> > algo = "aes256";
> > key-name-hint = "aeskey";
> > };
> >
> > when mkimage processes this it opens /dev/urandom to generate a unique
> > IV. It then uses this IV to perform the encryption and writes it IV to
> > the .fit image like so:
> >
> > cipher {
> > algo = "aes256";
> > key-name-hint = "aeskey";
> > iv = <0xa16e090c 0x7e116bf8 0x75c44329 0x3278c74d>;
> > }
> >
> > I don't think there is a need for a "iv-in-fit" property and
> > "iv-name-hint" can be deprecated.
>
> I think that we should keep the compatibility with previous code.
> If a company/project has started to used iv in the u-boot device tree,
> may be they want to continue without changing the format.
>
> Idea 1:
> if there is a property "iv-name-hint" in the FIT image, mkimage uses
> the old format, and put the iv in the u-boot device tree. Otherwise,
> mkimage generate a random iv an put it in the FIT image (recommanded solution).
>
> Idea 2:
> We manage four cases according to the properties in the its file:
> - property "iv-name-hint" and no flag "iv-in-fit" :
>   => the iv is static and added in the u-boot device tree (actual scheme)
> - property "iv-name-hint" and flag "iv-in-fit" :
>   => the iv is static and added in the FIT image
> - no property "iv-name-hint" and no flag "iv-in-fit" :
>   => the iv is generated and added to the u-boot device tree
> - no property "iv-name-hint" and flag "iv-in-fit" :
>   => the iv is generated and added in the FIT image (recomanded scheme)

My opinion Idea 1 is simpler both in implementation & documentation,
which is probably enough for me to say it's the way we should go.
Perhaps others could contribute an opinion here?

>
> >> > However, if adding "hashed-nodes" and "hashed-strings" properties to
> >> > the image signature is acceptable we can still support signing
> >> > ciphered images with no problems.
> >>
> >> I think that everything should be added to the signature. I think it's
> >> simpler and more safe.
> >>
> >> Have you planned to implement this/propose a patch please ?
> >> (of course, if not, I will try to found some time)
> >
> > Unfortunately right now it is crunch time at $DAYJOB to meet a
> > deadline by the end of September, so I don't have much (if any) time
> > to dedicate to working on U-Boot right now.
> >
> > There are actually five issues on my list to address in U-Boot/mkimage:
> >
> > * mkimage needs to generate encryption IV using /dev/urandom
> > * FIT image signatures need to include cipher node
> > * AES-GCM cipher support
> > * mkimage -B option doesn't zero padding bytes
> > * mkimage -B option unnecessarily pads the end of the image
>
> I've got a lot of work too,  so I can't do all those features.
> But I'll try to work on the (random) IV generation and set it in
> the FIT image.

I didn't mean to dump my whole bug list on you :)

In my opinion the image signature covering the cipher node is also
quite important if you have time.

I'll make sure to send you an email if I start on any of this to make
sure we aren't duplicating effort.

> > I was planning on working through these when I get time, but I have
> > not started on any of them yet. So, if you have time (and energy),
> > please, go ahead :)
>
> I'll do my best to start this work.

That's great news,

Patrick