[v8,00/22] add integrity and security to TPM2 transactions

Message

James Bottomley April 29, 2024, 8:27 p.m. UTC

The interest in securing the TPM against interposers, both active and
passive has risen to fever pitch with the demonstration of key
recovery against windows bitlocker:

https://dolosgroup.io/blog/2021/7/9/from-stolen-laptop-to-inside-the-company-network

And subsequently the same attack being successful against all the
Linux TPM based security solutions:

https://www.secura.com/blog/tpm-sniffing-attacks-against-non-bitlocker-targets

The attacks fall into two categories:

1. Passive Interposers, which sit on the bus and merely observe
2. Active Interposers, which try to manipulate TPM transactions on the
   bus using man in the middle and packet stealing to create TPM state
   the interposer owner desires.

Our broadest interposer target is the use of TPM_RS_PW for password
authorization which sends the actual password to the TPM without any
obfuscation and effectively hands it to any interposer. The way to fix
this is to use real sessions for HMAC capabilities to ensure integrity
and to use parameter and response encryption to ensure confidentiality
of the data flowing over the TPM bus.  HMAC sessions by agreeing a
challenge with the TPM and then giving a response which is a HMAC of
the password and the challenge, so the application proves knowledge of
the password to the TPM without ever transmitting the password itself.
Using HMAC sessions when sending commands to the TPM also provides
some measure of protection against active interposers, since the
interposer can't interfere with or delete a HMAC'd command (because
they can't manufacture a response with the correct HMAC).

To protect TPM transactions where there isn't a shared secret
(i.e. the command is something like a PCR extension which doesn't
involve a TPM object with a password) we have to do a bit more work to
set up sessions with a passed in encrypted secret (called a salt) to
act in place of the shared secret in the HMAC.  This secret salt is
effectively a random number encrypted to a public key of the TPM.  The
final piece of the puzzle is using parameter input and response return
encryption, so any interposer can't see the data passing from the
application to the TPM and vice versa.

The most insidious interposer attack of all is a reset attack: since
the interposer has access to the TPM bus, it can assert the TPM reset
line any time it wants.  When a TPM resets it mostly comes back in the
same state except that all the PCRs are reset to their initial values.
Controlling the reset line allows the interposer to change the PCR
state after the fact by resetting the TPM and then replaying PCR
extends to get the PCRs into a valid state to release secrets, so even
if an attack event was recorded, the record is erased.  This reset
attack violates the fundamental princible of non-repudiability of TPM
logs.  Defeating the reset attack involves tying all TPM operations
within the kernel to a property which will change detectably if the
TPM is reset.  For that reason, we tie all TPM sessions to the null
hierarchy we obtain at start of day and whose seed changes on every
reset.  If an active interposer asserts a TPM reset, the new null
primary won't match the kernel's stored one and all TPM operations
will start failing because of HMAC mismatches in the sessions.  So if
the kernel TPM code keeps operating, it guarantees that a reset hasn't
occurred.

The final part of the puzzle is that the machine owner must have a
fixed idea of the EK of their TPM and should have certified this with
the TPM manufacturer.  On every boot, the certified EK public key
should be used to do a make credential/activate credential attestation
key insertion and then the null key certified with the attestation
key.  We can follow a trust on first use model where an OS
installation will extract and verify a public EK and save it to a read
only file.

This patch series adds a simple API which can ensure the above
properties as a layered addition to the existing TPM handling code.
This series now includes protections for PCR extend, getting random
numbers from the TPM and data sealing and unsealing.  It therefore
eliminates all uses of TPM2_RS_PW in the kernel and adds encryption
protection to sensitive data flowing into and out of the TPM.  The
first four patches add more sophisticated buffer handling to the TPM
which is needed to build the more complex encryption and
authentication based commands.  Patch 6 adds all the generic
cryptography primitives and patches 7-9 use them in critical TPM
operations where we want to avoid or detect interposers.  Patch 10
exports the name of the null key we used for boot/run time
verification and patch 11 documents the security guarantees and
expectations.

This was originally sent over four years ago, with the last iteration
being:

https://lore.kernel.org/linux-integrity/[e-mail]/

I'm dusting it off now because various forces at Microsoft and Google
via the Open Compute Platform are making a lot of noise about
interposers and we in the linux kernel look critically lacking in that
regard, particularly for TPM trusted keys.

---
v2 fixes the problems smatch reported and adds more explanation about
the code motion in the first few patches
v3 rebases the encryption to be against Ard's new library function, the
aescfb addition of which appears as patch 1.
v4 refreshes Ard's patch, adds kernel doc (including a new patch to
add it to the moved tpm-buf functions) updates and rewords some commit
logs
v5: update to proposed tpm-buf implementation (for ease of use all
precursor patches are part of this series, so the actual session HMAC
and encryption begins at patch 10) and add review feedback
v6: split the original sessions patch into three and change the config
variable name
v7: Collect reviews and add extra patch to check for and disable the TPM on
detecting a reset attack.
v8: split KDF out, add tpm_ prefix + other cosmetic updates

James

---

Ard Biesheuvel (1):
  crypto: lib - implement library version of AES in CFB mode

James Bottomley (14):
  tpm: Move buffer handling from static inlines to real functions
  tpm: add buffer function to point to returned parameters
  tpm: export the context save and load commands
  tpm: Add NULL primary creation
  tpm: Add TCG mandated Key Derivation Functions (KDFs)
  tpm: Add HMAC session start and end functions
  tpm: Add HMAC session name/handle append
  tpm: Add the rest of the session HMAC API
  tpm: add hmac checks to tpm2_pcr_extend()
  tpm: add session encryption protection to tpm2_get_random()
  KEYS: trusted: Add session encryption protection to the seal/unseal
    path
  tpm: add the null key name as a sysfs export
  Documentation: add tpm-security.rst
  tpm: disable the TPM if NULL name changes

Jarkko Sakkinen (7):
  tpm: Remove unused tpm_buf_tag()
  tpm: Remove tpm_send()
  tpm: Update struct tpm_buf documentation comments
  tpm: Store the length of the tpm_buf data separately.
  tpm: TPM2B formatted buffers
  tpm: Add tpm_buf_read_{u8,u16,u32}
  KEYS: trusted: tpm2: Use struct tpm_buf for sized buffers

 Documentation/security/tpm/tpm-security.rst |  216 ++++
 drivers/char/tpm/Kconfig                    |   14 +
 drivers/char/tpm/Makefile                   |    2 +
 drivers/char/tpm/tpm-buf.c                  |  251 ++++
 drivers/char/tpm/tpm-chip.c                 |    6 +
 drivers/char/tpm/tpm-interface.c            |   26 +-
 drivers/char/tpm/tpm-sysfs.c                |   18 +
 drivers/char/tpm/tpm.h                      |   14 +
 drivers/char/tpm/tpm2-cmd.c                 |   53 +-
 drivers/char/tpm/tpm2-sessions.c            | 1280 +++++++++++++++++++
 drivers/char/tpm/tpm2-space.c               |   11 +-
 include/crypto/aes.h                        |    5 +
 include/keys/trusted_tpm.h                  |    2 -
 include/linux/tpm.h                         |  316 +++--
 lib/crypto/Kconfig                          |    5 +
 lib/crypto/Makefile                         |    3 +
 lib/crypto/aescfb.c                         |  257 ++++
 security/keys/trusted-keys/trusted_tpm1.c   |   23 +-
 security/keys/trusted-keys/trusted_tpm2.c   |  136 +-
 19 files changed, 2443 insertions(+), 195 deletions(-)
 create mode 100644 Documentation/security/tpm/tpm-security.rst
 create mode 100644 drivers/char/tpm/tpm-buf.c
 create mode 100644 drivers/char/tpm/tpm2-sessions.c
 create mode 100644 lib/crypto/aescfb.c

