[RFC/RFT,v4,0/5] Add generic trusted keys framework/subsystem
mbox series

Message ID 1565682784-10234-1-git-send-email-sumit.garg@linaro.org
Headers show
Series
  • Add generic trusted keys framework/subsystem
Related show

Message

Sumit Garg Aug. 13, 2019, 7:52 a.m. UTC
This patch-set is an outcome of discussion here [1]. It has evolved very
much since v1 to create, consolidate and generalize trusted keys
subsystem.

This framework has been tested with trusted keys support provided via TEE
but I wasn't able to test it with a TPM device as I don't possess one. It
would be really helpful if others could test this patch-set using a TPM
device.

[1] https://www.mail-archive.com/linux-doc@vger.kernel.org/msg30591.html

Changes in v4:
1. Separate patch for export of tpm_buf code to include/linux/tpm.h
2. Change TPM1.x trusted keys code to use common tpm_buf
3. Keep module name as trusted.ko only

Changes in v3:

Move TPM2 trusted keys code to trusted keys subsystem.

Changes in v2:

Split trusted keys abstraction patch for ease of review.

Sumit Garg (5):
  tpm: move tpm_buf code to include/linux/
  KEYS: trusted: use common tpm_buf for TPM1.x code
  KEYS: trusted: create trusted keys subsystem
  KEYS: trusted: move tpm2 trusted keys code
  KEYS: trusted: Add generic trusted keys framework

 crypto/asymmetric_keys/asym_tpm.c                  |   2 +-
 drivers/char/tpm/tpm-chip.c                        |   1 +
 drivers/char/tpm/tpm-interface.c                   |  56 ---
 drivers/char/tpm/tpm.h                             | 230 -----------
 drivers/char/tpm/tpm2-cmd.c                        | 308 +--------------
 include/keys/trusted-type.h                        |  45 +++
 include/keys/{trusted.h => trusted_tpm.h}          |  61 +--
 include/linux/tpm.h                                | 270 ++++++++++++-
 security/keys/Makefile                             |   2 +-
 security/keys/trusted-keys/Makefile                |   9 +
 security/keys/trusted-keys/trusted-common.c        | 343 ++++++++++++++++
 .../keys/{trusted.c => trusted-keys/trusted-tpm.c} | 437 +++++----------------
 security/keys/trusted-keys/trusted-tpm2.c          | 378 ++++++++++++++++++
 13 files changed, 1141 insertions(+), 1001 deletions(-)
 rename include/keys/{trusted.h => trusted_tpm.h} (64%)
 create mode 100644 security/keys/trusted-keys/Makefile
 create mode 100644 security/keys/trusted-keys/trusted-common.c
 rename security/keys/{trusted.c => trusted-keys/trusted-tpm.c} (72%)
 create mode 100644 security/keys/trusted-keys/trusted-tpm2.c

Comments

Mimi Zohar Aug. 14, 2019, 1:24 p.m. UTC | #1
Hi Sumit,

On Tue, 2019-08-13 at 13:22 +0530, Sumit Garg wrote:
> This patch-set is an outcome of discussion here [1]. It has evolved very
> much since v1 to create, consolidate and generalize trusted keys
> subsystem.
> 
> This framework has been tested with trusted keys support provided via TEE
> but I wasn't able to test it with a TPM device as I don't possess one. It
> would be really helpful if others could test this patch-set using a TPM
> device.

With the "CONFIG_HEADER_TEST" and "CONFIG_KERNEL_HEADER_TEST" config
options enabled, which is required for linux-next, it fails to build.

Mimi
Sumit Garg Aug. 15, 2019, 1:03 p.m. UTC | #2
Hi Mimi,

On Wed, 14 Aug 2019 at 18:54, Mimi Zohar <zohar@kernel.org> wrote:
>
> Hi Sumit,
>
> On Tue, 2019-08-13 at 13:22 +0530, Sumit Garg wrote:
> > This patch-set is an outcome of discussion here [1]. It has evolved very
> > much since v1 to create, consolidate and generalize trusted keys
> > subsystem.
> >
> > This framework has been tested with trusted keys support provided via TEE
> > but I wasn't able to test it with a TPM device as I don't possess one. It
> > would be really helpful if others could test this patch-set using a TPM
> > device.
>
> With the "CONFIG_HEADER_TEST" and "CONFIG_KERNEL_HEADER_TEST" config
> options enabled, which is required for linux-next, it fails to build.
>

TBH, I wasn't aware about this test feature for headers. It looks like
the header which fails this test is "include/keys/trusted_tpm.h" which
is basically a rename of "include/keys/trusted.h" plus changes in this
patch-set.

And "include/keys/trusted.h" header is already put under blacklist
here: "include/Kbuild +68" as it fails to build. So its that rename
due to which build failure is observed now.

