| Message ID | 1554480925-36-1-git-send-email-breno.lima@nxp.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Prabhakar Kushwaha |
| Headers | show |
| Series | [U-Boot] crypto: fsl: jr: Make job-rings assignment non-Secure dependent | expand |
On 05/04/2019 17:16, Breno Matheus Lima wrote: > + if ((IS_ENABLED(CONFIG_OPTEE) || > + !IS_ENABLED(CONFIG_ARMV7_BOOT_SEC_DEFAULT)) && > + !IS_ENABLED(CONFIG_CMD_DEKBLOB)) { So. How does this patch work if you want to do HABv4 encrypted authentication of an OP-TEE image prior to booting it ? i.e. You'll switch the job-ring ownership over to non-trusted world, call HAB and the BootROM will fail to authenticate. How about we improve the hab driver in u-boot ? 1. I notice somebody has already added a save_gd()/restore_gd() pair 2. How about a save_jr()/restore_jr() pair and 3. a bootrom_jr_setup() ? In pseudocode then do_hab_auth() { save_jr(); // save current job-ring context bootrom_jr_setup(); // set job-ring to bootrom mode hab_rvt_authenticate(); restore_jr(); // restore previous job-ring context } That gets us out of the nasty business of 1. Constraining the use case of CONFIG_OPTEE=y && authenticate(encrypted-dek-blob); 2. Pushing changes into upstream OP-TEE for job-ring assignment IMV - we should save and restore the job ring context so that all of this stuff will "just work" That should work for BootROM -> u-boot -> HABv4 auth(encryptedblob) -> Linux and BootROM -> u-boot -> run optee -> HABv4 auth(encrypted blob) -> Linux Basically you've described and additional dependency the BootROM has, so lets just "switch context" prior to calling into the BootROM and restore to a default non-secure job-ring assignment after. --- bod
On 06/04/2019 16:21, Bryan O'Donoghue wrote: > > 1. I notice somebody has already added a save_gd()/restore_gd() pair I'm referring to save_gd()/restore_gd() here : https://source.codeaurora.org/external/imx/uboot-imx/tree/arch/arm/mach-imx/hab.c?h=imx_v2018.03_4.14.78_1.0.0_ga but... it's still the right idea for this problem. Save context Switch to bootrom mode Restore context Job done
Hi Bryan, Em sáb, 6 de abr de 2019 às 12:21, Bryan O'Donoghue <bryan.odonoghue@linaro.org> escreveu: > > > > On 05/04/2019 17:16, Breno Matheus Lima wrote: > > + if ((IS_ENABLED(CONFIG_OPTEE) || > > + !IS_ENABLED(CONFIG_ARMV7_BOOT_SEC_DEFAULT)) && > > + !IS_ENABLED(CONFIG_CMD_DEKBLOB)) { > > So. > > How does this patch work if you want to do HABv4 encrypted > authentication of an OP-TEE image prior to booting it ? Thanks for your comments and for pointing this out, I agree that this can be an issue for OP-TEE users that may want to decrypt additional boot images at U-Boot level. The main issue is that all users are currently impacted in first authentication/decryption executed at BootROM level: BootROM -> HABv4 -> U-Boot We may need to come up with a solution suitable for all use cases. > > i.e. > > You'll switch the job-ring ownership over to non-trusted world, call HAB > and the BootROM will fail to authenticate. > > How about we improve the hab driver in u-boot ? > > 1. I notice somebody has already added a save_gd()/restore_gd() pair > 2. How about a save_jr()/restore_jr() pair and > 3. a bootrom_jr_setup() ? > > In pseudocode then > > do_hab_auth() > { > save_jr(); // save current job-ring context > bootrom_jr_setup(); // set job-ring to bootrom mode > hab_rvt_authenticate(); > restore_jr(); // restore previous job-ring context > } > > That gets us out of the nasty business of > > 1. Constraining the use case of CONFIG_OPTEE=y && > authenticate(encrypted-dek-blob); > > 2. Pushing changes into upstream OP-TEE for job-ring assignment > > IMV - we should save and restore the job ring context so that all of > this stuff will "just work" > > That should work for > > BootROM -> u-boot -> HABv4 auth(encryptedblob) -> Linux > > and > > BootROM -> u-boot -> run optee -> HABv4 auth(encrypted blob) -> Linux > > Basically you've described and additional dependency the BootROM has, so > lets just "switch context" prior to calling into the BootROM and restore > to a default non-secure job-ring assignment after. Yes, this can work for OP-TEE users decrypting additional boot images at U-Boot level, however all users won't be able to authenticate/decrypt the initial boot image at BootROM level. Out of reset CAAM job rings are assigned to TrustZone secure world and BootROM code is expecting blobs generated by the same environment. What about moving the job rings assignment to OP-TEE level? Something similar as we currently have in imx-optee-os project? https://source.codeaurora.org/external/imx/imx-optee-os/tree/core/drivers/caam/hal/imx_6_7/hal_jr.c?h=imx_4.14.78_1.0.0_ga#n31 U-Boot is running in TrustZone secure world in most of targets, in my opinion it makes sense to have at least 1 job ring assigned to TrustZone secure world. Best regards, Breno Lima
Hi Bryan, Seems that my last email didn't get in U-Boot mailing list, I'm sending again. Em sáb, 6 de abr de 2019 às 12:21, Bryan O'Donoghue <bryan.odonoghue@linaro.org> escreveu: > > > > On 05/04/2019 17:16, Breno Matheus Lima wrote: > > + if ((IS_ENABLED(CONFIG_OPTEE) || > > + !IS_ENABLED(CONFIG_ARMV7_BOOT_SEC_DEFAULT)) && > > + !IS_ENABLED(CONFIG_CMD_DEKBLOB)) { > > So. > > How does this patch work if you want to do HABv4 encrypted > authentication of an OP-TEE image prior to booting it ? Thanks for your comments and for pointing this out, I agree that this can be an issue for OP-TEE users that may want to decrypt additional boot images at U-Boot level. The main issue is that all users are currently impacted in first authentication/decryption executed at BootROM level: BootROM -> HABv4 -> U-Boot We may need to come up with a solution suitable for all use cases > > i.e. > > You'll switch the job-ring ownership over to non-trusted world, call HAB > and the BootROM will fail to authenticate. > > How about we improve the hab driver in u-boot ? > > 1. I notice somebody has already added a save_gd()/restore_gd() pair > 2. How about a save_jr()/restore_jr() pair and > 3. a bootrom_jr_setup() ? > > In pseudocode then > > do_hab_auth() > { > save_jr(); // save current job-ring context > bootrom_jr_setup(); // set job-ring to bootrom mode > hab_rvt_authenticate(); > restore_jr(); // restore previous job-ring context > } > > That gets us out of the nasty business of > > 1. Constraining the use case of CONFIG_OPTEE=y && > authenticate(encrypted-dek-blob); > > 2. Pushing changes into upstream OP-TEE for job-ring assignment > > IMV - we should save and restore the job ring context so that all of > this stuff will "just work" > > That should work for > > BootROM -> u-boot -> HABv4 auth(encryptedblob) -> Linux > > and > > BootROM -> u-boot -> run optee -> HABv4 auth(encrypted blob) -> Linux > > Basically you've described and additional dependency the BootROM has, so > lets just "switch context" prior to calling into the BootROM and restore > to a default non-secure job-ring assignment after. Yes, this can work for OP-TEE users decrypting additional boot images at U-Boot level, however all users won't be able to authenticate/decrypt the initial boot image at BootROM level. Out of reset CAAM job rings are assigned to TrustZone secure world and BootROM code is expecting blobs generated by the same environment. What about moving the job rings assignment to OP-TEE level? Something similar as we currently have in imx-optee-os project? https://source.codeaurora.org/external/imx/imx-optee-os/tree/core/drivers/caam/hal/imx_6_7/hal_jr.c?h=imx_4.14.78_1.0.0_ga#n31 U-Boot is running in TrustZone secure world in most of targets, in my opinion it makes sense to have at least 1 job ring assigned to TrustZone secure world. Best regards, Breno Lima
On 06/04/2019 22:41, Breno Matheus Lima wrote: > Hi Bryan, > > Em sáb, 6 de abr de 2019 às 12:21, Bryan O'Donoghue > <bryan.odonoghue@linaro.org> escreveu: >> >> >> >> On 05/04/2019 17:16, Breno Matheus Lima wrote: >> Basically you've described and additional dependency the BootROM has, so >> lets just "switch context" prior to calling into the BootROM and restore >> to a default non-secure job-ring assignment after. > > Yes, this can work for OP-TEE users decrypting additional boot images > at U-Boot level, however all users won't be able to > authenticate/decrypt the initial boot image at BootROM level. I don't understand that comment, perhaps you can give an example. > Out of > reset CAAM job rings are assigned to TrustZone secure world and > BootROM code is expecting blobs generated by the same environment. > > What about moving the job rings assignment to OP-TEE level? Something > similar as we currently have in imx-optee-os project? TBH, I see that as something that should be done _anyway_ not instead i.e. after u-boot, if you want to do 'stuff' with the CAAM you are either 1. Running in non-secure Linux, in which case you need the job-rings assigned to non-secure mode or 2. You are running inside of a TEE, in which case you absolutely need to have at least one job-ring ... and for the second case the right thing to do is to arbitrate ownership of job-rings via a DTB > https://source.codeaurora.org/external/imx/imx-optee-os/tree/core/drivers/caam/hal/imx_6_7/hal_jr.c?h=imx_4.14.78_1.0.0_ga#n31 > > U-Boot is running in TrustZone secure world in most of targets, in my > opinion it makes sense to have at least 1 job ring assigned to > TrustZone secure world. But if u-boot is running in secure-world save_jr_context(); setup_some_new_jr_context(); hab_authenticate_something(); restore_jr_context(); As a "quick fix", that's the way I'd do it. Just pivoting on CONFIG_OPTEE is pretty easy to break i.e. you can have CONFIG_OPTEE defined in your u-boot config but not actually be executing a TEE, in which case by the time you boot Linux your JR assignment is wrong.. The correct and flexible fix is passing a DTB descriptor that u-boot, OPTEE and Kernel can put data into so that there's a canonical description of which execution environment owns what. --- bod
Hi Bryan, Em dom, 7 de abr de 2019 às 05:05, Bryan O'Donoghue <bryan.odonoghue@linaro.org> escreveu: > > > > On 06/04/2019 22:41, Breno Matheus Lima wrote: > > Hi Bryan, > > > > Em sáb, 6 de abr de 2019 às 12:21, Bryan O'Donoghue > > <bryan.odonoghue@linaro.org> escreveu: > >> > >> > >> > >> On 05/04/2019 17:16, Breno Matheus Lima wrote: > >> Basically you've described and additional dependency the BootROM has, so > >> lets just "switch context" prior to calling into the BootROM and restore > >> to a default non-secure job-ring assignment after. > > > > Yes, this can work for OP-TEE users decrypting additional boot images > > at U-Boot level, however all users won't be able to > > authenticate/decrypt the initial boot image at BootROM level. > > I don't understand that comment, perhaps you can give an example. > You can take a look in application note AN12056: https://www.nxp.com/docs/en/application-note/AN12056.pdf Section 2.2 provides more details on the encrypted boot sequence, as you can see in Figure 1 the U-Boot image is decrypted at bootROM level after a system reset. Section 4 explains how to encrypt and sign an U-Boot image. As I mentioned in commit log, the ROM code expects DEK blobs encapsulated by the Secure World environments which commonly have JROWN_NS = 0. > > Out of > > reset CAAM job rings are assigned to TrustZone secure world and > > BootROM code is expecting blobs generated by the same environment. > > > > What about moving the job rings assignment to OP-TEE level? Something > > similar as we currently have in imx-optee-os project? > > TBH, I see that as something that should be done _anyway_ not instead > i.e. after u-boot, if you want to do 'stuff' with the CAAM you are either > > 1. Running in non-secure Linux, in which case you need the job-rings > assigned to non-secure mode or > > 2. You are running inside of a TEE, in which case you absolutely need > to have at least one job-ring > > ... and for the second case the right thing to do is to arbitrate > ownership of job-rings via a DTB > > > https://source.codeaurora.org/external/imx/imx-optee-os/tree/core/drivers/caam/hal/imx_6_7/hal_jr.c?h=imx_4.14.78_1.0.0_ga#n31 > > > > U-Boot is running in TrustZone secure world in most of targets, in my > > opinion it makes sense to have at least 1 job ring assigned to > > TrustZone secure world. > > But if u-boot is running in secure-world > > save_jr_context(); > setup_some_new_jr_context(); > hab_authenticate_something(); > restore_jr_context(); This can only work if we do similar operation in CMD_DEKBLOB: save_jr_context(); setup_some_new_jr_context(); blob_dek() restore_jr_context(); Both operations blob_dek() and hab_authenticate_image() at U-Boot level must have the Job Ring assigned for TrustZone secure world. The first authentication/decryption at bootROM level is expecting a DEK blobs generated by TrustZone secure world. > > As a "quick fix", that's the way I'd do it. Just pivoting on > CONFIG_OPTEE is pretty easy to break i.e. you can have CONFIG_OPTEE > defined in your u-boot config but not actually be executing a TEE, in > which case by the time you boot Linux your JR assignment is wrong.. Can you please provide more details on how this can break users that has CONFIG_OPTEE defined and are not executing a TEE? From my understanding Linux Kernel will be running in TZ secure world and JRs assigned to TZ non-secure world, CAAM driver can still be used on this condition (Similar as we currently have for mx7dsabresd target). In order to have a quick fix available, what about delaying the Job Ring assignment in U-Boot? Perhaps we can provide an U-Boot command to set the Job Ring ownership, users can add this command in their boot script just before booting Kernel and/or OP-TEE. > > The correct and flexible fix is passing a DTB descriptor that u-boot, > OPTEE and Kernel can put data into so that there's a canonical > description of which execution environment owns what. > Yes, I agree. We need a more flexible fix here. Best regards, Breno Lima
On 07/04/2019 19:56, Breno Matheus Lima wrote: > Hi Bryan, > > Em dom, 7 de abr de 2019 às 05:05, Bryan O'Donoghue > <bryan.odonoghue@linaro.org> escreveu: >> >> >> >> On 06/04/2019 22:41, Breno Matheus Lima wrote: >> save_jr_context(); >> setup_some_new_jr_context(); >> hab_authenticate_something(); >> restore_jr_context(); > > This can only work if we do similar operation in CMD_DEKBLOB: > > save_jr_context(); > setup_some_new_jr_context(); > blob_dek() > restore_jr_context(); > > Both operations blob_dek() and hab_authenticate_image() at U-Boot > level must have the Job Ring assigned for TrustZone secure world. The > first authentication/decryption at bootROM level is expecting a DEK > blobs generated by TrustZone secure world. Ah right, yes good point, I wasn't entirely following you on the DEK bit of it. Yes we would need to save and restore context for both cases i.e. any time we call into the BootROM and the BootROM wants to perform CAAM operations as a result. >> As a "quick fix", that's the way I'd do it. Just pivoting on >> CONFIG_OPTEE is pretty easy to break i.e. you can have CONFIG_OPTEE >> defined in your u-boot config but not actually be executing a TEE, in >> which case by the time you boot Linux your JR assignment is wrong.. > > Can you please provide more details on how this can break users that > has CONFIG_OPTEE defined and are not executing a TEE? My point here is you can't presuppose how a boot flow will work simply due to a CONFIG option. And indeed if you run an NXP TEE with a CAAM driver you still have the problem that the initial jr ownership defined by u-boot is now wrong by the time the kernel runs. > From my > understanding Linux Kernel will be running in TZ secure world and JRs > assigned to TZ non-secure world, CAAM driver can still be used on this > condition (Similar as we currently have for mx7dsabresd target). Linux will be running in _normal_ world. A TEE will be running in secure world. Then again a TEE might not be involved. As soon as you guys want to land your CAAM driver in upstream OP-TEE I think a conversation needs to be had re: assignment of job-ring ownership. Basically if your TEE wants a CAAM job-ring, that's fine but, that allocation needs to be stuffed into a DTB the kernel can access. > In order to have a quick fix available, what about delaying the Job > Ring assignment in U-Boot? > > Perhaps we can provide an U-Boot command to set the Job Ring > ownership, users can add this command in their boot script just before > booting Kernel and/or OP-TEE. Technically it could work but, how is it a better solution to have all users of the CAAM have to modify their boot flow, than put a context save/restore wrapper into hab_auth() and blob_dek() that hides the detail ? > >> >> The correct and flexible fix is passing a DTB descriptor that u-boot, >> OPTEE and Kernel can put data into so that there's a canonical >> description of which execution environment owns what. >> > > Yes, I agree. We need a more flexible fix here. DTB is the way to go.
Hi Bryan,
On Mon, Apr 8, 2019 at 5:10 AM Bryan O'Donoghue
<bryan.odonoghue@linaro.org> wrote:
> DTB is the way to go.
Could you please submit a patch that fixes the regression?
On 08/04/2019 13:58, Fabio Estevam wrote: > Hi Bryan, > > On Mon, Apr 8, 2019 at 5:10 AM Bryan O'Donoghue > <bryan.odonoghue@linaro.org> wrote: > >> DTB is the way to go. > > Could you please submit a patch that fixes the regression? > I can publish something soon, sure.
diff --git a/drivers/crypto/fsl/jr.c b/drivers/crypto/fsl/jr.c index cc8d3b02a5..23d5a64da0 100644 --- a/drivers/crypto/fsl/jr.c +++ b/drivers/crypto/fsl/jr.c @@ -635,11 +635,15 @@ int sec_init_idx(uint8_t sec_idx) #endif #endif - /* Set ownership of job rings to non-TrustZone mode by default */ - for (i = 0; i < ARRAY_SIZE(sec->jrliodnr); i++) { - jrown_ns = sec_in32(&sec->jrliodnr[i].ms); - jrown_ns |= JROWN_NS | JRMID_NS; - sec_out32(&sec->jrliodnr[i].ms, jrown_ns); + if ((IS_ENABLED(CONFIG_OPTEE) || + !IS_ENABLED(CONFIG_ARMV7_BOOT_SEC_DEFAULT)) && + !IS_ENABLED(CONFIG_CMD_DEKBLOB)) { + /* Set ownership of job rings to non-TrustZone mode. */ + for (i = 0; i < ARRAY_SIZE(sec->jrliodnr); i++) { + jrown_ns = sec_in32(&sec->jrliodnr[i].ms); + jrown_ns |= JROWN_NS | JRMID_NS; + sec_out32(&sec->jrliodnr[i].ms, jrown_ns); + } } ret = jr_init(sec_idx);
Commit 22191ac35344 ("drivers/crypto/fsl: assign job-rings to non-TrustZone") breaks HABv4 encrypted boot support in the following i.MX devices: - i.MX6UL - i.MX7S - i.MX7D - i.MX7ULP For preparing a HABv4 encrypted boot image it's necessary to encapsulate the generated DEK in a blob. In devices listed above the blob generation function takes into consideration the Job Ring TrustZone ownership configuration (JROWN_NS) and can be only decapsulated by the same configuration. The ROM code expects DEK blobs encapsulated by the Secure World environments which commonly have JROWN_NS = 0. As U-Boot is running in Secure World we must have JROWN_NS = 0 so the blobs generated by dek_blob tool can be decapsulated by the ROM code. Linux Kernel is booting by default in TrustZone Secure World in most of targets. Make job-rings assignment to non-Secure dependent of CONFIG_OPTEE and CONFIG_ARMV7_BOOT_SEC_DEFAULT to avoid a Kernel crash when booting Linux in non-Secure World. OP-TEE users can still use dek_blob command as job ring assignment is also dependent of CONFIG_CMD_DEKBLOB configuration. Signed-off-by: Breno Lima <breno.lima@nxp.com> --- drivers/crypto/fsl/jr.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)