mbox series

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

Message ID 20211206225725.77512-1-quic_gaurkash@quicinc.com (mailing list archive)
Headers show
Series Add wrapped key support for Qualcomm ICE | expand

Message

Gaurav Kashyap Dec. 6, 2021, 10:57 p.m. UTC
These patches add support to Qualcomm ICE for hardware
wrapped keys and are made on top of Eric Bigger's set of changes to
support wrapped keys.
Found here: https://lore.kernel.org/linux-block/20210916174928.65529-1-ebiggers@kernel.org).
Explanation and use of hardware-wrapped-keys can be found here:
Documentation/block/inline-encryption.rst

The first patch however is not related to wrapped keys. It is
for moving ICE functionality into a shared library which both ufs and
sdhci can use. The patch for sdhc-msm will be uploaded later.

Wrapped keys are supported in Qualcomm's ICE engine using
a proprietary hardware module known as Hardware Key Manager (HWKM).
The patches are revolved around that and testing for them
can be done only on a HWKM supported Qualcomm device.

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.

The ideal scenario is to test by mounting initramfs on an upstream
kernel and then using the fscrypt tool to setup directories with
encryption policies. Testing and development for that is still under way.

All the SCM calls however were individually tested from UFS by invoking
a test API on bootup which is not part of these patchsets.

Gaurav Kashyap (10):
  soc: qcom: new common library for ICE functionality
  scsi: ufs: qcom: move ICE functionality to common library
  qcom_scm: scm call for deriving a software secret
  soc: qcom: add HWKM library for storage encryption
  scsi: ufs: prepare to support wrapped keys
  soc: qcom: add wrapped key support for ICE
  qcom_scm: scm call for create, prepare and import keys
  scsi: ufs: add support for generate, import and prepare keys
  soc: qcom: support for generate, import and prepare key
  arm64: dts: qcom: sm8350: add ice and hwkm mappings

 arch/arm64/boot/dts/qcom/sm8350.dtsi |   5 +-
 drivers/firmware/qcom_scm.c          | 286 +++++++++++++++++++
 drivers/firmware/qcom_scm.h          |   4 +
 drivers/scsi/ufs/Kconfig             |   1 +
 drivers/scsi/ufs/ufs-qcom-ice.c      | 227 +++++----------
 drivers/scsi/ufs/ufs-qcom.c          |   4 +
 drivers/scsi/ufs/ufs-qcom.h          |  22 +-
 drivers/scsi/ufs/ufshcd-crypto.c     |  96 ++++++-
 drivers/scsi/ufs/ufshcd.h            |  20 +-
 drivers/soc/qcom/Kconfig             |   7 +
 drivers/soc/qcom/Makefile            |   1 +
 drivers/soc/qcom/qti-ice-common.c    | 402 +++++++++++++++++++++++++++
 drivers/soc/qcom/qti-ice-hwkm.c      | 111 ++++++++
 drivers/soc/qcom/qti-ice-regs.h      | 264 ++++++++++++++++++
 include/linux/qcom_scm.h             |  30 +-
 include/linux/qti-ice-common.h       |  40 +++
 16 files changed, 1345 insertions(+), 175 deletions(-)
 create mode 100644 drivers/soc/qcom/qti-ice-common.c
 create mode 100644 drivers/soc/qcom/qti-ice-hwkm.c
 create mode 100644 drivers/soc/qcom/qti-ice-regs.h
 create mode 100644 include/linux/qti-ice-common.h

Comments

Eric Biggers Jan. 6, 2022, 7:47 p.m. UTC | #1
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);
Gaurav Kashyap Jan. 6, 2022, 9:14 p.m. UTC | #2
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);
Eric Biggers Jan. 27, 2022, 12:51 a.m. UTC | #3
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