Comments

Jarkko Sakkinen April 29, 2024, 10:22 p.m. UTC | #1

On Mon Apr 29, 2024 at 11:27 PM EEST, James Bottomley wrote:
> The interest in securing the TPM against interposers, both active and
> passive has risen to fever pitch with the demonstration of key
> recovery against windows bitlocker:
>
> https://dolosgroup.io/blog/2021/7/9/from-stolen-laptop-to-inside-the-company-network
>
> And subsequently the same attack being successful against all the
> Linux TPM based security solutions:
>
> https://www.secura.com/blog/tpm-sniffing-attacks-against-non-bitlocker-targets
>
> The attacks fall into two categories:
>
> 1. Passive Interposers, which sit on the bus and merely observe
> 2. Active Interposers, which try to manipulate TPM transactions on the
>    bus using man in the middle and packet stealing to create TPM state
>    the interposer owner desires.
>
> Our broadest interposer target is the use of TPM_RS_PW for password
> authorization which sends the actual password to the TPM without any
> obfuscation and effectively hands it to any interposer. The way to fix
> this is to use real sessions for HMAC capabilities to ensure integrity
> and to use parameter and response encryption to ensure confidentiality
> of the data flowing over the TPM bus.  HMAC sessions by agreeing a
> challenge with the TPM and then giving a response which is a HMAC of
> the password and the challenge, so the application proves knowledge of
> the password to the TPM without ever transmitting the password itself.
> Using HMAC sessions when sending commands to the TPM also provides
> some measure of protection against active interposers, since the
> interposer can't interfere with or delete a HMAC'd command (because
> they can't manufacture a response with the correct HMAC).
>
> To protect TPM transactions where there isn't a shared secret
> (i.e. the command is something like a PCR extension which doesn't
> involve a TPM object with a password) we have to do a bit more work to
> set up sessions with a passed in encrypted secret (called a salt) to
> act in place of the shared secret in the HMAC.  This secret salt is
> effectively a random number encrypted to a public key of the TPM.  The
> final piece of the puzzle is using parameter input and response return
> encryption, so any interposer can't see the data passing from the
> application to the TPM and vice versa.
>
> The most insidious interposer attack of all is a reset attack: since
> the interposer has access to the TPM bus, it can assert the TPM reset
> line any time it wants.  When a TPM resets it mostly comes back in the
> same state except that all the PCRs are reset to their initial values.
> Controlling the reset line allows the interposer to change the PCR
> state after the fact by resetting the TPM and then replaying PCR
> extends to get the PCRs into a valid state to release secrets, so even
> if an attack event was recorded, the record is erased.  This reset
> attack violates the fundamental princible of non-repudiability of TPM
> logs.  Defeating the reset attack involves tying all TPM operations
> within the kernel to a property which will change detectably if the
> TPM is reset.  For that reason, we tie all TPM sessions to the null
> hierarchy we obtain at start of day and whose seed changes on every
> reset.  If an active interposer asserts a TPM reset, the new null
> primary won't match the kernel's stored one and all TPM operations
> will start failing because of HMAC mismatches in the sessions.  So if
> the kernel TPM code keeps operating, it guarantees that a reset hasn't
> occurred.
>
> The final part of the puzzle is that the machine owner must have a
> fixed idea of the EK of their TPM and should have certified this with
> the TPM manufacturer.  On every boot, the certified EK public key
> should be used to do a make credential/activate credential attestation
> key insertion and then the null key certified with the attestation
> key.  We can follow a trust on first use model where an OS
> installation will extract and verify a public EK and save it to a read
> only file.
>
> This patch series adds a simple API which can ensure the above
> properties as a layered addition to the existing TPM handling code.
> This series now includes protections for PCR extend, getting random
> numbers from the TPM and data sealing and unsealing.  It therefore
> eliminates all uses of TPM2_RS_PW in the kernel and adds encryption
> protection to sensitive data flowing into and out of the TPM.  The
> first four patches add more sophisticated buffer handling to the TPM
> which is needed to build the more complex encryption and
> authentication based commands.  Patch 6 adds all the generic
> cryptography primitives and patches 7-9 use them in critical TPM
> operations where we want to avoid or detect interposers.  Patch 10
> exports the name of the null key we used for boot/run time
> verification and patch 11 documents the security guarantees and
> expectations.
>
> This was originally sent over four years ago, with the last iteration
> being:
>
> https://lore.kernel.org/linux-integrity/[e-mail]/
>
> I'm dusting it off now because various forces at Microsoft and Google
> via the Open Compute Platform are making a lot of noise about
> interposers and we in the linux kernel look critically lacking in that
> regard, particularly for TPM trusted keys.
>
> ---
> v2 fixes the problems smatch reported and adds more explanation about
> the code motion in the first few patches
> v3 rebases the encryption to be against Ard's new library function, the
> aescfb addition of which appears as patch 1.
> v4 refreshes Ard's patch, adds kernel doc (including a new patch to
> add it to the moved tpm-buf functions) updates and rewords some commit
> logs
> v5: update to proposed tpm-buf implementation (for ease of use all
> precursor patches are part of this series, so the actual session HMAC
> and encryption begins at patch 10) and add review feedback
> v6: split the original sessions patch into three and change the config
> variable name
> v7: Collect reviews and add extra patch to check for and disable the TPM on
> detecting a reset attack.
> v8: split KDF out, add tpm_ prefix + other cosmetic updates
>
> James
>
> ---
>
> Ard Biesheuvel (1):
>   crypto: lib - implement library version of AES in CFB mode
>
> James Bottomley (14):
>   tpm: Move buffer handling from static inlines to real functions
>   tpm: add buffer function to point to returned parameters
>   tpm: export the context save and load commands
>   tpm: Add NULL primary creation
>   tpm: Add TCG mandated Key Derivation Functions (KDFs)
>   tpm: Add HMAC session start and end functions
>   tpm: Add HMAC session name/handle append
>   tpm: Add the rest of the session HMAC API
>   tpm: add hmac checks to tpm2_pcr_extend()
>   tpm: add session encryption protection to tpm2_get_random()
>   KEYS: trusted: Add session encryption protection to the seal/unseal
>     path
>   tpm: add the null key name as a sysfs export
>   Documentation: add tpm-security.rst
>   tpm: disable the TPM if NULL name changes
>
> Jarkko Sakkinen (7):
>   tpm: Remove unused tpm_buf_tag()
>   tpm: Remove tpm_send()
>   tpm: Update struct tpm_buf documentation comments
>   tpm: Store the length of the tpm_buf data separately.
>   tpm: TPM2B formatted buffers
>   tpm: Add tpm_buf_read_{u8,u16,u32}
>   KEYS: trusted: tpm2: Use struct tpm_buf for sized buffers
>
>  Documentation/security/tpm/tpm-security.rst |  216 ++++
>  drivers/char/tpm/Kconfig                    |   14 +
>  drivers/char/tpm/Makefile                   |    2 +
>  drivers/char/tpm/tpm-buf.c                  |  251 ++++
>  drivers/char/tpm/tpm-chip.c                 |    6 +
>  drivers/char/tpm/tpm-interface.c            |   26 +-
>  drivers/char/tpm/tpm-sysfs.c                |   18 +
>  drivers/char/tpm/tpm.h                      |   14 +
>  drivers/char/tpm/tpm2-cmd.c                 |   53 +-
>  drivers/char/tpm/tpm2-sessions.c            | 1280 +++++++++++++++++++
>  drivers/char/tpm/tpm2-space.c               |   11 +-
>  include/crypto/aes.h                        |    5 +
>  include/keys/trusted_tpm.h                  |    2 -
>  include/linux/tpm.h                         |  316 +++--
>  lib/crypto/Kconfig                          |    5 +
>  lib/crypto/Makefile                         |    3 +
>  lib/crypto/aescfb.c                         |  257 ++++
>  security/keys/trusted-keys/trusted_tpm1.c   |   23 +-
>  security/keys/trusted-keys/trusted_tpm2.c   |  136 +-
>  19 files changed, 2443 insertions(+), 195 deletions(-)
>  create mode 100644 Documentation/security/tpm/tpm-security.rst
>  create mode 100644 drivers/char/tpm/tpm-buf.c
>  create mode 100644 drivers/char/tpm/tpm2-sessions.c
>  create mode 100644 lib/crypto/aescfb.c

