[RFC,v2,0/6] Introduce TEE based Trusted Keys support
mbox series

Message ID 1564489420-677-1-git-send-email-sumit.garg@linaro.org
Headers show
Series
  • Introduce TEE based Trusted Keys support
Related show

Message

Sumit Garg July 30, 2019, 12:23 p.m. UTC
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 series also adds some TEE features like:

Patch #1, #2 enables support for registered kernel shared memory with TEE.

Patch #3 enables support for private kernel login method required for
cases like trusted keys where we don't wan't user-space to directly access
TEE service to retrieve trusted key contents.

Rest of the patches from #4 to #6 adds support for TEE based trusted keys.

This patch-set has been tested with OP-TEE based pseudo TA which can be
found here [1].

Also, this patch-set is dependent on generic Trusted Keys framework
patch-set [2].

[1] https://github.com/OP-TEE/optee_os/pull/3082
[2] https://lkml.org/lkml/2019/7/18/284

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 (6):
  tee: optee: allow kernel pages to register as shm
  tee: enable support to register kernel memory
  tee: add private login method for kernel clients
  KEYS: trusted: Introduce TEE based Trusted Keys
  doc: keys: Document usage of TEE based Trusted Keys
  MAINTAINERS: Add entry for TEE based Trusted Keys

 Documentation/security/keys/index.rst       |   1 +
 Documentation/security/keys/tee-trusted.rst |  93 +++++++++
 MAINTAINERS                                 |   9 +
 drivers/tee/optee/call.c                    |   7 +
 drivers/tee/tee_core.c                      |   6 +
 drivers/tee/tee_shm.c                       |  16 +-
 include/keys/trusted-type.h                 |   3 +
 include/keys/trusted_tee.h                  |  66 +++++++
 include/linux/tee_drv.h                     |   1 +
 include/uapi/linux/tee.h                    |   8 +
 security/keys/Kconfig                       |   3 +
 security/keys/trusted-keys/Makefile         |   3 +-
 security/keys/trusted-keys/trusted-tee.c    | 282 ++++++++++++++++++++++++++++
 security/keys/trusted-keys/trusted.c        |   3 +
 14 files changed, 498 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/security/keys/tee-trusted.rst
 create mode 100644 include/keys/trusted_tee.h
 create mode 100644 security/keys/trusted-keys/trusted-tee.c

Comments

Janne Karhunen July 31, 2019, 7:11 a.m. UTC | #1
Hi,

Interesting, I wrote something similar and posted it to the lists a while back:
https://github.com/jkrh/linux/commit/d77ea03afedcb5fd42234cd834da8f8a0809f6a6

Since there are no generic 'TEEs' available, I implemented the same
thing as a generic protocol translator. The shared memory binding for
instance already assumes fair amount about the TEE and how that is
physically present in the system. Besides, the help from usage of shm
is pretty limited due to the size of the keydata.


--
Janne




On Tue, Jul 30, 2019 at 3:26 PM Sumit Garg <sumit.garg@linaro.org> wrote:
>
> 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 series also adds some TEE features like:
>
> Patch #1, #2 enables support for registered kernel shared memory with TEE.
>
> Patch #3 enables support for private kernel login method required for
> cases like trusted keys where we don't wan't user-space to directly access
> TEE service to retrieve trusted key contents.
>
> Rest of the patches from #4 to #6 adds support for TEE based trusted keys.
>
> This patch-set has been tested with OP-TEE based pseudo TA which can be
> found here [1].
>
> Also, this patch-set is dependent on generic Trusted Keys framework
> patch-set [2].
>
> [1] https://github.com/OP-TEE/optee_os/pull/3082
> [2] https://lkml.org/lkml/2019/7/18/284
>
> 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 (6):
>   tee: optee: allow kernel pages to register as shm
>   tee: enable support to register kernel memory
>   tee: add private login method for kernel clients
>   KEYS: trusted: Introduce TEE based Trusted Keys
>   doc: keys: Document usage of TEE based Trusted Keys
>   MAINTAINERS: Add entry for TEE based Trusted Keys
>
>  Documentation/security/keys/index.rst       |   1 +
>  Documentation/security/keys/tee-trusted.rst |  93 +++++++++
>  MAINTAINERS                                 |   9 +
>  drivers/tee/optee/call.c                    |   7 +
>  drivers/tee/tee_core.c                      |   6 +
>  drivers/tee/tee_shm.c                       |  16 +-
>  include/keys/trusted-type.h                 |   3 +
>  include/keys/trusted_tee.h                  |  66 +++++++
>  include/linux/tee_drv.h                     |   1 +
>  include/uapi/linux/tee.h                    |   8 +
>  security/keys/Kconfig                       |   3 +
>  security/keys/trusted-keys/Makefile         |   3 +-
>  security/keys/trusted-keys/trusted-tee.c    | 282 ++++++++++++++++++++++++++++
>  security/keys/trusted-keys/trusted.c        |   3 +
>  14 files changed, 498 insertions(+), 3 deletions(-)
>  create mode 100644 Documentation/security/keys/tee-trusted.rst
>  create mode 100644 include/keys/trusted_tee.h
>  create mode 100644 security/keys/trusted-keys/trusted-tee.c
>
> --
> 2.7.4
>
Janne Karhunen July 31, 2019, 10:21 a.m. UTC | #2
Hi,

