RE: [PATCH 00/10] Add wrapped key support for Qualcomm ICE

[Gaurav Kashyap <gaurkash@qti.qualcomm.com>]

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@kernel.org> 
Sent: Thursday, January 6, 2022 11:48 AM
To: Gaurav Kashyap (QUIC) <quic_gaurkash@quicinc.com>
Cc: linux-scsi@vger.kernel.org; linux-arm-msm@vger.kernel.org; linux-mmc@vger.kernel.org; linux-block@vger.kernel.org; linux-fscrypt@vger.kernel.org; thara.gopinath@linaro.org; Neeraj Soni (QUIC) <quic_neersoni@quicinc.com>; Dinesh Garg <dineshg@quicinc.com>
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);