Thanks for the update!

I think I asked this already earlier but unfortunately could not
find the corresponding email from lore.

Anyway, I've tested this series with QEMU i.e. to the point that
I know that it does not break anything in the case when things are
working as expected.

What I would like to test is the negative case when the null key
name changes and see what happens.

I recall that you had some version of QEMU that had ability to test
this and my latest question on that was what QEMU baseline it was
expected to be applied over.

Since I could not find the email subthread I neither have the patch nor
do know the baseline. So if you could help with these details then we
can move forward.

I can also work with QEMU Git fork if you have one and point out
QEMU_OVERRIDE_SRCDIR to the clone.

It is somewhat mandatory IMHO to be able to test this to both 
directions, right?

BR, Jarkko

Jarkko Sakkinen April 29, 2024, 10:26 p.m. UTC | #2

On Tue Apr 30, 2024 at 1:22 AM EEST, Jarkko Sakkinen wrote:
> On Mon Apr 29, 2024 at 11:27 PM EEST, James Bottomley wrote:
> > The interest in securing the TPM against interposers, both active and
> > passive has risen to fever pitch with the demonstration of key
> > recovery against windows bitlocker:
> >
> > https://dolosgroup.io/blog/2021/7/9/from-stolen-laptop-to-inside-the-company-network
> >
> > And subsequently the same attack being successful against all the
> > Linux TPM based security solutions:
> >
> > https://www.secura.com/blog/tpm-sniffing-attacks-against-non-bitlocker-targets
> >
> > The attacks fall into two categories:
> >
> > 1. Passive Interposers, which sit on the bus and merely observe
> > 2. Active Interposers, which try to manipulate TPM transactions on the
> >    bus using man in the middle and packet stealing to create TPM state
> >    the interposer owner desires.
> >
> > Our broadest interposer target is the use of TPM_RS_PW for password
> > authorization which sends the actual password to the TPM without any
> > obfuscation and effectively hands it to any interposer. The way to fix
> > this is to use real sessions for HMAC capabilities to ensure integrity
> > and to use parameter and response encryption to ensure confidentiality
> > of the data flowing over the TPM bus.  HMAC sessions by agreeing a
> > challenge with the TPM and then giving a response which is a HMAC of
> > the password and the challenge, so the application proves knowledge of
> > the password to the TPM without ever transmitting the password itself.
> > Using HMAC sessions when sending commands to the TPM also provides
> > some measure of protection against active interposers, since the
> > interposer can't interfere with or delete a HMAC'd command (because
> > they can't manufacture a response with the correct HMAC).
> >
> > To protect TPM transactions where there isn't a shared secret
> > (i.e. the command is something like a PCR extension which doesn't
> > involve a TPM object with a password) we have to do a bit more work to
> > set up sessions with a passed in encrypted secret (called a salt) to
> > act in place of the shared secret in the HMAC.  This secret salt is
> > effectively a random number encrypted to a public key of the TPM.  The
> > final piece of the puzzle is using parameter input and response return
> > encryption, so any interposer can't see the data passing from the
> > application to the TPM and vice versa.
> >
> > The most insidious interposer attack of all is a reset attack: since
> > the interposer has access to the TPM bus, it can assert the TPM reset
> > line any time it wants.  When a TPM resets it mostly comes back in the
> > same state except that all the PCRs are reset to their initial values.
> > Controlling the reset line allows the interposer to change the PCR
> > state after the fact by resetting the TPM and then replaying PCR
> > extends to get the PCRs into a valid state to release secrets, so even
> > if an attack event was recorded, the record is erased.  This reset
> > attack violates the fundamental princible of non-repudiability of TPM
> > logs.  Defeating the reset attack involves tying all TPM operations
> > within the kernel to a property which will change detectably if the
> > TPM is reset.  For that reason, we tie all TPM sessions to the null
> > hierarchy we obtain at start of day and whose seed changes on every
> > reset.  If an active interposer asserts a TPM reset, the new null
> > primary won't match the kernel's stored one and all TPM operations
> > will start failing because of HMAC mismatches in the sessions.  So if
> > the kernel TPM code keeps operating, it guarantees that a reset hasn't
> > occurred.
> >
> > The final part of the puzzle is that the machine owner must have a
> > fixed idea of the EK of their TPM and should have certified this with
> > the TPM manufacturer.  On every boot, the certified EK public key
> > should be used to do a make credential/activate credential attestation
> > key insertion and then the null key certified with the attestation
> > key.  We can follow a trust on first use model where an OS
> > installation will extract and verify a public EK and save it to a read
> > only file.
> >
> > This patch series adds a simple API which can ensure the above
> > properties as a layered addition to the existing TPM handling code.
> > This series now includes protections for PCR extend, getting random
> > numbers from the TPM and data sealing and unsealing.  It therefore
> > eliminates all uses of TPM2_RS_PW in the kernel and adds encryption
> > protection to sensitive data flowing into and out of the TPM.  The
> > first four patches add more sophisticated buffer handling to the TPM
> > which is needed to build the more complex encryption and
> > authentication based commands.  Patch 6 adds all the generic
> > cryptography primitives and patches 7-9 use them in critical TPM
> > operations where we want to avoid or detect interposers.  Patch 10
> > exports the name of the null key we used for boot/run time
> > verification and patch 11 documents the security guarantees and
> > expectations.
> >
> > This was originally sent over four years ago, with the last iteration
> > being:
> >
> > https://lore.kernel.org/linux-integrity/[e-mail]/
> >
> > I'm dusting it off now because various forces at Microsoft and Google
> > via the Open Compute Platform are making a lot of noise about
> > interposers and we in the linux kernel look critically lacking in that
> > regard, particularly for TPM trusted keys.
> >
> > ---
> > v2 fixes the problems smatch reported and adds more explanation about
> > the code motion in the first few patches
> > v3 rebases the encryption to be against Ard's new library function, the
> > aescfb addition of which appears as patch 1.
> > v4 refreshes Ard's patch, adds kernel doc (including a new patch to
> > add it to the moved tpm-buf functions) updates and rewords some commit
> > logs
> > v5: update to proposed tpm-buf implementation (for ease of use all
> > precursor patches are part of this series, so the actual session HMAC
> > and encryption begins at patch 10) and add review feedback
> > v6: split the original sessions patch into three and change the config
> > variable name
> > v7: Collect reviews and add extra patch to check for and disable the TPM on
> > detecting a reset attack.
> > v8: split KDF out, add tpm_ prefix + other cosmetic updates
> >
> > James
> >
> > ---
> >
> > Ard Biesheuvel (1):
> >   crypto: lib - implement library version of AES in CFB mode
> >
> > James Bottomley (14):
> >   tpm: Move buffer handling from static inlines to real functions
> >   tpm: add buffer function to point to returned parameters
> >   tpm: export the context save and load commands
> >   tpm: Add NULL primary creation
> >   tpm: Add TCG mandated Key Derivation Functions (KDFs)
> >   tpm: Add HMAC session start and end functions
> >   tpm: Add HMAC session name/handle append
> >   tpm: Add the rest of the session HMAC API
> >   tpm: add hmac checks to tpm2_pcr_extend()
> >   tpm: add session encryption protection to tpm2_get_random()
> >   KEYS: trusted: Add session encryption protection to the seal/unseal
> >     path
> >   tpm: add the null key name as a sysfs export
> >   Documentation: add tpm-security.rst
> >   tpm: disable the TPM if NULL name changes
> >
> > Jarkko Sakkinen (7):
> >   tpm: Remove unused tpm_buf_tag()
> >   tpm: Remove tpm_send()
> >   tpm: Update struct tpm_buf documentation comments
> >   tpm: Store the length of the tpm_buf data separately.
> >   tpm: TPM2B formatted buffers
> >   tpm: Add tpm_buf_read_{u8,u16,u32}
> >   KEYS: trusted: tpm2: Use struct tpm_buf for sized buffers
> >
> >  Documentation/security/tpm/tpm-security.rst |  216 ++++
> >  drivers/char/tpm/Kconfig                    |   14 +
> >  drivers/char/tpm/Makefile                   |    2 +
> >  drivers/char/tpm/tpm-buf.c                  |  251 ++++
> >  drivers/char/tpm/tpm-chip.c                 |    6 +
> >  drivers/char/tpm/tpm-interface.c            |   26 +-
> >  drivers/char/tpm/tpm-sysfs.c                |   18 +
> >  drivers/char/tpm/tpm.h                      |   14 +
> >  drivers/char/tpm/tpm2-cmd.c                 |   53 +-
> >  drivers/char/tpm/tpm2-sessions.c            | 1280 +++++++++++++++++++
> >  drivers/char/tpm/tpm2-space.c               |   11 +-
> >  include/crypto/aes.h                        |    5 +
> >  include/keys/trusted_tpm.h                  |    2 -
> >  include/linux/tpm.h                         |  316 +++--
> >  lib/crypto/Kconfig                          |    5 +
> >  lib/crypto/Makefile                         |    3 +
> >  lib/crypto/aescfb.c                         |  257 ++++
> >  security/keys/trusted-keys/trusted_tpm1.c   |   23 +-
> >  security/keys/trusted-keys/trusted_tpm2.c   |  136 +-
> >  19 files changed, 2443 insertions(+), 195 deletions(-)
> >  create mode 100644 Documentation/security/tpm/tpm-security.rst
> >  create mode 100644 drivers/char/tpm/tpm-buf.c
> >  create mode 100644 drivers/char/tpm/tpm2-sessions.c
> >  create mode 100644 lib/crypto/aescfb.c
>
> Thanks for the update!
>
> I think I asked this already earlier but unfortunately could not
> find the corresponding email from lore.
>
> Anyway, I've tested this series with QEMU i.e. to the point that
> I know that it does not break anything in the case when things are
> working as expected.
>
> What I would like to test is the negative case when the null key
> name changes and see what happens.
>
> I recall that you had some version of QEMU that had ability to test
> this and my latest question on that was what QEMU baseline it was
> expected to be applied over.
>
> Since I could not find the email subthread I neither have the patch nor
> do know the baseline. So if you could help with these details then we
> can move forward.
>
> I can also work with QEMU Git fork if you have one and point out
> QEMU_OVERRIDE_SRCDIR to the clone.
>
> It is somewhat mandatory IMHO to be able to test this to both 
> directions, right?