To clarify a bit further - my thought was to support any type of trust
source. Remote, local or both. Just having one particular type of
locally bound 'TEE' sounded very limited, especially when nothing from
the TEE execution side is really needed for supporting the kernel
crypto. What you really need is the seal/unseal transaction going
somewhere and where that somewhere is does not matter much. With the
user mode helper in between anyone can easily add their own thing in
there.


--
Janne

On Wed, Jul 31, 2019 at 10:11 AM Janne Karhunen
<janne.karhunen@gmail.com> wrote:
>
> Hi,
>
> Interesting, I wrote something similar and posted it to the lists a while back:
> https://github.com/jkrh/linux/commit/d77ea03afedcb5fd42234cd834da8f8a0809f6a6
>
> Since there are no generic 'TEEs' available, I implemented the same
> thing as a generic protocol translator. The shared memory binding for
> instance already assumes fair amount about the TEE and how that is
> physically present in the system. Besides, the help from usage of shm
> is pretty limited due to the size of the keydata.
>
>
> --
> Janne
>
>
>
>
> On Tue, Jul 30, 2019 at 3:26 PM Sumit Garg <sumit.garg@linaro.org> wrote:
> >
> > 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 series also adds some TEE features like:
> >
> > Patch #1, #2 enables support for registered kernel shared memory with TEE.
> >
> > Patch #3 enables support for private kernel login method required for
> > cases like trusted keys where we don't wan't user-space to directly access
> > TEE service to retrieve trusted key contents.
> >
> > Rest of the patches from #4 to #6 adds support for TEE based trusted keys.
> >
> > This patch-set has been tested with OP-TEE based pseudo TA which can be
> > found here [1].
> >
> > Also, this patch-set is dependent on generic Trusted Keys framework
> > patch-set [2].
> >
> > [1] https://github.com/OP-TEE/optee_os/pull/3082
> > [2] https://lkml.org/lkml/2019/7/18/284
> >
> > 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 (6):
> >   tee: optee: allow kernel pages to register as shm
> >   tee: enable support to register kernel memory
> >   tee: add private login method for kernel clients
> >   KEYS: trusted: Introduce TEE based Trusted Keys
> >   doc: keys: Document usage of TEE based Trusted Keys
> >   MAINTAINERS: Add entry for TEE based Trusted Keys
> >
> >  Documentation/security/keys/index.rst       |   1 +
> >  Documentation/security/keys/tee-trusted.rst |  93 +++++++++
> >  MAINTAINERS                                 |   9 +
> >  drivers/tee/optee/call.c                    |   7 +
> >  drivers/tee/tee_core.c                      |   6 +
> >  drivers/tee/tee_shm.c                       |  16 +-
> >  include/keys/trusted-type.h                 |   3 +
> >  include/keys/trusted_tee.h                  |  66 +++++++
> >  include/linux/tee_drv.h                     |   1 +
> >  include/uapi/linux/tee.h                    |   8 +
> >  security/keys/Kconfig                       |   3 +
> >  security/keys/trusted-keys/Makefile         |   3 +-
> >  security/keys/trusted-keys/trusted-tee.c    | 282 ++++++++++++++++++++++++++++
> >  security/keys/trusted-keys/trusted.c        |   3 +
> >  14 files changed, 498 insertions(+), 3 deletions(-)
> >  create mode 100644 Documentation/security/keys/tee-trusted.rst
> >  create mode 100644 include/keys/trusted_tee.h
> >  create mode 100644 security/keys/trusted-keys/trusted-tee.c
> >
> > --
> > 2.7.4
> >
Sumit Garg July 31, 2019, 10:26 a.m. UTC | #3
Hi Janne,

On Wed, 31 Jul 2019 at 12:41, Janne Karhunen <janne.karhunen@gmail.com> wrote:
>
> Hi,
>
> Interesting, I wrote something similar and posted it to the lists a while back:
> https://github.com/jkrh/linux/commit/d77ea03afedcb5fd42234cd834da8f8a0809f6a6
>
> Since there are no generic 'TEEs' available,

