Hey Eric Have you tested that QCOM_SCM_ES_DERIVE_SW_SECRET is working properly? - You will need updated trustzone build for that (as I was missing a minor detail in the original one pertaining to SW secret) , please request again on the same ticket for the updated build. - I have reminded the people in Qualcomm too to provide you the build. - Note that with the new build you should be using the correct directions, i.e QCOM_SCM_RO where intended Warm Regards Gaurav Kashyap -----Original Message----- From: Eric Biggers <ebiggers@xxxxxxxxxx> Sent: Thursday, January 6, 2022 11:48 AM To: Gaurav Kashyap (QUIC) <quic_gaurkash@xxxxxxxxxxx> Cc: linux-scsi@xxxxxxxxxxxxxxx; linux-arm-msm@xxxxxxxxxxxxxxx; linux-mmc@xxxxxxxxxxxxxxx; linux-block@xxxxxxxxxxxxxxx; linux-fscrypt@xxxxxxxxxxxxxxx; thara.gopinath@xxxxxxxxxx; Neeraj Soni (QUIC) <quic_neersoni@xxxxxxxxxxx>; Dinesh Garg <dineshg@xxxxxxxxxxx> Subject: Re: [PATCH 00/10] Add wrapped key support for Qualcomm ICE WARNING: This email originated from outside of Qualcomm. Please be wary of any links or attachments, and do not enable macros. Hi Gaurav, On Mon, Dec 06, 2021 at 02:57:15PM -0800, Gaurav Kashyap wrote: > Testing: > Test platform: SM8350 HDK/MTP > Engineering trustzone image (based on sm8350) is required to test this > feature. This is because of version changes of HWKM. > HWKM version 2 and moving forward has a lot of restrictions on the key > management due to which the launched SM8350 solution (based on v1) > cannot be used and some modifications are required in trustzone. I've been trying to test this patchset on a SM8350 HDK using the TrustZone image you provided, but it's not completely working yet. This is the kernel branch I'm using: https://git.kernel.org/pub/scm/fs/fscrypt/fscrypt.git/log/?h=wip-wrapped-keys. It has my v4 patchset with your patchset rebased on top of it, some qcom_scm.c fixes I had to make (see below), and some extra logging messages. This is how I'm building and booting a kernel on the board: https://github.com/ebiggers/fscryptctl/blob/wip-wrapped-keys/scripts/sm8350-buildkernel.sh And this is the test script I'm running: https://github.com/ebiggers/fscryptctl/blob/wip-wrapped-keys/scripts/wrappedkey-test.sh. It imports or generates a hardware-wrapped key, then tries to set up a directory on an ext4 filesystem that is encrypted with that key. This uses new 'fscryptctl' commands to access the new blk-crypto ioctls; the version of 'fscryptctl' on the branch the scripts are on has all the needed changes. QCOM_SCM_ES_IMPORT_ICE_KEY, QCOM_SCM_ES_GENERATE_ICE_KEY, QCOM_SCM_ES_PREPARE_ICE_KEY, all seem to work. However, QCOM_SCM_ES_DERIVE_SW_SECRET doesn't work; it always returns -EINVAL. For example: Importing hardware-wrapped key [ 187.606109] blk-crypto: entering BLKCRYPTOCREATEKEY [ 187.611648] calling QCOM_SCM_ES_IMPORT_ICE_KEY; raw_key=5858585858585858585858585858585858585858585858585858585858585858 [ 187.628180] QCOM_SCM_ES_IMPORT_ICE_KEY succeeded; longterm_wrapped_key=fab51aa07fb6c2bf2fea60a8120e8d35a9e53865b594e0fb6279e7951a34864591f1c1c4e26f9421039377c1ac311ff9241a0152030000000000000000000000 [ 187.646433] blk-crypto: exiting BLKCRYPTOCREATEKEY; ret=0 Preparing hardware-wrapped key [ 187.653129] blk-crypto: entering BLKCRYPTOPREPAREKEY [ 187.660356] calling QCOM_SCM_ES_PREPARE_ICE_KEY; longterm_wrapped_key=fab51aa07fb6c2bf2fea60a8120e8d35a9e53865b594e0fb6279e7951a34864591f1c1c4e26f9421039377c1ac311ff9241a0152030000000000000000000000 [ 187.680420] QCOM_SCM_ES_PREPARE_ICE_KEY succeeded; ephemeral_wrapped_key=1fbf5d501854858d6faaf52c9d22bebc576012e40485ba75e7d19e88f74b3400eb1a8836e28232939e990df6007659b1241a0152030000000000000000000000 [ 187.698791] blk-crypto: exiting BLKCRYPTOPREPAREKEY; ret=0 Adding hardware-wrapped key [ 187.705515] calling blk_crypto_derive_sw_secret(); wrapped_key_size=68 [ 187.714075] in qti_ice_derive_sw_secret() [ 187.718212] calling qti_ice_hwkm_init() [ 187.722157] calling qti_ice_hwkm_init_sequence(version=1) [ 187.727715] setting standard mode [ 187.731134] checking BIST status [ 187.734464] configuring ICE registers [ 187.738230] disabling CRC check [ 187.741479] setting RSP_FIFO_FULL bit [ 187.745247] calling qcom_scm_derive_sw_secret() [ 187.749920] calling QCOM_SCM_ES_DERIVE_SW_SECRET; wrapped_key=1fbf5d501854858d6faaf52c9d22bebc576012e40485ba75e7d19e88f74b3400eb1a8836e28232939e990df6007659b1241a0152030000000000000000000000, secret_size=32 [ 187.768834] QCOM_SCM_ES_DERIVE_SW_SECRET failed with error -22 [ 187.774838] blk_crypto_derive_sw_secret() returned -22 error: adding key to /mnt: Invalid argument You can see that the wrapped_key being passed to QCOM_SCM_ES_DERIVE_SW_SECRET matches the ephemeral_wrapped_key that was returned earlier by QCOM_SCM_ES_PREPARE_ICE_KEY, and that secret_size is 32. So the arguments are as expected. However, QCOM_SCM_ES_DERIVE_SW_SECRET still fails. This still occurs if QCOM_SCM_ES_GENERATE_ICE_KEY is used instead of QCOM_SCM_ES_IMPORT_ICE_KEY. Have you tested that QCOM_SCM_ES_DERIVE_SW_SECRET is working properly? For reference, these are the fixes I had to apply to qcom_scm.c to get things working until that point. This included fixing the direction of the first arguments to the SCM calls, and fixing the return values. Note, I also tested leaving QCOM_SCM_ES_DERIVE_SW_SECRET using QCOM_SCM_RO instead of QCOM_SCM_RW, but the result was still the same --- it still returned -EINVAL. diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c index d57f52015640..002b57a1473d 100644 --- a/drivers/firmware/qcom_scm.c +++ b/drivers/firmware/qcom_scm.c @@ -1087,7 +1087,7 @@ int qcom_scm_derive_sw_secret(const u8 *wrapped_key, u32 wrapped_key_size, struct qcom_scm_desc desc = { .svc = QCOM_SCM_SVC_ES, .cmd = QCOM_SCM_ES_DERIVE_SW_SECRET, - .arginfo = QCOM_SCM_ARGS(4, QCOM_SCM_RO, + .arginfo = QCOM_SCM_ARGS(4, QCOM_SCM_RW, QCOM_SCM_VAL, QCOM_SCM_RW, QCOM_SCM_VAL), .args[1] = wrapped_key_size, @@ -1148,7 +1148,7 @@ EXPORT_SYMBOL(qcom_scm_derive_sw_secret); * This SCM calls adds support for the generate key IOCTL to interface * with the secure environment to generate and return a wrapped key.. * - * Return: 0 on success; -errno on failure. + * Return: size of the resulting key on success; -errno on failure. */ int qcom_scm_generate_ice_key(u8 *longterm_wrapped_key, u32 longterm_wrapped_key_size) @@ -1188,7 +1188,7 @@ int qcom_scm_generate_ice_key(u8 *longterm_wrapped_key, dma_free_coherent(__scm->dev, longterm_wrapped_key_size, longterm_wrapped_keybuf, longterm_wrapped_key_phys); - return ret; + return ret ?: longterm_wrapped_key_size; } EXPORT_SYMBOL(qcom_scm_generate_ice_key); @@ -1209,7 +1209,7 @@ EXPORT_SYMBOL(qcom_scm_generate_ice_key); * with the secure environment to rewrap the wrapped key with an * ephemeral wrapping key. * - * Return: 0 on success; -errno on failure. + * Return: size of the resulting key on success; -errno on failure. */ int qcom_scm_prepare_ice_key(const u8 *longterm_wrapped_key, u32 longterm_wrapped_key_size, @@ -1219,7 +1219,7 @@ int qcom_scm_prepare_ice_key(const u8 *longterm_wrapped_key, struct qcom_scm_desc desc = { .svc = QCOM_SCM_SVC_ES, .cmd = QCOM_SCM_ES_PREPARE_ICE_KEY, - .arginfo = QCOM_SCM_ARGS(4, QCOM_SCM_RO, + .arginfo = QCOM_SCM_ARGS(4, QCOM_SCM_RW, QCOM_SCM_VAL, QCOM_SCM_RW, QCOM_SCM_VAL), .args[1] = longterm_wrapped_key_size, @@ -1270,7 +1270,7 @@ int qcom_scm_prepare_ice_key(const u8 *longterm_wrapped_key, dma_free_coherent(__scm->dev, longterm_wrapped_key_size, longterm_wrapped_keybuf, longterm_wrapped_key_phys); - return ret; + return ret ?: ephemeral_wrapped_key_size; } EXPORT_SYMBOL(qcom_scm_prepare_ice_key); @@ -1289,7 +1289,7 @@ EXPORT_SYMBOL(qcom_scm_prepare_ice_key); * the secure environment to import a raw key and generate a longterm * wrapped key. * - * Return: 0 on success; -errno on failure. + * Return: size of the resulting key on success; -errno on failure. */ int qcom_scm_import_ice_key(const u8 *imported_key, u32 imported_key_size, u8 *longterm_wrapped_key, @@ -1298,7 +1298,7 @@ int qcom_scm_import_ice_key(const u8 *imported_key, u32 imported_key_size, struct qcom_scm_desc desc = { .svc = QCOM_SCM_SVC_ES, .cmd = QCOM_SCM_ES_IMPORT_ICE_KEY, - .arginfo = QCOM_SCM_ARGS(4, QCOM_SCM_RO, + .arginfo = QCOM_SCM_ARGS(4, QCOM_SCM_RW, QCOM_SCM_VAL, QCOM_SCM_RW, QCOM_SCM_VAL), .args[1] = imported_key_size, @@ -1344,7 +1344,7 @@ int qcom_scm_import_ice_key(const u8 *imported_key, u32 imported_key_size, dma_free_coherent(__scm->dev, imported_key_size, imported_keybuf, imported_key_phys); - return ret; + return ret ?: longterm_wrapped_key_size; } EXPORT_SYMBOL(qcom_scm_import_ice_key);
