Message-ID: <efc0d71d27854a3bbe008fffc666d638@quicinc.com>
Date: Tue, 25 Jun 2024 04:58:31 +0000
From: "Gaurav Kashyap (QUIC)" <quic_gaurkash@...cinc.com>
To: Eric Biggers <ebiggers@...nel.org>,
        Gaurav Kashyap
	<gaurkash@....qualcomm.com>
CC: "dmitry.baryshkov@...aro.org" <dmitry.baryshkov@...aro.org>,
        "linux-arm-msm@...r.kernel.org" <linux-arm-msm@...r.kernel.org>,
        "linux-scsi@...r.kernel.org" <linux-scsi@...r.kernel.org>,
        "andersson@...nel.org" <andersson@...nel.org>,
        "neil.armstrong@...aro.org"
	<neil.armstrong@...aro.org>,
        srinivas.kandagatla
	<srinivas.kandagatla@...aro.org>,
        "krzysztof.kozlowski+dt@...aro.org"
	<krzysztof.kozlowski+dt@...aro.org>,
        "conor+dt@...nel.org"
	<conor+dt@...nel.org>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-mmc@...r.kernel.org" <linux-mmc@...r.kernel.org>,
        kernel
	<kernel@...cinc.com>,
        "linux-crypto@...r.kernel.org"
	<linux-crypto@...r.kernel.org>,
        "devicetree@...r.kernel.org"
	<devicetree@...r.kernel.org>,
        "Om Prakash Singh (QUIC)"
	<quic_omprsing@...cinc.com>,
        "Bao D. Nguyen (QUIC)"
	<quic_nguyenb@...cinc.com>,
        bartosz.golaszewski
	<bartosz.golaszewski@...aro.org>,
        "konrad.dybcio@...aro.org"
	<konrad.dybcio@...aro.org>,
        "ulf.hansson@...aro.org"
	<ulf.hansson@...aro.org>,
        "jejb@...ux.ibm.com" <jejb@...ux.ibm.com>,
        "martin.petersen@...cle.com" <martin.petersen@...cle.com>,
        "mani@...nel.org"
	<mani@...nel.org>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "herbert@...dor.apana.org.au" <herbert@...dor.apana.org.au>,
        Prasad Sodagudi
	<psodagud@...cinc.com>,
        Sonal Gupta <sonalg@...cinc.com>
Subject: RE: [PATCH v5 04/15] soc: qcom: ice: add hwkm support in ice


Hey Eric