There is already a generic TEE interface driver available in kernel.
Have a look here: "Documentation/tee.txt".

> I implemented the same
> thing as a generic protocol translator. The shared memory binding for
> instance already assumes fair amount about the TEE and how that is
> physically present in the system. Besides, the help from usage of shm
> is pretty limited due to the size of the keydata.
>

If you look at patch #1 and #2, they add support to register kernel
memory buffer (keydata buffer in this case) with TEE to operate on. So
there isn't any limitation due to the size of the keydata.

-Sumit

>
> --
> Janne
>
>
>
>
> On Tue, Jul 30, 2019 at 3:26 PM Sumit Garg <sumit.garg@linaro.org> wrote:
> >
> > 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 series also adds some TEE features like:
> >
> > Patch #1, #2 enables support for registered kernel shared memory with TEE.
> >
> > Patch #3 enables support for private kernel login method required for
> > cases like trusted keys where we don't wan't user-space to directly access
> > TEE service to retrieve trusted key contents.
> >
> > Rest of the patches from #4 to #6 adds support for TEE based trusted keys.
> >
> > This patch-set has been tested with OP-TEE based pseudo TA which can be
> > found here [1].
> >
> > Also, this patch-set is dependent on generic Trusted Keys framework
> > patch-set [2].
> >
> > [1] https://github.com/OP-TEE/optee_os/pull/3082
> > [2] https://lkml.org/lkml/2019/7/18/284
> >
> > 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 (6):
> >   tee: optee: allow kernel pages to register as shm
> >   tee: enable support to register kernel memory
> >   tee: add private login method for kernel clients
> >   KEYS: trusted: Introduce TEE based Trusted Keys
> >   doc: keys: Document usage of TEE based Trusted Keys
> >   MAINTAINERS: Add entry for TEE based Trusted Keys
> >
> >  Documentation/security/keys/index.rst       |   1 +
> >  Documentation/security/keys/tee-trusted.rst |  93 +++++++++
> >  MAINTAINERS                                 |   9 +
> >  drivers/tee/optee/call.c                    |   7 +
> >  drivers/tee/tee_core.c                      |   6 +
> >  drivers/tee/tee_shm.c                       |  16 +-
> >  include/keys/trusted-type.h                 |   3 +
> >  include/keys/trusted_tee.h                  |  66 +++++++
> >  include/linux/tee_drv.h                     |   1 +
> >  include/uapi/linux/tee.h                    |   8 +
> >  security/keys/Kconfig                       |   3 +
> >  security/keys/trusted-keys/Makefile         |   3 +-
> >  security/keys/trusted-keys/trusted-tee.c    | 282 ++++++++++++++++++++++++++++
> >  security/keys/trusted-keys/trusted.c        |   3 +
> >  14 files changed, 498 insertions(+), 3 deletions(-)
> >  create mode 100644 Documentation/security/keys/tee-trusted.rst
> >  create mode 100644 include/keys/trusted_tee.h
> >  create mode 100644 security/keys/trusted-keys/trusted-tee.c
> >
> > --
> > 2.7.4
> >
Janne Karhunen July 31, 2019, 11:02 a.m. UTC | #4
On Wed, Jul 31, 2019 at 1:26 PM Sumit Garg <sumit.garg@linaro.org> wrote:

> > Interesting, I wrote something similar and posted it to the lists a while back:
> > https://github.com/jkrh/linux/commit/d77ea03afedcb5fd42234cd834da8f8a0809f6a6
> >
> > Since there are no generic 'TEEs' available,
>
> There is already a generic TEE interface driver available in kernel.
> Have a look here: "Documentation/tee.txt".

I guess my wording was wrong, tried to say that physical TEEs in the
wild vary massively hardware wise. Generalizing these things is rough.


> > I implemented the same
> > thing as a generic protocol translator. The shared memory binding for
> > instance already assumes fair amount about the TEE and how that is
> > physically present in the system. Besides, the help from usage of shm
> > is pretty limited due to the size of the keydata.
> >
>
> If you look at patch #1 and #2, they add support to register kernel
> memory buffer (keydata buffer in this case) with TEE to operate on. So
> there isn't any limitation due to the size of the keydata.

Ah, didn't mean that. Meant that the keydata is typically pretty small
in size, so there is limited benefit from passing that in via shm if
that complicates anything.
Sumit Garg July 31, 2019, 1:58 p.m. UTC | #5
On Wed, 31 Jul 2019 at 15:51, Janne Karhunen <janne.karhunen@gmail.com> wrote:
>
> Hi,
>
> To clarify a bit further - my thought was to support any type of trust
> source.

That could be very well accomplished via Trusted Keys abstraction
framework [1]. A trust source just need to implement following APIs:

struct trusted_key_ops ts_trusted_key_ops = {
       .migratable = 0, /* non-migratable */
       .init = init_ts_trusted,
       .seal = ts_key_seal,
       .unseal = ts_key_unseal,
       .get_random = ts_get_random,
       .cleanup = cleanup_ts_trusted,
};

> Remote, local or both. Just having one particular type of
> locally bound 'TEE' sounded very limited,

TEE is just one of trust source like TPM, we can have other trust
source as mentioned above.

> especially when nothing from
> the TEE execution side is really needed for supporting the kernel
> crypto. What you really need is the seal/unseal transaction going
> somewhere and where that somewhere is does not matter much.

Its only the seal/unseal operations that are provided by TEE driver
that hooks up under trusted keys abstraction layer.

> With the
> user mode helper in between anyone can easily add their own thing in
> there.
>

Isn't actual purpose to have trusted keys is to protect user-space
from access to kernel keys in plain format? Doesn't user mode helper
defeat that purpose in one way or another?

>

[1] https://lkml.org/lkml/2019/7/18/284

-Sumit

> --
> Janne
>
> On Wed, Jul 31, 2019 at 10:11 AM Janne Karhunen
> <janne.karhunen@gmail.com> wrote:
> >
> > Hi,
> >
> > Interesting, I wrote something similar and posted it to the lists a while back:
> > https://github.com/jkrh/linux/commit/d77ea03afedcb5fd42234cd834da8f8a0809f6a6
> >
> > Since there are no generic 'TEEs' available, I implemented the same
> > thing as a generic protocol translator. The shared memory binding for
> > instance already assumes fair amount about the TEE and how that is
> > physically present in the system. Besides, the help from usage of shm
> > is pretty limited due to the size of the keydata.
> >
> >
> > --
> > Janne
> >
> >
> >
> >
> > On Tue, Jul 30, 2019 at 3:26 PM Sumit Garg <sumit.garg@linaro.org> wrote:
> > >
> > > 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 series also adds some TEE features like:
> > >
> > > Patch #1, #2 enables support for registered kernel shared memory with TEE.
> > >
> > > Patch #3 enables support for private kernel login method required for
> > > cases like trusted keys where we don't wan't user-space to directly access
> > > TEE service to retrieve trusted key contents.
> > >
> > > Rest of the patches from #4 to #6 adds support for TEE based trusted keys.
> > >
> > > This patch-set has been tested with OP-TEE based pseudo TA which can be
> > > found here [1].
> > >
> > > Also, this patch-set is dependent on generic Trusted Keys framework
> > > patch-set [2].
> > >
> > > [1] https://github.com/OP-TEE/optee_os/pull/3082
> > > [2] https://lkml.org/lkml/2019/7/18/284
> > >
> > > 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 (6):
> > >   tee: optee: allow kernel pages to register as shm
> > >   tee: enable support to register kernel memory
> > >   tee: add private login method for kernel clients
> > >   KEYS: trusted: Introduce TEE based Trusted Keys
> > >   doc: keys: Document usage of TEE based Trusted Keys
> > >   MAINTAINERS: Add entry for TEE based Trusted Keys
> > >
> > >  Documentation/security/keys/index.rst       |   1 +
> > >  Documentation/security/keys/tee-trusted.rst |  93 +++++++++
> > >  MAINTAINERS                                 |   9 +
> > >  drivers/tee/optee/call.c                    |   7 +
> > >  drivers/tee/tee_core.c                      |   6 +
> > >  drivers/tee/tee_shm.c                       |  16 +-
> > >  include/keys/trusted-type.h                 |   3 +
> > >  include/keys/trusted_tee.h                  |  66 +++++++
> > >  include/linux/tee_drv.h                     |   1 +
> > >  include/uapi/linux/tee.h                    |   8 +
> > >  security/keys/Kconfig                       |   3 +
> > >  security/keys/trusted-keys/Makefile         |   3 +-
> > >  security/keys/trusted-keys/trusted-tee.c    | 282 ++++++++++++++++++++++++++++
> > >  security/keys/trusted-keys/trusted.c        |   3 +
> > >  14 files changed, 498 insertions(+), 3 deletions(-)
> > >  create mode 100644 Documentation/security/keys/tee-trusted.rst
> > >  create mode 100644 include/keys/trusted_tee.h
> > >  create mode 100644 security/keys/trusted-keys/trusted-tee.c
> > >
> > > --
> > > 2.7.4
> > >
Sumit Garg July 31, 2019, 2:23 p.m. UTC | #6
On Wed, 31 Jul 2019 at 16:33, Janne Karhunen <janne.karhunen@gmail.com> wrote:
>
> On Wed, Jul 31, 2019 at 1:26 PM Sumit Garg <sumit.garg@linaro.org> wrote:
>
> > > Interesting, I wrote something similar and posted it to the lists a while back:
> > > https://github.com/jkrh/linux/commit/d77ea03afedcb5fd42234cd834da8f8a0809f6a6
> > >
> > > Since there are no generic 'TEEs' available,
> >
> > There is already a generic TEE interface driver available in kernel.
> > Have a look here: "Documentation/tee.txt".
>
> I guess my wording was wrong, tried to say that physical TEEs in the
> wild vary massively hardware wise. Generalizing these things is rough.
>

