[v8,0/2] fTPM: firmware TPM running in TEE
mbox series

Message ID 20190705204746.27543-1-sashal@kernel.org
Headers show
Series
  • fTPM: firmware TPM running in TEE
Related show

Message

Sasha Levin July 5, 2019, 8:47 p.m. UTC
Changes from v7:

 - Address Jarkko's comments.

Sasha Levin (2):
  fTPM: firmware TPM running in TEE
  fTPM: add documentation for ftpm driver

 Documentation/security/tpm/index.rst        |   1 +
 Documentation/security/tpm/tpm_ftpm_tee.rst |  27 ++
 drivers/char/tpm/Kconfig                    |   5 +
 drivers/char/tpm/Makefile                   |   1 +
 drivers/char/tpm/tpm_ftpm_tee.c             | 350 ++++++++++++++++++++
 drivers/char/tpm/tpm_ftpm_tee.h             |  40 +++
 6 files changed, 424 insertions(+)
 create mode 100644 Documentation/security/tpm/tpm_ftpm_tee.rst
 create mode 100644 drivers/char/tpm/tpm_ftpm_tee.c
 create mode 100644 drivers/char/tpm/tpm_ftpm_tee.h

Comments

Jarkko Sakkinen July 11, 2019, 8:08 p.m. UTC | #1
On Fri, Jul 05, 2019 at 04:47:44PM -0400, Sasha Levin wrote:
> Changes from v7:
> 
>  - Address Jarkko's comments.
> 
> Sasha Levin (2):
>   fTPM: firmware TPM running in TEE
>   fTPM: add documentation for ftpm driver
> 
>  Documentation/security/tpm/index.rst        |   1 +
>  Documentation/security/tpm/tpm_ftpm_tee.rst |  27 ++
>  drivers/char/tpm/Kconfig                    |   5 +
>  drivers/char/tpm/Makefile                   |   1 +
>  drivers/char/tpm/tpm_ftpm_tee.c             | 350 ++++++++++++++++++++
>  drivers/char/tpm/tpm_ftpm_tee.h             |  40 +++
>  6 files changed, 424 insertions(+)
>  create mode 100644 Documentation/security/tpm/tpm_ftpm_tee.rst
>  create mode 100644 drivers/char/tpm/tpm_ftpm_tee.c
>  create mode 100644 drivers/char/tpm/tpm_ftpm_tee.h
> 
> -- 
> 2.20.1
> 

I applied the patches now. Appreciate a lot the patience with these.
Thank you.

/Jarkko
Ilias Apalodimas July 11, 2019, 8:10 p.m. UTC | #2
On Thu, Jul 11, 2019 at 11:08:58PM +0300, Jarkko Sakkinen wrote:
> On Fri, Jul 05, 2019 at 04:47:44PM -0400, Sasha Levin wrote:
> > Changes from v7:
> > 
> >  - Address Jarkko's comments.
> > 
> > Sasha Levin (2):
> >   fTPM: firmware TPM running in TEE
> >   fTPM: add documentation for ftpm driver
> > 
> >  Documentation/security/tpm/index.rst        |   1 +
> >  Documentation/security/tpm/tpm_ftpm_tee.rst |  27 ++
> >  drivers/char/tpm/Kconfig                    |   5 +
> >  drivers/char/tpm/Makefile                   |   1 +
> >  drivers/char/tpm/tpm_ftpm_tee.c             | 350 ++++++++++++++++++++
> >  drivers/char/tpm/tpm_ftpm_tee.h             |  40 +++
> >  6 files changed, 424 insertions(+)
> >  create mode 100644 Documentation/security/tpm/tpm_ftpm_tee.rst
> >  create mode 100644 drivers/char/tpm/tpm_ftpm_tee.c
> >  create mode 100644 drivers/char/tpm/tpm_ftpm_tee.h
> > 
> > -- 
> > 2.20.1
> > 
> 
> I applied the patches now. Appreciate a lot the patience with these.
> Thank you.
> 

Will report back any issues when we start using it on real hardware
rather than QEMU