Right and obviously 3rd option is to send a PR to
https://gitlab.com/jarkkojs/linux-tpmdd-test.

I.e. patch file goes to patches/qemu (BR2_GLOBAL_PATCH_DIR
points there).

BR, Jarkko

Jarkko Sakkinen April 29, 2024, 11:49 p.m. UTC | #3

On Tue Apr 30, 2024 at 1:26 AM EEST, Jarkko Sakkinen wrote:
> Right and obviously 3rd option is to send a PR to
> https://gitlab.com/jarkkojs/linux-tpmdd-test.
>
> I.e. patch file goes to patches/qemu (BR2_GLOBAL_PATCH_DIR
> points there).

Stefan, can I do a "zero QEMU changes" negative test for
changing null seed by somehow reseting swtpm? That would
be best possible option (if it is possible).

It does not matter what side-effects it has on swtpm side
as long as the hmac path gets invalidated, as then the
device is rendered as unusable.

BR, Jarkko

Stefan Berger April 30, 2024, 11:18 a.m. UTC | #4

On 4/29/24 19:49, Jarkko Sakkinen wrote:
> On Tue Apr 30, 2024 at 1:26 AM EEST, Jarkko Sakkinen wrote:
>> Right and obviously 3rd option is to send a PR to
>> https://gitlab.com/jarkkojs/linux-tpmdd-test.
>>
>> I.e. patch file goes to patches/qemu (BR2_GLOBAL_PATCH_DIR
>> points there).
> 
> Stefan, can I do a "zero QEMU changes" negative test for
> changing null seed by somehow reseting swtpm? That would
> be best possible option (if it is possible).

You cannot easily reset swtpm without changing 'something' and resetting 
the NULL seed only works when running TPM2_Startup. You could modify 
some TPM2 command to do what HierarchyStartup does with the nullSeed to 
simulate what you want.

> 
> It does not matter what side-effects it has on swtpm side
> as long as the hmac path gets invalidated, as then the
> device is rendered as unusable.
> 
> BR, Jarkko

Jarkko Sakkinen April 30, 2024, 6:37 p.m. UTC | #5

On Tue Apr 30, 2024 at 2:18 PM EEST, Stefan Berger wrote:
>
>
> On 4/29/24 19:49, Jarkko Sakkinen wrote:
> > On Tue Apr 30, 2024 at 1:26 AM EEST, Jarkko Sakkinen wrote:
> >> Right and obviously 3rd option is to send a PR to
> >> https://gitlab.com/jarkkojs/linux-tpmdd-test.
> >>
> >> I.e. patch file goes to patches/qemu (BR2_GLOBAL_PATCH_DIR
> >> points there).
> > 
> > Stefan, can I do a "zero QEMU changes" negative test for
> > changing null seed by somehow reseting swtpm? That would
> > be best possible option (if it is possible).
>
> You cannot easily reset swtpm without changing 'something' and resetting 
> the NULL seed only works when running TPM2_Startup. You could modify 
> some TPM2 command to do what HierarchyStartup does with the nullSeed to 
> simulate what you want.

Hmm... I'm not too eager to modify swtpm itself.

So one hacky option might be to run swtpm using system() in an
interposer program and that program would again create chardev
using cuse.

That program would again have to modify traffic "at some point".

Maybe +1 command after TPM2_StartAuthSession or later?

BR, Jarkko

Stefan Berger April 30, 2024, 6:57 p.m. UTC | #6

