Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

boot_copy_region uses boot_encrypt before boot_enc_init if a swap is continued #2183

Open
Olstyle opened this issue Jan 21, 2025 · 5 comments
Labels
crypto Encryption support

Comments

@Olstyle
Copy link

Olstyle commented Jan 21, 2025

swap_run will reach down to boot_copy_region which always calls boot_encrypt for encrypted images, but the AES context is only initialized via boot_enc_load if a swap was NOT ongoing.
This effectively breaks swap continuation for encoded images if the used crypto library relies on initialization.
Which is the case for mbedtls_aes_init and will pretty much always be the case for HW based implementations.

The following screenshots show my HW implementation failing, but according to my review, mbedtls will fail at the same point!

Image
Image

@de-nordic de-nordic added the crypto Encryption support label Jan 21, 2025
@de-nordic
Copy link
Collaborator

What about loop doing the boot_enc_init here

if (bs->enckey[slot][i] != 0xff) {
break;
}
}
boot_enc_init(BOOT_CURR_ENC(state), slot);
if (i != BOOT_ENC_KEY_SIZE) {
boot_enc_set_key(BOOT_CURR_ENC(state), slot, bs);
}
}

@Olstyle
Copy link
Author

Olstyle commented Jan 21, 2025

What about loop doing the boot_enc_init here

mcuboot/boot/bootutil/src/loader.c

Lines 1625 to 1635 in 0674798

     if (bs->enckey[slot][i] != 0xff) { 
         break; 
     } 
 } 

 boot_enc_init(BOOT_CURR_ENC(state), slot); 

 if (i != BOOT_ENC_KEY_SIZE) { 
     boot_enc_set_key(BOOT_CURR_ENC(state), slot, bs); 
 } 

}

Your are right, this seems to be the fixed in main, but only recently
7e3a1ce
I am working on the latest release 2.1.0, which still had that issue. So actually you already fixed it but didn't make a bugfix release out of it.

@de-nordic
Copy link
Collaborator

Your are right, this seems to be the fixed in main, but only recently 7e3a1ce I am working on the latest release 2.1.0, which still had that issue. So actually you already fixed it but didn't make a bugfix release out of it.

True. My bad.

@de-nordic
Copy link
Collaborator

Can we close the issue?

@Olstyle
Copy link
Author

Olstyle commented Jan 23, 2025

Swap continuation with the favored crypto library (since tinycrypt is discontinued) is broken in the release version.
To me this sounds like a patch release would be reasonable.
At least that's what I would do in my project.
From the pure POV of main this issue it can be considered solved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crypto Encryption support
Projects
None yet
Development

No branches or pull requests

2 participants