Hi Sandrine,
On Thu, 5 Dec 2019 at 20:31, Sandrine Bailleux Sandrine.Bailleux@arm.com wrote:
Also, I am still trying to get my head around how this would integrate with a cryptographic engine where the key does not leave the chip. I can imagine that we could get the address of the encrypted firmware image from the FIP, pass that to a cryptographic engine, request it to decrypt it and store the result somewhere in Trusted RAM. In this case, we wouldn't call plat_get_fip_encryption_key(). Do you have any idea how we would pull this off? Like how the different modules (IO layer, crypto module, image parser module, ...) would integrate together?
In this case, I would expect platform to provide key identifier rather than actual key as part of plat_get_fip_encryption_key() which is then passed onto auth_decrypt() that is implementation specific to cryptographic engine in similar terms as currently done for mbedTLS backend.
Ah I see, so plat_get_fip_encryption_key() could return either the key itself or a key identifier. Just like plat_get_rotpk_info() can return either the key or a hash of it today. However, in the case of plat_get_rotpk_info(), it also returns some flags indicating which one it is. Don't we need something similar for plat_get_fip_encryption_key()? How will the crypto module be able to tell the difference between a key and a key ID otherwise? Or would you expect a given crypto module backend to always use either keys or key IDs, but not both?
It perfectly makes sense for crypto module backend to support both key ID and actual key. But I was thinking from use-case perspective that why would one like to expose key to the firmware if the crypto module could better protect it. And I could come up with one use-case where the SSK (burnt in SoC fuses) is provided as an actual key and BSSK (accessible only to crypto module) is provided via a key ID.
So yes I agree with you to provide additional flags field along with key buffer. Also, I think it makes sense to rename "plat_get_fip_encryption_key()" to "plat_get_fip_enc_key_info()".
I have some concerns around the generation of the initialization vectors in the encrypt_fw tool. Right now, IVs are simply a random sequence of bytes (obtained through a call to OpenSSL's RAND_bytes() API). Now, I would imagine that RAND_bytes() is typically based on a good random number generator and thus will generate different sequences every time it is called. At least, as long as it is called from the same machine every time. But what if we encrypt a new FIP bundle from a different machine, say in the context of a firmware update? Is it not possible that it might choose the same IV out of bad luck?
Perhaps that's an issue left to provisioning/manufacturing time and is out of the scope here. But it worries me because AFAIU, the security of AES-GCM is critically undermined if the same nonce is used multiple times with the same key (see section 5.1.1. "Nonce reuse" in RFC 5116). If the encryption key is the SSK (rather than the BSSK) then I guess the probability is even higher, as it is shared amongst a class of devices.
Agree that "nonce" should be unique and using a random number generator available on build machine was an effort towards that. But thinking about the case that you have mentioned, I think we could have an optional user provided "nonce" as an input to "encrypt_fw" tool, so that the user is aware to randomly generate and provide a unique nonce.
Yes, I like your suggestion of specifying the nonce to the tool. But I think it should be the default behaviour then. You mention an *optional* user-provided nonce, I would like to suggest we make this mandatory instead. We could provide an option to request the tool to generate the nonce, intended for development purposes.
Making nonce as default and mandatory command line option seems to be overkill for a user. It may also lead to unsafe practices of hardcoding a nonce in Makefile, shell scripts etc. So I think we should keep this as optional only for a narrow use-case where people might not trust uniqueness property of RNG from multiple build machines.
BTW, I have already added nonce command line option as part of v3 patch-set [1]. Please have a look.
[1] https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/2496/2..3
Impact on memory footprint and performance
Do you know what the performance impact is when this feature is enabled in TF-A, to decrypt images at boot time? Obviously it depends on the platform and whether there is a dedicated cryptographic engine, and I suppose you cannot really get any relevant measurements out of QEMU but I would be interested if you've got any rough numbers.
Following are measurements based on qemu for mbedTLS software library based authenticated decryption:
BL31 plain: NOTICE: Load image time: 137us, size: 28KB BL31 encrypted: NOTICE: Load image time: 3979us, size: 28KB
BL32 plain: NOTICE: Load image time: 1791us, size: 360KB BL32 encrypted: NOTICE: Load image time: 36339us, size: 360KB
Thanks. So it's a 29% increase for BL31 and 20% for BL32. I would have naively expected similar percentages, any idea why we get such a difference between BL31 and BL32? I am just curious. Maybe it's down to how the crypto algorithm/mode of operation works under the hood?
Yes this additional increase for BL31 seems to be due to one time initialization operations carried on first invocation of mbedTLS gcm APIs, especially time taken to generate AES tables (see call to aes_gen_tables() in mbedtls/library/aes.c +576).
<snip>
encrypt_fw tool
We have some floating ideas around re-implementing the tools (fiptool, certtool) in a scripting language (possibly python) in the future and also doing a better job at sharing a common description of the list of images to boot/authenticate between the firmware and the host tools. But we're not there yet, so I agree that implementing this new tool in C from the same "mold" as fiptool and certtool is what makes the most sense today. It's just another tool we will have to rework if and when we get there.
Sounds like a good idea to have these tools being python based.
BTW, we noticed some copyright headers attributed to both Arm and Linaro and pre-dating year 2019, e.g. in https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/2496/3/tools/... :
- Copyright (c) 2015, ARM Limited and Contributors. All rights reserved.
- Copyright (c) 2019, Linaro Limited
I am guessing this is because this tool was derived from the existing cert_create tool code, is that right?
Yes, some files were derived from existing tools.
I did not understand why this new tool needs to know what image it is encrypting. For example, one possible invocation could be:
tools/encrypt_fw/encrypt_fw \ -k 1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef \ --soc-fw bl31.bin \ --soc-fw-enc bl31_enc.bin \ --tos-fw bl32.bin \ --tos-fw-enc bl32_enc.bin
Why not invoking the tool once per image instead? As in:
encrypt_fw -k key -in ifile -out ofile
for BL31, then for BL32? Does the tool do anything different based on the type of image it receives?
"encrypt_fw" tool doesn't infer anything based on image type but image types were added for more user visibility and ease of use as follows:
- Provides the capability to encrypt multiple firmwares on single invocation.
I am not really convinced that this is a useful feature. I would rather have an external script (or the build system) invoking the tool multiple times, once per firmware image. Putting that complexity in the tool itself seems unnecessary to me.
Also, it makes the tool TBBR specific, as it has to know the list of images it's allowed to encrypt. Unfortunately, we already have this TBBR knowledge embedded into the fiptool/cert_create tool today but we would like to change that in the future. It does not scale well with new images or alternative chains of trust.
Ok, fair enough. My initial thoughts were to align with fiptool/cert_create tool, but since we plan to get away with image knowledge built into these tools so I am fine to make "encrypt_fw" tool image agnostic as well.
- Restricts usage of tool for FIP payloads only.
Why would we want that?
Not a major issue, just to tell user which fw images supports encryption.
- Better align with Makefile framework to build command line args
while building different images and finally invoke tool at once before creating FIP payload.
Could we not invoke the tool for each image as we go along? As in, we build BL31 and just after we generate the encrypted version of it. Then we build BL32 and its encrypted version. And so on. While we do that, we build the fiptool command line that will indeed put all the encrypted images in the final FIP image. How does that sound?
Sounds good, will give it a try.
-Sumit
Regards, Sandrine