Re: [PATCH v7 4/4] MAINTAINERS: Add entry for TEE based Trusted Keys

Re: [PATCH v7 4/4] MAINTAINERS: Add entry for TEE based Trusted Keys

[Jarkko Sakkinen]

[-- Attachment #1: Type: text/plain, Size: 3082 bytes --]

On Tue, Oct 13, 2020 at 04:58:47PM +0530, Sumit Garg wrote:
> On Tue, 13 Oct 2020 at 07:52, Jarkko Sakkinen
> <jarkko.sakkinen@linux.intel.com> wrote:
> >
> > On Wed, Oct 07, 2020 at 03:37:48PM +0530, Sumit Garg wrote:
> > > Add MAINTAINERS entry for TEE based Trusted Keys framework.
> > >
> > > Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
> > > Acked-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > > ---
> > >  MAINTAINERS | 8 ++++++++
> > >  1 file changed, 8 insertions(+)
> > >
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index 48aff80..eb3d889 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -9663,6 +9663,14 @@ F:     include/keys/trusted-type.h
> > >  F:   include/keys/trusted_tpm.h
> > >  F:   security/keys/trusted-keys/
> > >
> > > +KEYS-TRUSTED-TEE
> > > +M:   Sumit Garg <sumit.garg@linaro.org>
> > > +L:   linux-integrity(a)vger.kernel.org
> > > +L:   keyrings(a)vger.kernel.org
> > > +S:   Supported
> > > +F:   include/keys/trusted_tee.h
> > > +F:   security/keys/trusted-keys/trusted_tee.c
> > > +
> > >  KEYS/KEYRINGS
> > >  M:   David Howells <dhowells@redhat.com>
> > >  M:   Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > > --
> > > 2.7.4
> >
> > I'm sorry but I think I have changed my mind on this. This has been
> > spinning for a while and sometimes conclusions change over the time.
> >
> > I don't think that we really need a separate subsystem tag.
> 
> I don't see it as a separate subsystem but rather a kind of underlying
> trust source (TEE) driver plugged into existing trusted keys
> subsystem. We could relate it to the RNG subsystem as well where there
> is a subsystem maintainer and specific driver maintainers.
> 
> IMO, having a dedicated entry like this brings clarity in maintenance
> and in future we may have more trust sources like this added where
> everyone may not have access to all the trust sources to test.

More entries pointing to the exact same stuff does not necessarily mean
clarity in my books.

> > I'd be for a
> > new M-entry or R-entry to the existing subsystem tag. It's essential to
> > have ack from someone with ARM and TEE knowledge but this way too heavy
> > for the purpose.
> 
> If you still think otherwise then I am fine with a new M-entry for
> existing trusted keys subsystem as well.

Adding a M-entry does makes sense because trusted keys backends can be
based on various technologies and standard. It's a different in that
sense than lets say a TPM hardware driver.

> > I also see it the most manageable if the trusted keys PR's come from a
> > single source.
> 
> I echo here with you to have a single source for trusted keys PR's
> irrespective of whether we go with a separate trust source entry or
> update existing subsystem entry.
> 
> -Sumit

And I echo that oviously if there is someone to say the final ack about
TEE, I will require that as the minimum to ever pick any of those
changes :-)

I would resolve this with just the M-entry, and we can *later on*
restructure, if there is a need for that. These things are not sealed
to stone.

/Jarkko

Re: [PATCH v7 4/4] MAINTAINERS: Add entry for TEE based Trusted Keys

[Sumit Garg]

