mbox series

[v2,00/10] Hardware wrapped key support for qcom ice and ufs

Message ID 20230719170423.220033-1-quic_gaurkash@quicinc.com
Headers show
Series Hardware wrapped key support for qcom ice and ufs | expand

Message

Gaurav Kashyap July 19, 2023, 5:04 p.m. UTC
These patches add support to Qualcomm ICE (Inline Crypto Enginr) for hardware
wrapped keys using Qualcomm Hardware Key Manager (HWKM) and are made on top
of a rebased version  Eric Bigger's set of changes to support wrapped keys in
fscrypt and block below:
https://git.kernel.org/pub/scm/fs/fscrypt/linux.git/log/?h=wrapped-keys-v7
(The rebased patches are not uploaded here)

Ref v1 here:
https://lore.kernel.org/linux-scsi/20211206225725.77512-1-quic_gaurkash@quicinc.com/

Explanation and use of hardware-wrapped-keys can be found here:
Documentation/block/inline-encryption.rst

This patch is organized as follows:

Patch 1 - Prepares ICE and storage layers (UFS and EMMC) to pass around wrapped keys.
Patch 2 - Adds a new SCM api to support deriving software secret when wrapped keys are used
Patch 3-4 - Adds support for wrapped keys in the ICE driver. This includes adding HWKM support
Patch 5-6 - Adds support for wrapped keys in UFS
Patch 7-10 - Supports generate, prepare and import functionality in ICE and UFS

NOTE: MMC will have similar changes to UFS and will be uploaded in a different patchset
      Patch 3, 4, 8, 10 will have MMC equivalents.

Testing:
Test platform: SM8550 MTP
Engineering trustzone image is required to test this feature only
for SM8550. For SM8650 onwards, all trustzone changes to support this
will be part of the released images.
The engineering changes primarily contain hooks to generate, import and
prepare keys for HW wrapped disk encryption.

