Improvements to FIT ciphering

[Philippe REYNES]

Hi Simon and Patrick,

Sorry, I've missed this serie of patches.
I check them and send a review ASAP.

Regards,
Philippe


> +Philippe Reynes too
> 
> 
> On Mon, 27 Jul 2020 at 16:50, Patrick Oppenlander
> <patrick.oppenlander at gmail.com> wrote:
>> 
>> On Fri, Jul 24, 2020 at 12:06 PM Patrick Oppenlander
>> <patrick.oppenlander at gmail.com> wrote:
>> > 
>> > Hi,
>> > 
>> > I recently posted some patches to the list [1], [2], [3] to address
>> > some issues with the cipher support in mkimage. Hopefully someone gets
>> > a chance to review these patches as I think mkimage is a bit broken
>> > without them.
>> > 
>> > While considering using U-Boot cipher support in a product I work on,
>> > I have convinced myself that the handling of the encryption IV could
>> > be better, especially given that mkimage is using AES-CBC mode.
>> > Please, correct me if I have missed something.
>> > 
>> > 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.
>> > 
>> > 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).
>> > 
>> > 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.
>> > 
>> > 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.
>> > 
>> > 
>> > Kind regards,
>> > 
>> > Patrick
>> > 
>> > [1] https://lists.denx.de/pipermail/u-boot/2020-July/420399.html
>> > [2] https://lists.denx.de/pipermail/u-boot/2020-July/420400.html
>> > [3] https://lists.denx.de/pipermail/u-boot/2020-July/420401.html
>> 
>> Hi Simon,
>> 
>> I posted this writeup to the u-boot list and forgot to CC you. Sorry about that.
>> 
>> Patrick