Improvements to FIT ciphering

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

Hi Simon & Philippe,

I've been thinking about this some more and have added a few points
below. I will need feedback before proposing any patches for the
remaining issues.

On Fri, Jul 24, 2020 at 12:06 PM Patrick Oppenlander
<patrick.oppenlander@gmail.com> wrote:
>
> Issue #1
> ========
>
> Currently, mkimage treats the IV in the same manner as the encryption
> key. There is an iv-name-hint property which mkimage uses to read the
> IV from a file in the keys directory. This can then be written to
> u-boot.dtb along with the encryption key.
>
> The problem with that is that u-boot.dtb is baked in at production
> time and is generally not field upgradable. That means that the IV is
> also baked in which is considered bad practice especially when using
> CBC mode (see CBC IV attack). In general it is my understanding that
> you should never use a key+IV twice regardless of cipher or mode.
>
> In my opinion a better solution would have been to write the IV into
> the FIT image instead of iv-name-hint (it's only 16 bytes!), and
> regenerate it (/dev/random?) each and every time the data is ciphered.
>

If U-Boot needs to continue supporting AES-CBC I think the only option
here is to deprecate the "iv-name-hint" property and replace it with
an "iv" property. This should be possible in a backward-compatible
manner.

>
> An even better solution is to use AES-GCM (or something similar) as
> this includes the IV with the ciphertext, simplifying the above, and
> also provides authentication addressing another issue (see below).
>

In my opinion it would be better to deprecate AES-CBC and replace it
with AES-GCM. I can see no advantages to supporting both, and can see
no reason to use AES-CBC over AES-GCM apart from a minor performance
advantage.

> Issue #2
> =======
>
> The current implementation uses encrypt-then-sign. I like this
> approach as it means that the FIT image can be verified outside of
> U-Boot without requiring encryption keys. It is also considered best
> practise.
>
> However, for this to be secure, the details of the cipher need to be
> included in the signature, otherwise an attacker can change the cipher
> or key/iv properties.
>
> I do not believe that properties in the cipher node are currently
> included when signing a FIT configuration including an encrypted
> image. That should be a simple fix. Fixing it for image signatures
> might be a bit more tricky.

I have posted a patch [1] which Philippe has reviewed which includes
the cipher node when signing a configuration.

It looks to be a much more intrusive (and incompatible) change to
include the cipher node in an image signature. Perhaps it would be
better for mkimage to issue a warning or error in this case and
document why it is not recommended?

I don't personally have a use case for signing an image. All of my FIT
images use configuration signatures instead. Is there a common use
case for which this needs to be solved or could we say that signing an
encrypted image is simply not supported?

> Issue #3
> =======
>
> Due to the nature of encrypt-then-sign U-Boot can verify that the
> ciphertext is unmodified, but it has no way of making sure that the
> key used to encrypt the image matches the key in u-boot.fit used for
> decryption. This can result in an attempt to boot gibberish and I
> think it can open up certain attack vectors.
>
> The best way I know of to fix this is to use an authenticated
> encryption mode such as AES-GCM or something similar.

I still think this is the best approach.

Patrick

[1] https://lists.denx.de/pipermail/u-boot/2020-July/421989.html