The changes were tested by mounting initramfs and running the fscryptctl
tool (Ref: https://github.com/ebiggers/fscryptctl/tree/wip-wrapped-keys) to
generate and prepare keys, as well as to set policies on folders, which
consequently invokes disk encryption flows through UFS.

Gaurav Kashyap (10):
  ice, ufs, mmc: use blk_crypto_key for program_key
  qcom_scm: scm call for deriving a software secret
  soc: qcom: ice: add hwkm support in ice
  soc: qcom: ice: support for hardware wrapped keys
  ufs: core: support wrapped keys in ufs core
  ufs: host: wrapped keys support in ufs qcom
  qcom_scm: scm call for create, prepare and import keys
  ufs: core: add support for generate, import and prepare keys
  soc: qcom: support for generate, import and prepare key
  ufs: host: support for generate, import and prepare key

 drivers/firmware/qcom_scm.c            | 292 +++++++++++++++++++++++
 drivers/firmware/qcom_scm.h            |   4 +
 drivers/mmc/host/cqhci-crypto.c        |   7 +-
 drivers/mmc/host/cqhci.h               |   2 +
 drivers/mmc/host/sdhci-msm.c           |   6 +-
 drivers/soc/qcom/ice.c                 | 309 +++++++++++++++++++++++--
 drivers/ufs/core/ufshcd-crypto.c       |  92 +++++++-
 drivers/ufs/host/ufs-qcom.c            |  63 ++++-
 include/linux/firmware/qcom/qcom_scm.h |  13 ++
 include/soc/qcom/ice.h                 |  18 +-
 include/ufs/ufshcd.h                   |  25 ++
 11 files changed, 797 insertions(+), 34 deletions(-)

Comments

Eric Biggers July 20, 2023, 2:55 a.m. UTC | #1
Hi Gaurav,

On Wed, Jul 19, 2023 at 10:04:14AM -0700, Gaurav Kashyap wrote:
> These patches add support to Qualcomm ICE (Inline Crypto Enginr) for hardware
> wrapped keys using Qualcomm Hardware Key Manager (HWKM) and are made on top
> of a rebased version  Eric Bigger's set of changes to support wrapped keys in
> fscrypt and block below:
> https://git.kernel.org/pub/scm/fs/fscrypt/linux.git/log/?h=wrapped-keys-v7
> (The rebased patches are not uploaded here)
> 
> Ref v1 here:
> https://lore.kernel.org/linux-scsi/20211206225725.77512-1-quic_gaurkash@quicinc.com/
> 
> Explanation and use of hardware-wrapped-keys can be found here:
> Documentation/block/inline-encryption.rst
> 
> This patch is organized as follows:
> 
> Patch 1 - Prepares ICE and storage layers (UFS and EMMC) to pass around wrapped keys.
> Patch 2 - Adds a new SCM api to support deriving software secret when wrapped keys are used
> Patch 3-4 - Adds support for wrapped keys in the ICE driver. This includes adding HWKM support
> Patch 5-6 - Adds support for wrapped keys in UFS
> Patch 7-10 - Supports generate, prepare and import functionality in ICE and UFS
> 
> NOTE: MMC will have similar changes to UFS and will be uploaded in a different patchset
>       Patch 3, 4, 8, 10 will have MMC equivalents.
> 
> Testing:
> Test platform: SM8550 MTP
> Engineering trustzone image is required to test this feature only
> for SM8550. For SM8650 onwards, all trustzone changes to support this
> will be part of the released images.
> The engineering changes primarily contain hooks to generate, import and
> prepare keys for HW wrapped disk encryption.
> 
> The changes were tested by mounting initramfs and running the fscryptctl
> tool (Ref: https://github.com/ebiggers/fscryptctl/tree/wip-wrapped-keys) to
> generate and prepare keys, as well as to set policies on folders, which
> consequently invokes disk encryption flows through UFS.
> 
> Gaurav Kashyap (10):
>   ice, ufs, mmc: use blk_crypto_key for program_key
>   qcom_scm: scm call for deriving a software secret
>   soc: qcom: ice: add hwkm support in ice
>   soc: qcom: ice: support for hardware wrapped keys
>   ufs: core: support wrapped keys in ufs core
>   ufs: host: wrapped keys support in ufs qcom
>   qcom_scm: scm call for create, prepare and import keys
>   ufs: core: add support for generate, import and prepare keys
>   soc: qcom: support for generate, import and prepare key
>   ufs: host: support for generate, import and prepare key
> 
>  drivers/firmware/qcom_scm.c            | 292 +++++++++++++++++++++++
>  drivers/firmware/qcom_scm.h            |   4 +
>  drivers/mmc/host/cqhci-crypto.c        |   7 +-
>  drivers/mmc/host/cqhci.h               |   2 +
>  drivers/mmc/host/sdhci-msm.c           |   6 +-
>  drivers/soc/qcom/ice.c                 | 309 +++++++++++++++++++++++--
>  drivers/ufs/core/ufshcd-crypto.c       |  92 +++++++-
>  drivers/ufs/host/ufs-qcom.c            |  63 ++++-
>  include/linux/firmware/qcom/qcom_scm.h |  13 ++
>  include/soc/qcom/ice.h                 |  18 +-
>  include/ufs/ufshcd.h                   |  25 ++
>  11 files changed, 797 insertions(+), 34 deletions(-)


Thank you for continuing to work on this!

According to your cover letter, this feature requires a custom TrustZone image
to work on SM8550.  Will that image be made available outside Qualcomm?

Also according to your cover letter, this feature will work on SM8650 out of the
box.  That's great to hear.  However, SM8650 does not appear to be publicly
available yet or have any upstream kernel support.  Do you know approximately
when a SM8650 development board will become available to the general public?

Also, can you please make available a git branch somewhere that contains your
patchset?  It sounds like this depends on
https://git.kernel.org/pub/scm/fs/fscrypt/linux.git/log/?h=wrapped-keys-v7, but
actually a version of it that you've rebased, which I don't have access to.
Without being able to apply your patchset, I can't properly review it.

Thanks!

- Eric
Gaurav Kashyap Aug. 1, 2023, 5:31 p.m. UTC | #2
Hey Eric, thanks for your reply. Pleasure working with you again.

Please find answers inline

-----Original Message-----
From: Eric Biggers <ebiggers@kernel.org> 
Sent: Wednesday, July 19, 2023 7:56 PM
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; Om Prakash Singh <omprsing@qti.qualcomm.com>; Prasad Sodagudi (QUIC) <quic_psodagud@quicinc.com>; Arun Menon (SSG) <avmenon@quicinc.com>; abel.vesa@linaro.org; Seshu Madhavi Puppala (QUIC) <quic_spuppala@quicinc.com>
Subject: Re: [PATCH v2 00/10] Hardware wrapped key support for qcom ice and ufs

Hi Gaurav,

On Wed, Jul 19, 2023 at 10:04:14AM -0700, Gaurav Kashyap wrote:
> These patches add support to Qualcomm ICE (Inline Crypto Enginr) for 
> hardware wrapped keys using Qualcomm Hardware Key Manager (HWKM) and 
> are made on top of a rebased version  Eric Bigger's set of changes to 
> support wrapped keys in fscrypt and block below:
> https://git.kernel.org/pub/scm/fs/fscrypt/linux.git/log/?h=wrapped-key
> s-v7 (The rebased patches are not uploaded here)
> 
> Ref v1 here:
> https://lore.kernel.org/linux-scsi/20211206225725.77512-1-quic_gaurkas
> h@quicinc.com/
> 
> Explanation and use of hardware-wrapped-keys can be found here:
> Documentation/block/inline-encryption.rst
> 
> This patch is organized as follows:
> 
> Patch 1 - Prepares ICE and storage layers (UFS and EMMC) to pass around wrapped keys.
> Patch 2 - Adds a new SCM api to support deriving software secret when 
> wrapped keys are used Patch 3-4 - Adds support for wrapped keys in the 
> ICE driver. This includes adding HWKM support Patch 5-6 - Adds support 
> for wrapped keys in UFS Patch 7-10 - Supports generate, prepare and 
> import functionality in ICE and UFS
> 
> NOTE: MMC will have similar changes to UFS and will be uploaded in a different patchset
>       Patch 3, 4, 8, 10 will have MMC equivalents.
> 
> Testing:
> Test platform: SM8550 MTP
> Engineering trustzone image is required to test this feature only for 
> SM8550. For SM8650 onwards, all trustzone changes to support this will 
> be part of the released images.
> The engineering changes primarily contain hooks to generate, import 
> and prepare keys for HW wrapped disk encryption.
> 
> The changes were tested by mounting initramfs and running the 
> fscryptctl tool (Ref: 
> https://github.com/ebiggers/fscryptctl/tree/wip-wrapped-keys) to 
> generate and prepare keys, as well as to set policies on folders, which consequently invokes disk encryption flows through UFS.
> 
> Gaurav Kashyap (10):
>   ice, ufs, mmc: use blk_crypto_key for program_key
>   qcom_scm: scm call for deriving a software secret
>   soc: qcom: ice: add hwkm support in ice
>   soc: qcom: ice: support for hardware wrapped keys
>   ufs: core: support wrapped keys in ufs core
>   ufs: host: wrapped keys support in ufs qcom
>   qcom_scm: scm call for create, prepare and import keys
>   ufs: core: add support for generate, import and prepare keys
>   soc: qcom: support for generate, import and prepare key
>   ufs: host: support for generate, import and prepare key
> 
>  drivers/firmware/qcom_scm.c            | 292 +++++++++++++++++++++++
>  drivers/firmware/qcom_scm.h            |   4 +
>  drivers/mmc/host/cqhci-crypto.c        |   7 +-
>  drivers/mmc/host/cqhci.h               |   2 +
>  drivers/mmc/host/sdhci-msm.c           |   6 +-
>  drivers/soc/qcom/ice.c                 | 309 +++++++++++++++++++++++--
>  drivers/ufs/core/ufshcd-crypto.c       |  92 +++++++-
>  drivers/ufs/host/ufs-qcom.c            |  63 ++++-
>  include/linux/firmware/qcom/qcom_scm.h |  13 ++
>  include/soc/qcom/ice.h                 |  18 +-
>  include/ufs/ufshcd.h                   |  25 ++
>  11 files changed, 797 insertions(+), 34 deletions(-)


Thank you for continuing to work on this!

According to your cover letter, this feature requires a custom TrustZone image to work on SM8550.  Will that image be made available outside Qualcomm?
--> Unfortunately, I don't think there is a way to do that. You can still request for one through our customer engineering team like before.

Also according to your cover letter, this feature will work on SM8650 out of the box.  That's great to hear.  However, SM8650 does not appear to be publicly available yet or have any upstream kernel support.  Do you know approximately when a SM8650 development board will become available to the general public?
--> I meant it will be available in the future releases. As of today, I don't have any information on the timelines

Also, can you please make available a git branch somewhere that contains your patchset?  It sounds like this depends on https://git.kernel.org/pub/scm/fs/fscrypt/linux.git/log/?h=wrapped-keys-v7, but actually a version of it that you've rebased, which I don't have access to.
Without being able to apply your patchset, I can't properly review it.
--> As for the fscrypt patches,
      I have not changed much functionally from the v7 patch, just merge conflicts.
      I will update this thread once I figure out a git location.

Thanks!

- Eric
Eric Biggers Aug. 10, 2023, 5:36 a.m. UTC | #3
On Tue, Aug 01, 2023 at 05:31:59PM +0000, Gaurav Kashyap (QUIC) wrote:
> 
> According to your cover letter, this feature requires a custom TrustZone image to work on SM8550.  Will that image be made available outside Qualcomm?
> --> Unfortunately, I don't think there is a way to do that. You can still request for one through our customer engineering team like before.

I think it's already been shown that that is not a workable approach.

> Also, can you please make available a git branch somewhere that contains your patchset?  It sounds like this depends on https://git.kernel.org/pub/scm/fs/fscrypt/linux.git/log/?h=wrapped-keys-v7, but actually a version of it that you've rebased, which I don't have access to.
> Without being able to apply your patchset, I can't properly review it.
> --> As for the fscrypt patches,
>       I have not changed much functionally from the v7 patch, just merge conflicts.
>       I will update this thread once I figure out a git location.
> 

Any update on this?  Most kernel developers just create a GitHub repo if they
don't have kernel.org access.

- Eric
Gaurav Kashyap Aug. 11, 2023, 12:27 a.m. UTC | #4
-----Original Message-----
From: Eric Biggers <ebiggers@kernel.org> 
Sent: Wednesday, August 9, 2023 10:37 PM
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; Om Prakash Singh <omprsing@qti.qualcomm.com>; Prasad Sodagudi (QUIC) <quic_psodagud@quicinc.com>; Arun Menon (SSG) <avmenon@quicinc.com>; abel.vesa@linaro.org; Seshu Madhavi Puppala (QUIC) <quic_spuppala@quicinc.com>
Subject: Re: [PATCH v2 00/10] Hardware wrapped key support for qcom ice and ufs

On Tue, Aug 01, 2023 at 05:31:59PM +0000, Gaurav Kashyap (QUIC) wrote:
> 
> According to your cover letter, this feature requires a custom TrustZone image to work on SM8550.  Will that image be made available outside Qualcomm?
> --> Unfortunately, I don't think there is a way to do that. You can still request for one through our customer engineering team like before.

I think it's already been shown that that is not a workable approach.

> Also, can you please make available a git branch somewhere that contains your patchset?  It sounds like this depends on https://git.kernel.org/pub/scm/fs/fscrypt/linux.git/log/?h=wrapped-keys-v7, but actually a version of it that you've rebased, which I don't have access to.
> Without being able to apply your patchset, I can't properly review it.
> --> As for the fscrypt patches,
>       I have not changed much functionally from the v7 patch, just merge conflicts.
>       I will update this thread once I figure out a git location.
> 

Any update on this?  Most kernel developers just create a GitHub repo if they don't have kernel.org access.

>> Here are the git links
>> https://github.com/quic-gaurkash/linux/tree/fscrypt
>> https://github.com/quic-gaurkash/linux/tree/wrapped_keys_ice_ufs

- Eric
Srinivas Kandagatla Aug. 25, 2023, 10:19 a.m. UTC | #5
On 19/07/2023 18:04, Gaurav Kashyap wrote:
> These patches add support to Qualcomm ICE (Inline Crypto Enginr) for hardware
> wrapped keys using Qualcomm Hardware Key Manager (HWKM) and are made on top
> of a rebased version  Eric Bigger's set of changes to support wrapped keys in
> fscrypt and block below:
> https://git.kernel.org/pub/scm/fs/fscrypt/linux.git/log/?h=wrapped-keys-v7
> (The rebased patches are not uploaded here)
> 
> Ref v1 here:
> https://lore.kernel.org/linux-scsi/20211206225725.77512-1-quic_gaurkash@quicinc.com/
> 
> Explanation and use of hardware-wrapped-keys can be found here:
> Documentation/block/inline-encryption.rst
> 
> This patch is organized as follows:
> 
> Patch 1 - Prepares ICE and storage layers (UFS and EMMC) to pass around wrapped keys.
> Patch 2 - Adds a new SCM api to support deriving software secret when wrapped keys are used
> Patch 3-4 - Adds support for wrapped keys in the ICE driver. This includes adding HWKM support
> Patch 5-6 - Adds support for wrapped keys in UFS
> Patch 7-10 - Supports generate, prepare and import functionality in ICE and UFS
> 
> NOTE: MMC will have similar changes to UFS and will be uploaded in a different patchset
>        Patch 3, 4, 8, 10 will have MMC equivalents.
> 
> Testing:
> Test platform: SM8550 MTP
> Engineering trustzone image is required to test this feature only
> for SM8550. For SM8650 onwards, all trustzone changes to support this
> will be part of the released images.

AFAIU, Prior to these proposed changes in scm, HWKM was done with help 
of TA(Trusted Application) for generate, import, unwrap ... functionality.

1. What is the reason for moving this from TA to new smc calls?

Is this because of missing smckinvoke support in upstream?

How scalable is this approach? Are we going to add new sec sys calls to 
every interface to TA?

2. How are the older SoCs going to deal with this, given that you are 
changing drivers that are common across these?

Have you tested these patches on any older platforms?

What happens if someone want to add support to wrapped keys to this 
platforms in upstream, How is that going to be handled?

As I understand with this, we will endup with two possible solutions 
over time in upstream.


thanks,
--srini

> The engineering changes primarily contain hooks to generate, import and
> prepare keys for HW wrapped disk encryption.
> 
> The changes were tested by mounting initramfs and running the fscryptctl
> tool (Ref: https://github.com/ebiggers/fscryptctl/tree/wip-wrapped-keys) to
> generate and prepare keys, as well as to set policies on folders, which
> consequently invokes disk encryption flows through UFS.
> 
> Gaurav Kashyap (10):
>    ice, ufs, mmc: use blk_crypto_key for program_key
>    qcom_scm: scm call for deriving a software secret
>    soc: qcom: ice: add hwkm support in ice
>    soc: qcom: ice: support for hardware wrapped keys
>    ufs: core: support wrapped keys in ufs core
>    ufs: host: wrapped keys support in ufs qcom
>    qcom_scm: scm call for create, prepare and import keys
>    ufs: core: add support for generate, import and prepare keys
>    soc: qcom: support for generate, import and prepare key
>    ufs: host: support for generate, import and prepare key
> 
>   drivers/firmware/qcom_scm.c            | 292 +++++++++++++++++++++++
>   drivers/firmware/qcom_scm.h            |   4 +
>   drivers/mmc/host/cqhci-crypto.c        |   7 +-
>   drivers/mmc/host/cqhci.h               |   2 +
>   drivers/mmc/host/sdhci-msm.c           |   6 +-
>   drivers/soc/qcom/ice.c                 | 309 +++++++++++++++++++++++--
>   drivers/ufs/core/ufshcd-crypto.c       |  92 +++++++-
>   drivers/ufs/host/ufs-qcom.c            |  63 ++++-
>   include/linux/firmware/qcom/qcom_scm.h |  13 ++
>   include/soc/qcom/ice.h                 |  18 +-
>   include/ufs/ufshcd.h                   |  25 ++
>   11 files changed, 797 insertions(+), 34 deletions(-)
>
Eric Biggers Aug. 25, 2023, 9:07 p.m. UTC | #6
Hi Srinivas,

On Fri, Aug 25, 2023 at 11:19:41AM +0100, Srinivas Kandagatla wrote:
> 
> On 19/07/2023 18:04, Gaurav Kashyap wrote:
> > These patches add support to Qualcomm ICE (Inline Crypto Enginr) for hardware
> > wrapped keys using Qualcomm Hardware Key Manager (HWKM) and are made on top
> > of a rebased version  Eric Bigger's set of changes to support wrapped keys in
> > fscrypt and block below:
> > https://git.kernel.org/pub/scm/fs/fscrypt/linux.git/log/?h=wrapped-keys-v7
> > (The rebased patches are not uploaded here)
> > 
> > Ref v1 here:
> > https://lore.kernel.org/linux-scsi/20211206225725.77512-1-quic_gaurkash@quicinc.com/
> > 
> > Explanation and use of hardware-wrapped-keys can be found here:
> > Documentation/block/inline-encryption.rst
> > 
> > This patch is organized as follows:
> > 
> > Patch 1 - Prepares ICE and storage layers (UFS and EMMC) to pass around wrapped keys.
> > Patch 2 - Adds a new SCM api to support deriving software secret when wrapped keys are used
> > Patch 3-4 - Adds support for wrapped keys in the ICE driver. This includes adding HWKM support
> > Patch 5-6 - Adds support for wrapped keys in UFS
> > Patch 7-10 - Supports generate, prepare and import functionality in ICE and UFS
> > 
> > NOTE: MMC will have similar changes to UFS and will be uploaded in a different patchset
> >        Patch 3, 4, 8, 10 will have MMC equivalents.
> > 
> > Testing:
> > Test platform: SM8550 MTP
> > Engineering trustzone image is required to test this feature only
> > for SM8550. For SM8650 onwards, all trustzone changes to support this
> > will be part of the released images.
> 
> AFAIU, Prior to these proposed changes in scm, HWKM was done with help of
> TA(Trusted Application) for generate, import, unwrap ... functionality.
> 
> 1. What is the reason for moving this from TA to new smc calls?
> 
> Is this because of missing smckinvoke support in upstream?
> 
> How scalable is this approach? Are we going to add new sec sys calls to
> every interface to TA?
> 
> 2. How are the older SoCs going to deal with this, given that you are
> changing drivers that are common across these?
> 
> Have you tested these patches on any older platforms?
> 
> What happens if someone want to add support to wrapped keys to this
> platforms in upstream, How is that going to be handled?
> 
> As I understand with this, we will endup with two possible solutions over
> time in upstream.

It's true that Qualcomm based Android devices already use HW-wrapped keys on
SoCs earlier than SM8650.  The problem is that the key generation, import, and
conversion were added to Android's KeyMint HAL, as a quick way to get the
feature out the door when it was needed (so to speak).  Unfortunately this
coupled this feature unnecessarily to the Android KeyMint and the corresponding
(closed source) userspace HAL provided by Qualcomm, which it's not actually
related to.  I'd guess that Qualcomm's closed source userspace HAL makes SMC
calls into Qualcomm's KeyMint TA, but I have no insight into those details.

The new SMC calls eliminate the dependency on the Android-specific KeyMint.
They're also being documented by Qualcomm.  So, as this patchset does, they can
be used by Linux in the implementation of new ioctls which provide a vendor
independent interface to HW-wrapped key generation, import, and conversion.

I think the new approach is the only one that is viable outside the Android
context.  As such, I don't think anyone has any plan to upstream support for
HW-wrapped keys for older Qualcomm SoCs that lack the new interface.

- Eric
Srinivas Kandagatla Aug. 29, 2023, 5:11 p.m. UTC | #7
On 25/08/2023 22:07, Eric Biggers wrote:
> Hi Srinivas,
> 
> On Fri, Aug 25, 2023 at 11:19:41AM +0100, Srinivas Kandagatla wrote:
>>
>> On 19/07/2023 18:04, Gaurav Kashyap wrote:
>>> These patches add support to Qualcomm ICE (Inline Crypto Enginr) for hardware
>>> wrapped keys using Qualcomm Hardware Key Manager (HWKM) and are made on top
>>> of a rebased version  Eric Bigger's set of changes to support wrapped keys in
>>> fscrypt and block below:
>>> https://git.kernel.org/pub/scm/fs/fscrypt/linux.git/log/?h=wrapped-keys-v7
>>> (The rebased patches are not uploaded here)
>>>
>>> Ref v1 here:
>>> https://lore.kernel.org/linux-scsi/20211206225725.77512-1-quic_gaurkash@quicinc.com/
>>>
>>> Explanation and use of hardware-wrapped-keys can be found here:
>>> Documentation/block/inline-encryption.rst
>>>
>>> This patch is organized as follows:
>>>
>>> Patch 1 - Prepares ICE and storage layers (UFS and EMMC) to pass around wrapped keys.
>>> Patch 2 - Adds a new SCM api to support deriving software secret when wrapped keys are used
>>> Patch 3-4 - Adds support for wrapped keys in the ICE driver. This includes adding HWKM support
>>> Patch 5-6 - Adds support for wrapped keys in UFS
>>> Patch 7-10 - Supports generate, prepare and import functionality in ICE and UFS
>>>
>>> NOTE: MMC will have similar changes to UFS and will be uploaded in a different patchset
>>>         Patch 3, 4, 8, 10 will have MMC equivalents.
>>>
>>> Testing:
>>> Test platform: SM8550 MTP
>>> Engineering trustzone image is required to test this feature only
>>> for SM8550. For SM8650 onwards, all trustzone changes to support this
>>> will be part of the released images.
>>
>> AFAIU, Prior to these proposed changes in scm, HWKM was done with help of
>> TA(Trusted Application) for generate, import, unwrap ... functionality.
>>
>> 1. What is the reason for moving this from TA to new smc calls?
>>
>> Is this because of missing smckinvoke support in upstream?
>>
>> How scalable is this approach? Are we going to add new sec sys calls to
>> every interface to TA?
>>
>> 2. How are the older SoCs going to deal with this, given that you are
>> changing drivers that are common across these?
>>
>> Have you tested these patches on any older platforms?
>>
>> What happens if someone want to add support to wrapped keys to this
>> platforms in upstream, How is that going to be handled?
>>
>> As I understand with this, we will endup with two possible solutions over
>> time in upstream.
> 
> It's true that Qualcomm based Android devices already use HW-wrapped keys on
> SoCs earlier than SM8650.  The problem is that the key generation, import, and
> conversion were added to Android's KeyMint HAL, as a quick way to get the
> feature out the door when it was needed (so to speak).  Unfortunately this

There is an attempt in 2021 todo exactly same thing I guess,

https://patchwork.kernel.org/project/linux-fscrypt/cover/20211206225725.77512-1-quic_gaurkash@quicinc.com/

If this was the right thing to do they why is the TZ firmware on SoCs 
after 2021 not having support for this ?

> coupled this feature unnecessarily to the Android KeyMint and the corresponding
> (closed source) userspace HAL provided by Qualcomm, which it's not actually

So how does Andriod kernel upgrades work after applying this patchset on 
platforms like SM8550 or SM8450 or SM8250..or any old platforms.

> related to.  I'd guess that Qualcomm's closed source userspace HAL makes SMC
> calls into Qualcomm's KeyMint TA, but I have no insight into those details.
> 
If we have an smcinvoke tee driver we can talk to to this TA.

> The new SMC calls eliminate the dependency on the Android-specific KeyMint.

I can see that.

Am not against adding this new interface, but is this new interface 
leaving a gap for older platforms?


Is there any other technical reason for moving out from TA based to a 
smc calls?

And are we doing a quick solution here to fix something


> They're also being documented by Qualcomm.  So, as this patchset does, they can
> be used by Linux in the implementation of new ioctls which provide a vendor
> independent interface to HW-wrapped key generation, import, and conversion.
> 
> I think the new approach is the only one that is viable outside the Android
> context.  As such, I don't think anyone has any plan to upstream support for
> HW-wrapped keys for older Qualcomm SoCs that lack the new interface.

AFAIU, There are other downstream Qualcomm LE platforms that use wrapped 
key support with the older interface.
What happens to them whey then upgrade the kernel?

Does TA interface still continue to work with the changes that went into 
common drivers (ufs/sd)?

--srini

> 
> - Eric
Eric Biggers Aug. 29, 2023, 6:12 p.m. UTC | #8
Hi Srinivas,

On Tue, Aug 29, 2023 at 06:11:55PM +0100, Srinivas Kandagatla wrote:
> > It's true that Qualcomm based Android devices already use HW-wrapped keys on
> > SoCs earlier than SM8650.  The problem is that the key generation, import, and
> > conversion were added to Android's KeyMint HAL, as a quick way to get the
> > feature out the door when it was needed (so to speak).  Unfortunately this
> 
> There is an attempt in 2021 todo exactly same thing I guess,
> 
> https://patchwork.kernel.org/project/linux-fscrypt/cover/20211206225725.77512-1-quic_gaurkash@quicinc.com/
> 
> If this was the right thing to do they why is the TZ firmware on SoCs after
> 2021 not having support for this ?

That's on Qualcomm.  But my understanding is that it's just taking them several
years to get the TZ changes out due to their branching schedule.  Garauv
submitted the TZ changes in 2021, but apparently SM8550 had already branched at
that point, so SM8650 is the first one that will have it.

Just because it takes Qualcomm a while to get the firmware support for this
feature deployed doesn't mean that it's the wrong approach.

> 
> > coupled this feature unnecessarily to the Android KeyMint and the corresponding
> > (closed source) userspace HAL provided by Qualcomm, which it's not actually
> 
> So how does Andriod kernel upgrades work after applying this patchset on
> platforms like SM8550 or SM8450 or SM8250..or any old platforms.

The same way they did before.  Older devices won't use this new code.

BTW, this is irrelevant for upstream.

> 
> > related to.  I'd guess that Qualcomm's closed source userspace HAL makes SMC
> > calls into Qualcomm's KeyMint TA, but I have no insight into those details.
> > 
> If we have an smcinvoke tee driver we can talk to to this TA.
> 
> > The new SMC calls eliminate the dependency on the Android-specific KeyMint.
> 
> I can see that.
> 
> Am not against adding this new interface, but is this new interface leaving
> a gap for older platforms?
> 
> 
> Is there any other technical reason for moving out from TA based to a smc
> calls?
> 
> And are we doing a quick solution here to fix something

I have very little insight into Qualcomm's old interface, which is tied to the
Android-specific KeyMint and is not known to be usable outside the Android
context or without the closed source userspace HAL from Qualcomm.

I understand that Linux kernel features that are only usable with closed source
userspace libraries are heavily frowned on.  As are features that are tied to
Android and cannot be used on other Linux distros.

If Qualcomm can document the old interface and show that it's usable directly by
the Linux kernel, then we could consider it.  But without that, the new
interface is our only option.

> > They're also being documented by Qualcomm.  So, as this patchset does, they can
> > be used by Linux in the implementation of new ioctls which provide a vendor
> > independent interface to HW-wrapped key generation, import, and conversion.
> > 
> > I think the new approach is the only one that is viable outside the Android
> > context.  As such, I don't think anyone has any plan to upstream support for
> > HW-wrapped keys for older Qualcomm SoCs that lack the new interface.
> 
> AFAIU, There are other downstream Qualcomm LE platforms that use wrapped key
> support with the older interface.
> What happens to them whey then upgrade the kernel?
> 
> Does TA interface still continue to work with the changes that went into
> common drivers (ufs/sd)?

This is a strange line of questioning for upstream review, as this feature does
not exist upstream.  This is the first time it will be supported by upstream
Linux, ever.  Adding support for this feature does not break anything.

Downstream users who implemented a less well designed version of this feature
can continue to use their existing code.

BTW, I am the person who has gotten stuck maintaining the framework for
HW-wrapped key support in the Android Common Kernels...  So if you're trying to
make things "easier" for me, please don't.  I want to have a properly designed
version of the feature upstream, and then I'll change Android to use that
whenever possible.  That's the only real long-term solution.

- Eric
Konrad Dybcio Aug. 29, 2023, 9:06 p.m. UTC | #9
On 19.07.2023 19:04, Gaurav Kashyap wrote:
> These patches add support to Qualcomm ICE (Inline Crypto Enginr) for hardware
> wrapped keys using Qualcomm Hardware Key Manager (HWKM) and are made on top
> of a rebased version  Eric Bigger's set of changes to support wrapped keys in
> fscrypt and block below:
> https://git.kernel.org/pub/scm/fs/fscrypt/linux.git/log/?h=wrapped-keys-v7
> (The rebased patches are not uploaded here)
Please make sure to also CC maintainers of the related subsystems.

I recommend using the b4 tool [1], which takes care of this and similar
issues.

[1] https://b4.docs.kernel.org/en/latest/index.html

Konrad
Srinivas Kandagatla Aug. 30, 2023, 10 a.m. UTC | #10
Hi Eric,

On 29/08/2023 19:12, Eric Biggers wrote:
> 
>>> They're also being documented by Qualcomm.  So, as this patchset does, they can
>>> be used by Linux in the implementation of new ioctls which provide a vendor
>>> independent interface to HW-wrapped key generation, import, and conversion.
>>>
>>> I think the new approach is the only one that is viable outside the Android
>>> context.  As such, I don't think anyone has any plan to upstream support for
>>> HW-wrapped keys for older Qualcomm SoCs that lack the new interface.
>> AFAIU, There are other downstream Qualcomm LE platforms that use wrapped key
>> support with the older interface.
>> What happens to them whey then upgrade the kernel?
>>
>> Does TA interface still continue to work with the changes that went into
>> common drivers (ufs/sd)?
> This is a strange line of questioning for upstream review, as this feature does
> not exist upstream.  This is the first time it will be supported by upstream
> Linux, ever.  Adding support for this feature does not break anything.
These are not unusual questions, what am trying to understand here is 
below questions for better context, big picture and review/test. At the 
end of the day we all want to get these features available in upstream.

1. How backward compatibility of this wrapped key support. I guess the 
answer is NO.

2. secondly reasons behind this change. Am still not really convinced 
with the current technical reasoning to shift from TA based approach to 
this. But I guess this is all done to dump the closed source userspace 
thingy. Am hoping that this can be made available to other older SoCs at 
some point in time.

3. We are adding these apis/callbacks in common code without doing any 
compatible or SoC checks. Is this going to be a issue if someone tries 
fscrypt?

--srini

> 
> Downstream users who implemented a less well designed version of this feature
> can continue to use their existing code.
Eric Biggers Aug. 30, 2023, 4:12 p.m. UTC | #11
On Wed, Aug 30, 2023 at 11:00:07AM +0100, Srinivas Kandagatla wrote:
> 
> 3. We are adding these apis/callbacks in common code without doing any
> compatible or SoC checks. Is this going to be a issue if someone tries
> fscrypt?

ufs-qcom only declares support for wrapped keys if it's supported.  See patch 5
of this series:

+	if (qcom_ice_hwkm_supported(host->ice))
+		hba->quirks |= UFSHCD_QUIRK_USES_WRAPPED_CRYPTO_KEYS;

That in turn uses:

+bool qcom_ice_hwkm_supported(struct qcom_ice *ice)
+{
+	return (ice->hwkm_version > 0);
+}
+EXPORT_SYMBOL_GPL(qcom_ice_hwkm_supported);

Which in turn comes from the ICE version being >= 3.2.  It does seem a bit
suspicious; it probably should check for both the ICE version and the
availability of QCOM_SCM_ES_GENERATE_ICE_KEY, QCOM_SCM_ES_PREPARE_ICE_KEY, and
QCOM_SCM_ES_IMPORT_ICE_KEY.  Regardless, it sounds like you want it to be
determined by something set in the device tree instead?  I don't think it's been
demonstrated that that's necessary.  If we can detect the hardware capabilities
dynamically, we should do that, right?

- Eric
Srinivas Kandagatla Aug. 30, 2023, 4:44 p.m. UTC | #12
On 30/08/2023 17:12, Eric Biggers wrote:
> On Wed, Aug 30, 2023 at 11:00:07AM +0100, Srinivas Kandagatla wrote:
>>
>> 3. We are adding these apis/callbacks in common code without doing any
>> compatible or SoC checks. Is this going to be a issue if someone tries
>> fscrypt?
> 
> ufs-qcom only declares support for wrapped keys if it's supported.  See patch 5
> of this series:
> 
> +	if (qcom_ice_hwkm_supported(host->ice))
> +		hba->quirks |= UFSHCD_QUIRK_USES_WRAPPED_CRYPTO_KEYS;
> 
> That in turn uses:
> 
> +bool qcom_ice_hwkm_supported(struct qcom_ice *ice)
> +{
> +	return (ice->hwkm_version > 0);
> +}
> +EXPORT_SYMBOL_GPL(qcom_ice_hwkm_supported);
> 
> Which in turn comes from the ICE version being >= 3.2.  It does seem a bit
> suspicious; it probably should check for both the ICE version and the
> availability of QCOM_SCM_ES_GENERATE_ICE_KEY, QCOM_SCM_ES_PREPARE_ICE_KEY, and
> QCOM_SCM_ES_IMPORT_ICE_KEY.  Regardless, it sounds like you want it to be
> determined by something set in the device tree instead?  I don't think it's been
> demonstrated that that's necessary.  If we can detect the hardware capabilities
> dynamically, we should do that, right?

I don't mind either way.

It would be perfect if we can dynamically query the TZ version to 
determine these capabilities.


If not we are left with some way to derive that information either via 
DT or other means.

--srini
> 
> - Eric
Gaurav Kashyap Sept. 19, 2023, 11:18 p.m. UTC | #13
Hello Srinivas,

On Tue, Sep 12, 2023 at 3:07 AM, Srinivas Kandagatla wrote:
> Hi Eric/Gaurav,
> 
> Adding more information and some questions to this discussion,
> 
> On 25/08/2023 14:07, Eric Biggers wrote:
> > Hi Srinivas,
> >
> > On Fri, Aug 25, 2023 at 11:19:41AM +0100, Srinivas Kandagatla wrote:
> >>
> >> On 19/07/2023 18:04, Gaurav Kashyap wrote:
> >>> These patches add support to Qualcomm ICE (Inline Crypto Enginr) for
> >>> hardware wrapped keys using Qualcomm Hardware Key Manager
> (HWKM) and
> >>> are made on top of a rebased version  Eric Bigger's set of changes
> >>> to support wrapped keys in fscrypt and block below:
> >>> https://git.kernel.org/pub/scm/fs/fscrypt/linux.git/log/?h=wrapped-k
> >>> eys-v7 (The rebased patches are not uploaded here)
> >>>
> >>> Ref v1 here:
> >>> https://lore.kernel.org/linux-scsi/20211206225725.77512-1-quic_gaurk
> >>> ash@quicinc.com/
> >>>
> >>> Explanation and use of hardware-wrapped-keys can be found here:
> >>> Documentation/block/inline-encryption.rst
> >>>
> >>> This patch is organized as follows:
> >>>
> >>> Patch 1 - Prepares ICE and storage layers (UFS and EMMC) to pass around
> wrapped keys.
> >>> Patch 2 - Adds a new SCM api to support deriving software secret
> >>> when wrapped keys are used Patch 3-4 - Adds support for wrapped keys
> >>> in the ICE driver. This includes adding HWKM support Patch 5-6 -
> >>> Adds support for wrapped keys in UFS Patch 7-10 - Supports generate,
> >>> prepare and import functionality in ICE and UFS
> >>>
> >>> NOTE: MMC will have similar changes to UFS and will be uploaded in a
> different patchset
> >>>         Patch 3, 4, 8, 10 will have MMC equivalents.
> >>>
> >>> Testing:
> >>> Test platform: SM8550 MTP
> >>> Engineering trustzone image is required to test this feature only
> >>> for SM8550. For SM8650 onwards, all trustzone changes to support
> >>> this will be part of the released images.
> >>
> >> AFAIU, Prior to these proposed changes in scm, HWKM was done with
> >> help of TA(Trusted Application) for generate, import, unwrap ...
> functionality.
> >>
> >> 1. What is the reason for moving this from TA to new smc calls?
> >>
> >> Is this because of missing smckinvoke support in upstream?
> >>
> >> How scalable is this approach? Are we going to add new sec sys calls
> >> to every interface to TA?
> >>
> >> 2. How are the older SoCs going to deal with this, given that you are
> >> changing drivers that are common across these?
> >>
> >> Have you tested these patches on any older platforms?
> >>
> >> What happens if someone want to add support to wrapped keys to this
> >> platforms in upstream, How is that going to be handled?
> >>
> >> As I understand with this, we will endup with two possible solutions
> >> over time in upstream.
> >
> > It's true that Qualcomm based Android devices already use HW-wrapped
> > keys on SoCs earlier than SM8650.  The problem is that the key
> > generation, import, and conversion were added to Android's KeyMint
> > HAL, as a quick way to get the feature out the door when it was needed
> > (so to speak).  Unfortunately this coupled this feature unnecessarily
> > to the Android KeyMint and the corresponding (closed source) userspace
> > HAL provided by Qualcomm, which it's not actually related to.  I'd
> > guess that Qualcomm's closed source userspace HAL makes SMC calls into
> Qualcomm's KeyMint TA, but I have no insight into those details.
> >
> > The new SMC calls eliminate the dependency on the Android-specific
> KeyMint.
> > They're also being documented by Qualcomm.  So, as this patchset does,
> > they can be used by Linux in the implementation of new ioctls which
> > provide a vendor independent interface to HW-wrapped key generation,
> import, and conversion.
> >
> > I think the new approach is the only one that is viable outside the
> > Android context.  As such, I don't think anyone has any plan to
> > upstream support for
> 
> Just bit of history afaiu.
> 
> on Qcom SoCs there are 3 ways to talk to Trusted service/Trusted application.
> 
> 1> Adding SCM calls. This is not scalable solution, imagine we keep
> adding new scm calls and static services to the TZ as required and this is going
> to bloat up the tz image size. Not only that, new SoCs would need to maintain
> backward compatibility, which is not going to happen.
> AFAIU this is discouraged in general and Qcom at some point in time will move
> away from this..
> 
> 2> using QSEECOM: This has some scalable issues, which is now replaced
> with smcinvoke.
> 
> 3> smcinvoke: This is preferred way to talk to any QTEE service or
> application. The issue is that this is based on some downstream UAPI which is
> not upstream ready yet.
> 
> IMO, adding a solution that is just going to live for few years is questionable
> for upstream.
> 
> Fixing [3] seems to be much scalable solution along with it we will also get
> support for this feature in all the Qualcomm platforms.
> 
> Am interested to hear what Gaurav has to say on this.
> 
> 
> --srini
> 
> 

What you are referring to is wrapped keys being generated in a Trusted Application (in case of android and some other solutions, keymaster), and eventually being programmed via SCM calls in TZ.
And, you are suggesting instead of ioctl->scm call, we should be doing ioctl->smcinvoke->TA

What Eric and I are adding here should not be just considered a stopgap, but a path for potential clients to generate and manage wrapped keys without requiring any TA.
The TA way will continue to work, and those wrapped keys can be programmed to ICE using fscrypt.

Unless, is the suggestion that wrapped keys should always be generated and managed in a trusted application?
And then programmed though kernel->SCM call interface?

> > HW-wrapped keys for older Qualcomm SoCs that lack the new interface.
> >
> > - Eric