On 4/30/24 14:37, Jarkko Sakkinen wrote:
> On Tue Apr 30, 2024 at 2:18 PM EEST, Stefan Berger wrote:
>>
>>
>> On 4/29/24 19:49, Jarkko Sakkinen wrote:
>>> On Tue Apr 30, 2024 at 1:26 AM EEST, Jarkko Sakkinen wrote:
>>>> Right and obviously 3rd option is to send a PR to
>>>> https://gitlab.com/jarkkojs/linux-tpmdd-test.
>>>>
>>>> I.e. patch file goes to patches/qemu (BR2_GLOBAL_PATCH_DIR
>>>> points there).
>>>
>>> Stefan, can I do a "zero QEMU changes" negative test for
>>> changing null seed by somehow reseting swtpm? That would
>>> be best possible option (if it is possible).
>>
>> You cannot easily reset swtpm without changing 'something' and resetting
>> the NULL seed only works when running TPM2_Startup. You could modify
>> some TPM2 command to do what HierarchyStartup does with the nullSeed to
>> simulate what you want.
> 
> Hmm... I'm not too eager to modify swtpm itself.

You would modify libtpms. You may just want to copy the few relevant 
lines from HierarchyStartup function into another TPM 2 command that 
then resets the null seed and whatever else you need reset as a side 
effect. This sounds simpler to me than what you are proposing with 
system(). Who or what would run system().

> 
> So one hacky option might be to run swtpm using system() in an
> interposer program and that program would again create chardev
> using cuse.
> 
> That program would again have to modify traffic "at some point".
> 
> Maybe +1 command after TPM2_StartAuthSession or later?
> 
> BR, Jarkko

James Bottomley April 30, 2024, 7:23 p.m. UTC | #7

On Tue, 2024-04-30 at 01:22 +0300, Jarkko Sakkinen wrote:
> On Mon Apr 29, 2024 at 11:27 PM EEST, James Bottomley wrote:
> > The interest in securing the TPM against interposers, both active
> > and
> > passive has risen to fever pitch with the demonstration of key
> > recovery against windows bitlocker:
> > 
> > https://dolosgroup.io/blog/2021/7/9/from-stolen-laptop-to-inside-the-company-network
> > 
> > And subsequently the same attack being successful against all the
> > Linux TPM based security solutions:
> > 
> > https://www.secura.com/blog/tpm-sniffing-attacks-against-non-bitlocker-targets
> > 
> > The attacks fall into two categories:
> > 
> > 1. Passive Interposers, which sit on the bus and merely observe
> > 2. Active Interposers, which try to manipulate TPM transactions on
> > the
> >    bus using man in the middle and packet stealing to create TPM
> > state
> >    the interposer owner desires.
> > 
> > Our broadest interposer target is the use of TPM_RS_PW for password
> > authorization which sends the actual password to the TPM without
> > any
> > obfuscation and effectively hands it to any interposer. The way to
> > fix
> > this is to use real sessions for HMAC capabilities to ensure
> > integrity
> > and to use parameter and response encryption to ensure
> > confidentiality
> > of the data flowing over the TPM bus.  HMAC sessions by agreeing a
> > challenge with the TPM and then giving a response which is a HMAC
> > of
> > the password and the challenge, so the application proves knowledge
> > of
> > the password to the TPM without ever transmitting the password
> > itself.
> > Using HMAC sessions when sending commands to the TPM also provides
> > some measure of protection against active interposers, since the
> > interposer can't interfere with or delete a HMAC'd command (because
> > they can't manufacture a response with the correct HMAC).
> > 
> > To protect TPM transactions where there isn't a shared secret
> > (i.e. the command is something like a PCR extension which doesn't
> > involve a TPM object with a password) we have to do a bit more work
> > to
> > set up sessions with a passed in encrypted secret (called a salt)
> > to
> > act in place of the shared secret in the HMAC.  This secret salt is
> > effectively a random number encrypted to a public key of the TPM. 
> > The
> > final piece of the puzzle is using parameter input and response
> > return
> > encryption, so any interposer can't see the data passing from the
> > application to the TPM and vice versa.
> > 
> > The most insidious interposer attack of all is a reset attack:
> > since
> > the interposer has access to the TPM bus, it can assert the TPM
> > reset
> > line any time it wants.  When a TPM resets it mostly comes back in
> > the
> > same state except that all the PCRs are reset to their initial
> > values.
> > Controlling the reset line allows the interposer to change the PCR
> > state after the fact by resetting the TPM and then replaying PCR
> > extends to get the PCRs into a valid state to release secrets, so
> > even
> > if an attack event was recorded, the record is erased.  This reset
> > attack violates the fundamental princible of non-repudiability of
> > TPM
> > logs.  Defeating the reset attack involves tying all TPM operations
> > within the kernel to a property which will change detectably if the
> > TPM is reset.  For that reason, we tie all TPM sessions to the null
> > hierarchy we obtain at start of day and whose seed changes on every
> > reset.  If an active interposer asserts a TPM reset, the new null
> > primary won't match the kernel's stored one and all TPM operations
> > will start failing because of HMAC mismatches in the sessions.  So
> > if
> > the kernel TPM code keeps operating, it guarantees that a reset
> > hasn't
> > occurred.
> > 
> > The final part of the puzzle is that the machine owner must have a
> > fixed idea of the EK of their TPM and should have certified this
> > with
> > the TPM manufacturer.  On every boot, the certified EK public key
> > should be used to do a make credential/activate credential
> > attestation
> > key insertion and then the null key certified with the attestation
> > key.  We can follow a trust on first use model where an OS
> > installation will extract and verify a public EK and save it to a
> > read
> > only file.
> > 
> > This patch series adds a simple API which can ensure the above
> > properties as a layered addition to the existing TPM handling code.
> > This series now includes protections for PCR extend, getting random
> > numbers from the TPM and data sealing and unsealing.  It therefore
> > eliminates all uses of TPM2_RS_PW in the kernel and adds encryption
> > protection to sensitive data flowing into and out of the TPM.  The
> > first four patches add more sophisticated buffer handling to the
> > TPM
> > which is needed to build the more complex encryption and
> > authentication based commands.  Patch 6 adds all the generic
> > cryptography primitives and patches 7-9 use them in critical TPM
> > operations where we want to avoid or detect interposers.  Patch 10
> > exports the name of the null key we used for boot/run time
> > verification and patch 11 documents the security guarantees and
> > expectations.
> > 
> > This was originally sent over four years ago, with the last
> > iteration
> > being:
> > 
> > https://lore.kernel.org/linux-integrity/[e-mail]/
> > 
> > I'm dusting it off now because various forces at Microsoft and
> > Google
> > via the Open Compute Platform are making a lot of noise about
> > interposers and we in the linux kernel look critically lacking in
> > that
> > regard, particularly for TPM trusted keys.
> > 
> > ---
> > v2 fixes the problems smatch reported and adds more explanation
> > about
> > the code motion in the first few patches
> > v3 rebases the encryption to be against Ard's new library function,
> > the
> > aescfb addition of which appears as patch 1.
> > v4 refreshes Ard's patch, adds kernel doc (including a new patch to
> > add it to the moved tpm-buf functions) updates and rewords some
> > commit
> > logs
> > v5: update to proposed tpm-buf implementation (for ease of use all
> > precursor patches are part of this series, so the actual session
> > HMAC
> > and encryption begins at patch 10) and add review feedback
> > v6: split the original sessions patch into three and change the
> > config
> > variable name
> > v7: Collect reviews and add extra patch to check for and disable
> > the TPM on
> > detecting a reset attack.
> > v8: split KDF out, add tpm_ prefix + other cosmetic updates
> > 
> > James
> > 
> > ---
> > 
> > Ard Biesheuvel (1):
> >   crypto: lib - implement library version of AES in CFB mode
> > 
> > James Bottomley (14):
> >   tpm: Move buffer handling from static inlines to real functions
> >   tpm: add buffer function to point to returned parameters
> >   tpm: export the context save and load commands
> >   tpm: Add NULL primary creation
> >   tpm: Add TCG mandated Key Derivation Functions (KDFs)
> >   tpm: Add HMAC session start and end functions
> >   tpm: Add HMAC session name/handle append
> >   tpm: Add the rest of the session HMAC API
> >   tpm: add hmac checks to tpm2_pcr_extend()
> >   tpm: add session encryption protection to tpm2_get_random()
> >   KEYS: trusted: Add session encryption protection to the
> > seal/unseal
> >     path
> >   tpm: add the null key name as a sysfs export
> >   Documentation: add tpm-security.rst
> >   tpm: disable the TPM if NULL name changes
> > 
> > Jarkko Sakkinen (7):
> >   tpm: Remove unused tpm_buf_tag()
> >   tpm: Remove tpm_send()
> >   tpm: Update struct tpm_buf documentation comments
> >   tpm: Store the length of the tpm_buf data separately.
> >   tpm: TPM2B formatted buffers
> >   tpm: Add tpm_buf_read_{u8,u16,u32}
> >   KEYS: trusted: tpm2: Use struct tpm_buf for sized buffers
> > 
> >  Documentation/security/tpm/tpm-security.rst |  216 ++++
> >  drivers/char/tpm/Kconfig                    |   14 +
> >  drivers/char/tpm/Makefile                   |    2 +
> >  drivers/char/tpm/tpm-buf.c                  |  251 ++++
> >  drivers/char/tpm/tpm-chip.c                 |    6 +
> >  drivers/char/tpm/tpm-interface.c            |   26 +-
> >  drivers/char/tpm/tpm-sysfs.c                |   18 +
> >  drivers/char/tpm/tpm.h                      |   14 +
> >  drivers/char/tpm/tpm2-cmd.c                 |   53 +-
> >  drivers/char/tpm/tpm2-sessions.c            | 1280
> > +++++++++++++++++++
> >  drivers/char/tpm/tpm2-space.c               |   11 +-
> >  include/crypto/aes.h                        |    5 +
> >  include/keys/trusted_tpm.h                  |    2 -
> >  include/linux/tpm.h                         |  316 +++--
> >  lib/crypto/Kconfig                          |    5 +
> >  lib/crypto/Makefile                         |    3 +
> >  lib/crypto/aescfb.c                         |  257 ++++
> >  security/keys/trusted-keys/trusted_tpm1.c   |   23 +-
> >  security/keys/trusted-keys/trusted_tpm2.c   |  136 +-
> >  19 files changed, 2443 insertions(+), 195 deletions(-)
> >  create mode 100644 Documentation/security/tpm/tpm-security.rst
> >  create mode 100644 drivers/char/tpm/tpm-buf.c
> >  create mode 100644 drivers/char/tpm/tpm2-sessions.c
> >  create mode 100644 lib/crypto/aescfb.c
> 
> Thanks for the update!
> 
> I think I asked this already earlier but unfortunately could not
> find the corresponding email from lore.