There are already well defined GlobalPlatform Standards to generalize
the TEE interface. One of them is GlobalPlatform TEE Client API [1]
which provides the basis for this TEE interface.

>
> > > I implemented the same
> > > thing as a generic protocol translator. The shared memory binding for
> > > instance already assumes fair amount about the TEE and how that is
> > > physically present in the system. Besides, the help from usage of shm
> > > is pretty limited due to the size of the keydata.
> > >
> >
> > If you look at patch #1 and #2, they add support to register kernel
> > memory buffer (keydata buffer in this case) with TEE to operate on. So
> > there isn't any limitation due to the size of the keydata.
>
> Ah, didn't mean that. Meant that the keydata is typically pretty small
> in size, so there is limited benefit from passing that in via shm if
> that complicates anything.
>

Ah, ok. Do you think of any better approach rather than to use SHM?

[1] https://globalplatform.org/specs-library/tee-client-api-specification/

-Sumit

>
> --
> Janne
Janne Karhunen Aug. 1, 2019, 6:21 a.m. UTC | #7
On Wed, Jul 31, 2019 at 4:58 PM Sumit Garg <sumit.garg@linaro.org> wrote:

> > To clarify a bit further - my thought was to support any type of trust
> > source.
>
> That could be very well accomplished via Trusted Keys abstraction
> framework [1]. A trust source just need to implement following APIs:
>
> struct trusted_key_ops ts_trusted_key_ops = {
>        .migratable = 0, /* non-migratable */
>        .init = init_ts_trusted,
>        .seal = ts_key_seal,
>        .unseal = ts_key_unseal,
>        .get_random = ts_get_random,
>        .cleanup = cleanup_ts_trusted,
> };

Which is basically the same as implementing a new keytype in the
kernel; abstraction is not raised in any considerable manner this way?

I chose the userspace plugin due to this, you can use userspace aids
to provide any type of service. Use the crypto library you desire to
do the magic you want.


> > With the
> > user mode helper in between anyone can easily add their own thing in
> > there.
>
> Isn't actual purpose to have trusted keys is to protect user-space
> from access to kernel keys in plain format? Doesn't user mode helper
> defeat that purpose in one way or another?

Not really. CPU is in the user mode while running the code, but the
code or the secure keydata being is not available to the 'normal'
userspace. It's like microkernel service/driver this way. The usermode
driver is part of the kernel image and it runs on top of a invisible
rootfs.


--
Janne
Janne Karhunen Aug. 1, 2019, 6:36 a.m. UTC | #8
On Wed, Jul 31, 2019 at 5:23 PM Sumit Garg <sumit.garg@linaro.org> wrote:

> > I guess my wording was wrong, tried to say that physical TEEs in the
> > wild vary massively hardware wise. Generalizing these things is rough.
> >
>
> There are already well defined GlobalPlatform Standards to generalize
> the TEE interface. One of them is GlobalPlatform TEE Client API [1]
> which provides the basis for this TEE interface.

I'm aware of it - I have implemented a large part of the GP TEE APIs
earlier (primarily the crypto functions). Does the TEE you work with
actually support GP properly? Can I take a look at the code?

Normally the TEE implementations are well-guarded secrets and the
state of the implementation is quite random. In many cases keeping
things secret is fine from my point of view, given that it is a RoT
after all. The secrecy is the core business here. So, this is why I
opted the userspace 'secret' route - no secrets in the kernel, but
it's fine for the userspace. Umh was a logical fit to implement it.


--
Janne
Rouven Czerwinski Aug. 1, 2019, 6:50 a.m. UTC | #9
On Thu, 2019-08-01 at 09:36 +0300, Janne Karhunen wrote:
> On Wed, Jul 31, 2019 at 5:23 PM Sumit Garg <sumit.garg@linaro.org>
> wrote:
> 
> > > I guess my wording was wrong, tried to say that physical TEEs in
> > > the
> > > wild vary massively hardware wise. Generalizing these things is
> > > rough.
> > > 
> > 
> > There are already well defined GlobalPlatform Standards to
> > generalize
> > the TEE interface. One of them is GlobalPlatform TEE Client API [1]
> > which provides the basis for this TEE interface.
> 
> I'm aware of it - I have implemented a large part of the GP TEE APIs
> earlier (primarily the crypto functions). Does the TEE you work with
> actually support GP properly? Can I take a look at the code?

