| Message ID | 20211206225725.77512-1-quic_gaurkash@quicinc.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Add wrapped key support for Qualcomm ICE | expand |
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);
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);
Hi Gaurav, On Thu, Jan 06, 2022 at 09:14:22PM +0000, Gaurav Kashyap wrote: > 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 > I verified that the latest TrustZone build is working; thanks for the help! Note, these are the branches I'm using for now: * Kernel patches: https://git.kernel.org/pub/scm/fs/fscrypt/fscrypt.git/log/?h=wip-wrapped-keys * fscryptctl tool and test scripts: https://github.com/ebiggers/fscryptctl/tree/wip-wrapped-keys The kernel branch is based on v5.17-rc1. I haven't changed your patches from your latest series other than rebasing them and adding a fix "qcom_scm: fix return values" on top. Note that v5.16-rc5 and later have a bug where the UFS controller on SM8350 isn't recognized. Therefore, my branch contains a fix from Bjorn Andersson for that bug, which I applied from the mailing list. One oddity I noticed is that if I import the same raw key twice, the long-term wrapped key blob is the same. This implies that the key encryption algorithm used by the Qualcomm hardware is deterministic, which is unexpected. I would expect the wrapped key to contain a random nonce. Do you know why deterministic encryption is used? This probably isn't much of a problem, but it's unexpected. Besides that, I think the next steps are for you to address the comments I've left on this series, and for me to get started on adding ciphertext verification tests for this to xfstests (alongside the other fscrypt ciphertext verification tests that are already there) so that we can prove this feature is actually encrypting the data as intended. - Eric