Feature/patch caam huk by cneveux · Pull Request #3149 · OP-TEE/optee_os

cneveux · 2019-07-24T11:48:35Z

Add the NXP CAAM drivers:

Hardware Unique Key

This PR includes the PR #2962, PR #3135 and PR #3136. The latest commit "core:driver implementation NXP CAAM Driver - ECC" is added to the PR #3136

This patch implementation is a preliminary implementation because of missing correct SNVS driver that will be in a separated patch.

Add C inline function to execute assembly instruction WFE/SEV use in C code Signed-off-by: Cedric Neveux <[email protected]> Reviewed-by: Jens Wiklander <[email protected]> Acked-by: Jens Wiklander <[email protected]>

Add registers to handle CAAM clocks Signed-off-by: Cedric Neveux <[email protected]>

Add dt functions: - dt_get_irq get the interrupt number of a node - dt_node_offset_by_compatible_status find a node compatible with specified 'compatible' input and check if the node status correspond to the expected one - dt_disable_status disable the 'status' field of node's prop - dt_set_secure_status set 'secure-status' field of node's prop and disable the 'status' field in the same time Signed-off-by: Cedric Neveux <[email protected]>

To allow any driver or initialization function to change the DT, moved the DT pack operation at the end of the generic boot. Move from update_external_dt to release_external_dt Signed-off-by: Cedric Neveux <[email protected]> Acked-by: Jens Wiklander <[email protected]>

Add the NXP CAAM drivers: - Random generator (instantiation and random generation) - Hash Add a generic cryptographic driver interface connecting TEE Crypto generic APIs to HW driver interface The Generic Crypto Driver interface in the core/driver/crypto/crypto_api is implemented to be able to use an other HW driver that is not the CAAM driver. Signed-off-by: Cedric Neveux <[email protected]>

Add the NXP CAAM drivers: - Cipher (AES/DES/DES3) Add a generic Cipher cryptographic driver interface connecting TEE Crypto generic APIs to HW driver interface Signed-off-by: Cedric Neveux <[email protected]>

Add the NXP CAAM drivers: - Asymmetric ECC Add a generic ECC cryptographic driver interface connecting TEE Crypto generic APIs to HW driver interface Signed-off-by: Cedric Neveux <[email protected]>

If supported, the cryptographic driver must be call during the system cryptographic initialization. This is needed to be able to respond to the crypto operation during the boot (e.g, HUK generation) Signed-off-by: Cedric Neveux <[email protected]>

Emantor · 2019-07-24T12:17:01Z

Looking at this I doubt this is working as intended. Take tee_fs_init_key_manager as an example:

service_init_late runs before driver_init
tee_fs_init_key_manager calls huk_subkey_derive which calls tee_otp_get_hw_unique_key
your implementation of tee_otp_get_hw_unique_key tries to use the CAAM, but CAAM is not initialized yet (due to initcall ordering) and returns the zero key.
The platform runs the zero key as before wihtout the tee_otp_get_hw_unique_key implementation.

So only callers of huk_subkey_dervice after driver_init benefit from the HUK implementation.

cneveux · 2019-07-24T12:42:14Z

Looking at this I doubt this is working as intended. Take tee_fs_init_key_manager as an example:

service_init_late runs before driver_init

tee_fs_init_key_manager calls huk_subkey_derive which calls tee_otp_get_hw_unique_key

your implementation of tee_otp_get_hw_unique_key tries to use the CAAM, but CAAM is not initialized yet (due to initcall ordering) and returns the zero key.

The platform runs the zero key as before wihtout the tee_otp_get_hw_unique_key implementation.

So only callers of huk_subkey_dervice after driver_init benefit from the HUK implementation.

@Emantor
This is why I submitted the commit 6840e66 in which I changed the initialization of the crypto driver (crypto_driver_init function).

This function is static inline if the CFG_CRYPTO_DRIVER is not defined (in other words if the Crypto Driver is not enabled). In this case the CAAM Driver HUK generation is not called and default TEE core function are used as today.
This function is the CAAM Driver Controller function if the Crypto Driver is enabled. And in this case the driver_init(crypto_driver_init) is not built. Hence the HUK generation is going thru the CAAM driver.

Add the CAAM driver (part of CAAM Blob) to support the generation of a Hardware Unique Key based on the device efuse. This is a preliminary implementation to enable the HUK generation Missing in this patch is the i.MX SNVS functions: - Check if the device is closed or not - Selection of the OTPMK key used to generate the HUK Those functions will be implemented in a separated PR in which the SNVS driver will be updated and functional Add weak function implementation in the plat-imx platform tee_otp_get_hw_unique_key Signed-off-by: Cedric Neveux <[email protected]>