AFAIK Sumit is working with the OP-TEE implementation, which can be
found on github: https://github.com/op-tee/optee_os

Regards,
Rouven
Janne Karhunen Aug. 1, 2019, 7:30 a.m. UTC | #10
On Thu, Aug 1, 2019 at 9:50 AM Rouven Czerwinski
<r.czerwinski@pengutronix.de> wrote:

> > I'm aware of it - I have implemented a large part of the GP TEE APIs
> > earlier (primarily the crypto functions). Does the TEE you work with
> > actually support GP properly? Can I take a look at the code?
>
> AFAIK Sumit is working with the OP-TEE implementation, which can be
> found on github: https://github.com/op-tee/optee_os

Thanks, I will take a look. The fundamental problem with these things
is that there are infinite amount of ways how TEEs and ROTs can be
done in terms of the hardware and software. I really doubt there are 2
implementations in existence that are even remotely compatible in real
life. As such, all things TEE/ROT would logically really belong in the
userland and thanks to the bpfilter folks now the umh logic really
makes that possible ... I think. The key implementation I did was just
an RFC on the concept, what if we start to move the stuff that really
belongs in the userspace to this pseudo-userland. It's not kernel, but
it's not commonly accessible userland either. The shared memory would
also work without any modifications between the umh based TEE/ROT
driver and the userland if needed.

Anyway, just my .02c. I guess having any new support in the kernel for
new trust sources is good and improvement from the current state. I
can certainly make my stuff work with your setup as well, what ever
people think is the best.


--
Janne
Sumit Garg Aug. 1, 2019, 7:40 a.m. UTC | #11
On Thu, 1 Aug 2019 at 11:51, Janne Karhunen <janne.karhunen@gmail.com> wrote:
>
> On Wed, Jul 31, 2019 at 4:58 PM Sumit Garg <sumit.garg@linaro.org> wrote:
>
> > > To clarify a bit further - my thought was to support any type of trust
> > > source.
> >
> > That could be very well accomplished via Trusted Keys abstraction
> > framework [1]. A trust source just need to implement following APIs:
> >
> > struct trusted_key_ops ts_trusted_key_ops = {
> >        .migratable = 0, /* non-migratable */
> >        .init = init_ts_trusted,
> >        .seal = ts_key_seal,
> >        .unseal = ts_key_unseal,
> >        .get_random = ts_get_random,
> >        .cleanup = cleanup_ts_trusted,
> > };
>
> Which is basically the same as implementing a new keytype in the
> kernel; abstraction is not raised in any considerable manner this way?
>

It doesn't create a new keytype. There is only single keytype:
"trusted" which could be implemented via one of the trust source
available in the system like TPM, TEE etc.

> I chose the userspace plugin due to this, you can use userspace aids
> to provide any type of service. Use the crypto library you desire to
> do the magic you want.

Here TEE isn't similar to a user-space crypto library. In our case TEE
is based on ARM TrustZone which only allows TEE communications to be
initiated from privileged mode. So why would you like to route
communications via user-mode (which is less secure) when we have
standardised TEE interface available in kernel?

>
>
> > > With the
> > > user mode helper in between anyone can easily add their own thing in
> > > there.
> >
> > Isn't actual purpose to have trusted keys is to protect user-space
> > from access to kernel keys in plain format? Doesn't user mode helper
> > defeat that purpose in one way or another?
>
> Not really. CPU is in the user mode while running the code, but the
> code or the secure keydata being is not available to the 'normal'
> userspace. It's like microkernel service/driver this way. The usermode
> driver is part of the kernel image and it runs on top of a invisible
> rootfs.
>

Can you elaborate here with an example regarding how this user-mode
helper will securely communicate with a hardware based trust source
with other user-space processes denied access to that trust source?

-Sumit

>
> --
> Janne
Sumit Garg Aug. 1, 2019, 7:58 a.m. UTC | #12
On Thu, 1 Aug 2019 at 13:00, Janne Karhunen <janne.karhunen@gmail.com> wrote:
>
> On Thu, Aug 1, 2019 at 9:50 AM Rouven Czerwinski
> <r.czerwinski@pengutronix.de> wrote:
>
> > > I'm aware of it - I have implemented a large part of the GP TEE APIs
> > > earlier (primarily the crypto functions). Does the TEE you work with
> > > actually support GP properly? Can I take a look at the code?
> >
> > AFAIK Sumit is working with the OP-TEE implementation, which can be
> > found on github: https://github.com/op-tee/optee_os
>
> Thanks, I will take a look.