[-- Attachment #1: Type: text/plain, Size: 3381 bytes --]

On Tue, 13 Oct 2020 at 19:10, Jarkko Sakkinen <jarkko@kernel.org> wrote:
>
> On Tue, Oct 13, 2020 at 04:58:47PM +0530, Sumit Garg wrote:
> > On Tue, 13 Oct 2020 at 07:52, Jarkko Sakkinen
> > <jarkko.sakkinen@linux.intel.com> wrote:
> > >
> > > On Wed, Oct 07, 2020 at 03:37:48PM +0530, Sumit Garg wrote:
> > > > Add MAINTAINERS entry for TEE based Trusted Keys framework.
> > > >
> > > > Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
> > > > Acked-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > > > ---
> > > >  MAINTAINERS | 8 ++++++++
> > > >  1 file changed, 8 insertions(+)
> > > >
> > > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > > index 48aff80..eb3d889 100644
> > > > --- a/MAINTAINERS
> > > > +++ b/MAINTAINERS
> > > > @@ -9663,6 +9663,14 @@ F:     include/keys/trusted-type.h
> > > >  F:   include/keys/trusted_tpm.h
> > > >  F:   security/keys/trusted-keys/
> > > >
> > > > +KEYS-TRUSTED-TEE
> > > > +M:   Sumit Garg <sumit.garg@linaro.org>
> > > > +L:   linux-integrity(a)vger.kernel.org
> > > > +L:   keyrings(a)vger.kernel.org
> > > > +S:   Supported
> > > > +F:   include/keys/trusted_tee.h
> > > > +F:   security/keys/trusted-keys/trusted_tee.c
> > > > +
> > > >  KEYS/KEYRINGS
> > > >  M:   David Howells <dhowells@redhat.com>
> > > >  M:   Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > > > --
> > > > 2.7.4
> > >
> > > I'm sorry but I think I have changed my mind on this. This has been
> > > spinning for a while and sometimes conclusions change over the time.
> > >
> > > I don't think that we really need a separate subsystem tag.
> >
> > I don't see it as a separate subsystem but rather a kind of underlying
> > trust source (TEE) driver plugged into existing trusted keys
> > subsystem. We could relate it to the RNG subsystem as well where there
> > is a subsystem maintainer and specific driver maintainers.
> >
> > IMO, having a dedicated entry like this brings clarity in maintenance
> > and in future we may have more trust sources like this added where
> > everyone may not have access to all the trust sources to test.
>
> More entries pointing to the exact same stuff does not necessarily mean
> clarity in my books.
>
> > > I'd be for a
> > > new M-entry or R-entry to the existing subsystem tag. It's essential to
> > > have ack from someone with ARM and TEE knowledge but this way too heavy
> > > for the purpose.
> >
> > If you still think otherwise then I am fine with a new M-entry for
> > existing trusted keys subsystem as well.
>
> Adding a M-entry does makes sense because trusted keys backends can be
> based on various technologies and standard. It's a different in that
> sense than lets say a TPM hardware driver.
>
> > > I also see it the most manageable if the trusted keys PR's come from a
> > > single source.
> >
> > I echo here with you to have a single source for trusted keys PR's
> > irrespective of whether we go with a separate trust source entry or
> > update existing subsystem entry.
> >
> > -Sumit
>
> And I echo that oviously if there is someone to say the final ack about
> TEE, I will require that as the minimum to ever pick any of those
> changes :-)
>
> I would resolve this with just the M-entry, and we can *later on*
> restructure, if there is a need for that. These things are not sealed
> to stone.

Okay, will add a M-entry for existing trusted keys subsystem.

-Sumit

>
> /Jarkko

[PATCH v7 0/4] Introduce TEE based Trusted Keys support

[Sumit Garg]