On 06/21/2024, 9:02 AM PDT, Eric Biggers wrote:
> On Fri, Jun 21, 2024 at 03:35:40PM +0000, Gaurav Kashyap wrote:
> > Hello Eric
> >
> > On 06/20/2024, 9:48 PM PDT, Eric Biggers wrote:
> > > On Thu, Jun 20, 2024 at 02:57:40PM +0300, Dmitry Baryshkov wrote:
> > > > > > >
> > > > > > > > Is it possible to use both kind of keys when working on
> > > > > > > > standard
> > > mode?
> > > > > > > > If not, it should be the user who selects what type of
> > > > > > > > keys to be
> > > used.
> > > > > > > > Enforcing this via DT is not a way to go.
> > > > > > > >
> > > > > > >
> > > > > > > Unfortunately, that support is not there yet. When you say
> > > > > > > user, do you mean to have it as a filesystem mount option?
> > > > > >
> > > > > > During cryptsetup time. When running e.g. cryptsetup I, as a
> > > > > > user, would like to be able to use either a hardware-wrapped
> > > > > > key or a
> > > standard key.
> > > > > >
> > > > >
> > > > > What we are looking for with these patches is for
> > > > > per-file/folder
> > > encryption using fscrypt policies.
> > > > > Cryptsetup to my understanding supports only full-disk , and
> > > > > does not support FBE (File-Based)
> > > >
> > > > I must admit, I mostly used dm-crypt beforehand, so I had to look
> > > > at fscrypt now. Some of my previous comments might not be fully
> > > > applicable.
> > > >
> > > > > Hence the idea here is that we mount an unencrypted device (with
> > > > > the inlinecrypt option that indicates inline encryption is
> > > > > supported) And
> > > specify policies (links to keys) for different folders.
> > > > >
> > > > > > > The way the UFS/EMMC crypto layer is designed currently is
> > > > > > > that, this information is needed when the modules are loaded.
> > > > > > >
> > > > > > > https://lore.kernel.org/all/20231104211259.17448-2-ebiggers@
> > > > > > > kern el.org /#Z31drivers:ufs:core:ufshcd-crypto.c
> > > > > >
> > > > > > I see that the driver lists capabilities here. E.g. that it
> > > > > > supports HW-wrapped keys. But the line doesn't specify that
> > > > > > standard
> > > keys are not supported.
> > > > > >
> > > > >
> > > > > Those are capabilities that are read from the storage controller.
> > > > > However, wrapped keys Are not a standard in the ICE JEDEC
> > > > > specification, and in most cases, is a value add coming from the SoC.
> > > > >
> > > > > QCOM SOC and firmware currently does not support both kinds of
> > > > > keys in
> > > the HWKM mode.
> > > > > That is something we are internally working on, but not available yet.
> > > >
> > > > I'd say this is a significant obstacle, at least from my point of
> > > > view. I understand that the default might be to use hw-wrapped
> > > > keys, but it should be possible for the user to select non-HW keys
> > > > if the ability to recover the data is considered to be important.
> > > > Note, I'm really pointing to the user here, not to the system
> > > > integrator. So using DT property or specifying kernel arguments to
> > > > switch between these modes is not really an option.
> > > >
> > > > But I'd really love to hear some feedback from linux-security
> > > > and/or linux-fscrypt here.
> > > >
> > > > In my humble opinion the user should be able to specify that the
> > > > key is wrapped using the hardware KMK. Then if the hardware has
> > > > already started using the other kind of keys, it should be able to
> > > > respond with -EINVAL / whatever else. Then the user can evict
> > > > previously programmed key and program a desired one.
> > > >
> > > > > > Also, I'd have expected that hw-wrapped keys are handled using
> > > > > > trusted keys mechanism (see security/keys/trusted-keys/).
> > > > > > Could you please point out why that's not the case?
> > > > > >
> > > > >
> > > > > I will evaluate this.
> > > > > But my initial response is that we currently cannot communicate
> > > > > to our TPM directly from HLOS, but goes through QTEE, and I
> > > > > don't think our qtee currently interfaces with the open source
> > > > > tee driver. The
> > > interface is through QCOM SCM driver.
> > > >
> > > > Note, this is just an API interface, see how it is implemented for
> > > > the CAAM hardware.
> > > >
> > >
> > > The problem is that this patchset was sent out without the patches
> > > that add the block and filesystem-level framework for
> > > hardware-wrapped inline encryption keys, which it depends on.  So
> > > it's lacking context.  The proposed framework can be found at
> > > https://lore.kernel.org/linux-
> > > block/20231104211259.17448-1-ebiggers@...nel.org/T/#u
> > >
> >
> > I have only been adding the fscryp patch link as part of the cover letter - as
> a dependency.
> > https://lore.kernel.org/all/20240617005825.1443206-1-quic_gaurkash@qui
> > cinc.com/ If you would like me to include it in the patch series
> > itself, I can do that as well.
> >
> 
> I think including all prerequisite patches would be helpful for reviewers.

Noted. I'll do that for the next patch.

> 
> Thanks for continuing to work on this!
> 
> I still need to get ahold of a sm8650 based device and test this out.  Is the
> SM8650 HDK the only option, or is there a sm8650 based phone with
> upstream support yet?

There are some devices released with SM8650 (Snapdragon 8 Gen 3). Sorry, I have
not kept track of which. I know the S24s were released with that. But there should be
more in the market.

> 
> - Eric

Powered by blists - more mailing lists