SeaBIOS January 2016

seabios@seabios.org

27 participants
50 discussions

Re: [SeaBIOS] SeaBIOS Digest, Vol 72, Issue 33
by Kevin O'Connor 08 Jan '16

08 Jan '16

On Fri, Jan 08, 2016 at 12:19:52PM -0500, Stefan Berger wrote: > "Kevin O'Connor" <kevin(a)koconnor.net> wrote on 01/08/2016 11:41:13 AM: > > On Thu, Jan 07, 2016 at 03:39:13PM -0500, Stefan Berger wrote: > > > "Kevin O'Connor" <kevin(a)koconnor.net> wrote on 01/07/2016 03:14:37 PM: > > > > I don't have input on what TPM2 organization should look like, > mainly > > > > because I don't know what TPM2 entails. I gather the TIS commands > are > > > > changing, but what else changes? Does the ACPI log, BIOS interface, > > > > or tpm menu change? Do you have a pointer to the TPM2 spec (when I > > > > last looked it seemed that TPM2 was still being worked on). > > > > > > The TIS got more registers; some flags allow detection of the TPM > version. > > > > > > All commands changed -- no backwards compatibility. The header > 'fields' > > > are the same, their ordinal and tag values are not. > > > > > > Spec: > > > > http://www.trustedcomputinggroup.org/resources/tpm_library_specification > > > > Thanks. Does the hardware interface change as well (ie, is it still > > the same reads/writes to MMIO at 0xfed40000)? > > > > It has the same address, but one or two more registers. Does it require a different tpm_drivers.c implementation - with something like tpmhw1_transmit() and tpmhw2_transmit() functions? > > My initial thought would be to do what you've proposed - have wrapper > > functions around the TPM commands (eg, tpm_extend, tpm_get_capability, > > read_permanent_flags) and teach those functions how to send the two > > different styles of commands (and translate the responses if > > necessary). > > So the good thing is that some of the code can be shared between 1.2 and > 2.0, > to a certain 'depth' at least. An example of a shared function would be > this one. > > static void > tpm_add_event_separators(void) > { > static const u8 evt_separator[] = {0xff,0xff,0xff,0xff}; > u32 pcrIndex; > for (pcrIndex = 0; pcrIndex <= 7; pcrIndex++) > tpm_add_measurement_to_log(pcrIndex, EV_SEPARATOR, > NULL, 0, > evt_separator, > sizeof(evt_separator)); > } > > Following this function further down: > > tpm_add_measurement_to_log() [on current master] can be completely > shared as well. tpm_log_extend_event would need to become a function that > branches into tpm12_log_extend_event and tpm2_log_extend_event, depending > on detected version of TPM. Sounds like a new tpm_extend() function could be made with just the hardware command. And then it could handle the v1 and v2 cases and thus reduce the amount of duplicated code. > tpm_log_event could again be shared since ACPI logging is the same. > Same for tpm_fill_hash for as long as we only support sha1. > > Basically all functions where commands are created cannot be shared. > Also TPM 2's initialization is a bit different and it supports more > hashes. If the init is notable different maybe just do tpm1_startup() and tpm2_startup()? > So it actually speaks against splitting this up into different files, but > the > outcome may be that the code would show a mix of tpm12_*, tpm2_*, and > tpm_* functions in the format of > > tpm12_foo() { [...] } > > tpm2_foo() { [...] } > > tpm_foo() { > switch (tpmversion) { > 1.2: > return tpm12_foo() > 2: > return tpm2_foo() > } > } Okay. But, I would say that if both tpm12_foo() and tpm2_foo() are both only a few lines that it might be better to just inline it all into tpm_foo(). An 'if' might be more succinct also - hopefully there isn't a tpm3 in the works.. > tpm_xyz() { [...] } > > tpm12_bar() { [...] } > > tpm2_bar() { [...] } > > [...] > > That's what I did before... Oh, were there tpm2 patches available? I must have missed them. > If none of the code could be shared the decision to split it up completely > would be a lot easier. Agreed. -Kevin

2 1

Re: [SeaBIOS] SeaBIOS Digest, Vol 72, Issue 33
by Kevin O'Connor 08 Jan '16

08 Jan '16

