bhyve: reinitialise swtpm's TPM at startup
AcceptedPublic
Actions

Authored by quentin.thebault_defenso.fr on Wed, Sep 2, 1:47 PM.

Details

Reviewers

corvink
novel
bnovkov
markj
kevans

Group Reviewers

bhyve

Summary

bhyve does not currently reinitialise swtpm on startup, which results in
PCRs being carried over across reboots. This means that any guest VM
implementing measured boot controls, such as Windows with BitLocker and
Secure Boot enabled, will consider the system compromised and refuse to
start or ask for a recovery key.

swtpm has a control channel that allows us to send commands for that
purpose: CMD_INIT shuts the TPM down and starts it again. When swptpm
has been configured with flags startup-clear, it clears the PCRs, like a
cold boot on real hardware.

Connect a second socket when the new ctrl_path option is set and send
CMD_INIT there. Without the option nothing changes.

MFC after: 2 weeks
Sponsored by: Defenso

Signed-off-by: Quentin Thébault <quentin.thebault@defenso.fr>

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 77156
Build 74039: arc lint + arc unit

Event Timeline

quentin.thebault_defenso.fr created this revision.Wed, Sep 2, 1:47 PM

Herald added a reviewer: bhyve. · View Herald TranscriptWed, Sep 2, 1:47 PM

Herald added subscribers: ziaee, bcran, rgrimes, imp. · View Herald Transcript

quentin.thebault_defenso.fr requested review of this revision.Wed, Sep 2, 1:47 PM

Harbormaster completed remote builds in B76405: Diff 185622.Wed, Sep 2, 1:47 PM

kevans added a subscriber: kevans.Wed, Sep 2, 5:59 PM

kevans added inline comments.

usr.sbin/bhyve/bhyve.8
853	New sentence must begin on a new line
usr.sbin/bhyve/tpm_emul_swtpm.c
38	This doesn't seem to need `__packed`, 2x uint32_t lines up nicely to avoid padding.
56	It's maybee worth checking for truncation here so that you can provide a better error than 'unable to conect'
66	I guess this will get optimized away anyways, but here and below: 0 is the same in every byte order, no?
72	Not so sure about `EFAULT` here? That seems like it might be misleading- maybe better to just preserve `errno` befofre `warnx` and return that?

quentin.thebault_defenso.fr marked 3 inline comments as done.Thu, Sep 3, 9:24 AM

quentin.thebault_defenso.fr added inline comments.

usr.sbin/bhyve/tpm_emul_swtpm.c
72	That is how those errors are processed in the existing `tpm_swtpm_execute_cmd` function a bit below. Should I correct both?

Remove unnecessary bits and improve error management for strlcpy and send/recv functions (incl. on existing code for harmonization).

Harbormaster completed remote builds in B76425: Diff 185701.Thu, Sep 3, 2:19 PM

quentin.thebault_defenso.fr marked 2 inline comments as done.Thu, Sep 3, 2:20 PM

quentin.thebault_defenso.fr added reviewers: novel, bnovkov, markj.Sun, Sep 20, 11:24 PM

This doesn't seem like it particularly violates any bhyve principles, so I may proceed with commtting this within the next week or so with no objection otherwise from bhyve folks

usr.sbin/bhyve/bhyve.8
861	perhaps, to drive the point home.

Explicit that the option is needed for measured boot in manpage.

Harbormaster completed remote builds in B77156: Diff 187381.Tue, Sep 22, 7:07 AM

kevans accepted this revision.Tue, Sep 22, 12:49 PM

This revision is now accepted and ready to land.Tue, Sep 22, 12:49 PM

bnovkov added inline comments.Tue, Sep 22, 1:13 PM

usr.sbin/bhyve/bhyve.8
847	Is there a reason why we shouldn't reinitialize the `swtpm` on every reboot?
usr.sbin/bhyve/tpm_emul_swtpm.c
155	This is a new configuration variable so it should be documented in `bhyve_conf(5)`.