It seems to be an easy fix for this build failure via following changes:

diff --git a/include/keys/trusted_tpm.h b/include/keys/trusted_tpm.h
index 7b593447920b..ca1bec0ef65d 100644
--- a/include/keys/trusted_tpm.h
+++ b/include/keys/trusted_tpm.h
@@ -2,6 +2,9 @@
 #ifndef __TRUSTED_TPM_H
 #define __TRUSTED_TPM_H

+#include <keys/trusted-type.h>
+#include <linux/tpm_command.h>
+
 /* implementation specific TPM constants */
 #define MAX_BUF_SIZE                   1024
 #define TPM_GETRANDOM_SIZE             14

So I will include above changes in this patch-set and also remove
"include/keys/trusted.h" header from the blacklist.

-Sumit

> Mimi
Mimi Zohar Aug. 15, 2019, 3:06 p.m. UTC | #3
On Thu, 2019-08-15 at 18:33 +0530, Sumit Garg wrote:
> Hi Mimi,
> 
> On Wed, 14 Aug 2019 at 18:54, Mimi Zohar <zohar@kernel.org> wrote:
> >
> > Hi Sumit,
> >
> > On Tue, 2019-08-13 at 13:22 +0530, Sumit Garg wrote:
> > > This patch-set is an outcome of discussion here [1]. It has evolved very
> > > much since v1 to create, consolidate and generalize trusted keys
> > > subsystem.
> > >
> > > This framework has been tested with trusted keys support provided via TEE
> > > but I wasn't able to test it with a TPM device as I don't possess one. It
> > > would be really helpful if others could test this patch-set using a TPM
> > > device.
> >
> > With the "CONFIG_HEADER_TEST" and "CONFIG_KERNEL_HEADER_TEST" config
> > options enabled, which is required for linux-next, it fails to build.
> >
> 
> TBH, I wasn't aware about this test feature for headers. 

It's new to me too.

> It looks like
> the header which fails this test is "include/keys/trusted_tpm.h" which
> is basically a rename of "include/keys/trusted.h" plus changes in this
> patch-set.
> 
> And "include/keys/trusted.h" header is already put under blacklist
> here: "include/Kbuild +68" as it fails to build. So its that rename
> due to which build failure is observed now.
> 
> It seems to be an easy fix for this build failure via following changes:
> 
> diff --git a/include/keys/trusted_tpm.h b/include/keys/trusted_tpm.h
> index 7b593447920b..ca1bec0ef65d 100644
> --- a/include/keys/trusted_tpm.h
> +++ b/include/keys/trusted_tpm.h
> @@ -2,6 +2,9 @@
>  #ifndef __TRUSTED_TPM_H
>  #define __TRUSTED_TPM_H
> 
> +#include <keys/trusted-type.h>
> +#include <linux/tpm_command.h>
> +
>  /* implementation specific TPM constants */
>  #define MAX_BUF_SIZE                   1024
>  #define TPM_GETRANDOM_SIZE             14
> 
> So I will include above changes in this patch-set and also remove
> "include/keys/trusted.h" header from the blacklist.

That works, thanks.  With this patch set, at least the EVM trusted key
is properly being decrypted by the encrypted key with both a TPM 1.2
and PTT TPM 2.0.  My laptop still boots properly.  Over the weekend
I'll try to actually review the patches.

Mimi
Sumit Garg Aug. 16, 2019, 4:58 a.m. UTC | #4
On Thu, 15 Aug 2019 at 20:36, Mimi Zohar <zohar@kernel.org> wrote:
>
> On Thu, 2019-08-15 at 18:33 +0530, Sumit Garg wrote:
> > Hi Mimi,
> >
> > On Wed, 14 Aug 2019 at 18:54, Mimi Zohar <zohar@kernel.org> wrote:
> > >
> > > Hi Sumit,
> > >
> > > On Tue, 2019-08-13 at 13:22 +0530, Sumit Garg wrote:
> > > > This patch-set is an outcome of discussion here [1]. It has evolved very
> > > > much since v1 to create, consolidate and generalize trusted keys
> > > > subsystem.
> > > >
> > > > This framework has been tested with trusted keys support provided via TEE
> > > > but I wasn't able to test it with a TPM device as I don't possess one. It
> > > > would be really helpful if others could test this patch-set using a TPM
> > > > device.
> > >
> > > With the "CONFIG_HEADER_TEST" and "CONFIG_KERNEL_HEADER_TEST" config
> > > options enabled, which is required for linux-next, it fails to build.
> > >
> >
> > TBH, I wasn't aware about this test feature for headers.
>
> It's new to me too.
>
> > It looks like
> > the header which fails this test is "include/keys/trusted_tpm.h" which
> > is basically a rename of "include/keys/trusted.h" plus changes in this
> > patch-set.
> >
> > And "include/keys/trusted.h" header is already put under blacklist
> > here: "include/Kbuild +68" as it fails to build. So its that rename
> > due to which build failure is observed now.
> >
> > It seems to be an easy fix for this build failure via following changes:
> >
> > diff --git a/include/keys/trusted_tpm.h b/include/keys/trusted_tpm.h
> > index 7b593447920b..ca1bec0ef65d 100644
> > --- a/include/keys/trusted_tpm.h
> > +++ b/include/keys/trusted_tpm.h
> > @@ -2,6 +2,9 @@
> >  #ifndef __TRUSTED_TPM_H
> >  #define __TRUSTED_TPM_H
> >
> > +#include <keys/trusted-type.h>
> > +#include <linux/tpm_command.h>
> > +
> >  /* implementation specific TPM constants */
> >  #define MAX_BUF_SIZE                   1024
> >  #define TPM_GETRANDOM_SIZE             14
> >
> > So I will include above changes in this patch-set and also remove
> > "include/keys/trusted.h" header from the blacklist.
>
> That works, thanks.  With this patch set, at least the EVM trusted key
> is properly being decrypted by the encrypted key with both a TPM 1.2
> and PTT TPM 2.0.  My laptop still boots properly.  Over the weekend
> I'll try to actually review the patches.
>