On Thu, Jan 07, 2016 at 03:39:13PM -0500, Stefan Berger wrote: > "Kevin O'Connor" <kevin(a)koconnor.net> wrote on 01/07/2016 03:14:37 PM: > > I don't have input on what TPM2 organization should look like, mainly > > because I don't know what TPM2 entails. I gather the TIS commands are > > changing, but what else changes? Does the ACPI log, BIOS interface, > > or tpm menu change? Do you have a pointer to the TPM2 spec (when I > > last looked it seemed that TPM2 was still being worked on). > > The TIS got more registers; some flags allow detection of the TPM version. > > All commands changed -- no backwards compatibility. The header 'fields' > are the same, their ordinal and tag values are not. > > Spec: > http://www.trustedcomputinggroup.org/resources/tpm_library_specification Thanks. Does the hardware interface change as well (ie, is it still the same reads/writes to MMIO at 0xfed40000)? My initial thought would be to do what you've proposed - have wrapper functions around the TPM commands (eg, tpm_extend, tpm_get_capability, read_permanent_flags) and teach those functions how to send the two different styles of commands (and translate the responses if necessary). -Kevin

2 1

Re: [SeaBIOS] SeaBIOS Digest, Vol 72, Issue 33
by Kevin O'Connor 08 Jan '16

08 Jan '16

On Thu, Jan 07, 2016 at 05:08:53PM -0500, Stefan Berger wrote: > "Kevin O'Connor" <kevin(a)koconnor.net> wrote on 01/07/2016 03:14:37 PM: > > On Thu, Jan 07, 2016 at 01:57:42PM -0500, Stefan Berger wrote: > > > At some point I'll bring the TPM 2 patches back to life following the > many > > > recent changes... Now my question is how to organize the code. Should > there > > > be one file where we essentially have the above type of code branching > into > > > TPM 1.2 & TPM 2 specific functions and TPM 1.2 and TPM 2 code in > separate > > > files or the above type of functions at the end of the current file > and then > > > a section with TPM 1.2 code and another with TPM 2? It's a few patches > > > building on top of each other, so want to avoid churn... > > > > I was playing with a few additional patches on top of your last series > > - I pushed them to my testing branch if you want to take a look. > > > I found this: > if (hashdata || hashdata_length) > > it should probably be: > if (hashdata && hashdata_length) > > But now I am arguing against the spec. They want 0 bytes to be 'measured'? I interpreted the spec as saying it should be possible to measure the memory at physical address zero (which is the interrupt descriptor table when in real mode). However, it does seem a bit silly. > I would also shut down the TPM if an extend didn't work; after all > the TPM will 'quote' the state of the PCRs in an attestation. By > temporarily deactivating it, it will not be able to quote > anymore. Though that type of failure should never occur. Okay. > I am not sure about a failed getcapability whether it's right or > wrong to shut down the TPM. Basically this should never happen, > either. I would tend to shut it down based on the fact that > something is wrong with the device. Okay. -Kevin

1 0

Re: [SeaBIOS] SeaBIOS Digest, Vol 72, Issue 33
by Kevin O'Connor 08 Jan '16

08 Jan '16

On Mon, Dec 21, 2015 at 11:50:07AM -0500, Stefan Berger wrote: > "Kevin O'Connor" <kevin(a)koconnor.net> wrote on 12/17/2015 05:22:56 PM: > > > > > On Mon, Nov 30, 2015 at 11:32:05AM +0000, Wim Vervoorn wrote: > > > Hello, > > > > > > I noticed that a lot of work is going on for the TPM support in > SeaBIOS. > > > > > > All of this work is TPM 1.2 based. I was wondering if there are any > > > plans to support TPM 2.0 in the future. > > > > I'm not aware of any plans. > > We're working on it... > > > So maybe you have some comments to the following: > > There will be a patch for probing the TPM TIS hardware interface for > whether there's a TPM 1.2 or a TPM 2. > We then have a patch for prefixing all TPM 1.2 functions with tpm12_ and > then introduce functions like these ones here: > > static ... tpm12_foo() { ... } > static ... tpm2_foo() { ... } > > tpm_foo() > { > [...] > > switch (tpmversion) { > case TPM_VERSION_1_2: > tpm12_foo() > break; > case TPM_VERSION_2: > tpm2_foo(); > break; > } > > [...] > } Is the difference between 1.2 and 2.0 so large that the above is needed? -Kevin

4 6

[PATCH v2 0/6] Fixes for physical presence and API
by Stefan Berger 07 Jan '16

07 Jan '16