For documentation, refer to: https://optee.readthedocs.io/

> The fundamental problem with these things
> is that there are infinite amount of ways how TEEs and ROTs can be
> done in terms of the hardware and software. I really doubt there are 2
> implementations in existence that are even remotely compatible in real
> life.

I agree with you regarding implementation specific nature of TEE but
having a standardized client interface does solves the problem.

> As such, all things TEE/ROT would logically really belong in the
> userland and thanks to the bpfilter folks now the umh logic really
> makes that possible ... I think. The key implementation I did was just
> an RFC on the concept, what if we start to move the stuff that really
> belongs in the userspace to this pseudo-userland. It's not kernel, but
> it's not commonly accessible userland either. The shared memory would
> also work without any modifications between the umh based TEE/ROT
> driver and the userland if needed.
>
> Anyway, just my .02c. I guess having any new support in the kernel for
> new trust sources is good and improvement from the current state. I
> can certainly make my stuff work with your setup as well, what ever
> people think is the best.

Yes your implementation can very well fit under trusted keys
abstraction framework without creating a new keytype: "ext-trusted".

-Sumit

>
>
> --
> Janne
Janne Karhunen Aug. 1, 2019, 7:59 a.m. UTC | #13
On Thu, Aug 1, 2019 at 10:40 AM Sumit Garg <sumit.garg@linaro.org> wrote:

> > I chose the userspace plugin due to this, you can use userspace aids
> > to provide any type of service. Use the crypto library you desire to
> > do the magic you want.
>
> Here TEE isn't similar to a user-space crypto library. In our case TEE
> is based on ARM TrustZone which only allows TEE communications to be
> initiated from privileged mode. So why would you like to route
> communications via user-mode (which is less secure) when we have
> standardised TEE interface available in kernel?

The physical access guards for reading/writing the involved critical
memory are identical as far as I know? Layered security is generally a
good thing, and the userspace pass actually adds a layer, so not sure
which is really safer?

In my case the rerouting was to done generalize it. Any type of trust
source, anywhere.


> > > Isn't actual purpose to have trusted keys is to protect user-space
> > > from access to kernel keys in plain format? Doesn't user mode helper
> > > defeat that purpose in one way or another?
> >
> > Not really. CPU is in the user mode while running the code, but the
> > code or the secure keydata being is not available to the 'normal'
> > userspace. It's like microkernel service/driver this way. The usermode
> > driver is part of the kernel image and it runs on top of a invisible
> > rootfs.
>
> Can you elaborate here with an example regarding how this user-mode
> helper will securely communicate with a hardware based trust source
> with other user-space processes denied access to that trust source?

The other user mode processes will never see the device node to open.
There is none in existence for them; it only exists in the ramfs based
root for the user mode helper.


--
Janne
Janne Karhunen Aug. 1, 2019, 8:30 a.m. UTC | #14
On Thu, Aug 1, 2019 at 10:58 AM Sumit Garg <sumit.garg@linaro.org> wrote:

> > Anyway, just my .02c. I guess having any new support in the kernel for
> > new trust sources is good and improvement from the current state. I
> > can certainly make my stuff work with your setup as well, what ever
> > people think is the best.
>
> Yes your implementation can very well fit under trusted keys
> abstraction framework without creating a new keytype: "ext-trusted".

The fundamental problem with the 'standardized kernel tee' still
exists - it will never be generic in real life. Getting all this in
the kernel will solve your problem and sell this particular product,
but it is quite unlikely to help that many users. If the security is
truly important to you, would you really trust any of this code to
someone else? In this day and age, I really doubt many do. Everyone
does their own thing, so this is why I really see all that as a
userspace problem.


--
Janne
Sumit Garg Aug. 1, 2019, 10 a.m. UTC | #15
On Thu, 1 Aug 2019 at 13:30, Janne Karhunen <janne.karhunen@gmail.com> wrote:
>
> On Thu, Aug 1, 2019 at 10:40 AM Sumit Garg <sumit.garg@linaro.org> wrote:
>
> > > I chose the userspace plugin due to this, you can use userspace aids
> > > to provide any type of service. Use the crypto library you desire to
> > > do the magic you want.
> >
> > Here TEE isn't similar to a user-space crypto library. In our case TEE
> > is based on ARM TrustZone which only allows TEE communications to be
> > initiated from privileged mode. So why would you like to route
> > communications via user-mode (which is less secure) when we have
> > standardised TEE interface available in kernel?
>
> The physical access guards for reading/writing the involved critical
> memory are identical as far as I know? Layered security is generally a
> good thing, and the userspace pass actually adds a layer, so not sure
> which is really safer?
>