[-- Attachment #1: Type: text/plain, Size: 2758 bytes --]

Add support for TEE based trusted keys where TEE provides the functionality
to seal and unseal trusted keys using hardware unique key. Also, this is
an alternative in case platform doesn't possess a TPM device.

This patch-set has been tested with OP-TEE based early TA which is already
merged in upstream [1].

[1] https://github.com/OP-TEE/optee_os/commit/f86ab8e7e0de869dfa25ca05a37ee070d7e5b86b

Changes in v7:
1. Added a trusted.source module parameter in order to enforce user's
   choice in case a particular platform posses both TPM and TEE.
2. Refine commit description for patch #1.

Changes in v6:
1. Revert back to dynamic detection of trust source.
2. Drop author mention from trusted_core.c and trusted_tpm1.c files.
3. Rebased to latest tpmdd/master.

Changes in v5:
1. Drop dynamic detection of trust source and use compile time flags
   instead.
2. Rename trusted_common.c -> trusted_core.c.
3. Rename callback: cleanup() -> exit().
4. Drop "tk" acronym.
5. Other misc. comments.
6. Added review tags for patch #3 and #4.

Changes in v4:
1. Pushed independent TEE features separately:
  - Part of recent TEE PR: https://lkml.org/lkml/2020/5/4/1062
2. Updated trusted-encrypted doc with TEE as a new trust source.
3. Rebased onto latest tpmdd/master.

Changes in v3:
1. Update patch #2 to support registration of multiple kernel pages.
2. Incoporate dependency patch #4 in this patch-set:
   https://patchwork.kernel.org/patch/11091435/

Changes in v2:
1. Add reviewed-by tags for patch #1 and #2.
2. Incorporate comments from Jens for patch #3.
3. Switch to use generic trusted keys framework.

Sumit Garg (4):
  KEYS: trusted: Add generic trusted keys framework
  KEYS: trusted: Introduce TEE based Trusted Keys
  doc: trusted-encrypted: updates with TEE as a new trust source
  MAINTAINERS: Add entry for TEE based Trusted Keys

 Documentation/security/keys/trusted-encrypted.rst | 203 ++++++++++---
 MAINTAINERS                                       |   8 +
 include/keys/trusted-type.h                       |  47 +++
 include/keys/trusted_tee.h                        |  55 ++++
 include/keys/trusted_tpm.h                        |  17 +-
 security/keys/trusted-keys/Makefile               |   2 +
 security/keys/trusted-keys/trusted_core.c         | 334 +++++++++++++++++++++
 security/keys/trusted-keys/trusted_tee.c          | 278 ++++++++++++++++++
 security/keys/trusted-keys/trusted_tpm1.c         | 336 ++++------------------
 9 files changed, 953 insertions(+), 327 deletions(-)
 create mode 100644 include/keys/trusted_tee.h
 create mode 100644 security/keys/trusted-keys/trusted_core.c
 create mode 100644 security/keys/trusted-keys/trusted_tee.c

-- 
2.7.4

[PATCH v7 4/4] MAINTAINERS: Add entry for TEE based Trusted Keys

[Sumit Garg]

[-- Attachment #1: Type: text/plain, Size: 797 bytes --]

Add MAINTAINERS entry for TEE based Trusted Keys framework.

Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
Acked-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 MAINTAINERS | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 48aff80..eb3d889 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9663,6 +9663,14 @@ F:	include/keys/trusted-type.h
 F:	include/keys/trusted_tpm.h
 F:	security/keys/trusted-keys/
 
+KEYS-TRUSTED-TEE
+M:	Sumit Garg <sumit.garg@linaro.org>
+L:	linux-integrity(a)vger.kernel.org
+L:	keyrings(a)vger.kernel.org
+S:	Supported
+F:	include/keys/trusted_tee.h
+F:	security/keys/trusted-keys/trusted_tee.c
+
 KEYS/KEYRINGS
 M:	David Howells <dhowells@redhat.com>
 M:	Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
-- 
2.7.4

Re: [PATCH v7 4/4] MAINTAINERS: Add entry for TEE based Trusted Keys

[Jarkko Sakkinen]

[-- Attachment #1: Type: text/plain, Size: 1383 bytes --]

On Wed, Oct 07, 2020 at 03:37:48PM +0530, Sumit Garg wrote:
> Add MAINTAINERS entry for TEE based Trusted Keys framework.
> 
> Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
> Acked-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> ---
>  MAINTAINERS | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 48aff80..eb3d889 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -9663,6 +9663,14 @@ F:	include/keys/trusted-type.h
>  F:	include/keys/trusted_tpm.h
>  F:	security/keys/trusted-keys/
>  
> +KEYS-TRUSTED-TEE
> +M:	Sumit Garg <sumit.garg@linaro.org>
> +L:	linux-integrity(a)vger.kernel.org
> +L:	keyrings(a)vger.kernel.org
> +S:	Supported
> +F:	include/keys/trusted_tee.h
> +F:	security/keys/trusted-keys/trusted_tee.c
> +
>  KEYS/KEYRINGS
>  M:	David Howells <dhowells@redhat.com>
>  M:	Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> -- 
> 2.7.4

I'm sorry but I think I have changed my mind on this. This has been
spinning for a while and sometimes conclusions change over the time.

I don't think that we really need a separate subsystem tag. I'd be for a
new M-entry or R-entry to the existing subsystem tag. It's essential to
have ack from someone with ARM and TEE knowledge but this way too heavy
for the purpose.

I also see it the most manageable if the trusted keys PR's come from a
single source.

/Jarkko

Re: [PATCH v7 4/4] MAINTAINERS: Add entry for TEE based Trusted Keys

[Sumit Garg]

[-- Attachment #1: Type: text/plain, Size: 2328 bytes --]

On Tue, 13 Oct 2020 at 07:52, Jarkko Sakkinen
<jarkko.sakkinen@linux.intel.com> wrote:
>
> On Wed, Oct 07, 2020 at 03:37:48PM +0530, Sumit Garg wrote:
> > Add MAINTAINERS entry for TEE based Trusted Keys framework.
> >
> > Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
> > Acked-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > ---
> >  MAINTAINERS | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 48aff80..eb3d889 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -9663,6 +9663,14 @@ F:     include/keys/trusted-type.h
> >  F:   include/keys/trusted_tpm.h
> >  F:   security/keys/trusted-keys/
> >
> > +KEYS-TRUSTED-TEE
> > +M:   Sumit Garg <sumit.garg@linaro.org>
> > +L:   linux-integrity(a)vger.kernel.org
> > +L:   keyrings(a)vger.kernel.org
> > +S:   Supported
> > +F:   include/keys/trusted_tee.h
> > +F:   security/keys/trusted-keys/trusted_tee.c
> > +
> >  KEYS/KEYRINGS
> >  M:   David Howells <dhowells@redhat.com>
> >  M:   Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > --
> > 2.7.4
>
> I'm sorry but I think I have changed my mind on this. This has been
> spinning for a while and sometimes conclusions change over the time.
>
> I don't think that we really need a separate subsystem tag.

I don't see it as a separate subsystem but rather a kind of underlying
trust source (TEE) driver plugged into existing trusted keys
subsystem. We could relate it to the RNG subsystem as well where there
is a subsystem maintainer and specific driver maintainers.

IMO, having a dedicated entry like this brings clarity in maintenance
and in future we may have more trust sources like this added where
everyone may not have access to all the trust sources to test.

> I'd be for a
> new M-entry or R-entry to the existing subsystem tag. It's essential to
> have ack from someone with ARM and TEE knowledge but this way too heavy
> for the purpose.

If you still think otherwise then I am fine with a new M-entry for
existing trusted keys subsystem as well.

>
> I also see it the most manageable if the trusted keys PR's come from a
> single source.

I echo here with you to have a single source for trusted keys PR's
irrespective of whether we go with a separate trust source entry or
update existing subsystem entry.

-Sumit

>
> /Jarkko