Thanks Mimi for testing this patch-set.

-Sumit

> Mimi
Jarkko Sakkinen Aug. 19, 2019, 4:54 p.m. UTC | #5
On Tue, Aug 13, 2019 at 01:22:59PM +0530, Sumit Garg wrote:
> This patch-set is an outcome of discussion here [1]. It has evolved very
> much since v1 to create, consolidate and generalize trusted keys
> subsystem.
> 
> This framework has been tested with trusted keys support provided via TEE
> but I wasn't able to test it with a TPM device as I don't possess one. It
> would be really helpful if others could test this patch-set using a TPM
> device.

I think 1/5-4/5 make up a non-RFC patch set that needs to reviewed,
tested and merged as a separate entity.

On the other hand 5/5 cannot be merged even if I fully agreed on
the code change as without TEE patch it does not add any value for
Linux.

To straighten up thing I would suggest that the next patch set
version would only consists of the first four patches and we meld
them to the shape so that we can land them to the mainline. Then
it should be way more easier to concentrate the actual problem you
are trying to resolve.

/Jarkko
Sumit Garg Aug. 20, 2019, 5:46 a.m. UTC | #6
On Mon, 19 Aug 2019 at 22:24, Jarkko Sakkinen
<jarkko.sakkinen@linux.intel.com> wrote:
>
> On Tue, Aug 13, 2019 at 01:22:59PM +0530, Sumit Garg wrote:
> > This patch-set is an outcome of discussion here [1]. It has evolved very
> > much since v1 to create, consolidate and generalize trusted keys
> > subsystem.
> >
> > This framework has been tested with trusted keys support provided via TEE
> > but I wasn't able to test it with a TPM device as I don't possess one. It
> > would be really helpful if others could test this patch-set using a TPM
> > device.
>
> I think 1/5-4/5 make up a non-RFC patch set that needs to reviewed,
> tested and merged as a separate entity.
>

Okay.

> On the other hand 5/5 cannot be merged even if I fully agreed on
> the code change as without TEE patch it does not add any value for
> Linux.
>

I agree here that 5/5 should go along with TEE patch-set. But if you
look at initial v1 patch-set, the idea was to get feedback on trusted
keys abstraction as a standalone patch along with testing using a TPM
(1.x or 2.0).

Since Mimi has tested this patch-set with TPM (1.x & 2.0), I am happy
to merge 5/5 with TEE patch-set. But it would be nice if I could get
feedback on 5/5 before I send next version of TEE patch-set.

> To straighten up thing I would suggest that the next patch set
> version would only consists of the first four patches and we meld
> them to the shape so that we can land them to the mainline. Then
> it should be way more easier to concentrate the actual problem you
> are trying to resolve.
>

Okay will send next patch-set version with first four patches only.

-Sumit

> /Jarkko
Jarkko Sakkinen Aug. 21, 2019, 7:12 p.m. UTC | #7
On Tue, Aug 20, 2019 at 11:16:46AM +0530, Sumit Garg wrote:
> I agree here that 5/5 should go along with TEE patch-set. But if you
> look at initial v1 patch-set, the idea was to get feedback on trusted
> keys abstraction as a standalone patch along with testing using a TPM
> (1.x or 2.0).
> 
> Since Mimi has tested this patch-set with TPM (1.x & 2.0), I am happy
> to merge 5/5 with TEE patch-set. But it would be nice if I could get
> feedback on 5/5 before I send next version of TEE patch-set.

OK, that is understandable. I'll check it out tomorrow.

/Jarkko