AFAIK, layered security is better in case we move from lower privilege
level to higher privilege level rather than in reverse order.

-Sumit

> In my case the rerouting was to done generalize it. Any type of trust
> source, anywhere.
>
>
> > > > Isn't actual purpose to have trusted keys is to protect user-space
> > > > from access to kernel keys in plain format? Doesn't user mode helper
> > > > defeat that purpose in one way or another?
> > >
> > > Not really. CPU is in the user mode while running the code, but the
> > > code or the secure keydata being is not available to the 'normal'
> > > userspace. It's like microkernel service/driver this way. The usermode
> > > driver is part of the kernel image and it runs on top of a invisible
> > > rootfs.
> >
> > Can you elaborate here with an example regarding how this user-mode
> > helper will securely communicate with a hardware based trust source
> > with other user-space processes denied access to that trust source?
>
> The other user mode processes will never see the device node to open.
> There is none in existence for them; it only exists in the ramfs based
> root for the user mode helper.
>
>
> --
> Janne
Sumit Garg Aug. 1, 2019, 10:27 a.m. UTC | #16
On Thu, 1 Aug 2019 at 14:00, Janne Karhunen <janne.karhunen@gmail.com> wrote:
>
> On Thu, Aug 1, 2019 at 10:58 AM Sumit Garg <sumit.garg@linaro.org> wrote:
>
> > > Anyway, just my .02c. I guess having any new support in the kernel for
> > > new trust sources is good and improvement from the current state. I
> > > can certainly make my stuff work with your setup as well, what ever
> > > people think is the best.
> >
> > Yes your implementation can very well fit under trusted keys
> > abstraction framework without creating a new keytype: "ext-trusted".
>
> The fundamental problem with the 'standardized kernel tee' still
> exists - it will never be generic in real life. Getting all this in
> the kernel will solve your problem and sell this particular product,
> but it is quite unlikely to help that many users. If the security is
> truly important to you, would you really trust any of this code to
> someone else? In this day and age, I really doubt many do.

There are already multiple platforms supported by OP-TEE [1] which
could benefit from this trusted keys interface.

> Everyone
> does their own thing, so this is why I really see all that as a
> userspace problem.
>

IMO, we should try to use standardized interfaces which are well
thought off rather than implementing your own.

[1] https://optee.readthedocs.io/general/platforms.html


-Sumit

>
> --
> Janne
Janne Karhunen Aug. 1, 2019, 10:40 a.m. UTC | #17
On Thu, Aug 1, 2019 at 1:00 PM Sumit Garg <sumit.garg@linaro.org> wrote:

> > > Here TEE isn't similar to a user-space crypto library. In our case TEE
> > > is based on ARM TrustZone which only allows TEE communications to be
> > > initiated from privileged mode. So why would you like to route
> > > communications via user-mode (which is less secure) when we have
> > > standardised TEE interface available in kernel?
> >
> > The physical access guards for reading/writing the involved critical
> > memory are identical as far as I know? Layered security is generally a
> > good thing, and the userspace pass actually adds a layer, so not sure
> > which is really safer?
>
> AFAIK, layered security is better in case we move from lower privilege
> level to higher privilege level rather than in reverse order.

You can look at this in many ways. Another way to look at it is that
the services should be provided with the least amount of permissions
required for the task. Further you can containerize something, the
better.

As for your PLATFORMS support: it is all nice, but there is no way to
convince op-tee or any other tee to be adopted by many real users.
Every serious user can and will do their own thing, or at very best,
buy it from someone who did their own thing and is trusted. There is
zero chance that samsung, huawei, apple, nsa, google, rambus, payment
system vendors, .. would actually share the tee (or probably even the
interfaces). It is just too vital and people do not trust each other
anymore :(

Anyway, enough about the topic from my side. I guess people will tell
what they want, I'm fine with any, and it is all progress from the
current state :)


--
Janne
Jarkko Sakkinen Aug. 4, 2019, 8:48 p.m. UTC | #18
On Tue, Jul 30, 2019 at 05:53:34PM +0530, Sumit Garg wrote:
>   tee: optee: allow kernel pages to register as shm
>   tee: enable support to register kernel memory
>   tee: add private login method for kernel clients
>   KEYS: trusted: Introduce TEE based Trusted Keys
>   doc: keys: Document usage of TEE based Trusted Keys
>   MAINTAINERS: Add entry for TEE based Trusted Keys

Skimmed through the patches. I think it is better to sort out the
current LKM dependency issue with trusted.ko and get TPM 1.2 and TPM 2.0
trusted keys code consolidated before it makes sense to really go detail
on this.

/Jarkko