On 1/29/19 4:24 PM, Kevin O'Connor wrote:
On Tue, Jan 22, 2019 at 10:46:24AM -0500, Stefan Berger wrote:
Implement a TPM 2.0 menu item that allows a user to toggle the activation
of PCR banks of the TPM 2.0. After successful activation we shut down the
TPM 2.0 and reset the machine.
Thanks. In general, it looks fine to me. Just for my understanding,
could you give a high level overview of when/why a user would use this
menu and what functionality they would miss if this menu were not
available?
I'll add this to a v3: A TPM 2.0 may have multiple PCR banks, such as
for SHA1, SHA256, SHA384, SHA512, and SM3-256. One or multiple of those
banks may be active (by factory for example) and modifying the set of
active PCR banks is only possible while in the firmware since it
requires platform authorization. However, Per spec we generate a random
password for the platform authorization before we activate the boot
loader and throw that password away so that one cannot make any
modifications that require this type of authorization while running an OS.
The only other method of doing this is via the Physical Presence
Interface by posting a code (and parameter in this particular case) from
the OS. But this requires the firmware to act on it upon reboot.
I also have a few comments - see below.
[...]
--- a/src/tcgbios.c
+++ b/src/tcgbios.c
[...]
@@ -1780,6 +1926,13 @@ tpm20_process_cfg(tpm_ppi_code msgCode, int verbose)
if (!ret)
ret = tpm20_clear();
break;
+
+ case TPM_PPI_OP_SET_PCR_BANKS:
+ ret = tpm20_set_pcrbanks(pprm);
+ if (!ret)
+ ret = tpm_simple_cmd(0, TPM2_CC_Shutdown,
+ 2, TPM2_SU_CLEAR,
TPM_DURATION_TYPE_SHORT);
+ break;
}
if (ret)
[...]
static void
tpm20_menu(void)
{
int scan_code;
tpm_ppi_code msgCode;
+ u32 pprm;
+ u8 active_banks;
+ int do_reset = 0;
for (;;) {
printf("1. Clear TPM\n");
+ printf("2. Change active PCR banks\n");
printf("\nIf no change is desired or if this menu was reached by "
"mistake, press ESC to\n"
@@ -1988,11 +2204,20 @@ tpm20_menu(void)
case 2:
msgCode = TPM_PPI_OP_CLEAR;
break;
+ case 3:
+ if (tpm20_menu_change_active_pcrbanks(&active_banks) < 0)
+ continue;
+ msgCode = TPM_PPI_OP_SET_PCR_BANKS;
+ pprm = active_banks;
+ do_reset = 1;
+ break;
default:
continue;
}
- tpm20_process_cfg(msgCode, 0);
+ tpm20_process_cfg(msgCode, pprm, 0);
+ if (do_reset)
+ reset();
}
FWIW, I find the above confusing. Could
tpm20_menu_change_active_pcrbanks() just directly invoke
tpm20_set_pcrbanks() et al when needed?
It certainly could. I can change this.
--- a/src/util.h
+++ b/src/util.h
@@ -38,6 +38,8 @@ struct usbdevice_s;
int bootprio_find_usb(struct usbdevice_s *usbdev, int lun);
int get_keystroke(int msec);
+#define SCANCODE_A 0x1e
The convention for util.h is to only have forward declarations for
variables and functions - no macros, typedefs, or struct definitions.
Also, you might want take a look at possibly using ascii_to_keycode().
Ok.
Stefan
-Kevin
_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-le...@seabios.org