swtpm icon indicating copy to clipboard operation
swtpm copied to clipboard

Send a TPM2_Shutdown before TPMLIB_Terminate() in case client did not send it

Open stefanberger opened this issue 2 years ago • 9 comments

If necessary send a TPM2_Shutdown() command to libtpms before processing CMD_INIT. However, this is only necessary for a TPM 2 and only if the TPM2_Shutdown command has not been sent by the client (VM TPM driver) as the last command as it should do under normal circumstances, for example upon graceful VM shutdown.

This fixes a bug where abrupt VM resets may trigger the TPM 2's dictionary attack lockout logic due to the TPM 2 not having received a TPM2_Shutdown command before it was reset using CMD_INIT for example. An OS driver is typically supposed to send a TPM2_Shutdown to the TPM 2 but an abrupt VM reset prevents it.

There are 3 control commands where this needs to be done since they call TPMLIB_Terminate():

  • CMD_STOP: This command is typically called before setting the state blobs of the TPM or before configuring the buffer size [QEMU, test cases].

  • CMD_INIT: This command is called for resetting and initializing the TPM 2.

  • CMD_SHUTDOWN: This command is called for a graceful shutdown of the TPM 2.

There are no negative side effects to be expected if TPM2_Shutdown() is sent before any of these. Also, since none of these are sent before the state of the TPM is marshalled (for migration for example) migrated state will not have a TPM2_Shutdown() applied to it (accidentally).

Edk2 sends a sequence of TPM2_Shutdown(SU_STATE) + TPM2_GetRandom() before suspend-to-ram. Upon wake up a CMD_INIT is sent to the TPM to reset it, which in this case now requires a TPM2_Shutdown(SU_STATE) to be sent to the TPM 2 so that certain TPM 2 state is available again upon resume. To avoid invaliding the SU_STATE, first send a TPM2_Shutdown(SU_STATE) in all cases and only if this fails send a TPM2_Shutdown(SU_CLEAR). This way the internal state is preserved and the VM (or user) are expected to use TPM2_Startup(SU_CLEAR) when staring up the TPM 2 and no previous state needs to be resumed.

Note: The TPM 2 spec describes the command as follows:

"This command is used to prepare the TPM for a power cycle. The shutdownType parameter indicates how the subsequent TPM2_Startup() will be processed.[...] This command saves TPM state but does not change the state other than the internal indication that the context has been saved. The TPM shall continue to accept commands. If a subsequent command changes TPM state saved by this command, then the effect of this command is nullified. The TPM MAY nullify this command for any subsequent command rather than check whether the command changed state saved by this command. If this command is nullified and if no TPM2_Shutdown() occurs before the next TPM2_Startup(), then the next TPM2_Startup() shall be TPM2_Startup(CLEAR)."

Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2087538 Signed-off-by: Stefan Berger [email protected]

stefanberger avatar May 30 '22 16:05 stefanberger

Pull Request Test Coverage Report for Build 3924

  • 52 of 53 (98.11%) changed or added relevant lines in 4 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.3%) to 75.136%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/swtpm/tpmlib.c 28 29 96.55%
<!-- Total: 52 53
Totals Coverage Status
Change from base Build 3921: 0.3%
Covered Lines: 6210
Relevant Lines: 8265

💛 - Coveralls

coveralls avatar May 30 '22 18:05 coveralls

I will remove the warning print-out. It will become a hidden feature of swtpm then to send TPM2_Shutdown when needed.

$ ./test_tpm2_probe
Need to be root to run test with CUSE interface.
Need to be root to run test with CUSE interface.
==== Starting swtpm with interfaces socket+socket ====
swtpm: Warning: Need to send TPM2_Shutdown(SU_CLEAR); previous command: 0x0000017e
OK
==== Starting swtpm with interfaces socket+unix ====
swtpm: Warning: Need to send TPM2_Shutdown(SU_CLEAR); previous command: 0x0000017e
OK
==== Starting swtpm with interfaces unix+unix ====
swtpm: Warning: Need to send TPM2_Shutdown(SU_CLEAR); previous command: 0x0000017e
OK

stefanberger avatar May 31 '22 14:05 stefanberger

Looks good otherwise. It would be nice to have a test for it, and some quote of the specification

elmarco avatar May 31 '22 15:05 elmarco

Looks good otherwise. It would be nice to have a test for it, and some quote of the specification

