RE: [RFC PATCH 2/5] char: rpmb: provide a user space interface

From: "Winkler, Tomas" <tomas.winkler@intel.com>
To: Arnd Bergmann <arnd@linaro.org>
Cc: "Alex Bennée" <alex.bennee@linaro.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"maxim.uvarov@linaro.org" <maxim.uvarov@linaro.org>,
	"joakim.bech@linaro.org" <joakim.bech@linaro.org>,
	"ilias.apalodimas@linaro.org" <ilias.apalodimas@linaro.org>,
	"ruchika.gupta@linaro.org" <ruchika.gupta@linaro.org>,
	"Huang, Yang" <yang.huang@intel.com>,
	"Zhu, Bing" <bing.zhu@intel.com>,
	"Matti.Moell@opensynergy.com" <Matti.Moell@opensynergy.com>,
	"hmo@opensynergy.com" <hmo@opensynergy.com>,
	"linux-mmc@vger.kernel.org" <linux-mmc@vger.kernel.org>,
	"linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>,
	"linux-nvme@lists.infradead.org" <linux-nvme@lists.infradead.org>,
	"Ulf Hansson" <ulf.hansson@linaro.org>,
	"Linus Walleij" <linus.walleij@linaro.org>,
	"Arnd Bergmann" <arnd.bergmann@linaro.org>,
	"Usyskin, Alexander" <alexander.usyskin@intel.com>,
	"Avri Altman" <avri.altman@sandisk.com>
Subject: RE: [RFC PATCH 2/5] char: rpmb: provide a user space interface
Date: Fri, 5 Mar 2021 06:31:14 +0000	[thread overview]
Message-ID: <02dc8b75f0344c659e85ed4267d66102@intel.com> (raw)
In-Reply-To: <CAK8P3a0ATHxzS02_5kypbGwHYLaWZmEPG8xtZchWuuM-93o8CA@mail.gmail.com>

> On Thu, Mar 4, 2021 at 8:54 PM Winkler, Tomas <tomas.winkler@intel.com>
> wrote:
> > > Winkler, Tomas <tomas.winkler@intel.com> writes:
> > > >> "Winkler, Tomas" <tomas.winkler@intel.com> writes:
> > > >>
> > > >> >> The user space API is achieved via a number of synchronous
> IOCTLs.
> > > >> >>
> > > >> >>   * RPMB_IOC_VER_CMD - simple versioning API
> > > >> >>   * RPMB_IOC_CAP_CMD - query of underlying capabilities
> > > >> >>   * RPMB_IOC_PKEY_CMD - one time programming of access key
> > > >> >>   * RPMB_IOC_COUNTER_CMD - query the write counter
> > > >> >>   * RPMB_IOC_WBLOCKS_CMD - write blocks to device
> > > >> >>   * RPMB_IOC_RBLOCKS_CMD - read blocks from device
> > > >> >>
> > > >> >> The keys used for programming and writing blocks to the device
> > > >> >> are key_serial_t handles as provided by the keyctl() interface.
> > > >> >>
> > > >> >> [AJB: here there are two key differences between this and the
> > > >> >> original proposal. The first is the dropping of the sequence
> > > >> >> of preformated frames in favour of explicit actions. The
> > > >> >> second is the introduction of key_serial_t and the keyring API
> > > >> >> for referencing the key to use]
> > > >> >
> > > >> > Putting it gently I'm not sure this is good idea, from the
> > > >> > security point of
> > > >> view.
> > > >> > The key has to be possession of the one that signs the frames
> > > >> > as they are,
> > > >> it doesn't mean it is linux kernel keyring, it can be other party
> > > >> on different system.
> > > >> > With this approach you will make the other usecases not applicable.
> > > >> > It is less then trivial to move key securely from one system to
> another.
> > > >>
> > > >> OK I can understand the desire for such a use-case but it does
> > > >> constrain the interface on the kernel with access to the hardware
> > > >> to purely providing a pipe to the raw hardware while also having
> > > >> to expose the details of the HW to userspace.
> > > > This is the use case in Android. The key is in the "trusty" which
> > > > different os running in a virtual environment. The file storage
> > > > abstraction is implemented there. I'm not sure the point of
> > > > constraining the kernel, can you please elaborate on that.
> > >
> > > Well the kernel is all about abstracting differences not baking in
> assumptions.
> > > However can I ask a bit more about this security model?
> > > Is the secure enclave just a separate userspace process or is it in
> > > a separate virtual machine? Is it accessible at all by the kernel running the
> driver?
> >
> > It's not an assumption this is working for few years already
> > (https://source.android.com/security/trusty#application_services)
> > The model is that you have a trusted environment (TEE)  in which can be in
> any of the form you described above.
> > And there is established agreement via the RPMB key that TEE is only
> > entity that can produce content to be stored on RPBM, The RPMB
> hardware also ensure that nobody can catch it in the middle and replay that
> storage event.
> >
> > My point is that interface you are suggesting is not covering all possible
> usages of RPMB, actually usages that are already in place.
> 
> It turned out that the application that we (Linaro) need does have the same
> requirements and needs to store the key in a TEE, transferring the message
> with the MAC into the kernel, rather than keeping the key stored in user
> space or kernel.
> 
> However, after I had a look at the nvme-rpmb user space implementation, I
> found that this is different, and always expects the key to be stored in a local
> file:
> https://github.com/linux-nvme/nvme-cli/blob/master/nvme-rpmb.c#L878

This doesn't make it very safe

> This both works with the same kernel interface though, as the kernel would
> still get the data along with the HMAC, rather than having the key stored in
> the kernel, but it does mean that the frame gets passed to the kernel in a
> device specific layout, with at least nvme using an incompatible layout from
> everything else.

NVMe is not by JEDEC so this layout is different and there are some changes but the overall storage operations are the same story.
 I do have a solution also for NVME inclusion into the framework. I haven't published that part yet.  
It won't support virtio part,  as this requires some legal work to include that into  virtio spec.

Thanks
Tomas

>         Arnd