Thanks
/Ilias
> /Jarkko
Sasha Levin July 11, 2019, 8:35 p.m. UTC | #3
On Thu, Jul 11, 2019 at 11:10:59PM +0300, Ilias Apalodimas wrote:
>On Thu, Jul 11, 2019 at 11:08:58PM +0300, Jarkko Sakkinen wrote:
>> On Fri, Jul 05, 2019 at 04:47:44PM -0400, Sasha Levin wrote:
>> > Changes from v7:
>> >
>> >  - Address Jarkko's comments.
>> >
>> > Sasha Levin (2):
>> >   fTPM: firmware TPM running in TEE
>> >   fTPM: add documentation for ftpm driver
>> >
>> >  Documentation/security/tpm/index.rst        |   1 +
>> >  Documentation/security/tpm/tpm_ftpm_tee.rst |  27 ++
>> >  drivers/char/tpm/Kconfig                    |   5 +
>> >  drivers/char/tpm/Makefile                   |   1 +
>> >  drivers/char/tpm/tpm_ftpm_tee.c             | 350 ++++++++++++++++++++
>> >  drivers/char/tpm/tpm_ftpm_tee.h             |  40 +++
>> >  6 files changed, 424 insertions(+)
>> >  create mode 100644 Documentation/security/tpm/tpm_ftpm_tee.rst
>> >  create mode 100644 drivers/char/tpm/tpm_ftpm_tee.c
>> >  create mode 100644 drivers/char/tpm/tpm_ftpm_tee.h
>> >
>> > --
>> > 2.20.1
>> >
>>
>> I applied the patches now. Appreciate a lot the patience with these.
>> Thank you.

Thanks Jarkko!

>Will report back any issues when we start using it on real hardware
>rather than QEMU

And thank you Ilias, let us know if we can help with the setup.

--
Thanks,
Sasha
Jarkko Sakkinen July 12, 2019, 3:37 a.m. UTC | #4
On Thu, Jul 11, 2019 at 11:10:59PM +0300, Ilias Apalodimas wrote:
> Will report back any issues when we start using it on real hardware
> rather than QEMU
> 
> Thanks
> /Ilias

That would awesome. PR is far away so there is time to add more
tested-by's. Thanks.

/Jarkko
Ilias Apalodimas July 15, 2019, 9:05 a.m. UTC | #5
On Fri, Jul 12, 2019 at 06:37:58AM +0300, Jarkko Sakkinen wrote:
> On Thu, Jul 11, 2019 at 11:10:59PM +0300, Ilias Apalodimas wrote:
> > Will report back any issues when we start using it on real hardware
> > rather than QEMU
> > 
> > Thanks
> > /Ilias
> 
> That would awesome. PR is far away so there is time to add more
> tested-by's. Thanks.
> 

I tested the basic fucntionality on QEMU and with the code only built as a
module. You can add my tested-by on this if you want

> /Jarkko
Jarkko Sakkinen Aug. 1, 2019, 4:35 p.m. UTC | #6
On Mon, Jul 15, 2019 at 12:05:25PM +0300, Ilias Apalodimas wrote:
> On Fri, Jul 12, 2019 at 06:37:58AM +0300, Jarkko Sakkinen wrote:
> > On Thu, Jul 11, 2019 at 11:10:59PM +0300, Ilias Apalodimas wrote:
> > > Will report back any issues when we start using it on real hardware
> > > rather than QEMU
> > > 
> > > Thanks
> > > /Ilias
> > 
> > That would awesome. PR is far away so there is time to add more
> > tested-by's. Thanks.
> > 
> 
> I tested the basic fucntionality on QEMU and with the code only built as a
> module. You can add my tested-by on this if you want

Thank you. Added.

/Jarkko
Jarkko Sakkinen Aug. 4, 2019, 9:44 p.m. UTC | #7
On Thu, Jul 11, 2019 at 11:08:58PM +0300, Jarkko Sakkinen wrote:
> On Fri, Jul 05, 2019 at 04:47:44PM -0400, Sasha Levin wrote:
> > Changes from v7:
> > 
> >  - Address Jarkko's comments.
> > 
> > Sasha Levin (2):
> >   fTPM: firmware TPM running in TEE
> >   fTPM: add documentation for ftpm driver
> > 
> >  Documentation/security/tpm/index.rst        |   1 +
> >  Documentation/security/tpm/tpm_ftpm_tee.rst |  27 ++
> >  drivers/char/tpm/Kconfig                    |   5 +
> >  drivers/char/tpm/Makefile                   |   1 +
> >  drivers/char/tpm/tpm_ftpm_tee.c             | 350 ++++++++++++++++++++
> >  drivers/char/tpm/tpm_ftpm_tee.h             |  40 +++
> >  6 files changed, 424 insertions(+)
> >  create mode 100644 Documentation/security/tpm/tpm_ftpm_tee.rst
> >  create mode 100644 drivers/char/tpm/tpm_ftpm_tee.c
> >  create mode 100644 drivers/char/tpm/tpm_ftpm_tee.h
> > 
> > -- 
> > 2.20.1
> > 
> 
> I applied the patches now. Appreciate a lot the patience with these.
> Thank you.

Hi, can you possibly fix these:

005-tpm-tpm_ftpm_tee-A-driver-for-firmware-TPM-running-i.patch
---------------------------------------------------------------
WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#10:
https://www.microsoft.com/en-us/research/publication/ftpm-software-implementation-tpm-chip/ .