Well, you did, but at that time I didn't have the null name change
detection so:

> 
> Anyway, I've tested this series with QEMU i.e. to the point that
> I know that it does not break anything in the case when things are
> working as expected.
> 
> What I would like to test is the negative case when the null key
> name changes and see what happens.
> 
> I recall that you had some version of QEMU that had ability to test
> this and my latest question on that was what QEMU baseline it was
> expected to be applied over.

Yes, I added patches to qemu to make it talk directly to the mssim TPM
reference implementation

https://github.com/microsoft/ms-tpm-20-ref

so I could be sure I was testing against the reference implementation.
However, they also have the advantage that you can use wireshark to
dump the TPM transactions (ensuring encryption).  You can also tamper
with the TPM state from the outside by connecting to the TPM socket.

For the case you want, you can simulate a reset by killing and
restarting the tpm server (you have to power it up and issue the
startup command manually).  The next TPM command the kernel tries
should see the null name change and react accordingly.

It looks like the current qemu patches fail to apply again, so I just
reposted them against qemu git head:

https://lore.kernel.org/qemu-devel/[e-mail]/

> Since I could not find the email subthread I neither have the patch
> nor do know the baseline. So if you could help with these details
> then we can move forward.
> 
> I can also work with QEMU Git fork if you have one and point out
> QEMU_OVERRIDE_SRCDIR to the clone.

I only have the patches in a local git repository, but I could push
qemu up onto kernel.org if it would help?

James

Jarkko Sakkinen April 30, 2024, 9:48 p.m. UTC | #8