dmcilvaney · 2019-07-24T20:46:36Z

+	 * If there is an error during the Master key derivation, let the device
+	 * booting with a 0's key
+	 */
+	if (ret != TEE_SUCCESS) {


Maybe only hide errors when some sort of testing/development flag is set? If feels odd to allow the device to boot when the security module is known to be in a bad state.

My understanding of the CAAM is that this will fail if High Assurance Boot (HAB) is not enabled on the device, which is likely during development. We allow a zero key through when using CFG_RPMB_TESTKEY since that is our primary use case for tee_otp_get_hw_unique_key(), although the system PTA now also offers a key derivation function via system_derive_ta_unique_key()...

Unfortunately I don't recall having come across a good config flag for that in OP-TEE which also covers the non-RPMB path (tee_fs_init_key_manager()).

Emantor · 2019-07-25T05:42:34Z

+	desc_add_word(desc, DESC_HEADER(0));
+
+	/* Load the key modifier */
+	desc_add_word(desc, LD_NOIMM(CLASS_2, REG_KEY, BLOB_KEY_MODIFIER_SIZE));


Why is a key modifier used for the Master Key Verification Blob?
AFAICS this has no effect for the MKVB.

Testing reveals that this has an effect, but the MKVB does also work without the key modifier. In theory this could be used to derive different keys, however OP-TEE already does this in software and its not really required for the MKVB.

cneveux added 8 commits July 22, 2019 08:17

core:arm64 Add sev/wfe inline function

f23d8a7

Add C inline function to execute assembly instruction WFE/SEV use in C code Signed-off-by: Cedric Neveux <[email protected]> Reviewed-by: Jens Wiklander <[email protected]> Acked-by: Jens Wiklander <[email protected]>

plat-imx Add CRM registers mapping

4f92c31

Add registers to handle CAAM clocks Signed-off-by: Cedric Neveux <[email protected]>

core:driver implementation NXP CAAM Driver - Cipher

10b0c01

Add the NXP CAAM drivers: - Cipher (AES/DES/DES3) Add a generic Cipher cryptographic driver interface connecting TEE Crypto generic APIs to HW driver interface Signed-off-by: Cedric Neveux <[email protected]>

core:driver implementation NXP CAAM Driver - ECC

a151559

Add the NXP CAAM drivers: - Asymmetric ECC Add a generic ECC cryptographic driver interface connecting TEE Crypto generic APIs to HW driver interface Signed-off-by: Cedric Neveux <[email protected]>

cneveux mentioned this pull request Jul 24, 2019

core:driver implementation NXP CAAM Driver #2962

Closed

cneveux force-pushed the feature/patch_caam_huk branch from 651596d to bcb5356 Compare July 24, 2019 12:51

dmcilvaney reviewed Jul 24, 2019

View reviewed changes

Emantor reviewed Jul 25, 2019

View reviewed changes

cneveux mentioned this pull request Jul 26, 2019

Feature/patch caam rsa #3152

Closed

Emantor mentioned this pull request Jul 31, 2019

Minimal HUK implementation without full CAAM driver #3160

Closed

cneveux closed this Aug 28, 2019

cneveux deleted the feature/patch_caam_huk branch October 13, 2020 12:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/patch caam huk#3149

Feature/patch caam huk#3149
cneveux wants to merge 9 commits intoOP-TEE:masterfrom
cneveux:feature/patch_caam_huk

cneveux commented Jul 24, 2019

Uh oh!

Emantor commented Jul 24, 2019 •

edited

Loading

Uh oh!

cneveux commented Jul 24, 2019

Uh oh!

dmcilvaney Jul 24, 2019

Uh oh!

Emantor Jul 25, 2019

Uh oh!

Emantor Jul 30, 2019 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

cneveux commented Jul 24, 2019

Uh oh!

Emantor commented Jul 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cneveux commented Jul 24, 2019

Uh oh!

dmcilvaney Jul 24, 2019

Choose a reason for hiding this comment

Uh oh!

Emantor Jul 25, 2019

Choose a reason for hiding this comment

Uh oh!

Emantor Jul 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Emantor commented Jul 24, 2019 •

edited

Loading

Emantor Jul 30, 2019 •

edited

Loading