Patch 1 fixes the return of a missing digest in the HashLogExtendEvent command. Beyond thad that, the patches extend the handling of physical presence to HW physical presence and rework the showing of the TPM menu to depend on whether SW physical presence could be asserted or HW physical presence has been found to be enabled. Regards, Stefan Stefan Berger (6): Copy digest into HashLogExentEvent response Move assert_physical_presence and dependencies Add support for harware physical presence Rework the assertion of physical presence Remove usage of PP_CMD_ENABLE from all but one place Do not set TPM in failure mode if menu command fails src/boot.c | 2 +- src/std/tcg.h | 1 + src/tcgbios.c | 227 ++++++++++++++++++++++++---------------------------------- src/tcgbios.h | 1 + 4 files changed, 98 insertions(+), 133 deletions(-) -- 2.4.3

2 11

[PATCH] usb: Remove usbdev->slotid field
by Kevin O'Connor 07 Jan '16

07 Jan '16

The usbdev->slotid field is xhci specific and on xhci it is always reachable from usbdev->defpipe->slotid. Signed-off-by: Kevin O'Connor <kevin(a)koconnor.net> --- src/hw/usb-xhci.c | 6 ++++-- src/hw/usb.h | 1 - 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/hw/usb-xhci.c b/src/hw/usb-xhci.c index 3359cff..089cae7 100644 --- a/src/hw/usb-xhci.c +++ b/src/hw/usb-xhci.c @@ -1006,9 +1006,11 @@ xhci_alloc_pipe(struct usbdevice_s *usbdev free(dev); goto fail; } - pipe->slotid = usbdev->slotid = slotid; + pipe->slotid = slotid; } else { - pipe->slotid = usbdev->slotid; + struct xhci_pipe *defpipe = container_of( + usbdev->defpipe, struct xhci_pipe, pipe); + pipe->slotid = defpipe->slotid; // Send configure command. int cc = xhci_cmd_configure_endpoint(xhci, pipe->slotid, in); if (cc != CC_SUCCESS) { diff --git a/src/hw/usb.h b/src/hw/usb.h index efb5e6f..94e12b2 100644 --- a/src/hw/usb.h +++ b/src/hw/usb.h @@ -22,7 +22,6 @@ struct usb_pipe { struct usbdevice_s { struct usbhub_s *hub; struct usb_pipe *defpipe; - u32 slotid; u32 port; struct usb_config_descriptor *config; struct usb_interface_descriptor *iface; -- 2.5.0

1 1

[PATCH 0/5] Fixes for physical presence and API
by Stefan Berger 06 Jan '16

06 Jan '16

Patch 1 fixes the return of a missing digest in the HashLogExtendEvent command. Beyond thad that, the patches extend the handling of physical presence to HW physical presence and rework the showing of the TPM menu to depend on whether SW physical presence could be asserted or HW physical presence has been found to be asserted. Regards, Stefan Stefan Berger (5): Copy digest into HashLogExentEvent response Move assert_physical_presence and dependencies Add support for harware physical presence Rework the assertion of physical presence Give up physical presence when setting TPM into failure mode src/boot.c | 2 +- src/std/tcg.h | 2 + src/tcgbios.c | 242 +++++++++++++++++++++++++++++++++++++--------------------- src/tcgbios.h | 1 + 4 files changed, 157 insertions(+), 90 deletions(-) -- 2.4.3

2 9

Re: [SeaBIOS] [PATCH 00/10] Some TPM simplifications
by Kevin O'Connor 06 Jan '16

06 Jan '16

