Thanks Soby, Sumit, Sandrine and Joakim!

Agree with Joakim/Soby about encrypt then sign and sign then encrypt being okay, when authenticated encryption is used. I'd like to point out again, that the link provided by Sumit talks about encrypt-then-sign and sign-then-encrypt, using asymmetric encryption and asymmetric signing only, and as Joakim rightly pointed out(and as i did in my earlier email), does not necessarily apply here. They are not talking about symmetric encryption with asymmetric signing, which is what PSA-TBFU and TBBR are talking about. I usually don't like signing plain text and encrypt(not authenticated encryption, just encryption like aes-cbc etc) plain text due to https://moxie.org/blog/the-cryptographic-doom-principle/(written by the creator of the the WhatsApp end-to-end encryption protocol, this applies to symmetric encryption and symmetric signing/MAC's, but it applies to any ciphertext that is decrypted without verifying its integrity). If we were to sign the plain text then encrypt the firmware, the size of the encrypted file needs to be integrity protected as well, not just the bit that indicates that the file is encrypted. If not, when we decrypt the firmware image, we can perform some creative attacks on symmetric encryption, as specified in the link, and requires careful implementation of error handling/reporting on decryption errors.

I also like Soby's approach of having an attribute EP_ENCRYPTED and relying on platform_pre_image_load() and platform_post_image_load() to do the decryption and that addresses my non-spec related concerns. It also allows for decrypt-then-authenticate(which i wouldn't use) and authenticate-then-decrypt. It also puts the attribute that indicates firmware encryption in the image descriptor table which is integrity protected by virtue of it being in the ROM or being signed, and also leaves the FIP layer unaltered.

Thanks -Raghu

On January 27, 2020 at 6:42 AM, Soby Mathew Soby.Mathew@arm.com wrote:

On 27/01/2020 12:34, Joakim Bech via TF-A wrote:

on raw binaries are there so we can be sure that we're loading unmodified firmware coming from the one owning a private key corresponding to the public key hardcoded into the device (or via a hash of the public key). I think we all can agree

OK, I have finally managed to catch up on this thread. Apologies for the delayed response.

As Joakim mentions, I think both the mentioned cases ie. encrypt plain text -> sign and sign -> encrypt are valid and it depend on the threat model and security requirement.

I have had a brief look at the patch stack and coupling the feature to the FIP does not seem like a good idea to me( + the added complexity protecting the ToC). Currently meta data of the firmware images is passed OOB via the `bl_mem_params_node_t` descriptor to the BL images whereas this patch breaks that convention. It is better to follow the set convention and avoid dependency on FIP format (btw platforms need not use FIP format and can use other packaging formats).

The iv data to decrypt seems to be prepended to the encrypted file in the fip which is making custom manipulations file pointer manipulations which is raising some red flags.

https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/2495/8/driver...

IMO, this should not visible to be FIP driver. I am not sure of the best practice for sending iv data but if it has to be prepended, it should be FIP agnostic.

The TBBR assumes a single owner for all images whereas the TBFU supercedes this spec by allowing multiple owners for different images. We are in the process of enhancing the CoT for different Root of Trusts for different images and there needs to be capability to encrypt with different keys for different images based on ownership. The current implementation has some limitations like introducing a platform API invocation within the driver layer to get the single key which is not ideal IMO.

I haven't fully flushed out my ideas, but thoughts are based on enhancements done by Masahiro for adding decompression support for BL images. See https://github.com/ARM-software/arm-trusted-firmware/pull/1224 .

Basically, this patch series allows any filter to be setup before/after the images are loaded. It relies on pre-load and post load hooks which are platform specific to perform the filter operation. So, if a new image attribute `EP_ENCRYPTED` is added ep_info_exp.h, then BL2 needs to do the following in bl2_plat_handle_pre_image_load() { if EP_ENCRYPTED is set : load the image to SRAM and decrypt using crypto module }

This will cater for `decrypt-then-authenticate` flow.

The fip_tool no longer needs to be aware of encryption and the build process just needs to pipe the encrypted binaries to the fip_tool.

Similarly if `authenticate-then-decrypt` needs to be supported, then all that the platform needs to do is implement decrypt in bl2_plat_handle_post_image_load().

The platform can now use different keys to use for different BL images if it needs to do so.

Some usecases require the firmware images to be re-encrypted using HUK after firmware update (aka firmware binding) and following this design will allow this to be done as well.

Best Regards Soby Mathew