On Tue Apr 30, 2024 at 10:23 PM EEST, James Bottomley wrote:
> On Tue, 2024-04-30 at 01:22 +0300, Jarkko Sakkinen wrote:
> > On Mon Apr 29, 2024 at 11:27 PM EEST, James Bottomley wrote:
> > > The interest in securing the TPM against interposers, both active
> > > and
> > > passive has risen to fever pitch with the demonstration of key
> > > recovery against windows bitlocker:
> > > 
> > > https://dolosgroup.io/blog/2021/7/9/from-stolen-laptop-to-inside-the-company-network
> > > 
> > > And subsequently the same attack being successful against all the
> > > Linux TPM based security solutions:
> > > 
> > > https://www.secura.com/blog/tpm-sniffing-attacks-against-non-bitlocker-targets
> > > 
> > > The attacks fall into two categories:
> > > 
> > > 1. Passive Interposers, which sit on the bus and merely observe
> > > 2. Active Interposers, which try to manipulate TPM transactions on
> > > the
> > >    bus using man in the middle and packet stealing to create TPM
> > > state
> > >    the interposer owner desires.
> > > 
> > > Our broadest interposer target is the use of TPM_RS_PW for password
> > > authorization which sends the actual password to the TPM without
> > > any
> > > obfuscation and effectively hands it to any interposer. The way to
> > > fix
> > > this is to use real sessions for HMAC capabilities to ensure
> > > integrity
> > > and to use parameter and response encryption to ensure
> > > confidentiality
> > > of the data flowing over the TPM bus.  HMAC sessions by agreeing a
> > > challenge with the TPM and then giving a response which is a HMAC
> > > of
> > > the password and the challenge, so the application proves knowledge
> > > of
> > > the password to the TPM without ever transmitting the password
> > > itself.
> > > Using HMAC sessions when sending commands to the TPM also provides
> > > some measure of protection against active interposers, since the
> > > interposer can't interfere with or delete a HMAC'd command (because
> > > they can't manufacture a response with the correct HMAC).
> > > 
> > > To protect TPM transactions where there isn't a shared secret
> > > (i.e. the command is something like a PCR extension which doesn't
> > > involve a TPM object with a password) we have to do a bit more work
> > > to
> > > set up sessions with a passed in encrypted secret (called a salt)
> > > to
> > > act in place of the shared secret in the HMAC.  This secret salt is
> > > effectively a random number encrypted to a public key of the TPM. 
> > > The
> > > final piece of the puzzle is using parameter input and response
> > > return
> > > encryption, so any interposer can't see the data passing from the
> > > application to the TPM and vice versa.
> > > 
> > > The most insidious interposer attack of all is a reset attack:
> > > since
> > > the interposer has access to the TPM bus, it can assert the TPM
> > > reset
> > > line any time it wants.  When a TPM resets it mostly comes back in
> > > the
> > > same state except that all the PCRs are reset to their initial
> > > values.
> > > Controlling the reset line allows the interposer to change the PCR
> > > state after the fact by resetting the TPM and then replaying PCR
> > > extends to get the PCRs into a valid state to release secrets, so
> > > even
> > > if an attack event was recorded, the record is erased.  This reset
> > > attack violates the fundamental princible of non-repudiability of
> > > TPM
> > > logs.  Defeating the reset attack involves tying all TPM operations
> > > within the kernel to a property which will change detectably if the
> > > TPM is reset.  For that reason, we tie all TPM sessions to the null
> > > hierarchy we obtain at start of day and whose seed changes on every
> > > reset.  If an active interposer asserts a TPM reset, the new null
> > > primary won't match the kernel's stored one and all TPM operations
> > > will start failing because of HMAC mismatches in the sessions.  So
> > > if
> > > the kernel TPM code keeps operating, it guarantees that a reset
> > > hasn't
> > > occurred.
> > > 
> > > The final part of the puzzle is that the machine owner must have a
> > > fixed idea of the EK of their TPM and should have certified this
> > > with
> > > the TPM manufacturer.  On every boot, the certified EK public key
> > > should be used to do a make credential/activate credential
> > > attestation
> > > key insertion and then the null key certified with the attestation
> > > key.  We can follow a trust on first use model where an OS
> > > installation will extract and verify a public EK and save it to a
> > > read
> > > only file.
> > > 
> > > This patch series adds a simple API which can ensure the above
> > > properties as a layered addition to the existing TPM handling code.
> > > This series now includes protections for PCR extend, getting random
> > > numbers from the TPM and data sealing and unsealing.  It therefore
> > > eliminates all uses of TPM2_RS_PW in the kernel and adds encryption
> > > protection to sensitive data flowing into and out of the TPM.  The
> > > first four patches add more sophisticated buffer handling to the
> > > TPM
> > > which is needed to build the more complex encryption and
> > > authentication based commands.  Patch 6 adds all the generic
> > > cryptography primitives and patches 7-9 use them in critical TPM
> > > operations where we want to avoid or detect interposers.  Patch 10
> > > exports the name of the null key we used for boot/run time
> > > verification and patch 11 documents the security guarantees and
> > > expectations.
> > > 
> > > This was originally sent over four years ago, with the last
> > > iteration
> > > being:
> > > 
> > > https://lore.kernel.org/linux-integrity/[e-mail]/
> > > 
> > > I'm dusting it off now because various forces at Microsoft and
> > > Google
> > > via the Open Compute Platform are making a lot of noise about
> > > interposers and we in the linux kernel look critically lacking in
> > > that
> > > regard, particularly for TPM trusted keys.
> > > 
> > > ---
> > > v2 fixes the problems smatch reported and adds more explanation
> > > about
> > > the code motion in the first few patches
> > > v3 rebases the encryption to be against Ard's new library function,
> > > the
> > > aescfb addition of which appears as patch 1.
> > > v4 refreshes Ard's patch, adds kernel doc (including a new patch to
> > > add it to the moved tpm-buf functions) updates and rewords some
> > > commit
> > > logs
> > > v5: update to proposed tpm-buf implementation (for ease of use all
> > > precursor patches are part of this series, so the actual session
> > > HMAC
> > > and encryption begins at patch 10) and add review feedback
> > > v6: split the original sessions patch into three and change the
> > > config
> > > variable name
> > > v7: Collect reviews and add extra patch to check for and disable
> > > the TPM on
> > > detecting a reset attack.
> > > v8: split KDF out, add tpm_ prefix + other cosmetic updates
> > > 
> > > James
> > > 
> > > ---
> > > 
> > > Ard Biesheuvel (1):
> > >   crypto: lib - implement library version of AES in CFB mode
> > > 
> > > James Bottomley (14):
> > >   tpm: Move buffer handling from static inlines to real functions
> > >   tpm: add buffer function to point to returned parameters
> > >   tpm: export the context save and load commands
> > >   tpm: Add NULL primary creation
> > >   tpm: Add TCG mandated Key Derivation Functions (KDFs)
> > >   tpm: Add HMAC session start and end functions
> > >   tpm: Add HMAC session name/handle append
> > >   tpm: Add the rest of the session HMAC API
> > >   tpm: add hmac checks to tpm2_pcr_extend()
> > >   tpm: add session encryption protection to tpm2_get_random()
> > >   KEYS: trusted: Add session encryption protection to the
> > > seal/unseal
> > >     path
> > >   tpm: add the null key name as a sysfs export
> > >   Documentation: add tpm-security.rst
> > >   tpm: disable the TPM if NULL name changes
> > > 
> > > Jarkko Sakkinen (7):
> > >   tpm: Remove unused tpm_buf_tag()
> > >   tpm: Remove tpm_send()
> > >   tpm: Update struct tpm_buf documentation comments
> > >   tpm: Store the length of the tpm_buf data separately.
> > >   tpm: TPM2B formatted buffers
> > >   tpm: Add tpm_buf_read_{u8,u16,u32}
> > >   KEYS: trusted: tpm2: Use struct tpm_buf for sized buffers
> > > 
> > >  Documentation/security/tpm/tpm-security.rst |  216 ++++
> > >  drivers/char/tpm/Kconfig                    |   14 +
> > >  drivers/char/tpm/Makefile                   |    2 +
> > >  drivers/char/tpm/tpm-buf.c                  |  251 ++++
> > >  drivers/char/tpm/tpm-chip.c                 |    6 +
> > >  drivers/char/tpm/tpm-interface.c            |   26 +-
> > >  drivers/char/tpm/tpm-sysfs.c                |   18 +
> > >  drivers/char/tpm/tpm.h                      |   14 +
> > >  drivers/char/tpm/tpm2-cmd.c                 |   53 +-
> > >  drivers/char/tpm/tpm2-sessions.c            | 1280
> > > +++++++++++++++++++
> > >  drivers/char/tpm/tpm2-space.c               |   11 +-
> > >  include/crypto/aes.h                        |    5 +
> > >  include/keys/trusted_tpm.h                  |    2 -
> > >  include/linux/tpm.h                         |  316 +++--
> > >  lib/crypto/Kconfig                          |    5 +
> > >  lib/crypto/Makefile                         |    3 +
> > >  lib/crypto/aescfb.c                         |  257 ++++
> > >  security/keys/trusted-keys/trusted_tpm1.c   |   23 +-
> > >  security/keys/trusted-keys/trusted_tpm2.c   |  136 +-
> > >  19 files changed, 2443 insertions(+), 195 deletions(-)
> > >  create mode 100644 Documentation/security/tpm/tpm-security.rst
> > >  create mode 100644 drivers/char/tpm/tpm-buf.c
> > >  create mode 100644 drivers/char/tpm/tpm2-sessions.c
> > >  create mode 100644 lib/crypto/aescfb.c
> > 
> > Thanks for the update!
> > 
> > I think I asked this already earlier but unfortunately could not
> > find the corresponding email from lore.
>
> Well, you did, but at that time I didn't have the null name change
> detection so:
>
> > 
> > Anyway, I've tested this series with QEMU i.e. to the point that
> > I know that it does not break anything in the case when things are
> > working as expected.
> > 
> > What I would like to test is the negative case when the null key
> > name changes and see what happens.
> > 
> > I recall that you had some version of QEMU that had ability to test
> > this and my latest question on that was what QEMU baseline it was
> > expected to be applied over.
>
> Yes, I added patches to qemu to make it talk directly to the mssim TPM
> reference implementation
>
> https://github.com/microsoft/ms-tpm-20-ref
>
> so I could be sure I was testing against the reference implementation.
> However, they also have the advantage that you can use wireshark to
> dump the TPM transactions (ensuring encryption).  You can also tamper
> with the TPM state from the outside by connecting to the TPM socket.
>
> For the case you want, you can simulate a reset by killing and
> restarting the tpm server (you have to power it up and issue the
> startup command manually).  The next TPM command the kernel tries
> should see the null name change and react accordingly.
>
> It looks like the current qemu patches fail to apply again, so I just
> reposted them against qemu git head:
>
> https://lore.kernel.org/qemu-devel/[e-mail]/
>
> > Since I could not find the email subthread I neither have the patch
> > nor do know the baseline. So if you could help with these details
> > then we can move forward.
> > 
> > I can also work with QEMU Git fork if you have one and point out
> > QEMU_OVERRIDE_SRCDIR to the clone.
>
> I only have the patches in a local git repository, but I could push
> qemu up onto kernel.org if it would help?

