From mboxrd@z Thu Jan 1 00:00:00 1970 From: Philippe REYNES Date: Fri, 7 Aug 2020 19:03:07 +0200 (CEST) Subject: Improvements to FIT ciphering In-Reply-To: References: Message-ID: <638649134.465093.1596819787967.JavaMail.zimbra@softathome.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 > 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