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

Eric Biggers
2022-01-27T00:51:26Z

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