On Wed, Jan 06, 2016 at 11:55:19AM -0500, Stefan Berger wrote: > "Kevin O'Connor" <kevin(a)koconnor.net> wrote on 01/05/2016 11:03:00 PM: > > On Tue, Jan 05, 2016 at 10:07:54PM -0500, Stefan Berger wrote: > > > "Kevin O'Connor" <kevin(a)koconnor.net> wrote on 01/05/2016 08:55:51 PM: > > > > Then it sounds like the only time we need to call tpm_set_failure is > > > > on a failure of a TPM_ORD_Extend command. It might also make sense > to > > > > deactivate the TPM if we detect the hardware but don't have the acpi > > > > tables present. > > > > > > I would also deactivate it if it returned an error to > > > TPM_ORD_Startup, TPM_ORD_SelfTestFull. Since the menu is written in > > > such a way that the user only has the choices that are valid for the > > > current state, also those commands have to work, unless the TPM is > > > defective. Or is that too strict? > > > > Attempting to deactivate if TPM_ORD_Startup or TPM_ORD_SelfTestFull > > fail makes sense. > > > > I wonder if the code could attempt to assert physical presence in > > tpm_startup() and only enable the tpm menu if that succeeds. > > There are two ways to assert physical presence, one is via software, the > other via hardware. I should have said, "I wonder if the code could check if software physical presence has been enabled in tpm_startup() and only enable the tpm menu if that succeeds". That is, could we issue a TPM_ORD_PhysicalPresence(NOT_PRESENT) and if it succeeds then we enable the menu and make sure we always issue a LOCK before booting. If the command fails then the code assumes it has no requirements wrt physical presence. If that's too restrictive, could the startup code query if either CMD_ENABLE has already been set or if LifetimeLock has not been set? > For hardware assertion there's a PIN on the chip that indicates the > state of a dip switch for example. Problem is, this assertion cannot > easily be read as a flag. We have to infer this via a command. So > the trick seems to be to send TPM_PhysicalEnable/TPM_PhysicalDisable > with the value that's already there. Is there any BIOS requirements when hardware physical presence is in use though? If the hardware physical presence is set, then the BIOS can't limit the owner/activate/enable commands. So, why bother with the tpm menu - the user could just administer those settings from the OS. -Kevin

2 1

Re: [SeaBIOS] [PATCH 00/10] Some TPM simplifications
by Kevin O'Connor 06 Jan '16

06 Jan '16

On Tue, Jan 05, 2016 at 10:07:54PM -0500, Stefan Berger wrote: > "Kevin O'Connor" <kevin(a)koconnor.net> wrote on 01/05/2016 08:55:51 PM: > > Then it sounds like the only time we need to call tpm_set_failure is > > on a failure of a TPM_ORD_Extend command. It might also make sense to > > deactivate the TPM if we detect the hardware but don't have the acpi > > tables present. > > I would also deactivate it if it returned an error to > TPM_ORD_Startup, TPM_ORD_SelfTestFull. Since the menu is written in > such a way that the user only has the choices that are valid for the > current state, also those commands have to work, unless the TPM is > defective. Or is that too strict? Attempting to deactivate if TPM_ORD_Startup or TPM_ORD_SelfTestFull fail makes sense. I wonder if the code could attempt to assert physical presence in tpm_startup() and only enable the tpm menu if that succeeds. BTW, if I'm reading the specs correctly, CMD_ENABLE is likely to fail on real hardware as manufacturers are supposed to set LifetimeLock at the factory. It also appears that CMD_ENABLE alters non volatile memory, so writing it on every boot may cause wear on the device? -Kevin

2 1

Re: [SeaBIOS] [PATCH 00/10] Some TPM simplifications
by Kevin O'Connor 06 Jan '16

06 Jan '16

On Tue, Jan 05, 2016 at 05:05:20PM -0500, Stefan Berger wrote: > "Kevin O'Connor" <kevin(a)koconnor.net> wrote on 01/05/2016 03:35:35 PM: [...] > > - Take "measurements" during the boot process so that later on users > > can verify if some low-level code has changed (and thus attempt to > > identify if malicious code may have been inserted into the > > firmware). > > > > - The major requirement here seems to be that if we can't take a > > measurement that we either "cap" measurements or shutdown the TPM. > > If we don't do this, it opens the possibility of a malicious > > program forging measurements. > > That's correct. We don't cap the measurements but try to temporarily > deactivate the TPM until the next reboot. > > > > > - It's also useful to skip taking measurements if the TPM device is > > not present so that we don't waste CPU time taking measurements > > that will never be used. > > Correct. Then it sounds like the only time we need to call tpm_set_failure is on a failure of a TPM_ORD_Extend command. It might also make sense to deactivate the TPM if we detect the hardware but don't have the acpi tables present. > > - Implement physical presence capability so that critical settings in > > the TPM can only be changed by someone that can prove they are > > physically present at the hardware (and thus attempt to prevent > > malicious code that temporarily obtains escalated privileges from > > altering these important settings). > > > > - The major requirement here seems to be that the "physical > > presence" lock always be set prior to starting the boot loader. > > Yes, we have to give up physical presence and lock that. Does deactivating also lock the physical presence? The tpm_prepboot() code may not run if the tpm is in a "not working" state. -Kevin

2 1

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

SeaBIOS January 2016