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

Eric Biggers
2022-01-06T19:48:02Z

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);