[PATCH 2/5] drivers: security: Add TPM2 implementation of security devices

[Simon Glass]

Hi Sean,

On Thu, 17 Aug 2023 at 17:29, Sean Edmond <seanedmond at linux.microsoft.com>
wrote:
>
> Hi Simon,
>
> On 2023-08-17 6:41 a.m., Simon Glass wrote:
> > Hi Sean,
> >
> > On Fri, 11 Aug 2023 at 18:28, <seanedmond at linux.microsoft.com> wrote:
> >> From: Stephen Carlson <stcarlso at linux.microsoft.com>
> >>
> >> This implementation of the security uclass driver allows existing TPM2
> >> devices declared in the device tree to be referenced for storing the OS
> >> anti-rollback counter, using the TPM2 non-volatile storage API.
> >>
> >> Signed-off-by: Stephen Carlson <stcarlso at linux.microsoft.com>
> >> ---
> >>   MAINTAINERS                     |   1 +
> >>   drivers/security/Makefile       |   1 +
> >>   drivers/security/security-tpm.c | 173
++++++++++++++++++++++++++++++++
> >>   include/tpm-v2.h                |   1 +
> >>   4 files changed, 176 insertions(+)
> >>   create mode 100644 drivers/security/security-tpm.c
> > This is a bit wonky w.r.t driver model. The TPM itself should
> > implement this API, perhaps ina separate file shared with all v2 TPMs.
> > You should not be opening the device, etc. in here.
> >
> > I hope that makes sense...you effectively need to turn the TPM into a
> > multi-function device within driver model. Of course TPMs are
> > multi-function devices anyway, but here you are making their functions
> > available more explicitly with a nicer API.
> >
> > Then you can call the TPM API to do what you want, but at least you
> > know that the TPM has been probed before you start.
> >
> > Regards,
> > Simon
> >
> Let's step back for a moment to understand our intention with this
feature.
>
> We want secure storage for the anti-rollback counter.  The intention is
> that this storage is provided by whatever "secure driver" (let's start
> calling it the "rollback driver").  On our platform, we're using a
> different "rollback driver" which accesses a non-TPM based secure
> storage (which we can't upstream because it's proprietary).  For the
> purpose of making this feature publicly available, we've added the
> TPM-backed "rollback driver" (this patch).  I'll make this intention
> more clear in the documentation, which should lead to less confusion?
>
> At the end of the day, all we require is dm_security_arbvn_get() and
> dm_security_arbvn_set(), to get/set from the secure storage (we'll
> rename these to something less ugly, because yes arbvn is gross).  We
> don't want to lock this feature to the TPM, because it doesn't enable
> us/others to choose a different secure storage mechanism.

It doesn't need to be locked to the TPM. But since you have a uclass you
can have different drivers implementing the same UCLASS_ROLLBACK:

- a 'stand-alone' one that does strage and secret things
- a TPM-based one that makes TPM calls

>
> W.r.t opening/initializing the TPM.  Someone has to open it?  In our
> case, it's being opened earlier with our measured boot, so "-EBUSY" in
> returned on tpm_open() (we return and everyone is happy).  My
> understanding is that this is the currently available way for multiple
> drivers to access the TPM.  Ilias has recommended we use
> tpm_auto_start(), which is an enhancement I'm planning to make.  This
> should clean thing up?  If this doesn't meet your expectations, can you
> describe in more detail how we should turn the TPM into a
> "multi-function device"?

The TPM will be probed automatically by probing its child device. Not
opened, though...that would have to happen elsewhere.

But it doesn't mean you need to turn the TPM into a multi-function device.
It's just that, in most cases, others would use a TPM to provide the
rollback counters.

For testing purposes, you should probably create a device which is a child
of the sandbox TPM2 and run the tests with that.

Regards,
Simon