It will become silent. And for RHEL I would recommend taking the latest version 0.7.3 + these patches of wait for 0.7.4, but this may take a bit...

stefanberger avatar May 31 '22 15:05 stefanberger

I have looked into how to trigger the DA counter to increase upon reset but couldn't figure it out so far...

stefanberger avatar May 31 '22 15:05 stefanberger

In the end there's one concern that I have about this series and that is that we accidentally send a TPM2_Shutdown(SU_CLEAR) after a TPM2_Shutdown(SU_STATE) was issued. The relevant scenarios to trigger this are systemctl suspend and systemctl hibernate that both send a TPM2_Shutdown(SU_STATE) but then also completely shut down the VM. The BIOS upon boot then uses TPM2_Startup(SU_CLEAR) clearing the state and the TPM2_Shutdown(SU_STATE) was useless. So I am wondering whether this is normal expected behavior for an x86 VM that upon systemctl suspend [and systemctl hibernate] the VM completely shuts down?

stefanberger avatar Jun 05 '22 02:06 stefanberger

I did some more tests with suspend-to-ram (systemctl suspend) with EDK2. Here the Linux drivers sends TPM2_Shutdown(SU_STATE) and then a TPM2_GetRandom() is sent to the TPM 2 before the VM suspends. Once the VM awakes again, QEMU resets the TIS or CRB interface and swtpm is also reset via CMD_INIT (as if the whole VM was reset). We cannot simply send a TPM2_Shutdown(SU_CLEAR) before the CMD_INIT since the TPM2_Shutdown(SU_STATE) was sent but the lastCommand would be TPM2_CC_GetRandom. So, determining whether to send a TPM2_Shutdown() by looking at the lastCommand that was sent to the TPM doesn't always work and in this case it would mess up the state of the TPM.

Note: EDK2's (of QEMU 7.0) doesn't seem to have TPM support compiled in.

stefanberger avatar Jun 06 '22 02:06 stefanberger

I did some more tests with suspend-to-ram (systemctl suspend) with EDK2. Here the Linux drivers sends TPM2_Shutdown(SU_STATE) and then a TPM2_GetRandom() is sent to the TPM 2 before the VM suspends. Once the VM awakes again, QEMU resets the TIS or CRB interface and swtpm is also reset via CMD_INIT (as if the whole VM was reset). We cannot simply send a TPM2_Shutdown(SU_CLEAR) before the CMD_INIT since the TPM2_Shutdown(SU_STATE) was sent but the lastCommand would be TPM2_CC_GetRandom. So, determining whether to send a TPM2_Shutdown() by looking at the lastCommand that was sent to the TPM doesn't always work and in this case it would mess up the state of the TPM.

The solution is to send a TPM2_Shutdown(SU_STATE) in case lastCommand is not TPM2_Shutdown. If this command fails TPM2_Shutdown(SU_CLEAR) is sent. The SU_STATE stores additional state that TPM2_Startup(SU_STATE) can then restore and TPM2_Startup(SU_CLEAR) would simply ignore. Now we have to trust the BIOS to use the right command and not cause stored state to be restored unnecessarily. During normal startup the BIOS would use SU_CLEAR, so we should be fine.

stefanberger avatar Jun 06 '22 12:06 stefanberger

Once I have confirmation that this patch resolves the issue presented in the bugzilla I will merge it.

stefanberger avatar Jun 09 '22 11:06 stefanberger

I was planning to merge this later this week unless there are comments or concerns.

stefanberger avatar Aug 16 '22 13:08 stefanberger

I am wondering whether to add a flag --flags auto-shutdown to enable this feature.

stefanberger avatar Aug 17 '22 17:08 stefanberger

I am wondering whether to add a flag --flags auto-shutdown to enable this feature.

Perhaps the other way around (disable-auto-shutdown), if we believe the new behavior is a fix and it doesn't break migration etc.

elmarco avatar Aug 18 '22 06:08 elmarco

Perhaps the other way around (disable-auto-shutdown), if we believe the new behavior is a fix and it doesn't break migration etc.

I don't think it breaks anything. It rather fixes an unexpected behavior of the TPM 2 for the virtualization case though an abrupt reset on a physical machine may cause the same. I'll do the disable-auto-shutdown instead now.

stefanberger avatar Aug 18 '22 12:08 stefanberger