WARNING: Non-standard signature: Co-authored-by:
#18:
Co-authored-by: Sasha Levin <sashal@kernel.org>

WARNING: prefer 'help' over '---help---' for new help texts
#39: FILE: drivers/char/tpm/Kconfig:167:
+config TCG_FTPM_TEE

WARNING: please write a paragraph that describes the config symbol fully
#39: FILE: drivers/char/tpm/Kconfig:167:
+config TCG_FTPM_TEE

WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#57:
new file mode 100644

WARNING: please, no space before tabs
#102: FILE: drivers/char/tpm/tpm_ftpm_tee.c:41:
+ * ^IIn case of success the number of bytes received.$

WARNING: please, no space before tabs
#131: FILE: drivers/char/tpm/tpm_ftpm_tee.c:70:
+ * ^IIn case of success, returns 0.$

WARNING: please, no space before tabs
#276: FILE: drivers/char/tpm/tpm_ftpm_tee.c:215:
+ * ^IOn success, 0. On failure, -errno.$

WARNING: please, no space before tabs
#366: FILE: drivers/char/tpm/tpm_ftpm_tee.c:305:
+ * ^I0 always.$

ERROR: code indent should use tabs where possible
#387: FILE: drivers/char/tpm/tpm_ftpm_tee.c:326:
+        /* memory allocated with devm_kzalloc() is freed automatically */$

WARNING: DT compatible string "microsoft,ftpm" appears un-documented -- check ./Documentation/devicetree/bindings/
#393: FILE: drivers/char/tpm/tpm_ftpm_tee.c:332:
+	{ .compatible = "microsoft,ftpm" },

WARNING: DT compatible string vendor "microsoft" appears un-documented -- check ./Documentation/devicetree/bindings/vendor-prefixes.yaml
#393: FILE: drivers/char/tpm/tpm_ftpm_tee.c:332:
+	{ .compatible = "microsoft,ftpm" },

total: 1 errors, 11 warnings, 405 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

NOTE: Whitespace errors detected.
      You may wish to use scripts/cleanpatch or scripts/cleanfile

I temporarily dropped the patches but can apply them once the issues
are fixed.

/Jarkko
Sasha Levin Aug. 5, 2019, 6:05 p.m. UTC | #8
On Mon, Aug 05, 2019 at 12:44:28AM +0300, Jarkko Sakkinen wrote:
>On Thu, Jul 11, 2019 at 11:08:58PM +0300, Jarkko Sakkinen wrote:
>> On Fri, Jul 05, 2019 at 04:47:44PM -0400, Sasha Levin wrote:
>> > Changes from v7:
>> >
>> >  - Address Jarkko's comments.
>> >
>> > Sasha Levin (2):
>> >   fTPM: firmware TPM running in TEE
>> >   fTPM: add documentation for ftpm driver
>> >
>> >  Documentation/security/tpm/index.rst        |   1 +
>> >  Documentation/security/tpm/tpm_ftpm_tee.rst |  27 ++
>> >  drivers/char/tpm/Kconfig                    |   5 +
>> >  drivers/char/tpm/Makefile                   |   1 +
>> >  drivers/char/tpm/tpm_ftpm_tee.c             | 350 ++++++++++++++++++++
>> >  drivers/char/tpm/tpm_ftpm_tee.h             |  40 +++
>> >  6 files changed, 424 insertions(+)
>> >  create mode 100644 Documentation/security/tpm/tpm_ftpm_tee.rst
>> >  create mode 100644 drivers/char/tpm/tpm_ftpm_tee.c
>> >  create mode 100644 drivers/char/tpm/tpm_ftpm_tee.h
>> >
>> > --
>> > 2.20.1
>> >
>>
>> I applied the patches now. Appreciate a lot the patience with these.
>> Thank you.
>
>Hi, can you possibly fix these:

Any objection to sending you a patch on top of your tree instead?

