Hi Patrick,
Sorry for this late anwser, I was very busy this week.
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.
I prefer to keep the support of aes-cbc, and I like the idea of storing the IV in the FIT image.
But I don't really understand the issue with iv-name-hint. To stay compatible, for example, we could simply add a propert "iv-store-in-fit" in the device tree.
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.
I also think that AES-GCM is a really good idea.
But I prefer to keep aes-cbc support. And to go further, I think we may support several algo (for example AES-CTR). The algo choice may change depending on the project. The boot speed may be very important (or not).
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 see the issue, but I haven't looked deeply ....
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?
We may provide a warning, but not allowing it seems a bit "hard". Is it really problematic to not sign the cipher node ?
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.
I agree that supporting AES-GCM would increase the security, so it is a really good idea. But, I think that we should not impose aes-gcm.
Patrick
[1] https://lists.denx.de/pipermail/u-boot/2020-July/421989.html
In few words, I like the idea of supporting AES-GCM.
Best regards, Philippe