That definitely does help. I can point out my build to that repository,
(or actually clone of it).

As said the "valid flow" has been tested multiple times. I guess I can
hold v6.10 PR to next week so there is still time to barely squeeze this
to v6.10.

BR, Jarkko

James Bottomley April 30, 2024, 10:31 p.m. UTC | #9

On Wed, 2024-05-01 at 00:48 +0300, Jarkko Sakkinen wrote:
> On Tue Apr 30, 2024 at 10:23 PM EEST, James Bottomley wrote:
> > On Tue, 2024-04-30 at 01:22 +0300, Jarkko Sakkinen wrote:
[...]
> > > Since I could not find the email subthread I neither have the
> > > patch nor do know the baseline. So if you could help with these
> > > details then we can move forward.
> > > 
> > > I can also work with QEMU Git fork if you have one and point out
> > > QEMU_OVERRIDE_SRCDIR to the clone.
> > 
> > I only have the patches in a local git repository, but I could push
> > qemu up onto kernel.org if it would help?
> 
> That definitely does help. I can point out my build to that
> repository, (or actually clone of it).

OK, it's the mssim branch here:

https://git.kernel.org/pub/scm/linux/kernel/git/jejb/qemu.git/log/?h=mssim

It's based on qemu head, but it works for me.

James

Jarkko Sakkinen April 30, 2024, 10:46 p.m. UTC | #10

On Wed May 1, 2024 at 1:31 AM EEST, James Bottomley wrote:
> On Wed, 2024-05-01 at 00:48 +0300, Jarkko Sakkinen wrote:
> > On Tue Apr 30, 2024 at 10:23 PM EEST, James Bottomley wrote:
> > > On Tue, 2024-04-30 at 01:22 +0300, Jarkko Sakkinen wrote:
> [...]
> > > > Since I could not find the email subthread I neither have the
> > > > patch nor do know the baseline. So if you could help with these
> > > > details then we can move forward.
> > > > 
> > > > I can also work with QEMU Git fork if you have one and point out
> > > > QEMU_OVERRIDE_SRCDIR to the clone.
> > > 
> > > I only have the patches in a local git repository, but I could push
> > > qemu up onto kernel.org if it would help?
> > 
> > That definitely does help. I can point out my build to that
> > repository, (or actually clone of it).
>
> OK, it's the mssim branch here:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/jejb/qemu.git/log/?h=mssim
>
> It's based on qemu head, but it works for me.
>
> James

OK, cool. I'll test this on Thursday as tomorrow is national holiday
in Finland. However, I'll quickly just try it in my host before going
to sleep (if there is any feedback to be given).

I happen to run Tumbleweed nowadays so I can just install ibmswtpm2
package, i.e. should take only a short while.

BR, Jarkko

Jarkko Sakkinen April 30, 2024, 11:10 p.m. UTC | #11

On Wed May 1, 2024 at 1:46 AM EEST, Jarkko Sakkinen wrote:
> On Wed May 1, 2024 at 1:31 AM EEST, James Bottomley wrote:
> > On Wed, 2024-05-01 at 00:48 +0300, Jarkko Sakkinen wrote:
> > > On Tue Apr 30, 2024 at 10:23 PM EEST, James Bottomley wrote:
> > > > On Tue, 2024-04-30 at 01:22 +0300, Jarkko Sakkinen wrote:
> > [...]
> > > > > Since I could not find the email subthread I neither have the
> > > > > patch nor do know the baseline. So if you could help with these
> > > > > details then we can move forward.
> > > > > 
> > > > > I can also work with QEMU Git fork if you have one and point out
> > > > > QEMU_OVERRIDE_SRCDIR to the clone.
> > > > 
> > > > I only have the patches in a local git repository, but I could push
> > > > qemu up onto kernel.org if it would help?
> > > 
> > > That definitely does help. I can point out my build to that
> > > repository, (or actually clone of it).
> >
> > OK, it's the mssim branch here:
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/jejb/qemu.git/log/?h=mssim
> >
> > It's based on qemu head, but it works for me.
> >
> > James
>
> OK, cool. I'll test this on Thursday as tomorrow is national holiday
> in Finland. However, I'll quickly just try it in my host before going
> to sleep (if there is any feedback to be given).
>
> I happen to run Tumbleweed nowadays so I can just install ibmswtpm2
> package, i.e. should take only a short while.

OK, so I got to the point that I can startup a VM with mssim so I'll
do the "final test" Thursday :-)

I have no time to review the qemu patches but you could add my
tested-by if you want to those.

BR, Jarkko

Jarkko Sakkinen May 3, 2024, 11:18 p.m. UTC | #12

On Wed May 1, 2024 at 1:31 AM EEST, James Bottomley wrote:
> On Wed, 2024-05-01 at 00:48 +0300, Jarkko Sakkinen wrote:
> > On Tue Apr 30, 2024 at 10:23 PM EEST, James Bottomley wrote:
> > > On Tue, 2024-04-30 at 01:22 +0300, Jarkko Sakkinen wrote:
> [...]
> > > > Since I could not find the email subthread I neither have the
> > > > patch nor do know the baseline. So if you could help with these
> > > > details then we can move forward.
> > > > 
> > > > I can also work with QEMU Git fork if you have one and point out
> > > > QEMU_OVERRIDE_SRCDIR to the clone.
> > > 
> > > I only have the patches in a local git repository, but I could push
> > > qemu up onto kernel.org if it would help?
> > 
> > That definitely does help. I can point out my build to that
> > repository, (or actually clone of it).
>
> OK, it's the mssim branch here:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/jejb/qemu.git/log/?h=mssim
>
> It's based on qemu head, but it works for me.

OK so

Reviewed-by: Jarkko Sakkinen <[e-mail]>
Tested-by: Jarkko Sakkinen <[e-mail]>

National holiday mixed up this week a bit and I've been preparing also
abstract to ethprague (https://ethprague.com/) which I need to put out
during weekend, thus the delays.

Pretty good timing BTW also considering latest developments in systemd.
Let's stay put for any emerging issues with this through the release
cycle.

If possible, CC me to QEMU patch set iterations. I'll pick these during
the weekend.

Thanks for the good work (and patience in the review cycles)! Awesome
achievement.

BR, Jarkko