boot: Add support of xip encryption mode for swap#2685
Conversation
d3zd3z
left a comment
There was a problem hiding this comment.
A couple of specific comments, notably that I believe we should change to signing the ciphertext and not the plaintext, which avoids having to decrypt the image in software.
A few other things, Zephyr seems to be missing CMake changes to allow this code to be compiled.
It is important to explain the the IV calculation, and for example that either the key itself must be unique per image, or the extended TLV must be used to ensure that the IV is different for each image encrypted.
There are some memory issues, memset on stack variables is generally optimized away, and some uses of memcmp instead of a constant time comparison.
Also, it isn't clear to me what the 32 bytes of zero padding are after the salt.
I think the missing calls to zeroize concern prk and T? But, other memset calls need to zeroize. And we need a platform-specific correct zeroize (which can be rather difficult to write).
And, the Zephyr kconfig could have a depends on !BOOT_ENCRYPT_IMAGE to detect the misconfiguration early, rather than only after a compile failure.
| computed as: | ||
|
|
||
| ``` | ||
| counter = (encryption_address + offset) / 16 |
There was a problem hiding this comment.
I'm not clear what these two values are. Below, offset is described as the byte offset within the image. But "encryption address" is not explained. Is this the base address of the first slot? Please make sure to explain it, especially that it is not a property of which slot the image resides in.
| * Compute SHA-256 hash over header + decrypted payload + protected TLVs. | ||
| * | ||
| * Mirrors bootutil_img_hash() logic but uses boot_decrypt_xip() for | ||
| * payload decryption instead of boot_enc_decrypt(). |
There was a problem hiding this comment.
This, I think is a bit of a fundamental problem here. With swap-based software decryption, the image signature is over the plaintext. This is because for the normal run case, the ciphertext is not available. For XIP, I think it makes much more sense to perform the signature over the ciphertext. This will allow the image to be verified without ever needing to do software decryption (with a goal that boot_decrypt_xip() is not present, and that the key is only used to give to the hardware and forgotten).
| * a strong override of this function. | ||
| */ | ||
| __attribute__((weak)) | ||
| fih_ret boot_image_check_hook(int img_index, int slot) |
There was a problem hiding this comment.
As mentioned above, I think this verification will make more sense if we change XIP encryption to sign the ciphertext instead of the plaintext.
But, it is also important to ensure that with encryption, signature verification is mandatory. It shouldn't be possible to build a configuration that has encryption validates the entire ciphertext before loading the encryption key into the hardware.
Without the signature verification, an attacker can trivially XOR parts of the image, which will apply as a XOR of the plaintext as well. Well known areas, such as vectors can easily be manipulated to refer to non-checked code as an attack.
|
|
||
| void xip_enc_zeroize(void) | ||
| { | ||
| (void)memset(xip_keys, 0, sizeof(xip_keys)); |
There was a problem hiding this comment.
There are some various zeroize implementations in the tree, and perhaps this needs to be generalized more. However, beyond this, there doesn't seem to be anything that ever calls this function.
| bootutil_aes_ctr_drop(&aes_ctr); | ||
|
|
||
| /* Zeroize key from stack */ | ||
| (void)memset(key, 0, sizeof(key)); |
There was a problem hiding this comment.
This needs to use a platform provided zeroize, as compilers are free to optimize away a memset of a stack variable. This applies in all instances throughout the code.
| return -1; | ||
| } | ||
|
|
||
| if ((tlv_len < ECIES_STD_TLV_SIZE) || (tlv_len > ECIES_EXT_TLV_SIZE)) { |
There was a problem hiding this comment.
This check will allow a tlv with length anywhere between STD and EXT TLV size. However, the code below will only check if the value is greater than STD, and if so, assume that the extended data is available. If the tlv_len is between these values, this will result in that code reading from uninitialized memory.
This check should require the tlv_len to be either the STD or the EXT sizes, and not in between.
|
Hi, This PR gives the impression that it provides generic support for encrypted XIP for other vendors. However, not all implementations are based on AES-CTR, so the image has to be re-encrypted using the platform’s encryption scheme. In addition, some implementations cannot set the initial AES-CTR offset value (base address), as it is hardwired to be computed from the beginning of the flash memory. This could be fixed on imgtool side which could provide an argument for initial offset, so the computed cipher could be readable for the implementation. |
Hi, thanks for the review.
|
13438a2 to
20af253
Compare
fbac3e1 to
c0c7885
Compare
Signed-off-by: INFINEON\DovhalA <artem.dovhal@infineon.com> Security properties doc. update
2a55134 to
828d030
Compare
| e.g. \${CMAKE_CURRENT_LIST_DIR} will allow referencing a file in that directory, these | ||
| will be automatically configured by the build system. | ||
|
|
||
| config BOOT_ENCRYPT_XIP |
There was a problem hiding this comment.
My remaining question is why are there two Kconfigs here? That leaves four combinations that could be selected, isn't this just a single configuration?
Signed-off-by: INFINEON\DovhalA <artem.dovhal@infineon.com>
5be5d2e to
f3e3c06
Compare
Signed-off-by: DOAR-Infineon <122998278+DOAR-Infineon@users.noreply.github.com> Signed-off-by: INFINEON\DovhalA <artem.dovhal@infineon.com>
f3e3c06 to
4516240
Compare
d3zd3z
left a comment
There was a problem hiding this comment.
In addition to the inline comments, there don't seem to be any tests that test for failures (corrupting an image, and showing validation failing, for example).
| code. The mandatory signature, computed over the ciphertext, closes this | ||
| attack vector before the AES key is ever loaded into the hardware. | ||
|
|
||
| * **Every image must use a unique encryption key and IV.** This is a hard |
There was a problem hiding this comment.
Here we document "unique encryption key and IV". The code seems to allow the IV to be entirely zero, but it is unclear to me if that matches this property. Realistically, I would recommend not allowing the zero-IV case. Is there hardware that needs that?
| "XIP encryption keeps images encrypted in both slots and does not use " \ | ||
| "the standard encrypt/decrypt-during-swap mechanism." | ||
| #endif | ||
|
|
There was a problem hiding this comment.
What about the combination of MCUBOOT_DIRECT_XIP and MCUBOOT_END_IMAGES_XIP? does this get rejected somewhere I've missed?
| uint32_t br_image_off; | ||
| #if defined(MCUBOOT_ENC_IMAGES_XIP) | ||
| /** AES-128 key for post-boot XIP hardware crypto setup. */ | ||
| uint32_t br_xip_key[4]; |
There was a problem hiding this comment.
What is the lifetime of these values? Is there something that explicitly wipes the keys?
| struct boot_rsp rsp; | ||
| /* ... boot_go(&rsp) ... */ | ||
|
|
||
| /* rsp.xip_key[4] -- 16-byte AES-128 key */ |
There was a problem hiding this comment.
The fields have been renamed in the struct, and that needs to be reflected here.
| copies the encryption key and initialisation vector (IV) that were extracted | ||
| during validation into the boot response structure: | ||
|
|
||
| * `rsp->xip_key` --- the AES key used by the hardware crypto region. |
There was a problem hiding this comment.
These fields have different names in the real struct.
Documentation updates. Signed-off-by: DOAR-Infineon <122998278+DOAR-Infineon@users.noreply.github.com> Signed-off-by: INFINEON\DovhalA <artem.dovhal@infineon.com>
bb20a52 to
0675c32
Compare
Signed-off-by: DOAR-Infineon <122998278+DOAR-Infineon@users.noreply.github.com> Signed-off-by: INFINEON\DovhalA <artem.dovhal@infineon.com>
0675c32 to
c089471
Compare
Standard
MCUbootencryption decrypts images during the swap process so thatthe
primary slotalways contains plaintext. This works well for internal flashbut prevents execute-in-place (XIP) from external memory, because decrypted
data would be exposed on the external bus.
XIP encryption keeps images encrypted in all slots. A hardware crypto engine
(e.g., Infineon SMIF with on-the-fly AES decryption) provides transparent
decryption during code execution. The CPU fetches ciphertext from flash and the
hardware returns plaintext — no software decryption step is needed at runtime.
How XIP encryption for swap works
Build-time encryption — Images are encrypted by
imgtool(oredgeprotecttools) using AES-CTR before signing. The AES-CTR noncefollows the edgeprotecttools format:
where
counter_LE32is the flash-area-relative byte offset encoded as alittle-endian 32-bit integer, and
nonce[0:12]is the first 12 bytes ofthe HKDF-derived
xip_iv. The counter is not slot-dependent; both theprimary and secondary slots store identical ciphertext.
Ciphertext signing — The SHA-256 hash and ECDSA signature are computed
over the encrypted image (header + ciphertext + protected TLVs). The
bootloader never performs software decryption during validation — the
signature covers the exact bytes that reside in flash.
Mandatory signature — Encrypted XIP images must be signed. The
bootloader rejects unsigned encrypted images to prevent XOR attacks: an
attacker who knows plaintext at a given offset could XOR it against the
ciphertext to recover the keystream. A mandatory signature closes this
attack vector.
Swap copies raw bytes — During an upgrade, swap moves encrypted blocks
between slots without any encryption or decryption.
MCUBOOT_ENC_IMAGESis not defined; the data is byte-for-byte identical regardless of which
slot it resides in.
Post-boot hardware setup — After
boot_go()returns, the applicationstartup code configures hardware crypto regions for each image. The AES key
and IV are available in the
boot_rspstructure:Keys are cleared from library-internal storage before jumping to the
application.
Bootloader validation flow — The
boot_image_check_hook()validatesencrypted XIP images without software decryption:
IMAGE_TLV_SHA256IMAGE_TLV_ECDSA_SIGIMAGE_TLV_ENC_EC256to extract AES key/IVExtended ECIES TLV format
XIP images use an extended ECIES TLV (177 bytes) that adds a 32-byte HKDF
salt field beyond the standard ECIES-P256 layout:
The
hkdf_saltis a per-image random value used as an HKDF diversifier,ensuring that every image derives a unique AES key and IV — a critical
requirement for AES-CTR security. The final 32-byte field is reserved for
future use and must be zero.
Platform requirements
A platform using XIP encryption must provide or override:
boot_image_check_hook(state, img_index, slot)— Performs complete imagevalidation including hash and signature verification over ciphertext and
ECIES key unwrap. A default (weak) implementation is provided by the XIP
encryption library (
xip_enc_validate.c). Thestatepointer providesaccess to boot loader context (slot flash areas, secondary offset for
swap-offset mode, max image size); it may be
NULLwhen called outside thenormal boot flow (e.g. serial recovery).
boot_xip_populate_rsp(img_index, rsp)— Called after successfulvalidation to copy the extracted AES key and IV into the
boot_rspstructure. The application uses these values to configure hardware crypto
regions.
Hardware crypto setup — After
boot_go()returns, the applicationmust configure the hardware crypto engine (e.g., SMIF crypto regions) using
the key material from
boot_rspbefore jumping to the application image.