--
Thanks,
Sasha
Jarkko Sakkinen Aug. 5, 2019, 10:51 p.m. UTC | #9
On Mon, Aug 05, 2019 at 02:05:18PM -0400, Sasha Levin wrote:
> On Mon, Aug 05, 2019 at 12:44:28AM +0300, Jarkko Sakkinen wrote:
> > On Thu, Jul 11, 2019 at 11:08:58PM +0300, Jarkko Sakkinen wrote:
> > > On Fri, Jul 05, 2019 at 04:47:44PM -0400, Sasha Levin wrote:
> > > > Changes from v7:
> > > >
> > > >  - Address Jarkko's comments.
> > > >
> > > > Sasha Levin (2):
> > > >   fTPM: firmware TPM running in TEE
> > > >   fTPM: add documentation for ftpm driver
> > > >
> > > >  Documentation/security/tpm/index.rst        |   1 +
> > > >  Documentation/security/tpm/tpm_ftpm_tee.rst |  27 ++
> > > >  drivers/char/tpm/Kconfig                    |   5 +
> > > >  drivers/char/tpm/Makefile                   |   1 +
> > > >  drivers/char/tpm/tpm_ftpm_tee.c             | 350 ++++++++++++++++++++
> > > >  drivers/char/tpm/tpm_ftpm_tee.h             |  40 +++
> > > >  6 files changed, 424 insertions(+)
> > > >  create mode 100644 Documentation/security/tpm/tpm_ftpm_tee.rst
> > > >  create mode 100644 drivers/char/tpm/tpm_ftpm_tee.c
> > > >  create mode 100644 drivers/char/tpm/tpm_ftpm_tee.h
> > > >
> > > > --
> > > > 2.20.1
> > > >
> > > 
> > > I applied the patches now. Appreciate a lot the patience with these.
> > > Thank you.
> > 
> > Hi, can you possibly fix these:
> 
> Any objection to sending you a patch on top of your tree instead?

Go ahead. Added the previous patches to my master.

/Jarkko
Rouven Czerwinski Aug. 7, 2019, 1:21 p.m. UTC | #10
Hi,

I spent some time with the fTPM module and TA on a Nitrogen6X with the
latest OP-TEE master. After stumbling through the "tee_supplicant no
persistent storage" problem, my module now issues the following error
message on module load:

[   34.633252] tpm tpm0: ftpm_tee_tpm_op_send: SUBMIT_COMMAND invoke error: 0xffff0006
[   34.641035] tpm tpm0: tpm_try_transmit: send(): error -65530
[   34.647008] tpm tpm0: ftpm_tee_tpm_op_send: SUBMIT_COMMAND invoke error: 0xffff0006
[   34.654788] tpm tpm0: tpm_try_transmit: send(): error -65530
[   34.660480] ftpm-tee ftpm: ftpm_tee_probe: tpm_chip_register failed with rc=-65530
[   34.678087] ftpm-tee: probe of ftpm failed with error -65530

To me the TEE_ERROR_BAD_PARAMETERS indicates some ABI issue between the
TA and the kernel module. Note that I built the TA from 
https://github.com/microsoft/MSRSec.git with commit
6bb57db632c424f87cbaf7ec6f9c89be7682b3c0. Maybe this is not the correct
version, I had some problems building the module from the repository
mentioned in the Patches

Regards,
Rouven Czerwinski
Sasha Levin Aug. 8, 2019, 1:08 a.m. UTC | #11
On Tue, Aug 06, 2019 at 01:51:32AM +0300, Jarkko Sakkinen wrote:
>On Mon, Aug 05, 2019 at 02:05:18PM -0400, Sasha Levin wrote:
>> On Mon, Aug 05, 2019 at 12:44:28AM +0300, Jarkko Sakkinen wrote:
>> > On Thu, Jul 11, 2019 at 11:08:58PM +0300, Jarkko Sakkinen wrote:
>> > > On Fri, Jul 05, 2019 at 04:47:44PM -0400, Sasha Levin wrote:
>> > > > Changes from v7:
>> > > >
>> > > >  - Address Jarkko's comments.
>> > > >
>> > > > Sasha Levin (2):
>> > > >   fTPM: firmware TPM running in TEE
>> > > >   fTPM: add documentation for ftpm driver
>> > > >
>> > > >  Documentation/security/tpm/index.rst        |   1 +
>> > > >  Documentation/security/tpm/tpm_ftpm_tee.rst |  27 ++
>> > > >  drivers/char/tpm/Kconfig                    |   5 +
>> > > >  drivers/char/tpm/Makefile                   |   1 +
>> > > >  drivers/char/tpm/tpm_ftpm_tee.c             | 350 ++++++++++++++++++++
>> > > >  drivers/char/tpm/tpm_ftpm_tee.h             |  40 +++
>> > > >  6 files changed, 424 insertions(+)
>> > > >  create mode 100644 Documentation/security/tpm/tpm_ftpm_tee.rst
>> > > >  create mode 100644 drivers/char/tpm/tpm_ftpm_tee.c
>> > > >  create mode 100644 drivers/char/tpm/tpm_ftpm_tee.h
>> > > >
>> > > > --
>> > > > 2.20.1
>> > > >
>> > >
>> > > I applied the patches now. Appreciate a lot the patience with these.
>> > > Thank you.
>> >
>> > Hi, can you possibly fix these:
>>
>> Any objection to sending you a patch on top of your tree instead?
>
>Go ahead. Added the previous patches to my master.

Thanks! I'm getting back home on Monday and I'll send it out right away.

--
Thanks,
Sasha